From 387c4b2c21a44360386a9b8ce6849e7f1b8a3de9 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 4 May 2017 15:11:15 +0300 Subject: Backport of multiple_assignees_feature [ci skip] --- app/models/concerns/issuable.rb | 27 +++++---------------------- app/models/concerns/milestoneish.rb | 2 +- app/models/global_milestone.rb | 6 +++--- app/models/issue.rb | 35 ++++++++++++++++++++++++++++++++--- app/models/issue_assignee.rb | 29 +++++++++++++++++++++++++++++ app/models/merge_request.rb | 32 ++++++++++++++++++++++++++++++++ app/models/milestone.rb | 5 ++++- app/models/user.rb | 5 +++++ 8 files changed, 111 insertions(+), 30 deletions(-) create mode 100644 app/models/issue_assignee.rb (limited to 'app/models') diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 26dbf4d9570..edf4e9e5d78 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -26,7 +26,6 @@ module Issuable cache_markdown_field :description, issuable_state_filter_enabled: true belongs_to :author, class_name: "User" - belongs_to :assignee, class_name: "User" belongs_to :updated_by, class_name: "User" belongs_to :milestone has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do @@ -65,11 +64,8 @@ module Issuable validates :title, presence: true, length: { maximum: 255 } scope :authored, ->(user) { where(author_id: user) } - scope :assigned_to, ->(u) { where(assignee_id: u.id)} scope :recent, -> { reorder(id: :desc) } scope :order_position_asc, -> { reorder(position: :asc) } - scope :assigned, -> { where("assignee_id IS NOT NULL") } - scope :unassigned, -> { where("assignee_id IS NULL") } scope :of_projects, ->(ids) { where(project_id: ids) } scope :of_milestones, ->(ids) { where(milestone_id: ids) } scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) } @@ -92,7 +88,6 @@ module Issuable attr_mentionable :description participant :author - participant :assignee participant :notes_with_associations strip_attributes :title @@ -102,13 +97,6 @@ module Issuable after_save :update_assignee_cache_counts, if: :assignee_id_changed? after_save :record_metrics, unless: :imported? - def update_assignee_cache_counts - # make sure we flush the cache for both the old *and* new assignees(if they exist) - previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was - previous_assignee&.update_cache_counts - assignee&.update_cache_counts - end - # We want to use optimistic lock for cases when only title or description are involved # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html def locking_enabled? @@ -237,10 +225,6 @@ module Issuable today? && created_at == updated_at end - def is_being_reassigned? - assignee_id_changed? - end - def open? opened? || reopened? end @@ -269,7 +253,11 @@ module Issuable # DEPRECATED repository: project.hook_attrs.slice(:name, :url, :description, :homepage) } - hook_data[:assignee] = assignee.hook_attrs if assignee + if self.is_a?(Issue) + hook_data[:assignees] = assignees.map(&:hook_attrs) if assignees.any? + else + hook_data[:assignee] = assignee.hook_attrs if assignee + end hook_data end @@ -331,11 +319,6 @@ module Issuable false end - def assignee_or_author?(user) - # We're comparing IDs here so we don't need to load any associations. - author_id == user.id || assignee_id == user.id - end - def record_metrics metrics = self.metrics || create_metrics metrics.record! diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index f449229864d..a3472af5c55 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -40,7 +40,7 @@ module Milestoneish def issues_visible_to_user(user) memoize_per_user(user, :issues_visible_to_user) do IssuesFinder.new(user, issues_finder_params) - .execute.where(milestone_id: milestoneish_ids) + .execute.includes(:assignees).where(milestone_id: milestoneish_ids) end end diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index 0afbca2cb32..538615130a7 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -36,7 +36,7 @@ class GlobalMilestone closed = count_by_state(milestones_by_state_and_title, 'closed') all = milestones_by_state_and_title.map { |(_, title), _| title }.uniq.count - { + { opened: opened, closed: closed, all: all @@ -86,7 +86,7 @@ class GlobalMilestone end def issues - @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignee, :labels) + @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignees, :labels) end def merge_requests @@ -94,7 +94,7 @@ class GlobalMilestone end def participants - @participants ||= milestones.includes(:participants).map(&:participants).flatten.compact.uniq + @participants ||= milestones.map(&:participants).flatten.uniq end def labels diff --git a/app/models/issue.rb b/app/models/issue.rb index 78bde6820da..27e3ed9bc7f 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -24,10 +24,17 @@ class Issue < ActiveRecord::Base has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all + has_many :issue_assignees + has_many :assignees, class_name: "User", through: :issue_assignees + validates :project, presence: true scope :in_projects, ->(project_ids) { where(project_id: project_ids) } + scope :assigned, -> { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') } + scope :unassigned, -> { where('NOT EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') } + scope :assigned_to, ->(u) { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = ? AND issue_id = issues.id)', u.id)} + scope :without_due_date, -> { where(due_date: nil) } scope :due_before, ->(date) { where('issues.due_date < ?', date) } scope :due_between, ->(from_date, to_date) { where('issues.due_date >= ?', from_date).where('issues.due_date <= ?', to_date) } @@ -37,13 +44,15 @@ class Issue < ActiveRecord::Base scope :created_after, -> (datetime) { where("created_at >= ?", datetime) } - scope :include_associations, -> { includes(:assignee, :labels, project: :namespace) } + scope :include_associations, -> { includes(:labels, project: :namespace) } after_save :expire_etag_cache attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true + participant :assignees + state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed @@ -63,10 +72,14 @@ class Issue < ActiveRecord::Base end def hook_attrs + assignee_ids = self.assignee_ids + attrs = { total_time_spent: total_time_spent, human_total_time_spent: human_total_time_spent, - human_time_estimate: human_time_estimate + human_time_estimate: human_time_estimate, + assignee_ids: assignee_ids, + assignee_id: assignee_ids.first # This key is deprecated } attributes.merge!(attrs) @@ -114,6 +127,22 @@ class Issue < ActiveRecord::Base "id DESC") end + # Returns a Hash of attributes to be used for Twitter card metadata + def card_attributes + { + 'Author' => author.try(:name), + 'Assignee' => assignee_list + } + end + + def assignee_or_author?(user) + author_id == user.id || assignees.exists?(user.id) + end + + def assignee_list + assignees.map(&:name).to_sentence + end + # `from` argument can be a Namespace or Project. def to_reference(from = nil, full: false) reference = "#{self.class.reference_prefix}#{iid}" @@ -248,7 +277,7 @@ class Issue < ActiveRecord::Base true elsif confidential? author == user || - assignee == user || + assignees.include?(user) || project.team.member?(user, Gitlab::Access::REPORTER) else project.public? || diff --git a/app/models/issue_assignee.rb b/app/models/issue_assignee.rb new file mode 100644 index 00000000000..b94b55bb1d1 --- /dev/null +++ b/app/models/issue_assignee.rb @@ -0,0 +1,29 @@ +class IssueAssignee < ActiveRecord::Base + extend Gitlab::CurrentSettings + + belongs_to :issue + belongs_to :assignee, class_name: "User", foreign_key: :user_id + + after_create :update_assignee_cache_counts + after_destroy :update_assignee_cache_counts + + # EE-specific + after_create :update_elasticsearch_index + after_destroy :update_elasticsearch_index + # EE-specific + + def update_assignee_cache_counts + assignee&.update_cache_counts + end + + def update_elasticsearch_index + if current_application_settings.elasticsearch_indexing? + ElasticIndexerWorker.perform_async( + :update, + 'Issue', + issue.id, + changed_fields: ['assignee_ids'] + ) + end + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 12c5481cd6d..35231bab12e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -17,6 +17,8 @@ class MergeRequest < ActiveRecord::Base has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all + belongs_to :assignee, class_name: "User" + serialize :merge_params, Hash after_create :ensure_merge_request_diff, unless: :importing? @@ -114,8 +116,14 @@ class MergeRequest < ActiveRecord::Base scope :join_project, -> { joins(:target_project) } scope :references_project, -> { references(:target_project) } + scope :assigned, -> { where("assignee_id IS NOT NULL") } + scope :unassigned, -> { where("assignee_id IS NULL") } + scope :assigned_to, ->(u) { where(assignee_id: u.id)} + + participant :assignee after_save :keep_around_commit + after_save :update_assignee_cache_counts, if: :assignee_id_changed? def self.reference_prefix '!' @@ -177,6 +185,30 @@ class MergeRequest < ActiveRecord::Base work_in_progress?(title) ? title : "WIP: #{title}" end + def update_assignee_cache_counts + # make sure we flush the cache for both the old *and* new assignees(if they exist) + previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was + previous_assignee&.update_cache_counts + assignee&.update_cache_counts + end + + # Returns a Hash of attributes to be used for Twitter card metadata + def card_attributes + { + 'Author' => author.try(:name), + 'Assignee' => assignee.try(:name) + } + end + + # This method is needed for compatibility with issues to not mess view and other code + def assignees + Array(assignee) + end + + def assignee_or_author?(user) + author_id == user.id || assignee_id == user.id + end + # `from` argument can be a Namespace or Project. def to_reference(from = nil, full: false) reference = "#{self.class.reference_prefix}#{iid}" diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 652b1551928..c06bfe0ccdd 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -21,7 +21,6 @@ class Milestone < ActiveRecord::Base has_many :issues has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues has_many :merge_requests - has_many :participants, -> { distinct.reorder('users.name') }, through: :issues, source: :assignee has_many :events, as: :target, dependent: :destroy scope :active, -> { with_state(:active) } @@ -107,6 +106,10 @@ class Milestone < ActiveRecord::Base end end + def participants + User.joins(assigned_issues: :milestone).where("milestones.id = ?", id) + end + def self.sort(method) case method.to_s when 'due_date_asc' diff --git a/app/models/user.rb b/app/models/user.rb index 2b7ebe6c1a7..a3126cbb644 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -89,6 +89,7 @@ class User < ActiveRecord::Base has_many :subscriptions, dependent: :destroy has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id, class_name: "Event" has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy + has_one :abuse_report, dependent: :destroy, foreign_key: :user_id has_many :reported_abuse_reports, dependent: :destroy, foreign_key: :reporter_id, class_name: "AbuseReport" has_many :spam_logs, dependent: :destroy @@ -99,6 +100,10 @@ class User < ActiveRecord::Base has_many :award_emoji, dependent: :destroy has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id + has_many :issue_assignees + has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue + has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" + # Issues that a user owns are expected to be moved to the "ghost" user before # the user is destroyed. If the user owns any issues during deletion, this # should be treated as an exceptional condition. -- cgit v1.2.3