diff options
author | Simon Knox <simon@gitlab.com> | 2017-09-16 01:05:52 +0000 |
---|---|---|
committer | Simon Knox <simon@gitlab.com> | 2017-09-16 01:05:52 +0000 |
commit | 03b2c069e56d2957c9d49bd837cecab11f3f96d7 (patch) | |
tree | efe4827a182bc843d6698d6ca3c8a2432fa57044 | |
parent | 46395b78caab1f79ea7014e1266d7a76b79a57ac (diff) | |
parent | c2caf85f85dbed2d2d57d56bf1a29e84355ad9e4 (diff) | |
download | gitlab-ce-03b2c069e56d2957c9d49bd837cecab11f3f96d7.tar.gz |
Merge branch '9-5-stable' into '9-5-stable-patch-5'
# Conflicts:
# db/schema.rb
-rw-r--r-- | app/models/project.rb | 4 | ||||
-rw-r--r-- | app/models/project_feature.rb | 2 | ||||
-rw-r--r-- | app/services/projects/update_service.rb | 5 | ||||
-rw-r--r-- | changelogs/unreleased/issue_37640.yml | 6 | ||||
-rw-r--r-- | db/post_migrate/20170913180600_fix_projects_without_project_feature.rb | 33 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | spec/services/projects/update_service_spec.rb | 21 |
7 files changed, 67 insertions, 6 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 36988217678..c80e706234a 100644 --- a/app/models/project.rb +++ b/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 diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index c8fabb16dc1..9ab69295f68 100644 --- a/app/models/project_feature.rb +++ b/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 diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index ca02b456ae5..876fc78310b 100644 --- a/app/services/projects/update_service.rb +++ b/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 diff --git a/changelogs/unreleased/issue_37640.yml b/changelogs/unreleased/issue_37640.yml new file mode 100644 index 00000000000..d806ed64bed --- /dev/null +++ b/changelogs/unreleased/issue_37640.yml @@ -0,0 +1,6 @@ +--- +title: Fix project feature being deleted when updating project with invalid visibility + level +merge_request: +author: +type: fixed diff --git a/db/post_migrate/20170913180600_fix_projects_without_project_feature.rb b/db/post_migrate/20170913180600_fix_projects_without_project_feature.rb new file mode 100644 index 00000000000..bfa9ad80c7d --- /dev/null +++ b/db/post_migrate/20170913180600_fix_projects_without_project_feature.rb @@ -0,0 +1,33 @@ +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 diff --git a/db/schema.rb b/db/schema.rb index 71efe05c659..af282fa4bf6 100644 --- a/db/schema.rb +++ b/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: 20170831195038) do +ActiveRecord::Schema.define(version: 20170913180600) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 3036ced3ef8..faf24cc9ca4 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/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 |