Fix queries review comments (#717)

* move loader out

* Resolve comments

* Rename test

Author: adhityamamallan

src/config/theme/theme-light.config.ts

@@ -14,6 +14,7 @@ const themeLight = {
     backgroundSecondary: '#F3F3F3',
     backgroundTertiary: '#E8E8E8',
     backgroundWarningLight: '#FDF2DC',
+    backgroundPositive: '#0E8345',
   },
 } satisfies DeepPartial<MakeExtendable<Theme>>;
 

…tests__/workflow-queries-loader.test.tsx …ries/__tests__/workflow-queries.test.tsxsrc/views/workflow-queries/workflow-queries-loader/__tests__/workflow-queries-loader.test.tsx renamed to src/views/workflow-queries/__tests__/workflow-queries.test.tsx src/views/workflow-queries/workflow-queries-loader/__tests__/workflow-queries-loader.test.tsx renamed to src/views/workflow-queries/__tests__/workflow-queries.test.tsx

@@ -8,9 +8,9 @@ import { render, screen, act } from '@/test-utils/rtl';
 import { type FetchWorkflowQueryTypesResponse } from '@/route-handlers/fetch-workflow-query-types/fetch-workflow-query-types.types';
 import { type QueryWorkflowResponse } from '@/route-handlers/query-workflow/query-workflow.types';
 
-import WorkflowQueriesLoader from '../workflow-queries-loader';
+import WorkflowQueries from '../workflow-queries';
 
