diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/controllers/projects/discussions_controller.rb | 6 | ||||
| -rw-r--r-- | app/helpers/notes_helper.rb | 8 | ||||
| -rw-r--r-- | app/models/diff_note.rb | 37 | ||||
| -rw-r--r-- | app/models/discussion.rb | 4 | ||||
| -rw-r--r-- | app/models/legacy_diff_note.rb | 12 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 15 | ||||
| -rw-r--r-- | app/models/note.rb | 35 |
7 files changed, 73 insertions, 44 deletions
diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb index 620b41b1011..b2e8733ccb7 100644 --- a/app/controllers/projects/discussions_controller.rb +++ b/app/controllers/projects/discussions_controller.rb @@ -5,8 +5,6 @@ class Projects::DiscussionsController < Projects::ApplicationController before_action :authorize_resolve_discussion! def resolve - return render_404 unless discussion.resolvable? - discussion.resolve!(current_user) MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) @@ -18,8 +16,6 @@ class Projects::DiscussionsController < Projects::ApplicationController end def unresolve - return render_404 unless discussion.resolvable? - discussion.unresolve! render json: { @@ -34,7 +30,7 @@ class Projects::DiscussionsController < Projects::ApplicationController end def discussion - @discussion ||= @merge_request.find_discussion(params[:id]) || render_404 + @discussion ||= @merge_request.find_diff_discussion(params[:id]) || render_404 end def authorize_resolve_discussion! diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 6b00ebc98e7..da230f76bae 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -49,7 +49,7 @@ module NotesHelper } if use_legacy_diff_note - discussion_id = LegacyDiffNote.build_discussion_id( + discussion_id = LegacyDiffNote.discussion_id( @comments_target[:noteable_type], @comments_target[:noteable_id] || @comments_target[:commit_id], line_code @@ -57,10 +57,10 @@ module NotesHelper data.merge!( note_type: LegacyDiffNote.name, - discussion_id: Digest::SHA1.hexdigest(discussion_id) + discussion_id: discussion_id ) else - discussion_id = DiffNote.build_discussion_id( + discussion_id = DiffNote.discussion_id( @comments_target[:noteable_type], @comments_target[:noteable_id] || @comments_target[:commit_id], position @@ -69,7 +69,7 @@ module NotesHelper data.merge!( position: position.to_json, note_type: DiffNote.name, - discussion_id: Digest::SHA1.hexdigest(discussion_id) + discussion_id: discussion_id ) end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index cfcdce790e5..1313739f322 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -15,8 +15,9 @@ class DiffNote < Note validate :positions_complete validate :verify_supported + after_initialize :ensure_original_discussion_id before_validation :set_original_position, :update_position, on: :create - before_validation :set_line_code + before_validation :set_line_code, :set_original_discussion_id after_save :keep_around_commits class << self @@ -33,14 +34,6 @@ class DiffNote < Note { position: position.to_json } end - def discussion_id - @discussion_id ||= Digest::SHA1.hexdigest(self.class.build_discussion_id(noteable_type, noteable_id || commit_id, position)) - end - - def original_discussion_id - @original_discussion_id ||= Digest::SHA1.hexdigest(self.class.build_discussion_id(noteable_type, noteable_id || commit_id, original_position)) - end - def position=(new_position) if new_position.is_a?(String) new_position = JSON.parse(new_position) rescue nil @@ -106,11 +99,7 @@ class DiffNote < Note def discussion return unless resolvable? - discussion_notes = self.noteable.notes.fresh.select { |n| n.discussion_id == self.discussion_id } - - return if discussion_notes.empty? - - Discussion.new(discussion_notes) + self.noteable.find_diff_discussion(self.discussion_id) end def to_discussion @@ -139,6 +128,26 @@ class DiffNote < Note self.line_code = self.position.line_code(self.project.repository) end + def ensure_original_discussion_id + return unless self.persisted? + return if self.original_discussion_id + + set_original_discussion_id + update_column(:original_discussion_id, self.original_discussion_id) + end + + def set_original_discussion_id + self.original_discussion_id = Digest::SHA1.hexdigest(build_original_discussion_id) + end + + def build_discussion_id + self.class.build_discussion_id(noteable_type, noteable_id || commit_id, position) + end + + def build_original_discussion_id + self.class.build_discussion_id(noteable_type, noteable_id || commit_id, original_position) + end + def update_position return unless supported? return if for_commit? diff --git a/app/models/discussion.rb b/app/models/discussion.rb index b26d2282b00..3fddc084af2 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -57,7 +57,7 @@ class Discussion def id first_note.discussion_id end - + alias_method :to_param, :id def diff_discussion? @@ -93,7 +93,7 @@ class Discussion return false unless resolvable? current_user == self.noteable.author || - current_user.can?(:push_code, self.project) + current_user.can?(:resolve_note, self.project) end def resolve!(current_user) diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index f15553d706e..8e26cbe9835 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -8,8 +8,8 @@ class LegacyDiffNote < Note before_create :set_diff class << self - def build_discussion_id(noteable_type, noteable_id, line_code, active = true) - [super(noteable_type, noteable_id), line_code, active].join("-") + def build_discussion_id(noteable_type, noteable_id, line_code) + [super(noteable_type, noteable_id), line_code].join("-") end end @@ -21,10 +21,6 @@ class LegacyDiffNote < Note { line_code: line_code } end - def discussion_id - @discussion_id ||= Digest::SHA1.hexdigest(self.class.build_discussion_id(noteable_type, noteable_id || commit_id, line_code)) - end - def project_repository if RequestStore.active? RequestStore.fetch("project:#{project_id}:repository") { self.project.repository } @@ -119,4 +115,8 @@ class LegacyDiffNote < Note diffs = noteable.raw_diffs(Commit.max_diff_options) diffs.find { |d| d.new_path == self.diff.new_path } end + + def build_discussion_id + self.class.build_discussion_id(noteable_type, noteable_id || commit_id, line_code) + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5bb48117851..31badb54188 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -425,16 +425,23 @@ class MergeRequest < ActiveRecord::Base discussions end - def find_discussion(discussion_id) - discussions.find { |d| d.id == discussion_id } + def diff_discussions + @diff_discussions ||= self.notes.diff_notes.discussions + end + + def find_diff_discussion(discussion_id) + notes = self.notes.diff_notes.where(discussion_id: discussion_id).fresh.to_a + return if notes.empty? + + Discussion.new(notes) end def discussions_resolvable? - discussions.any?(&:resolvable?) + diff_discussions.any?(&:resolvable?) end def discussions_resolved? - discussions_resolvable? && discussions.none?(&:to_be_resolved?) + discussions_resolvable? && diff_discussions.none?(&:to_be_resolved?) end def hook_attrs diff --git a/app/models/note.rb b/app/models/note.rb index 732b2ff9bf7..fb0d0ebc396 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -70,7 +70,9 @@ class Note < ActiveRecord::Base project: [:project_members, { group: [:group_members] }]) end + after_initialize :ensure_discussion_id before_validation :nullify_blank_type, :nullify_blank_line_code + before_validation :set_discussion_id after_save :keep_around_commit class << self @@ -82,6 +84,10 @@ class Note < ActiveRecord::Base [:discussion, noteable_type.try(:underscore), noteable_id].join("-") end + def self.discussion_id(*args) + Digest::SHA1.hexdigest(build_discussion_id(*args)) + end + def discussions Discussion.for_notes(all) end @@ -142,15 +148,6 @@ class Note < ActiveRecord::Base resolvable? && !resolved? end - def discussion_id - @discussion_id ||= - if for_merge_request? - Digest::SHA1.hexdigest([:discussion, :note, id].join("-")) - else - Digest::SHA1.hexdigest(self.class.build_discussion_id(noteable_type, noteable_id || commit_id)) - end - end - def max_attachment_size current_application_settings.max_attachment_size.megabytes.to_i end @@ -256,4 +253,24 @@ class Note < ActiveRecord::Base def nullify_blank_line_code self.line_code = nil if self.line_code.blank? end + + def ensure_discussion_id + return unless self.persisted? + return if self.discussion_id + + set_discussion_id + update_column(:discussion_id, self.discussion_id) + end + + def set_discussion_id + self.discussion_id = Digest::SHA1.hexdigest(build_discussion_id) + end + + def build_discussion_id + if for_merge_request? + [:discussion, :note, id].join("-") + else + self.class.build_discussion_id(noteable_type, noteable_id || commit_id) + end + end end |