Merge branch...

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

[MR Page] Migrate to GlModal for Cherry-pick MR

See merge request gitlab-org/gitlab!53169

app/assets/javascripts/mr_notes/index.js

 import Vue from 'vue';
 import store from '~/mr_notes/stores';
+import initCherryPickCommitModal from '~/projects/commit/init_cherry_pick_commit_modal';
 import initRevertCommitModal from '~/projects/commit/init_revert_commit_modal';
 import initDiffsApp from '../diffs';
 import discussionCounter from '../notes/components/discussion_counter.vue';
@@ -22,6 +23,7 @@ export default function initMrNotes() {
 
   document.addEventListener('merged:UpdateActions', () => {
     initRevertCommitModal();
+    initCherryPickCommitModal();
   });
 
   // eslint-disable-next-line no-new

app/assets/javascripts/projects/commit/components/branches_dropdown.vue

@@ -78,6 +78,7 @@ export default {
       :name="branch"
       :is-checked="isSelected(branch)"
       is-check-item
+      data-testid="dropdown-item"
       @click="selectBranch(branch)"
     >
       {{ branch }}

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

@@ -5,7 +5,7 @@ import { deprecatedCreateFlash as Flash } from '~/flash';
 import { s__, __ } from '~/locale';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import modalEventHub from '~/projects/commit/event_hub';
-import { OPEN_REVERT_MODAL } from '~/projects/commit/constants';
+import { OPEN_REVERT_MODAL, OPEN_CHERRY_PICK_MODAL } from '~/projects/commit/constants';
 import MrWidgetAuthorTime from '../mr_widget_author_time.vue';
 import statusIcon from '../mr_widget_status_icon.vue';
 import eventHub from '../../event_hub';
@@ -106,6 +106,9 @@ export default {
     openRevertModal() {
       modalEventHub.$emit(OPEN_REVERT_MODAL);
     },
+    openCherryPickModal() {
+      modalEventHub.$emit(OPEN_CHERRY_PICK_MODAL);
+    },
   },
 };
 </script>
@@ -148,9 +151,7 @@ export default {
           v-gl-tooltip.hover
           :title="cherryPickTitle"
           size="small"
-          href="#modal-cherry-pick-commit"
-          data-toggle="modal"
-          data-container="body"
+          @click="openCherryPickModal"
         >
           {{ cherryPickLabel }}
         </gl-button>

app/views/projects/commit/_change.html.haml

 - case type.to_s
 - when 'revert'
-  - label = s_('ChangeTypeAction|Revert')
-  - branch_label = s_('ChangeTypeActionLabel|Revert in branch')
   - revert_merge_request = _('Revert this merge request')
   - revert_commit = _('Revert this commit')
-  - description = s_('ChangeTypeAction|This will create a new commit in order to revert the existing changes.')
   - title = commit.merged_merge_request(current_user) ? revert_merge_request : revert_commit
 
-  - if defined?(pajamas)
   .js-revert-commit-modal{ data: { title: title,
     endpoint: revert_namespace_project_commit_path(commit, namespace_id: @project.namespace.full_path, project_id: @project),
     branch: @project.default_branch,
@@ -15,15 +11,10 @@
     branch_collaboration: @project.branch_allows_collaboration?(current_user, selected_branch).to_s,
     existing_branch: ERB::Util.html_escape(selected_branch),
     branches_endpoint: project_branches_path(@project) } }
-  - else
-    = render "projects/commit/commit_modal", title: title, type: type, commit: commit, branch_label: branch_label, description: description, label: label
 
 - when 'cherry-pick'
-  - label = s_('ChangeTypeAction|Cherry-pick')
-  - branch_label = s_('ChangeTypeActionLabel|Pick into branch')
   - title = commit.merged_merge_request(current_user) ? _('Cherry-pick this merge request') : _('Cherry-pick this commit')
 
-  - if defined?(pajamas)
   .js-cherry-pick-commit-modal{ data: { title: title,
     endpoint: cherry_pick_namespace_project_commit_path(commit, namespace_id: @project.namespace.full_path, project_id: @project),
     branch: @project.default_branch,
@@ -31,5 +22,3 @@
     branch_collaboration: @project.branch_allows_collaboration?(current_user, selected_branch).to_s,
     existing_branch: ERB::Util.html_escape(selected_branch),
     branches_endpoint: project_branches_path(@project) } }
-  - else
-    = render "projects/commit/commit_modal", title: title, type: type, commit: commit, branch_label: branch_label, description: description, label: label

app/views/projects/commit/_commit_modal.html.haml

-.modal{ id: "modal-#{type}-commit", tabindex: -1 }
-  .modal-dialog
-    .modal-content
-      .modal-header
-        %h3.page-title= title
-        %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') }
-          %span{ "aria-hidden": true } ×
-      .modal-body
-        - if description
-          %p= description
-        = form_tag [type.underscore, @project, commit], method: :post, remote: false, class: "js-#{type}-form js-requires-input" do
-          .form-group.branch
-            = label_tag 'start_branch', branch_label, class: 'label-bold'
-
-            = hidden_field_tag :start_branch, @project.default_branch, id: 'start_branch'
-            = dropdown_tag(@project.default_branch, options: { title: s_("BranchSwitcherTitle|Switch branch"), filter: true, placeholder: s_("BranchSwitcherPlaceholder|Search branches"), toggle_class: 'js-project-refs-dropdown dynamic', dropdown_class: 'dropdown-menu-selectable', data: { field_name: "start_branch", selected: @project.default_branch, start_branch: @project.default_branch, refs_url: project_branches_path(@project), submit_form_on_click: false } })
-
-            - if can?(current_user, :push_code, @project)
-              = render 'shared/new_merge_request_checkbox'
-            - else
-              = hidden_field_tag 'create_merge_request', 1, id: nil
-          .form-actions
-            = submit_tag label, class: 'gl-button btn btn-success'
-            = link_to _("Cancel"), '#', class: "gl-button btn btn-cancel", "data-dismiss" => "modal"
-
-            = render 'shared/projects/edit_information'

app/views/projects/commit/pipelines.html.haml

@@ -6,5 +6,5 @@
 = render 'projects/commit/pipelines_list', endpoint: pipelines_project_commit_path(@project, @commit.id)
 
 - if can_collaborate_with_project?(@project)
-  = render "projects/commit/change", type: 'revert', commit: @commit, pajamas: true
-  = render "projects/commit/change", type: 'cherry-pick', commit: @commit, pajamas: true
+  = render "projects/commit/change", type: 'revert', commit: @commit
+  = render "projects/commit/change", type: 'cherry-pick', commit: @commit

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

@@ -17,5 +17,5 @@
   .limited-width-notes
     = render "shared/notes/notes_with_form", :autocomplete => true
     - if can_collaborate_with_project?(@project)
-      = render "projects/commit/change", type: 'revert', commit: @commit, pajamas: true
-      = render "projects/commit/change", type: 'cherry-pick', commit: @commit, pajamas: true
+      = render "projects/commit/change", type: 'revert', commit: @commit
+      = render "projects/commit/change", type: 'cherry-pick', commit: @commit

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

@@ -103,8 +103,8 @@
 = 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, pajamas: true
+  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit
 - if @merge_request.can_be_cherry_picked?
-  = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title
+  = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit
 
 #js-review-bar

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, pajamas: true
+  = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit
 - if @merge_request.can_be_cherry_picked?
-  = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title
+  = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit

changelogs/unreleased/229679-migrate-data-toggle-modal-to-glmodal-cherry-picking-com.yml

+---
+title: "[MR Page] Migrate to GlModal for Cherry-pick MR"
+merge_request: 53169
+author:
+type: other

locale/gitlab.pot

@@ -4734,12 +4734,6 @@ msgstr ""
 msgid "Branch not loaded - %{branchId}"
 msgstr ""
 
-msgid "BranchSwitcherPlaceholder|Search branches"
-msgstr ""
-
-msgid "BranchSwitcherTitle|Switch branch"
-msgstr ""
-
 msgid "Branches"
 msgstr ""
 
@@ -5345,12 +5339,6 @@ msgstr ""
 msgid "ChangeReviewer|Unassigned"
 msgstr ""
 
-msgid "ChangeTypeActionLabel|Pick into branch"
-msgstr ""
-
-msgid "ChangeTypeActionLabel|Revert in branch"
-msgstr ""
-
 msgid "ChangeTypeAction|A new branch will be created in your fork and a new merge request will be started."
 msgstr ""
 

spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe 'Merge request > User cherry-picks', :js do
       it 'does not show a Cherry-pick button' do
         visit project_merge_request_path(project, merge_request)
 
-        expect(page).not_to have_link 'Cherry-pick'
+        expect(page).not_to have_button 'Cherry-pick'
       end
     end
 
@@ -40,7 +40,7 @@ RSpec.describe 'Merge request > User cherry-picks', :js do
       it 'shows a Cherry-pick button' do
         visit project_merge_request_path(project, merge_request)
 
-        expect(page).to have_link 'Cherry-pick'
+        expect(page).to have_button 'Cherry-pick'
       end
 
       it 'hides the cherry pick button for an archived project' do
@@ -48,7 +48,7 @@ RSpec.describe 'Merge request > User cherry-picks', :js do
 
         visit project_merge_request_path(project, merge_request)
 
-        expect(page).not_to have_link 'Cherry-pick'
+        expect(page).not_to have_button 'Cherry-pick'
       end
     end
 
@@ -56,18 +56,12 @@ RSpec.describe 'Merge request > User cherry-picks', :js do
       before do
         visit project_merge_request_path(project, merge_request)
 
-        click_link('Cherry-pick')
+        click_button('Cherry-pick')
       end
 
       it 'shows the cherry-pick modal' do
         expect(page).to have_content('Cherry-pick this merge request')
       end
-
-      it 'closes the cherry-pick modal with escape keypress' do
-        find('#modal-cherry-pick-commit').send_keys(:escape)
-
-        expect(page).not_to have_content('Start a new merge request with these changes')
-      end
     end
   end
 end

spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -77,15 +77,26 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
     end
 
     it 'allows me to merge, see cherry-pick modal and load branches list', :sidekiq_might_not_need_inline do
+      modal_selector = '[data-testid="modal-commit"]'
+
       wait_for_requests
       click_button 'Merge'
 
       wait_for_requests
-      click_link 'Cherry-pick'
-      page.find('.js-project-refs-dropdown').click
+
+      click_button 'Cherry-pick'
+
+      page.within(modal_selector) do
+        click_button 'master'
+      end
+
+      page.within("#{modal_selector} .dropdown-menu") do
+        find('[data-testid="dropdown-search-box"]').set('')
+
         wait_for_requests
 
-      expect(page.all('.js-cherry-pick-form .dropdown-content li').size).to be > 1
+        expect(page.all('[data-testid="dropdown-item"]').size).to be > 1
+      end
     end
   end
 

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

@@ -4,7 +4,7 @@ import mountComponent from 'helpers/vue_mount_component_helper';
 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';
+import { OPEN_REVERT_MODAL, OPEN_CHERRY_PICK_MODAL } from '~/projects/commit/constants';
 
 describe('MRWidgetMerged', () => {
   let vm;
@@ -163,6 +163,14 @@ describe('MRWidgetMerged', () => {
     expect(eventHubSpy).toHaveBeenCalledWith(OPEN_REVERT_MODAL);
   });
 
+  it('emits event to open the cherry-pick modal on cherry-pick button click', () => {
+    const eventHubSpy = jest.spyOn(modalEventHub, '$emit');
+
+    getByRole(vm.$el, 'button', { name: /Cherry-pick/i }).click();
+
+    expect(eventHubSpy).toHaveBeenCalledWith(OPEN_CHERRY_PICK_MODAL);
+  });
+
   it('has merged by information', () => {
     expect(vm.$el.textContent).toContain('Merged by');
     expect(vm.$el.textContent).toContain('Administrator');