Austenem/CAT-754 Refactor "My Lists" #3671

austenem · 2025-01-22T17:51:39Z

Summary

Broad update to the "My Lists" feature across the portal that leverages the new UKV API.

Major updates include:

Lists are now tied to a user's profile.
- Lists will now persist across devices (previously, these were stored locally).
- As a result, lists will be restricted to authenticated users going forward.
"Save to list" buttons are now present on all search pages, dataset tables, and processed dataset sections.
Language on all "My Lists" pages and alerts have been updated.
All associated files have been converted to TypeScript.

Design Documentation/Original Tickets

CAT-754 Jira ticket

Testing

Tested using the following checklist:

Screenshots/Video

Updated Search page

Updated Collections dataset table

Updated "My Lists" page for logged-out users

Example of banner shown after transfer of local lists to a user's profile

Checklist

Code follows the project's coding standards
- Lint checks pass locally
- New CHANGELOG-your-feature-name-here.md is present in the root directory, describing the change(s) in full sentences.
Unit tests covering the new feature have been added
All existing tests pass
Any relevant documentation in JIRA/Confluence has been updated to reflect the new feature
Any new functionalities have appropriate analytics functionalities added

john-conroy

I'm a little confused why we are continuing to use a store for the saved lists/entities. Maybe it would be helpful to have a chat between the three of us?

context/app/static/js/components/savedLists/constants.ts

context/app/static/js/components/savedLists/api.ts

context/app/static/js/components/savedLists/hooks.ts

john-conroy

Wow, a lot of changes here! I still need to check this out locally.

...s/components/detailPage/entityHeader/EntityHeaderActionButtons/EntityHeaderActionButtons.tsx

john-conroy · 2025-01-30T14:49:38Z

context/app/static/js/components/detailPage/summary/SummaryBody/SummaryBody.tsx

+  description: propDescription,
+  creationLabel: propCreationLabel,
+  creationDate: propCreationDate,


Why rename the variables with prop*?

This was to avoid naming the final description/creation info variables in the function body like checkedDescription - this way they get to keep their intended names (description, etc) and there's a clear distinction between these and the optional prop variables.

.../static/js/components/detailPage/summary/SummarySaveEntityButton/SummarySaveEntityButton.tsx

context/app/static/js/components/savedLists/CreateListDialog/CreateListDialog.tsx

john-conroy · 2025-01-30T15:10:57Z

