Commit cd139ab0 authored by Phil Hughes's avatar Phil Hughes

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

Reverts changes in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45471
whilst also changing the widget polling to not include
the reverted properties in the response.
parent 6f2da83d
<script> <script>
import { GlIcon, GlSprintf, GlLink } from '@gitlab/ui'; import { GlIcon } from '@gitlab/ui';
import { __ } from '~/locale'; import { __ } from '~/locale';
import mrEventHub from '../eventhub'; import mrEventHub from '../eventhub';
...@@ -18,29 +18,16 @@ const STATUS = { ...@@ -18,29 +18,16 @@ const STATUS = {
export default { export default {
components: { components: {
GlIcon, GlIcon,
GlSprintf,
GlLink,
}, },
props: { props: {
initialState: { initialState: {
type: String, type: String,
required: true, required: true,
}, },
initialIsReverted: {
type: Boolean,
required: true,
},
initialRevertedPath: {
type: String,
required: false,
default: null,
},
}, },
data() { data() {
return { return {
state: this.initialState, state: this.initialState,
isReverted: this.initialIsReverted,
revertedPath: this.initialRevertedPath,
}; };
}, },
computed: { computed: {
...@@ -61,10 +48,8 @@ export default { ...@@ -61,10 +48,8 @@ export default {
mrEventHub.$off('mr.state.updated', this.updateState); mrEventHub.$off('mr.state.updated', this.updateState);
}, },
methods: { methods: {
updateState({ state, reverted, revertedPath }) { updateState({ state }) {
this.state = state; this.state = state;
this.reverted = reverted;
this.revertedPath = revertedPath;
}, },
}, },
}; };
...@@ -78,17 +63,7 @@ export default { ...@@ -78,17 +63,7 @@ export default {
data-testid="status-icon" data-testid="status-icon"
/> />
<span class="gl-display-none gl-display-sm-block"> <span class="gl-display-none gl-display-sm-block">
<gl-sprintf v-if="isReverted" :message="__('Merged (%{linkStart}reverted%{linkEnd})')"> {{ statusHumanName }}
<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>
</span> </span>
</div> </div>
</template> </template>
...@@ -2,7 +2,7 @@ import Vue from 'vue'; ...@@ -2,7 +2,7 @@ import Vue from 'vue';
import ZenMode from '~/zen_mode'; import ZenMode from '~/zen_mode';
import initIssuableSidebar from '~/init_issuable_sidebar'; import initIssuableSidebar from '~/init_issuable_sidebar';
import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable'; 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 initPipelines from '~/commit/pipelines/pipelines_bundle';
import initSourcegraph from '~/sourcegraph'; import initSourcegraph from '~/sourcegraph';
import loadAwardsHandler from '~/awards_handler'; import loadAwardsHandler from '~/awards_handler';
...@@ -29,8 +29,6 @@ export default function () { ...@@ -29,8 +29,6 @@ export default function () {
return h(StatusBox, { return h(StatusBox, {
props: { props: {
initialState: el.dataset.state, initialState: el.dataset.state,
initialIsReverted: parseBoolean(el.dataset.isReverted),
initialRevertedPath: el.dataset.revertedPath,
}, },
}); });
}, },
......
...@@ -158,8 +158,6 @@ export default class MergeRequestStore { ...@@ -158,8 +158,6 @@ export default class MergeRequestStore {
mrEventHub.$emit('mr.state.updated', { mrEventHub.$emit('mr.state.updated', {
state: this.mergeRequestState, state: this.mergeRequestState,
reverted: data.reverted,
reverted_path: data.revertedPath,
}); });
} }
......
...@@ -428,10 +428,6 @@ class Commit ...@@ -428,10 +428,6 @@ class Commit
end end
def has_been_reverted?(current_user, notes_association = nil) 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) ext = Gitlab::ReferenceExtractor.new(project, current_user)
notes_association ||= notes_with_associations notes_association ||= notes_with_associations
...@@ -439,7 +435,7 @@ class Commit ...@@ -439,7 +435,7 @@ class Commit
note.all_references(current_user, extractor: ext) note.all_references(current_user, extractor: ext)
end 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 end
def change_type_title(user) def change_type_title(user)
......
...@@ -1593,26 +1593,6 @@ class MergeRequest < ApplicationRecord ...@@ -1593,26 +1593,6 @@ class MergeRequest < ApplicationRecord
!merge_commit.has_been_reverted?(current_user, notes_association) !merge_commit.has_been_reverted?(current_user, notes_association)
end 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 def merged_at
strong_memoize(:merged_at) do strong_memoize(:merged_at) do
next unless merged? next unless merged?
......
...@@ -114,14 +114,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity ...@@ -114,14 +114,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
end end
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 private
delegate :current_user, to: :request delegate :current_user, to: :request
......
...@@ -40,12 +40,11 @@ ...@@ -40,12 +40,11 @@
.issuable-meta .issuable-meta
%ul.controls.d-flex.align-items-end %ul.controls.d-flex.align-items-end
- if merge_request.merged?
%li.issuable-status.d-none.d-sm-inline-block %li.issuable-status.d-none.d-sm-inline-block
- if merge_request.reverted_by_merge_request?(current_user)
= _('MERGED (REVERTED)')
- elsif merge_request.merged?
= _('MERGED') = _('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') = sprite_icon('cancel', css_class: 'gl-vertical-align-text-bottom')
= _('CLOSED') = _('CLOSED')
= render 'shared/merge_request_pipeline_status', merge_request: merge_request = render 'shared/merge_request_pipeline_status', merge_request: merge_request
......
...@@ -3,8 +3,6 @@ ...@@ -3,8 +3,6 @@
- can_reopen_merge_request = can?(current_user, :reopen_merge_request, @merge_request) - can_reopen_merge_request = can?(current_user, :reopen_merge_request, @merge_request)
- state_human_name, state_icon_name = state_name_with_icon(@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) - 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? - if @merge_request.closed_or_merged_without_fork?
.gl-alert.gl-alert-danger.gl-mb-5 .gl-alert.gl-alert-danger.gl-mb-5
...@@ -14,12 +12,9 @@ ...@@ -14,12 +12,9 @@
.detail-page-header.border-bottom-0.pt-0.pb-0 .detail-page-header.border-bottom-0.pt-0.pb-0
.detail-page-header-body .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!') = sprite_icon(state_icon_name, css_class: 'gl-display-block gl-display-sm-none!')
%span.gl-display-none.gl-display-sm-block %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 .issuable-meta
......
---
title: Indicate reverted status of a merged merge request
merge_request: 45471
author: Kev @KevSlashNull
type: added
...@@ -59,10 +59,6 @@ mainline: ...@@ -59,10 +59,6 @@ mainline:
git revert -m 2 7a39eb0 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 <!-- ## Troubleshooting
Include any troubleshooting steps that you can foresee. If you know beforehand what issues Include any troubleshooting steps that you can foresee. If you know beforehand what issues
......
...@@ -16965,9 +16965,6 @@ msgstr "" ...@@ -16965,9 +16965,6 @@ msgstr ""
msgid "MERGED" msgid "MERGED"
msgstr "" msgstr ""
msgid "MERGED (REVERTED)"
msgstr ""
msgid "MR widget|Back to the Merge request" msgid "MR widget|Back to the Merge request"
msgstr "" msgstr ""
...@@ -17730,18 +17727,9 @@ msgstr "" ...@@ -17730,18 +17727,9 @@ msgstr ""
msgid "MergeRequest|Search files (%{modifier_key}P)" msgid "MergeRequest|Search files (%{modifier_key}P)"
msgstr "" msgstr ""
msgid "MergeRequest|reverted"
msgstr ""
msgid "Merged" msgid "Merged"
msgstr "" msgstr ""
msgid "Merged (%{linkStart}reverted%{linkEnd})"
msgstr ""
msgid "Merged (%{reverted})"
msgstr ""
msgid "Merged MRs" msgid "Merged MRs"
msgstr "" msgstr ""
......
...@@ -42,7 +42,6 @@ describe('Merge request status box component', () => { ...@@ -42,7 +42,6 @@ describe('Merge request status box component', () => {
it('renders human readable test', () => { it('renders human readable test', () => {
factory({ factory({
initialState: testCase.state, initialState: testCase.state,
initialIsReverted: false,
}); });
expect(wrapper.text()).toContain(testCase.name); expect(wrapper.text()).toContain(testCase.name);
...@@ -51,7 +50,6 @@ describe('Merge request status box component', () => { ...@@ -51,7 +50,6 @@ describe('Merge request status box component', () => {
it('sets css class', () => { it('sets css class', () => {
factory({ factory({
initialState: testCase.state, initialState: testCase.state,
initialIsReverted: false,
}); });
expect(wrapper.classes()).toContain(testCase.class); expect(wrapper.classes()).toContain(testCase.class);
...@@ -60,7 +58,6 @@ describe('Merge request status box component', () => { ...@@ -60,7 +58,6 @@ describe('Merge request status box component', () => {
it('renders icon', () => { it('renders icon', () => {
factory({ factory({
initialState: testCase.state, initialState: testCase.state,
initialIsReverted: false,
}); });
expect(wrapper.find('[data-testid="status-icon"]').props('name')).toBe(testCase.icon); expect(wrapper.find('[data-testid="status-icon"]').props('name')).toBe(testCase.icon);
...@@ -68,29 +65,14 @@ describe('Merge request status box component', () => { ...@@ -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 () => { it('updates with eventhub event', async () => {
factory({ factory({
initialState: 'opened', initialState: 'opened',
initialIsReverted: false,
}); });
expect(wrapper.text()).toContain('Open'); expect(wrapper.text()).toContain('Open');
mrEventHub.$emit('mr.state.updated', { state: 'closed', reverted: false }); mrEventHub.$emit('mr.state.updated', { state: 'closed' });
await nextTick(); await nextTick();
......
...@@ -750,27 +750,6 @@ eos ...@@ -750,27 +750,6 @@ eos
end end
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 describe '#has_been_reverted?' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:issue) { create(:issue, author: user, project: project) } let(:issue) { create(:issue, author: user, project: project) }
......
...@@ -2455,85 +2455,6 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -2455,85 +2455,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
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 describe '#can_be_reverted?' do
subject { create(:merge_request, source_project: create(:project, :repository)) } subject { create(:merge_request, source_project: create(:project, :repository)) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment