feat: add custom filter function #632

KevinRohlf · 2024-11-20T15:42:37Z

Implements #614

Proposed Changes

add a custom filter function

vercel · 2024-11-20T15:42:43Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mvg-portal	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 18, 2024 0:59am

filters.config.js

src/components/Search/Filter.tsx

filters.config.js

Abrom8 · 2024-11-21T15:45:53Z

src/components/Search/utils.ts

  let searchTerm = text || ''
  let nestedQuery
  if (tags) {
    filters.push(getFilterTerm('metadata.tags.keyword', tags))
+  } else if (gaiax) {


question(blocking): Is this query not compatible with tags or why is it only applied if tags is falsy?

src/@types/aquarius/BaseQueryParams.d.ts

filters.config.js

src/@types/aquarius/BaseQueryParams.d.ts

src/@utils/aquarius/index.ts

Abrom8 · 2024-11-21T16:29:19Z

src/@utils/aquarius/index.ts

+export function getFilter(...args: any[]) {
+  let filters = []
+  if (typeof args[0] === 'object') {
+    args[0].forEach((arg) => {


suggestion(blocking): In general prefer usage of for...of over forEach for readability and performance. More context: https://biomejs.dev/linter/rules/no-for-each/

This comment applies to all places in the code where forEach is being used.

…ery interface and FILTER_VALUES, update getSearchQuery function

moritzkirstein · 2024-11-25T15:21:13Z

src/@types/aquarius/BaseQueryParams.d.ts

+interface BoolFilter {
+  bool: BoolFilterQuery
+}
+
+interface BoolFilterQuery {
+  must: {
+    exists: {
+      field: string
+    }
+  }
+  must_not?: {
+    term: {
+      [key: string]: string
+    }
+  }
+}
+


suggestion(blocking): The BoolFilterQuery type is named to generally for what it does. Specifically these are the type definitions for the new must_exist and must_exits_and_not_empty filters of this PR. Naming these accordingly is suggested (e.g. MustExistQuery and MustNotTermQuery or similar)

I also suggest to rename the key identifier to field to better communicate what value is expected here.

moritzkirstein · 2024-11-25T15:31:35Z

src/@types/aquarius/BaseQueryParams.d.ts

+interface BoolFilter {
+  bool: BoolFilterQuery
+}
+
+interface BoolFilterQuery {
+  must: {
+    exists: {
+      field: string
+    }
+  }
+  must_not?: {
+    term: {
+      [key: string]: string
+    }
+  }
+}


suggestion(non-blocking): this is also a nice use-case for typescript generics

In our case, these filters should always be used on a field path that is to be defined by the user. We expect a certain behaviour though, e.g. something like the following could improve DX with TS, as it helps enforcing correct types (in this case the field path value).

type FieldPath = string interface BoolFilter<T extends FieldPath> { bool: MustExistQuery<T> & MustNotTermQuery<T> } interface MustExistQuery<T extends FieldPath> { must: { exists: { field: T } } } interface MustNotTermQuery<T extends FieldPath> { must_not?: { term: { [field in T]: string } } }

moritzkirstein · 2024-11-25T15:36:38Z

src/@types/aquarius/BaseQueryParams.d.ts

@@ -11,6 +28,7 @@ interface BaseQueryParams {
  sortOptions?: SortOptions
  aggs?: any
  filters?: FilterTerm[]
+  boolFilter?: BoolFilter[]


issue(blocking): i don't think this is supported by elasticsearch (array of bool) and should be adjusted.

suggestion: we could treat it like filters, where we define what the BoolTerm should look like and then allow passing an array BoolTerm[] here.
Also possible is to only allow the defined type to be passed, e.g. BoolFilter

moritzkirstein · 2024-11-25T15:39:45Z

src/@utils/aquarius/index.ts

@@ -56,6 +57,39 @@ export function getFilterTerm(
  }
 }

+export function getFilter(...args: any[]) {


issue(blocking): we should try not to use any as type here. If it is needed, we need to add a comment disabling eslint for this line and describing why the any type is used

moritzkirstein · 2024-11-25T15:41:12Z

src/@utils/aquarius/index.ts

+      const filter = arg.split('=')
+      filters = [...filters, filter]


suggestion(blocking): move this code to a named function and reuse it to increase readability

moritzkirstein · 2024-11-25T15:48:57Z

src/@utils/aquarius/index.ts

@@ -160,7 +197,8 @@ export function generateBaseQuery(
                ...(baseQueryParams.showSaas === false ? [saasFieldExists] : [])
              ]
            }
-          }
+          },
+          ...(baseQueryParams.boolFilter || [])


suggestion(non-blocking): use an empty object rather than empty array for better readability of the code.

moritzkirstein · 2024-11-25T15:50:32Z

src/components/Search/Filter.tsx

+    let updatedFilters
+    if (option.queryPath) {
+      updatedFilters = filters[filterId].includes(
+        `${option.queryPath}=${option.value}`


suggestion(blocking): make this re-usable by wrapping it into a named function. Can then be accessed in the various places in code (e.g. getFilterQueryString or similar)

this also will get more readable once adapting existing filters to the new, more flexible structure and a lot of code can be removed here.

moritzkirstein · 2024-11-25T15:51:25Z

src/components/Search/Filter.tsx

+        ? {
+            ...filters,
+            [filterId]: filters[filterId].filter(
+              (e) => e !== `${option.queryPath}=${option.value}`


suggestion(blocking): avoid shorthand variable name and replace with more readable variant

moritzkirstein · 2024-11-25T16:01:38Z

filters.config.js

+    {
+      id: 'serviceType',
+      label: 'Service Type',
+      type: 'filterList',
+      options: [
+        { label: 'datasets', value: FilterByTypeOptions.Data },
+        { label: 'algorithms', value: FilterByTypeOptions.Algorithm },
+        { label: 'saas', value: FilterByTypeOptions.Saas }
+      ]
+    },
+    {
+      id: 'accessType',
+      label: 'Access Type',
+      type: 'filterList',
+      options: [
+        { label: 'download', value: FilterByAccessOptions.Download },
+        { label: 'compute', value: FilterByAccessOptions.Compute }
+      ]
+    },


suggestion(blocking): to reduce complexity and avoid having to define filters differently depending on the id we should simply move these filters over to the new specs. This would then look something like the example in the issue:

{ id: 'serviceType', label: 'Service Type', type: 'filterList', // add a "global" query path that gets used as fallback if options have none defined queryPath: 'metadata.type', options: [ { label: 'datasets', value: FilterByTypeOptions.Data }, { label: 'algorithms', value: FilterByTypeOptions.Algorithm }, { label: 'saas', value: FilterByTypeOptions.Saas } ] }, //...

…ved type handling

feat: add custom filter function

1868ebc

KevinRohlf requested a review from a team as a code owner November 20, 2024 15:42

KevinRohlf changed the base branch from main to development November 20, 2024 15:43

vercel bot deployed to Preview November 20, 2024 15:43 View deployment

Abrom8 requested changes Nov 21, 2024

View reviewed changes

chore: implements all filters to filters.config.js, move BoolFilterQu…

c2819f9

…ery interface and FILTER_VALUES, update getSearchQuery function

KevinRohlf requested a review from Abrom8 November 22, 2024 18:52

vercel bot deployed to Preview November 22, 2024 18:53 View deployment

moritzkirstein requested changes Nov 25, 2024

View reviewed changes

feat: enhance filter functionality with dynamic query paths and impro…

401b690

…ved type handling

vercel bot deployed to Preview December 3, 2024 07:06 View deployment

KevinRohlf requested a review from moritzkirstein December 3, 2024 07:08

fix: update filter component keys

303f283

vercel bot deployed to Preview December 5, 2024 06:13 View deployment

fix: isSelected on top position

302b545

vercel bot deployed to Preview December 5, 2024 07:24 View deployment

feat: add MUST_EXIST & MUSTS_EXITS_AND_NON_EMPTY filters for all ids

581edd0

vercel bot deployed to Preview December 18, 2024 12:59 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add custom filter function #632

feat: add custom filter function #632

KevinRohlf commented Nov 20, 2024 •

edited

Loading

vercel bot commented Nov 20, 2024 •

edited

Loading

Abrom8 Nov 21, 2024

Abrom8 Nov 21, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024

moritzkirstein Nov 25, 2024 •

edited

Loading

feat: add custom filter function #632

Are you sure you want to change the base?

feat: add custom filter function #632

Conversation

KevinRohlf commented Nov 20, 2024 • edited Loading

Proposed Changes

vercel bot commented Nov 20, 2024 • edited Loading

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

moritzkirstein Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

KevinRohlf commented Nov 20, 2024 •

edited

Loading

vercel bot commented Nov 20, 2024 •

edited

Loading

moritzkirstein Nov 25, 2024 •

edited

Loading