context/app/static/js/hooks/useSavedEntityData.ts

 function useSavedEntityData(savedEntities: SavedEntities, source: string[]) {
  const query = useMemo(
    () => ({
      query: getIDsQuery(Object.keys(savedEntities)),
      _source: source,
+      size: Object.keys(savedEntities).length,


I would either bump this to 10000 when we are returning a source or set it to 0 for purely aggregations queries.

Got it - is this just a performance concern?

context/app/static/js/components/savedLists/SavedListPanel/hooks.ts

context/app/static/js/components/savedLists/SavedListsContent/style.ts

john-conroy · 2025-01-30T15:17:49Z

context/app/static/js/components/savedLists/api.ts

+          404: listUUID ? `No list with UUID ${listUUID} found.` : 'No lists for the current user were found.',
+        },
+      }),
+    { revalidateOnFocus: hasAccess, refreshInterval: 1000 * 60 },


Do we need to refresh this on an interval or only once the user has performed some action which might affect what's returned?

The refresh interval isn't needed - just removed. This was based on the workspaces fetcher function.

john-conroy · 2025-01-30T15:19:15Z

context/app/static/js/hooks/useSavedEntityData.ts

+function sortEntities(searchHits: Required<SearchHit<Entity>>[], savedEntities: SavedEntities) {
+  if (!searchHits) return [];
+
+  return [...searchHits].sort((a, b) => {
+    const entityA: SavedEntity = savedEntities[a._id] ?? { dateSaved: 0 };
+    const entityB: SavedEntity = savedEntities[b._id] ?? { dateSaved: 0 };
+
+    const dateA = entityA.dateSaved ?? 0;
+    const dateB = entityB.dateSaved ?? 0;
+
+    return dateB - dateA;
+  });
+}
+


Can we apply sort to the ES query?

I don't know of a good way to do this in the query (apart from maybe passing in a custom comparator function for the sort) - the dateSaved property is only accessible by looking up the UUID in the savedEntities object, and not from any info returned by ES. What would you recommend?

context/app/static/js/components/savedLists/hooks.ts

NickAkhmetov

Great improvements! I will also need to test this locally 😄

context/app/static/js/components/detailPage/summary/SummarySaveEntityButton/index.ts

context/app/static/js/components/savedLists/SaveEntitiesButton/SaveEntitiesButton.tsx

NickAkhmetov · 2025-01-30T15:34:23Z

context/app/static/js/components/savedLists/SaveEntitiesButton/SaveEntitiesButton.tsx

+        handleSaveEntities({ entityUUIDs: uuids }).catch((error) => {
+          console.error(error);
+        });


Would it be helpful to track errored attempts to save as well? @tsliaw

I've put together a list of lists-related events to track that Tiffany and I will go over together - that will be a separate PR (CAT-1113). I don't believe based on the event tracking doc that we are currently tracking errors for most events, but that's definitely an option to consider.

We don't typically track error states, so I don't see the need to track this one unless we expect this error to occur often.

context/app/static/js/components/savedLists/SavedListsDescription/SavedListsDescription.tsx

context/app/static/js/components/savedLists/SavedListsContent/style.ts

NickAkhmetov · 2025-01-30T18:06:52Z

context/app/static/js/components/savedLists/SaveEntitiesButton/SaveEntitiesButton.tsx

+  let tooltip = '';
+
+  if (disabled) {
+    tooltip = allInSavedEntities ? `All ${entityLabel} are already saved.` : `Select ${entityTypes} to save.`;
+  } else {
+    tooltip = `Save ${entityLabel}.`;
+  }


Maybe we can pull this out into a helper function instead to keep things more readable? e.g.

// Outside of the component function createTooltip(entityLabel: string, allInSavedEntities: boolean, disabled: boolean) { if (disabled) { return `Save ${entityLabel}.` } if (allInSavedEntities) { return `All ${entityLabel} are already saved.` } return `Select ${entityLabel}s to save.` } ... // Back inside the component const tooltip = createTooltip(entityLabel, allInSavedEntities, disabled)

That would also let us add some unit tests if need be.

NickAkhmetov · 2025-01-30T18:12:34Z

context/app/static/js/components/savedLists/SaveEntitiesButton/SaveEntitiesButton.tsx

+      onClick={() => {
+        trackSave({ action: 'Save To List', label: generateCommaList(Array.from(uuids)) });
+        handleSaveEntities({ entityUUIDs: uuids }).catch((error) => {
+          console.error(error);
+        });
+      }}


In my experience, useCallback is necessary whenever passing event handler functions down, since it breaks memoization otherwise.

We could also use useEventCallback from MUI (https://usehooks-ts.com/react-hook/use-event-callback - same hook, different library) which was designed to be easier to use than useCallback and address issues like facebook/react#14099.

context/app/static/js/components/savedLists/SavedListPanel/hooks.ts

john-conroy · 2025-02-04T17:40:41Z

@austenem can you merge main into this branch so the virtualenvs are aligned between the branches?

austenem added 30 commits January 6, 2025 17:02

add ukv endpoint

a3d8db0

pull authenticated lists into my lists page

c175b6c

implement add functionality

f053c9e

refactor hooks

79e358b

update deletion functionality

474e6ff

update lists functionality

535979c

fix typing

72d9e54

add entity to list functionality

b2721d8

edit and deletion functionality

f9a2919

liststobedeleted functionality

8a4b318

remove savedlistscontent

5ac2d16

update stores

e0a60e1

move logic to hooks

b0c654f

typescript conversions and list page update

2ad59ef

adjust useeffect and language

56476b0

add documentation, minor updates

5ab11ea

fix re-rendering issue

c3a2e27

copy over local lists and entities

7a1b127

update local to remote copy logic

62be1cd

update save entity logic

d45d82d

update alerts

4f8b5e2

continue adding alerts

5a86baa

add save entities button to search page

2714c12

update saved items table, add button to table

65dd5ab

adjust button tooltips

6738147

add barrel files

19f6c9b

convert to ts

8b6ca64

continue conversions

b6f1209

separate out api and store

972cba2

update tests

c994ddc

austenem added 2 commits January 22, 2025 13:36

update list page

135fbf3

update isLoading bool to pass test

7be52bb

john-conroy reviewed Jan 22, 2025

View reviewed changes

austenem added 14 commits January 24, 2025 10:35

remove lists page for unauthenticated users

dc73e1f

refactor alerts

264190b

hide list buttons from unauthenticated users

bbbc912

finish refactoring hooks

b5d114e

fix new list dialog

6f60d73

minor updates and extend alerts

b4091cb

update jest test

255339d

update test and implement copy logic

cdc7d18

update hook to use default export

5706f9b

update storage description

7b60842

minor cleanup

aa17827

finish deleting stores

e665d2f

update logged out description

71d431e

add save entity button to processed datasets

d22059f

austenem requested a review from john-conroy January 28, 2025 22:56

john-conroy reviewed Jan 30, 2025

View reviewed changes

context/app/static/js/components/savedLists/hooks.ts Outdated Show resolved Hide resolved

NickAkhmetov reviewed Jan 30, 2025

View reviewed changes

austenem added 2 commits January 31, 2025 16:22

review cleanup 1

3acb253

update import

bc39893

austenem requested review from NickAkhmetov and john-conroy January 31, 2025 21:29

john-conroy reviewed Feb 4, 2025

View reviewed changes

context/app/static/js/components/savedLists/SavedListPanel/hooks.ts Outdated Show resolved Hide resolved

context/app/static/js/components/savedLists/SavedListPanel/hooks.ts Outdated Show resolved Hide resolved

austenem added 2 commits February 4, 2025 15:41

Merge branch 'main' into austenem/cat-754-refactor-my-lists

73bf0ef

update hooks

471b152

austenem requested a review from john-conroy February 4, 2025 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Austenem/CAT-754 Refactor "My Lists" #3671

Austenem/CAT-754 Refactor "My Lists" #3671

austenem commented Jan 22, 2025 •

edited

Loading

john-conroy left a comment

john-conroy left a comment

john-conroy Jan 30, 2025

austenem Jan 30, 2025

john-conroy Jan 30, 2025

austenem Jan 30, 2025

john-conroy Jan 30, 2025

austenem Jan 31, 2025

john-conroy Jan 30, 2025

austenem Jan 31, 2025

NickAkhmetov left a comment

NickAkhmetov Jan 30, 2025

austenem Jan 31, 2025

tsliaw Feb 3, 2025

NickAkhmetov Jan 30, 2025

NickAkhmetov Jan 30, 2025

john-conroy commented Feb 4, 2025

Austenem/CAT-754 Refactor "My Lists" #3671

Are you sure you want to change the base?

Austenem/CAT-754 Refactor "My Lists" #3671

Conversation

austenem commented Jan 22, 2025 • edited Loading

Summary

Design Documentation/Original Tickets

Testing

Screenshots/Video

Checklist

john-conroy left a comment

Choose a reason for hiding this comment

john-conroy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickAkhmetov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

john-conroy commented Feb 4, 2025

austenem commented Jan 22, 2025 •

edited

Loading