diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-04-06 10:05:57 -0500 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-04-06 10:51:45 -0500 |
commit | cc656a11992483911cefe035d579096581cfde57 (patch) | |
tree | 5af4a41d9ae36b3900fdfa0b312b125995c8818c /app/models | |
parent | 64c1735c22871d94e0bda8b9f5aece2a29739236 (diff) | |
download | gitlab-ce-cc656a11992483911cefe035d579096581cfde57.tar.gz |
Refactor resolvability checks based on type
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/concerns/issuable.rb | 11 | ||||
-rw-r--r-- | app/models/concerns/noteable.rb | 25 | ||||
-rw-r--r-- | app/models/concerns/resolvable_discussion.rb | 7 | ||||
-rw-r--r-- | app/models/concerns/resolvable_note.rb | 12 | ||||
-rw-r--r-- | app/models/diff_discussion.rb | 6 | ||||
-rw-r--r-- | app/models/diff_note.rb | 2 | ||||
-rw-r--r-- | app/models/discussion.rb | 6 | ||||
-rw-r--r-- | app/models/discussion_note.rb | 6 | ||||
-rw-r--r-- | app/models/individual_note_discussion.rb | 6 | ||||
-rw-r--r-- | app/models/legacy_diff_discussion.rb | 7 | ||||
-rw-r--r-- | app/models/legacy_diff_note.rb | 3 | ||||
-rw-r--r-- | app/models/note.rb | 9 | ||||
-rw-r--r-- | app/models/out_of_context_discussion.rb | 8 |
13 files changed, 73 insertions, 35 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index b4dded7e27e..3d2258d5e3e 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -292,17 +292,6 @@ module Issuable self.class.to_ability_name end - # Convert this Issuable class name to a format usable by notifications. - # - # Examples: - # - # issuable.class # => MergeRequest - # issuable.human_class_name # => "merge request" - - def human_class_name - @human_class_name ||= self.class.name.titleize.downcase - end - # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index 631df4757a4..772ff6a6d2f 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -1,4 +1,29 @@ module Noteable + # Names of all implementers of `Noteable` that support resolvable notes. + RESOLVABLE_TYPES = %w(MergeRequest).freeze + + def base_class_name + self.class.base_class.name + end + + # Convert this Noteable class name to a format usable by notifications. + # + # Examples: + # + # noteable.class # => MergeRequest + # noteable.human_class_name # => "merge request" + def human_class_name + @human_class_name ||= base_class_name.titleize.downcase + end + + def supports_resolvable_notes? + RESOLVABLE_TYPES.include?(base_class_name) + end + + def supports_discussions? + DiscussionNote::NOTEABLE_TYPES.include?(base_class_name) + end + def discussion_notes notes end diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index 5cb51196a94..dd979e7bb17 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/app/models/concerns/resolvable_discussion.rb @@ -20,6 +20,8 @@ module ResolvableDiscussion :last_note ) + delegate :potentially_resolvable?, to: :first_note + delegate :resolved_at, :resolved_by, @@ -27,11 +29,6 @@ module ResolvableDiscussion allow_nil: true end - # Keep this method in sync with the `potentially_resolvable` scope on `ResolvableNote` - def potentially_resolvable? - for_merge_request? - end - def resolvable? return @resolvable if @resolvable.present? diff --git a/app/models/concerns/resolvable_note.rb b/app/models/concerns/resolvable_note.rb index 2c70678429a..05eb6f86704 100644 --- a/app/models/concerns/resolvable_note.rb +++ b/app/models/concerns/resolvable_note.rb @@ -1,6 +1,7 @@ module ResolvableNote extend ActiveSupport::Concern + # Names of all subclasses of `Note` that can be resolvable. RESOLVABLE_TYPES = %w(DiffNote DiscussionNote).freeze included do @@ -8,10 +9,8 @@ module ResolvableNote validates :resolved_by, presence: true, if: :resolved? - # Keep this scope in sync with the logic in `#potentially_resolvable?` in subclasses of `Discussion` that are resolvable. - # `RESOLVABLE_TYPES` should include names of all subclasses that are resolvable (where the method can return true), and - # the scope should also match the criteria `ResolvableDiscussion#potentially_resolvable?` puts on resolvability. - scope :potentially_resolvable, -> { where(type: RESOLVABLE_TYPES).where(noteable_type: 'MergeRequest') } + # Keep this scope in sync with `#potentially_resolvable?` + scope :potentially_resolvable, -> { where(type: RESOLVABLE_TYPES).where(noteable_type: Noteable::RESOLVABLE_TYPES) } # Keep this scope in sync with `#resolvable?` scope :resolvable, -> { potentially_resolvable.user } @@ -31,7 +30,10 @@ module ResolvableNote end end - delegate :potentially_resolvable?, to: :to_discussion + # Keep this method in sync with the `potentially_resolvable` scope + def potentially_resolvable? + RESOLVABLE_TYPES.include?(self.class.name) && noteable.supports_resolvable_notes? + end # Keep this method in sync with the `resolvable` scope def resolvable? diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 22912b30c6a..d9b7e484e0f 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -1,7 +1,13 @@ # A discussion on merge request or commit diffs consisting of `DiffNote` notes. +# +# A discussion of this type can be resolvable. class DiffDiscussion < Discussion include DiscussionOnDiff + def self.note_class + DiffNote + end + delegate :position, :original_position, diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 9185a5a0ad8..1523244f8a8 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -1,4 +1,6 @@ # A note on merge request or commit diffs +# +# A note of this type can be resolvable. class DiffNote < Note include NoteOnDiff diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 8268a140403..2c0e6379e6a 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -1,4 +1,6 @@ # A non-diff discussion on an issue, merge request, commit, or snippet, consisting of `DiscussionNote` notes. +# +# A discussion of this type can be resolvable. class Discussion include ResolvableDiscussion @@ -54,6 +56,10 @@ class Discussion nil end + def self.note_class + DiscussionNote + end + def initialize(notes, noteable = nil) @notes = notes @noteable = noteable diff --git a/app/models/discussion_note.rb b/app/models/discussion_note.rb index 337fbc8435c..e660b024083 100644 --- a/app/models/discussion_note.rb +++ b/app/models/discussion_note.rb @@ -1,7 +1,9 @@ -# A note in a non-diff discussion on an issue, merge request, commit, or snippet +# A note in a non-diff discussion on an issue, merge request, commit, or snippet. +# +# A note of this type can be resolvable. class DiscussionNote < Note + # Names of all implementers of `Noteable` that support discussions. NOTEABLE_TYPES = %w(MergeRequest Issue Commit Snippet).freeze - RESOLVABLE_TYPES = %w(MergeRequest).freeze validates :noteable_type, inclusion: { in: NOTEABLE_TYPES } diff --git a/app/models/individual_note_discussion.rb b/app/models/individual_note_discussion.rb index 42318fa3d6e..c3f21c55240 100644 --- a/app/models/individual_note_discussion.rb +++ b/app/models/individual_note_discussion.rb @@ -1,8 +1,10 @@ # A discussion to wrap a single `Note` note on the root of an issue, merge request, # commit, or snippet, that is not displayed as a discussion. +# +# A discussion of this type is never resolvable. class IndividualNoteDiscussion < Discussion - def potentially_resolvable? - false + def self.note_class + Note end def individual_note? diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb index 6515762c92d..cb2651a03f8 100644 --- a/app/models/legacy_diff_discussion.rb +++ b/app/models/legacy_diff_discussion.rb @@ -1,6 +1,9 @@ # A discussion on merge request or commit diffs consisting of `LegacyDiffNote` notes. +# # All new diff discussions are of the type `DiffDiscussion`, but any diff discussions created # before the introduction of the new implementation still use `LegacyDiffDiscussion`. +# +# A discussion of this type is never resolvable. class LegacyDiffDiscussion < Discussion include DiscussionOnDiff @@ -8,8 +11,8 @@ class LegacyDiffDiscussion < Discussion true end - def potentially_resolvable? - false + def self.note_class + LegacyDiffNote end def collapsed? diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index b9fddb2ea79..9a77557ebcd 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -1,6 +1,9 @@ # A note on merge request or commit diffs, using the legacy implementation. +# # All new diff notes are of the type `DiffNote`, but any diff notes created # before the introduction of the new implementation still use `LegacyDiffNote`. +# +# A note of this type is never resolvable. class LegacyDiffNote < Note include NoteOnDiff diff --git a/app/models/note.rb b/app/models/note.rb index cd4996bdbae..401e3d7bcbc 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -1,3 +1,6 @@ +# A note on the root of an issue, merge request, commit, or snippet. +# +# A note of this type is never resolvable. class Note < ActiveRecord::Base extend ActiveModel::Naming include Gitlab::CurrentSettings @@ -225,11 +228,7 @@ class Note < ActiveRecord::Base end def can_be_discussion_note? - DiscussionNote::NOTEABLE_TYPES.include?(self.noteable_type) && !part_of_discussion? - end - - def can_be_resolvable? - DiscussionNote::RESOLVABLE_TYPES.include?(self.noteable_type) + self.noteable.supports_discussions? && !part_of_discussion? end def discussion_class(noteable = nil) diff --git a/app/models/out_of_context_discussion.rb b/app/models/out_of_context_discussion.rb index 31c6d5cff8b..85794630f70 100644 --- a/app/models/out_of_context_discussion.rb +++ b/app/models/out_of_context_discussion.rb @@ -2,6 +2,8 @@ # contains that commit, they are displayed as if they were a discussion. # # This represents one of those discussions, consisting of `Note` notes. +# +# A discussion of this type is never resolvable. class OutOfContextDiscussion < Discussion # Returns an array of discussion ID components def self.build_discussion_id(note) @@ -13,8 +15,8 @@ class OutOfContextDiscussion < Discussion def self.override_discussion_id(note) discussion_id(note) end - - def potentially_resolvable? - false + + def self.note_class + Note end end |