diff --git a/src/components/Form/FormWrapper.tsx b/src/components/Form/FormWrapper.tsx index b6c63efe437e..51e33fc460b0 100644 --- a/src/components/Form/FormWrapper.tsx +++ b/src/components/Form/FormWrapper.tsx @@ -90,6 +90,7 @@ function FormWrapper({ submitFlexEnabled = true, enabledWhenOffline, buttonVariant, + isSubmitButtonNeutral = false, formID, shouldUseScrollView = true, scrollContextEnabled = false, @@ -217,6 +218,7 @@ function FormWrapper({ ]} enabledWhenOffline={enabledWhenOffline} buttonVariant={buttonVariant} + isButtonNeutral={isSubmitButtonNeutral} disablePressOnEnter={disablePressOnEnter} enterKeyEventListenerPriority={enterKeyEventListenerPriority} shouldRenderFooterAboveSubmit={shouldRenderFooterAboveSubmit} diff --git a/src/components/Form/types.ts b/src/components/Form/types.ts index 9f8687cdfdc3..7a9691271551 100644 --- a/src/components/Form/types.ts +++ b/src/components/Form/types.ts @@ -178,6 +178,9 @@ type FormProps = ForwardedFSClassProp /** The visual variant of the submit button, which controls its color scheme */ buttonVariant?: ButtonVariant; + /** Whether the submit button has the neutral color in place of `buttonVariant`, so that a button in `footerContent` can be the suggested action */ + isSubmitButtonNeutral?: boolean; + /** Should fix the errors alert be displayed when there is an error in the form */ shouldHideFixErrorsAlert?: boolean; diff --git a/src/components/FormAlertWithSubmitButton.tsx b/src/components/FormAlertWithSubmitButton.tsx index ef7c9afabe65..bebe455d288f 100644 --- a/src/components/FormAlertWithSubmitButton.tsx +++ b/src/components/FormAlertWithSubmitButton.tsx @@ -52,6 +52,9 @@ type FormAlertWithSubmitButtonProps = WithSentryLabel & { /** The visual variant of the submit button, which controls its color scheme */ buttonVariant?: ButtonVariant; + /** Whether the submit button has the neutral color in place of `buttonVariant` */ + isButtonNeutral?: boolean; + /** Custom content to display in the footer after submit button */ footerContent?: React.ReactNode; @@ -98,6 +101,7 @@ function FormAlertWithSubmitButton({ enabledWhenOffline = false, disablePressOnEnter = false, buttonVariant = CONST.BUTTON_VARIANT.SUCCESS, + isButtonNeutral = false, footerContent, buttonRef, buttonStyles, @@ -116,6 +120,7 @@ function FormAlertWithSubmitButton({ }: FormAlertWithSubmitButtonProps) { const styles = useThemeStyles(); const style = [!shouldRenderFooterAboveSubmit && footerContent && addButtonBottomPadding ? styles.mb3 : undefined, buttonStyles]; + const variant = isButtonNeutral ? undefined : buttonVariant; const {isLoading, startWithLoading} = usePressLoading({isLoading: isOnyxLoading}); @@ -147,7 +152,7 @@ function FormAlertWithSubmitButton({ {shouldRenderFooterAboveSubmit && footerContent} {isOffline && !enabledWhenOffline ? ( + ) + } // An empty description sends no request, so nothing would stop a loading state that starts on the press. shouldShowLoadingImmediatelyOnPress={!canSkipDescription} keyboardSubmitBehavior={CONST.KEYBOARD_SUBMIT_BEHAVIOR.SUBMIT_ONLY} @@ -338,23 +370,8 @@ function DescribeRuleForm({ {!!refusal && ( - {translate(IMPOSSIBLE_TITLE_BY_REASON[refusal.reason ?? ''] ?? 'workspace.rules.newRule.impossible.noCapability')} - {!!refusal.explanation && {refusal.explanation}} - {!!refusalSuggestedPrompt && ( - <> - {refusalSuggestedPrompt} - - - )} + {!!refusalDetail && {refusalDetail}} + {!!refusalSuggestedPrompt && {translate('workspace.rules.newRule.impossible.quotedSuggestion', refusalSuggestedPrompt)}} )} diff --git a/tests/ui/DescribeRuleModalTest.tsx b/tests/ui/DescribeRuleModalTest.tsx index 3852e9282481..781e8a7817ad 100644 --- a/tests/ui/DescribeRuleModalTest.tsx +++ b/tests/ui/DescribeRuleModalTest.tsx @@ -24,6 +24,7 @@ import {translateLocal} from '../utils/TestHelper'; import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; let mockPrompt = ''; +let mockIsSubmitButtonNeutral: boolean | undefined; jest.mock('@react-navigation/native', () => { const actualNavigation = jest.requireActual('@react-navigation/native'); @@ -36,7 +37,7 @@ jest.mock('@react-navigation/native', () => { }); jest.mock('@hooks/useLocalize', () => jest.fn(() => ({ - translate: (key: string) => key, + translate: (key: string, ...params: unknown[]) => [key, ...params].join(' '), })), ); jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false}))); @@ -81,17 +82,32 @@ jest.mock('@components/Form/FormProvider', () => { Pressable: React.ComponentType<{accessibilityLabel?: string; onPress?: () => void; children?: React.ReactNode}>; Text: React.ComponentType<{children?: React.ReactNode}>; }>('react-native'); - return ({children, onSubmit, submitButtonText}: {children: React.ReactNode; onSubmit: (values: {prompt: string}) => void; submitButtonText: string}) => - ReactModule.createElement( + return ({ + children, + onSubmit, + submitButtonText, + footerContent, + isSubmitButtonNeutral, + }: { + children: React.ReactNode; + onSubmit: (values: {prompt: string}) => void; + submitButtonText: string; + footerContent?: React.ReactNode; + isSubmitButtonNeutral?: boolean; + }) => { + mockIsSubmitButtonNeutral = isSubmitButtonNeutral; + return ReactModule.createElement( ReactModule.Fragment, null, children, + footerContent, ReactModule.createElement( Pressable, {accessibilityLabel: submitButtonText, onPress: () => onSubmit({prompt: mockPrompt})}, ReactModule.createElement(Text, null, submitButtonText), ), ); + }; }); const POLICY_ID = 'policy1'; @@ -266,9 +282,16 @@ describe('DescribeRuleModal', () => { // Then the modal stays open with the reason, the explanation and the suggestion, and nothing reaches the editor form expect(onRuleGenerated).not.toHaveBeenCalled(); expect(onClose).not.toHaveBeenCalled(); - expect(screen.getByText('workspace.rules.newRule.impossible.noCapability')).toBeOnTheScreen(); - expect(screen.getByText(IMPOSSIBLE_ANSWER.explanation)).toBeOnTheScreen(); - expect(screen.getByText(IMPOSSIBLE_ANSWER.suggestedPrompt)).toBeOnTheScreen(); + expect(screen.getByText(`workspace.rules.newRule.impossible.explanationWithSuggestion ${IMPOSSIBLE_ANSWER.explanation}`)).toBeOnTheScreen(); + expect(screen.queryByText('workspace.rules.newRule.impossible.noCapability')).toBeNull(); + expect(screen.getByText(`workspace.rules.newRule.impossible.quotedSuggestion ${IMPOSSIBLE_ANSWER.suggestedPrompt}`)).toBeOnTheScreen(); + + // And Use suggestion is the suggested action, so Next turns neutral + expect(screen.getByText('workspace.rules.newRule.impossible.useSuggestion')).toBeOnTheScreen(); + expect(mockIsSubmitButtonNeutral).toBe(true); + + // And the submit button keeps Next, because an editor has no agent rule to create anyway + expect(screen.getByLabelText('common.next')).toBeOnTheScreen(); // And the form stops loading without an error, because the answer is shown in the form const form = await getOnyxValue(ONYXKEYS.FORMS.NEW_RULE_PROMPT_FORM); @@ -276,15 +299,15 @@ describe('DescribeRuleModal', () => { expect(await getPromptErrors()).toEqual([]); }); - it('describes the suggested rule when the admin taps Try this instead', async () => { + it('describes the suggested rule when the admin taps Use suggestion', async () => { // Given Concierge answered that the description cannot be done and suggested another one const logSpy = jest.spyOn(Log, 'info'); await renderFlagForReviewModal(jest.fn(), jest.fn()); const generationID = describeRule(writeSpy, 'Text the approver when a report is submitted'); await answerGeneration({...IMPOSSIBLE_ANSWER, generationID}); - // When the admin taps Try this instead - fireEvent.press(screen.getByText('workspace.rules.newRule.impossible.tryThisInstead')); + // When the admin taps Use suggestion + fireEvent.press(screen.getByText('workspace.rules.newRule.impossible.useSuggestion')); await waitForBatchedUpdatesWithAct(); // Then Concierge gets the suggested description for the same editor, so the admin does not retype it @@ -294,17 +317,17 @@ describe('DescribeRuleModal', () => { expect(logSpy).toHaveBeenCalledWith('[GenerateRule] Tried the suggested description', false, {policyID: POLICY_ID, generationID, reason: IMPOSSIBLE_ANSWER.reason}); // And the refusal goes away, because it belongs to the earlier description - expect(screen.queryByText(IMPOSSIBLE_ANSWER.explanation)).toBeNull(); + expect(screen.queryByText(`workspace.rules.newRule.impossible.explanationWithSuggestion ${IMPOSSIBLE_ANSWER.explanation}`)).toBeNull(); logSpy.mockRestore(); }); - it('disables Try this instead and sends no request while offline', async () => { + it('disables Use suggestion and sends no request while offline', async () => { // Given Concierge answered that the description cannot be done and suggested another one await renderFlagForReviewModal(jest.fn(), jest.fn()); const generationID = describeRule(writeSpy, 'Text the approver when a report is submitted'); await answerGeneration({...IMPOSSIBLE_ANSWER, generationID}); - // When the admin goes offline and taps Try this instead + // When the admin goes offline and taps Use suggestion mockedUseNetwork.mockReturnValue({isOffline: true}); screen.rerender( { />, ); await waitForBatchedUpdatesWithAct(); - fireEvent.press(screen.getByText('workspace.rules.newRule.impossible.tryThisInstead')); + fireEvent.press(screen.getByText('workspace.rules.newRule.impossible.useSuggestion')); await waitForBatchedUpdatesWithAct(); // Then no new GenerateRule request is sent, because generating a rule needs a connection, like the form's own submit expect(getGenerateRuleRequests(writeSpy)).toHaveLength(1); - expect(screen.getByText(IMPOSSIBLE_ANSWER.explanation)).toBeOnTheScreen(); + expect(screen.getByText(`workspace.rules.newRule.impossible.explanationWithSuggestion ${IMPOSSIBLE_ANSWER.explanation}`)).toBeOnTheScreen(); }); - it('shows the general title and no suggestion when the reason is unknown and nothing is suggested', async () => { + it('shows the general text and no suggestion when the reason is unknown and nothing is explained or suggested', async () => { // Given the flag for review editor asked for a rule await renderFlagForReviewModal(jest.fn(), jest.fn()); const generationID = describeRule(writeSpy, 'Text the approver when a report is submitted'); - // When Concierge answers with a reason that this app version does not know, and with no suggestion + // When Concierge answers with a reason that this app version does not know, with no explanation and no suggestion await answerGeneration({ ...IMPOSSIBLE_ANSWER, generationID, + explanation: '', // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- simulates an answer from a server that knows more reasons than this app version reason: 'futureReason' as GeneratedRuleImpossibleReason, suggestedPrompt: '', }); - // Then the modal shows the general title and no Try this instead button, because there is nothing to try + // Then the modal still says why with the general text, and shows no Use suggestion button, because there is nothing to try expect(screen.getByText('workspace.rules.newRule.impossible.noCapability')).toBeOnTheScreen(); - expect(screen.queryByText('workspace.rules.newRule.impossible.tryThisInstead')).toBeNull(); + expect(screen.queryByText('workspace.rules.newRule.impossible.useSuggestion')).toBeNull(); + + // And Next stays the suggested action + expect(mockIsSubmitButtonNeutral).toBe(false); }); }); diff --git a/tests/ui/RulesNewPageDescribeRuleTest.tsx b/tests/ui/RulesNewPageDescribeRuleTest.tsx index 2e75ef86708d..2408fecddde1 100644 --- a/tests/ui/RulesNewPageDescribeRuleTest.tsx +++ b/tests/ui/RulesNewPageDescribeRuleTest.tsx @@ -49,7 +49,7 @@ jest.mock('@hooks/useLazyAsset', () => ({ })); jest.mock('@hooks/useLocalize', () => jest.fn(() => ({ - translate: (key: string) => key, + translate: (key: string, ...params: unknown[]) => [key, ...params].join(' '), })), ); jest.mock('@hooks/usePolicy', () => jest.fn()); @@ -463,12 +463,43 @@ describe('New rule describe section', () => { }); // Then New rule shows the reason and the suggestion, and nothing opens, because no rule can be drafted - expect(screen.getByText('workspace.rules.newRule.impossible.noCapability')).toBeOnTheScreen(); - expect(screen.getByText('Comment on the report when it is submitted')).toBeOnTheScreen(); + expect(screen.getByText('workspace.rules.newRule.impossible.explanationWithSuggestion Rules cannot send text messages.')).toBeOnTheScreen(); + expect(screen.getByText('workspace.rules.newRule.impossible.quotedSuggestion Comment on the report when it is submitted')).toBeOnTheScreen(); expect(Navigation.navigate).not.toHaveBeenCalled(); expect(await getOnyxValue(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM_DRAFT)).toBeUndefined(); }); + it('drafts the refused description as an agent rule when the admin taps Create anyway', async () => { + // Given Concierge refused a description on New rule and suggested another one + const logSpy = jest.spyOn(Log, 'info'); + const prompt = 'Text the approver when a report is submitted'; + await renderRulesNewPage(); + const {generationID} = describeRule(writeSpy, prompt); + await answerGeneration({ + generationID, + state: CONST.GENERATED_RULE.STATE.IMPOSSIBLE, + reason: CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_CAPABILITY, + explanation: 'Rules cannot send text messages.', + suggestedPrompt: 'Comment on the report when it is submitted', + }); + const generateRuleRequestCount = writeSpy.mock.calls.filter(([command]) => command === WRITE_COMMANDS.GENERATE_RULE).length; + + // When the admin keeps their own description and taps Create anyway, the button that replaces Next next to Use suggestion + fireEvent.press(screen.getByLabelText('workspace.rules.newRule.impossible.createAnyway')); + await waitForBatchedUpdatesWithAct(); + + // Then the description becomes the agent rule draft without another request to Concierge, because Concierge already answered it + expect(writeSpy.mock.calls.filter(([command]) => command === WRITE_COMMANDS.GENERATE_RULE)).toHaveLength(generateRuleRequestCount); + expect(await getOnyxValue(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM_DRAFT)).toEqual({prompt}); + expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.RULES_AGENT_NEW.getRoute(POLICY_ID, generationID)); + expect(logSpy).toHaveBeenCalledWith('[GenerateRule] Created an agent rule after a refusal', false, { + policyID: POLICY_ID, + generationID, + reason: CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_CAPABILITY, + }); + logSpy.mockRestore(); + }); + it('loads the policy categories when the workspace uses categories and none are loaded', async () => { // Given a workspace that uses categories, with no categories loaded setAdminPolicy(true);