From 7e49d3ba79ac4df32db62d5f26259530cb5e1595 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 30 Apr 2024 15:02:54 +0200 Subject: [PATCH 1/2] feat: disable the query bar controls while GenAI is running COMPASS-7839 --- .../pipeline-header/pipeline-actions.spec.tsx | 31 +++ .../pipeline-header/pipeline-actions.tsx | 11 +- .../src/components/option-editor.tsx | 3 + .../src/components/query-bar-row.tsx | 4 + .../src/components/query-bar.spec.tsx | 180 +++++++++++++++--- .../src/components/query-bar.tsx | 36 ++-- .../src/components/query-option.tsx | 5 + 7 files changed, 223 insertions(+), 47 deletions(-) diff --git a/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.spec.tsx b/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.spec.tsx index ee3cf3165b4..d9dbde2a2f4 100644 --- a/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.spec.tsx +++ b/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.spec.tsx @@ -13,6 +13,7 @@ import { createSandboxFromDefaultPreferences, } from 'compass-preferences-model'; import { PreferencesProvider } from 'compass-preferences-model/provider'; +import { AIPipelineActionTypes } from '../../../modules/pipeline-builder/pipeline-ai'; describe('PipelineActions', function () { afterEach(cleanup); @@ -275,6 +276,36 @@ describe('PipelineActions', function () { ).to.equal('true'); }); + it('should disable actions while ai is fetching', function () { + const { store, rerender } = renderPipelineActions({ + pipeline: [{ $match: { _id: 1 } }], + }); + + store.dispatch({ + type: AIPipelineActionTypes.AIPipelineStarted, + requestId: 'pineapples', + }); + rerender(); + + expect( + screen + .getByTestId('pipeline-toolbar-explain-aggregation-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + + expect( + screen + .getByTestId('pipeline-toolbar-export-aggregation-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + + expect( + screen + .getByTestId('pipeline-toolbar-run-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + }); + it('should disable export button when pipeline is $out / $merge', function () { renderPipelineActions({ pipeline: [{ $out: 'foo' }], diff --git a/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.tsx b/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.tsx index e64f7d942f3..5207541ae34 100644 --- a/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.tsx +++ b/packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.tsx @@ -170,17 +170,20 @@ const mapState = (state: RootState) => { const isMergeOrOutPipeline = isOutputStage(lastStage); const hasSyntaxErrors = getIsPipelineInvalidFromBuilderState(state, false); const isBuilderView = state.workspace === 'builder'; + const isAIFetching = state.pipelineBuilder.aiPipeline.status === 'fetching'; return { - isRunButtonDisabled: hasSyntaxErrors, - isExplainButtonDisabled: hasSyntaxErrors, - isExportButtonDisabled: isMergeOrOutPipeline || hasSyntaxErrors, + isRunButtonDisabled: hasSyntaxErrors || isAIFetching, + isExplainButtonDisabled: hasSyntaxErrors || isAIFetching, + isExportButtonDisabled: + isMergeOrOutPipeline || hasSyntaxErrors || isAIFetching, showAIEntry: !state.pipelineBuilder.aiPipeline.isInputVisible && resultPipeline.length > 0 && isBuilderView, showUpdateViewButton: Boolean(state.editViewName), - isUpdateViewButtonDisabled: !state.isModified || hasSyntaxErrors, + isUpdateViewButtonDisabled: + !state.isModified || hasSyntaxErrors || isAIFetching, showCollectionScanInsight: state.insights.isCollectionScan, }; }; diff --git a/packages/compass-query-bar/src/components/option-editor.tsx b/packages/compass-query-bar/src/components/option-editor.tsx index 779eb8f26f4..394643f4ebc 100644 --- a/packages/compass-query-bar/src/components/option-editor.tsx +++ b/packages/compass-query-bar/src/components/option-editor.tsx @@ -93,6 +93,7 @@ type OptionEditorProps = { value?: string; ['data-testid']?: string; insights?: Signal | Signal[]; + disabled?: boolean; }; export const OptionEditor: React.FunctionComponent = ({ @@ -108,6 +109,7 @@ export const OptionEditor: React.FunctionComponent = ({ value = '', ['data-testid']: dataTestId, insights, + disabled = false, }) => { const showInsights = usePreference('showInsights'); const editorContainerRef = useRef(null); @@ -199,6 +201,7 @@ export const OptionEditor: React.FunctionComponent = ({ onFocus={onFocus} onPaste={onPaste} onBlur={onBlur} + readOnly={disabled} /> {showInsights && insights && (
diff --git a/packages/compass-query-bar/src/components/query-bar-row.tsx b/packages/compass-query-bar/src/components/query-bar-row.tsx index a2e8c9d5850..0dbed1ddff8 100644 --- a/packages/compass-query-bar/src/components/query-bar-row.tsx +++ b/packages/compass-query-bar/src/components/query-bar-row.tsx @@ -19,12 +19,14 @@ type QueryBarRowProps = { queryOptionsLayout: QueryBarRowLayout; onApply?(): void; placeholders?: Record; + disabled?: boolean; }; export const QueryBarRow: React.FunctionComponent = ({ queryOptionsLayout, onApply, placeholders, + disabled, }) => { return (
@@ -35,6 +37,7 @@ export const QueryBarRow: React.FunctionComponent = ({ id={`query-bar-option-input-${queryOptionsLayout}`} onApply={onApply} placeholder={placeholders?.[queryOptionsLayout]} + disabled={disabled} /> ) : ( queryOptionsLayout.map((optionName: QueryOption) => ( @@ -44,6 +47,7 @@ export const QueryBarRow: React.FunctionComponent = ({ id={`query-bar-option-input-${optionName}`} onApply={onApply} placeholder={placeholders?.[optionName]} + disabled={disabled} /> )) )} diff --git a/packages/compass-query-bar/src/components/query-bar.spec.tsx b/packages/compass-query-bar/src/components/query-bar.spec.tsx index f1b566b0cb8..424966e3577 100644 --- a/packages/compass-query-bar/src/components/query-bar.spec.tsx +++ b/packages/compass-query-bar/src/components/query-bar.spec.tsx @@ -1,6 +1,6 @@ import React from 'react'; import type { ComponentProps } from 'react'; -import { cleanup, render, screen } from '@testing-library/react'; +import { cleanup, render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { expect } from 'chai'; import sinon from 'sinon'; @@ -10,6 +10,7 @@ import { Provider } from '../stores/context'; import { configureStore } from '../stores/query-bar-store'; import type { QueryBarExtraArgs, RootState } from '../stores/query-bar-store'; import { toggleQueryOptions } from '../stores/query-bar-reducer'; +import { AIQueryActionTypes } from '../stores/ai-query-reducer'; import type { PreferencesAccess } from 'compass-preferences-model'; import { createSandboxFromDefaultPreferences } from 'compass-preferences-model'; import { mapQueryToFormFields } from '../utils/query'; @@ -29,7 +30,6 @@ const noop = () => { /* no op */ }; -const exportToLanguageButtonId = 'query-bar-open-export-to-language-button'; const queryHistoryButtonId = 'query-history-button'; const queryHistoryComponentTestId = 'query-history'; @@ -51,7 +51,7 @@ describe('QueryBar Component', function () { } as QueryBarExtraArgs); store.dispatch(toggleQueryOptions(expanded)); - render( + const component = ( @@ -60,7 +60,6 @@ describe('QueryBar Component', function () { buttonLabel="Apply" onApply={noop} onReset={noop} - showExportToLanguageButton resultId="123" {...props} /> @@ -69,6 +68,16 @@ describe('QueryBar Component', function () { ); + + const result = render(component); + + return { + ...result, + store, + rerender: () => { + result.rerender(component); + }, + }; }; beforeEach(async function () { @@ -84,7 +93,6 @@ describe('QueryBar Component', function () { renderQueryBar({ onApply: onApplySpy, onReset: onResetSpy, - showExportToLanguageButton: true, }); }); @@ -126,23 +134,158 @@ describe('QueryBar Component', function () { }); }); - describe('with one query option', function () { + describe('when rendered', function () { beforeEach(function () { renderQueryBar({ - queryOptionsLayout: ['project'], - expanded: true, onApply: onApplySpy, onReset: onResetSpy, }); }); - it('renders the expanded inputs', function () { + it('renders the filter input', function () { + const filterInput = screen.getByTestId('query-bar-option-filter-input'); + expect(filterInput).to.exist; + expect(filterInput).to.have.attribute( + 'id', + 'query-bar-option-input-filter' + ); + const queryInputs = screen.getAllByRole('textbox'); - expect(queryInputs.length).to.equal(2); + expect(queryInputs.length).to.equal(1); + }); + + it('renders the query history button', function () { + const queryHistoryButton = screen.queryByTestId(queryHistoryButtonId); + expect(queryHistoryButton).to.be.visible; + }); + + it('does not render the query history popover', function () { + const queryHistory = screen.queryByTestId(queryHistoryComponentTestId); + expect(queryHistory).to.not.exist; }); }); - describe('with ai enabled', function () { + describe('when ai is ready', function () { + beforeEach(function () { + renderQueryBar( + { + queryOptionsLayout: ['project'], + expanded: true, + onApply: onApplySpy, + onReset: onResetSpy, + }, + {} + ); + }); + + it('query controls are enabled', function () { + expect( + screen + .getByTestId('query-bar-open-export-to-language-button') + .getAttribute('aria-disabled') + ).to.equal('false'); + expect( + screen + .getByTestId('query-bar-apply-filter-button') + .getAttribute('aria-disabled') + ).to.equal('false'); + expect( + screen + .getByTestId('query-bar-open-export-to-language-button') + .getAttribute('aria-disabled') + ).to.equal('false'); + expect( + screen + .getByTestId('query-bar-open-export-to-language-button') + .getAttribute('aria-disabled') + ).to.equal('false'); + }); + + it('editors are not readonly', function () { + expect( + within(screen.getByTestId('query-bar-option-filter-input')) + .getByRole('textbox') + .getAttribute('aria-readonly') + ).to.not.exist; + expect( + within(screen.getByTestId('query-bar-option-project-input')) + .getByRole('textbox') + .getAttribute('aria-readonly') + ).to.not.exist; + }); + }); + + describe('while ai is fetching', function () { + it('query controls are disabled', function () { + const { store, rerender } = renderQueryBar( + { + queryOptionsLayout: ['project'], + expanded: true, + onApply: onApplySpy, + onReset: onResetSpy, + }, + {} + ); + + store.dispatch({ + type: AIQueryActionTypes.AIQueryStarted, + requestId: 'pineapples', + }); + rerender(); + + expect( + screen + .getByTestId('query-bar-open-export-to-language-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + expect( + screen + .getByTestId('query-bar-apply-filter-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + expect( + screen + .getByTestId('query-bar-open-export-to-language-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + expect( + screen + .getByTestId('query-bar-open-export-to-language-button') + .getAttribute('aria-disabled') + ).to.equal('true'); + }); + + it('editors are readonly', function () { + const store = configureStore({}, { + preferences, + logger: createNoopLoggerAndTelemetry(), + } as QueryBarExtraArgs); + + store.dispatch({ + type: AIQueryActionTypes.AIQueryStarted, + requestId: 'pineapples', + }); + + render( + + + + ); + + expect( + within(screen.getByTestId('query-bar-option-filter-input')) + .getByRole('textbox') + .getAttribute('aria-readonly') + ).to.equal('true'); + }); + }); + + describe('with ai is enabled', function () { beforeEach(async function () { await preferences.savePreferences({ enableGenAIFeatures: true, @@ -207,21 +350,6 @@ describe('QueryBar Component', function () { }); }); - describe('when showExportToLanguageButton is false', function () { - beforeEach(function () { - renderQueryBar({ - showExportToLanguageButton: false, - }); - }); - - it('does not render the exportToLanguage button', function () { - const exportToLanguageButton = screen.queryByTestId( - exportToLanguageButtonId - ); - expect(exportToLanguageButton).to.not.exist; - }); - }); - describe('with three query options', function () { beforeEach(function () { renderQueryBar({ diff --git a/packages/compass-query-bar/src/components/query-bar.tsx b/packages/compass-query-bar/src/components/query-bar.tsx index bf1456aa700..bf6fc129cd7 100644 --- a/packages/compass-query-bar/src/components/query-bar.tsx +++ b/packages/compass-query-bar/src/components/query-bar.tsx @@ -119,13 +119,13 @@ type QueryBarProps = { applyId: number; filterHasContent: boolean; showExplainButton?: boolean; - showExportToLanguageButton?: boolean; valid: boolean; expanded: boolean; placeholders?: Record; onExplain?: () => void; insights?: Signal | Signal[]; isAIInputVisible?: boolean; + isAIFetching?: boolean; onShowAIInputClick: () => void; onHideAIInputClick: () => void; }; @@ -147,13 +147,13 @@ export const QueryBar: React.FunctionComponent = ({ applyId, filterHasContent, showExplainButton = false, - showExportToLanguageButton = true, valid: isQueryValid, expanded: isQueryOptionsExpanded, placeholders, onExplain, insights, isAIInputVisible = false, + isAIFetching = false, onShowAIInputClick, onHideAIInputClick, }) => { @@ -230,6 +230,7 @@ export const QueryBar: React.FunctionComponent = ({ onApply={onApply} placeholder={filterPlaceholder} insights={insights} + disabled={isAIFetching} /> {showAIEntryButton && (
@@ -247,7 +248,7 @@ export const QueryBar: React.FunctionComponent = ({ title="View the execution plan for the current query" data-testid="query-bar-explain-button" onClick={onExplain} - disabled={!isQueryValid} + disabled={!isQueryValid || isAIFetching} size="small" type="button" > @@ -258,7 +259,7 @@ export const QueryBar: React.FunctionComponent = ({ aria-label="Reset query" data-testid="query-bar-reset-filter-button" onClick={onReset} - disabled={!queryChanged} + disabled={!queryChanged || isAIFetching} size="small" type="button" > @@ -266,7 +267,7 @@ export const QueryBar: React.FunctionComponent = ({ - {showExportToLanguageButton && ( - - )} + {queryOptionsLayout && queryOptionsLayout.length > 0 && (
@@ -308,6 +308,7 @@ export const QueryBar: React.FunctionComponent = ({ queryOptionsLayout={queryOptionRowLayout} key={`query-bar-row-${rowIndex}`} onApply={onApply} + disabled={isAIFetching} placeholders={placeholders} /> ))} @@ -331,6 +332,7 @@ export default connect( valid: isQueryValid(fields), applyId: applyId, isAIInputVisible: aiQuery.isInputVisible, + isAIFetching: aiQuery.status === 'fetching', }; }, (dispatch: QueryBarThunkDispatch, ownProps: OwnProps) => { diff --git a/packages/compass-query-bar/src/components/query-option.tsx b/packages/compass-query-bar/src/components/query-option.tsx index f2580618d28..c4ab84b7f60 100644 --- a/packages/compass-query-bar/src/components/query-option.tsx +++ b/packages/compass-query-bar/src/components/query-option.tsx @@ -87,6 +87,7 @@ type QueryOptionProps = { placeholder?: string | HTMLElement; onApply?(): void; insights?: Signal | Signal[]; + disabled?: boolean; }; // Helper component to allow flexible computation of extra props for the TextInput @@ -118,6 +119,7 @@ const QueryOption: React.FunctionComponent = ({ value, onApply, insights, + disabled = false, }) => { const { track } = useLoggerAndTelemetry('COMPASS-QUERY-BAR-UI'); const darkMode = useDarkMode(); @@ -169,6 +171,7 @@ const QueryOption: React.FunctionComponent = ({