From 5c3fda2daf691f6e3de5ae79492179d0d0a27277 Mon Sep 17 00:00:00 2001
From: Scott Dickerson <sdickers@redhat.com>
Date: Fri, 17 Nov 2023 17:00:44 -0500
Subject: [PATCH] :bug: Refactor dependencies, select row by name and provider
 (#1554)

Resolves: #1509

The Dependencies table was setup to use the `AnalysisDependency.name` as
the selected row key. Since the `.name` field is not unique across all
rows multiple rows would be erroneously selected at the same time. To
solve the problem, a UI only unique id is generated after fetching and
used for the table selection itemId.

Change details:
  - Dependencies fetch query updated to inject a UI uid and return
    `WithUiId<AnalysisDependency>[]`. This transform is done outside the
    react-query `useQuery()` handling.

  - Updated the table to use the `WithUiId<>` key `_ui_unique_id` for
    selection.

  - Since the button rendered in the "Found in" column had no function,
    the button was removed.

  - "Found in" column moved to the last column to match the placement of
    matching columns on the Archetypes and Issues tables.

  - Use i18next plural aware keys for the "Found in" text to avoid the
    "(s)" suffix.

  - Fixed the `id` and `aria-label` on the table.

  - Added `key` to the label column `Label`s.

Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
---
 client/public/locales/en/translation.json     |  2 +
 .../app/pages/dependencies/dependencies.tsx   | 54 ++++++++-----------
 client/src/app/queries/dependencies.ts        | 26 ++++++++-
 3 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/client/public/locales/en/translation.json b/client/public/locales/en/translation.json
index f7cd9808c4..46a981598c 100644
--- a/client/public/locales/en/translation.json
+++ b/client/public/locales/en/translation.json
@@ -96,6 +96,8 @@
     "selectMany": "Select {{what}}",
     "selectOne": "Select a {{what}}",
     "selectAn": "Select an {{what}}",
+    "totalApplications": "{{count}} application",
+    "totalApplications_plural": "{{count}} applications",
     "workPriority": "Work priority (1=low, 10=high)"
   },
   "dialog": {
diff --git a/client/src/app/pages/dependencies/dependencies.tsx b/client/src/app/pages/dependencies/dependencies.tsx
index df9c9d3591..0c61a698cb 100644
--- a/client/src/app/pages/dependencies/dependencies.tsx
+++ b/client/src/app/pages/dependencies/dependencies.tsx
@@ -1,6 +1,5 @@
 import * as React from "react";
 import {
-  Button,
   Label,
   LabelGroup,
   PageSection,
@@ -30,7 +29,6 @@ import { useFetchDependencies } from "@app/queries/dependencies";
 import { useSelectionState } from "@migtools/lib-ui";
 import { DependencyAppsDetailDrawer } from "./dependency-apps-detail-drawer";
 import { useSharedAffectedApplicationFilterCategories } from "../issues/helpers";
-import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
 import { getParsedLabel } from "@app/utils/rules-utils";
 
 export const Dependencies: React.FC = () => {
@@ -47,8 +45,6 @@ export const Dependencies: React.FC = () => {
       foundIn: "Found in",
       provider: "Language",
       labels: "Labels",
-      sha: "SHA",
-      version: "Version",
     },
     isFilterEnabled: true,
     isSortEnabled: true,
@@ -101,7 +97,7 @@ export const Dependencies: React.FC = () => {
 
   const tableControls = useTableControlProps({
     ...tableControlState, // Includes filterState, sortState and paginationState
-    idProperty: "name",
+    idProperty: "_ui_unique_id",
     currentPageItems,
     totalItemCount,
     isLoading: isFetching,
@@ -123,7 +119,7 @@ export const Dependencies: React.FC = () => {
       getTrProps,
       getTdProps,
     },
-    activeItemDerivedState: { activeItem, clearActiveItem, setActiveItem },
+    activeItemDerivedState: { activeItem, clearActiveItem },
   } = tableControls;
 
   return (
@@ -151,14 +147,18 @@ export const Dependencies: React.FC = () => {
               </ToolbarItem>
             </ToolbarContent>
           </Toolbar>
-          <Table {...tableProps} aria-label="Migration waves table">
+          <Table
+            {...tableProps}
+            id="dependencies-table"
+            aria-label="Dependencies table"
+          >
             <Thead>
               <Tr>
                 <TableHeaderContentWithControls {...tableControls}>
                   <Th {...getThProps({ columnKey: "name" })} />
-                  <Th {...getThProps({ columnKey: "foundIn" })} />
                   <Th {...getThProps({ columnKey: "provider" })} />
                   <Th {...getThProps({ columnKey: "labels" })} />
+                  <Th {...getThProps({ columnKey: "foundIn" })} />
                 </TableHeaderContentWithControls>
               </Tr>
             </Thead>
@@ -182,36 +182,28 @@ export const Dependencies: React.FC = () => {
                       <Td width={25} {...getTdProps({ columnKey: "name" })}>
                         {dependency.name}
                       </Td>
-                      <Td width={10} {...getTdProps({ columnKey: "foundIn" })}>
-                        <Button
-                          className={spacing.pl_0}
-                          variant="link"
-                          onClick={(_) => {
-                            if (activeItem && activeItem === dependency) {
-                              clearActiveItem();
-                            } else {
-                              setActiveItem(dependency);
-                            }
-                          }}
-                        >
-                          {`${dependency.applications} application(s)`}
-                        </Button>
-                      </Td>
                       <Td width={10} {...getTdProps({ columnKey: "provider" })}>
                         {dependency.provider}
                       </Td>
                       <Td width={10} {...getTdProps({ columnKey: "labels" })}>
                         <LabelGroup>
-                          {dependency?.labels?.map((label) => {
-                            if (getParsedLabel(label).labelType !== "language")
-                              return (
-                                <Label>
-                                  {getParsedLabel(label).labelValue}
-                                </Label>
-                              );
+                          {dependency?.labels?.map((label, index) => {
+                            const { labelType, labelValue } =
+                              getParsedLabel(label);
+
+                            return labelType !== "language" ? (
+                              <Label key={`${index}-${labelValue}`}>
+                                {labelValue}
+                              </Label>
+                            ) : undefined;
                           })}
                         </LabelGroup>
                       </Td>
+                      <Td width={10} {...getTdProps({ columnKey: "foundIn" })}>
+                        {t("composed.totalApplications", {
+                          count: dependency.applications,
+                        })}
+                      </Td>
                     </TableRowContentWithControls>
                   </Tr>
                 ))}
@@ -227,7 +219,7 @@ export const Dependencies: React.FC = () => {
       </PageSection>
       <DependencyAppsDetailDrawer
         dependency={activeItem || null}
-        onCloseClick={() => setActiveItem(null)}
+        onCloseClick={() => clearActiveItem()}
       />
     </>
   );
diff --git a/client/src/app/queries/dependencies.ts b/client/src/app/queries/dependencies.ts
index 12af280903..6fecde893f 100644
--- a/client/src/app/queries/dependencies.ts
+++ b/client/src/app/queries/dependencies.ts
@@ -1,14 +1,16 @@
+import { useMemo } from "react";
 import { useQuery } from "@tanstack/react-query";
 import {
   AnalysisAppDependency,
   AnalysisDependency,
   HubPaginatedResult,
   HubRequestParams,
+  WithUiId,
 } from "@app/api/models";
 import { getAppDependencies, getDependencies } from "@app/api/rest";
 
 export interface IDependenciesFetchState {
-  result: HubPaginatedResult<AnalysisDependency>;
+  result: HubPaginatedResult<WithUiId<AnalysisDependency>>;
   isFetching: boolean;
   fetchError: unknown;
   refetch: () => void;
@@ -32,8 +34,28 @@ export const useFetchDependencies = (
     onError: (error) => console.log("error, ", error),
     keepPreviousData: true,
   });
+
+  const result = useMemo(() => {
+    if (!data) {
+      return { data: [], total: 0, params };
+    }
+
+    const syntheticData: WithUiId<AnalysisDependency>[] = data.data.map(
+      (dep) => ({
+        ...dep,
+        _ui_unique_id: `${dep.name}/${dep.provider}`,
+      })
+    );
+
+    return {
+      data: syntheticData,
+      total: data.total,
+      params: data.params,
+    };
+  }, [data, params]);
+
   return {
-    result: data || { data: [], total: 0, params },
+    result,
     isFetching: isLoading,
     fetchError: error,
     refetch,