🐛 Refactor dependencies, select row by name and provider #1554

sjd78 · 2023-11-16T20:23:22Z

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 Labels.

sjd78 · 2023-11-16T20:23:51Z

Before:

After:

codecov · 2023-11-16T20:28:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f10823a) 40.49% compared to head (af060d7) 40.49%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1554   +/-   ##
=======================================
  Coverage   40.49%   40.49%           
=======================================
  Files         145      145           
  Lines        4638     4638           
  Branches     1087     1087           
=======================================
  Hits         1878     1878           
  Misses       2746     2746           
  Partials       14       14

Flag	Coverage Δ
client	`40.49% <ø> (ø)`
server	`∅ <ø> (∅)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mturley

LGTM but an abstraction jumped out at me, take it or leave it.

mturley · 2023-11-16T21:13:45Z

client/src/app/queries/dependencies.ts

+  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]);


I wonder if we can abstract out something like:

useWithUiId(data, (dep) => `${dep.name}/${dep.provider}`);

This indicates to me that maybe internally the table-controls/batteries/helpers could just take a getItemId callback in the first place if you don't have a usable idProperty. Will factor that into thinking on current refactors over there.

LGTM as-is though if you want to merge.

idProperty: string | (item: TItem) -> string?

;-)

…#1552) Changes: - Refactor the header and layout of tabs for the dependencies detail drawer following the changes in #1542. - Show the dependency name and provider in the header since those two fields together are the row unique key (see #1554). - Update the filter for the dependency applications to include both the name and the provider to match the row unique key (see #1554). Signed-off-by: Scott J Dickerson <sdickers@redhat.com>

Resolves: konveyor#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. Signed-off-by: Scott J Dickerson <sdickers@redhat.com>

Following up on konveyor#1554, create hook `useWithUiId()` to handle injecting UI id to objects. Any object `T` going will to the hook will come out as a `WithUiId<T>` object. Tables using the UI id have been adjusted to use the Constant `UI_UNIQUE_ID` as the `WithUiId<T>` table data `idProperty`. All uses of `WithUiId<T>` are now handled the same way. Signed-off-by: Scott J Dickerson <sdickers@redhat.com>

Following up on #1554, create hook `useWithUiId()` to handle injecting UI id to objects. Any object `T` going will to the hook will come out as a `WithUiId<T>` object. Tables using the UI id have been adjusted to use the Constant `UI_UNIQUE_ID` as the `WithUiId<T>` table data `idProperty`. All uses of `WithUiId<T>` are now handled the same way. --------- Signed-off-by: Scott J Dickerson <sdickers@redhat.com>

sjd78 requested review from mturley, gitdallas and ibolton336 November 16, 2023 20:24

sjd78 mentioned this pull request Nov 16, 2023

👻 Refactor Dependencies details drawer layout and unique row handling #1552

Merged

mturley approved these changes Nov 16, 2023

View reviewed changes

sjd78 force-pushed the dep-table-row-select branch from 12476d6 to af060d7 Compare November 17, 2023 21:52

sjd78 merged commit 5c3fda2 into konveyor:main Nov 17, 2023
6 checks passed

sjd78 mentioned this pull request Nov 17, 2023

🌱 Refactor WithUiId handling to use hook useWithUiId() #1555

Merged

This was referenced Nov 22, 2023

operator konveyor-operator (0.3.0-beta.3) k8s-operatorhub/community-operators#3570

Merged

operator konveyor-operator (0.3.0-beta.3) redhat-openshift-ecosystem/community-operators-prod#3652

Merged

This was referenced Jan 23, 2024

operator konveyor-operator (0.3.0) k8s-operatorhub/community-operators#3793

Merged

operator konveyor-operator (0.3.0) redhat-openshift-ecosystem/community-operators-prod#3927

Merged

sjd78 deleted the dep-table-row-select branch February 9, 2024 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Refactor dependencies, select row by name and provider #1554

🐛 Refactor dependencies, select row by name and provider #1554

sjd78 commented Nov 16, 2023 •

edited

Loading

sjd78 commented Nov 16, 2023

codecov bot commented Nov 16, 2023 •

edited

Loading

mturley left a comment

mturley Nov 16, 2023 •

edited

Loading

sjd78 Nov 16, 2023

🐛 Refactor dependencies, select row by name and provider #1554

🐛 Refactor dependencies, select row by name and provider #1554

Conversation

sjd78 commented Nov 16, 2023 • edited Loading

sjd78 commented Nov 16, 2023

codecov bot commented Nov 16, 2023 • edited Loading

Codecov Report

mturley left a comment

Choose a reason for hiding this comment

mturley Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

sjd78 Nov 16, 2023

Choose a reason for hiding this comment

sjd78 commented Nov 16, 2023 •

edited

Loading

codecov bot commented Nov 16, 2023 •

edited

Loading

mturley Nov 16, 2023 •

edited

Loading