Update qa selectors for MR page objects

Removes deprecated qa classes in favor of data-qa-selectors
Conforms selectors to naming guidelines
Removes use of qa selectors in tests other than e2e

app/assets/javascripts/batch_comments/components/publish_button.vue

@@ -40,7 +40,8 @@ export default {
 <template>
   <gl-button
     :loading="isPublishing"
-    class="js-publish-draft-button qa-submit-review"
+    class="js-publish-draft-button"
+    data-qa-selector="submit_review_button"
     :category="category"
     :variant="variant"
     @click="onClick"

app/assets/javascripts/batch_comments/components/review_bar.vue

@@ -27,7 +27,10 @@ export default {
 <template>
   <div v-show="draftsCount > 0">
     <nav class="review-bar-component">
-      <div class="review-bar-content qa-review-bar d-flex gl-justify-content-end">
+      <div
+        class="review-bar-content d-flex gl-justify-content-end"
+        data-qa-selector="review_bar_content"
+      >
         <preview-dropdown />
         <publish-button class="gl-ml-3" show-count />
       </div>

app/assets/javascripts/diffs/components/inline_diff_table_row.vue

@@ -139,7 +139,8 @@ export default {
           v-show="shouldShowCommentButton"
           ref="addDiffNoteButton"
           type="button"
-          class="add-diff-note note-button js-add-diff-note-button qa-diff-comment"
+          class="add-diff-note note-button js-add-diff-note-button"
+          data-qa-selector="diff_comment_button"
           :disabled="line.commentsDisabled"
           @click="handleCommentButton"
         >
@@ -167,10 +168,11 @@ export default {
         "
       />
     </td>
-    <td ref="newTd" class="diff-line-num new_line qa-new-diff-line" :class="classNameMapCell">
+    <td ref="newTd" class="diff-line-num new_line" :class="classNameMapCell">
       <a
         v-if="line.new_line"
         ref="lineNumberRefNew"
+        data-qa-selector="new_diff_line_link"
         :data-linenumber="line.new_line"
         :href="line.lineHref"
         @click="setHighlightedRow(line.lineCode)"

app/assets/javascripts/reports/components/report_section.vue

@@ -190,7 +190,8 @@ export default {
           v-if="isCollapsible"
           type="button"
           data-testid="report-section-expand-button"
-          class="js-collapse-btn btn float-right btn-sm align-self-center qa-expand-report-button"
+          class="js-collapse-btn btn float-right btn-sm align-self-center"
+          data-qa-selector="expand_report_button"
           @click="toggleCollapsed"
         >
           {{ collapseText }}

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue

@@ -175,7 +175,7 @@ export default {
             :href="mr.emailPatchesPath"
             class="js-download-email-patches"
             download
-            data-qa-selector="download_email_patches"
+            data-qa-selector="download_email_patches_menu_item"
           >
             {{ s__('mrWidget|Email patches') }}
           </gl-dropdown-item>
@@ -183,7 +183,7 @@ export default {
             :href="mr.plainDiffPath"
             class="js-download-plain-diff"
             download
-            data-qa-selector="download_plain_diff"
+            data-qa-selector="download_plain_diff_menu_item"
           >
             {{ s__('mrWidget|Plain diff') }}
           </gl-dropdown-item>

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

@@ -176,16 +176,22 @@ export default {
           <gl-button
             :loading="isMakingRequest"
             variant="success"
-            class="qa-mr-rebase-button"
+            data-qa-selector="mr_rebase_button"
             @click="rebase"
           >
             {{ __('Rebase') }}
           </gl-button>
-          <span v-if="!rebasingError" class="gl-font-weight-bold" data-testid="rebase-message">{{
+          <span
+            v-if="!rebasingError"
+            class="gl-font-weight-bold"
+            data-testid="rebase-message"
+            data-qa-selector="no_fast_forward_message_content"
+            >{{
               __(
                 'Fast-forward merge is not possible. Rebase the source branch onto the target branch.',
               )
-          }}</span>
+            }}</span
+          >
           <span v-else class="gl-font-weight-bold danger" data-testid="rebase-message">{{
             rebasingError
           }}</span>

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

@@ -460,7 +460,8 @@ export default {
               <gl-button
                 size="medium"
                 category="primary"
-                class="qa-merge-button accept-merge-request"
+                class="accept-merge-request"
+                data-testid="merge-button"
                 :variant="mergeButtonVariant"
                 :disabled="isMergeButtonDisabled"
                 :loading="isMakingRequest"
@@ -483,7 +484,7 @@ export default {
                 <gl-dropdown-item
                   icon-name="warning"
                   button-class="accept-merge-request js-merge-immediately-button"
-                  data-qa-selector="merge_immediately_option"
+                  data-qa-selector="merge_immediately_menu_item"
                   @click="handleMergeImmediatelyButtonClick"
                 >
                   {{ __('Merge immediately') }}
@@ -562,7 +563,11 @@ export default {
         :merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath"
       />
       <template v-if="shouldShowMergeControls">
-        <div v-if="!shouldShowMergeEdit" class="mr-fast-forward-message">
+        <div
+          v-if="!shouldShowMergeEdit"
+          class="mr-fast-forward-message"
+          data-qa-selector="fast_forward_message_content"
+        >
           {{ __('Fast-forward merge without a merge commit') }}
         </div>
         <commits-header

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

@@ -13,7 +13,7 @@ export default {
   <div class="mr-widget-body media">
     <status-icon :show-disabled-button="true" status="warning" />
     <div class="media-body space-children">
-      <span class="bold">
+      <span class="bold" data-qa-selector="head_mismatch_content">
         {{
           s__(`mrWidget|The source branch HEAD has recently changed.
 Please reload the page and review the changes before merging`)

app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue

@@ -107,7 +107,7 @@ export default {
 
 <template>
   <div class="md-suggestion-header border-bottom-0 mt-2">
-    <div class="qa-suggestion-diff-header js-suggestion-diff-header font-weight-bold">
+    <div class="js-suggestion-diff-header font-weight-bold">
       {{ __('Suggested change') }}
       <a v-if="helpPagePath" :href="helpPagePath" :aria-label="__('Help')" class="js-help-btn">
         <gl-icon name="question-o" css-classes="link-highlight" />

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

@@ -45,7 +45,7 @@
             %li= link_to 'Report abuse', new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request))
 
       - if can_update_merge_request
-        = link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-md-block btn gl-button btn-default btn-grouped js-issuable-edit qa-edit-button"
+        = link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-md-block btn gl-button btn-default btn-grouped js-issuable-edit", data: { qa_selector: "edit_button" }
 
       - if can_update_merge_request && !are_close_and_open_buttons_hidden
         = render 'projects/merge_requests/close_reopen_draft_report_toggle'

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

@@ -35,7 +35,7 @@
               = tab_link_for @merge_request, :pipelines do
                 = _("Pipelines")
                 %span.badge.badge-pill.js-pipelines-mr-count= number_of_pipelines
-          = render "projects/merge_requests/tabs/tab", name: "diffs", class: "diffs-tab qa-diffs-tab", id: "diffs-tab" do
+          = render "projects/merge_requests/tabs/tab", name: "diffs", class: "diffs-tab", id: "diffs-tab", qa_selector: "diffs_tab" do
             = tab_link_for @merge_request, :diffs do
               = _("Changes")
               %span.badge.badge-pill= @merge_request.diff_size

app/views/shared/issuable/_form.html.haml

@@ -72,7 +72,7 @@
 
   %span.gl-mr-3
     - if issuable.new_record?
-      = form.submit "Submit #{issuable.class.model_name.human.downcase}", class: 'gl-button btn btn-confirm qa-issuable-create-button'
+      = form.submit "Submit #{issuable.class.model_name.human.downcase}", class: 'gl-button btn btn-confirm', data: { qa_selector: 'issuable_create_button' }
     - else
       = form.submit 'Save changes', class: 'gl-button btn btn-confirm'
 

ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue

@@ -589,7 +589,7 @@ export default {
               <div class="text-nowrap">
                 {{ n__('%d URL scanned', '%d URLs scanned', dastSummary.scannedResourcesCount) }}
               </div>
-              <gl-link v-gl-modal.dastUrl class="ml-2" data-qa-selector="dast-ci-job-link">
+              <gl-link v-gl-modal.dastUrl class="ml-2" data-testid="dast-ci-job-link">
                 {{ __('View details') }}
               </gl-link>
               <dast-modal
@@ -613,7 +613,7 @@ export default {
             :status-icon="secretDetectionStatusIcon"
             :popover-options="secretScanningPopover"
             class="js-secret-scanning"
-            data-qa-selector="secret_scan_report"
+            data-testid="secret-scan-report"
           >
             <template #summary>
               <security-summary :message="groupedSecretDetectionText" />

ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml

@@ -14,9 +14,9 @@
         %tr.d-none.d-sm-table-row
           %th.w-25= s_('CodeOwner|Pattern')
           %th= _('Members')
-      %tbody{ data: { qa_selector: "approver_list" } }
+      %tbody{ data: { qa_selector: "approver_list_content" } }
         - code_owner_rules.each do |code_owner_approval_rule|
-          %tr{ data: { qa_selector: "approver" } }
+          %tr{ data: { qa_selector: "approver_content" } }
             %td
               - if code_owner_approval_rule.section != Gitlab::CodeOwners::Entry::DEFAULT_SECTION
                 %span.text-muted.small.d-block= code_owner_approval_rule.section

ee/spec/features/merge_request/user_views_blocked_merge_request_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe 'Merge Request > User views blocked MR', :js do
   let(:project) { blocked_mr.target_project }
   let(:user) { create(:user) }
 
-  let(:merge_button) { find('.qa-merge-button') }
+  let(:merge_button) { find('[data-testid="merge-button"]') }
 
   before do
     project.add_developer(user)

ee/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

@@ -78,7 +78,7 @@ describe('ReadyToMerge', () => {
   const findResolveItemsMessage = () => wrapper.find(GlSprintf);
   const findPipelineConflictMessage = () =>
     wrapper.find('[data-testid="pipeline-succeed-conflict"]');
-  const findMergeButton = () => wrapper.find('.qa-merge-button');
+  const findMergeButton = () => wrapper.find('[data-testid="merge-button"]');
   const findMergeButtonDropdown = () => wrapper.find('.js-merge-moment');
   const findMergeImmediatelyButton = () => wrapper.find('.js-merge-immediately-button');
   const findMergeTrainHelperText = () => wrapper.find(MergeTrainHelperText);

ee/spec/frontend/vue_shared/security_reports/grouped_security_reports_app_spec.js

@@ -442,7 +442,7 @@ describe('Grouped security reports app', () => {
     });
 
     it('renders', () => {
-      expect(wrapper.find('[data-qa-selector="coverage_fuzzing_report"]').exists()).toBe(true);
+      expect(wrapper.find('.js-coverage-fuzzing-widget').exists()).toBe(true);
     });
   });
 
@@ -556,7 +556,7 @@ describe('Grouped security reports app', () => {
     });
 
     it('shows the scanned URLs count and opens a modal', async () => {
-      const jobLink = wrapper.find('[data-qa-selector="dast-ci-job-link"]');
+      const jobLink = wrapper.find('[data-testid="dast-ci-job-link"]');
 
       expect(wrapper.text()).toContain('211 URLs scanned');
       expect(jobLink.exists()).toBe(true);
@@ -596,7 +596,7 @@ describe('Grouped security reports app', () => {
 
       return waitForMutation(wrapper.vm.$store, types.RECEIVE_DAST_DIFF_SUCCESS).then(() => {
         expect(wrapper.text()).not.toContain('0 URLs scanned');
-        expect(wrapper.find('[data-qa-selector="dast-ci-job-link"]').exists()).toBe(false);
+        expect(wrapper.find('[data-testid="dast-ci-job-link"]').exists()).toBe(false);
       });
     });
   });
@@ -624,7 +624,7 @@ describe('Grouped security reports app', () => {
       });
 
       it('should render the component', () => {
-        expect(wrapper.find('[data-qa-selector="secret_scan_report"]').exists()).toBe(true);
+        expect(wrapper.find('[data-testid="secret-scan-report"]').exists()).toBe(true);
       });
 
       it('should set diffEndpoint', () => {
@@ -646,7 +646,7 @@ describe('Grouped security reports app', () => {
       });
 
       it('should not render the component', () => {
-        expect(wrapper.find('[data-qa-selector="secret_scan_report"]').exists()).toBe(false);
+        expect(wrapper.find('[data-testid="secret-scan-report"]').exists()).toBe(false);
       });
     });
   });

qa/qa/ee/page/merge_request/show.rb

@@ -16,12 +16,12 @@ module QA
               prepend Page::Component::LicenseManagement
 
               view 'app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue' do
-                element :head_mismatch, "The source branch HEAD has recently changed." # rubocop:disable QA/ElementWithPattern
+                element :head_mismatch_content
               end
 
               view 'ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml' do
-                element :approver
-                element :approver_list
+                element :approver_content
+                element :approver_list_content
               end
 
               view 'ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue' do
@@ -93,8 +93,8 @@ module QA
           end
 
           def approvers
-            within_element :approver_list do
-              all_elements(:approver, minimum: 1).map { |item| item.find('img')['title'] }
+            within_element :approver_list_content do
+              all_elements(:approver_content, minimum: 1).map { |item| item.find('img')['title'] }
             end
           end
 
@@ -214,7 +214,7 @@ module QA
 
           def skip_merge_train_and_merge_immediately
             click_element :merge_moment_dropdown
-            click_element :merge_immediately_option
+            click_element :merge_immediately_menu_item
 
             # Wait for the warning modal dialog to appear
             wait_for_animated_element :merge_immediately_button

qa/qa/page/merge_request/show.rb

@@ -9,8 +9,8 @@ module QA
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue' do
           element :download_dropdown
-          element :download_email_patches
-          element :download_plain_diff
+          element :download_email_patches_menu_item
+          element :download_plain_diff_menu_item
           element :open_in_web_ide_button
         end
 
@@ -21,9 +21,9 @@ module QA
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue' do
           element :merge_button
-          element :fast_forward_message, 'Fast-forward merge without a merge commit' # rubocop:disable QA/ElementWithPattern
+          element :fast_forward_message_content
           element :merge_moment_dropdown
-          element :merge_immediately_option
+          element :merge_immediately_menu_item
         end
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_enabled.vue' do
@@ -40,7 +40,7 @@ module QA
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue' do
           element :mr_rebase_button
-          element :no_fast_forward_message, 'Fast-forward merge is not possible' # rubocop:disable QA/ElementWithPattern
+          element :no_fast_forward_message_content
         end
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/states/squash_before_merge.vue' do
@@ -68,7 +68,8 @@ module QA
         end
 
         view 'app/assets/javascripts/diffs/components/inline_diff_table_row.vue' do
-          element :new_diff_line
+          element :diff_comment_button
+          element :new_diff_line_link
         end
 
         view 'app/views/projects/merge_requests/_mr_title.html.haml' do
@@ -76,11 +77,11 @@ module QA
         end
 
         view 'app/assets/javascripts/batch_comments/components/publish_button.vue' do
-          element :submit_review
+          element :submit_review_button
         end
 
         view 'app/assets/javascripts/batch_comments/components/review_bar.vue' do
-          element :review_bar
+          element :review_bar_content
         end
 
         view 'app/assets/javascripts/notes/components/note_form.vue' do
@@ -128,18 +129,18 @@ module QA
         end
 
         def submit_pending_reviews
-          within_element(:review_bar) do
+          within_element(:review_bar_content) do
             click_element(:review_preview_toggle)
-            click_element(:submit_review)
+            click_element(:submit_review_button)
 
             # After clicking the button, wait for it to disappear
             # before moving on to the next part of the test
-            has_no_element?(:submit_review)
+            has_no_element?(:submit_review_button)
           end
         end
 
         def discard_pending_reviews
-          within_element(:review_bar) do
+          within_element(:review_bar_content) do
             click_element(:discard_review)
           end
           click_element(:modal_delete_pending_comments)
@@ -158,8 +159,8 @@ module QA
           wait_until(sleep_interval: 5) do
             has_css?('a[data-linenumber="1"]')
           end
-          all_elements(:new_diff_line, minimum: 1).first.hover
-          click_element(:diff_comment)
+          all_elements(:new_diff_line_link, minimum: 1).first.hover
+          click_element(:diff_comment_button)
           fill_element(:reply_field, text)
         end
 
@@ -183,11 +184,11 @@ module QA
         end
 
         def fast_forward_possible?
-          has_text?('Fast-forward merge without a merge commit')
+          has_element?(:fast_forward_message_content)
         end
 
         def fast_forward_not_possible?
-          has_text?('Fast-forward merge is not possible')
+          has_element?(:no_fast_forward_message_content)
         end
 
         def has_file?(file_name)
@@ -241,7 +242,7 @@ module QA
 
         def merge_immediately!
           click_element(:merge_moment_dropdown)
-          click_element(:merge_immediately_option)
+          click_element(:merge_immediately_menu_item)
         end
 
         def merge_when_pipeline_succeeds!
@@ -295,7 +296,7 @@ module QA
           click_element(:mr_rebase_button)
 
           success = wait_until do
-            has_text?('Fast-forward merge without a merge commit')
+            fast_forward_possible?
           end
 
           raise "Rebase did not appear to be successful" unless success
@@ -312,12 +313,12 @@ module QA
 
         def view_email_patches
           click_element(:download_dropdown)
-          visit_link_in_element(:download_email_patches)
+          visit_link_in_element(:download_email_patches_menu_item)
         end
 
         def view_plain_diff
           click_element(:download_dropdown)
-          visit_link_in_element(:download_plain_diff)
+          visit_link_in_element(:download_plain_diff_menu_item)
         end
 
         def wait_for_merge_request_error_message
@@ -340,7 +341,7 @@ module QA
 
         def add_suggestion_to_diff(suggestion, line)
           find("a[data-linenumber='#{line}']").hover
-          click_element(:diff_comment)
+          click_element(:diff_comment_button)
           click_element(:suggestion_button)
           initial_content = find_element(:reply_field).value
           fill_element(:reply_field, '')

spec/frontend/diffs/components/inline_diff_table_row_spec.js

@@ -215,14 +215,14 @@ describe('InlineDiffTableRow', () => {
         const TEST_LINE_NUMBER = 1;
 
         describe.each`
-          lineProps                                                                                     | findLineNumber       | expectedHref            | expectedClickArg
-          ${{ line_code: TEST_LINE_CODE, old_line: TEST_LINE_NUMBER }}                                  | ${findLineNumberOld} | ${`#${TEST_LINE_CODE}`} | ${TEST_LINE_CODE}
-          ${{ line_code: undefined, old_line: TEST_LINE_NUMBER }}                                       | ${findLineNumberOld} | ${'#'}                  | ${undefined}
-          ${{ line_code: undefined, left: { line_code: TEST_LINE_CODE }, old_line: TEST_LINE_NUMBER }}  | ${findLineNumberOld} | ${'#'}                  | ${TEST_LINE_CODE}
-          ${{ line_code: undefined, right: { line_code: TEST_LINE_CODE }, new_line: TEST_LINE_NUMBER }} | ${findLineNumberNew} | ${'#'}                  | ${TEST_LINE_CODE}
+          lineProps                                                                                     | findLineNumber       | expectedHref            | expectedClickArg  | expectedQaSelector
+          ${{ line_code: TEST_LINE_CODE, old_line: TEST_LINE_NUMBER }}                                  | ${findLineNumberOld} | ${`#${TEST_LINE_CODE}`} | ${TEST_LINE_CODE} | ${undefined}
+          ${{ line_code: undefined, old_line: TEST_LINE_NUMBER }}                                       | ${findLineNumberOld} | ${'#'}                  | ${undefined}      | ${undefined}
+          ${{ line_code: undefined, left: { line_code: TEST_LINE_CODE }, old_line: TEST_LINE_NUMBER }}  | ${findLineNumberOld} | ${'#'}                  | ${TEST_LINE_CODE} | ${undefined}
+          ${{ line_code: undefined, right: { line_code: TEST_LINE_CODE }, new_line: TEST_LINE_NUMBER }} | ${findLineNumberNew} | ${'#'}                  | ${TEST_LINE_CODE} | ${'new_diff_line_link'}
         `(
           'with line ($lineProps)',
-          ({ lineProps, findLineNumber, expectedHref, expectedClickArg }) => {
+          ({ lineProps, findLineNumber, expectedHref, expectedClickArg, expectedQaSelector }) => {
             beforeEach(() => {
               jest.spyOn(store, 'dispatch').mockImplementation();
               createComponent({
@@ -235,6 +235,7 @@ describe('InlineDiffTableRow', () => {
               expect(findLineNumber().attributes()).toEqual({
                 href: expectedHref,
                 'data-linenumber': TEST_LINE_NUMBER.toString(),
+                'data-qa-selector': expectedQaSelector,
               });
             });