Merge branch...

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

[Commit Page] Migrate to GlModal for cherry-pick commit

See merge request gitlab-org/gitlab!51650

app/assets/javascripts/pages/projects/commit/pipelines/index.js

 import { initCommitBoxInfo } from '~/projects/commit_box/info';
 import initPipelines from '~/commit/pipelines/pipelines_bundle';
+import initCommitActions from '~/projects/commit';
 
 initCommitBoxInfo();
 initPipelines();
+initCommitActions();

app/assets/javascripts/pages/projects/commit/show/index.js

@@ -14,8 +14,7 @@ import flash from '~/flash';
 import { __ } from '~/locale';
 import loadAwardsHandler from '~/awards_handler';
 import { initCommitBoxInfo } from '~/projects/commit_box/info';
-import initRevertCommitTrigger from '~/projects/commit/init_revert_commit_trigger';
-import initRevertCommitModal from '~/projects/commit/init_revert_commit_modal';
+import initCommitActions from '~/projects/commit';
 
 const hasPerfBar = document.querySelector('.with-performance-bar');
 const performanceHeight = hasPerfBar ? 35 : 0;
@@ -47,5 +46,4 @@ if (filesContainer.length) {
   new Diff();
 }
 loadAwardsHandler();
-initRevertCommitModal();
-initRevertCommitTrigger();
+initCommitActions();

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

