diff options
author | Douwe Maan <douwe@gitlab.com> | 2019-06-04 17:06:21 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2019-06-04 17:06:21 +0300 |
commit | 57f9856492775fef8e0bedc503e207b1d372f738 (patch) | |
tree | 728a1a282c243a93b8faa3151c899e302b0334d6 | |
parent | 280aa61eec7efa0b60501546b6e1efedf0b9dc85 (diff) | |
parent | 7412e2fa7478e1ec6a5a0014b7a4bdc6d07133b2 (diff) |
Merge branch 'bvl-design-diff-notes-ce' into 'master'
Adjustments related to DiffNotes on diffs outside of a project's main repository
See merge request gitlab-org/gitlab-ce!29023
-rw-r--r-- | app/models/concerns/noteable.rb | 12 | ||||
-rw-r--r-- | app/models/concerns/resolvable_note.rb | 2 | ||||
-rw-r--r-- | app/models/diff_note.rb | 18 | ||||
-rw-r--r-- | lib/api/discussions.rb | 2 | ||||
-rw-r--r-- | spec/models/concerns/noteable_spec.rb | 12 |
5 files changed, 34 insertions, 12 deletions
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index bfd0c36942b..4b428b0af83 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -3,14 +3,16 @@ module Noteable extend ActiveSupport::Concern - # `Noteable` class names that support resolvable notes. - RESOLVABLE_TYPES = %w(MergeRequest).freeze - class_methods do # `Noteable` class names that support replying to individual notes. def replyable_types %w(Issue MergeRequest) end + + # `Noteable` class names that support resolvable notes. + def resolvable_types + %w(MergeRequest) + end end # The timestamp of the note (e.g. the :created_at or :updated_at attribute if provided via @@ -36,7 +38,7 @@ module Noteable end def supports_resolvable_notes? - RESOLVABLE_TYPES.include?(base_class_name) + self.class.resolvable_types.include?(base_class_name) end def supports_discussions? @@ -131,3 +133,5 @@ module Noteable ) end end + +Noteable.extend(Noteable::ClassMethods) diff --git a/app/models/concerns/resolvable_note.rb b/app/models/concerns/resolvable_note.rb index 16ea330701d..2d2d5fb7168 100644 --- a/app/models/concerns/resolvable_note.rb +++ b/app/models/concerns/resolvable_note.rb @@ -12,7 +12,7 @@ module ResolvableNote validates :resolved_by, presence: true, if: :resolved? # Keep this scope in sync with `#potentially_resolvable?` - scope :potentially_resolvable, -> { where(type: RESOLVABLE_TYPES).where(noteable_type: Noteable::RESOLVABLE_TYPES) } + 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 } diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index feabea9b8ba..1a87fc47c56 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -15,7 +15,9 @@ class DiffNote < Note validates :original_position, presence: true validates :position, presence: true validates :line_code, presence: true, line_code: true, if: :on_text? - validates :noteable_type, inclusion: { in: noteable_types } + # We need to evaluate the `noteable` types when running the validation since + # EE might have added a type when the module was prepended + validates :noteable_type, inclusion: { in: -> (_note) { noteable_types } } validate :positions_complete validate :verify_supported validate :diff_refs_match_commit, if: :for_commit? @@ -44,7 +46,7 @@ class DiffNote < Note # Returns the diff file from `position` def latest_diff_file strong_memoize(:latest_diff_file) do - position.diff_file(project.repository) + position.diff_file(repository) end end @@ -111,7 +113,7 @@ class DiffNote < Note if note_diff_file diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) Gitlab::Diff::File.new(diff, - repository: project.repository, + repository: repository, diff_refs: original_position.diff_refs) elsif created_at_diff?(noteable.diff_refs) # We're able to use the already persisted diffs (Postgres) if we're @@ -122,7 +124,7 @@ class DiffNote < Note # `Diff::FileCollection::MergeRequestDiff`. noteable.diffs(original_position.diff_options).diff_files.first else - original_position.diff_file(self.project.repository) + original_position.diff_file(repository) end # Since persisted diff files already have its content "unfolded" @@ -137,7 +139,7 @@ class DiffNote < Note end def set_line_code - self.line_code = self.position.line_code(self.project.repository) + self.line_code = self.position.line_code(repository) end def verify_supported @@ -171,6 +173,10 @@ class DiffNote < Note shas << self.position.head_sha end - project.repository.keep_around(*shas) + repository.keep_around(*shas) + end + + def repository + noteable.respond_to?(:repository) ? noteable.repository : project.repository end end diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb index 5928ee1657b..693172b7d08 100644 --- a/lib/api/discussions.rb +++ b/lib/api/discussions.rb @@ -206,7 +206,7 @@ module API delete_note(noteable, params[:note_id]) end - if Noteable::RESOLVABLE_TYPES.include?(noteable_type.to_s) + if Noteable.resolvable_types.include?(noteable_type.to_s) desc "Resolve/unresolve an existing #{noteable_type.to_s.downcase} discussion" do success Entities::Discussion end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index ee613b199ad..e17b98536fa 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -260,4 +260,16 @@ describe Noteable do end end end + + describe '.replyable_types' do + it 'exposes the replyable types' do + expect(described_class.replyable_types).to include('Issue', 'MergeRequest') + end + end + + describe '.resolvable_types' do + it 'exposes the replyable types' do + expect(described_class.resolvable_types).to include('MergeRequest') + end + end end |