Merge branch '357388-resolve-user-popover-not-showing-on-compliance-report' into 'master'

Resolve user popover not displaying on compliance report

See merge request gitlab-org/gitlab!84275

ee/app/assets/javascripts/compliance_dashboard/components/drawer_sections/merged_by.vue

@@ -34,8 +34,10 @@ export default {
 <template>
   <div>
     <drawer-section-header>{{ $options.i18n.header }}</drawer-section-header>
+    <!-- The key attribute is required so that the node updates when the user changes, which in turn updates the user popover event. -->
     <gl-avatar-link
       v-if="hasMergedBy"
+      :key="mergedBy.id"
       :title="mergedBy.name"
       :href="mergedBy.web_url"
       class="js-user-link"

ee/app/assets/javascripts/compliance_dashboard/components/shared/user_avatar.vue

@@ -18,7 +18,9 @@ export default {
 </script>
 
 <template>
+  <!-- The key attribute is required so that the node updates when the user changes, which in turn updates the user popover event. -->
   <gl-avatar-link
+    :key="user.id"
     target="blank"
     :href="user.webUrl"
     :title="user.name"

ee/app/assets/javascripts/compliance_dashboard/graphql/mappers.js

 import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
+import { getIdFromGraphQLId, convertNodeIdsFromGraphQLIds } from '~/graphql_shared/utils';
 
 export const mapViolations = (nodes = []) => {
   return nodes.map((node) => ({
     ...node,
     mergeRequest: {
       ...node.mergeRequest,
-      committers: node.mergeRequest.committers?.nodes || [],
-      approvedByUsers: node.mergeRequest.approvedBy?.nodes || [],
-      participants: node.mergeRequest.participants?.nodes || [],
-      // TODO: Once the legacy dashboard is removed (https://gitlab.com/gitlab-org/gitlab/-/issues/346266) we can update the drawer to use the new attributes and remove these 2 mappings
+      committers: convertNodeIdsFromGraphQLIds(node.mergeRequest.committers?.nodes || []),
+      approvedByUsers: convertNodeIdsFromGraphQLIds(node.mergeRequest.approvedBy?.nodes || []),
+      participants: convertNodeIdsFromGraphQLIds(node.mergeRequest.participants?.nodes || []),
       reference: node.mergeRequest.ref,
-      mergedBy: convertObjectPropsToSnakeCase(node.mergeRequest.mergeUser),
+      mergedBy: {
+        ...convertObjectPropsToSnakeCase(node.mergeRequest.mergeUser),
+        id: getIdFromGraphQLId(node.mergeRequest.mergeUser?.id),
+      },
       project: {
         ...node.mergeRequest.project,
         complianceFramework: node.mergeRequest.project?.complianceFrameworks?.nodes[0] || null,
       },
     },
+    violatingUser: {
+      ...node.violatingUser,
+      id: getIdFromGraphQLId(node.violatingUser.id),
+    },
   }));
 };

ee/spec/features/groups/security/compliance_dashboards_spec.rb

@@ -55,8 +55,9 @@ RSpec.describe 'Compliance Dashboard', :js do
   end
 
   context 'when there are merge requests' do
-    let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') }
-    let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') }
+    let_it_be(:user_2) { create(:user) }
+    let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, author: user, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') }
+    let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, author: user_2, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') }
 
     context 'chain of custody report' do
       it_behaves_like 'exports a merge commit-specific CSV'
@@ -87,6 +88,24 @@ RSpec.describe 'Compliance Dashboard', :js do
         expect(first_row).to have_content(merge_request_2.title)
       end
 
+      it 'shows the correct user avatar popover content when the drawer is switched', :aggregate_failures do
+        first_row.click
+        drawer_user_avatar.hover
+
+        within '.popover' do
+          expect(page).to have_content(user.name)
+          expect(page).to have_content(user.username)
+        end
+
+        second_row.click
+        drawer_user_avatar.hover
+
+        within '.popover' do
+          expect(page).to have_content(user_2.name)
+          expect(page).to have_content(user_2.username)
+        end
+      end
+
       context 'violations filter' do
         it 'can filter by date range' do
           set_date_range(7.days.ago.to_date, 6.days.ago.to_date)
@@ -109,6 +128,16 @@ RSpec.describe 'Compliance Dashboard', :js do
     find('tbody tr', match: :first)
   end
 
+  def second_row
+    all('tbody tr')[1]
+  end
+
+  def drawer_user_avatar
+    page.within('.gl-drawer') do
+      first('.js-user-link')
+    end
+  end
+
   def set_date_range(start_date, end_date)
     page.within('[data-testid="violations-date-range-picker"]') do
       all('input')[0].set(start_date)

ee/spec/frontend/compliance_dashboard/components/report_spec.js

@@ -248,7 +248,7 @@ describe('ComplianceReport component', () => {
     });
 
     it('renders the violation reason', () => {
-      const { violatingUser, reason } = violations[0];
+      const { violatingUser, reason } = mapViolations(violations)[0];
 
       expect(findViolationReason().props()).toMatchObject({
         reason,

ee/spec/frontend/compliance_dashboard/graphql/mappers_spec.js

 import { mapViolations } from 'ee/compliance_dashboard/graphql/mappers';
 import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
+import { getIdFromGraphQLId, convertNodeIdsFromGraphQLIds } from '~/graphql_shared/utils';
 import { createComplianceViolation } from '../mock_data';
 
 describe('mapViolations', () => {
   it('returns the expected result', () => {
     const violation = createComplianceViolation();
-    const { mergeRequest } = mapViolations([violation])[0];
+    const mappedViolation = mapViolations([violation])[0];
 
-    expect(mergeRequest).toMatchObject({
-      committers: [],
-      approvedByUsers: [],
-      participants: violation.mergeRequest.participants.nodes,
-      reference: mergeRequest.ref,
-      mergedBy: convertObjectPropsToSnakeCase(mergeRequest.mergeUser),
-      project: {
-        ...violation.project,
-        complianceFramework: violation.mergeRequest.project?.complianceFrameworks?.nodes[0],
+    expect(mappedViolation).toMatchObject({
+      mergeRequest: {
+        committers: convertNodeIdsFromGraphQLIds(violation.mergeRequest.committers.nodes),
+        approvedByUsers: convertNodeIdsFromGraphQLIds(violation.mergeRequest.approvedBy.nodes),
+        participants: convertNodeIdsFromGraphQLIds(violation.mergeRequest.participants.nodes),
+        reference: violation.mergeRequest.ref,
+        mergedBy: {
+          ...convertObjectPropsToSnakeCase(violation.mergeRequest.mergeUser),
+          id: getIdFromGraphQLId(violation.mergeRequest.mergeUser?.id),
+        },
+      },
+      violatingUser: {
+        ...violation.violatingUser,
+        id: getIdFromGraphQLId(violation.violatingUser.id),
       },
     });
   });