korbit-ai[bot] commented on code in PR #33298: URL: https://github.com/apache/superset/pull/33298#discussion_r2109475509
########## superset/views/logs.py: ########## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from flask_appbuilder import permission_name +from flask_appbuilder.api import expose +from flask_appbuilder.security.decorators import has_access + +from superset.superset_typing import FlaskResponse + +from .base import BaseSupersetView + + +class ActionLogView(BaseSupersetView): + route_base = "/" Review Comment: ### Incorrect Route Base Path <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The route_base is set to the root path '/' which could potentially conflict with other routes and doesn't reflect the module's specific purpose. ###### Why this matters Using the root path as route_base means all routes in this view will be relative to '/', which could lead to route conflicts with other views and make the URL structure less organized. ###### Suggested change ∙ *Feature Preview* Change the route_base to be specific to the action log feature: ```python route_base = "/actionlog/" ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a49087aa-32f0-48d4-8bde-49e10ccc7429/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a49087aa-32f0-48d4-8bde-49e10ccc7429?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a49087aa-32f0-48d4-8bde-49e10ccc7429?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a49087aa-32f0-48d4-8bde-49e10ccc7429?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a49087aa-32f0-48d4-8bde-49e10ccc7429) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:22cd39b5-3459-42e8-a7b8-4c311437e836 --> [](22cd39b5-3459-42e8-a7b8-4c311437e836) ########## superset-frontend/src/features/groups/types.ts: ########## @@ -0,0 +1,31 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +export interface BaseGroupListModalProps { + show: boolean; + onHide: () => void; + onSave: () => void; +} + +export interface FormValues { + name: string; + label?: string; + description?: string; + roles: number[]; + users: { value: number; label: string }[]; Review Comment: ### Weak User Type Safety <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The users array type allows for potential mismatches between value and label, as there's no guarantee the number value corresponds to the correct user's label. ###### Why this matters This could lead to incorrect user associations if the value-label pairs aren't properly maintained, causing data integrity issues in group management. ###### Suggested change ∙ *Feature Preview* Create a proper User type and use it in the form values: ```typescript interface User { id: number; username: string; } export interface FormValues { // ... other fields users: User[]; } ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/088d90fe-c7dc-49a8-a566-a38093b54eee/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/088d90fe-c7dc-49a8-a566-a38093b54eee?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/088d90fe-c7dc-49a8-a566-a38093b54eee?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/088d90fe-c7dc-49a8-a566-a38093b54eee?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/088d90fe-c7dc-49a8-a566-a38093b54eee) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:6790da2e-d80f-40c2-97b9-3d2a7de53c5e --> [](6790da2e-d80f-40c2-97b9-3d2a7de53c5e) ########## superset/views/log/api.py: ########## @@ -55,6 +69,9 @@ class LogRestApi(LogMixin, BaseSupersetModelRestApi): "duration_ms", "referrer", ] + search_filters = { + "user": [FilterRelationOneToManyEqual], + } Review Comment: ### Missing Filter Relationship Documentation <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The search_filters configuration lacks documentation about the relationship filter's purpose ###### Why this matters Without context about the filter's purpose, other developers may not understand the relationship being filtered or how to properly use it ###### Suggested change ∙ *Feature Preview* Add a brief comment explaining the relationship: ```python search_filters = { # Filter logs by matching against the associated user "user": [FilterRelationOneToManyEqual], } ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a1114e10-b9f8-45e1-b1e8-8b9451113a29/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a1114e10-b9f8-45e1-b1e8-8b9451113a29?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a1114e10-b9f8-45e1-b1e8-8b9451113a29?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a1114e10-b9f8-45e1-b1e8-8b9451113a29?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a1114e10-b9f8-45e1-b1e8-8b9451113a29) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:4a7dc390-2688-4a0f-9726-3cbbd5991f03 --> [](4a7dc390-2688-4a0f-9726-3cbbd5991f03) ########## superset-frontend/src/pages/ActionLog/index.tsx: ########## @@ -0,0 +1,282 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useMemo } from 'react'; +import { t, css } from '@superset-ui/core'; +import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu'; +import { useListViewResource } from 'src/views/CRUD/hooks'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import ListView, { Filters, FilterOperator } from 'src/components/ListView'; +// eslint-disable-next-line no-restricted-imports +import { Typography } from 'antd-v5'; +import { fetchUserOptions } from 'src/features/groups/utils'; + +export type ActionLogObject = { + user: { + username: string; + }; + action: string; + dttm: string | null; + dashboard_id?: number; + slice_id?: number; + json?: string; + duration_ms?: number; + referrer?: string; +}; Review Comment: ### Missing field descriptions in ActionLogObject type <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The ActionLogObject type lacks documentation describing the purpose and usage of each field. ###### Why this matters Without field descriptions, other developers may misuse fields or misinterpret their significance in the action logging system. ###### Suggested change ∙ *Feature Preview* /** * Type representing an action log entry in the system. */ export type ActionLogObject = { /** User who performed the action */ user: { username: string; }; /** Description of the action performed */ action: string; /** Timestamp when the action occurred */ dttm: string | null; /** ID of the related dashboard, if applicable */ dashboard_id?: number; /** ID of the related slice/chart, if applicable */ slice_id?: number; /** Additional action details in JSON format */ json?: string; /** Time taken to perform the action in milliseconds */ duration_ms?: number; /** URL that referred to this action */ referrer?: string; }; ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33f91fcf-997a-4dd5-a4ed-33567def466d/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33f91fcf-997a-4dd5-a4ed-33567def466d?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33f91fcf-997a-4dd5-a4ed-33567def466d?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33f91fcf-997a-4dd5-a4ed-33567def466d?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33f91fcf-997a-4dd5-a4ed-33567def466d) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:9aa3c9c6-5b34-41fa-81dc-dff24f5d337c --> [](9aa3c9c6-5b34-41fa-81dc-dff24f5d337c) ########## superset/views/log/api.py: ########## @@ -45,7 +46,20 @@ resource_name = "log" allow_browser_login = True list_columns = [ - "user.username", + "user", + "user_id", + "action", + "dttm", + "json", + "slice_id", + "dashboard_id", + "user_id", + "duration_ms", + "referrer", + ] Review Comment: ### Duplicate Column Definition <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The list_columns array contains a duplicate entry for 'user_id' ###### Why this matters Duplicate columns in the list can cause data redundancy and potential confusion in API responses ###### Suggested change ∙ *Feature Preview* Remove the duplicate 'user_id' entry from list_columns: ```python list_columns = [ "user", "user_id", "action", "dttm", "json", "slice_id", "dashboard_id", "duration_ms", "referrer", ] ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/aa23a055-2c19-4983-b728-bbc2e426df9a/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/aa23a055-2c19-4983-b728-bbc2e426df9a?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/aa23a055-2c19-4983-b728-bbc2e426df9a?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/aa23a055-2c19-4983-b728-bbc2e426df9a?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/aa23a055-2c19-4983-b728-bbc2e426df9a) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:2c70edf5-7d52-4603-afb8-9e901524624f --> [](2c70edf5-7d52-4603-afb8-9e901524624f) ########## superset-frontend/src/features/groups/utils.ts: ########## @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { SupersetClient, t } from '@superset-ui/core'; +import rison from 'rison'; +import { FormValues } from './types'; + +export const createGroup = async (values: FormValues) => { + await SupersetClient.post({ + endpoint: '/api/v1/security/groups/', + jsonPayload: { ...values, users: values.users.map(user => user.value) }, + }); +}; + +export const updateGroup = async (groupId: number, values: FormValues) => { + await SupersetClient.put({ + endpoint: `/api/v1/security/groups/${groupId}`, + jsonPayload: { ...values, users: values.users.map(user => user.value) }, + }); +}; + +export const deleteGroup = async (groupId: number) => + SupersetClient.delete({ + endpoint: `/api/v1/security/groups/${groupId}`, + }); + +export const fetchUserOptions = async ( + filterValue: string, + page: number, + pageSize: number, + addDangerToast: (msg: string) => void, +) => { + const query = rison.encode({ + filter: filterValue, + page, + page_size: pageSize, + order_column: 'username', + order_direction: 'asc', + }); + + try { + const response = await SupersetClient.get({ + endpoint: `/api/v1/security/users/?q=${query}`, + }); + + const results = response.json?.result || []; + + return { + data: results.map((user: any) => ({ + value: user.id, + label: user.username, + })), + totalCount: response.json?.count ?? 0, + }; + } catch (error) { + addDangerToast(t('There was an error while fetching users')); + return { data: [], totalCount: 0 }; + } Review Comment: ### Generic Error Message in User Fetch <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The fetchUserOptions error handler uses a generic error message without including the actual error details. ###### Why this matters Generic error messages make it difficult to diagnose and debug issues in production environments. ###### Suggested change ∙ *Feature Preview* ```typescript catch (error) { addDangerToast(t('Failed to fetch users: ${error}')); return { data: [], totalCount: 0 }; } ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f6bddde3-79dc-4bf3-83c3-cd48880b0a2a/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f6bddde3-79dc-4bf3-83c3-cd48880b0a2a?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f6bddde3-79dc-4bf3-83c3-cd48880b0a2a?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f6bddde3-79dc-4bf3-83c3-cd48880b0a2a?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f6bddde3-79dc-4bf3-83c3-cd48880b0a2a) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:04dc2441-5927-4095-8d51-3c123719857a --> [](04dc2441-5927-4095-8d51-3c123719857a) ########## superset-frontend/src/features/groups/utils.ts: ########## @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { SupersetClient, t } from '@superset-ui/core'; +import rison from 'rison'; +import { FormValues } from './types'; + +export const createGroup = async (values: FormValues) => { + await SupersetClient.post({ + endpoint: '/api/v1/security/groups/', + jsonPayload: { ...values, users: values.users.map(user => user.value) }, + }); +}; + +export const updateGroup = async (groupId: number, values: FormValues) => { + await SupersetClient.put({ + endpoint: `/api/v1/security/groups/${groupId}`, + jsonPayload: { ...values, users: values.users.map(user => user.value) }, + }); +}; Review Comment: ### Duplicate User Transformation Logic <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Duplicate transformation logic for user values in both createGroup and updateGroup functions ###### Why this matters Violates DRY principle and increases maintenance burden. If the transformation logic needs to change, it must be updated in multiple places, risking inconsistency. ###### Suggested change ∙ *Feature Preview* Extract the common transformation logic into a separate function: ```typescript const transformGroupPayload = (values: FormValues) => ({ ...values, users: values.users.map(user => user.value) }); export const createGroup = async (values: FormValues) => { await SupersetClient.post({ endpoint: '/api/v1/security/groups/', jsonPayload: transformGroupPayload(values), }); }; ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e11577b1-ea32-4f75-9dd3-cbbbd6eb7014/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e11577b1-ea32-4f75-9dd3-cbbbd6eb7014?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e11577b1-ea32-4f75-9dd3-cbbbd6eb7014?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e11577b1-ea32-4f75-9dd3-cbbbd6eb7014?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e11577b1-ea32-4f75-9dd3-cbbbd6eb7014) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:72bf0948-aab3-44e7-bd6d-d0eaad267da9 --> [](72bf0948-aab3-44e7-bd6d-d0eaad267da9) ########## superset-frontend/src/pages/ActionLog/index.tsx: ########## @@ -0,0 +1,282 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useMemo } from 'react'; +import { t, css } from '@superset-ui/core'; +import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu'; +import { useListViewResource } from 'src/views/CRUD/hooks'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import ListView, { Filters, FilterOperator } from 'src/components/ListView'; +// eslint-disable-next-line no-restricted-imports +import { Typography } from 'antd-v5'; Review Comment: ### Unexplained eslint disable <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Disabled eslint rule without explanation of why the restricted import is necessary ###### Why this matters Without understanding why this import restriction was bypassed, future developers may be confused about whether to follow or ignore the import restriction pattern ###### Suggested change ∙ *Feature Preview* Add a comment explaining why antd-v5 Typography needs to be imported directly instead of through an intermediary module ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1b58f30a-6556-4837-a7d7-a13da89b6790/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1b58f30a-6556-4837-a7d7-a13da89b6790?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1b58f30a-6556-4837-a7d7-a13da89b6790?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1b58f30a-6556-4837-a7d7-a13da89b6790?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1b58f30a-6556-4837-a7d7-a13da89b6790) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:f49a6c2f-72bc-4382-a504-b8c3bcd2f190 --> [](f49a6c2f-72bc-4382-a504-b8c3bcd2f190) ########## superset-frontend/src/pages/ActionLog/index.tsx: ########## @@ -0,0 +1,282 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useMemo } from 'react'; +import { t, css } from '@superset-ui/core'; +import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu'; +import { useListViewResource } from 'src/views/CRUD/hooks'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import ListView, { Filters, FilterOperator } from 'src/components/ListView'; +// eslint-disable-next-line no-restricted-imports +import { Typography } from 'antd-v5'; +import { fetchUserOptions } from 'src/features/groups/utils'; + +export type ActionLogObject = { + user: { + username: string; + }; + action: string; + dttm: string | null; + dashboard_id?: number; + slice_id?: number; + json?: string; + duration_ms?: number; + referrer?: string; +}; + +const PAGE_SIZE = 25; + +function ActionLogList() { + const { addDangerToast, addSuccessToast } = useToasts(); + const initialSort = [{ id: 'dttm', desc: true }]; + const subMenuButtons: SubMenuProps['buttons'] = []; + + const { + state: { + loading, + resourceCount: LogsCount, + resourceCollection: Logs, + bulkSelectEnabled, + }, + fetchData, + refreshData, + toggleBulkSelect, + } = useListViewResource<ActionLogObject>( + 'log', + t('Log'), + addDangerToast, + false, + ); + const filters: Filters = useMemo( + () => [ + { + Header: t('Users'), + key: 'user', + id: 'user', + input: 'select', + operator: FilterOperator.RelationOneMany, + unfilteredLabel: t('All'), + fetchSelects: async (filterValue, page, pageSize) => + fetchUserOptions(filterValue, page, pageSize, addDangerToast), Review Comment: ### Missing User Options Cache <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? User options fetching lacks caching mechanism, potentially causing redundant API calls for the same filter values. ###### Why this matters Without caching, each filter interaction could trigger new API calls even for previously fetched data, increasing server load and degrading user experience. ###### Suggested change ∙ *Feature Preview* Implement a caching mechanism for user options: ```typescript const userOptionsCache = new Map(); const getCachedUserOptions = async (filterValue, page, pageSize, addDangerToast) => { const cacheKey = `${filterValue}-${page}-${pageSize}`; if (userOptionsCache.has(cacheKey)) { return userOptionsCache.get(cacheKey); } const result = await fetchUserOptions(filterValue, page, pageSize, addDangerToast); userOptionsCache.set(cacheKey, result); return result; }; ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b5d133a2-fad3-4b8b-a2aa-80f408bdbf81/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b5d133a2-fad3-4b8b-a2aa-80f408bdbf81?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b5d133a2-fad3-4b8b-a2aa-80f408bdbf81?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b5d133a2-fad3-4b8b-a2aa-80f408bdbf81?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b5d133a2-fad3-4b8b-a2aa-80f408bdbf81) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:d367b5ef-1164-49fc-b2f0-9fe0502ba3d2 --> [](d367b5ef-1164-49fc-b2f0-9fe0502ba3d2) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
