From 1671c2d8fb9bf19cc68ad9cc8716796d89f21d28 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Fri, 10 Jan 2025 14:41:33 -0700 Subject: [PATCH] optimistic updates for assigning permission groups --- .../datasets/permissions/queryRequests.ts | 82 +++++--- .../busterv2/datasets/permissions/requests.ts | 7 +- web/src/api/busterv2/index.ts | 1 + .../permissions/HeaderExplanation.tsx | 22 +++ .../permissions/PermissionPermissionGroup.tsx | 7 - ...verviewSearch.tsx => PermissionSearch.tsx} | 13 +- .../permissions/PermissionsAppContainer.tsx | 5 +- .../_PermissionOverview/HeaderExplanation.tsx | 17 -- .../PermissionListUser.tsx | 10 - .../PermissionListUserContainer.tsx | 89 +++++---- .../PermissionOverview.tsx | 9 +- ...PermissionListPermissionGroupContainer.tsx | 178 ++++++++++++++++++ .../PermissionPermissionGroup.tsx | 78 ++++++++ .../_PermissionPermissionGroup/index.ts | 1 + .../_ThreadItemsContainer.tsx | 2 - .../list/BusterInfiniteList/index.ts | 1 + 16 files changed, 407 insertions(+), 115 deletions(-) create mode 100644 web/src/app/app/datasets/[datasetId]/permissions/HeaderExplanation.tsx delete mode 100644 web/src/app/app/datasets/[datasetId]/permissions/PermissionPermissionGroup.tsx rename web/src/app/app/datasets/[datasetId]/permissions/{_PermissionOverview/PermissionOverviewSearch.tsx => PermissionSearch.tsx} (58%) delete mode 100644 web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/HeaderExplanation.tsx delete mode 100644 web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUser.tsx create mode 100644 web/src/app/app/datasets/[datasetId]/permissions/_PermissionPermissionGroup/PermissionListPermissionGroupContainer.tsx create mode 100644 web/src/app/app/datasets/[datasetId]/permissions/_PermissionPermissionGroup/PermissionPermissionGroup.tsx create mode 100644 web/src/app/app/datasets/[datasetId]/permissions/_PermissionPermissionGroup/index.ts diff --git a/web/src/api/busterv2/datasets/permissions/queryRequests.ts b/web/src/api/busterv2/datasets/permissions/queryRequests.ts index 1e6b8bddf..cece85b5c 100644 --- a/web/src/api/busterv2/datasets/permissions/queryRequests.ts +++ b/web/src/api/busterv2/datasets/permissions/queryRequests.ts @@ -9,8 +9,9 @@ import { updatePermissionUsers } from './requests'; import { useMemoizedFn } from 'ahooks'; -import { QueryClient } from '@tanstack/react-query'; +import { QueryClient, useQueryClient } from '@tanstack/react-query'; import { getDatasetPermissionsOverview_server } from './serverRequests'; +import { ListPermissionUsersResponse } from './responseInterfaces'; export const useGetDatasetPermissionsOverview = (dataset_id: string) => { const queryFn = useMemoizedFn(() => getDatasetPermissionsOverview({ dataset_id })); @@ -38,7 +39,8 @@ export const useListPermissionGroups = (dataset_id: string) => { return useCreateReactQuery({ queryKey: ['list_permission_groups', dataset_id], - queryFn + queryFn, + staleTime: 1000 * 5 // 5 seconds }); }; @@ -61,43 +63,77 @@ export const useListPermissionUsers = (dataset_id: string) => { }; export const useUpdatePermissionGroups = (dataset_id: string) => { - const mutationFn = useMemoizedFn((groups: { id: string; assigned: boolean }[]) => - updatePermissionGroups({ dataset_id, groups }) - ); - const onSuccess = useMemoizedFn(() => { - // queryClient.invalidateQueries({ queryKey: ['dataset_permissions_overview', dataset_id] }); + const queryClient = useQueryClient(); + const mutationFn = useMemoizedFn((groups: { id: string; assigned: boolean }[]) => { + const keyedChanges: Record = {}; + groups.forEach(({ id, assigned }) => { + keyedChanges[id] = { id, assigned }; + }); + queryClient.setQueryData( + ['list_permission_groups', dataset_id], + (oldData: ListPermissionUsersResponse[]) => { + return oldData?.map((group) => { + const updatedGroup = keyedChanges[group.id]; + if (updatedGroup) return { ...group, assigned: updatedGroup.assigned }; + return group; + }); + } + ); + + return updatePermissionGroups({ dataset_id, groups }); }); return useCreateReactMutation({ - mutationFn, - onSuccess + mutationFn }); }; export const useUpdateDatasetGroups = (dataset_id: string) => { - const mutationFn = useMemoizedFn((groups: { id: string; assigned: boolean }[]) => - updateDatasetGroups({ dataset_id, groups }) - ); - const onSuccess = useMemoizedFn(() => { - // queryClient.invalidateQueries({ queryKey: ['dataset_permissions_overview', dataset_id] }); + const queryClient = useQueryClient(); + const mutationFn = useMemoizedFn((groups: { id: string; assigned: boolean }[]) => { + const keyedChanges: Record = {}; + groups.forEach(({ id, assigned }) => { + keyedChanges[id] = { id, assigned }; + }); + queryClient.setQueryData( + ['list_dataset_groups', dataset_id], + (oldData: ListPermissionUsersResponse[]) => { + return oldData?.map((group) => { + const updatedGroup = keyedChanges[group.id]; + if (updatedGroup) return { ...group, assigned: updatedGroup.assigned }; + return group; + }); + } + ); + return updateDatasetGroups({ dataset_id, groups }); }); return useCreateReactMutation({ - mutationFn, - onSuccess + mutationFn }); }; export const useUpdatePermissionUsers = (dataset_id: string) => { - const mutationFn = useMemoizedFn((users: { id: string; assigned: boolean }[]) => - updatePermissionUsers({ dataset_id, users }) - ); - const onSuccess = useMemoizedFn(() => { - // queryClient.invalidateQueries({ queryKey: ['dataset_permissions_overview', dataset_id] }); + const queryClient = useQueryClient(); + const mutationFn = useMemoizedFn((users: { id: string; assigned: boolean }[]) => { + const keyedChanges: Record = {}; + users.forEach(({ id, assigned }) => { + keyedChanges[id] = { id, assigned }; + }); + queryClient.setQueryData( + ['list_permission_users', dataset_id], + (oldData: ListPermissionUsersResponse[]) => { + return oldData?.map((user) => { + const updatedUser = keyedChanges[user.id]; + if (updatedUser) return { ...user, assigned: updatedUser.assigned }; + return user; + }); + } + ); + return updatePermissionUsers({ dataset_id, users }); }); return useCreateReactMutation({ - mutationFn, - onSuccess + mutationFn }); }; diff --git a/web/src/api/busterv2/datasets/permissions/requests.ts b/web/src/api/busterv2/datasets/permissions/requests.ts index decae6a22..46722542e 100644 --- a/web/src/api/busterv2/datasets/permissions/requests.ts +++ b/web/src/api/busterv2/datasets/permissions/requests.ts @@ -50,7 +50,12 @@ export const updatePermissionGroups = async ({ }: { dataset_id: string; groups: { id: string; assigned: boolean }[]; -}): Promise => { +}): Promise< + { + id: string; + assigned: boolean; + }[] +> => { return await mainApi.put(`/datasets/${dataset_id}/permission_groups`, groups); }; diff --git a/web/src/api/busterv2/index.ts b/web/src/api/busterv2/index.ts index 2223d2aea..143c7e4f3 100644 --- a/web/src/api/busterv2/index.ts +++ b/web/src/api/busterv2/index.ts @@ -7,3 +7,4 @@ export * from './search'; export * from './assets'; export * from './api_keys'; export * from './sql'; +export * from './datasets'; diff --git a/web/src/app/app/datasets/[datasetId]/permissions/HeaderExplanation.tsx b/web/src/app/app/datasets/[datasetId]/permissions/HeaderExplanation.tsx new file mode 100644 index 000000000..e65de6522 --- /dev/null +++ b/web/src/app/app/datasets/[datasetId]/permissions/HeaderExplanation.tsx @@ -0,0 +1,22 @@ +import React from 'react'; +import { Title, Text } from '@/components'; + +export const HeaderExplanation: React.FC<{ + className?: string; + title?: string; + description?: string; +}> = React.memo( + ({ + className = '', + title = 'Access & lineage', + description = 'View which users can query this dataset. Lineage is provided to show where each user’s access originates from.' + }) => { + return ( +
+ {title} + {description} +
+ ); + } +); +HeaderExplanation.displayName = 'HeaderExplanation'; diff --git a/web/src/app/app/datasets/[datasetId]/permissions/PermissionPermissionGroup.tsx b/web/src/app/app/datasets/[datasetId]/permissions/PermissionPermissionGroup.tsx deleted file mode 100644 index 41e96c8c3..000000000 --- a/web/src/app/app/datasets/[datasetId]/permissions/PermissionPermissionGroup.tsx +++ /dev/null @@ -1,7 +0,0 @@ -import React from 'react'; - -export const PermissionPermissionGroup: React.FC<{}> = React.memo(({}) => { - return
asdf
; -}); - -PermissionPermissionGroup.displayName = 'PermissionPermissionGroup'; diff --git a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverviewSearch.tsx b/web/src/app/app/datasets/[datasetId]/permissions/PermissionSearch.tsx similarity index 58% rename from web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverviewSearch.tsx rename to web/src/app/app/datasets/[datasetId]/permissions/PermissionSearch.tsx index 8dc796a48..1bdf288d1 100644 --- a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverviewSearch.tsx +++ b/web/src/app/app/datasets/[datasetId]/permissions/PermissionSearch.tsx @@ -3,20 +3,21 @@ import { Input } from 'antd'; import { AppMaterialIcons } from '@/components'; import { useMemoizedFn } from 'ahooks'; -export const PermissionOverviewSearch: React.FC<{ +export const PermissionSearch: React.FC<{ className?: string; searchText: string; setSearchText: (text: string) => void; -}> = ({ className = '', searchText, setSearchText }) => { + placeholder?: string; +}> = ({ className = '', searchText, setSearchText, placeholder = 'Search by name or email' }) => { const onChange = useMemoizedFn((e: React.ChangeEvent) => { setSearchText(e.target.value); }); return ( -
+
); }; -PermissionOverviewSearch.displayName = 'PermissionOverviewSearch'; +PermissionSearch.displayName = 'PermissionOverviewSearch'; diff --git a/web/src/app/app/datasets/[datasetId]/permissions/PermissionsAppContainer.tsx b/web/src/app/app/datasets/[datasetId]/permissions/PermissionsAppContainer.tsx index f3bc816fd..c16e0147d 100644 --- a/web/src/app/app/datasets/[datasetId]/permissions/PermissionsAppContainer.tsx +++ b/web/src/app/app/datasets/[datasetId]/permissions/PermissionsAppContainer.tsx @@ -6,7 +6,7 @@ import { AnimatePresence, motion } from 'framer-motion'; import { PermissionApps } from './config'; import { PermissionDatasetGroups } from './PermissionDatasetGroups'; import { PermissionOverview } from './_PermissionOverview'; -import { PermissionPermissionGroup } from './PermissionPermissionGroup'; +import { PermissionPermissionGroup } from './_PermissionPermissionGroup/PermissionPermissionGroup'; import { PermissionUsers } from './PermissionUsers'; const selectedAppComponent: Record> = { @@ -26,7 +26,8 @@ export const PermissionsAppContainer: React.FC<{ datasetId: string }> = React.me return { initial: { opacity: 0 }, animate: { opacity: 1 }, - exit: { opacity: 0 } + exit: { opacity: 0 }, + transition: { duration: 0.125 } }; }, []); diff --git a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/HeaderExplanation.tsx b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/HeaderExplanation.tsx deleted file mode 100644 index f8b093331..000000000 --- a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/HeaderExplanation.tsx +++ /dev/null @@ -1,17 +0,0 @@ -import React from 'react'; -import { Title, Text } from '@/components'; - -export const HeaderExplanation: React.FC<{ className?: string }> = React.memo( - ({ className = '' }) => { - return ( -
- Access & lineage - - {`View which users can query this dataset. Lineage is provided to show where each user’s - access originates from.`} - -
- ); - } -); -HeaderExplanation.displayName = 'HeaderExplanation'; diff --git a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUser.tsx b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUser.tsx deleted file mode 100644 index 49f1353ab..000000000 --- a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUser.tsx +++ /dev/null @@ -1,10 +0,0 @@ -import React from 'react'; -import { DatasetPermissionOverviewUser } from '@/api/busterv2/datasets'; - -export const PermissionListUser: React.FC<{ - className?: string; - user: DatasetPermissionOverviewUser; -}> = React.memo(({ className = '', user }) => { - return
swag
; -}); -PermissionListUser.displayName = 'PermissionListUser'; diff --git a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUserContainer.tsx b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUserContainer.tsx index 8446391b6..932ca27f2 100644 --- a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUserContainer.tsx +++ b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionListUserContainer.tsx @@ -15,26 +15,29 @@ export const PermissionListUserContainer: React.FC<{ const numberOfUsers = filteredUsers.length; - const columns: BusterListColumn[] = [ - { - title: 'Name', - dataIndex: 'name', - width: 290, - render: (_: string, user: DatasetPermissionOverviewUser) => { - return ; + const columns: BusterListColumn[] = useMemo( + () => [ + { + title: 'Name', + dataIndex: 'name', + width: 290, + render: (_: string, user: DatasetPermissionOverviewUser) => { + return ; + } + }, + { + title: 'Lineage', + dataIndex: 'lineage', + render: ( + lineage: DatasetPermissionOverviewUser['lineage'], + user: DatasetPermissionOverviewUser + ) => { + return ; + } } - }, - { - title: 'Lineage', - dataIndex: 'lineage', - render: ( - lineage: DatasetPermissionOverviewUser['lineage'], - user: DatasetPermissionOverviewUser - ) => { - return ; - } - } - ]; + ], + [] + ); const { cannotQueryUsers, canQueryUsers } = useMemo(() => { const result: { @@ -66,28 +69,32 @@ export const PermissionListUserContainer: React.FC<{ return result; }, [filteredUsers]); - const rows = [ - { - id: 'header-assigned', - data: {}, - hidden: canQueryUsers.length === 0, - rowSection: { - title: 'Assigned', - secondaryTitle: numberOfUsers.toString() - } - }, - ...canQueryUsers, - { - id: 'header-not-assigned', - data: {}, - hidden: cannotQueryUsers.length === 0, - rowSection: { - title: 'Not Assigned', - secondaryTitle: cannotQueryUsers.length.toString() - } - }, - ...cannotQueryUsers - ].filter((row) => !(row as any).hidden); + const rows = useMemo( + () => + [ + { + id: 'header-assigned', + data: {}, + hidden: canQueryUsers.length === 0, + rowSection: { + title: 'Assigned', + secondaryTitle: numberOfUsers.toString() + } + }, + ...canQueryUsers, + { + id: 'header-not-assigned', + data: {}, + hidden: cannotQueryUsers.length === 0, + rowSection: { + title: 'Not Assigned', + secondaryTitle: cannotQueryUsers.length.toString() + } + }, + ...cannotQueryUsers + ].filter((row) => !(row as any).hidden), + [canQueryUsers, cannotQueryUsers, numberOfUsers] + ); return ( <> diff --git a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverview.tsx b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverview.tsx index a880a2aa6..6237bacdb 100644 --- a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverview.tsx +++ b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionOverview/PermissionOverview.tsx @@ -3,13 +3,10 @@ import { useGetDatasetPermissionsOverview } from '@/api/busterv2/datasets'; import React, { useMemo, useState, useTransition, useEffect } from 'react'; -import { Title, Text } from '@/components/text'; -import { Input } from 'antd'; import { useMemoizedFn } from 'ahooks'; -import { AppMaterialIcons } from '@/components'; import { useDebounceFn } from 'ahooks'; -import { HeaderExplanation } from './HeaderExplanation'; -import { PermissionOverviewSearch } from './PermissionOverviewSearch'; +import { HeaderExplanation } from '../HeaderExplanation'; +import { PermissionSearch } from '../PermissionSearch'; import { PermissionListUserContainer } from './PermissionListUserContainer'; export const PermissionOverview: React.FC<{ @@ -65,7 +62,7 @@ export const PermissionOverview: React.FC<{ <>
- + {/* You can use filteredUsers here to display the results */}
diff --git a/web/src/app/app/datasets/[datasetId]/permissions/_PermissionPermissionGroup/PermissionListPermissionGroupContainer.tsx b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionPermissionGroup/PermissionListPermissionGroupContainer.tsx new file mode 100644 index 000000000..7d7f7ae39 --- /dev/null +++ b/web/src/app/app/datasets/[datasetId]/permissions/_PermissionPermissionGroup/PermissionListPermissionGroupContainer.tsx @@ -0,0 +1,178 @@ +import { ListPermissionGroupsResponse, useUpdatePermissionGroups } from '@/api/busterv2/datasets'; +import { BusterListColumn, BusterListRowItem } from '@/components/list'; +import { BusterInfiniteList } from '@/components/list/BusterInfiniteList'; +import { useMemoizedFn } from 'ahooks'; +import { Select } from 'antd'; +import { createStyles } from 'antd-style'; +import React, { useMemo, useState } from 'react'; + +export const PermissionListPermissionGroupContainer: React.FC<{ + filteredPermissionGroups: ListPermissionGroupsResponse[]; + datasetId: string; +}> = React.memo(({ filteredPermissionGroups, datasetId }) => { + const { styles, cx } = useStyles(); + const { mutateAsync: updatePermissionGroups } = useUpdatePermissionGroups(datasetId); + const [selectedRowKeys, setSelectedRowKeys] = useState([]); + + const numberOfPermissionGroups = filteredPermissionGroups.length; + + const onSelectAssigned = useMemoizedFn(async (params: { id: string; assigned: boolean }) => { + updatePermissionGroups([params]); + }); + + const columns: BusterListColumn[] = useMemo( + () => [ + { + title: 'Name', + dataIndex: 'name', + width: 270, + render: (name: string) => { + return ; + } + }, + { + title: 'Assigned', + dataIndex: 'assigned', + render: (assigned: boolean, permissionGroup: ListPermissionGroupsResponse) => { + return ( +
+ +
+ ); + } + } + ], + [] + ); + + const { cannotQueryPermissionGroups, canQueryPermissionGroups } = useMemo(() => { + const result: { + cannotQueryPermissionGroups: BusterListRowItem[]; + canQueryPermissionGroups: BusterListRowItem[]; + } = filteredPermissionGroups.reduce<{ + cannotQueryPermissionGroups: BusterListRowItem[]; + canQueryPermissionGroups: BusterListRowItem[]; + }>( + (acc, permissionGroup) => { + if (permissionGroup.assigned) { + acc.canQueryPermissionGroups.push({ + id: permissionGroup.id, + data: permissionGroup + }); + } else { + acc.cannotQueryPermissionGroups.push({ + id: permissionGroup.id, + data: permissionGroup + }); + } + return acc; + }, + { + cannotQueryPermissionGroups: [] as BusterListRowItem[], + canQueryPermissionGroups: [] as BusterListRowItem[] + } + ); + return result; + }, [filteredPermissionGroups]); + + const rows = useMemo( + () => + [ + { + id: 'header-assigned', + data: {}, + hidden: canQueryPermissionGroups.length === 0, + rowSection: { + title: 'Assigned', + secondaryTitle: canQueryPermissionGroups.length.toString() + } + }, + ...canQueryPermissionGroups, + { + id: 'header-not-assigned', + data: {}, + hidden: cannotQueryPermissionGroups.length === 0, + rowSection: { + title: 'Not Assigned', + secondaryTitle: cannotQueryPermissionGroups.length.toString() + } + }, + ...cannotQueryPermissionGroups + ].filter((row) => !(row as any).hidden), + [canQueryPermissionGroups, cannotQueryPermissionGroups, numberOfPermissionGroups] + ); + + return ( + <> +
+ { + console.log('scrolled'); + }} + emptyState={
No teams found
} + /> +
+ + ); +}); + +PermissionListPermissionGroupContainer.displayName = 'PermissionListTeamContainer'; + +const useStyles = createStyles(({ css, token }) => ({ + container: css` + border: 0.5px solid ${token.colorBorder}; + border-radius: ${token.borderRadius}px; + overflow: hidden; + ` +})); + +const PermissionGroupInfoCell = React.memo(({ name }: { name: string }) => { + return
{name}
; +}); + +const options = [ + { + label: 'Assigned', + value: true + }, + { + label: 'Not Assigned', + value: false + } +]; + +const PermissionGroupAssignedCell = React.memo( + ({ + id, + assigned, + onSelect + }: { + id: string; + assigned: boolean; + onSelect: (value: { id: string; assigned: boolean }) => void; + }) => { + return ( +