From e6dfca0e25d7e18ddec360e476797ee848034610 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Wed, 16 Apr 2025 09:38:02 -0600 Subject: [PATCH] initialize file views --- .../ReasoningMessagePills.tsx | 7 +- .../helpers/getFileViewFromRoute.test.ts | 41 +++++ .../ChatLayout/ChatLayoutContext/index.ts | 1 - .../ChatLayoutContext/publicHelpers.ts | 8 - .../useGetChatParams.test.tsx | 157 ++++++++++++++++++ .../ChatLayoutContext/useGetChatParams.tsx | 3 +- .../useLayoutConfig/helpers.test.ts | 144 ++++++++++++++++ .../useLayoutConfig/useLayoutConfig.ts | 33 ++-- .../FileContainer/FileContainer.tsx | 2 +- 9 files changed, 358 insertions(+), 38 deletions(-) create mode 100644 web/src/layouts/ChatLayout/ChatLayoutContext/helpers/getFileViewFromRoute.test.ts delete mode 100644 web/src/layouts/ChatLayout/ChatLayoutContext/publicHelpers.ts create mode 100644 web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.test.tsx create mode 100644 web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/helpers.test.ts diff --git a/web/src/controllers/ReasoningController/ReasoningMessages/ReasoningMessage_PillContainers/ReasoningMessagePills.tsx b/web/src/controllers/ReasoningController/ReasoningMessages/ReasoningMessage_PillContainers/ReasoningMessagePills.tsx index 34eb5e161..aeee14d65 100644 --- a/web/src/controllers/ReasoningController/ReasoningMessages/ReasoningMessage_PillContainers/ReasoningMessagePills.tsx +++ b/web/src/controllers/ReasoningController/ReasoningMessages/ReasoningMessage_PillContainers/ReasoningMessagePills.tsx @@ -63,7 +63,7 @@ export const ReasoningMessagePills: React.FC<{ animate={pills.length > 0 ? 'visible' : 'hidden'} className={'flex w-full flex-wrap gap-1.5 overflow-hidden'}> {pills.map((pill) => ( - + ))} @@ -88,10 +88,9 @@ const Pill: React.FC<{ onClick={() => !!id && !!type && onClick?.({ id, type })} variants={pillVariants} className={cn( - 'text-text-secondary bg-item-active border-border hover:bg-item-hover-active h-[18px] min-h-[18px] rounded-sm border px-1 text-xs', - className, + 'text-text-secondary bg-item-active border-border hover:bg-item-hover-active flex h-[18px] min-h-[18px] items-center justify-center rounded-sm border px-1 text-xs whitespace-nowrap transition-all', !!onClick && 'cursor-pointer', - 'flex items-center justify-center whitespace-nowrap' + className )}> {text} diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/helpers/getFileViewFromRoute.test.ts b/web/src/layouts/ChatLayout/ChatLayoutContext/helpers/getFileViewFromRoute.test.ts new file mode 100644 index 000000000..b9ebc8874 --- /dev/null +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/helpers/getFileViewFromRoute.test.ts @@ -0,0 +1,41 @@ +import { BusterRoutes } from '@/routes/busterRoutes'; +import { getFileViewFromRoute } from './getFileViewFromRoute'; + +describe('getFileViewFromRoute', () => { + test('should return chart view for metric chart route', () => { + expect(getFileViewFromRoute(BusterRoutes.APP_METRIC_ID_CHART)).toBe('chart'); + }); + + test('should return results view for metric results routes', () => { + expect(getFileViewFromRoute(BusterRoutes.APP_METRIC_ID_RESULTS)).toBe('results'); + expect(getFileViewFromRoute(BusterRoutes.APP_CHAT_ID_METRIC_ID_RESULTS)).toBe('results'); + }); + + test('should return file view for file-related routes', () => { + expect(getFileViewFromRoute(BusterRoutes.APP_METRIC_ID_FILE)).toBe('file'); + expect(getFileViewFromRoute(BusterRoutes.APP_CHAT_ID_METRIC_ID_FILE)).toBe('file'); + expect(getFileViewFromRoute(BusterRoutes.APP_CHAT_ID_DASHBOARD_ID_FILE)).toBe('file'); + expect(getFileViewFromRoute(BusterRoutes.APP_DASHBOARD_ID_FILE)).toBe('file'); + }); + + test('should return dashboard view for dashboard routes', () => { + expect(getFileViewFromRoute(BusterRoutes.APP_CHAT_ID_DASHBOARD_ID)).toBe('dashboard'); + expect(getFileViewFromRoute(BusterRoutes.APP_DASHBOARD_ID)).toBe('dashboard'); + }); + + test('should consistently return file view for chat metric routes', () => { + expect(getFileViewFromRoute(BusterRoutes.APP_CHAT_ID_METRIC_ID)).toBe('file'); + expect(getFileViewFromRoute(BusterRoutes.APP_CHAT_ID_METRIC_ID_FILE)).toBe('file'); + }); + + test('should return undefined for routes not in the mapping', () => { + // @ts-expect-error Testing with an invalid route + expect(getFileViewFromRoute('INVALID_ROUTE')).toBeUndefined(); + }); + + test('type safety - should accept only BusterRoutes enum values', () => { + // This test is mainly for TypeScript compilation - it should error if we try to pass invalid types + const validRoute = BusterRoutes.APP_DASHBOARD_ID; + expect(() => getFileViewFromRoute(validRoute)).not.toThrow(); + }); +}); diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/index.ts b/web/src/layouts/ChatLayout/ChatLayoutContext/index.ts index a1fcfa050..34deed202 100644 --- a/web/src/layouts/ChatLayout/ChatLayoutContext/index.ts +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/index.ts @@ -1,4 +1,3 @@ export * from './ChatLayoutContext'; -export * from './publicHelpers'; export * from './useLayoutConfig'; export * from './helpers'; diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/publicHelpers.ts b/web/src/layouts/ChatLayout/ChatLayoutContext/publicHelpers.ts deleted file mode 100644 index 7b92dfa00..000000000 --- a/web/src/layouts/ChatLayout/ChatLayoutContext/publicHelpers.ts +++ /dev/null @@ -1,8 +0,0 @@ -import type { ThoughtFileType } from '@/api/asset_interfaces'; - -const OPENABLE_FILES = new Set(['metric', 'dashboard', 'reasoning']); - -export const isOpenableFile = (type: ThoughtFileType | null): boolean => { - if (!type) return false; - return OPENABLE_FILES.has(type); -}; diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.test.tsx b/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.test.tsx new file mode 100644 index 000000000..d15730df1 --- /dev/null +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.test.tsx @@ -0,0 +1,157 @@ +import { renderHook } from '@testing-library/react'; +import { useGetChatParams } from './useGetChatParams'; +import * as navigation from 'next/navigation'; +import * as appLayout from '@/context/BusterAppLayout'; + +// Mock the required hooks and modules +jest.mock('next/navigation', () => ({ + useParams: jest.fn(), + useSearchParams: jest.fn(() => ({ + get: jest.fn() + })) +})); + +jest.mock('@/context/BusterAppLayout', () => ({ + useAppLayoutContextSelector: jest.fn() +})); + +describe('useGetChatParams', () => { + const mockUseParams = navigation.useParams as jest.Mock; + const mockUseSearchParams = navigation.useSearchParams as jest.Mock; + const mockUseAppLayoutContextSelector = appLayout.useAppLayoutContextSelector as jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + mockUseSearchParams.mockImplementation(() => ({ + get: jest.fn().mockReturnValue(null) + })); + mockUseAppLayoutContextSelector.mockReturnValue('default-route'); + }); + + test('returns undefined values when no params are provided', () => { + mockUseParams.mockReturnValue({}); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current).toEqual({ + isVersionHistoryMode: false, + chatId: undefined, + metricId: undefined, + dashboardId: undefined, + collectionId: undefined, + datasetId: undefined, + messageId: undefined, + metricVersionNumber: undefined, + dashboardVersionNumber: undefined, + currentRoute: 'default-route', + secondaryView: null + }); + }); + + test('correctly processes chat and message IDs', () => { + mockUseParams.mockReturnValue({ + chatId: 'chat-123', + messageId: 'msg-456' + }); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.chatId).toBe('chat-123'); + expect(result.current.messageId).toBe('msg-456'); + }); + + test('handles metric version number from path parameter', () => { + mockUseParams.mockReturnValue({ + metricId: 'metric-123', + versionNumber: '42' + }); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.metricId).toBe('metric-123'); + expect(result.current.metricVersionNumber).toBe(42); + }); + + test('handles metric version number from query parameter', () => { + mockUseParams.mockReturnValue({ + metricId: 'metric-123' + }); + mockUseSearchParams.mockImplementation(() => ({ + get: (param: string) => (param === 'metric_version_number' ? '43' : null) + })); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.metricVersionNumber).toBe(43); + }); + + test('handles dashboard version number from path parameter', () => { + mockUseParams.mockReturnValue({ + dashboardId: 'dashboard-123', + versionNumber: '44' + }); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.dashboardId).toBe('dashboard-123'); + expect(result.current.dashboardVersionNumber).toBe(44); + }); + + test('handles dashboard version number from query parameter', () => { + mockUseParams.mockReturnValue({ + dashboardId: 'dashboard-123' + }); + mockUseSearchParams.mockImplementation(() => ({ + get: (param: string) => (param === 'dashboard_version_number' ? '45' : null) + })); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.dashboardVersionNumber).toBe(45); + }); + + test('correctly identifies version history mode', () => { + mockUseSearchParams.mockImplementation(() => ({ + get: (param: string) => (param === 'secondary_view' ? 'version-history' : null) + })); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.isVersionHistoryMode).toBe(true); + }); + + test('handles collection and dataset IDs', () => { + mockUseParams.mockReturnValue({ + collectionId: 'collection-123', + datasetId: 'dataset-456' + }); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.collectionId).toBe('collection-123'); + expect(result.current.datasetId).toBe('dataset-456'); + }); + + test('preserves current route from app layout context', () => { + mockUseAppLayoutContextSelector.mockReturnValue('custom-route'); + + const { result } = renderHook(() => useGetChatParams()); + + expect(result.current.currentRoute).toBe('custom-route'); + }); + + test('returns consistent values on multiple renders without param changes', () => { + mockUseParams.mockReturnValue({ + chatId: 'chat-123', + metricId: 'metric-123', + versionNumber: '46' + }); + + const { result, rerender } = renderHook(() => useGetChatParams()); + const firstRender = result.current; + + rerender(); + + expect(result.current).toEqual(firstRender); + }); +}); diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx b/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx index a724cd0ac..d963c3b1a 100644 --- a/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/useGetChatParams.tsx @@ -1,7 +1,7 @@ 'use client'; import { useAppLayoutContextSelector } from '@/context/BusterAppLayout'; -import { useParams, useSearchParams, useSelectedLayoutSegments } from 'next/navigation'; +import { useParams, useSearchParams } from 'next/navigation'; import { useMemo } from 'react'; import { FileViewSecondary } from './useLayoutConfig'; @@ -24,7 +24,6 @@ export const useGetChatParams = () => { messageId: string | undefined; }; const searchParams = useSearchParams(); - const segments = useSelectedLayoutSegments(); const queryMetricVersionNumber = searchParams.get('metric_version_number'); const queryDashboardVersionNumber = searchParams.get('dashboard_version_number'); const secondaryView = searchParams.get('secondary_view') as FileViewSecondary | undefined; diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/helpers.test.ts b/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/helpers.test.ts new file mode 100644 index 000000000..fa35159e8 --- /dev/null +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/helpers.test.ts @@ -0,0 +1,144 @@ +import { BusterRoutes } from '@/routes'; +import { initializeFileViews } from './helpers'; +import { FileViewSecondary } from './interfaces'; + +describe('initializeFileViews', () => { + it('should return empty object when no metricId or dashboardId is provided', () => { + const result = initializeFileViews({ + metricId: undefined, + dashboardId: undefined, + currentRoute: BusterRoutes.APP_HOME, + secondaryView: undefined + }); + + expect(result).toEqual({}); + }); + + it('should initialize metric view with chart as default when route does not map to a view', () => { + const metricId = 'metric123'; + const result = initializeFileViews({ + metricId, + dashboardId: undefined, + currentRoute: BusterRoutes.APP_HOME, + secondaryView: undefined + }); + + expect(result).toEqual({ + [metricId]: { + selectedFileView: 'chart', + fileViewConfig: { + chart: { + secondaryView: undefined + } + } + } + }); + }); + + it('should initialize metric view with specific view from route', () => { + const metricId = 'metric123'; + const result = initializeFileViews({ + metricId, + dashboardId: undefined, + currentRoute: BusterRoutes.APP_METRIC_ID_RESULTS, + secondaryView: undefined + }); + + expect(result).toEqual({ + [metricId]: { + selectedFileView: 'results', + fileViewConfig: { + results: { + secondaryView: undefined + } + } + } + }); + }); + + it('should initialize dashboard view with dashboard as default when route does not map to a view', () => { + const dashboardId = 'dashboard123'; + const result = initializeFileViews({ + metricId: undefined, + dashboardId, + currentRoute: BusterRoutes.APP_HOME, + secondaryView: undefined + }); + + expect(result).toEqual({ + [dashboardId]: { + selectedFileView: 'dashboard', + fileViewConfig: { + dashboard: { + secondaryView: undefined + } + } + } + }); + }); + + it('should initialize dashboard view with specific view from route', () => { + const dashboardId = 'dashboard123'; + const result = initializeFileViews({ + metricId: undefined, + dashboardId, + currentRoute: BusterRoutes.APP_DASHBOARD_ID_FILE, + secondaryView: undefined + }); + + expect(result).toEqual({ + [dashboardId]: { + selectedFileView: 'file', + fileViewConfig: { + file: { + secondaryView: undefined + } + } + } + }); + }); + + it('should include secondaryView in metric config when provided', () => { + const metricId = 'metric123'; + const secondaryView: FileViewSecondary = 'chart-edit'; + const result = initializeFileViews({ + metricId, + dashboardId: undefined, + currentRoute: BusterRoutes.APP_METRIC_ID_CHART, + secondaryView + }); + + expect(result).toEqual({ + [metricId]: { + selectedFileView: 'chart', + fileViewConfig: { + chart: { + secondaryView: 'chart-edit' + } + } + } + }); + }); + + it('should include secondaryView in dashboard config when provided', () => { + const dashboardId = 'dashboard123'; + const secondaryView: FileViewSecondary = 'version-history'; + const result = initializeFileViews({ + metricId: undefined, + dashboardId, + currentRoute: BusterRoutes.APP_DASHBOARD_ID, + secondaryView + }); + + expect(result).toEqual({ + [dashboardId]: { + selectedFileView: 'dashboard', + fileViewConfig: { + dashboard: { + secondaryView: 'version-history' + } + } + } + }); + }); +}); diff --git a/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/useLayoutConfig.ts b/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/useLayoutConfig.ts index 29aebcb0e..6b24d1456 100644 --- a/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/useLayoutConfig.ts +++ b/web/src/layouts/ChatLayout/ChatLayoutContext/useLayoutConfig/useLayoutConfig.ts @@ -7,8 +7,8 @@ import { useMemoizedFn, useUpdateEffect } from '@/hooks'; import { create } from 'mutative'; import { ChatLayoutView } from '../../interfaces'; import type { SelectedFile } from '../../interfaces'; -import { timeout } from '@/lib'; -import { BusterRoutes } from '@/routes'; +import { timeout } from '@/lib/timeout'; +import { BusterRoutes } from '@/routes/busterRoutes'; import { SelectedFileSecondaryRenderRecord } from '../../FileContainer/FileContainerSecondary'; import { ChatParams } from '../useGetChatParams'; import { initializeFileViews } from './helpers'; @@ -194,24 +194,13 @@ export const useLayoutConfig = ({ }); }, [metricId, secondaryView, dashboardId, currentRoute]); - return useMemo( - () => ({ - selectedLayout, - selectedFileView, - selectedFileViewSecondary, - selectedFileViewRenderSecondary, - onSetFileView, - closeSecondaryView, - onCollapseFileClick - }), - [ - selectedLayout, - selectedFileView, - selectedFileViewSecondary, - selectedFileViewRenderSecondary, - onSetFileView, - closeSecondaryView, - onCollapseFileClick - ] - ); + return { + selectedLayout, + selectedFileView, + selectedFileViewSecondary, + selectedFileViewRenderSecondary, + onSetFileView, + closeSecondaryView, + onCollapseFileClick + }; }; diff --git a/web/src/layouts/ChatLayout/FileContainer/FileContainer.tsx b/web/src/layouts/ChatLayout/FileContainer/FileContainer.tsx index e4bd66f33..c773b88d5 100644 --- a/web/src/layouts/ChatLayout/FileContainer/FileContainer.tsx +++ b/web/src/layouts/ChatLayout/FileContainer/FileContainer.tsx @@ -77,7 +77,7 @@ export const FileContainer: React.FC = ({ children }) => { useUpdateLayoutEffect(() => { setTimeout(() => { - //TODO revaluate this? + //TODO revaluate this? What is this for? animateOpenSplitter(isOpenSecondary ? 'open' : 'closed'); }, 20); }, [isOpenSecondary]);