feat(ui/graphql) Add ability to sort search results from search results page #8595

chriscollins3456 · 2023-08-08T19:33:42Z

This PR updates our searchAcrossEntities endpoint to accept a new sortInput field that allows users to pass in a way to sort their search results. This endpoint will accept ASCENDING or DESCENDING on any field on an entity (so long as it's stored in ES on the entity index). Right now we have it only built out to accept a singular sorting option, but this sortInput could be extended later to accept a list of fields or something even more complicated.

This adds a dropdown in search results to choose from 4 sorting options we provide in the UI

Default recommended/relevance (what it always has been)
sort descending by name
sort ascending by name
descending by last updated in source system

Adding a new sorting option is very easy, just 2 lines of code change - check out my final commit for an example.

Screenshots:

sort name ascending

sort name descending

last modified in source platform

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

iprentic · 2023-08-08T20:09:38Z

metadata-service/restli-client/src/main/java/com/linkedin/entity/client/RestliEntityClient.java

@@ -456,7 +456,7 @@ public SearchResult searchAcrossEntities(@Nonnull List<String> entities, @Nonnul
  @Nonnull
  public SearchResult searchAcrossEntities(@Nonnull List<String> entities, @Nonnull String input,
      @Nullable Filter filter, int start, int count, @Nullable SearchFlags searchFlags,
-      @Nonnull final Authentication authentication, @Nullable List<String> facets)
+      @Nullable SortCriterion sortCriterion, @Nonnull final Authentication authentication, @Nullable List<String> facets)
      throws RemoteInvocationException {

    final EntitiesDoSearchAcrossEntitiesRequestBuilder requestBuilder =


I think the EntitiesDoSearchAcrossEntitiesRequestBuilder actually has a parameter for the sort criterion: Do you want to add it here as well?

nice good call! adding now

joshuaeilers · 2023-08-08T23:19:22Z

datahub-web-react/src/app/search/SearchPage.tsx

    const { filterMode, filterModeRef, setFilterMode } = useFilterMode(filters, unionType);
+    const { selectedSortOption } = useSearchContext();


This looks like a great place for a useSelectedSortOption custom hook

joshuaeilers · 2023-08-08T23:21:24Z

datahub-web-react/src/app/search/context/SearchContextProvider.tsx

+import { DEFAULT_SORT_OPTION, SORT_OPTIONS } from './constants';
+
+export default function SearchContextProvider({ children }: { children: React.ReactNode }) {
+    const [selectedSortOption, setSelectedSortOption] = useState(DEFAULT_SORT_OPTION);


Not sure if I'm missing something, but why do we need another state value here? Couldn't we just map what we need from params into another var and pass that into the context? so updateSelectedSortOption would mutate the url and then params would swap the value for a fresh context object.

yeah tbh this is because I built it without storing in the URLfirst to start and then added storing in the URL later - but now with fresh eyes I totally agree. I like having this context as it's easier to grab values and set values and we can extend it later for other global search state stuff.

so I think i'll just remove this local state and pass in selectedSortOption as the value from URL params and the setter function to update url params.

joshuaeilers · 2023-08-08T23:22:58Z

datahub-web-react/src/app/search/context/constants.ts

+export const SORT_OPTIONS = {
+    [RECOMMENDED]: { label: 'Recommended', field: RECOMMENDED, sortOrder: SortOrder.Descending },
+    [`${NAME_FIELD}_${SortOrder.Ascending}`]: { label: 'A to Z', field: NAME_FIELD, sortOrder: SortOrder.Ascending },
+    [`${NAME_FIELD}_${SortOrder.Descending}`]: { label: 'Z to A', field: NAME_FIELD, sortOrder: SortOrder.Descending },
+    [`${LAST_OPERATION_TIME_FIELD}_${SortOrder.Descending}`]: {
+        label: 'Last Modified in Platform',
+        field: LAST_OPERATION_TIME_FIELD,
+        sortOrder: SortOrder.Descending,
+    },
+};


Soooooo good

joshuaeilers · 2023-08-08T23:23:24Z

datahub-web-react/src/app/search/context/useSearchContext.tsx

+
+export function useSearchContext() {
+    return useContext(SearchContext);
+}


Could throw a few other custom hooks in here for selecting off of this context

joshuaeilers · 2023-08-08T23:26:51Z

datahub-web-react/src/app/search/utils/utils.ts

+import { RouteComponentProps } from 'react-router';
+
+export function updateUrlParam(history: RouteComponentProps['history'], key: string, value: string) {
+    const url = new URL(window.location.href);
+    const { searchParams } = url;
+    searchParams.set(key, value);
+    history.replace(url.search);
+}


This feels even more generic than search and could go in a higher-level utils file somewhere.

I'm curious how the browse sidebar behaves when we mutate url params without adjusting a filter.... should be fine but I know we got a useEffect in there that kicks off some stuff when params change.

yeah I could see that and may move this. But I tested the browse sidebar with this functionality and things look solid. it's not a URL param they're listening to so things look unaffected

joshuaeilers · 2023-08-09T15:59:03Z

datahub-web-react/src/app/AppProviders.tsx

@@ -13,7 +14,9 @@ export default function AppProviders({ children }: Props) {
        <AppConfigProvider>
            <UserContextProvider>
                <EducationStepsProvider>
-                    <QuickFiltersProvider>{children}</QuickFiltersProvider>
+                    <QuickFiltersProvider>
+                        <SearchContextProvider>{children}</SearchContextProvider>


So happy we have this now.

This is probably where our subscription state could go in the future if we ever wanted to hoist it up.

joshuaeilers · 2023-08-09T16:00:54Z

datahub-web-react/src/app/search/context/SearchContext.tsx

+
+export function useSearchContext() {
+    const context = useContext(SearchContext);
+    if (context === null) throw new Error(`${useSearchContext.name} must be used under a SearchContextProvider`);


For this guard to work you would need to initialize createContext with null I think. Making the type up there SearchContextType | null. Not a big deal since we're not building a library here, just a wire-up for our state.

joshuaeilers · 2023-08-09T16:01:51Z

datahub-web-react/src/app/search/sorting/SearchSortSelect.tsx

+`;
+
+export default function SearchSortSelect() {
+    const { selectedSortOption, setSelectedSortOption } = useSearchContext();


Could be another opportunity for some custom hooks, non blocking feedback here of course!

what would the benefit of using two different custom hooks here? at the end of the day I'm just trying to get these values from our context

In this example, not much. But as contexts grow and become more complex, having less usage of the global context and more specific selection of the values becomes cleaner imo.

…ts page (#8595)

chriscollins3456 added 6 commits August 8, 2023 13:39

make changes to backend searchAcrossEntities

e67cbd8

Update frontend to display sort dropdown and pass sort into backend

7670082

store sort order in the URL

834fefd

minor edits

709a322

finish implementing endpoint

0e9e60a

add last modified in source platform as filter

cda22a4

github-actions bot added product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment labels Aug 8, 2023

chriscollins3456 changed the title ~~feat(ui) Add ability to sort search results from search results page~~ feat(ui/graphql) Add ability to sort search results from search results page Aug 8, 2023

chriscollins3456 added 2 commits August 8, 2023 15:47

fix broken tests

b2d4970

minor edits

4cd33f6

iprentic reviewed Aug 8, 2023

View reviewed changes

vercel bot deployed to Preview August 8, 2023 20:19 View deployment

joshuaeilers reviewed Aug 8, 2023

View reviewed changes

chriscollins3456 added 2 commits August 9, 2023 10:47

PR edits from review

0c82967

update resltli searchAcrossEntities to sort

05a720f

chriscollins3456 requested review from joshuaeilers and iprentic August 9, 2023 14:51

vercel bot deployed to Preview August 9, 2023 15:15 View deployment

move provider higher up and fix query bug

2c1b82a

joshuaeilers reviewed Aug 9, 2023

View reviewed changes

joshuaeilers approved these changes Aug 9, 2023

View reviewed changes

vercel bot deployed to Preview August 9, 2023 16:10 View deployment

update name from recommended to relevance

4efd410

vercel bot deployed to Preview August 9, 2023 19:33 View deployment

chriscollins3456 merged commit 265e2a4 into datahub-project:master Aug 9, 2023
35 of 36 checks passed

yoonhyejin pushed a commit that referenced this pull request Aug 24, 2023

feat(ui/graphql) Add ability to sort search results from search resul…

666435f

…ts page (#8595)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui/graphql) Add ability to sort search results from search results page #8595

feat(ui/graphql) Add ability to sort search results from search results page #8595

chriscollins3456 commented Aug 8, 2023

iprentic Aug 8, 2023

chriscollins3456 Aug 9, 2023

joshuaeilers Aug 8, 2023

joshuaeilers Aug 8, 2023

chriscollins3456 Aug 9, 2023

joshuaeilers Aug 8, 2023

joshuaeilers Aug 8, 2023

joshuaeilers Aug 8, 2023

joshuaeilers Aug 8, 2023

chriscollins3456 Aug 9, 2023

joshuaeilers Aug 9, 2023

joshuaeilers Aug 9, 2023

joshuaeilers Aug 9, 2023

joshuaeilers Aug 9, 2023

chriscollins3456 Aug 9, 2023

joshuaeilers Aug 9, 2023

		const { filterMode, filterModeRef, setFilterMode } = useFilterMode(filters, unionType);
		const { selectedSortOption } = useSearchContext();

feat(ui/graphql) Add ability to sort search results from search results page #8595

feat(ui/graphql) Add ability to sort search results from search results page #8595

Conversation

chriscollins3456 commented Aug 8, 2023

Checklist

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

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