add a version number to reports

This commit is contained in:
Nate Kelley 2025-08-04 21:37:23 -06:00
parent 4a6c36c4b0
commit 124523ade9
No known key found for this signature in database
GPG Key ID: FD90372AB8D98B4F
9 changed files with 449 additions and 12 deletions

View File

@ -65,7 +65,7 @@ export const prefetchGetReportsList = async (
* Hook to get an individual report by ID
*/
export const useGetReport = <T = GetReportIndividualResponse>(
reportId: string | undefined,
{ reportId, versionNumber }: { reportId: string | undefined; versionNumber?: number },
options?: Omit<
UseQueryOptions<GetReportIndividualResponse, RustApiError, T>,
'queryKey' | 'queryFn'
@ -76,7 +76,7 @@ export const useGetReport = <T = GetReportIndividualResponse>(
});
return useQuery({
...queryKeys.reportsGetReport(reportId!),
...queryKeys.reportsGetReport(reportId!, versionNumber),
queryFn,
enabled: !!reportId,
select: options?.select,

View File

@ -0,0 +1,61 @@
import last from 'lodash/last';
import { useMemo } from 'react';
import { useGetReport } from '@/api/buster_rest/reports';
import { useChatLayoutContextSelector } from '@/layouts/ChatLayout';
import { canEdit } from '@/lib/share';
export const useIsReportReadOnly = ({
reportId,
readOnly
}: {
reportId: string;
readOnly?: boolean;
}) => {
const isVersionHistoryMode = useChatLayoutContextSelector((x) => x.isVersionHistoryMode);
const reportVersionNumber = useChatLayoutContextSelector((x) => x.reportVersionNumber);
const {
data: reportData,
isFetched,
isError
} = useGetReport(
{ reportId, versionNumber: reportVersionNumber },
{
select: (x) => ({
permission: x.permission,
versions: x.versions,
version_number: x.version_number
})
}
);
const isViewingOldVersion = useMemo(() => {
if (!reportVersionNumber) return false;
if (reportVersionNumber !== last(reportData?.versions)?.version_number) return true;
return false;
}, [reportVersionNumber, reportData]);
const isReadOnly = useMemo(() => {
if (readOnly) return true;
if (isError) return true;
if (!isFetched) return true;
if (!canEdit(reportData?.permission)) return true;
if (isVersionHistoryMode) return true;
if (isViewingOldVersion) return true;
return false;
}, [
isError,
isFetched,
reportData,
reportVersionNumber,
isVersionHistoryMode,
isViewingOldVersion
]);
return {
isFetched,
isError,
isVersionHistoryMode,
isReadOnly,
isViewingOldVersion
};
};

View File

@ -0,0 +1,71 @@
import React from 'react';
import { useGetReport } from '@/api/buster_rest/reports';
import type { SegmentedItem } from '@/components/ui/segmented';
import { AppSegmented } from '@/components/ui/segmented';
import { Text } from '@/components/ui/typography';
import { useIsReportReadOnly } from '@/context/Reports/useIsReportReadOnly';
import { useChatLayoutContextSelector } from '../../ChatLayoutContext';
import type { FileView } from '../../ChatLayoutContext/useLayoutConfig';
import type { FileContainerSegmentProps } from './interfaces';
import { assetParamsToRoute } from '@/lib/assets';
export const ReportContainerHeaderSegment: React.FC<FileContainerSegmentProps> = (props) => {
const { selectedFileId, overrideOldVersionMessage } = props;
const { isViewingOldVersion, isFetched, isError } = useIsReportReadOnly({
reportId: selectedFileId || ''
});
if (!isFetched || isError) return null;
if (isViewingOldVersion && !overrideOldVersionMessage) {
return <ReportOldVersion />;
}
return <ReportSegments {...props} />;
};
ReportContainerHeaderSegment.displayName = 'ReportContainerHeaderSegment';
const ReportSegments: React.FC<FileContainerSegmentProps> = React.memo(
({ selectedFileView, chatId, isVersionHistoryMode }) => {
const reportId = useChatLayoutContextSelector((x) => x.reportId) || '';
const reportVersionNumber = useChatLayoutContextSelector((x) => x.reportVersionNumber);
const { error } = useGetReport({ reportId: reportId });
const segmentOptions: SegmentedItem<FileView>[] = React.useMemo(() => {
return [
{
label: 'Report',
value: 'report',
link: assetParamsToRoute({
chatId,
assetId: reportId,
type: 'report'
})
},
{
label: 'Files',
value: 'file',
link: assetParamsToRoute({
chatId,
assetId: reportId,
type: 'report',
page: 'files'
})
}
];
}, [chatId, error, reportId, reportVersionNumber, isVersionHistoryMode]);
return <AppSegmented type="button" options={segmentOptions} value={selectedFileView} />;
}
);
ReportSegments.displayName = 'ReportSegments';
const ReportOldVersion: React.FC = () => {
return (
<Text truncate variant={'secondary'}>
You are viewing an old version of this report
</Text>
);
};

View File

@ -9,7 +9,7 @@ import { createMetricRoute, type MetricRouteParams } from './createMetricRoute';
import { createDashboardRoute, type DashboardRouteParams } from './createDashboardRoute';
import { createReasoningRoute } from './createReasoningRoute';
import { createDatasetRoute } from './createDatasetRoute';
import { createReportRoute } from './createReportRoute';
import { createReportRoute, type ReportRouteParams } from './createReportRoute';
type UnionOfFileTypes = FileType | ReasoningFileType | ReasoingMessage_ThoughtFileType;
@ -21,12 +21,13 @@ type OtherRouteParams = {
versionNumber?: number; //will first try and use metricVersionNumber assuming it is a metric, then dashboardVersionNumber assuming it is a dashboard, then versionNumber
metricVersionNumber?: number; //if this is provided, it will be used instead of versionNumber
dashboardVersionNumber?: number; //if this is provided, it will be used instead of versionNumber
reportVersionNumber?: number; //if this is provided, it will be used instead of versionNumber
page?: undefined;
secondaryView?: undefined | null | string;
type: UnionOfFileTypes;
};
type BaseParams = MetricRouteParams | DashboardRouteParams | OtherRouteParams;
type BaseParams = MetricRouteParams | DashboardRouteParams | OtherRouteParams | ReportRouteParams;
export const assetParamsToRoute = ({
chatId,
@ -75,9 +76,12 @@ export const assetParamsToRoute = ({
}
if (type === 'report') {
const { reportVersionNumber } = rest as ReportRouteParams;
return createReportRoute({
assetId,
chatId
chatId,
page,
reportVersionNumber: reportVersionNumber || versionNumber
});
}

View File

@ -0,0 +1,263 @@
import { describe, it, expect, vi } from 'vitest';
import { createReportRoute, type ReportRouteParams } from './createReportRoute';
import { BusterRoutes } from '@/routes/busterRoutes';
// Mock the createBusterRoute function
vi.mock('@/routes/busterRoutes', () => ({
BusterRoutes: {
APP_REPORTS_ID: '/app/reports/:reportId',
APP_REPORTS_ID_FILE: '/app/reports/:reportId/file?report_version_number=:reportVersionNumber',
APP_CHAT_ID_REPORT_ID: '/app/chats/:chatId/reports/:reportId',
APP_CHAT_ID_REPORT_ID_FILE:
'/app/chats/:chatId/reports/:reportId/file?report_version_number=:reportVersionNumber'
},
createBusterRoute: vi.fn((params) => {
// Mock implementation that returns the route with parameters substituted
let route = params.route;
// Replace route parameters with actual values
if (params.reportId) {
route = route.replace(':reportId', params.reportId);
}
if (params.chatId) {
route = route.replace(':chatId', params.chatId);
}
if (params.reportVersionNumber !== undefined) {
route = route.replace(':reportVersionNumber', params.reportVersionNumber.toString());
}
return route;
})
}));
describe('createReportRoute', () => {
describe('standalone report routes (no chatId)', () => {
it('should create basic report route when page is undefined', () => {
// Test case: Basic report route without page parameter
// Expected output: Standard report route
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-123'
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-123');
});
it('should create basic report route when page is "report"', () => {
// Test case: Explicit report page
// Expected output: Standard report route
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-456',
page: 'report'
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-456');
});
it('should create file route when page is "files"', () => {
// Test case: File page without version number
// Expected output: File route with undefined version number
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-789',
page: 'files'
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-789/file?report_version_number=undefined');
});
it('should create file route with version number when page is "files"', () => {
// Test case: File page with specific version number
// Expected output: File route with version number parameter
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-101',
page: 'files',
reportVersionNumber: 5
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-101/file?report_version_number=5');
});
it('should handle reportVersionNumber on report page (should be ignored)', () => {
// Test case: Version number provided but page is report
// Expected output: Standard report route (version number ignored)
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-202',
page: 'report',
reportVersionNumber: 3
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-202');
});
});
describe('chat context report routes (with chatId)', () => {
it('should create chat report route when page is undefined', () => {
// Test case: Chat context report without page parameter
// Expected output: Chat report route
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-chat-1',
chatId: 'chat-123'
};
const result = createReportRoute(params);
expect(result).toBe('/app/chats/chat-123/reports/report-chat-1');
});
it('should create chat report route when page is "report"', () => {
// Test case: Explicit report page in chat context
// Expected output: Chat report route
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-chat-2',
chatId: 'chat-456',
page: 'report'
};
const result = createReportRoute(params);
expect(result).toBe('/app/chats/chat-456/reports/report-chat-2');
});
it('should create chat file route when page is "files"', () => {
// Test case: File page in chat context without version
// Expected output: Chat file route with undefined version
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-chat-3',
chatId: 'chat-789',
page: 'files'
};
const result = createReportRoute(params);
expect(result).toBe(
'/app/chats/chat-789/reports/report-chat-3/file?report_version_number=undefined'
);
});
it('should create chat file route with version number when page is "files"', () => {
// Test case: File page in chat context with version number
// Expected output: Chat file route with version parameter
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-chat-4',
chatId: 'chat-101',
page: 'files',
reportVersionNumber: 7
};
const result = createReportRoute(params);
expect(result).toBe('/app/chats/chat-101/reports/report-chat-4/file?report_version_number=7');
});
it('should handle reportVersionNumber on chat report page (should be ignored)', () => {
// Test case: Version number in chat context on report page
// Expected output: Chat report route (version number ignored)
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-chat-5',
chatId: 'chat-202',
page: 'report',
reportVersionNumber: 4
};
const result = createReportRoute(params);
expect(result).toBe('/app/chats/chat-202/reports/report-chat-5');
});
});
describe('edge cases and validation', () => {
it('should handle empty asset ID', () => {
// Test case: Empty asset ID
// Expected output: Route with empty reportId
const params: Omit<ReportRouteParams, 'type'> = {
assetId: ''
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/');
});
it('should handle empty chat ID when provided', () => {
// Test case: Empty chat ID
// Expected output: Chat route with empty chatId
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-test',
chatId: ''
};
const result = createReportRoute(params);
expect(result).toBe('/app/chats//reports/report-test');
});
it('should handle reportVersionNumber of 0', () => {
// Test case: Version number is 0 (valid but falsy)
// Expected output: File route with version 0
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-version-zero',
page: 'files',
reportVersionNumber: 0
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-version-zero/file?report_version_number=0');
});
it('should handle negative reportVersionNumber', () => {
// Test case: Negative version number
// Expected output: File route with negative version
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-negative',
page: 'files',
reportVersionNumber: -1
};
const result = createReportRoute(params);
expect(result).toBe('/app/reports/report-negative/file?report_version_number=-1');
});
});
describe('page type validation', () => {
it('should default to "report" page when page is undefined', () => {
// Test case: No page parameter provided
// Expected output: Should default to report behavior
const params: Omit<ReportRouteParams, 'type'> = {
assetId: 'report-default'
};
const result = createReportRoute(params);
// Should create standard report route, not file route
expect(result).toBe('/app/reports/report-default');
expect(result).not.toContain('/file');
});
it('should handle all valid page types', () => {
// Test case: Test all valid page types
// Expected output: Correct routes for each page type
const baseParams = { assetId: 'report-page-test' };
const reportResult = createReportRoute({ ...baseParams, page: 'report' });
const filesResult = createReportRoute({ ...baseParams, page: 'files' });
const undefinedResult = createReportRoute({ ...baseParams, page: undefined });
expect(reportResult).toBe('/app/reports/report-page-test');
expect(filesResult).toBe(
'/app/reports/report-page-test/file?report_version_number=undefined'
);
expect(undefinedResult).toBe('/app/reports/report-page-test');
});
});
});