-jest.mock('../../workflow-queries-tile/workflow-queries-tile', () =>
+jest.mock('../workflow-queries-tile/workflow-queries-tile', () =>
   jest.fn(({ name, onClick, runQuery }) => (
     <div onClick={onClick}>
       <div>Mock tile: {name}</div>
@@ -19,18 +19,16 @@ jest.mock('../../workflow-queries-tile/workflow-queries-tile', () =>
   ))
 );
 
-jest.mock(
-  '../../workflow-queries-result-json/workflow-queries-result-json',
-  () =>
-    jest.fn(({ data }) => (
-      <div>
-        <div>Mock JSON</div>
-        <div>{JSON.stringify(data)}</div>
-      </div>
-    ))
+jest.mock('../workflow-queries-result-json/workflow-queries-result-json', () =>
+  jest.fn(({ data }) => (
+    <div>
+      <div>Mock JSON</div>
+      <div>{JSON.stringify(data)}</div>
+    </div>
+  ))
 );
 
-describe(WorkflowQueriesLoader.name, () => {
+describe(WorkflowQueries.name, () => {
   it('renders without error and does not show excluded query type', async () => {
     await setup({});
 
@@ -71,11 +69,14 @@ async function setup({ error }: { error?: boolean }) {
 
   render(
     <Suspense>
-      <WorkflowQueriesLoader
-        domain="mock-domain"
-        cluster="mock-cluster"
-        workflowId="mock-workflow-id"
-        runId="mock-run-id"
+      <WorkflowQueries
+        params={{
+          domain: 'mock-domain',
+          cluster: 'mock-cluster',
+          workflowId: 'mock-workflow-id',
+          runId: 'mock-run-id',
+          workflowTab: 'queries',
+        }}
       />
     </Suspense>,
     {

src/views/workflow-queries/helpers/__tests__/get-workflow-query-status.test.ts

@@ -11,10 +11,10 @@ describe(getWorkflowQueryStatus.name, () => {
     expected: WorkflowQueryStatus;
   }> = [
     {
-      name: 'returns fetching if isFetching is true',
+      name: 'returns loading if isFetching is true',
       status: 'pending',
       isFetching: true,
-      expected: 'fetching',
+      expected: 'loading',
     },
     {
       name: 'returns success if query status is success',
@@ -29,10 +29,10 @@ describe(getWorkflowQueryStatus.name, () => {
       expected: 'error',
     },
     {
-      name: 'returns pending if query status is pending',
+      name: 'returns idle if query status is pending',
       status: 'pending',
       isFetching: false,
-      expected: 'pending',
+      expected: 'idle',
     },
   ];
 

src/views/workflow-queries/helpers/get-workflow-query-status.ts

@@ -10,7 +10,7 @@ export default function getWorkflowQueryStatus({
   isFetching: boolean;
 }): WorkflowQueryStatus {
   if (isFetching) {
-    return 'fetching';
+    return 'loading';
   }
 
   switch (queryStatus) {
@@ -19,6 +19,6 @@ export default function getWorkflowQueryStatus({
     case 'success':
       return 'success';
     case 'pending':
-      return 'pending';
+      return 'idle';
   }
 }

…ies-loader/hooks/use-workflow-queries.ts …ow-queries/hooks/use-workflow-queries.tssrc/views/workflow-queries/workflow-queries-loader/hooks/use-workflow-queries.ts renamed to src/views/workflow-queries/hooks/use-workflow-queries.ts src/views/workflow-queries/workflow-queries-loader/hooks/use-workflow-queries.ts renamed to src/views/workflow-queries/hooks/use-workflow-queries.ts

@@ -28,13 +28,18 @@ export default function useWorkflowQueries({
         RequestError,
         QueryWorkflowResponse,
         [
+          'workflow_query',
           { workflowId: string; runId: string; name: string },
           string | undefined,
         ]
       >
     >((name) => ({
-      queryKey: [{ workflowId, runId, name }, inputs[name]] as const,
-      queryFn: ({ queryKey: [{ name }, input] }) =>
+      queryKey: [
+        'workflow_query',
+        { workflowId, runId, name },
+        inputs[name],
+      ] as const,
+      queryFn: ({ queryKey: [_, { name }, input] }) =>
         request(
           `/api/domains/${domain}/${cluster}/workflows/${workflowId}/${runId}/query/${name}`,
           {

src/views/workflow-queries/workflow-queries-loader/workflow-queries-loader.constants.ts

@@ -4,8 +4,7 @@ export const styled = {
   ViewContainer: createStyled<'div', { $isError: boolean }>(
     'div',
     ({ $theme, $isError }) => ({
-      // TODO: revisit this when PageSection is fixed
-      minHeight: '50vh',
+      flex: '1 0 150px',
       alignSelf: 'stretch',
       display: 'flex',
       flexDirection: 'row',

src/views/workflow-queries/workflow-queries-loader/workflow-queries-loader.tsx

@@ -43,8 +43,6 @@ export default function WorkflowQueriesResultJson(props: Props) {
             content={() => <>Copied</>}
           >
             <Button
-              // TODO: Add overrides for the copy button so that
-              // it sticks to the top right when the window is resized
               onClick={() => {
                 copy(JSON.stringify(content, null, '\t'));
                 setShowTooltip(true);

src/views/workflow-queries/workflow-queries-result-json/workflow-queries-result-json.styles.ts

@@ -1,10 +1,20 @@
 import React from 'react';
 
-import { render } from '@/test-utils/rtl';
+import { render, screen } from '@/test-utils/rtl';
 
 import { type WorkflowQueryStatus } from '../../workflow-queries-tile/workflow-queries-tile.types';
 import WorkflowQueriesStatusIcon from '../workflow-queries-status-icon';
 
+jest.mock('react-icons/md', () => ({
+  ...jest.requireActual('react-icons/md'),
+  MdWarning: () => <div>Warning Icon</div>,
+  MdCheckCircle: () => <div>Circle Check Icon</div>,
+}));
+
+jest.mock('baseui/spinner', () => ({
+  Spinner: jest.fn(() => <div>Spinner</div>),
+}));
+
 describe(WorkflowQueriesStatusIcon.name, () => {
   afterEach(() => {
     jest.clearAllMocks();
@@ -13,33 +23,37 @@ describe(WorkflowQueriesStatusIcon.name, () => {
   const tests: Array<{
     name: string;
     status: WorkflowQueryStatus;
+    expected: string | null;
   }> = [
     {
-      name: 'should render correctly for fetching status',
-      status: 'fetching',
+      name: 'should render correctly for loading status',
+      status: 'loading',
+      expected: 'Spinner',
     },
     {
       name: 'should render correctly for success status',
       status: 'success',
+      expected: 'Circle Check Icon',
     },
     {
       name: 'should render correctly for error status',
       status: 'error',
+      expected: 'Warning Icon',
     },
     {
-      name: 'should render null for pending status',
-      status: 'pending',
+      name: 'should render null for idle status',
+      status: 'idle',
+      expected: null,
     },
   ];
 
   tests.forEach((test) => {
     it(test.name, () => {
-      const { container } = render(
-        <WorkflowQueriesStatusIcon status={test.status} />,
-        { isSnapshotTest: true }
-      );
+      render(<WorkflowQueriesStatusIcon status={test.status} />);
 
-      expect(container).toMatchSnapshot();
+      if (test.expected) {
+        expect(screen.getByText(test.expected)).toBeInTheDocument();
+      }
     });
   });
 });