Merge branch '219455-inappliable-reason-api' into 'master'

Include inapplicable_reason in API response

See merge request gitlab-org/gitlab!34149

app/models/suggestion.rb

@@ -38,11 +38,18 @@ class Suggestion < ApplicationRecord
   end
 
   def appliable?(cached: true)
-    !applied? &&
-      noteable.opened? &&
-      !outdated?(cached: cached) &&
-      different_content? &&
-      note.active?
+    inapplicable_reason(cached: cached).nil?
+  end
+
+  def inapplicable_reason(cached: true)
+    strong_memoize("inapplicable_reason_#{cached}") do
+      next :applied if applied?
+      next :merge_request_merged if noteable.merged?
+      next :merge_request_closed if noteable.closed?
+      next :source_branch_deleted unless noteable.source_branch_exists?
+      next :outdated if outdated?(cached: cached) || !note.active?
+      next :same_content unless different_content?
+    end
   end
 
   # Overwrites outdated column
@@ -53,6 +60,10 @@ class Suggestion < ApplicationRecord
     from_content != fetch_from_content
   end
 
+  def single_line?
+    lines_above.zero? && lines_below.zero?
+  end
+
   def target_line
     position.new_line
   end

app/serializers/suggestion_entity.rb

@@ -2,6 +2,7 @@
 
 class SuggestionEntity < API::Entities::Suggestion
   include RequestAwareEntity
+  include Gitlab::Utils::StrongMemoize
 
   unexpose :from_line, :to_line, :from_content, :to_content
   expose :diff_lines, using: DiffLineEntity do |suggestion|
@@ -9,7 +10,29 @@ class SuggestionEntity < API::Entities::Suggestion
   end
   expose :current_user do
     expose :can_apply do |suggestion|
-      Ability.allowed?(current_user, :apply_suggestion, suggestion)
+      can_apply?(suggestion)
+    end
+  end
+
+  expose :inapplicable_reason do |suggestion|
+    next _("You don't have write access to the source branch.") unless can_apply?(suggestion)
+    next if suggestion.appliable?
+
+    case suggestion.inapplicable_reason
+    when :merge_request_merged
+      _("This merge request was merged. To apply this suggestion, edit this file directly.")
+    when :merge_request_closed
+      _("This merge request is closed. To apply this suggestion, edit this file directly.")
+    when :source_branch_deleted
+      _("Can't apply as the source branch was deleted.")
+    when :outdated
+      phrase = suggestion.single_line? ? 'this line was' : 'these lines were'
+
+      _("Can't apply as %{phrase} changed in a more recent version.") % { phrase: phrase }
+    when :same_content
+      _("This suggestion already matches its content.")
+    else
+      _("Can't apply this suggestion.")
     end
   end
 
@@ -18,4 +41,10 @@ class SuggestionEntity < API::Entities::Suggestion
   def current_user
     request.current_user
   end
+
+  def can_apply?(suggestion)
+    strong_memoize(:can_apply) do
+      Ability.allowed?(current_user, :apply_suggestion, suggestion)
+    end
+  end
 end

locale/gitlab.pot

@@ -3987,9 +3987,18 @@ msgstr ""
 msgid "Can override approvers and approvals required per merge request"
 msgstr ""
 
+msgid "Can't apply as %{phrase} changed in a more recent version."
+msgstr ""
+
+msgid "Can't apply as the source branch was deleted."
+msgstr ""
+
 msgid "Can't apply as this line has changed or the suggestion already matches its content."
 msgstr ""
 
+msgid "Can't apply this suggestion."
+msgstr ""
+
 msgid "Can't create snippet: %{err}"
 msgstr ""
 
@@ -23554,9 +23563,15 @@ msgstr ""
 msgid "This merge request does not have accessibility reports"
 msgstr ""
 
+msgid "This merge request is closed. To apply this suggestion, edit this file directly."
+msgstr ""
+
 msgid "This merge request is locked."
 msgstr ""
 
+msgid "This merge request was merged. To apply this suggestion, edit this file directly."
+msgstr ""
+
 msgid "This namespace has already been taken! Please choose another one."
 msgstr ""
 
@@ -23647,6 +23662,9 @@ msgstr ""
 msgid "This subscription is for"
 msgstr ""
 
+msgid "This suggestion already matches its content."
+msgstr ""
+
 msgid "This timeout will take precedence when lower than project-defined timeout and accepts a human readable time input language like \"1 hour\". Values without specification represent seconds."
 msgstr ""
 
@@ -26294,6 +26312,9 @@ msgstr ""
 msgid "You don't have sufficient permission to perform this action."
 msgstr ""
 
+msgid "You don't have write access to the source branch."
+msgstr ""
+
 msgid "You don’t have access to Productivity Analytics in this group"
 msgstr ""
 

spec/models/suggestion_spec.rb

@@ -38,26 +38,106 @@ RSpec.describe Suggestion do
   end
 
   describe '#appliable?' do
-    context 'when patch is already applied' do
-      let(:suggestion) { create(:suggestion, :applied) }
+    let(:suggestion) { build(:suggestion) }
 
-      it 'returns false' do
-        expect(suggestion).not_to be_appliable
+    subject(:appliable) { suggestion.appliable? }
+
+    before do
+      allow(suggestion).to receive(:inapplicable_reason).and_return(inapplicable_reason)
+    end
+
+    context 'when inapplicable_reason is nil' do
+      let(:inapplicable_reason) { nil }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when inapplicable_reason is not nil' do
+      let(:inapplicable_reason) { :applied }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#inapplicable_reason' do
+    let(:merge_request) { create(:merge_request) }
+
+    let!(:note) do
+      create(
+        :diff_note_on_merge_request,
+        project: merge_request.project,
+        noteable: merge_request
+      )
+    end
+
+    let(:suggestion) { build(:suggestion, note: note) }
+
+    subject(:inapplicable_reason) { suggestion.inapplicable_reason }
+
+    context 'when suggestion is already applied' do
+      let(:suggestion) { build(:suggestion, :applied, note: note) }
+
+      it { is_expected.to eq(:applied) }
+    end
+
+    context 'when merge request was merged' do
+      before do
+        merge_request.mark_as_merged!
+      end
+
+      it { is_expected.to eq(:merge_request_merged) }
+    end
+
+    context 'when merge request is closed' do
+      before do
+        merge_request.close!
       end
+
+      it { is_expected.to eq(:merge_request_closed) }
     end
 
-    context 'when merge request is not opened' do
-      let(:merge_request) { create(:merge_request, :merged) }
-      let(:note) do
-        create(:diff_note_on_merge_request, project: merge_request.project,
-                                            noteable: merge_request)
+    context 'when source branch is deleted' do
+      before do
+        merge_request.project.repository.rm_branch(merge_request.author, merge_request.source_branch)
       end
 
-      let(:suggestion) { create(:suggestion, note: note) }
+      it { is_expected.to eq(:source_branch_deleted) }
+    end
 
-      it 'returns false' do
-        expect(suggestion).not_to be_appliable
+    context 'when content is outdated' do
+      before do
+        allow(suggestion).to receive(:outdated?).and_return(true)
+      end
+
+      it { is_expected.to eq(:outdated) }
+    end
+
+    context 'when note is outdated' do
+      before do
+        allow(note).to receive(:active?).and_return(false)
       end
+
+      it { is_expected.to eq(:outdated) }
+    end
+
+    context 'when applicable' do
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#single_line?' do
+    subject(:single_line) { suggestion.single_line? }
+
+    context 'when suggestion is for a single line' do
+      let(:suggestion) { build(:suggestion, lines_above: 0, lines_below: 0) }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when suggestion is for multiple lines' do
+      let(:suggestion) { build(:suggestion, lines_above: 2, lines_below: 0) }
+
+      it { is_expected.to eq(false) }
     end
   end
 end

