From 4697fd8b81a2c4bc9b7d1e3bc2d4ec982a12367a Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 20 Mar 2017 20:03:36 +0200 Subject: [PATCH 1/2] [Multiple essignee] Add migrations && model[ci skip] --- app/models/concerns/issuable.rb | 2 +- .../20170320171632_create_assignees_table.rb | 33 ++++++++++++++ .../20170320173259_migrate_assignees.rb | 43 +++++++++++++++++++ db/schema.rb | 22 +++++++--- 4 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20170320171632_create_assignees_table.rb create mode 100644 db/migrate/20170320173259_migrate_assignees.rb diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 810b39d24e1f58..d121e2bf8af080 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -25,7 +25,7 @@ module Issuable cache_markdown_field :description belongs_to :author, class_name: "User" - belongs_to :assignee, class_name: "User" + has_and_belongs_to_many :assignees, class_name: "User", join_table: :issuable_assignees belongs_to :updated_by, class_name: "User" belongs_to :milestone has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do diff --git a/db/migrate/20170320171632_create_assignees_table.rb b/db/migrate/20170320171632_create_assignees_table.rb new file mode 100644 index 00000000000000..94cd3c2eaab23f --- /dev/null +++ b/db/migrate/20170320171632_create_assignees_table.rb @@ -0,0 +1,33 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateAssigneesTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :issuable_assignees do |t| + t.references :user, foreign_key: true, index: true + t.references :issue, foreign_key: true, index: true + t.references :merge_request, foreign_key: true, index: true + end + end +end diff --git a/db/migrate/20170320173259_migrate_assignees.rb b/db/migrate/20170320173259_migrate_assignees.rb new file mode 100644 index 00000000000000..f56bcfef69ee22 --- /dev/null +++ b/db/migrate/20170320173259_migrate_assignees.rb @@ -0,0 +1,43 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateAssignees < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def up + execute <<-EOF + INSERT INTO issuable_assignees(issue_id, user_id) + SELECT id, assignee_id FROM issues WHERE assignee_id IS NOT NULL + EOF + + execute <<-EOF + INSERT INTO issuable_assignees(merge_request_id, user_id) + SELECT id, assignee_id FROM merge_requests WHERE assignee_id IS NOT NULL + EOF + end + + def down + execute <<-EOF + DELETE FROM issuable_assignees + EOF + end +end diff --git a/db/schema.rb b/db/schema.rb index 3186055c75c2f0..b17e2cefedbeef 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: 20170313133418) do +ActiveRecord::Schema.define(version: 20170320173259) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -120,14 +120,14 @@ t.integer "terminal_max_session_time", default: 0, null: false t.integer "minimum_mirror_sync_time", default: 15, null: false t.string "default_artifacts_expire_in", default: "0", null: false + t.integer "unique_ips_limit_per_user" + t.integer "unique_ips_limit_time_window" + t.boolean "unique_ips_limit_enabled", default: false, null: false t.string "elasticsearch_url", default: "http://localhost:9200" t.boolean "elasticsearch_aws", default: false, null: false t.string "elasticsearch_aws_region", default: "us-east-1" t.string "elasticsearch_aws_access_key" t.string "elasticsearch_aws_secret_access_key" - t.integer "unique_ips_limit_per_user" - t.integer "unique_ips_limit_time_window" - t.boolean "unique_ips_limit_enabled", default: false, null: false t.integer "geo_status_timeout", default: 10 end @@ -495,6 +495,16 @@ add_index "index_statuses", ["project_id"], name: "index_index_statuses_on_project_id", unique: true, using: :btree + create_table "issuable_assignees", force: :cascade do |t| + t.integer "user_id" + t.integer "issue_id" + t.integer "merge_request_id" + end + + add_index "issuable_assignees", ["issue_id"], name: "index_issuable_assignees_on_issue_id", using: :btree + add_index "issuable_assignees", ["merge_request_id"], name: "index_issuable_assignees_on_merge_request_id", using: :btree + add_index "issuable_assignees", ["user_id"], name: "index_issuable_assignees_on_user_id", using: :btree + create_table "issue_metrics", force: :cascade do |t| t.integer "issue_id", null: false t.datetime "first_mentioned_in_commit_at" @@ -1432,7 +1442,6 @@ t.string "organization" t.boolean "authorized_projects_populated" t.boolean "auditor", default: false, null: false - t.boolean "notified_of_own_activity", default: false, null: false t.boolean "ghost" end @@ -1488,6 +1497,9 @@ add_foreign_key "boards", "projects" add_foreign_key "chat_teams", "namespaces", on_delete: :cascade add_foreign_key "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade + add_foreign_key "issuable_assignees", "issues" + add_foreign_key "issuable_assignees", "merge_requests" + add_foreign_key "issuable_assignees", "users" add_foreign_key "issue_metrics", "issues", on_delete: :cascade add_foreign_key "label_priorities", "labels", on_delete: :cascade add_foreign_key "label_priorities", "projects", on_delete: :cascade -- GitLab From 8cf25e13d3626334653a918d2eb1185d2c8c4b23 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 21 Mar 2017 17:09:44 +0200 Subject: [PATCH 2/2] [Multiple essignees] Fix for merge request and issues pages[ci skip] --- .../concerns/issuable_collections.rb | 4 ++-- app/models/concerns/issuable.rb | 8 +++++-- app/models/concerns/participable.rb | 2 +- app/serializers/issuable_entity.rb | 2 +- app/views/projects/issues/_issue.html.haml | 5 +++-- .../merge_requests/_merge_request.html.haml | 5 +++-- app/views/shared/issuable/_sidebar.html.haml | 22 ++++++++++--------- 7 files changed, 28 insertions(+), 20 deletions(-) diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 85ae4985e58cf2..621c3c8afec485 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -40,11 +40,11 @@ def issuable_meta_data(issuable_collection, collection_type) end def issues_collection - issues_finder.execute.preload(:project, :author, :assignee, :labels, :milestone, project: :namespace) + issues_finder.execute.preload(:project, :author, :assignees, :labels, :milestone, project: :namespace) end def merge_requests_collection - merge_requests_finder.execute.preload(:source_project, :target_project, :author, :assignee, :labels, :milestone, :merge_request_diff, target_project: :namespace) + merge_requests_finder.execute.preload(:source_project, :target_project, :author, :assignees, :labels, :milestone, :merge_request_diff, target_project: :namespace) end def issues_finder diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index d121e2bf8af080..55409531911d60 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -91,7 +91,7 @@ def award_emojis_loaded? attr_mentionable :description participant :author - participant :assignee + participant :assignees participant :notes_with_associations strip_attributes :title @@ -309,11 +309,15 @@ def human_class_name @human_class_name ||= self.class.name.titleize.downcase end + def assignee_list + assignees.pluck(:name).join(', ') if assignees.any? + end + # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { 'Author' => author.try(:name), - 'Assignee' => assignee.try(:name) + 'Assignees' => assignees.pluck(:name).join(', ') } end diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb index 4865c0a14b1721..fd7be269e441f0 100644 --- a/app/models/concerns/participable.rb +++ b/app/models/concerns/participable.rb @@ -11,7 +11,7 @@ # # ... # # participant :author -# participant :assignee +# participant :assignees # participant :notes # # participant -> (current_user, ext) do diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb index 29aecb50849fce..55df99a414f700 100644 --- a/app/serializers/issuable_entity.rb +++ b/app/serializers/issuable_entity.rb @@ -1,7 +1,7 @@ class IssuableEntity < Grape::Entity expose :id expose :iid - expose :assignee_id + expose :assignee_ids expose :author_id expose :description expose :lock_version diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 74befc5f2e154e..5f142d16210c7d 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -13,9 +13,10 @@ %li CLOSED - - if issue.assignee + - if issue.assignees.any? %li - = link_to_member(@project, issue.assignee, name: false, title: "Assigned to :name") + - issue.assignees.each do |assignee| + = link_to_member(@project, assignee, name: false, title: "Assigned to :name") = render 'shared/issuable_meta_data', issuable: issue diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 11b7aaec70441c..ad0d11398b5270 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -25,9 +25,10 @@ = link_to merge_request_path(merge_request), class: "has-tooltip", title: "Cannot be merged automatically", data: { container: 'body' } do = icon('exclamation-triangle') - - if merge_request.assignee + - if merge_request.assignees.any? %li - = link_to_member(merge_request.source_project, merge_request.assignee, name: false, title: "Assigned to :name") + - merge_request.assignees.each do |assignee| + = link_to_member(merge_request.source_project, assignee, name: false, title: "Assigned to :name") = render 'shared/issuable_meta_data', issuable: merge_request diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index d8e79d62e8af45..ba067bd9ea74f4 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -23,9 +23,10 @@ = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, format: :json, html: { class: 'issuable-context-form inline-update js-issuable-update' } do |f| .block.assignee - .sidebar-collapsed-icon.sidebar-collapsed-user{ data: { toggle: "tooltip", placement: "left", container: "body" }, title: (issuable.assignee.name if issuable.assignee) } - - if issuable.assignee - = link_to_member(@project, issuable.assignee, size: 24) + .sidebar-collapsed-icon.sidebar-collapsed-user{ data: { toggle: "tooltip", placement: "left", container: "body" }, title: (issuable.assignee_list) } + - if issuable.assignees.any? + - issuable.assignees.each do |assignee| + = link_to_member(@project, assignee, size: 24) - else = icon('user', 'aria-hidden': 'true') .title.hide-collapsed @@ -34,13 +35,14 @@ - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.hide-collapsed - - if issuable.assignee - = link_to_member(@project, issuable.assignee, size: 32, extra_class: 'bold') do - - if issuable.instance_of?(MergeRequest) && !issuable.can_be_merged_by?(issuable.assignee) - %span.pull-right.cannot-be-merged{ data: { toggle: 'tooltip', placement: 'left' }, title: 'Not allowed to merge' } - = icon('exclamation-triangle', 'aria-hidden': 'true') - %span.username - = issuable.assignee.to_reference + - if issuable.assignees.any? + - issuable.assignees.each do |assignee| + = link_to_member(@project, assignee, size: 32, extra_class: 'bold') do + - if issuable.instance_of?(MergeRequest) && !issuable.can_be_merged_by?(assignee) + %span.pull-right.cannot-be-merged{ data: { toggle: 'tooltip', placement: 'left' }, title: 'Not allowed to merge' } + = icon('exclamation-triangle', 'aria-hidden': 'true') + %span.username + = assignee.to_reference - else %span.assign-yourself.no-value No assignee -- GitLab