Merge pull request Expensify#50700 from bernhardoj/fix/50577-inconsis…

…tent-money-request-and-task-behavior Fix inconsistent money request and task behavior
bernhardoj · Oct 19, 2024 · 66cf824 · 66cf824
2 parents f7d08e0 + 4d4edb1
commit 66cf824
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 65 deletions.
diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx
@@ -715,6 +715,7 @@ function MoneyRequestConfirmationList({
             }
 
             if (selectedParticipants.length === 0) {
+                setFormError('iou.error.noParticipantSelected');
                 return;
             }
             if (!isEditingSplitBill && isMerchantRequired && (isMerchantEmpty || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction)))) {
@@ -801,12 +802,10 @@ function MoneyRequestConfirmationList({
         }
 
         const shouldShowSettlementButton = iouType === CONST.IOU.TYPE.PAY;
-        const shouldDisableButton = selectedParticipants.length === 0;
 
         const button = shouldShowSettlementButton ? (
             <SettlementButton
                 pressOnEnter
-                isDisabled={shouldDisableButton}
                 onPress={confirm}
                 enablePaymentsRoute={ROUTES.IOU_SEND_ENABLE_PAYMENTS}
                 addBankAccountRoute={bankAccountRoute}
@@ -829,7 +828,6 @@ function MoneyRequestConfirmationList({
             <ButtonWithDropdownMenu
                 success
                 pressOnEnter
-                isDisabled={shouldDisableButton}
                 onPress={(event, value) => confirm(value as PaymentMethodType)}
                 options={splitOrRequestOptions}
                 buttonSize={CONST.DROPDOWN_BUTTON_SIZE.LARGE}
@@ -855,7 +853,6 @@ function MoneyRequestConfirmationList({
         isReadOnly,
         isTypeSplit,
         iouType,
-        selectedParticipants.length,
         confirm,
         bankAccountRoute,
         iouCurrencyCode,
@@ -926,6 +923,7 @@ function MoneyRequestConfirmationList({
                 shouldSingleExecuteRowSelect
                 canSelectMultiple={false}
                 shouldPreventDefaultFocusOnSelectRow
+                shouldShowListEmptyContent={false}
                 footerContent={footerContent}
                 listFooterContent={listFooterContent}
                 containerStyle={[styles.flexBasisAuto]}

diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx
@@ -718,7 +718,7 @@ function BaseSelectionList<TItem extends ListItem>(
                             </View>
                         )}
                     {!!headerContent && headerContent}
-                    {flattenedSections.allOptions.length === 0 ? (
+                    {flattenedSections.allOptions.length === 0 && (showLoadingPlaceholder || shouldShowListEmptyContent) ? (
                         renderListEmptyContent()
                     ) : (
                         <>

diff --git a/src/languages/en.ts b/src/languages/en.ts
@@ -955,6 +955,7 @@ const translations = {
             invalidSplit: 'The sum of splits must equal the total amount.',
             invalidSplitParticipants: 'Please enter an amount greater than zero for at least two participants.',
             invalidSplitYourself: 'Please enter a non-zero amount for your split.',
+            noParticipantSelected: 'Please select a participant.',
             other: 'Unexpected error. Please try again later.',
             genericCreateFailureMessage: 'Unexpected error submitting this expense. Please try again later.',
             genericCreateInvoiceFailureMessage: 'Unexpected error sending this invoice. Please try again later.',

diff --git a/src/languages/es.ts b/src/languages/es.ts
@@ -952,6 +952,7 @@ const translations = {
             invalidSplit: 'La suma de las partes debe ser igual al importe total.',
             invalidSplitParticipants: 'Introduce un importe superior a cero para al menos dos participantes.',
             invalidSplitYourself: 'Por favor, introduce una cantidad diferente de cero para tu parte.',
+            noParticipantSelected: 'Por favor, selecciona un participante.',
             other: 'Error inesperado. Por favor, inténtalo más tarde.',
             genericHoldExpenseFailureMessage: 'Error inesperado al bloquear el gasto. Por favor, inténtalo de nuevo más tarde.',
             genericUnholdExpenseFailureMessage: 'Error inesperado al desbloquear el gasto. Por favor, inténtalo de nuevo más tarde.',

diff --git a/src/pages/tasks/NewTaskPage.tsx b/src/pages/tasks/NewTaskPage.tsx
@@ -2,8 +2,7 @@ import {useFocusEffect} from '@react-navigation/native';
 import type {StackScreenProps} from '@react-navigation/stack';
 import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {InteractionManager, View} from 'react-native';
-import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
-import {withOnyx} from 'react-native-onyx';
+import {useOnyx} from 'react-native-onyx';
 import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
 import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
 import FormHelpMessage from '@components/FormHelpMessage';
@@ -27,35 +26,27 @@ import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
 import type SCREENS from '@src/SCREENS';
-import type {PersonalDetailsList, Report, Task} from '@src/types/onyx';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 
-type NewTaskPageOnyxProps = {
-    /** Task Creation Data */
-    task: OnyxEntry<Task>;
+type NewTaskPageProps = StackScreenProps<NewTaskNavigatorParamList, typeof SCREENS.NEW_TASK.ROOT>;
 
-    /** All of the personal details for everyone */
-    personalDetails: OnyxEntry<PersonalDetailsList>;
-
-    /** All reports shared with the user */
-    reports: OnyxCollection<Report>;
-};
-
-type NewTaskPageProps = NewTaskPageOnyxProps & StackScreenProps<NewTaskNavigatorParamList, typeof SCREENS.NEW_TASK.ROOT>;
-
-function NewTaskPage({task, reports, personalDetails, route}: NewTaskPageProps) {
+function NewTaskPage({route}: NewTaskPageProps) {
+    const [task] = useOnyx(ONYXKEYS.TASK);
+    const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
+    const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
     const styles = useThemeStyles();
     const {translate} = useLocalize();
-    const [assignee, setAssignee] = useState<TaskActions.Assignee>();
+    const assignee = useMemo(() => TaskActions.getAssignee(task?.assigneeAccountID ?? -1, personalDetails), [task?.assigneeAccountID, personalDetails]);
     const assigneeTooltipDetails = ReportUtils.getDisplayNamesWithTooltips(
         OptionsListUtils.getPersonalDetailsForAccountIDs(task?.assigneeAccountID ? [task.assigneeAccountID] : [], personalDetails),
         false,
     );
-    const [shareDestination, setShareDestination] = useState<TaskActions.ShareDestination>();
-    const [title, setTitle] = useState('');
-    const [description, setDescription] = useState('');
+    const shareDestination = useMemo(
+        () => (task?.shareDestination ? TaskActions.getShareDestination(task.shareDestination, reports, personalDetails) : undefined),
+        [task?.shareDestination, reports, personalDetails],
+    );
+    const parentReport = useMemo(() => (task?.shareDestination ? reports?.[`${ONYXKEYS.COLLECTION.REPORT}${task.shareDestination}`] : undefined), [reports, task?.shareDestination]);
     const [errorMessage, setErrorMessage] = useState('');
-    const [parentReport, setParentReport] = useState<OnyxEntry<Report>>();
 
     const hasDestinationError = task?.skipConfirmation && !task?.parentReportID;
     const isAllowedToCreateTask = useMemo(() => isEmptyObject(parentReport) || ReportUtils.isAllowedToComment(parentReport), [parentReport]);
@@ -79,38 +70,13 @@ function NewTaskPage({task, reports, personalDetails, route}: NewTaskPageProps)
     useEffect(() => {
         setErrorMessage('');
 
-        // If we have an assignee, we want to set the assignee data
-        // If there's an issue with the assignee chosen, we want to notify the user
-        if (task?.assignee) {
-            const displayDetails = TaskActions.getAssignee(task?.assigneeAccountID ?? -1, personalDetails);
-            setAssignee(displayDetails);
-        }
-
         // We only set the parentReportID if we are creating a task from a report
         // this allows us to go ahead and set that report as the share destination
         // and disable the share destination selector
         if (task?.parentReportID) {
             TaskActions.setShareDestinationValue(task.parentReportID);
         }
-
-        // If we have a share destination, we want to set the parent report and
-        // the share destination data
-        if (task?.shareDestination) {
-            setParentReport(reports?.[`report_${task.shareDestination}`]);
-            const displayDetails = TaskActions.getShareDestination(task.shareDestination, reports, personalDetails);
-            setShareDestination(displayDetails);
-        }
-
-        // If we have a title, we want to set the title
-        if (task?.title !== undefined) {
-            setTitle(task.title);
-        }
-
-        // If we have a description, we want to set the description
-        if (task?.description !== undefined) {
-            setDescription(task.description);
-        }
-    }, [personalDetails, reports, task?.assignee, task?.assigneeAccountID, task?.description, task?.parentReportID, task?.shareDestination, task?.title]);
+    }, [task?.assignee, task?.assigneeAccountID, task?.description, task?.parentReportID, task?.shareDestination, task?.title]);
 
     // On submit, we want to call the createTask function and wait to validate
     // the response
@@ -179,14 +145,14 @@ function NewTaskPage({task, reports, personalDetails, route}: NewTaskPageProps)
                         <View style={styles.mb5}>
                             <MenuItemWithTopDescription
                                 description={translate('task.title')}
-                                title={title}
+                                title={task?.title}
                                 onPress={() => Navigation.navigate(ROUTES.NEW_TASK_TITLE.getRoute(backTo))}
                                 shouldShowRightIcon
                                 rightLabel={translate('common.required')}
                             />
                             <MenuItemWithTopDescription
                                 description={translate('task.description')}
-                                title={description}
+                                title={task?.description}
                                 onPress={() => Navigation.navigate(ROUTES.NEW_TASK_DESCRIPTION.getRoute(backTo))}
                                 shouldShowRightIcon
                                 shouldParseTitle
@@ -234,14 +200,4 @@ function NewTaskPage({task, reports, personalDetails, route}: NewTaskPageProps)
 
 NewTaskPage.displayName = 'NewTaskPage';
 
-export default withOnyx<NewTaskPageProps, NewTaskPageOnyxProps>({
-    task: {
-        key: ONYXKEYS.TASK,
-    },
-    reports: {
-        key: ONYXKEYS.COLLECTION.REPORT,
-    },
-    personalDetails: {
-        key: ONYXKEYS.PERSONAL_DETAILS_LIST,
-    },
-})(NewTaskPage);
+export default NewTaskPage;