feat: [M3-8883] - Use our search parser on Images Landing (linode#11233)

* initial work * make overriding contains filter shape more generic * make things more generic * allow underscore in seaches * allow `:` in searches * only disable retries if the user is searching * add changeset * add changeset * use `errorText` prop @hana-akamai * add comment to summarize what is filterable --------- Co-authored-by: Banks Nussman <banks@nussman.us>
santoshp210-akamai · Nov 14, 2024 · b40d5bd · b40d5bd
1 parent afbd8d9
commit b40d5bd
Show file tree

Hide file tree

Showing 8 changed files with 119 additions and 26 deletions.
diff --git a/packages/api-v4/.changeset/pr-11233-added-1731517130535.md b/packages/api-v4/.changeset/pr-11233-added-1731517130535.md
@@ -0,0 +1,5 @@
+---
+"@linode/api-v4": Added
+---
+
+Missing `+eq` type to `FilterConditionTypes` interface ([#11233](https://github.com/linode/manager/pull/11233))
diff --git a/packages/api-v4/src/iam/types.ts b/packages/api-v4/src/iam/types.ts
@@ -50,4 +50,4 @@ export interface Roles {
   name: string;
   description: string;
   permissions?: PermissionType[];
-}
+}
diff --git a/packages/api-v4/src/types.ts b/packages/api-v4/src/types.ts
@@ -39,11 +39,12 @@ export interface RequestOptions {
   headers?: RequestHeaders;
 }
 
-interface FilterConditionTypes {
+export interface FilterConditionTypes {
   '+and'?: Filter[];
   '+or'?: Filter[] | string[];
   '+order_by'?: string;
   '+order'?: 'asc' | 'desc';
+  '+eq'?: string | number;
   '+gt'?: number;
   '+gte'?: number;
   '+lt'?: number;

diff --git a/packages/manager/.changeset/pr-11233-added-1731517041735.md b/packages/manager/.changeset/pr-11233-added-1731517041735.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Added
+---
+
+Ability to perform complex search queries on the Images landing page ([#11233](https://github.com/linode/manager/pull/11233))
diff --git a/packages/manager/src/features/Images/ImagesLanding/ImagesLanding.tsx b/packages/manager/src/features/Images/ImagesLanding/ImagesLanding.tsx
@@ -1,3 +1,4 @@
+import { getAPIFilterFromQuery } from '@linode/search';
 import {
   CircleProgress,
   IconButton,
@@ -28,6 +29,7 @@ import { TableCell } from 'src/components/TableCell';
 import { TableHead } from 'src/components/TableHead';
 import { TableRow } from 'src/components/TableRow';
 import { TableRowEmpty } from 'src/components/TableRowEmpty/TableRowEmpty';
+import { TableRowError } from 'src/components/TableRowError/TableRowError';
 import { TableSortCell } from 'src/components/TableSortCell';
 import { TextField } from 'src/components/TextField';
 import { Typography } from 'src/components/Typography';
@@ -59,7 +61,7 @@ import type { Handlers as ImageHandlers } from './ImagesActionMenu';
 import type { Filter, ImageStatus } from '@linode/api-v4';
 import type { Theme } from '@mui/material/styles';
 
-const searchQueryKey = 'query';
+const searchParamKey = 'query';
 
 const useStyles = makeStyles()((theme: Theme) => ({
   imageTable: {
@@ -102,10 +104,34 @@ export const ImagesLanding = () => {
     globalGrantType: 'add_images',
   });
   const queryParams = new URLSearchParams(location.search);
-  const imageLabelFromParam = queryParams.get(searchQueryKey) ?? '';
+  const query = queryParams.get(searchParamKey) ?? '';
 
   const queryClient = useQueryClient();
 
+  /**
+   * At the time of writing: `label`, `tags`, `size`, `status`, `region` are filterable.
+   *
+   * Some fields like `status` and `region` can't be used in complex filters using '+or' / '+and'
+   *
+   * Using `tags` in a '+or' is currently broken. See ARB-5792
+   */
+  const { error: searchParseError, filter } = getAPIFilterFromQuery(query, {
+    // Because Images have an array of region objects, we need to transform
+    // search queries like "region: us-east" to { regions: { region: "us-east" } }
+    // rather than the default behavior which is { region: { '+contains': "us-east" } }
+    filterShapeOverrides: {
+      '+contains': {
+        field: 'region',
+        filter: (value) => ({ regions: { region: value } }),
+      },
+      '+eq': {
+        field: 'region',
+        filter: (value) => ({ regions: { region: value } }),
+      },
+    },
+    searchableFieldsWithoutOperator: ['label', 'tags'],
+  });
+
   const paginationForManualImages = usePagination(1, 'images-manual', 'manual');
 
   const {
@@ -124,7 +150,7 @@ export const ImagesLanding = () => {
   const manualImagesFilter: Filter = {
     ['+order']: manualImagesOrder,
     ['+order_by']: manualImagesOrderBy,
-    ...(imageLabelFromParam && { label: { '+contains': imageLabelFromParam } }),
+    ...filter,
   };
 
   const {
@@ -148,6 +174,10 @@ export const ImagesLanding = () => {
       // to update Image region statuses. We should make the API
       // team and Images team implement events for this.
       refetchInterval: 30_000,
+      // If we have a search query, disable retries to keep the UI
+      // snappy if the user inputs an invalid X-Filter. Otherwise,
+      // pass undefined to use the default retry behavior.
+      retry: query ? false : undefined,
     }
   );
 
@@ -173,7 +203,7 @@ export const ImagesLanding = () => {
   const automaticImagesFilter: Filter = {
     ['+order']: automaticImagesOrder,
     ['+order_by']: automaticImagesOrderBy,
-    ...(imageLabelFromParam && { label: { '+contains': imageLabelFromParam } }),
+    ...filter,
   };
 
   const {
@@ -190,6 +220,12 @@ export const ImagesLanding = () => {
       ...automaticImagesFilter,
       is_public: false,
       type: 'automatic',
+    },
+    {
+      // If we have a search query, disable retries to keep the UI
+      // snappy if the user inputs an invalid X-Filter. Otherwise,
+      // pass undefined to use the default retry behavior.
+      retry: query ? false : undefined,
     }
   );
 
@@ -331,13 +367,13 @@ export const ImagesLanding = () => {
   };
 
   const resetSearch = () => {
-    queryParams.delete(searchQueryKey);
+    queryParams.delete(searchParamKey);
     history.push({ search: queryParams.toString() });
   };
 
   const onSearch = (e: React.ChangeEvent<HTMLInputElement>) => {
     queryParams.delete('page');
-    queryParams.set(searchQueryKey, e.target.value);
+    queryParams.set(searchParamKey, e.target.value);
     history.push({ search: queryParams.toString() });
   };
 
@@ -366,7 +402,7 @@ export const ImagesLanding = () => {
     return <CircleProgress />;
   }
 
-  if (manualImagesError || automaticImagesError) {
+  if (!query && (manualImagesError || automaticImagesError)) {
     return (
       <React.Fragment>
         <DocumentTitleSegment segment="Images" />
@@ -375,11 +411,7 @@ export const ImagesLanding = () => {
     );
   }
 
-  if (
-    manualImages?.results === 0 &&
-    automaticImages?.results === 0 &&
-    !imageLabelFromParam
-  ) {
+  if (manualImages?.results === 0 && automaticImages?.results === 0 && !query) {
     return <ImagesLandingEmptyState />;
   }
 
@@ -404,10 +436,9 @@ export const ImagesLanding = () => {
       />
       <TextField
         InputProps={{
-          endAdornment: imageLabelFromParam && (
+          endAdornment: query && (
             <InputAdornment position="end">
               {isFetching && <CircleProgress size="sm" />}
-
               <IconButton
                 aria-label="Clear"
                 data-testid="clear-images-search"
@@ -422,11 +453,12 @@ export const ImagesLanding = () => {
         onChange={debounce(400, (e) => {
           onSearch(e);
         })}
+        containerProps={{ mb: 2 }}
+        errorText={searchParseError?.message}
         hideLabel
         label="Search"
         placeholder="Search Images"
-        sx={{ mb: 2 }}
-        value={imageLabelFromParam}
+        value={query}
       />
       <Paper className={classes.imageTable}>
         <div className={classes.imageTableHeader}>
@@ -499,6 +531,12 @@ export const ImagesLanding = () => {
                 message={`No Custom Images to display.`}
               />
             )}
+            {manualImagesError && query && (
+              <TableRowError
+                colSpan={9}
+                message={manualImagesError[0].reason}
+              />
+            )}
             {manualImages?.data.map((manualImage) => (
               <ImageRow
                 event={manualImagesEvents[manualImage.id]}
@@ -572,6 +610,12 @@ export const ImagesLanding = () => {
                 message={`No Recovery Images to display.`}
               />
             )}
+            {automaticImagesError && query && (
+              <TableRowError
+                colSpan={9}
+                message={automaticImagesError[0].reason}
+              />
+            )}
             {automaticImages?.data.map((automaticImage) => (
               <ImageRow
                 event={automaticImagesEvents[automaticImage.id]}

diff --git a/packages/search/src/search.peggy b/packages/search/src/search.peggy
@@ -13,6 +13,7 @@ andQuery
 subQuery
   = '(' ws* query:orQuery ws* ')' { return query; }
   / EqualQuery
+  / TagQuery
   / ContainsQuery
   / NotEqualQuery
   / LessThanQuery
@@ -27,14 +28,25 @@ DefaultQuery
     }
 
 EqualQuery
-  = key:FilterableField ws* Equal ws* value:Number { return { [key]: value }; }
-  / key:FilterableField ws* Equal ws* value:String { return { [key]: value }; }
+  = key:FilterableField ws* Equal ws* value:SearchValue {
+      const override = options.filterShapeOverrides?.['+eq'];
+      if (override && key === override.field) {
+        return override.filter(value);
+      }
+      return { [key]: value };
+    }
 
 ContainsQuery
-  = key:FilterableField ws* Contains ws* value:String { return { [key]: { "+contains": value } }; }
+  = key:FilterableField ws* Contains ws* value:String {
+    const override = options.filterShapeOverrides?.['+contains'];
+    if (override && key === override.field) {
+      return override.filter(value);
+    }
+    return { [key]: { "+contains": value } };
+  }
 
 TagQuery
-  = "tag" ws* Equal ws* value:String { return { "tags": { "+contains": value } }; }
+  = "tag" ws* Contains ws* value:String { return { "tags": { "+contains": value } }; }
 
 NotEqualQuery
   = Not key:FilterableField ws* Equal ws* value:String { return { [key]: { "+neq": value } }; }
@@ -100,11 +112,15 @@ Word "word"
   = [a-zA-Z0-9\-\.]+ { return text(); }
 
 StringWithSpaces "string with spaces"
-  = [a-zA-Z0-9\-\. ]+ { return text(); }
+  = [a-zA-Z0-9\-|_\.\: ]+ { return text(); }
 
 Number "numeric search value"
   = number:[0-9\.]+ { return parseFloat(number.join("")); }
   / number:[0-9]+ { return parseInt(number.join(""), 10); }
 
+SearchValue
+  = Number
+  / String
+
 ws "whitespace"
   = [ \t\r\n]
diff --git a/packages/search/src/search.test.ts b/packages/search/src/search.test.ts
@@ -167,4 +167,20 @@ describe("getAPIFilterFromQuery", () => {
       error: null,
     });
   });
+
+  it("allows custom filter transformations on a per-field basis", () => {
+    const query = "region: us-east";
+
+    expect(
+      getAPIFilterFromQuery(query, {
+        searchableFieldsWithoutOperator: [],
+        filterShapeOverrides: {
+          '+contains': { field: 'region', filter: (value) => ({ regions: { region: value } }) }
+        }
+      })
+    ).toEqual({
+      filter: { regions: { region: 'us-east' } },
+      error: null,
+    });
+  });
 });
diff --git a/packages/search/src/search.ts b/packages/search/src/search.ts
@@ -1,5 +1,5 @@
 import { generate } from 'peggy';
-import type { Filter } from '@linode/api-v4';
+import type { Filter, FilterConditionTypes } from '@linode/api-v4';
 import grammar from './search.peggy?raw';
 
 const parser = generate(grammar);
@@ -8,10 +8,16 @@ interface Options {
   /**
    * Defines the API fields filtered against (currently using +contains)
    * when the search query contains no operators.
-   * 
+   *
    * @example ['label', 'tags']
    */
   searchableFieldsWithoutOperator: string[];
+  /**
+   * Somtimes, we may need to change the way the parser transforms operations
+   * into API filters. This option allows you to specify a custom transformation
+   * for a specific searchable field.
+   */
+  filterShapeOverrides?: Partial<Record<keyof FilterConditionTypes, { field: string; filter: (value: string) => Filter }>>;
 }
 
 /**
@@ -32,4 +38,4 @@ export function getAPIFilterFromQuery(query: string | null | undefined, options:
   }
 
   return { filter, error };
-}
+}