From 67b73598b989991d2c7a0ad8251dda44da91993a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com> Date: Wed, 24 Jul 2024 18:43:10 +0000 Subject: [PATCH] Adjust the specificity of work item close button We make the work item comment form action specific by clicking within the `work-item-comment-form-actions` data-testid context. However, within this specific context, we need to use a generic text because it changes depending where the work item is located. --- .../notes/work_item_comment_form.vue | 2 +- .../components/work_item_time_tracking.vue | 1 + .../shared/issuable/form/_metadata.html.haml | 2 +- ee/spec/features/issues/form_spec.rb | 24 ++++---- spec/features/broadcast_messages_spec.rb | 24 ++++---- ...r_sees_sidebar_updates_in_realtime_spec.rb | 6 +- spec/frontend/users_select/test_helper.js | 4 +- .../features/work_items_shared_examples.rb | 57 ++++++++++++------- 8 files changed, 72 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/work_items/components/notes/work_item_comment_form.vue b/app/assets/javascripts/work_items/components/notes/work_item_comment_form.vue index 69fe5ae37d96e..2d49964dc6291 100644 --- a/app/assets/javascripts/work_items/components/notes/work_item_comment_form.vue +++ b/app/assets/javascripts/work_items/components/notes/work_item_comment_form.vue @@ -217,7 +217,7 @@ export default { @keydown.esc.stop="cancelEditing" /> </comment-field-layout> - <div class="note-form-actions"> + <div class="note-form-actions" data-testid="work-item-comment-form-actions"> <gl-form-checkbox v-if="isNewDiscussion" v-model="isNoteInternal" diff --git a/app/assets/javascripts/work_items/components/work_item_time_tracking.vue b/app/assets/javascripts/work_items/components/work_item_time_tracking.vue index bb65a4197b6b2..df5753888cb4a 100644 --- a/app/assets/javascripts/work_items/components/work_item_time_tracking.vue +++ b/app/assets/javascripts/work_items/components/work_item_time_tracking.vue @@ -211,6 +211,7 @@ export default { <gl-modal modal-id="time-tracking-report" + data-testid="time-tracking-report-modal" hide-footer size="lg" :title="__('Time tracking report')" diff --git a/app/views/shared/issuable/form/_metadata.html.haml b/app/views/shared/issuable/form/_metadata.html.haml index a0ec7ca20ff7a..24e04818bf6b7 100644 --- a/app/views/shared/issuable/form/_metadata.html.haml +++ b/app/views/shared/issuable/form/_metadata.html.haml @@ -22,7 +22,7 @@ - if can?(current_user, :"set_#{issuable.to_ability_name}_metadata", issuable) .row.gl-pt-4 %div{ class: (has_due_date ? "col-lg-6" : "col-12") } - .form-group.row.merge-request-assignee + .form-group.row{ data: { testid: 'merge-request-assignee' } } = render "shared/issuable/form/metadata_issuable_assignee", issuable: issuable, form: form, has_due_date: has_due_date - if issuable.allows_reviewers? diff --git a/ee/spec/features/issues/form_spec.rb b/ee/spec/features/issues/form_spec.rb index 5b589240ee529..29ca4b4be52fa 100644 --- a/ee/spec/features/issues/form_spec.rb +++ b/ee/spec/features/issues/form_spec.rb @@ -88,21 +88,23 @@ expect(page).to have_link 'Assign to me' - click_button 'Unassigned' - click_link user2.name + within_testid('merge-request-assignee') do + click_button 'Unassigned' + click_link user2.name - expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user2.id.to_s) - expect(page).to have_button user2.name + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user2.id.to_s) + expect(page).to have_button user2.name - click_button 'Close' - click_link 'Assign to me' + click_button 'Close' + click_link 'Assign to me' - assignee_ids = page.all('input[name="issue[assignee_ids][]"]', visible: false) - expect(assignee_ids[0].value).to match(user2.id.to_s) - expect(assignee_ids[1].value).to match(user.id.to_s) + assignee_ids = page.all('input[name="issue[assignee_ids][]"]', visible: false) + expect(assignee_ids[0].value).to match(user2.id.to_s) + expect(assignee_ids[1].value).to match(user.id.to_s) - expect(page).to have_button "#{user2.name} + 1 more" - expect(page).not_to have_link 'Assign to me' + expect(page).to have_button "#{user2.name} + 1 more" + expect(page).not_to have_link 'Assign to me' + end click_button 'Select milestone' click_button milestone.title diff --git a/spec/features/broadcast_messages_spec.rb b/spec/features/broadcast_messages_spec.rb index c1a8e4d4b159d..768f73d55ab72 100644 --- a/spec/features/broadcast_messages_spec.rb +++ b/spec/features/broadcast_messages_spec.rb @@ -31,7 +31,7 @@ expect_to_be_on_explore_projects_page - find('body.page-initialised .js-dismiss-current-broadcast-notification').click + find(".js-dismiss-current-broadcast-notification[data-id='#{broadcast_message.id}']").click expect_message_dismissed end @@ -41,7 +41,7 @@ expect_to_be_on_explore_projects_page - find('body.page-initialised .js-dismiss-current-broadcast-notification').click + find(".js-dismiss-current-broadcast-notification[data-id='#{broadcast_message.id}']").click expect_message_dismissed @@ -57,7 +57,7 @@ expect_to_be_on_explore_projects_page - find('body.page-initialised .js-dismiss-current-broadcast-notification').click + find(".js-dismiss-current-broadcast-notification[data-id='#{broadcast_message.id}']").click expect_message_dismissed @@ -79,7 +79,7 @@ it 'is not dismissible' do visit path - expect(page).not_to have_selector('.js-dismiss-current-broadcast-notification') + expect(page).not_to have_selector(".js-dismiss-current-broadcast-notification[data-id=#{broadcast_message.id}]") end it 'does not replace placeholders' do @@ -127,7 +127,7 @@ visit path - expect_broadcast_message(text) + expect_broadcast_message(message.id, text) # seed the other cache original_strategy_value = Gitlab::Cache::JsonCache::STRATEGY_KEY_COMPONENTS @@ -135,7 +135,7 @@ page.refresh - expect_broadcast_message(text) + expect_broadcast_message(message.id, text) # delete on original cache stub_const('Gitlab::Cache::JsonCaches::JsonKeyed::STRATEGY_KEY_COMPONENTS', original_strategy_value) @@ -153,27 +153,27 @@ visit path - expect_no_broadcast_message + expect_no_broadcast_message(message.id) # other revision of GitLab does gets cache destroyed stub_const('Gitlab::Cache::JsonCaches::JsonKeyed::STRATEGY_KEY_COMPONENTS', new_strategy_value) page.refresh - expect_no_broadcast_message + expect_no_broadcast_message(message.id) end end - def expect_broadcast_message(text) - within_testid('banner-broadcast-message') do + def expect_broadcast_message(id, text) + within(".js-broadcast-notification-#{id}") do expect(page).to have_content text end end - def expect_no_broadcast_message + def expect_no_broadcast_message(id) expect_to_be_on_explore_projects_page - expect(page).not_to have_selector('[data-testid="banner-broadcast-message"]') + expect(page).not_to have_selector(".js-broadcast-notification-#{id}") end def expect_to_be_on_explore_projects_page diff --git a/spec/features/issues/user_sees_sidebar_updates_in_realtime_spec.rb b/spec/features/issues/user_sees_sidebar_updates_in_realtime_spec.rb index 28bef4ab4c01d..f6b97545863d1 100644 --- a/spec/features/issues/user_sees_sidebar_updates_in_realtime_spec.rb +++ b/spec/features/issues/user_sees_sidebar_updates_in_realtime_spec.rb @@ -50,8 +50,10 @@ wait_for_all_requests - click_button label.name - click_button 'Close' + page.within(labels_widget) do + click_button label.name + click_button 'Close' + end wait_for_requests diff --git a/spec/frontend/users_select/test_helper.js b/spec/frontend/users_select/test_helper.js index 05b0bf314a0aa..150818cecf254 100644 --- a/spec/frontend/users_select/test_helper.js +++ b/spec/frontend/users_select/test_helper.js @@ -10,7 +10,9 @@ import UsersSelect from '~/users_select'; const getUserSearchHTML = memoize((fixture) => { const parser = new DOMParser(); - const el = parser.parseFromString(fixture, 'text/html').querySelector('.merge-request-assignee'); + const el = parser + .parseFromString(fixture, 'text/html') + .querySelector('[data-testid=merge-request-assignee]'); return el.outerHTML; }); diff --git a/spec/support/shared_examples/features/work_items_shared_examples.rb b/spec/support/shared_examples/features/work_items_shared_examples.rb index 00bbc0b25fb42..15011d6085e68 100644 --- a/spec/support/shared_examples/features/work_items_shared_examples.rb +++ b/spec/support/shared_examples/features/work_items_shared_examples.rb @@ -27,9 +27,12 @@ RSpec.shared_examples 'work items toggle status button' do it 'successfully shows and changes the status of the work item' do - click_button 'Close', match: :first + within_testid 'work-item-comment-form-actions' do + # Depending of the context, the button's text could be `Close issue`, `Close key result`, `Close objective`, etc. + click_button 'Close', match: :first - expect(page).to have_button 'Reopen' + expect(page).to have_button 'Reopen' + end expect(work_item.reload.state).to eq('closed') end end @@ -624,7 +627,9 @@ def find_and_click_clear(selector) expect(page).to be_axe_clean.within('[role="dialog"]') - click_button 'Close' + within_testid 'set-time-estimate-modal' do + click_button 'Close' + end click_button 'time spent' expect(page).to be_axe_clean.within('[role="dialog"]') @@ -632,15 +637,19 @@ def find_and_click_clear(selector) it 'adds and removes an estimate', :aggregate_failures do click_button 'estimate' - fill_in 'Estimate', with: '5d' - click_button 'Save' + within_testid 'set-time-estimate-modal' do + fill_in 'Estimate', with: '5d' + click_button 'Save' + end expect(page).to have_text 'Estimate 5d' expect(page).to have_button '5d' expect(page).not_to have_button 'estimate' click_button '5d' - click_button 'Remove' + within_testid 'set-time-estimate-modal' do + click_button 'Remove' + end expect(page).not_to have_text 'Estimate 5d' expect(page).not_to have_button '5d' @@ -648,31 +657,39 @@ def find_and_click_clear(selector) end it 'adds and deletes time entries and view report', :aggregate_failures do - click_button 'time entry' - fill_in 'Time spent', with: '1d' - fill_in 'Summary', with: 'First summary' - click_button 'Save' + click_button 'Add time entry' + + within_testid 'create-timelog-modal' do + fill_in 'Time spent', with: '1d' + fill_in 'Summary', with: 'First summary' + click_button 'Save' + end click_button 'Add time entry' - fill_in 'Time spent', with: '2d' - fill_in 'Summary', with: 'Second summary' - click_button 'Save' + + within_testid 'create-timelog-modal' do + fill_in 'Time spent', with: '2d' + fill_in 'Summary', with: 'Second summary' + click_button 'Save' + end expect(page).to have_text 'Spent 3d' expect(page).to have_button '3d' click_button '3d' - expect(page).to have_css 'h2', text: 'Time tracking report' - expect(page).to have_text "1d #{user.name} First summary" - expect(page).to have_text "2d #{user.name} Second summary" + within_testid 'time-tracking-report-modal' do + expect(page).to have_css 'h2', text: 'Time tracking report' + expect(page).to have_text "1d #{user.name} First summary" + expect(page).to have_text "2d #{user.name} Second summary" - click_button 'Delete time spent', match: :first + click_button 'Delete time spent', match: :first - expect(page).to have_text "1d #{user.name} First summary" - expect(page).not_to have_text "2d #{user.name} Second summary" + expect(page).to have_text "1d #{user.name} First summary" + expect(page).not_to have_text "2d #{user.name} Second summary" - click_button 'Close' + click_button 'Close' + end expect(page).to have_text 'Spent 1d' expect(page).to have_button '1d' -- GitLab