[Due for payment 2025-02-18] Refactor actions in Travel.tsx

[cristipaval]

Coming from the following comments:

• Domain selection when enabling travel for workspaces with admins from multiple domains #55300 (comment)
• Domain selection when enabling travel for workspaces with admins from multiple domains #55300 (comment)
• Domain selection when enabling travel for workspaces with admins from multiple domains #55300 (comment)

Problem

The actions from Travel break the App guidelines and perform navigation, which should happen only from the views. They also break the naming convention for the acronyms.

Solution

Refactor the actions to comply with the App code style and guidelines.

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[Krishna2323]

🚨 Edited by proposal-police: This proposal was edited at 2025-01-25 01:14:15 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Refactor actions in Travel.tsx

What is the root cause of that problem?

Refactor

What changes do you think we should make in order to solve the problem?

• We should remove bookATrip from TripReservation Utils.
• Create a new function bookATrip inside EmptySearchView and ManageTrips.

    const [account] = useOnyx(ONYXKEYS.ACCOUNT);
    const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID);
    const [travelSettings] = useOnyx(ONYXKEYS.NVP_TRAVEL_SETTINGS);
    const [isSingleNewDotEntry] = useOnyx(ONYXKEYS.IS_SINGLE_NEW_DOT_ENTRY);

    const bookATrip = useCallback(() => {
        if (Str.isSMSLogin(account?.primaryLogin ?? '')) {
            setCTAErrorMessage(translate('travel.phoneError'));
            return;
        }
        const policy = PolicyUtils.getPolicy(activePolicyID);
        if (isEmptyObject(policy?.address)) {
            Navigation.navigate(ROUTES.WORKSPACE_PROFILE_ADDRESS.getRoute(activePolicyID, Navigation.getActiveRoute()));
            return;
        }
        if (!travelSettings?.hasAcceptedTerms) {
            Navigation.navigate(ROUTES.TRAVEL_TCS);
            return;
        }
        if (ctaErrorMessage) {
            setCTAErrorMessage('');
        }
        Link.openTravelDotLink(activePolicyID)
            ?.then(() => {
                if (!NativeModules.HybridAppModule || !isSingleNewDotEntry) {
                    return;
                }

                Log.info('[HybridApp] Returning to OldDot after opening TravelDot');
                NativeModules.HybridAppModule.closeReactNativeApp(false, false);
            })
            ?.catch(() => {
                setCTAErrorMessage(translate('travel.errorMessage'));
            });
    }, [account?.primaryLogin, activePolicyID, ctaErrorMessage, isSingleNewDotEntry, translate, travelSettings?.hasAcceptedTerms]);

• Replace the old bookATrip function with the new one inside EmptySearchView and ManageTrips.
• Update setCtaErrorMessage to setCTAErrorMessage in both components.

App/src/pages/Search/EmptySearchView.tsx

Line 64 in b59f23b

const [ctaErrorMessage, setCtaErrorMessage] = useState('');

App/src/pages/Travel/ManageTrips.tsx

Line 38 in b59f23b

const [ctaErrorMessage, setCtaErrorMessage] = useState('');

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore?

• We can just remove the logic for navigation from bookATrip and add that in the UI component before we call bookATrip.

App/src/libs/TripReservationUtils.ts

Lines 103 to 111 in b59f23b

	const policy = getPolicy(activePolicyID);
	if (isEmptyObject(policy?.address)) {
	Navigation.navigate(ROUTES.WORKSPACE_PROFILE_ADDRESS.getRoute(activePolicyID, Navigation.getActiveRoute()));
	return;
	}
	if (!travelSettings?.hasAcceptedTerms) {
	Navigation.navigate(ROUTES.TRAVEL_TCS);
	return;
	}

// In EmptySearchView and same should be done in other component.
                    buttons: [
                        {
                            buttonText: translate('search.searchResults.emptyTripResults.buttonText'),
                            buttonAction: () => {
                                if (isEmptyObject(policy?.address)) {
                                    Navigation.navigate(ROUTES.WORKSPACE_PROFILE_ADDRESS.getRoute(activePolicyID, Navigation.getActiveRoute()));
                                    return;
                                }
                                if (!travelSettings?.hasAcceptedTerms) {
                                    Navigation.navigate(ROUTES.TRAVEL_TCS);
                                    return;
                                }
                                TripsResevationUtils.bookATrip(translate, setCtaErrorMessage, ctaErrorMessage);
                            },
                            success: true,
                        },
                    ],

• If we are going with this option then we also need to change setCtaErrorMessage param to setCTAErrorMessage.
  

  App/src/libs/TripReservationUtils.ts

  Line 98 in b59f23b

  function bookATrip(translate: LocaleContextProps['translate'], setCtaErrorMessage: Dispatch<SetStateAction<string>>, ctaErrorMessage = ''): void {

[github-actions]

⚠️ @Krishna2323 Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

[Krishna2323]

PROPOSAL UPDATED

• Added alternative solution

[cristipaval]

This isn't external, at least for now. I'll come to this once I get the original PR merged.