diff --git a/apps/server/src/api/v2/dashboards/[id]/GET.test.ts b/apps/server/src/api/v2/dashboards/[id]/GET.test.ts index 83260100d..90bb13198 100644 --- a/apps/server/src/api/v2/dashboards/[id]/GET.test.ts +++ b/apps/server/src/api/v2/dashboards/[id]/GET.test.ts @@ -14,6 +14,7 @@ import { getPubliclyEnabledByUser } from '../../../../shared-helpers/get-publicl import { buildMetricResponse, fetchAndProcessMetricData, + getMetricsInAncestorAssetFromMetricIds, } from '../../../../shared-helpers/metric-helpers'; import { getDashboardHandler } from './GET'; @@ -36,6 +37,7 @@ vi.mock('../../../../shared-helpers/get-publicly-enabled-by-user', () => ({ vi.mock('../../../../shared-helpers/metric-helpers', () => ({ fetchAndProcessMetricData: vi.fn(), buildMetricResponse: vi.fn(), + getMetricsInAncestorAssetFromMetricIds: vi.fn(), })); vi.mock('js-yaml', () => ({ @@ -53,6 +55,7 @@ describe('getDashboardHandler', () => { const mockGetPubliclyEnabledByUser = getPubliclyEnabledByUser as Mock; const mockFetchAndProcessMetricData = fetchAndProcessMetricData as Mock; const mockBuildMetricResponse = buildMetricResponse as Mock; + const mockGetMetricsInAncestorAssetFromMetricIds = getMetricsInAncestorAssetFromMetricIds as Mock; // Mock data const mockUser: User = { @@ -108,6 +111,7 @@ describe('getDashboardHandler', () => { mockGetOrganizationMemberCount.mockResolvedValue(5); mockGetCollectionsAssociatedWithDashboard.mockResolvedValue([]); mockGetPubliclyEnabledByUser.mockResolvedValue(null); + mockGetMetricsInAncestorAssetFromMetricIds.mockResolvedValue([]); mockFetchAndProcessMetricData.mockResolvedValue({ metricFile: { id: 'metric-1' }, resolvedContent: { name: 'Test Metric' }, diff --git a/apps/server/src/api/v2/dashboards/[id]/GET.ts b/apps/server/src/api/v2/dashboards/[id]/GET.ts index cb3db9b43..2a29b7b3c 100644 --- a/apps/server/src/api/v2/dashboards/[id]/GET.ts +++ b/apps/server/src/api/v2/dashboards/[id]/GET.ts @@ -12,17 +12,13 @@ import { type GetDashboardResponse, } from '@buster/server-shared/dashboards'; import type { DashboardYml } from '@buster/server-shared/dashboards'; -import type { Metric } from '@buster/server-shared/metrics'; import type { VerificationStatus } from '@buster/server-shared/share'; import { zValidator } from '@hono/zod-validator'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; import yaml from 'js-yaml'; import { getPubliclyEnabledByUser } from '../../../../shared-helpers/get-publicly-enabled-by-user'; -import { - buildMetricResponse, - fetchAndProcessMetricData, -} from '../../../../shared-helpers/metric-helpers'; +import { getMetricsInAncestorAssetFromMetricIds } from '../../../../shared-helpers/metric-helpers'; interface GetDashboardHandlerParams { dashboardId: string; @@ -200,7 +196,7 @@ export async function getDashboardHandler( // Extract metric IDs from dashboard config const metricIds = extractMetricIds(resolvedContent); - const metrics = await getMetricsFromDashboardMetricIds(metricIds, user); + const metrics = await getMetricsInAncestorAssetFromMetricIds(metricIds, user); // Get the extra dashboard info concurrently const [individualPermissions, workspaceMemberCount, collections, publicEnabledBy] = @@ -263,39 +259,3 @@ export function extractMetricIds(content: DashboardYml): string[] { }); } } - -export async function getMetricsFromDashboardMetricIds( - metricIds: string[], - user: User -): Promise> { - const metricsObj: Record = {}; - - // Process metrics in chunks of 4 to manage concurrency - const results = []; - const chunkSize = 4; - - for (let i = 0; i < metricIds.length; i += chunkSize) { - const chunk = metricIds.slice(i, i + chunkSize); - const chunkPromises = chunk.map(async (metricId) => { - const processedData = await fetchAndProcessMetricData(metricId, user, { - publicAccessPreviouslyVerified: true, // Access is inherited from dashboard access at a minimum - }); - - // Build the metric response - const metric = await buildMetricResponse(processedData, user.id); - return { metricId, metric }; - }); - - const chunkResults = await Promise.all(chunkPromises); - results.push(...chunkResults); - } - - // Filter out failed metrics and build the response object - for (const result of results) { - if (result) { - metricsObj[result.metricId] = result.metric; - } - } - - return metricsObj; -} diff --git a/apps/server/src/api/v2/reports/[id]/GET.ts b/apps/server/src/api/v2/reports/[id]/GET.ts index b50167d86..58f77fc5d 100644 --- a/apps/server/src/api/v2/reports/[id]/GET.ts +++ b/apps/server/src/api/v2/reports/[id]/GET.ts @@ -1,5 +1,5 @@ import { checkPermission } from '@buster/access-controls'; -import { getReportFileById } from '@buster/database/queries'; +import { type User, getMetricIdsInReport, getReportFileById } from '@buster/database/queries'; import { GetReportParamsSchema, GetReportQuerySchema, @@ -8,19 +8,23 @@ import { import { zValidator } from '@hono/zod-validator'; import { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; +import { getMetricsInAncestorAssetFromMetricIds } from '../../../../shared-helpers/metric-helpers'; import { standardErrorHandler } from '../../../../utils/response'; export async function getReportHandler( reportId: string, - user: { id: string }, + user: User, versionNumber?: number | undefined, password?: string | undefined ): Promise { - const report = await getReportFileById({ - reportId, - userId: user.id, - versionNumber, - }); + const [report, metricIds] = await Promise.all([ + getReportFileById({ + reportId, + userId: user.id, + versionNumber, + }), + getMetricIdsInReport({ reportId }), + ]); const permission = await checkPermission({ userId: user.id, @@ -39,9 +43,12 @@ export async function getReportHandler( throw new HTTPException(403, { message: 'You do not have permission to view this report' }); } + const metrics = await getMetricsInAncestorAssetFromMetricIds(metricIds, user); + const response: GetReportResponse = { ...report, permission: permission.effectiveRole, + metrics, }; return response; diff --git a/apps/server/src/shared-helpers/metric-helpers.ts b/apps/server/src/shared-helpers/metric-helpers.ts index 77f040605..8aae0cb4e 100644 --- a/apps/server/src/shared-helpers/metric-helpers.ts +++ b/apps/server/src/shared-helpers/metric-helpers.ts @@ -12,6 +12,7 @@ import { DEFAULT_CHART_THEME, type DataMetadata, type GetMetricResponse, + type Metric, type MetricYml, } from '@buster/server-shared/metrics'; import type { AssetPermissionRole, VerificationStatus } from '@buster/server-shared/share'; @@ -270,3 +271,39 @@ export async function buildMetricResponse( return response; } + +export async function getMetricsInAncestorAssetFromMetricIds( + metricIds: string[], + user: User +): Promise> { + const metricsObj: Record = {}; + + // Process metrics in chunks of 4 to manage concurrency + const results = []; + const chunkSize = 4; + + for (let i = 0; i < metricIds.length; i += chunkSize) { + const chunk = metricIds.slice(i, i + chunkSize); + const chunkPromises = chunk.map(async (metricId) => { + const processedData = await fetchAndProcessMetricData(metricId, user, { + publicAccessPreviouslyVerified: true, // Access is inherited from dashboard access at a minimum + }); + + // Build the metric response + const metric = await buildMetricResponse(processedData, user.id); + return { metricId, metric }; + }); + + const chunkResults = await Promise.all(chunkPromises); + results.push(...chunkResults); + } + + // Filter out failed metrics and build the response object + for (const result of results) { + if (result) { + metricsObj[result.metricId] = result.metric; + } + } + + return metricsObj; +} diff --git a/packages/database/src/queries/metrics-to-reports/get-metrics-in-report.test.ts b/packages/database/src/queries/metrics-to-reports/get-metrics-in-report.test.ts new file mode 100644 index 000000000..60e7c2a33 --- /dev/null +++ b/packages/database/src/queries/metrics-to-reports/get-metrics-in-report.test.ts @@ -0,0 +1,258 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { GetMetricIdsInReportInputSchema, getMetricIdsInReport } from './get-metrics-in-report'; + +// Mock the database connection and schema +vi.mock('../../connection', () => ({ + db: { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(), + })), + })), + }, +})); + +vi.mock('../../schema', () => ({ + metricFilesToReportFiles: { + metricFileId: 'metricFileId', + reportFileId: 'reportFileId', + deletedAt: 'deletedAt', + }, +})); + +// Mock drizzle-orm functions +vi.mock('drizzle-orm', () => ({ + and: vi.fn((...conditions) => ({ type: 'and', conditions })), + eq: vi.fn((column, value) => ({ type: 'eq', column, value })), + isNull: vi.fn((column) => ({ type: 'isNull', column })), +})); + +import { db } from '../../connection'; + +const mockDb = vi.mocked(db); + +describe('GetMetricIdsInReportInputSchema', () => { + it('should validate valid UUID report ID', () => { + const validParams = { + reportId: '123e4567-e89b-12d3-a456-426614174000', + }; + + const result = GetMetricIdsInReportInputSchema.safeParse(validParams); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.reportId).toBe(validParams.reportId); + } + }); + + it('should reject invalid UUID format', () => { + const invalidParams = { + reportId: 'not-a-valid-uuid', + }; + + const result = GetMetricIdsInReportInputSchema.safeParse(invalidParams); + expect(result.success).toBe(false); + }); + + it('should reject missing report ID', () => { + const emptyParams = {}; + + const result = GetMetricIdsInReportInputSchema.safeParse(emptyParams); + expect(result.success).toBe(false); + }); + + it('should reject non-string report ID', () => { + const invalidParams = { + reportId: 123, + }; + + const result = GetMetricIdsInReportInputSchema.safeParse(invalidParams); + expect(result.success).toBe(false); + }); +}); + +describe('getMetricIdsInReport', () => { + const validParams = { + reportId: '123e4567-e89b-12d3-a456-426614174000', + }; + + let mockSelect: ReturnType; + let mockFrom: ReturnType; + let mockWhere: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + + mockWhere = vi.fn().mockResolvedValue([]); + mockFrom = vi.fn().mockReturnValue({ where: mockWhere }); + mockSelect = vi.fn().mockReturnValue({ from: mockFrom }); + mockDb.select = mockSelect; + }); + + it('should validate params before database query', async () => { + const invalidParams = { + reportId: 'invalid-uuid', + }; + + await expect(getMetricIdsInReport(invalidParams)).rejects.toThrow(); + expect(mockSelect).not.toHaveBeenCalled(); + }); + + it('should return metric IDs when found', async () => { + const mockResults = [ + { metricFileId: 'metric-1' }, + { metricFileId: 'metric-2' }, + { metricFileId: 'metric-3' }, + ]; + mockWhere.mockResolvedValue(mockResults); + + const result = await getMetricIdsInReport(validParams); + + expect(mockSelect).toHaveBeenCalledWith({ + metricFileId: expect.any(String), + }); + expect(mockFrom).toHaveBeenCalled(); + expect(mockWhere).toHaveBeenCalled(); + expect(result).toEqual(['metric-1', 'metric-2', 'metric-3']); + }); + + it('should return empty array when no metrics found', async () => { + mockWhere.mockResolvedValue([]); + + const result = await getMetricIdsInReport(validParams); + + expect(result).toEqual([]); + }); + + it('should use correct database query conditions', async () => { + await getMetricIdsInReport(validParams); + + // Verify the query was constructed with correct conditions + expect(mockSelect).toHaveBeenCalledWith({ + metricFileId: expect.any(String), + }); + expect(mockFrom).toHaveBeenCalled(); + expect(mockWhere).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'and', + conditions: expect.arrayContaining([ + expect.objectContaining({ + type: 'eq', + column: expect.any(String), + value: validParams.reportId, + }), // reportFileId condition + expect.objectContaining({ + type: 'isNull', + column: expect.any(String), + }), // not deleted condition + ]), + }) + ); + }); + + it('should handle database errors', async () => { + mockWhere.mockRejectedValue(new Error('Database query failed')); + + await expect(getMetricIdsInReport(validParams)).rejects.toThrow('Database query failed'); + }); + + it('should handle single metric result', async () => { + const mockResults = [{ metricFileId: 'single-metric-id' }]; + mockWhere.mockResolvedValue(mockResults); + + const result = await getMetricIdsInReport(validParams); + + expect(result).toEqual(['single-metric-id']); + expect(result).toHaveLength(1); + }); + + it('should handle large result sets', async () => { + const largeResults = Array.from({ length: 100 }, (_, i) => ({ + metricFileId: `metric-${i}`, + })); + mockWhere.mockResolvedValue(largeResults); + + const result = await getMetricIdsInReport(validParams); + + expect(result).toHaveLength(100); + expect(result[0]).toBe('metric-0'); + expect(result[99]).toBe('metric-99'); + }); + + it('should filter out soft-deleted relationships', async () => { + // This test verifies the query construction filters for deletedAt IS NULL + // since that happens at the database level + await getMetricIdsInReport(validParams); + + expect(mockWhere).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'and', + conditions: expect.arrayContaining([ + expect.objectContaining({ type: 'isNull' }), // Should have isNull condition for deletedAt + ]), + }) + ); + }); + + it('should filter by correct report ID', async () => { + const specificReportId = '123e4567-e89b-12d3-a456-426614174001'; + const paramsWithSpecificId = { reportId: specificReportId }; + + await getMetricIdsInReport(paramsWithSpecificId); + + expect(mockWhere).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'and', + conditions: expect.arrayContaining([ + expect.objectContaining({ + type: 'eq', + value: specificReportId, + }), // Should filter by the specific report ID + ]), + }) + ); + }); + + it('should handle metrics with UUID format', async () => { + const mockResults = [ + { metricFileId: '123e4567-e89b-12d3-a456-426614174001' }, + { metricFileId: '123e4567-e89b-12d3-a456-426614174002' }, + ]; + mockWhere.mockResolvedValue(mockResults); + + const result = await getMetricIdsInReport(validParams); + + expect(result).toEqual([ + '123e4567-e89b-12d3-a456-426614174001', + '123e4567-e89b-12d3-a456-426614174002', + ]); + }); + + it('should handle unexpected database response format gracefully', async () => { + // Edge case: database returns unexpected format + const unexpectedResults = [ + { metricFileId: null }, + { metricFileId: undefined }, + { metricFileId: '' }, + { metricFileId: 'valid-metric-id' }, + ]; + mockWhere.mockResolvedValue(unexpectedResults); + + const result = await getMetricIdsInReport(validParams); + + // The function should return what the database gives it + // Validation of UUIDs would happen at a higher level + expect(result).toEqual([null, undefined, '', 'valid-metric-id']); + }); + + it('should use correct table and columns', async () => { + await getMetricIdsInReport(validParams); + + // Verify we're selecting the right column + expect(mockSelect).toHaveBeenCalledWith({ + metricFileId: expect.any(String), + }); + + // Verify we're using the metricFilesToReportFiles table + expect(mockFrom).toHaveBeenCalled(); + }); +}); diff --git a/packages/database/src/queries/metrics-to-reports/get-metrics-in-report.ts b/packages/database/src/queries/metrics-to-reports/get-metrics-in-report.ts new file mode 100644 index 000000000..700ab115a --- /dev/null +++ b/packages/database/src/queries/metrics-to-reports/get-metrics-in-report.ts @@ -0,0 +1,49 @@ +import { and, eq, isNull } from 'drizzle-orm'; +import { z } from 'zod'; +import { db } from '../../connection'; +import { metricFilesToReportFiles } from '../../schema'; + +// Input validation schema +export const GetMetricIdsInReportInputSchema = z.object({ + reportId: z.string().uuid('Report ID must be a valid UUID'), +}); + +type GetMetricIdsInReportInput = z.infer; + +/** + * Retrieves all metric file IDs associated with a report that are not soft-deleted. + * @param params - Object containing the reportId + * @returns Promise - Array of metric file IDs + */ +export const getMetricIdsInReport = async ( + params: GetMetricIdsInReportInput +): Promise => { + const { reportId } = GetMetricIdsInReportInputSchema.parse(params); + + try { + const metricRelationships = await db + .select({ + metricFileId: metricFilesToReportFiles.metricFileId, + }) + .from(metricFilesToReportFiles) + .where( + and( + eq(metricFilesToReportFiles.reportFileId, reportId), + isNull(metricFilesToReportFiles.deletedAt) + ) + ); + + return metricRelationships.map((relationship) => relationship.metricFileId); + } catch (error) { + console.error('Error retrieving metric IDs for report:', { + reportId, + error: error instanceof Error ? error.message : error, + }); + + if (error instanceof Error) { + throw error; + } + + throw new Error('Failed to retrieve metric IDs for report'); + } +}; diff --git a/packages/database/src/queries/metrics-to-reports/index.ts b/packages/database/src/queries/metrics-to-reports/index.ts index be70fabf6..1b98611a4 100644 --- a/packages/database/src/queries/metrics-to-reports/index.ts +++ b/packages/database/src/queries/metrics-to-reports/index.ts @@ -1 +1,2 @@ export * from './update-metrics-to-reports'; +export * from './get-metrics-in-report'; diff --git a/packages/server-shared/src/reports/reports.types.ts b/packages/server-shared/src/reports/reports.types.ts index d2772c992..9a02dfd47 100644 --- a/packages/server-shared/src/reports/reports.types.ts +++ b/packages/server-shared/src/reports/reports.types.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import { AssetCollectionsSchema } from '../collections/shared-asset-collections'; +import { MetricSchema } from '../metrics'; import { ShareConfigSchema } from '../share'; import { VersionsSchema } from '../version-shared'; @@ -30,6 +31,7 @@ export const ReportResponseSchema = z.object({ collections: AssetCollectionsSchema, content: z.string(), ...ShareConfigSchema.shape, + metrics: z.record(z.string(), MetricSchema), }); export type ReportListItem = z.infer;