View File

@ -4,14 +4,40 @@ export type ReportRouteParams = {
assetId: string;
chatId?: string;
type: 'report';
page?: 'report' | 'files' | undefined;
secondaryView?: undefined;
reportVersionNumber?: number;
};
export const createReportRoute = ({
assetId: reportId,
chatId
chatId,
reportVersionNumber,
page = 'report'
}: Omit<ReportRouteParams, 'type'>) => {
// Report routes within a chat context
// Handle file page routes with version numbers
if (page === 'files') {
if (chatId) {
// Chat context file route
return createBusterRoute({
route: BusterRoutes.APP_CHAT_ID_REPORT_ID_FILE,
chatId,
reportId,
reportVersionNumber
});
}
// Standalone file route
return createBusterRoute({
route: BusterRoutes.APP_REPORTS_ID_FILE,
reportId,
reportVersionNumber
});
}
// Handle standard report page routes
if (chatId) {
// Report routes within a chat context
return createBusterRoute({
route: BusterRoutes.APP_CHAT_ID_REPORT_ID,
chatId,

View File

@ -6,6 +6,7 @@ export enum BusterAppRoutes {
APP_COLLECTIONS_ID = '/app/collections/:collectionId',
APP_REPORTS = '/app/reports',
APP_REPORTS_ID = '/app/reports/:reportId',
APP_REPORTS_ID_FILE = '/app/reports/:reportId/file?report_version_number=:reportVersionNumber',
APP_METRIC = '/app/metrics',
APP_METRIC_ID_CHART = '/app/metrics/:metricId/chart?secondary_view=:secondaryView&metric_version_number=:metricVersionNumber',
APP_METRIC_ID_RESULTS = '/app/metrics/:metricId/results?secondary_view=:secondaryView&metric_version_number=:metricVersionNumber',
@ -29,6 +30,7 @@ export enum BusterAppRoutes {
APP_CHAT_ID = '/app/chats/:chatId',
APP_CHAT_ID_REASONING_ID = '/app/chats/:chatId/reasoning/:messageId',
APP_CHAT_ID_REPORT_ID = '/app/chats/:chatId/reports/:reportId',
APP_CHAT_ID_REPORT_ID_FILE = '/app/chats/:chatId/reports/:reportId/file?report_version_number=:reportVersionNumber',
APP_CHAT_ID_METRIC_ID = '/app/chats/:chatId/metrics/:metricId?secondary_view=:secondaryView&metric_version_number=:metricVersionNumber',
APP_CHAT_ID_METRIC_ID_CHART = '/app/chats/:chatId/metrics/:metricId/chart?secondary_view=:secondaryView&metric_version_number=:metricVersionNumber',
APP_CHAT_ID_METRIC_ID_SQL = '/app/chats/:chatId/metrics/:metricId/sql?secondary_view=:secondaryView&metric_version_number=:metricVersionNumber',
@ -59,6 +61,17 @@ export type BusterAppRoutesWithArgs = {
route: BusterAppRoutes.APP_REPORTS_ID;
reportId: string;
};
[BusterAppRoutes.APP_REPORTS_ID_FILE]: {
route: BusterAppRoutes.APP_REPORTS_ID_FILE;
reportId: string;
reportVersionNumber?: number;
};
[BusterAppRoutes.APP_CHAT_ID_REPORT_ID_FILE]: {
route: BusterAppRoutes.APP_CHAT_ID_REPORT_ID_FILE;
chatId: string;
reportId: string;
reportVersionNumber?: number;
};
[BusterAppRoutes.APP_METRIC]: {
route: BusterAppRoutes.APP_METRIC;
};

View File

@ -127,7 +127,7 @@ export async function getReport(input: GetReportInput) {
const report = {
...reportData,
version_number: versionHistoryArray[versionHistoryArray.length - 1]?.version_number ?? 1,
version_history: versionHistoryArray,
versions: versionHistoryArray,
collections: reportCollectionsResult,
individual_permissions: individualPermissionsResult,
};

View File

@ -1,7 +1,7 @@
import type { ReportElements } from '@buster/database';
import { z } from 'zod';
import { AssetCollectionsSchema } from '../collections/shared-asset-collections';
import { IndividualPermissionsSchema } from '../share';
import { IndividualPermissionsSchema, ShareConfigSchema } from '../share';
import { VersionsSchema } from '../version-shared';
export const ReportListItemSchema = z.object({
@ -22,12 +22,11 @@ export const ReportIndividualResponseSchema = z.object({
created_by_id: z.string(),
created_by_name: z.string().nullable(),
created_by_avatar: z.string().nullable(),
publicly_accessible: z.boolean(),
version_number: z.number(),
version_history: VersionsSchema,
versions: VersionsSchema,
collections: AssetCollectionsSchema,
individual_permissions: IndividualPermissionsSchema,
content: z.any() as z.ZodType<ReportElements>, //we use any here because we don't know the type of the content, will be validated in the database
...ShareConfigSchema.shape,
});
export type ReportListItem = z.infer<typeof ReportListItemSchema>;