From 510f007c42cfd6ea51388c1e9c81757ac1bf5329 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Tue, 30 Sep 2025 11:25:57 -0600 Subject: [PATCH] =?UTF-8?q?check=20is=20fetch,=20do=20not=20just=20assume?= =?UTF-8?q?=20=F0=9F=A5=B4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/web/src/controllers/AppAssetNotFound.tsx | 8 +- apps/web/src/controllers/AppNoPageAccess.tsx | 9 +- .../useGetAssetPasswordConfig.test.ts | 142 ++++++++++++++++++ .../useGetAssetPasswordConfig.ts | 15 +- 4 files changed, 155 insertions(+), 19 deletions(-) create mode 100644 apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.test.ts diff --git a/apps/web/src/controllers/AppAssetNotFound.tsx b/apps/web/src/controllers/AppAssetNotFound.tsx index d7622752e..a4c896d21 100644 --- a/apps/web/src/controllers/AppAssetNotFound.tsx +++ b/apps/web/src/controllers/AppAssetNotFound.tsx @@ -1,17 +1,15 @@ import type { AssetType } from '@buster/server-shared/assets'; import type { ResponseMessageFileType } from '@buster/server-shared/chats'; import { Link } from '@tanstack/react-router'; -import React from 'react'; import { BusterLogo } from '@/assets/svg/BusterLogo'; import { Button } from '@/components/ui/buttons'; import { Title } from '@/components/ui/typography'; import { useMount } from '@/hooks/useMount'; -import { AppNoPageAccess } from './AppNoPageAccess'; export const AppAssetNotFound: React.FC<{ assetId: string; type: AssetType | ResponseMessageFileType; -}> = React.memo(({ type, assetId }) => { +}> = ({ type, assetId }) => { useMount(() => { console.info('AppAssetNotFound for asset:', assetId, 'and type:', type); }); @@ -32,6 +30,4 @@ export const AppAssetNotFound: React.FC<{ ); -}); - -AppNoPageAccess.displayName = 'AppNoPageAccess'; +}; diff --git a/apps/web/src/controllers/AppNoPageAccess.tsx b/apps/web/src/controllers/AppNoPageAccess.tsx index cef0c2a0a..7332aac72 100644 --- a/apps/web/src/controllers/AppNoPageAccess.tsx +++ b/apps/web/src/controllers/AppNoPageAccess.tsx @@ -1,7 +1,8 @@ import type { AssetType } from '@buster/server-shared/assets'; import type { ResponseMessageFileType } from '@buster/server-shared/chats'; import { Link, type LinkProps, useLocation } from '@tanstack/react-router'; -import React, { useMemo } from 'react'; +import type React from 'react'; +import { useMemo } from 'react'; import { BusterLogo } from '@/assets/svg/BusterLogo'; import { Button } from '@/components/ui/buttons'; import { Title } from '@/components/ui/typography'; @@ -19,7 +20,7 @@ const translationRecord: Record = { export const AppNoPageAccess: React.FC<{ assetId: string; type: AssetType | ResponseMessageFileType; -}> = React.memo(({ type }) => { +}> = ({ type }) => { const isAnonymousUser = useIsAnonymousSupabaseUser(); const location = useLocation(); @@ -66,6 +67,4 @@ export const AppNoPageAccess: React.FC<{ ); -}); - -AppNoPageAccess.displayName = 'AppNoPageAccess'; +}; diff --git a/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.test.ts b/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.test.ts new file mode 100644 index 000000000..983630ae4 --- /dev/null +++ b/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it } from 'vitest'; +import type { ApiError } from '@/api/errors'; +import { chatQueryKeys } from '@/api/query_keys/chat'; +import { collectionQueryKeys } from '@/api/query_keys/collection'; +import { dashboardQueryKeys } from '@/api/query_keys/dashboard'; +import { metricsQueryKeys } from '@/api/query_keys/metric'; +import { reportsQueryKeys } from '@/api/query_keys/reports'; +import { getAssetAccess, getSelectedQuery } from './useGetAssetPasswordConfig'; + +describe('getAssetAccess', () => { + it('should return password required state when error status is 418', () => { + const error = { status: 418 } as ApiError; + const result = getAssetAccess(error, true, ['test'], false); + + expect(result).toEqual({ + hasAccess: false, + passwordRequired: true, + isPublic: true, + isDeleted: false, + isFetched: true, + }); + }); + + it('should return deleted state when error status is 410', () => { + const error = { status: 410 } as ApiError; + const result = getAssetAccess(error, true, ['test'], false); + + expect(result).toEqual({ + hasAccess: false, + passwordRequired: false, + isPublic: false, + isDeleted: true, + isFetched: true, + }); + }); + + it('should return no access state when error status is 403', () => { + const error = { status: 403 } as ApiError; + const result = getAssetAccess(error, true, ['test'], false); + + expect(result).toEqual({ + hasAccess: false, + passwordRequired: false, + isPublic: false, + isDeleted: false, + isFetched: true, + }); + }); + + it('should return no access state when error has any other numeric status', () => { + const error = { status: 500 } as ApiError; + const result = getAssetAccess(error, true, ['test'], false); + + expect(result).toEqual({ + hasAccess: false, + passwordRequired: false, + isPublic: false, + isDeleted: false, + isFetched: true, + }); + }); + + it('should return has access state when no data and is fetched with no error', () => { + const result = getAssetAccess(null, true, ['test'], false); + + expect(result).toEqual({ + hasAccess: true, + passwordRequired: false, + isPublic: false, + isDeleted: false, + isFetched: true, + }); + }); + + it('should return has access state when no error and has data', () => { + const result = getAssetAccess(null, true, ['test'], true); + + expect(result).toEqual({ + hasAccess: true, + passwordRequired: false, + isPublic: false, + isDeleted: false, + isFetched: true, + }); + }); + + it('should return has access state when not fetched and no error', () => { + const result = getAssetAccess(null, false, ['test'], false); + + expect(result).toEqual({ + hasAccess: true, + passwordRequired: false, + isPublic: false, + isDeleted: false, + isFetched: false, + }); + }); +}); + +describe('getSelectedQuery', () => { + it('should return metric query for metric_file type', () => { + const result = getSelectedQuery('metric_file', 'test-id', 'LATEST'); + const expected = metricsQueryKeys.metricsGetMetric('test-id', 'LATEST'); + + expect(result).toEqual(expected); + }); + + it('should return dashboard query for dashboard_file type', () => { + const result = getSelectedQuery('dashboard_file', 'test-id', 123); + const expected = dashboardQueryKeys.dashboardGetDashboard('test-id', 123); + + expect(result).toEqual(expected); + }); + + it('should return report query for report_file type', () => { + const result = getSelectedQuery('report_file', 'test-id', 'LATEST'); + const expected = reportsQueryKeys.reportsGetReport('test-id', 'LATEST'); + + expect(result).toEqual(expected); + }); + + it('should return collection query for collection type', () => { + const result = getSelectedQuery('collection', 'test-id', 'LATEST'); + const expected = collectionQueryKeys.collectionsGetCollection('test-id'); + + expect(result).toEqual(expected); + }); + + it('should return chat query for reasoning type', () => { + const result = getSelectedQuery('reasoning', 'test-id', 'LATEST'); + const expected = chatQueryKeys.chatsGetChat('test-id'); + + expect(result).toEqual(expected); + }); + + it('should return chat query as default for chat type', () => { + const result = getSelectedQuery('chat', 'test-id', 'LATEST'); + const expected = chatQueryKeys.chatsGetChat('test-id'); + + expect(result).toEqual(expected); + }); +}); diff --git a/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.ts b/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.ts index deda1ef96..3151ed8e4 100644 --- a/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.ts +++ b/apps/web/src/layouts/AppAssetCheckLayout/useGetAssetPasswordConfig.ts @@ -17,11 +17,11 @@ interface AssetAccess { isFetched: boolean; } -const getAssetAccess = ( +export const getAssetAccess = ( error: ApiError | null, isFetched: boolean, selectedQuery: QueryKey, - hasData: boolean + _hasData: boolean ): AssetAccess => { if (error) { console.error('Error in getAssetAccess', error, isFetched, selectedQuery); @@ -60,13 +60,13 @@ const getAssetAccess = ( }; } - if (typeof error?.status === 'number' || !hasData) { + if (typeof error?.status === 'number') { return { hasAccess: false, passwordRequired: false, isPublic: false, isDeleted: false, - isFetched: true, + isFetched: isFetched, }; } @@ -91,11 +91,10 @@ export const useGetAssetPasswordConfig = ( [type, assetId, chosenVersionNumber] ); - const { error, isFetched, data, ...rest } = useQuery({ + const { error, isFetched, data } = useQuery({ queryKey: selectedQuery.queryKey, enabled: true, - select: useCallback((v: unknown) => !!v, []), - notifyOnChangeProps: ['error', 'isFetched', 'data'], + select: (v: unknown) => !!v, retry: false, initialData: false, }); @@ -103,7 +102,7 @@ export const useGetAssetPasswordConfig = ( return getAssetAccess(error, isFetched, selectedQuery.queryKey, !!data); }; -const getSelectedQuery = ( +export const getSelectedQuery = ( type: AssetType | ResponseMessageFileType, assetId: string, chosenVersionNumber: number | 'LATEST'