diff --git a/app/models/project.rb b/app/models/project.rb index ec40def6fb1307ab9fc2f8df09a8ecbd8f4c1ea6..94a6f3ba79980cf1d00f4d59b1875cc28ab841f6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -130,7 +130,7 @@ class Project < ActiveRecord::Base has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :protected_branches, dependent: :destroy - has_many :project_authorizations, dependent: :destroy + has_many :project_authorizations has_many :authorized_users, through: :project_authorizations, source: :user, class_name: 'User' has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source alias_method :members, :project_members diff --git a/app/models/user.rb b/app/models/user.rb index 66a768d54bb14615fa2a0cf5611d0fee8c152755..06dd98a318858f85a3a845a73874efa0b99cd3fa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -73,7 +73,7 @@ class User < ActiveRecord::Base has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy has_many :starred_projects, through: :users_star_projects, source: :project - has_many :project_authorizations, dependent: :destroy + has_many :project_authorizations has_many :authorized_projects, through: :project_authorizations, source: :project has_many :snippets, dependent: :destroy, foreign_key: :author_id @@ -444,7 +444,7 @@ class User < ActiveRecord::Base end def remove_project_authorizations(project_ids) - project_authorizations.where(id: project_ids).delete_all + project_authorizations.where(project_id: project_ids).delete_all end def set_authorized_projects_column diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 8559908e0c333e0ba0d1959bd10ffe962ca20ad6..21ec1bd9e6554cffa24b29a6c943df384c5ec048 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -35,7 +35,7 @@ module Users # rows not in the new list or with a different access level should be # removed. if !fresh[project_id] || fresh[project_id] != row.access_level - array << row.id + array << row.project_id end end @@ -100,7 +100,7 @@ module Users end def current_authorizations - user.project_authorizations.select(:id, :project_id, :access_level) + user.project_authorizations.select(:project_id, :access_level) end def fresh_authorizations diff --git a/changelogs/unreleased/remove-project-authorizations-id-column.yml b/changelogs/unreleased/remove-project-authorizations-id-column.yml new file mode 100644 index 0000000000000000000000000000000000000000..24c86f0fb1bd5ac99d917579de1bb6395c499177 --- /dev/null +++ b/changelogs/unreleased/remove-project-authorizations-id-column.yml @@ -0,0 +1,4 @@ +--- +title: Remove the project_authorizations.id column +merge_request: +author: diff --git a/db/post_migrate/20170106172224_remove_project_authorizations_id_column.rb b/db/post_migrate/20170106172224_remove_project_authorizations_id_column.rb new file mode 100644 index 0000000000000000000000000000000000000000..7c788160022907bc647d1bedadcab22454fdecb5 --- /dev/null +++ b/db/post_migrate/20170106172224_remove_project_authorizations_id_column.rb @@ -0,0 +1,12 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveProjectAuthorizationsIdColumn < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + remove_column :project_authorizations, :id, :primary_key + end +end diff --git a/db/schema.rb b/db/schema.rb index 9bce3b82d8256675dded6326c6e28bf1a930856d..f3bf7ced3930ef224229f4b9ee80e0cc199b432d 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: 20161227192806) do +ActiveRecord::Schema.define(version: 20170106172224) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -862,7 +862,7 @@ ActiveRecord::Schema.define(version: 20161227192806) do add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree add_index "personal_access_tokens", ["user_id"], name: "index_personal_access_tokens_on_user_id", using: :btree - create_table "project_authorizations", force: :cascade do |t| + create_table "project_authorizations", id: false, force: :cascade do |t| t.integer "user_id" t.integer "project_id" t.integer "access_level" @@ -1307,4 +1307,4 @@ ActiveRecord::Schema.define(version: 20161227192806) do add_foreign_key "subscriptions", "projects", on_delete: :cascade add_foreign_key "trending_projects", "projects", on_delete: :cascade add_foreign_key "u2f_registrations", "users" -end \ No newline at end of file +end diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index 1f6919151dee2c5d4934bf50a7f2afafdb15f0da..9fbb61565e35a32638192bf11564945d49ed81b2 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -20,7 +20,7 @@ describe Users::RefreshAuthorizedProjectsService do to_remove = create_authorization(project2, user) expect(service).to receive(:update_with_lease). - with([to_remove.id], [[user.id, project.id, Gitlab::Access::MASTER]]) + with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]]) service.execute end @@ -29,7 +29,7 @@ describe Users::RefreshAuthorizedProjectsService do to_remove = create_authorization(project, user, Gitlab::Access::DEVELOPER) expect(service).to receive(:update_with_lease). - with([to_remove.id], [[user.id, project.id, Gitlab::Access::MASTER]]) + with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]]) service.execute end @@ -90,7 +90,7 @@ describe Users::RefreshAuthorizedProjectsService do it 'removes authorizations that should be removed' do authorization = create_authorization(project, user) - service.update_authorizations([authorization.id]) + service.update_authorizations([authorization.project_id]) expect(user.project_authorizations).to be_empty end @@ -147,7 +147,12 @@ describe Users::RefreshAuthorizedProjectsService do end it 'sets the values to the project authorization rows' do - expect(hash.values).to eq([ProjectAuthorization.first]) + expect(hash.values.length).to eq(1) + + value = hash.values[0] + + expect(value.project_id).to eq(project.id) + expect(value.access_level).to eq(Gitlab::Access::MASTER) end end @@ -167,10 +172,6 @@ describe Users::RefreshAuthorizedProjectsService do expect(service.current_authorizations.length).to eq(1) end - it 'includes the row ID for every row' do - expect(row.id).to be_a_kind_of(Numeric) - end - it 'includes the project ID for every row' do expect(row.project_id).to eq(project.id) end