diff options
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/commit.rb | 7 | ||||
-rw-r--r-- | app/models/concerns/note_on_diff.rb | 52 | ||||
-rw-r--r-- | app/models/deployment.rb | 6 | ||||
-rw-r--r-- | app/models/diff_note.rb | 127 | ||||
-rw-r--r-- | app/models/legacy_diff_note.rb | 69 | ||||
-rw-r--r-- | app/models/merge_request.rb | 189 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 138 | ||||
-rw-r--r-- | app/models/note.rb | 12 | ||||
-rw-r--r-- | app/models/project_services/jira_service.rb | 2 | ||||
-rw-r--r-- | app/models/repository.rb | 10 | ||||
-rw-r--r-- | app/models/sent_notification.rb | 45 |
11 files changed, 454 insertions, 203 deletions
diff --git a/app/models/commit.rb b/app/models/commit.rb index 174ccbaea6c..2ef3973c160 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -214,6 +214,13 @@ class Commit @raw.short_id(7) end + def diff_refs + Gitlab::Diff::DiffRefs.new( + base_sha: self.parent_id || self.sha, + head_sha: self.sha + ) + end + def pipelines @pipeline ||= project.pipelines.where(sha: sha) end diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb new file mode 100644 index 00000000000..2785fbb21c9 --- /dev/null +++ b/app/models/concerns/note_on_diff.rb @@ -0,0 +1,52 @@ +module NoteOnDiff + extend ActiveSupport::Concern + + NUMBER_OF_TRUNCATED_DIFF_LINES = 16 + + included do + delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true + end + + def diff_note? + true + end + + def diff_file + raise NotImplementedError + end + + def diff_line + raise NotImplementedError + end + + def for_line?(line) + raise NotImplementedError + end + + def diff_attributes + raise NotImplementedError + end + + def can_be_award_emoji? + false + end + + # Returns an array of at most 16 highlighted lines above a diff note + def truncated_diff_lines + prev_lines = [] + + highlighted_diff_lines.each do |line| + if line.meta? + prev_lines.clear + else + prev_lines << line + + break if for_line?(line) + + prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + prev_lines + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index e498ca96e3c..520026c18dd 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -11,6 +11,8 @@ class Deployment < ActiveRecord::Base delegate :name, to: :environment, prefix: true + after_save :keep_around_commit + def commit project.commit(sha) end @@ -26,4 +28,8 @@ class Deployment < ActiveRecord::Base def last? self == environment.last_deployment end + + def keep_around_commit + project.repository.keep_around(self.sha) + end end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb new file mode 100644 index 00000000000..9671955db36 --- /dev/null +++ b/app/models/diff_note.rb @@ -0,0 +1,127 @@ +class DiffNote < Note + include NoteOnDiff + + serialize :original_position, Gitlab::Diff::Position + serialize :position, Gitlab::Diff::Position + + validates :original_position, presence: true + validates :position, presence: true + validates :diff_line, presence: true + validates :line_code, presence: true, line_code: true + validates :noteable_type, inclusion: { in: ['Commit', 'MergeRequest'] } + validate :positions_complete + validate :verify_supported + + before_validation :set_original_position, :update_position, on: :create + before_validation :set_line_code + after_save :keep_around_commits + + class << self + def build_discussion_id(noteable_type, noteable_id, position) + [super(noteable_type, noteable_id), *position.key].join("-") + end + end + + def new_diff_note? + true + end + + def diff_attributes + { position: position.to_json } + end + + def discussion_id + @discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, position) + end + + def original_discussion_id + @original_discussion_id ||= 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 + end + + if new_position.is_a?(Hash) + new_position = new_position.with_indifferent_access + new_position = Gitlab::Diff::Position.new(new_position) + end + + super(new_position) + end + + def diff_file + @diff_file ||= self.original_position.diff_file(self.project.repository) + end + + def diff_line + @diff_line ||= diff_file.line_for_position(self.original_position) if diff_file + end + + def for_line?(line) + diff_file.position(line) == self.original_position + end + + def active?(diff_refs = nil) + return false unless supported? + return true if for_commit? + + diff_refs ||= self.noteable.diff_refs + + self.position.diff_refs == diff_refs + end + + private + + def supported? + !self.for_merge_request? || self.noteable.support_new_diff_notes? + end + + def set_original_position + self.original_position = self.position.dup + end + + def set_line_code + self.line_code = self.position.line_code(self.project.repository) + end + + def update_position + return unless supported? + return if for_commit? + + return if active? + + Notes::DiffPositionUpdateService.new( + self.project, + nil, + old_diff_refs: self.position.diff_refs, + new_diff_refs: self.noteable.diff_refs, + paths: self.position.paths + ).execute(self) + end + + def verify_supported + return if supported? + + errors.add(:noteable, "doesn't support new-style diff notes") + end + + def positions_complete + return if self.original_position.complete? && self.position.complete? + + errors.add(:position, "is invalid") + end + + def keep_around_commits + project.repository.keep_around(self.original_position.base_sha) + project.repository.keep_around(self.original_position.start_sha) + project.repository.keep_around(self.original_position.head_sha) + + if self.position != self.original_position + project.repository.keep_around(self.position.base_sha) + project.repository.keep_around(self.position.start_sha) + project.repository.keep_around(self.position.head_sha) + end + end +end diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index 33d2a69ebaf..790dfd4d480 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -1,4 +1,6 @@ class LegacyDiffNote < Note + include NoteOnDiff + serialize :st_diff validates :line_code, presence: true, line_code: true @@ -11,12 +13,12 @@ class LegacyDiffNote < Note end end - def diff_note? + def legacy_diff_note? true end - def legacy_diff_note? - true + def diff_attributes + { line_code: line_code } end def discussion_id @@ -27,61 +29,20 @@ class LegacyDiffNote < Note line_code.split('_')[0] if line_code end - def diff_old_line - line_code.split('_')[1].to_i if line_code - end - - def diff_new_line - line_code.split('_')[2].to_i if line_code - end - def diff @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map) end - def diff_file_path - diff.new_path.presence || diff.old_path - end - - def diff_lines - @diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.each_line) + def diff_file + @diff_file ||= Gitlab::Diff::File.new(diff, repository: self.project.repository) if diff end def diff_line - @diff_line ||= diff_lines.find { |line| generate_line_code(line) == self.line_code } + @diff_line ||= diff_file.line_for_line_code(self.line_code) end - def diff_line_text - diff_line.try(:text) - end - - def diff_line_type - diff_line.try(:type) - end - - def highlighted_diff_lines - Gitlab::Diff::Highlight.new(diff_lines).highlight - end - - def truncated_diff_lines - max_number_of_lines = 16 - prev_match_line = nil - prev_lines = [] - - highlighted_diff_lines.each do |line| - if line.type == "match" - prev_lines.clear - prev_match_line = line - else - prev_lines << line - - break if generate_line_code(line) == self.line_code - - prev_lines.shift if prev_lines.length >= max_number_of_lines - end - end - - prev_lines + def for_line?(line) + !line.meta? && diff_file.line_code(line) == self.line_code end # Check if this note is part of an "active" discussion @@ -102,7 +63,7 @@ class LegacyDiffNote < Note if noteable_diff parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line) - @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line_text } + @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line.text } else @active = false end @@ -110,10 +71,6 @@ class LegacyDiffNote < Note @active end - def award_emoji_supported? - false - end - private def find_diff @@ -149,10 +106,6 @@ class LegacyDiffNote < Note self.class.where(attributes).last.try(:diff) end - def generate_line_code(line) - Gitlab::Diff::LineCode.generate(diff_file_path, line.new_pos, line.old_pos) - end - # Find the diff on noteable that matches our own def find_noteable_diff diffs = noteable.diffs(Commit.max_diff_options) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4f7e1d2f302..083e93f1ee7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -16,7 +16,7 @@ class MergeRequest < ActiveRecord::Base serialize :merge_params, Hash - after_create :create_merge_request_diff, unless: :importing + after_create :create_merge_request_diff, unless: :importing? after_update :update_merge_request_diff delegate :commits, :diffs, :real_size, to: :merge_request_diff, prefix: nil @@ -29,10 +29,6 @@ class MergeRequest < ActiveRecord::Base # when creating new merge request attr_accessor :can_be_created, :compare_commits, :compare - # Temporary fields to store target_sha, and base_sha to - # compare when importing pull requests from GitHub - attr_accessor :base_target_sha, :head_source_sha - state_machine :state, initial: :opened do event :close do transition [:reopened, :opened] => :closed @@ -89,12 +85,7 @@ class MergeRequest < ActiveRecord::Base state :cannot_be_merged around_transition do |merge_request, transition, block| - merge_request.record_timestamps = false - begin - block.call - ensure - merge_request.record_timestamps = true - end + Gitlab::Timeless.timeless(merge_request, &block) end end @@ -169,10 +160,6 @@ class MergeRequest < ActiveRecord::Base reference end - def last_commit - merge_request_diff ? merge_request_diff.last_commit : compare_commits.last - end - def first_commit merge_request_diff ? merge_request_diff.first_commit : compare_commits.first end @@ -182,15 +169,86 @@ class MergeRequest < ActiveRecord::Base end def diff_base_commit - if merge_request_diff + if persisted? merge_request_diff.base_commit - elsif source_sha - self.target_project.merge_base_commit(self.source_sha, self.target_branch) + elsif diff_start_commit && diff_head_commit + self.target_project.merge_base_commit(diff_start_sha, diff_head_sha) + end + end + + # MRs created before 8.4 don't store a MergeRequestDiff#base_commit_sha, + # but we need to get a commit for the "View file @ ..." link by deleted files, + # so we find the likely one if we can't get the actual one. + # This will not be the actual base commit if the target branch was merged into + # the source branch after the merge request was created, but it is good enough + # for the specific purpose of linking to a commit. + # It is not good enough for use in `Gitlab::Git::DiffRefs`, which needs the + # true base commit, so we can't simply have `#diff_base_commit` fall back on + # this method. + def likely_diff_base_commit + first_commit.parent || first_commit + end + + def diff_start_commit + if persisted? + merge_request_diff.start_commit + else + target_branch_head + end + end + + def diff_head_commit + if persisted? + merge_request_diff.head_commit + else + source_branch_head end end - def last_commit_short_sha - last_commit.short_id + def diff_start_sha + diff_start_commit.try(:sha) + end + + def diff_base_sha + diff_base_commit.try(:sha) + end + + def diff_head_sha + diff_head_commit.try(:sha) + end + + # When importing a pull request from GitHub, the old and new branches may no + # longer actually exist by those names, but we need to recreate the merge + # request diff with the right source and target shas. + # We use these attributes to force these to the intended values. + attr_writer :target_branch_sha, :source_branch_sha + + def source_branch_head + source_branch_ref = @source_branch_sha || source_branch + source_project.repository.commit(source_branch) if source_branch_ref + end + + def target_branch_head + target_branch_ref = @target_branch_sha || target_branch + target_project.repository.commit(target_branch) if target_branch_ref + end + + def target_branch_sha + target_branch_head.try(:sha) + end + + def source_branch_sha + source_branch_head.try(:sha) + end + + def diff_refs + return unless diff_start_commit || diff_base_commit + + Gitlab::Diff::DiffRefs.new( + base_sha: diff_base_sha, + start_sha: diff_start_sha, + head_sha: diff_head_sha + ) end def validate_branches @@ -227,21 +285,30 @@ class MergeRequest < ActiveRecord::Base def update_merge_request_diff if source_branch_changed? || target_branch_changed? - reload_code + reload_diff end end - def reload_code - if merge_request_diff && open? - merge_request_diff.reload_content - end + def reload_diff + return unless merge_request_diff && open? + + old_diff_refs = self.diff_refs + + merge_request_diff.reload_content + + new_diff_refs = self.diff_refs + + update_diff_notes_positions( + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs + ) end def check_if_can_be_merged return unless unchecked? can_be_merged = - !broken? && project.repository.can_be_merged?(source_sha, target_branch) + !broken? && project.repository.can_be_merged?(diff_head_sha, target_branch) if can_be_merged mark_as_mergeable @@ -293,7 +360,7 @@ class MergeRequest < ActiveRecord::Base !source_project.protected_branch?(source_branch) && !source_project.root_ref?(source_branch) && Ability.abilities.allowed?(current_user, :push_code, source_project) && - last_commit == source_project.commit(source_branch) + diff_head_commit == source_branch_head end def should_remove_source_branch? @@ -331,8 +398,8 @@ class MergeRequest < ActiveRecord::Base work_in_progress: work_in_progress? } - if last_commit - attrs.merge!(last_commit: last_commit.hook_attrs) + if diff_head_commit + attrs.merge!(last_commit: diff_head_commit.hook_attrs) end attributes.merge!(attrs) @@ -510,22 +577,6 @@ class MergeRequest < ActiveRecord::Base end end - def target_sha - return @base_target_sha if defined?(@base_target_sha) - - target_project.repository.commit(target_branch).try(:sha) - end - - def source_sha - return @head_source_sha if defined?(@head_source_sha) - - last_commit.try(:sha) || source_tip.try(:sha) - end - - def source_tip - source_branch && source_project.repository.commit(source_branch) - end - def fetch_ref target_project.repository.fetch_ref( source_project.repository.path_to_repo, @@ -558,10 +609,10 @@ class MergeRequest < ActiveRecord::Base def diverged_commits_count cache = Rails.cache.read(:"merge_request_#{id}_diverged_commits") - if cache.blank? || cache[:source_sha] != source_sha || cache[:target_sha] != target_sha + if cache.blank? || cache[:source_sha] != source_branch_sha || cache[:target_sha] != target_branch_sha cache = { - source_sha: source_sha, - target_sha: target_sha, + source_sha: source_branch_sha, + target_sha: target_branch_sha, diverged_commits_count: compute_diverged_commits_count } Rails.cache.write(:"merge_request_#{id}_diverged_commits", cache) @@ -571,9 +622,9 @@ class MergeRequest < ActiveRecord::Base end def compute_diverged_commits_count - return 0 unless source_sha && target_sha + return 0 unless source_branch_sha && target_branch_sha - Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_sha, target_sha).size + Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_branch_sha, target_branch_sha).size end private :compute_diverged_commits_count @@ -582,13 +633,7 @@ class MergeRequest < ActiveRecord::Base end def pipeline - @pipeline ||= source_project.pipeline(last_commit.id, source_branch) if last_commit && source_project - end - - def diff_refs - return nil unless diff_base_commit - - [diff_base_commit, last_commit] + @pipeline ||= source_project.pipeline(diff_head_sha, source_branch) if diff_head_sha && source_project end def merge_commit @@ -603,6 +648,36 @@ class MergeRequest < ActiveRecord::Base merge_commit end + def support_new_diff_notes? + diff_refs && diff_refs.complete? + end + + def update_diff_notes_positions(old_diff_refs:, new_diff_refs:) + return unless support_new_diff_notes? + return if new_diff_refs == old_diff_refs + + active_diff_notes = self.notes.diff_notes.select do |note| + note.new_diff_note? && note.active?(old_diff_refs) + end + + return if active_diff_notes.empty? + + paths = active_diff_notes.flat_map { |n| n.diff_file.paths }.uniq + + service = Notes::DiffPositionUpdateService.new( + self.project, + nil, + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + paths: paths + ) + + active_diff_notes.each do |note| + service.execute(note) + Gitlab::Timeless.timeless(note, &:save) + end + end + def keep_around_commit project.repository.keep_around(self.merge_commit_sha) end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 0fcde6fc8f1..ba235750aeb 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -7,7 +7,7 @@ class MergeRequestDiff < ActiveRecord::Base belongs_to :merge_request - delegate :head_source_sha, :target_branch, :source_branch, to: :merge_request, prefix: nil + delegate :source_branch_sha, :target_branch_sha, :target_branch, :source_branch, to: :merge_request, prefix: nil state_machine :state, initial: :empty do state :collected @@ -24,7 +24,7 @@ class MergeRequestDiff < ActiveRecord::Base serialize :st_diffs after_create :reload_content, unless: :importing? - after_save :keep_around_commit + after_save :keep_around_commits, unless: :importing? def reload_content reload_commits @@ -39,9 +39,9 @@ class MergeRequestDiff < ActiveRecord::Base if options[:ignore_whitespace_change] @diffs_no_whitespace ||= begin compare = Gitlab::Git::Compare.new( - self.repository.raw_repository, - self.base, - self.head, + repository.raw_repository, + self.start_commit_sha || self.target_branch_sha, + self.head_commit_sha || self.source_branch_sha, ) compare.diffs(options) end @@ -63,37 +63,39 @@ class MergeRequestDiff < ActiveRecord::Base end def base_commit - return nil unless self.base_commit_sha + return unless self.base_commit_sha - merge_request.target_project.commit(self.base_commit_sha) + project.commit(self.base_commit_sha) end - def last_commit_short_sha - @last_commit_short_sha ||= last_commit.short_id - end + def start_commit + return unless self.start_commit_sha - def dump_commits(commits) - commits.map(&:to_hash) + project.commit(self.start_commit_sha) end - def load_commits(array) - array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash), merge_request.source_project) } - end + def head_commit + return last_commit unless self.head_commit_sha - def dump_diffs(diffs) - if diffs.respond_to?(:map) - diffs.map(&:to_hash) - end + project.commit(self.head_commit_sha) end - def load_diffs(raw, options) - if raw.respond_to?(:each) - Gitlab::Git::DiffCollection.new(raw, options) - else - Gitlab::Git::DiffCollection.new([]) - end + def compare + @compare ||= + begin + # Update ref for merge request + merge_request.fetch_ref + + Gitlab::Git::Compare.new( + repository.raw_repository, + self.target_branch_sha, + self.source_branch_sha + ) + end end + private + # Collect array of Git::Commit objects # between target and source branches def unmerged_commits @@ -106,6 +108,14 @@ class MergeRequestDiff < ActiveRecord::Base commits end + def dump_commits(commits) + commits.map(&:to_hash) + end + + def load_commits(array) + array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash), merge_request.source_project) } + end + # Reload all commits related to current merge request from repo # and save it as array of hashes in st_commits db field def reload_commits @@ -120,6 +130,26 @@ class MergeRequestDiff < ActiveRecord::Base update_columns_serialized(new_attributes) end + # Collect array of Git::Diff objects + # between target and source branches + def unmerged_diffs + compare.diffs(Commit.max_diff_options) + end + + def dump_diffs(diffs) + if diffs.respond_to?(:map) + diffs.map(&:to_hash) + end + end + + def load_diffs(raw, options) + if raw.respond_to?(:each) + Gitlab::Git::DiffCollection.new(raw, options) + else + Gitlab::Git::DiffCollection.new([]) + end + end + # Reload diffs between branches related to current merge request from repo # and save it as array of hashes in st_diffs db field def reload_diffs @@ -147,59 +177,33 @@ class MergeRequestDiff < ActiveRecord::Base new_attributes[:st_diffs] = new_diffs - base_commit_sha = self.repository.merge_base(self.head, self.base) - new_attributes[:base_commit_sha] = base_commit_sha - - self.repository.keep_around(base_commit_sha) + new_attributes[:start_commit_sha] = self.target_branch_sha + new_attributes[:head_commit_sha] = self.source_branch_sha + new_attributes[:base_commit_sha] = branch_base_sha update_columns_serialized(new_attributes) - end - # Collect array of Git::Diff objects - # between target and source branches - def unmerged_diffs - compare.diffs(Commit.max_diff_options) + keep_around_commits end - def repository - merge_request.target_project.repository + def project + merge_request.target_project end - def source_sha - return head_source_sha if head_source_sha.present? - - source_commit = merge_request.source_project.commit(source_branch) - source_commit.try(:sha) + def repository + project.repository end - def target_sha - merge_request.target_sha - end + def branch_base_commit + return unless self.source_branch_sha && self.target_branch_sha - def base - self.target_sha || self.target_branch + project.merge_base_commit(self.source_branch_sha, self.target_branch_sha) end - def head - self.source_sha + def branch_base_sha + branch_base_commit.try(:sha) end - def compare - @compare ||= - begin - # Update ref for merge request - merge_request.fetch_ref - - Gitlab::Git::Compare.new( - self.repository.raw_repository, - self.base, - self.head - ) - end - end - - private - # # #save or #update_attributes providing changes on serialized attributes do a lot of # serialization and deserialization calls resulting in bad performance. @@ -223,7 +227,9 @@ class MergeRequestDiff < ActiveRecord::Base reload end - def keep_around_commit - self.repository.keep_around(self.base_commit_sha) + def keep_around_commits + repository.keep_around(target_branch_sha) + repository.keep_around(source_branch_sha) + repository.keep_around(branch_base_sha) end end diff --git a/app/models/note.rb b/app/models/note.rb index 81b5c47b738..ffffd0c0838 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -56,7 +56,7 @@ class Note < ActiveRecord::Base scope :inc_author, ->{ includes(:author) } scope :inc_author_project_award_emoji, ->{ includes(:project, :author, :award_emoji) } - scope :legacy_diff_notes, ->{ where(type: 'LegacyDiffNote') } + scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) } scope :non_diff_notes, ->{ where(type: ['Note', nil]) } scope :with_associations, -> do @@ -82,7 +82,7 @@ class Note < ActiveRecord::Base end def grouped_diff_notes - legacy_diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code) + diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code) end # Searches for notes matching the given query. @@ -115,6 +115,10 @@ class Note < ActiveRecord::Base false end + def new_diff_note? + false + end + def active? true end @@ -193,7 +197,7 @@ class Note < ActiveRecord::Base end def award_emoji? - award_emoji_supported? && contains_emoji_only? + can_be_award_emoji? && contains_emoji_only? end def emoji_awardable? @@ -204,7 +208,7 @@ class Note < ActiveRecord::Base self.line_code = nil if self.line_code.blank? end - def award_emoji_supported? + def can_be_award_emoji? noteable.is_a?(Awardable) end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 27bf08bf7d9..97bcbacf2b9 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -144,7 +144,7 @@ class JiraService < IssueTrackerService commit_id = if entity.is_a?(Commit) entity.id elsif entity.is_a?(MergeRequest) - entity.last_commit.id + entity.diff_head_sha end commit_url = build_entity_url(:commit, commit_id) diff --git a/app/models/repository.rb b/app/models/repository.rb index d232d422195..ba66bc47c29 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -653,16 +653,6 @@ class Repository end end - def blob_for_diff(commit, diff) - blob_at(commit.id, diff.file_path) - end - - def prev_blob_for_diff(commit, diff) - if commit.parent_id - blob_at(commit.parent_id, diff.old_path) - end - end - def refs_contains_sha(ref_type, sha) args = %W(#{Gitlab.config.git.bin_path} #{ref_type} --contains #{sha}) names = Gitlab::Popen.popen(args, path_to_repo).first diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index a2df899d012..016172c6d7e 100644 --- a/app/models/sent_notification.rb +++ b/app/models/sent_notification.rb @@ -1,4 +1,6 @@ class SentNotification < ActiveRecord::Base + serialize :position, Gitlab::Diff::Position + belongs_to :project belongs_to :noteable, polymorphic: true belongs_to :recipient, class_name: "User" @@ -7,7 +9,7 @@ class SentNotification < ActiveRecord::Base validates :reply_key, uniqueness: true validates :noteable_id, presence: true, unless: :for_commit? validates :commit_id, presence: true, if: :for_commit? - validates :line_code, line_code: true, allow_blank: true + validate :note_valid after_save :keep_around_commit @@ -20,7 +22,7 @@ class SentNotification < ActiveRecord::Base find_by(reply_key: reply_key) end - def record(noteable, recipient_id, reply_key, params = {}) + def record(noteable, recipient_id, reply_key, attrs = {}) return unless reply_key noteable_id = nil @@ -31,7 +33,7 @@ class SentNotification < ActiveRecord::Base noteable_id = noteable.id end - params.reverse_merge!( + attrs.reverse_merge!( project: noteable.project, noteable_type: noteable.class.name, noteable_id: noteable_id, @@ -40,13 +42,17 @@ class SentNotification < ActiveRecord::Base reply_key: reply_key ) - create(params) + create(attrs) end - def record_note(note, recipient_id, reply_key, params = {}) - params[:line_code] = note.line_code + def record_note(note, recipient_id, reply_key, attrs = {}) + if note.diff_note? + attrs[:note_type] = note.type + + attrs.merge!(note.diff_attributes) + end - record(note.noteable, recipient_id, reply_key, params) + record(note.noteable, recipient_id, reply_key, attrs) end end @@ -70,8 +76,33 @@ class SentNotification < ActiveRecord::Base self.reply_key end + def note_attributes + { + project: self.project, + author: self.recipient, + type: self.note_type, + noteable_type: self.noteable_type, + noteable_id: self.noteable_id, + commit_id: self.commit_id, + line_code: self.line_code, + position: self.position.to_json + } + end + + def create_note(note) + Notes::CreateService.new( + self.project, + self.recipient, + self.note_attributes.merge(note: note) + ).execute + end + private + def note_valid + Note.new(note_attributes.merge(note: "Test")).valid? + end + def keep_around_commit project.repository.keep_around(self.commit_id) end |