Merge pull request #1148 from buster-so/wells-bus-1906-report-should-send-metrics

adding metrics to reports/[id] endpoint

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' },

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<Record<string, Metric>> {
-  const metricsObj: Record<string, Metric> = {};
-
-  // 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;
-}

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<GetReportResponse> {
-  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;

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<Record<string, Metric>> {
+  const metricsObj: Record<string, Metric> = {};
+
+  // 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;
+}

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<typeof vi.fn>;
+  let mockFrom: ReturnType<typeof vi.fn>;
+  let mockWhere: ReturnType<typeof vi.fn>;
+
+  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();
+  });
+});

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<typeof GetMetricIdsInReportInputSchema>;
+
+/**
+ * Retrieves all metric file IDs associated with a report that are not soft-deleted.
+ * @param params - Object containing the reportId
+ * @returns Promise<string[]> - Array of metric file IDs
+ */
+export const getMetricIdsInReport = async (
+  params: GetMetricIdsInReportInput
+): Promise<string[]> => {
+  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');
+  }
+};

packages/database/src/queries/metrics-to-reports/index.ts

@@ -1 +1,2 @@
 export * from './update-metrics-to-reports';
+export * from './get-metrics-in-report';

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<typeof ReportListItemSchema>;