@@ -68,6 +68,7 @@ export default {
       autocomplete="off"
       :debounce="250"
       :placeholder="$options.i18n.searchPlaceholder"
+      data-testid="dropdown-search-box"
       @input="searchTermChanged"
     />
     <gl-dropdown-item

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

@@ -10,6 +10,9 @@ export default {
     displayText: {
       default: '',
     },
+    testId: {
+      default: '',
+    },
   },
   props: {
     openModal: {
@@ -26,7 +29,7 @@ export default {
 </script>
 
 <template>
-  <gl-link data-is-link="true" data-testid="revert-commit-link" @click="showModal">
+  <gl-link data-is-link="true" :data-testid="testId" @click="showModal">
     {{ displayText }}
   </gl-link>
 </template>

app/assets/javascripts/projects/commit/constants.js

@@ -2,6 +2,10 @@ import { s__, __ } from '~/locale';
 
 export const OPEN_REVERT_MODAL = 'openRevertModal';
 export const REVERT_MODAL_ID = 'revert-commit-modal';
+export const REVERT_LINK_TEST_ID = 'revert-commit-link';
+export const OPEN_CHERRY_PICK_MODAL = 'openCherryPickModal';
+export const CHERRY_PICK_MODAL_ID = 'cherry-pick-commit-modal';
+export const CHERRY_PICK_LINK_TEST_ID = 'cherry-pick-commit-link';
 
 export const I18N_MODAL = {
   startMergeRequest: s__('ChangeTypeAction|Start a %{newMergeRequest} with these changes'),
@@ -20,6 +24,11 @@ export const I18N_REVERT_MODAL = {
   actionPrimaryText: s__('ChangeTypeAction|Revert'),
 };
 
+export const I18N_CHERRY_PICK_MODAL = {
+  branchLabel: s__('ChangeTypeAction|Pick into branch'),
+  actionPrimaryText: s__('ChangeTypeAction|Cherry-pick'),
+};
+
 export const PREPENDED_MODAL_TEXT = s__(
   'ChangeTypeAction|This will create a new commit in order to revert the existing changes.',
 );

app/assets/javascripts/projects/commit/index.js

+import initRevertCommitTrigger from './init_revert_commit_trigger';
+import initRevertCommitModal from './init_revert_commit_modal';
+import initCherryPickCommitTrigger from './init_cherry_pick_commit_trigger';
+import initCherryPickCommitModal from './init_cherry_pick_commit_modal';
+
+export default () => {
+  initRevertCommitModal();
+  initRevertCommitTrigger();
+  initCherryPickCommitModal();
+  initCherryPickCommitTrigger();
+};

app/assets/javascripts/projects/commit/init_cherry_pick_commit_modal.js

+import Vue from 'vue';
+import CommitFormModal from './components/form_modal.vue';
+import { parseBoolean } from '~/lib/utils/common_utils';
+import createStore from './store';
+import {
+  I18N_MODAL,
+  I18N_CHERRY_PICK_MODAL,
+  OPEN_CHERRY_PICK_MODAL,
+  CHERRY_PICK_MODAL_ID,
+} from './constants';
+
+export default function initInviteMembersModal() {
+  const el = document.querySelector('.js-cherry-pick-commit-modal');
+  if (!el) {
+    return false;
+  }
+
+  const {
+    title,
+    endpoint,
+    branch,
+    pushCode,
+    branchCollaboration,
+    existingBranch,
+    branchesEndpoint,
+  } = el.dataset;
+
+  const store = createStore({
+    endpoint,
+    branchesEndpoint,
+    branch,
+    pushCode: parseBoolean(pushCode),
+    branchCollaboration: parseBoolean(branchCollaboration),
+    defaultBranch: branch,
+    modalTitle: title,
+    existingBranch,
+  });
+
+  return new Vue({
+    el,
+    store,
+    render: (createElement) =>
+      createElement(CommitFormModal, {
+        props: {
+          i18n: { ...I18N_CHERRY_PICK_MODAL, ...I18N_MODAL },
+          openModal: OPEN_CHERRY_PICK_MODAL,
+          modalId: CHERRY_PICK_MODAL_ID,
+        },
+      }),
+  });
+}

app/assets/javascripts/projects/commit/init_cherry_pick_commit_trigger.js

+import Vue from 'vue';
+import CommitFormTrigger from './components/form_trigger.vue';
+import { OPEN_CHERRY_PICK_MODAL, CHERRY_PICK_LINK_TEST_ID } from './constants';
+
+export default function initInviteMembersTrigger() {
+  const el = document.querySelector('.js-cherry-pick-commit-trigger');
+
+  if (!el) {
+    return false;
+  }
+
+  const { displayText } = el.dataset;
+
+  return new Vue({
+    el,
+    provide: { displayText, testId: CHERRY_PICK_LINK_TEST_ID },
+    render: (createElement) =>
+      createElement(CommitFormTrigger, { props: { openModal: OPEN_CHERRY_PICK_MODAL } }),
+  });
+}

app/assets/javascripts/projects/commit/init_revert_commit_trigger.js

 import Vue from 'vue';
-import RevertCommitTrigger from './components/form_trigger.vue';
-import { OPEN_REVERT_MODAL } from './constants';
+import CommitFormTrigger from './components/form_trigger.vue';
+import { OPEN_REVERT_MODAL, REVERT_LINK_TEST_ID } from './constants';
 
 export default function initInviteMembersTrigger() {
   const el = document.querySelector('.js-revert-commit-trigger');
@@ -13,8 +13,8 @@ export default function initInviteMembersTrigger() {
 
   return new Vue({
     el,
-    provide: { displayText },
+    provide: { displayText, testId: REVERT_LINK_TEST_ID },
     render: (createElement) =>
-      createElement(RevertCommitTrigger, { props: { openModal: OPEN_REVERT_MODAL } }),
+      createElement(CommitFormTrigger, { props: { openModal: OPEN_REVERT_MODAL } }),
   });
 }

app/helpers/commits_helper.rb

@@ -110,20 +110,16 @@ module CommitsHelper
     end
   end
 
-  def revert_commit_link(commit, continue_to_path, btn_class: nil, pajamas: false)
+  def revert_commit_link
     return unless current_user
 
-    action = 'revert'
-
-    if pajamas && can_collaborate_with_project?(@project)
-      tag(:div, data: { display_text: action.capitalize }, class: "js-revert-commit-trigger")
-    else
-      commit_action_link(action, commit, continue_to_path, btn_class: btn_class, has_tooltip: false)
-    end
+    tag(:div, data: { display_text: 'Revert' }, class: "js-revert-commit-trigger")
   end
 
-  def cherry_pick_commit_link(commit, continue_to_path, btn_class: nil, has_tooltip: true)
-    commit_action_link('cherry-pick', commit, continue_to_path, btn_class: btn_class, has_tooltip: has_tooltip)
+  def cherry_pick_commit_link
+    return unless current_user
+
+    tag(:div, data: { display_text: 'Cherry-pick' }, class: "js-cherry-pick-commit-trigger")
   end
 
   def commit_signature_badge_classes(additional_classes)
@@ -166,28 +162,6 @@ module CommitsHelper
     end
   end
 
-  def commit_action_link(action, commit, continue_to_path, btn_class: nil, has_tooltip: true)
-    return unless current_user
-
-    tooltip = "#{action.capitalize} this #{commit.change_type_title(current_user)} in a new merge request" if has_tooltip
-    btn_class = "btn btn-#{btn_class}" unless btn_class.nil?
-
-    if can_collaborate_with_project?(@project)
-      link_to action.capitalize, "#modal-#{action}-commit", 'data-toggle' => 'modal', 'data-container' => 'body', title: (tooltip if has_tooltip), class: "#{btn_class} #{'has-tooltip' if has_tooltip}"
-    elsif can?(current_user, :fork_project, @project)
-      continue_params = {
-        to: continue_to_path,
-        notice: "#{edit_in_new_fork_notice} Try to #{action} this commit again.",
-        notice_now: edit_in_new_fork_notice_now
-      }
-      fork_path = project_forks_path(@project,
-        namespace_key: current_user.namespace.id,
-        continue: continue_params)
-
-      link_to action.capitalize, fork_path, class: btn_class, method: :post, 'data-toggle' => 'tooltip', 'data-container' => 'body', title: (tooltip if has_tooltip)
-    end
-  end
-
   def view_file_button(commit_sha, diff_new_path, project, replaced: false)
     path = project_blob_path(project, tree_join(commit_sha, diff_new_path))
     title = replaced ? _('View replaced file @ ') : _('View file @ ')

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

@@ -22,4 +22,14 @@
   - 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,
+      push_code: can?(current_user, :push_code, @project).to_s,
+      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_box.html.haml

@@ -37,10 +37,10 @@
             #{ _('Browse Files') }
         - if can_collaborate && !@commit.has_been_reverted?(current_user)
           %li.clearfix
-            = revert_commit_link(@commit, project_commit_path(@project, @commit.id), pajamas: true)
+            = revert_commit_link
         - if can_collaborate
           %li.clearfix
-            = cherry_pick_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
+            = cherry_pick_commit_link
         - if can?(current_user, :push_code, @project)
           %li.clearfix
             = link_to s_('CreateTag|Tag'), new_project_tag_path(@project, ref: @commit)

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

@@ -4,3 +4,7 @@
 = render 'commit_box'
 = render 'ci_menu'
 = 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

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

@@ -18,4 +18,4 @@
     = 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, title: @commit.title
+      = render "projects/commit/change", type: 'cherry-pick', commit: @commit, pajamas: true

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

+---
+title: "[Commit Page] Migrate to GlModal for cherry-pick commit"
+merge_request: 51650
+author:
+type: other

locale/gitlab.pot

@@ -5269,6 +5269,9 @@ msgstr ""
 msgid "ChangeTypeAction|Cherry-pick"
 msgstr ""
 
+msgid "ChangeTypeAction|Pick into branch"
+msgstr ""
+
 msgid "ChangeTypeAction|Revert"
 msgstr ""
 

spec/features/projects/commit/cherry_pick_spec.rb

@@ -2,108 +2,126 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Cherry-pick Commits' do
-  let(:user) { create(:user) }
-  let(:group) { create(:group) }
-  let(:project) { create(:project, :repository, namespace: group) }
-  let(:master_pickable_commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') }
-  let(:master_pickable_merge) { project.commit('e56497bb5f03a90a51293fc6d516788730953899') }
+RSpec.describe 'Cherry-pick Commits', :js do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' }
+  let!(:project) { create_default(:project, :repository, namespace: user.namespace) }
+  let(:master_pickable_commit) { project.commit(sha) }
 
   before do
     sign_in(user)
-    project.add_maintainer(user)
-    visit project_commit_path(project, master_pickable_commit.id)
   end
 
-  context "I cherry-pick a commit" do
-    it do
-      find("a[href='#modal-cherry-pick-commit']").click
-      expect(page).not_to have_content('v1.0.0') # Only branches, not tags
-      page.within('#modal-cherry-pick-commit') do
-        uncheck 'create_merge_request'
-        click_button 'Cherry-pick'
+  context 'when clicking cherry-pick from the dropdown for a commit on pipelines tab' do
+    it 'launches the modal form' do
+      create(:ci_empty_pipeline, sha: sha)
+      visit project_commit_path(project, master_pickable_commit.id)
+      click_link 'Pipelines'
+
+      open_modal
+
+      page.within(modal_selector) do
+        expect(page).to have_content('Cherry-pick this commit')
       end
-      expect(page).to have_content('The commit has been successfully cherry-picked into master.')
     end
   end
 
-  context "I cherry-pick a merge commit" do
-    it do
-      find("a[href='#modal-cherry-pick-commit']").click
-      page.within('#modal-cherry-pick-commit') do
-        uncheck 'create_merge_request'
-        click_button 'Cherry-pick'
+  context 'when starting from the commit tab' do
+    before do
+      visit project_commit_path(project, master_pickable_commit.id)
     end
+
+    context 'when cherry-picking a commit' do
+      specify do
+        cherry_pick_commit
+
         expect(page).to have_content('The commit has been successfully cherry-picked into master.')
       end
     end
 
-  context "I cherry-pick a commit that was previously cherry-picked" do
-    it do
-      find("a[href='#modal-cherry-pick-commit']").click
-      page.within('#modal-cherry-pick-commit') do
-        uncheck 'create_merge_request'
-        click_button 'Cherry-pick'
+    context 'when cherry-picking a merge commit' do
+      specify do
+        cherry_pick_commit
+
+        expect(page).to have_content('The commit has been successfully cherry-picked into master.')
       end
-      visit project_commit_path(project, master_pickable_commit.id)
-      find("a[href='#modal-cherry-pick-commit']").click
-      page.within('#modal-cherry-pick-commit') do
-        uncheck 'create_merge_request'
-        click_button 'Cherry-pick'
     end
+
+    context 'when cherry-picking a commit that was previously cherry-picked' do
+      specify do
+        cherry_pick_commit
+
+        visit project_commit_path(project, master_pickable_commit.id)
+
+        cherry_pick_commit
+
         expect(page).to have_content('Sorry, we cannot cherry-pick this commit automatically.')
       end
     end
 
-  context "I cherry-pick a commit in a new merge request", :js do
-    it do
-      find('.header-action-buttons a.dropdown-toggle').click
-      find("a[href='#modal-cherry-pick-commit']").click
-      page.within('#modal-cherry-pick-commit') do
-        click_button 'Cherry-pick'
-      end
-
-      wait_for_requests
+    context 'when cherry-picking a commit in a new merge request' do
+      specify do
+        cherry_pick_commit(create_merge_request: true)
 
         expect(page).to have_content("The commit has been successfully cherry-picked into cherry-pick-#{master_pickable_commit.short_id}. You can now submit a merge request to get this change into the original branch.")
         expect(page).to have_content("From cherry-pick-#{master_pickable_commit.short_id} into master")
       end
     end
 
-  context "I cherry-pick a commit from a different branch", :js do
-    it do
-      find('.header-action-buttons a.dropdown-toggle').click
-      find(:css, "a[href='#modal-cherry-pick-commit']").click
+    context 'when I cherry-picking a commit from a different branch' do
+      specify do
+        open_modal
 
-      page.within('#modal-cherry-pick-commit') do
+        page.within(modal_selector) do
           click_button 'master'
         end
 
+        page.within("#{modal_selector} .dropdown-menu") do
+          find('[data-testid="dropdown-search-box"]').set('feature')
           wait_for_requests
-
-      page.within('#modal-cherry-pick-commit .dropdown-menu') do
-        find('.dropdown-input input').set('feature')
-        wait_for_requests
-        click_link "feature"
+          click_button 'feature'
         end
 
-      page.within('#modal-cherry-pick-commit') do
-        uncheck 'create_merge_request'
-        click_button 'Cherry-pick'
-      end
+        submit_cherry_pick
 
         expect(page).to have_content('The commit has been successfully cherry-picked into feature.')
       end
     end
 
     context 'when the project is archived' do
-    let(:project) { create(:project, :repository, :archived, namespace: group) }
+      let(:project) { create(:project, :repository, :archived, namespace: user.namespace) }
 
       it 'does not show the cherry-pick link' do
-      find('.header-action-buttons a.dropdown-toggle').click
+        open_dropdown
 
         expect(page).not_to have_text("Cherry-pick")
-      expect(page).not_to have_css("a[href='#modal-cherry-pick-commit']")
       end
     end
+  end
+
+  def cherry_pick_commit(create_merge_request: false)
+    open_modal
+
+    submit_cherry_pick(create_merge_request: create_merge_request)
+  end
+
+  def open_dropdown
+    find('.header-action-buttons .dropdown').click
+  end
+
+  def open_modal
+    open_dropdown
+    find('[data-testid="cherry-pick-commit-link"]').click
+  end
+
+  def submit_cherry_pick(create_merge_request: false)
+    page.within(modal_selector) do
+      uncheck('create_merge_request') unless create_merge_request
+      click_button('Cherry-pick')
+    end
+  end
+
+  def modal_selector
+    '[data-testid="modal-commit"]'
+  end
 end

spec/features/projects/commit/user_reverts_commit_spec.rb

@@ -6,24 +6,32 @@ RSpec.describe 'User reverts a commit', :js do
   include RepoHelpers
 
   let_it_be(:user) { create(:user) }
-  let(:project) { create(:project, :repository, namespace: user.namespace) }
+  let!(:project) { create_default(:project, :repository, namespace: user.namespace) }
 
   before do
     sign_in(user)
-
-    visit(project_commit_path(project, sample_commit.id))
   end
 
-  def revert_commit(create_merge_request: false)
-    find('.header-action-buttons .dropdown').click
-    find('[data-testid="revert-commit-link"]').click
+  context 'when clicking revert from the dropdown for a commit on pipelines tab' do
+    it 'launches the modal and is able to submit the revert' do
+      sha = '7d3b0f7cff5f37573aea97cebfd5692ea1689924'
+      create(:ci_empty_pipeline, sha: sha)
+      visit project_commit_path(project, project.commit(sha).id)
+      click_link 'Pipelines'
 
-    page.within('[data-testid="modal-commit"]') do
-      uncheck('create_merge_request') unless create_merge_request
-      click_button('Revert')
+      open_modal
+
+      page.within(modal_selector) do
+        expect(page).to have_content('Revert this commit')
+      end
     end
   end
 
+  context 'when starting from the commit tab' do
+    before do
+      visit project_commit_path(project, sample_commit.id)
+    end
+
     context 'without creating a new merge request' do
       it 'reverts a commit' do
         revert_commit
@@ -55,9 +63,32 @@ RSpec.describe 'User reverts a commit', :js do
       let(:project) { create(:project, :repository, :archived, namespace: user.namespace) }
 
       it 'does not show the revert link' do
-      find('.header-action-buttons .dropdown').click
+        open_dropdown
 
         expect(page).not_to have_link('Revert')
       end
     end
+  end
+
+  def revert_commit(create_merge_request: false)
+    open_modal
+
+    page.within(modal_selector) do
+      uncheck('create_merge_request') unless create_merge_request
+      click_button('Revert')
+    end
+  end
+
+  def open_dropdown
+    find('.header-action-buttons .dropdown').click
+  end
+
+  def open_modal
+    open_dropdown
+    find('[data-testid="revert-commit-link"]').click
+  end
+
+  def modal_selector
+    '[data-testid="modal-commit"]'
+  end
 end

spec/helpers/commits_helper_spec.rb

@@ -7,19 +7,38 @@ RSpec.describe CommitsHelper do
     context 'when current_user exists' do
       before do
         allow(helper).to receive(:current_user).and_return(double('User'))
-        allow(helper).to receive(:can_collaborate_with_project?).and_return(true)
       end
 
       it 'renders a div for Vue' do
-        result = helper.revert_commit_link('_commit_', '_path_', pajamas: true)
+        result = helper.revert_commit_link
 
         expect(result).to include('js-revert-commit-trigger')
       end
+    end
+
+    context 'when current_user does not exist' do
+      before do
+        allow(helper).to receive(:current_user).and_return(nil)
+      end
+
+      it 'does not render anything' do
+        result = helper.revert_commit_link
 
-      it 'does not render a div for Vue' do
-        result = helper.revert_commit_link('_commit_', '_path_')
+        expect(result).to be_nil
+      end
+    end
+  end
+
+  describe '#cherry_pick_commit_link' do
+    context 'when current_user exists' do
+      before do
+        allow(helper).to receive(:current_user).and_return(double('User'))
+      end
+
+      it 'renders a div for Vue' do
+        result = helper.cherry_pick_commit_link
 
-        expect(result).not_to include('js-revert-commit-trigger')
+        expect(result).to include('js-cherry-pick-commit-trigger')
       end
     end
 
@@ -29,7 +48,7 @@ RSpec.describe CommitsHelper do
       end
 
       it 'does not render anything' do
-        result = helper.revert_commit_link(double('Commit'), '_path_')
+        result = helper.cherry_pick_commit_link
 
         expect(result).to be_nil
       end