Skip to content

Commit a5a4072

Browse files
waleedlatif1waleedlatif1
committed
fix(org): let the group editor's catalog refresh, and hold activity only across windows
The catalog never went stale and had no read of its own, so a custom block created in another tab was left out of the allowlist a group materializes on its first narrowing. It now reads over its route on the shared stale time, and custom block changes invalidate it. Removing a collapsed recent sends one request at a time.
1 parent fb28e41 commit a5a4072

10 files changed

Lines changed: 152 additions & 49 deletions

File tree

‎apps/sim/hooks/queries/custom-blocks.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
1+
import { type QueryClient, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
22
import { requestJson } from '@/lib/api/client/request'
33
import {
44
type CustomBlock,
@@ -11,6 +11,13 @@ import {
1111
type UpdateCustomBlockBody,
1212
updateCustomBlockContract,
1313
} from '@/lib/api/contracts/custom-blocks'
14+
import { permissionGroupKeys } from '@/hooks/queries/utils/permission-group-keys'
15+
16+
/** A group editor's catalog overlays the organization's custom blocks, so a change here reaches it. */
17+
function invalidateCustomBlockReaders(queryClient: QueryClient) {
18+
queryClient.invalidateQueries({ queryKey: customBlockKeys.lists() })
19+
queryClient.invalidateQueries({ queryKey: permissionGroupKeys.editorCatalogs() })
20+
}
1421

1522
export const CUSTOM_BLOCK_LIST_STALE_TIME = 60 * 1000
1623
/** Short — the usage count is a pre-delete safety check and must stay fresh. */
@@ -79,9 +86,7 @@ export function usePublishCustomBlock() {
7986
const queryClient = useQueryClient()
8087
return useMutation({
8188
mutationFn: (body: PublishCustomBlockBody) => requestJson(publishCustomBlockContract, { body }),
82-
onSettled: () => {
83-
queryClient.invalidateQueries({ queryKey: customBlockKeys.lists() })
84-
},
89+
onSettled: () => invalidateCustomBlockReaders(queryClient),
8590
})
8691
}
8792

@@ -90,18 +95,14 @@ export function useUpdateCustomBlock() {
9095
return useMutation({
9196
mutationFn: ({ id, ...body }: UpdateCustomBlockBody & { id: string }) =>
9297
requestJson(updateCustomBlockContract, { params: { id }, body }),
93-
onSettled: () => {
94-
queryClient.invalidateQueries({ queryKey: customBlockKeys.lists() })
95-
},
98+
onSettled: () => invalidateCustomBlockReaders(queryClient),
9699
})
97100
}
98101

99102
export function useDeleteCustomBlock() {
100103
const queryClient = useQueryClient()
101104
return useMutation({
102105
mutationFn: (id: string) => requestJson(deleteCustomBlockContract, { params: { id } }),
103-
onSettled: () => {
104-
queryClient.invalidateQueries({ queryKey: customBlockKeys.lists() })
105-
},
106+
onSettled: () => invalidateCustomBlockReaders(queryClient),
106107
})
107108
}

‎apps/sim/hooks/queries/organization-activity.test.tsx‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* Failure mode: a held window shows another scope's numbers, when the workspace or organization
5+
* changes rather than the window.
6+
*/
7+
import {
8+
apiClientRequestMock,
9+
apiClientRequestMockFns,
10+
} from '@sim/testing/mocks/api-client-request.mock'
11+
import { nextNavigationMock } from '@sim/testing/mocks/next-navigation.mock'
12+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
13+
import { useOrganizationActivitySummary } from '@/hooks/queries/organization-activity'
14+
import { renderDataHook, unmountDataHooks } from '@/platform/embedded/lib/render-hook.test-support'
15+
16+
vi.mock('@/lib/api/client/request', () => apiClientRequestMock)
17+
vi.mock('next/navigation', () => nextNavigationMock)
18+
19+
const WEEK = { days: 7 } as never
20+
const MONTH = { days: 30 } as never
21+
22+
describe('holding the last window', () => {
23+
beforeEach(() => {
24+
apiClientRequestMockFns.mockRequestJson.mockReset()
25+
let reads = 0
26+
apiClientRequestMockFns.mockRequestJson.mockImplementation(() =>
27+
reads++ === 0 ? Promise.resolve({ read: 'first' }) : new Promise(() => {})
28+
)
29+
})
30+
afterEach(unmountDataHooks)
31+
32+
async function renderFirstThen(next: { window: never; workspaceId?: string }) {
33+
let props: { window: never; workspaceId?: string } = { window: WEEK }
34+
const rendered = await renderDataHook(() =>
35+
useOrganizationActivitySummary('org-1', props.window, {
36+
workspaceId: props.workspaceId,
37+
keepPreviousWindow: true,
38+
})
39+
)
40+
await vi.waitFor(() => expect(rendered.result.current.data).toEqual({ read: 'first' }))
41+
props = next
42+
await rendered.rerender()
43+
return rendered.result.current.data
44+
}
45+
46+
it('keeps showing the last read while a wider window loads', async () => {
47+
expect(await renderFirstThen({ window: MONTH })).toEqual({ read: 'first' })
48+
})
49+
50+
it('shows nothing of another workspace while it loads', async () => {
51+
expect(await renderFirstThen({ window: WEEK, workspaceId: 'ws-2' })).toBeUndefined()
52+
})
53+
})

‎apps/sim/hooks/queries/organization-activity.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { keepPreviousData, useQuery } from '@tanstack/react-query'
1+
import { useQuery } from '@tanstack/react-query'
22
import { requestJson } from '@/lib/api/client/request'
33
import {
44
getOrganizationActivityBreakdownContract,
@@ -46,6 +46,15 @@ interface ActivityQueryOptions {
4646
keepPreviousWindow?: boolean
4747
}
4848

49+
/** The same organization and workspace, so only the window differs between two summary keys. */
50+
function sameSummaryScope(previous: readonly unknown[], next: readonly unknown[]) {
51+
const windowAt = next.length - 2
52+
return (
53+
previous.length === next.length &&
54+
next.every((part, index) => index === windowAt || part === previous[index])
55+
)
56+
}
57+
4958
export function useOrganizationActivitySummary(
5059
organizationId: string,
5160
window: OrganizationUsageWindowKey,
@@ -61,7 +70,15 @@ export function useOrganizationActivitySummary(
6170
}),
6271
enabled: Boolean(organizationId) && enabled,
6372
staleTime: ORGANIZATION_ACTIVITY_STALE_TIME,
64-
placeholderData: keepPreviousWindow ? keepPreviousData : undefined,
73+
placeholderData: (previous, query) =>
74+
keepPreviousWindow &&
75+
query &&
76+
sameSummaryScope(
77+
query.queryKey,
78+
organizationActivityKeys.summary(organizationId, window, workspaceId)
79+
)
80+
? previous
81+
: undefined,
6582
})
6683
}
6784

