Merge branch...

Merge branch '229679-migrate-data-toggle-modal-to-glmodal-for-revert-cherry-picking-commits-3' into 'master'

[MR Page] Migrate to GlModal for revert MR

See merge request gitlab-org/gitlab!52019

app/assets/javascripts/mr_notes/index.js

@@ -7,6 +7,7 @@ import initSortDiscussions from '../notes/sort_discussions';
 import MergeRequest from '../merge_request';
 import { resetServiceWorkersPublicPath } from '../lib/utils/webpack';
 import initNotesApp from './init_notes';
+import initRevertCommitModal from '~/projects/commit/init_revert_commit_modal';
 
 export default function initMrNotes() {
   resetServiceWorkersPublicPath();
@@ -19,6 +20,10 @@ export default function initMrNotes() {
 
   initNotesApp();
 
+  document.addEventListener('merged:UpdateActions', () => {
+    initRevertCommitModal();
+  });
+
   // eslint-disable-next-line no-new
   new Vue({
     el: '#js-vue-discussion-counter',

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue

@@ -7,6 +7,8 @@ import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import MrWidgetAuthorTime from '../mr_widget_author_time.vue';
 import statusIcon from '../mr_widget_status_icon.vue';
 import eventHub from '../../event_hub';
+import modalEventHub from '~/projects/commit/event_hub';
+import { OPEN_REVERT_MODAL } from '~/projects/commit/constants';
 
 export default {
   name: 'MRWidgetMerged',
@@ -77,6 +79,9 @@ export default {
       return s__('mrWidget|Cherry-pick');
     },
   },
+  mounted() {
+    document.dispatchEvent(new CustomEvent('merged:UpdateActions'));
+  },
   methods: {
     removeSourceBranch() {
       this.isMakingRequest = true;
@@ -98,6 +103,9 @@ export default {
           Flash(__('Something went wrong. Please try again.'));
         });
     },
+    openRevertModal() {
+      modalEventHub.$emit(OPEN_REVERT_MODAL);
+    },
   },
 };
 </script>
@@ -119,9 +127,7 @@ export default {
           size="small"
           category="secondary"
           variant="warning"
-          href="#modal-revert-commit"
-          data-toggle="modal"
-          data-container="body"
+          @click="openRevertModal"
         >
           {{ revertLabel }}
         </gl-button>

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -365,6 +365,7 @@ export default {
             const el = document.createElement('div');
             el.innerHTML = res.data;
             document.body.appendChild(el);
+            document.dispatchEvent(new CustomEvent('merged:UpdateActions'));
             Project.initRefSwitcher();
           }
         })

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

@@ -100,7 +100,7 @@
 = render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @merge_request.assignees, reviewers: @merge_request.reviewers, source_branch: @merge_request.source_branch
 
 - if @merge_request.can_be_reverted?(current_user)
-  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, title: @merge_request.title
+  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, pajamas: true
 - if @merge_request.can_be_cherry_picked?
   = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title
 

app/views/projects/merge_requests/widget/_commit_change_content.html.haml

 - if @merge_request.can_be_reverted?(current_user)
-  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, title: @merge_request.title
+  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, pajamas: true
 - if @merge_request.can_be_cherry_picked?
   = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title

changelogs/unreleased/229679-migrate-data-toggle-modal-to-glmodal-for-revert.yml

+---
+title: "[MR Page] Migrate to GlModal for revert MR"
+merge_request: 52019
+author:
+type: other

spec/features/merge_request/user_reverts_merge_request_spec.rb

@@ -17,46 +17,28 @@ RSpec.describe 'User reverts a merge request', :js do
 
     wait_for_requests
 
-    visit(merge_request_path(merge_request))
+    # do not reload the page by visiting, let javascript update the page as it will validate we have loaded the modal
+    # code correctly on page update that adds the `revert` button
   end
 
   it 'reverts a merge request', :sidekiq_might_not_need_inline do
-    find("a[href='#modal-revert-commit']").click
+    revert_commit
 
-    page.within('#modal-revert-commit') do
-      uncheck('create_merge_request')
-      click_button('Revert')
-    end
+    wait_for_requests
 
     expect(page).to have_content('The merge request has been successfully reverted.')
-
-    wait_for_requests
   end
 
   it 'does not revert a merge request that was previously reverted', :sidekiq_might_not_need_inline do
-    find("a[href='#modal-revert-commit']").click
+    revert_commit
 
-    page.within('#modal-revert-commit') do
-      uncheck('create_merge_request')
-      click_button('Revert')
-    end
-
-    find("a[href='#modal-revert-commit']").click
-
-    page.within('#modal-revert-commit') do
-      uncheck('create_merge_request')
-      click_button('Revert')
-    end
+    revert_commit
 
     expect(page).to have_content('Sorry, we cannot revert this merge request automatically.')
   end
 
   it 'reverts a merge request in a new merge request', :sidekiq_might_not_need_inline do
-    find("a[href='#modal-revert-commit']").click
-
-    page.within('#modal-revert-commit') do
-      click_button('Revert')
-    end
+    revert_commit(create_merge_request: true)
 
     expect(page).to have_content('The merge request has been successfully reverted. You can now submit a merge request to get this change into the original branch.')
   end
@@ -68,4 +50,13 @@ RSpec.describe 'User reverts a merge request', :js do
 
     expect(page).not_to have_link('Revert')
   end
+
+  def revert_commit(create_merge_request: false)
+    click_button('Revert')
+
+    page.within('[data-testid="modal-commit"]') do
+      uncheck('create_merge_request') unless create_merge_request
+      click_button('Revert')
+    end
+  end
 end

spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb

@@ -160,7 +160,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
 
         it 'merges the merge request' do
           expect(page).to have_content('Merged by')
-          expect(page).to have_link('Revert')
+          expect(page).to have_button('Revert')
         end
       end
 
@@ -357,7 +357,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
 
         it 'merges the merge request' do
           expect(page).to have_content('Merged by')
-          expect(page).to have_link('Revert')
+          expect(page).to have_button('Revert')
         end
       end
 

spec/features/user_sees_revert_modal_spec.rb

@@ -7,26 +7,33 @@ RSpec.describe 'Merge request > User sees revert modal', :js, :sidekiq_might_not
   let(:user) { project.creator }
   let(:merge_request) { create(:merge_request, source_project: project) }
 
+  shared_examples 'showing the revert modal' do
+    it 'shows the revert modal' do
+      click_button('Revert')
+
+      page.within('[data-testid="modal-commit"]') do
+        expect(page).to have_content 'Revert this merge request'
+      end
+    end
+  end
+
   before do
     sign_in(user)
     visit(project_merge_request_path(project, merge_request))
     click_button('Merge')
 
     wait_for_requests
-
-    visit(merge_request_path(merge_request))
-    click_link('Revert')
   end
 
-  it 'shows the revert modal' do
-    page.within('.modal-header') do
-      expect(page).to have_content 'Revert this merge request'
-    end
+  context 'without page reload after merge validates js correctly loaded' do
+    it_behaves_like 'showing the revert modal'
   end
 
-  it 'closes the revert modal with escape keypress' do
-    find('#modal-revert-commit').send_keys(:escape)
+  context 'with page reload validates js correctly loaded' do
+    before do
+      visit(merge_request_path(merge_request))
+    end
 
-    expect(page).not_to have_selector('#modal-revert-commit', visible: true)
+    it_behaves_like 'showing the revert modal'
   end
 end

spec/frontend/vue_mr_widget/components/states/mr_widget_merged_spec.js

 import Vue from 'vue';
 import mountComponent from 'helpers/vue_mount_component_helper';
+import { getByRole } from '@testing-library/dom';
 import mergedComponent from '~/vue_merge_request_widget/components/states/mr_widget_merged.vue';
 import eventHub from '~/vue_merge_request_widget/event_hub';
+import modalEventHub from '~/projects/commit/event_hub';
+import { OPEN_REVERT_MODAL } from '~/projects/commit/constants';
 
 describe('MRWidgetMerged', () => {
   let vm;
@@ -16,6 +19,7 @@ describe('MRWidgetMerged', () => {
   };
 
   beforeEach(() => {
+    jest.spyOn(document, 'dispatchEvent');
     const Component = Vue.extend(mergedComponent);
     const mr = {
       isRemovingSourceBranch: false,
@@ -147,6 +151,18 @@ describe('MRWidgetMerged', () => {
     });
   });
 
+  it('calls dispatchDocumentEvent to load in the modal component', () => {
+    expect(document.dispatchEvent).toHaveBeenCalledWith(new CustomEvent('merged:UpdateActions'));
+  });
+
+  it('emits event to open the revert modal on revert button click', () => {
+    const eventHubSpy = jest.spyOn(modalEventHub, '$emit');
+
+    getByRole(vm.$el, 'button', { name: /Revert/i }).click();
+
+    expect(eventHubSpy).toHaveBeenCalledWith(OPEN_REVERT_MODAL);
+  });
+
   it('has merged by information', () => {
     expect(vm.$el.textContent).toContain('Merged by');
     expect(vm.$el.textContent).toContain('Administrator');

spec/frontend/vue_mr_widget/mr_widget_options_spec.js

@@ -67,6 +67,7 @@ describe('MrWidgetOptions', () => {
 
   describe('default', () => {
     beforeEach(() => {
+      jest.spyOn(document, 'dispatchEvent');
       return createComponent();
     });
 
@@ -353,6 +354,9 @@ describe('MrWidgetOptions', () => {
           return nextTick().then(() => {
             expect(wrapper.vm.service.fetchMergeActionsContent).toHaveBeenCalled();
             expect(document.body.textContent).toContain('hello world');
+            expect(document.dispatchEvent).toHaveBeenCalledWith(
+              new CustomEvent('merged:UpdateActions'),
+            );
           });
         });
       });