spec/serializers/suggestion_entity_spec.rb

@@ -13,10 +13,119 @@ RSpec.describe SuggestionEntity do
   subject { entity.as_json }
 
   it 'exposes correct attributes' do
-    expect(subject.keys).to match_array([:id, :appliable, :applied, :diff_lines, :current_user])
+    expect(subject.keys).to match_array([:id, :appliable, :applied, :diff_lines, :current_user, :inapplicable_reason])
   end
 
   it 'exposes current user abilities' do
     expect(subject[:current_user]).to include(:can_apply)
   end
+
+  describe 'inapplicable_reason' do
+    let(:inapplicable_reason) { subject[:inapplicable_reason] }
+
+    before do
+      allow(Ability).to receive(:allowed?).and_call_original
+
+      allow(Ability)
+        .to receive(:allowed?)
+        .with(user, :apply_suggestion, suggestion)
+        .and_return(can_apply_suggestion)
+    end
+
+    context 'when user can apply suggestion' do
+      let(:can_apply_suggestion) { true }
+
+      before do
+        allow(suggestion).to receive(:appliable?).and_return(appliable)
+      end
+
+      context 'and suggestion is appliable' do
+        let(:appliable) { true }
+
+        it 'returns nil' do
+          expect(inapplicable_reason).to be_nil
+        end
+      end
+
+      context 'but suggestion is not applicable' do
+        let(:appliable) { false }
+
+        before do
+          allow(suggestion).to receive(:inapplicable_reason).and_return(reason)
+        end
+
+        context 'and merge request was merged' do
+          let(:reason) { :merge_request_merged }
+
+          it 'returns appropriate message' do
+            expect(inapplicable_reason).to eq("This merge request was merged. To apply this suggestion, edit this file directly.")
+          end
+        end
+
+        context 'and source branch was deleted' do
+          let(:reason) { :source_branch_deleted }
+
+          it 'returns appropriate message' do
+            expect(inapplicable_reason).to eq("Can't apply as the source branch was deleted.")
+          end
+        end
+
+        context 'and merge request is closed' do
+          let(:reason) { :merge_request_closed }
+
+          it 'returns appropriate message' do
+            expect(inapplicable_reason).to eq("This merge request is closed. To apply this suggestion, edit this file directly.")
+          end
+        end
+
+        context 'and suggestion is outdated' do
+          let(:reason) { :outdated }
+
+          before do
+            allow(suggestion).to receive(:single_line?).and_return(single_line)
+          end
+
+          context 'and suggestion is for a single line' do
+            let(:single_line) { true }
+
+            it 'returns appropriate message' do
+              expect(inapplicable_reason).to eq("Can't apply as this line was changed in a more recent version.")
+            end
+          end
+
+          context 'and suggestion is for multiple lines' do
+            let(:single_line) { false }
+
+            it 'returns appropriate message' do
+              expect(inapplicable_reason).to eq("Can't apply as these lines were changed in a more recent version.")
+            end
+          end
+        end
+
+        context 'and suggestion has the same content' do
+          let(:reason) { :same_content }
+
+          it 'returns appropriate message' do
+            expect(inapplicable_reason).to eq("This suggestion already matches its content.")
+          end
+        end
+
+        context 'and suggestion is inapplicable for other reasons' do
+          let(:reason) { :some_other_reason }
+
+          it 'returns default message' do
+            expect(inapplicable_reason).to eq("Can't apply this suggestion.")
+          end
+        end
+      end
+    end
+
+    context 'when user cannot apply suggestion' do
+      let(:can_apply_suggestion) { false }
+
+      it 'returns appropriate message' do
+        expect(inapplicable_reason).to eq("You don't have write access to the source branch.")
+      end
+    end
+  end
 end