-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor actions in Travel.tsx #55755
Comments
Triggered auto assignment to @strepanier03 ( |
🚨 Edited by proposal-police: This proposal was edited at 2025-01-25 01:14:15 UTC. ProposalPlease 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?
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]);
App/src/pages/Search/EmptySearchView.tsx Line 64 in b59f23b
App/src/pages/Travel/ManageTrips.tsx Line 38 in b59f23b
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?NA What alternative solutions did you explore?
App/src/libs/TripReservationUtils.ts Lines 103 to 111 in b59f23b
// 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,
},
],
|
|
|
Coming from the following comments:
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.
The text was updated successfully, but these errors were encountered: