Merge branch 'ph/mrRevertedPathRevert' into 'master'

Revert "Merge branch '35824-Indicate-Reverted-MRs-in-MR-Header' into 'master'"

See merge request gitlab-org/gitlab!51577

app/assets/javascripts/merge_request/components/status_box.vue

 <script>
-import { GlIcon, GlSprintf, GlLink } from '@gitlab/ui';
+import { GlIcon } from '@gitlab/ui';
 import { __ } from '~/locale';
 import mrEventHub from '../eventhub';
 
@@ -18,29 +18,16 @@ const STATUS = {
 export default {
   components: {
     GlIcon,
-    GlSprintf,
-    GlLink,
   },
   props: {
     initialState: {
       type: String,
       required: true,
     },
-    initialIsReverted: {
-      type: Boolean,
-      required: true,
-    },
-    initialRevertedPath: {
-      type: String,
-      required: false,
-      default: null,
-    },
   },
   data() {
     return {
       state: this.initialState,
-      isReverted: this.initialIsReverted,
-      revertedPath: this.initialRevertedPath,
     };
   },
   computed: {
@@ -61,10 +48,8 @@ export default {
     mrEventHub.$off('mr.state.updated', this.updateState);
   },
   methods: {
-    updateState({ state, reverted, revertedPath }) {
+    updateState({ state }) {
       this.state = state;
-      this.reverted = reverted;
-      this.revertedPath = revertedPath;
     },
   },
 };
@@ -78,17 +63,7 @@ export default {
       data-testid="status-icon"
     />
     <span class="gl-display-none gl-display-sm-block">
-      <gl-sprintf v-if="isReverted" :message="__('Merged (%{linkStart}reverted%{linkEnd})')">
-        <template #link="{ content }">
-          <gl-link
-            :href="revertedPath"
-            class="gl-reset-color! gl-text-decoration-underline"
-            data-testid="reverted-link"
-            >{{ content }}</gl-link
-          >
-        </template>
-      </gl-sprintf>
-      <template v-else>{{ statusHumanName }}</template>
+      {{ statusHumanName }}
     </span>
   </div>
 </template>

app/assets/javascripts/pages/projects/merge_requests/init_merge_request_show.js

@@ -2,7 +2,7 @@ import Vue from 'vue';
 import ZenMode from '~/zen_mode';
 import initIssuableSidebar from '~/init_issuable_sidebar';
 import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable';
-import { handleLocationHash, parseBoolean } from '~/lib/utils/common_utils';
+import { handleLocationHash } from '~/lib/utils/common_utils';
 import initPipelines from '~/commit/pipelines/pipelines_bundle';
 import initSourcegraph from '~/sourcegraph';
 import loadAwardsHandler from '~/awards_handler';
@@ -29,8 +29,6 @@ export default function () {
       return h(StatusBox, {
         props: {
           initialState: el.dataset.state,
-          initialIsReverted: parseBoolean(el.dataset.isReverted),
-          initialRevertedPath: el.dataset.revertedPath,
         },
       });
     },

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -158,8 +158,6 @@ export default class MergeRequestStore {
 
     mrEventHub.$emit('mr.state.updated', {
       state: this.mergeRequestState,
-      reverted: data.reverted,
-      reverted_path: data.revertedPath,
     });
   }
 

app/models/commit.rb

@@ -428,10 +428,6 @@ class Commit
   end
 
   def has_been_reverted?(current_user, notes_association = nil)
-    reverting_commit(current_user, notes_association).present?
-  end
-
-  def reverting_commit(current_user, notes_association = nil)
     ext = Gitlab::ReferenceExtractor.new(project, current_user)
     notes_association ||= notes_with_associations
 
@@ -439,7 +435,7 @@ class Commit
       note.all_references(current_user, extractor: ext)
     end
 
-    ext.commits.find { |commit_ref| commit_ref.reverts_commit?(self, current_user) }
+    ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self, current_user) }
   end
 
   def change_type_title(user)

app/models/merge_request.rb

@@ -1593,26 +1593,6 @@ class MergeRequest < ApplicationRecord
     !merge_commit.has_been_reverted?(current_user, notes_association)
   end
 
-  def reverted_by_merge_request?(current_user)
-    reverting_merge_request(current_user).present?
-  end
-
-  def reverting_merge_request(current_user)
-    return unless merge_commit
-    return unless merged_at
-
-    reverting_commit = merge_commit.reverting_commit(current_user, notes_with_associations)
-
-    if reverting_commit
-      MergeRequestsFinder.new(
-        current_user,
-        project_id: project.id,
-        commit_sha: reverting_commit.sha,
-        state: 'merged'
-      ).execute.first
-    end
-  end
-
   def merged_at
     strong_memoize(:merged_at) do
       next unless merged?

app/serializers/merge_request_poll_cached_widget_entity.rb

@@ -114,14 +114,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
     end
   end
 
-  expose :reverted do |merge_request|
-    merge_request.reverted_by_merge_request?(current_user)
-  end
-
-  expose :reverted_path, if: -> (mr) { mr.reverted_by_merge_request?(current_user) } do |merge_request|
-    merge_request_path(merge_request.reverting_merge_request(current_user))
-  end
-
   private
 
   delegate :current_user, to: :request

app/views/projects/merge_requests/_merge_request.html.haml

@@ -40,12 +40,11 @@
 
     .issuable-meta
       %ul.controls.d-flex.align-items-end
-        %li.issuable-status.d-none.d-sm-inline-block
-          - if merge_request.reverted_by_merge_request?(current_user)
-            = _('MERGED (REVERTED)')
-          - elsif merge_request.merged?
+        - if merge_request.merged?
+          %li.issuable-status.d-none.d-sm-inline-block
             = _('MERGED')
-          - elsif merge_request.closed?
+        - elsif merge_request.closed?
+          %li.issuable-status.d-none.d-sm-inline-block
             = sprite_icon('cancel', css_class: 'gl-vertical-align-text-bottom')
             = _('CLOSED')
         = render 'shared/merge_request_pipeline_status', merge_request: merge_request

app/views/projects/merge_requests/_mr_title.html.haml

@@ -3,8 +3,6 @@
 - can_reopen_merge_request = can?(current_user, :reopen_merge_request, @merge_request)
 - state_human_name, state_icon_name = state_name_with_icon(@merge_request)
 - are_close_and_open_buttons_hidden = merge_request_button_hidden?(@merge_request, true) && merge_request_button_hidden?(@merge_request, false)
-- is_reverted = @merge_request.reverted_by_merge_request?(current_user)
-- reverted_mr_path = is_reverted ? merge_request_path(@merge_request.reverting_merge_request(current_user)) : nil
 
 - if @merge_request.closed_or_merged_without_fork?
   .gl-alert.gl-alert-danger.gl-mb-5
@@ -14,13 +12,10 @@
 
 .detail-page-header.border-bottom-0.pt-0.pb-0
   .detail-page-header-body
-    .issuable-status-box.status-box.js-mr-status-box{ class: status_box_class(@merge_request), data: { state: @merge_request.state, is_reverted: is_reverted.to_s, reverted_path: reverted_mr_path } }
+    .issuable-status-box.status-box.js-mr-status-box{ class: status_box_class(@merge_request), data: { state: @merge_request.state } }
       = sprite_icon(state_icon_name, css_class: 'gl-display-block gl-display-sm-none!')
       %span.gl-display-none.gl-display-sm-block
-        - if @merge_request.reverted_by_merge_request?(current_user)
-          = _('Merged (%{reverted})').html_safe % { reverted: link_to(s_('MergeRequest|reverted'), reverted_mr_path, class: 'gl-reset-color! gl-text-decoration-underline') }
-        - else
-          = state_human_name
+        = state_human_name
 
     .issuable-meta
       #js-issuable-header-warnings

changelogs/unreleased/35824-Indicate-Reverted-MRs-in-MR-Header.yml

----
-title: Indicate reverted status of a merged merge request
-merge_request: 45471
-author: Kev @KevSlashNull
-type: added

doc/user/project/merge_requests/revert_changes.md

@@ -59,10 +59,6 @@ mainline:
 git revert -m 2 7a39eb0
 ```
 
-From [GitLab 13.8 onwards](https://gitlab.com/gitlab-org/gitlab/-/issues/35824), merge requests
-reverted by another merge request through one of the methods described in this document
-will display a link to the reverted merge request at the top-left corner within the **Merged** badge.
-
 <!-- ## Troubleshooting
 
 Include any troubleshooting steps that you can foresee. If you know beforehand what issues

locale/gitlab.pot

@@ -16971,9 +16971,6 @@ msgstr ""
 msgid "MERGED"
 msgstr ""
 
-msgid "MERGED (REVERTED)"
-msgstr ""
-
 msgid "MR widget|Back to the Merge request"
 msgstr ""
 
@@ -17730,18 +17727,9 @@ msgstr ""
 msgid "MergeRequest|Search files (%{modifier_key}P)"
 msgstr ""
 
-msgid "MergeRequest|reverted"
-msgstr ""
-
 msgid "Merged"
 msgstr ""
 
-msgid "Merged (%{linkStart}reverted%{linkEnd})"
-msgstr ""
-
-msgid "Merged (%{reverted})"
-msgstr ""
-
 msgid "Merged MRs"
 msgstr ""
 

spec/frontend/merge_request/components/status_box_spec.js

@@ -42,7 +42,6 @@ describe('Merge request status box component', () => {
       it('renders human readable test', () => {
         factory({
           initialState: testCase.state,
-          initialIsReverted: false,
         });
 
         expect(wrapper.text()).toContain(testCase.name);
@@ -51,7 +50,6 @@ describe('Merge request status box component', () => {
       it('sets css class', () => {
         factory({
           initialState: testCase.state,
-          initialIsReverted: false,
         });
 
         expect(wrapper.classes()).toContain(testCase.class);
@@ -60,7 +58,6 @@ describe('Merge request status box component', () => {
       it('renders icon', () => {
         factory({
           initialState: testCase.state,
-          initialIsReverted: false,
         });
 
         expect(wrapper.find('[data-testid="status-icon"]').props('name')).toBe(testCase.icon);
@@ -68,29 +65,14 @@ describe('Merge request status box component', () => {
     });
   });
 
-  describe('when merge request is reverted', () => {
-    it('renders a link to the reverted merge request', () => {
-      factory({
-        initialState: 'merged',
-        initialIsReverted: true,
-        initialRevertedPath: 'http://test.com',
-      });
-
-      expect(wrapper.find('[data-testid="reverted-link"]').attributes('href')).toBe(
-        'http://test.com',
-      );
-    });
-  });
-
   it('updates with eventhub event', async () => {
     factory({
       initialState: 'opened',
-      initialIsReverted: false,
     });
 
     expect(wrapper.text()).toContain('Open');
 
-    mrEventHub.$emit('mr.state.updated', { state: 'closed', reverted: false });
+    mrEventHub.$emit('mr.state.updated', { state: 'closed' });
 
     await nextTick();
 

spec/models/commit_spec.rb

@@ -750,27 +750,6 @@ eos
     end
   end
 
-  describe '#reverting_commit' do
-    let(:user) { create(:user) }
-    let(:issue) { create(:issue, author: user, project: project) }
-
-    it 'returns the reverting commit' do
-      create(:note_on_issue,
-             noteable: issue,
-             system: true,
-             note: commit.revert_description(user),
-             project: issue.project)
-
-      expect_next_instance_of(Commit) do |revert_commit|
-        expect(revert_commit).to receive(:reverts_commit?)
-          .with(commit, user)
-          .and_return(true)
-      end
-
-      expect(commit.reverting_commit(user, issue.notes_with_associations)).to eq(commit)
-    end
-  end
-
   describe '#has_been_reverted?' do
     let(:user) { create(:user) }
     let(:issue) { create(:issue, author: user, project: project) }

spec/models/merge_request_spec.rb

@@ -2455,85 +2455,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
-  describe '#reverting_merge_request' do
-    subject { create(:merge_request, source_project: create(:project, :repository)) }
-
-    context 'when there is no merge_commit for the MR' do
-      before do
-        subject.metrics.update!(merged_at: Time.current.utc)
-      end
-
-      it 'returns nil' do
-        expect(subject.reverting_merge_request(nil)).to be_nil
-      end
-    end
-
-    context 'when the MR has been merged' do
-      before do
-        MergeRequests::MergeService
-          .new(subject.target_project, subject.author, { sha: subject.diff_head_sha })
-          .execute(subject)
-      end
-
-      context 'when there is no revert commit' do
-        it 'returns nil' do
-          expect(subject.reverting_merge_request(nil)).to be_nil
-        end
-      end
-
-      context 'when there is no merged_at for the MR' do
-        before do
-          subject.metrics.update!(merged_at: nil)
-        end
-
-        it 'returns nil' do
-          expect(subject.reverting_merge_request(nil)).to be_nil
-        end
-      end
-
-      context 'when there is a revert commit by MR' do
-        let(:current_user) { subject.author }
-        let(:branch) { subject.source_branch }
-        let(:project) { subject.source_project }
-
-        let(:revert_commit_id) do
-          params = {
-            commit: subject.merge_commit,
-            branch_name: branch,
-            start_branch: branch
-          }
-
-          Commits::RevertService.new(project, current_user, params).execute[:result]
-        end
-
-        let(:revert_merge_request) do
-          create(
-            :merge_request,
-            author: subject.author,
-            target_project: subject.target_project,
-            source_project: subject.source_project,
-            merge_commit_sha: revert_commit_id,
-            description: "This reverts merge request !#{subject.id}")
-        end
-
-        it 'returns the reverting merge request' do
-          ProcessCommitWorker.new.perform(project.id,
-                                          current_user.id,
-                                          project.commit(revert_commit_id).to_hash,
-                                          project.default_branch == branch)
-
-          MergeRequests::MergeService.new(
-            subject.target_project,
-            subject.author,
-            { sha: revert_merge_request.diff_head_sha }
-          ).execute(revert_merge_request)
-
-          expect(subject.reverting_merge_request(current_user)).to eq(revert_merge_request)
-        end
-      end
-    end
-  end
-
   describe '#can_be_reverted?' do
     subject { create(:merge_request, source_project: create(:project, :repository)) }