Merge branch 'issue_37640_9_5_stable' into '9-5-stable'

Fix project feature being deleted when updating project with invalid visibility level

See merge request gitlab-org/gitlab-ce!14235

app/models/project.rb

@@ -165,7 +165,7 @@ class Project < ActiveRecord::Base
   has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
   has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true
-  has_one :project_feature
+  has_one :project_feature, inverse_of: :project
   has_one :statistics, class_name: 'ProjectStatistics'
 
   # Container repositories need to remove data from the container registry,
@@ -192,7 +192,7 @@ class Project < ActiveRecord::Base
   has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
 
   accepts_nested_attributes_for :variables, allow_destroy: true
-  accepts_nested_attributes_for :project_feature
+  accepts_nested_attributes_for :project_feature, update_only: true
   accepts_nested_attributes_for :import_data
 
   delegate :name, to: :owner, allow_nil: true, prefix: true

app/models/project_feature.rb

@@ -41,6 +41,8 @@ class ProjectFeature < ActiveRecord::Base
   # http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to
   belongs_to :project, -> { unscope(where: :pending_delete) }
 
+  validates :project, presence: true
+
   validate :repository_children_level
 
   default_value_for :builds_access_level,         value: ENABLED, allows_nil: false

app/services/projects/update_service.rb

@@ -28,7 +28,10 @@ module Projects
 
         success
       else
-        error('Project could not be updated!')
+        model_errors = project.errors.full_messages.to_sentence
+        error_message = model_errors.presence || 'Project could not be updated!'
+
+        error(error_message)
       end
     end
 

changelogs/unreleased/issue_37640.yml

+---
+title: Fix project feature being deleted when updating project with invalid visibility
+  level
+merge_request:
+author:
+type: fixed

db/post_migrate/20170913180600_fix_projects_without_project_feature.rb

+class FixProjectsWithoutProjectFeature < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def up
+    # Deletes corrupted project features
+    sql = "DELETE FROM project_features WHERE project_id IS NULL"
+    execute(sql)
+
+    # Creates missing project features with private visibility
+    sql =
+      %Q{
+        INSERT INTO project_features(project_id, repository_access_level, issues_access_level, merge_requests_access_level, wiki_access_level,
+        builds_access_level, snippets_access_level, created_at, updated_at)
+          SELECT projects.id as project_id,
+          10 as repository_access_level,
+          10 as issues_access_level,
+          10 as merge_requests_access_level,
+          10 as wiki_access_level,
+          10 as builds_access_level ,
+          10 as snippets_access_level,
+          projects.created_at,
+          projects.updated_at
+          FROM projects
+          LEFT OUTER JOIN project_features ON project_features.project_id = projects.id
+          WHERE (project_features.id IS NULL)
+      }
+
+    execute(sql)
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170824162758) do
+ActiveRecord::Schema.define(version: 20170913180600) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

spec/services/projects/update_service_spec.rb

@@ -57,6 +57,21 @@ describe Projects::UpdateService, '#execute' do
         end
       end
     end
+
+    context 'when project visibility is higher than parent group' do
+      let(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
+
+      before do
+        project.update(namespace: group, visibility_level: group.visibility_level)
+      end
+
+      it 'does not update project visibility level' do
+        result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+        expect(result).to eq({ status: :error, message: 'Visibility level public is not allowed in a internal group.' })
+        expect(project.reload).to be_internal
+      end
+    end
   end
 
   describe 'when updating project that has forks' do
@@ -151,8 +166,10 @@ describe Projects::UpdateService, '#execute' do
     it 'returns an error result when record cannot be updated' do
       result = update_project(project, admin, { name: 'foo&bar' })
 
-      expect(result).to eq({ status: :error,
-                             message: 'Project could not be updated!' })
+      expect(result).to eq({
+        status: :error,
+        message: "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'."
+      })
     end
   end