diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-12-13 22:17:19 +0300 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-12-13 22:17:19 +0300 |
commit | ed3034bbb71d43b12944a9da29b5264cb3ff3312 (patch) | |
tree | 3110713f4455a4b3a830e177422663e082fc0eb9 /app/models | |
parent | eb81c1239ef86561b4304339609be32318419dbb (diff) |
Allow suggesting single line changes in diffs
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/concerns/noteable.rb | 4 | ||||
-rw-r--r-- | app/models/diff_note.rb | 13 | ||||
-rw-r--r-- | app/models/merge_request.rb | 5 | ||||
-rw-r--r-- | app/models/note.rb | 12 | ||||
-rw-r--r-- | app/models/suggestion.rb | 61 |
5 files changed, 94 insertions, 1 deletions
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index eb315058c3a..f2cad09e779 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -26,6 +26,10 @@ module Noteable DiscussionNote.noteable_types.include?(base_class_name) end + def supports_suggestion? + false + end + def discussions_rendered_on_frontend? false end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index c32008aa9c7..279603496b0 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -66,10 +66,23 @@ class DiffNote < Note self.original_position.diff_refs == diff_refs end + def supports_suggestion? + return false unless noteable.supports_suggestion? && on_text? + # We don't want to trigger side-effects of `diff_file` call. + return false unless file = fetch_diff_file + return false unless line = file.line_for_position(self.original_position) + + line&.suggestible? + end + def discussion_first_note? self == discussion.first_note end + def banzai_render_context(field) + super.merge(suggestions_filter_enabled: supports_suggestion?) + end + private def enqueue_diff_file_creation_job diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a13cac73d04..8052a54c504 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -363,6 +363,11 @@ class MergeRequest < ActiveRecord::Base end end + def supports_suggestion? + # Should be `true` when removing the FF. + Suggestion.feature_enabled? + end + # Calls `MergeWorker` to proceed with the merge process and # updates `merge_jid` with the MergeWorker#jid. # This helps tracking enqueued and ongoing merge jobs. diff --git a/app/models/note.rb b/app/models/note.rb index 17c7d97fa0a..becf14e9785 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -69,6 +69,12 @@ class Note < ActiveRecord::Base belongs_to :last_edited_by, class_name: 'User' has_many :todos + + # The delete_all definition is required here in order + # to generate the correct DELETE sql for + # suggestions.delete_all calls + has_many :suggestions, -> { order(:relative_order) }, + inverse_of: :note, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :events, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_one :system_note_metadata has_one :note_diff_file, inverse_of: :diff_note, foreign_key: :diff_note_id @@ -110,7 +116,7 @@ class Note < ActiveRecord::Base scope :inc_author, -> { includes(:author) } scope :inc_relations_for_view, -> do includes(:project, { author: :status }, :updated_by, :resolved_by, :award_emoji, - :system_note_metadata, :note_diff_file) + :system_note_metadata, :note_diff_file, :suggestions) end scope :with_notes_filter, -> (notes_filter) do @@ -226,6 +232,10 @@ class Note < ActiveRecord::Base Gitlab::HookData::NoteBuilder.new(self).build end + def supports_suggestion? + false + end + def for_commit? noteable_type == "Commit" end diff --git a/app/models/suggestion.rb b/app/models/suggestion.rb new file mode 100644 index 00000000000..cec5ea30f9d --- /dev/null +++ b/app/models/suggestion.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +class Suggestion < ApplicationRecord + FEATURE_FLAG = :diff_suggestions + + belongs_to :note, inverse_of: :suggestions + validates :note, presence: true + validates :commit_id, presence: true, if: :applied? + + delegate :original_position, :position, :diff_file, + :noteable, to: :note + + def self.feature_enabled? + Feature.enabled?(FEATURE_FLAG) + end + + def project + noteable.source_project + end + + def branch + noteable.source_branch + end + + # For now, suggestions only serve as a way to send patches that + # will change a single line (being able to apply multiple in the same place), + # which explains `from_line` and `to_line` being the same line. + # We'll iterate on that in https://gitlab.com/gitlab-org/gitlab-ce/issues/53310 + # when allowing multi-line suggestions. + def from_line + position.new_line + end + alias_method :to_line, :from_line + + def from_original_line + original_position.new_line + end + alias_method :to_original_line, :from_original_line + + # `from_line_index` and `to_line_index` represents diff/blob line numbers in + # index-like way (N-1). + def from_line_index + from_line - 1 + end + alias_method :to_line_index, :from_line_index + + def appliable? + return false unless note.supports_suggestion? + + !applied? && + noteable.opened? && + different_content? && + note.active? + end + + private + + def different_content? + from_content != to_content + end +end |