‎apps/sim/hooks/queries/search-history.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ export function useRemoveSearchHistorySource(organizationId: string, userId: str
5050
const queryClient = useQueryClient()
5151
const queryKey = searchHistoryKeys.list(organizationId, userId)
5252
return useMutation({
53-
mutationFn: (urls: readonly string[]) =>
54-
Promise.all(
55-
urls.map((url) =>
56-
requestJson(removeSearchHistorySourceContract, {
57-
params: { id: organizationId },
58-
body: { url },
59-
})
60-
)
61-
),
53+
mutationFn: async (urls: readonly string[]) => {
54+
// One at a time: the route has no bulk form, and a collapsed chip is a handful of sources.
55+
for (const url of urls) {
56+
await requestJson(removeSearchHistorySourceContract, {
57+
params: { id: organizationId },
58+
body: { url },
59+
})
60+
}
61+
},
6262
onMutate: async (urls) => {
6363
await queryClient.cancelQueries({ queryKey })
6464
const previous = queryClient.getQueryData<SearchHistoryResponse>(queryKey)

‎apps/sim/hooks/queries/utils/permission-group-keys.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
export const PERMISSION_GROUP_MEMBERS_STALE_TIME = 30 * 1000
22
export const PERMISSION_GROUPS_STALE_TIME = 60 * 1000
33
export const PERMISSION_GROUP_EDITOR_CATALOG_STALE_TIME = 5 * 60 * 1000
4-
export const PERMISSION_EDITOR_CATALOG_STALE_TIME = Number.POSITIVE_INFINITY
54

65
export const permissionGroupKeys = {
76
all: ['permissionGroups'] as const,
@@ -15,8 +14,9 @@ export const permissionGroupKeys = {
1514
[...permissionGroupKeys.detail(organizationId, id), 'members'] as const,
1615
userConfig: (workspaceId?: string) =>
1716
[...permissionGroupKeys.all, 'userConfig', workspaceId ?? ''] as const,
17+
editorCatalogs: () => [...permissionGroupKeys.all, 'editorCatalog'] as const,
1818
editorCatalog: (organizationId?: string) =>
19-
[...permissionGroupKeys.all, 'editorCatalog', organizationId ?? ''] as const,
19+
[...permissionGroupKeys.editorCatalogs(), organizationId ?? ''] as const,
2020
orgWorkspaces: (organizationId?: string) =>
2121
[...permissionGroupKeys.all, 'orgWorkspaces', organizationId ?? ''] as const,
2222
}

‎apps/sim/platform/embedded/app/(shell)/settings/_components/sections/access-control/access-control-section.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import {
3333
useAccessControlParams,
3434
useCreatePermissionGroup,
3535
useOrganizationWorkspaces,
36-
usePermissionEditorCatalog,
3736
usePermissionGroups,
3837
} from '@/platform/embedded/lib/data/access-control'
3938
import { plural } from '@/platform/embedded/lib/plural'
@@ -134,7 +133,6 @@ function CreateGroupModal({
134133

135134
export default function AccessControlSection({ organization }: SettingsSectionProps) {
136135
const organizationId = organization.organization.id
137-
usePermissionEditorCatalog(organizationId)
138136
const groups = usePermissionGroups(organizationId)
139137
const workspaces = useOrganizationWorkspaces(organizationId)
140138
const params = useAccessControlParams()

‎apps/sim/platform/embedded/lib/data/access-control/editor.test.tsx‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* Failure mode: the catalog goes stale while an admin edits. A group with an unrestricted allowlist
5+
* materializes it from the catalog on its first narrowing, so a block missing from the catalog is
6+
* silently left out of what gets stored. The catalog has to be able to read again, and creating a
7+
* custom block has to invalidate it.
8+
*/
9+
import { act } from 'react'
10+
import {
11+
apiClientRequestMock,
12+
apiClientRequestMockFns,
13+
} from '@sim/testing/mocks/api-client-request.mock'
14+
import { nextNavigationMock } from '@sim/testing/mocks/next-navigation.mock'
15+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
16+
import { usePublishCustomBlock } from '@/hooks/queries/custom-blocks'
17+
import { permissionGroupKeys } from '@/hooks/queries/utils/permission-group-keys'
18+
import { usePermissionEditorModel } from '@/platform/embedded/lib/data/access-control/editor'
19+
import { renderDataHook, unmountDataHooks } from '@/platform/embedded/lib/render-hook.test-support'
20+
21+
vi.mock('@/lib/api/client/request', () => apiClientRequestMock)
22+
vi.mock('next/navigation', () => nextNavigationMock)
23+
24+
const CATALOG = { blocks: [], providers: [], knowledgeConnectors: [] }
25+
26+
describe('the permission editor catalog', () => {
27+
beforeEach(() => {
28+
apiClientRequestMockFns.mockRequestJson.mockReset()
29+
apiClientRequestMockFns.mockRequestJson.mockResolvedValue(CATALOG)
30+
})
31+
afterEach(unmountDataHooks)
32+
33+
it('reads over its route when no server seed reached it', async () => {
34+
const rendered = await renderDataHook(() => usePermissionEditorModel('org-1'))
35+
await vi.waitFor(() => expect(rendered.result.current.model).not.toBeNull())
36+
expect(
37+
apiClientRequestMockFns.mockRequestJson.mock.calls.some(([contract]) =>
38+
(contract as { path: string }).path.includes('editor-catalog')
39+
)
40+
).toBe(true)
41+
})
42+
43+
it('is invalidated when a custom block is published', async () => {
44+
const rendered = await renderDataHook(() => usePublishCustomBlock())
45+
rendered.queryClient.setQueryData(permissionGroupKeys.editorCatalog('org-1'), CATALOG)
46+
await act(async () => {
47+
await rendered.result.current.mutateAsync({} as never)
48+
})
49+
expect(
50+
rendered.queryClient.getQueryState(permissionGroupKeys.editorCatalog('org-1'))?.isInvalidated
51+
).toBe(true)
52+
})
53+
})

‎apps/sim/platform/embedded/lib/data/access-control/editor.ts‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
'use client'
22

33
import { useMemo } from 'react'
4-
import { skipToken, useQuery } from '@tanstack/react-query'
5-
import { useRouter } from 'next/navigation'
64
import { useQueryState } from 'nuqs'
75
import type { ModelFacts } from '@/lib/permission-groups/model-gate'
6+
import { usePermissionGroupEditorCatalog } from '@/ee/access-control/hooks/permission-groups'
87
import { createEditorCatalog } from '@/ee/access-control/lib/editor'
98
import type { PermissionGroupEditorCatalog } from '@/ee/access-control/lib/editor-catalog-model'
109
import {
@@ -18,10 +17,6 @@ import {
1817
groupTabUrlKeys,
1918
} from '@/ee/access-control/search-params'
2019
import { useBlacklistedProviders } from '@/hooks/queries/allowed-providers'
21-
import {
22-
PERMISSION_EDITOR_CATALOG_STALE_TIME,
23-
permissionGroupKeys,
24-
} from '@/hooks/queries/utils/permission-group-keys'
2520
import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter'
2621
import { useSettingsSearch } from '@/hooks/use-settings-search'
2722
import {
@@ -116,22 +111,9 @@ function toEditorModel(catalog: PermissionGroupEditorCatalog, blacklist: readonl
116111

117112
export type EditorModel = ReturnType<typeof toEditorModel>
118113

119-
/**
120-
* The route's server read seeds this; a failed read leaves it empty and a refresh repeats it. Nothing
121-
* can refetch it, so the section holds an observer to keep it through time spent on the group list.
122-
*/
123-
export function usePermissionEditorCatalog(organizationId: string) {
124-
return useQuery<PermissionGroupEditorCatalog>({
125-
queryKey: permissionGroupKeys.editorCatalog(organizationId),
126-
queryFn: skipToken,
127-
staleTime: PERMISSION_EDITOR_CATALOG_STALE_TIME,
128-
})
129-
}
130-
131114
/** The block and provider catalog the editor resolves access against, or the read's failure. */
132115
export function usePermissionEditorModel(organizationId: string) {
133-
const router = useRouter()
134-
const catalog = usePermissionEditorCatalog(organizationId)
116+
const catalog = usePermissionGroupEditorCatalog(organizationId)
135117
const blacklist = useBlacklistedProviders()
136118
const model = useMemo(
137119
() =>
@@ -145,7 +127,7 @@ export function usePermissionEditorModel(organizationId: string) {
145127
)
146128
return {
147129
model,
148-
retryCatalog: () => router.refresh(),
130+
retryCatalog: () => void catalog.refetch(),
149131
providers: {
150132
pending: blacklist.isPending,
151133
failed: blacklist.isError,

‎apps/sim/platform/embedded/lib/data/access-control/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,5 @@ export type { EditorBlock, EditorModel } from './editor'
1515
export {
1616
useAccessControlParams,
1717
useGroupDetailParams,
18-
usePermissionEditorCatalog,
1918
usePermissionEditorModel,
2019
} from './editor'

‎apps/sim/platform/embedded/lib/data/prefetch-permission-editor.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import 'server-only'
22
import type { SessionPrincipal } from '@sim/auth/principal'
33
import { type DehydratedState, dehydrate } from '@tanstack/react-query'
44
import {
5-
PERMISSION_EDITOR_CATALOG_STALE_TIME,
5+
PERMISSION_GROUP_EDITOR_CATALOG_STALE_TIME,
66
permissionGroupKeys,
77
} from '@/hooks/queries/utils/permission-group-keys'
88
import { makeQueryClient } from '@/hooks/queries/utils/query-client'
@@ -19,7 +19,7 @@ export async function prefetchPermissionEditorCatalog(
1919
const queryClient = makeQueryClient()
2020
await queryClient.prefetchQuery({
2121
queryKey: permissionGroupKeys.editorCatalog(organizationId),
22-
staleTime: PERMISSION_EDITOR_CATALOG_STALE_TIME,
22+
staleTime: PERMISSION_GROUP_EDITOR_CATALOG_STALE_TIME,
2323
queryFn: async () => {
2424
const { readPermissionGroupEditorCatalog } = await import(
2525
'@/lib/permission-groups/application/editor-catalog'

0 commit comments

Comments
 (0)