Merge pull request #1218 from buster-so/big-nate-bus-2005-asset-check-should-wait-for-fetch-before-showing-errors

check is fetch, do not just assume 🥴

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<{
       </div>
     </div>
   );
-});
+};
-
-AppNoPageAccess.displayName = 'AppNoPageAccess';

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<AssetType | ResponseMessageFileType, string> = {
 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<{
       </div>
     </div>
   );
-});
+};
-
-AppNoPageAccess.displayName = 'AppNoPageAccess';

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);
+  });
+});

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, []),
+    select: (v: unknown) => !!v,
-    notifyOnChangeProps: ['error', 'isFetched', 'data'],
     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'