Skip to content
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.

Sign up for GitHub

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

Prevent travel booking for SMS logins #48180

Merged
merged 10 commits into from
Sep 3, 2024
Merged

Prevent travel booking for SMS logins #48180

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5ddc97a
Display error
Gonals Aug 28, 2024
a9a3875
correctly show error
Gonals Aug 28, 2024
d62d476
style
Gonals Aug 28, 2024
8fe9f7c
spnish
Gonals Aug 28, 2024
6f3c395
typescript
Gonals Aug 28, 2024
e387323
prettier
Gonals Aug 28, 2024
f8014c3
new message
Gonals Aug 28, 2024
b384936
Merge branch 'main' into alberto-travelSMS
Gonals Sep 2, 2024
1bbf430
Add margin
Gonals Sep 2, 2024
968e757
smaller margin
Gonals Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2107,6 +2107,7 @@ export default {
tripSummary: 'Trip summary',
departs: 'Departs',
errorMessage: 'Something went wrong. Please try again later.',
phoneError: 'To book travel, your primary login must be a valid email',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we link them in the error to the place where they can update this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe! Giving it a shot.
I'm also considering rephrasing this, as we call it "Default contact method" in newdot, istead of "primary login" like we do in oldDot. What do you think?

Copy link
Contributor Author

@Gonals Gonals Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it can be done, but we don't really have an existing structure for it. We could do something similar to what we do for receipt errors in DotIndicatorMessage, but it would be, basically, writing another exception for this specific error. Since we have other similar errors that don't really link anywhere, I don't think it is worth it for this case.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make for a better, more useful error message if we did. Perhaps we have a contributor do it as follow-up?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about this @Gonals?

},
workspace: {
common: {
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2138,6 +2138,7 @@ export default {
tripSummary: 'Resumen del viaje',
departs: 'Sale',
errorMessage: 'Ha ocurrido un error. Por favor, inténtalo mas tarde.',
phoneError: 'Para reservar viajes, tu login principal debe ser un correo electrónico válido',
},
workspace: {
common: {
Expand Down
6 changes: 6 additions & 0 deletions src/pages/Travel/ManageTrips.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import {Str} from 'expensify-common';

const tripsFeatures: FeatureListItem[] = [
{
Expand All @@ -35,6 +36,7 @@
const {translate} = useLocalize();
const [travelSettings] = useOnyx(ONYXKEYS.NVP_TRAVEL_SETTINGS);
const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID);
const [account] = useOnyx(ONYXKEYS.ACCOUNT);
const policy = usePolicy(activePolicyID);

const [ctaErrorMessage, setCtaErrorMessage] = useState('');
Expand All @@ -60,6 +62,10 @@
ctaText={translate('travel.bookTravel')}
ctaAccessibilityLabel={translate('travel.bookTravel')}
onCtaPress={() => {
if (Str.isSMSLogin(account.primaryLogin)) {

Check failure on line 65 in src/pages/Travel/ManageTrips.tsx

View workflow job for this annotation

GitHub Actions / typecheck 

'account' is possibly 'undefined'.

Check failure on line 65 in src/pages/Travel/ManageTrips.tsx

View workflow job for this annotation

GitHub Actions / typecheck 

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
setCtaErrorMessage(translate('travel.phoneError'));
return;
}
if (!hasPolicyAddress) {
Navigation.navigate(ROUTES.WORKSPACE_PROFILE_ADDRESS.getRoute(activePolicyID ?? '-1'));
return;
Expand Down
Loading