Add validation to requestCall page and add policy check by parasharrajat · Pull Request #5169 · Expensify/App

parasharrajat · 2021-09-09T19:20:37Z

Details

#4861 (comment)

Fixed Issues

$ #4861

Tests | QA Steps

Go to staging.new.expensify.com
Login with any account
Search for Concierge
Click on the Call icon
Don't write anything on the phone number and click call.
Nothing should happen.
Enter a phone number and click call.

Tested On

Screenshots

Web | Mobile Web | Desktop

iOS

Android

parasharrajat · 2021-09-09T19:21:51Z

@Jag96 Please let me know if you want to add the Phone number validation on the frontend as well.

Jag96 · 2021-09-09T19:30:39Z

@parasharrajat yes I think we should add the phone number validation you mentioned in your proposal to this as well 👍

parasharrajat · 2021-09-09T19:49:12Z

@Jag96 one Question:
What should I do?

I can disable the button until the phone is valid.
I can let the button enabled for invalid phone and validate the number on submit and then show red border around the input with a message Please provide a valid phone number.
I can show the red border as soon as the user starts typing and shown, until the entered number is not correct. I don't feel this is right as another part of the app does not follow this.

Jag96 · 2021-09-09T20:16:24Z

Good question, commented in slack to see what the best way to handle this would be.

parasharrajat · 2021-09-09T20:52:57Z

@Jag96 Updated. Please let me know the error message and its translation in Spanish.

Jag96 · 2021-09-09T21:12:30Z

@parasharrajat let's use this one:

App/src/languages/en.js

Line 636 in dad9ddd

    
           growlMessageInvalidPhone: 'That doesn’t look like a valid phone number. Try again with the country code.\ne.g. +15005550006',

We no longer show it in a growl, so maybe update the name of it to errorMessageInvalidPhone instead

parasharrajat · 2021-09-09T21:28:44Z

Sure. thanks

parasharrajat · 2021-09-09T21:39:09Z

Updated.

Jag96

Based on the updates in slack can you update this to use the solution in step 2 of #5169 (comment) so the submit button isn't disabled?

parasharrajat · 2021-09-10T21:11:37Z

Ok.
Now, How should we manage the behavior?

Button is always active.
User enters wrong phone, we show That doesn’t look like a valid phone number. Try again with the country code. e.g. +15005550006 error.
But what if user tries to submit without any phone number, Will the error be shown? if so, what will be the error message?
There are first and last name fields, Growl is shown when any of them is empty. Is this the correct behavior?

Jag96 · 2021-09-10T21:25:37Z

Button is always active.

👍

User enters wrong phone, we show That doesn’t look like a valid phone number. Try again with the country code. e.g. +15005550006 error.

👍

But what if user tries to submit without any phone number, Will the error be shown? if so, what will be the error message?

We can show an error and use

App/src/languages/en.js

Line 428 in e33fcf7

    
           noPhoneNumber: 'Please enter a phone number including the country code e.g +447814266907',

if the phone number is blank on submit.

There are first and last name fields, Growl is shown when any of them is empty. Is this the correct behavior?

Yes, that's using a different component so I'd leave that functionality the same for this PR. It looks like the growl is using success though (Growl.success(this.props.translate('requestCallPage.growlMessageEmptyName'))), can you update it to use error instead?

parasharrajat · 2021-09-10T21:47:46Z

@Jag96 Updated

Jag96 · 2021-09-10T22:11:54Z

+                                        this.setState({phoneNumberError: this.props.translate('requestCallPage.errorMessageInvalidPhone')});
+                                    }
+                                }}
+                                onChangeText={phoneNumber => this.setState({phoneNumber, phoneNumberError: ''})}


It looks like clearing the phoneNumberError here is causing the error message to disappear as soon as the user starts typing, so the user can't see the phone number example as they type. Can you set the phoneNumberError to empty inside the onSubmit function instead so the error message remains until they resubmit?

parasharrajat · 2021-09-10T22:24:39Z

Done

Jag96

LGTM, thanks for updating!

OSBotify · 2021-09-10T23:12:33Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-09-13T19:26:38Z

🚀 Deployed to staging by @Jag96 in version: 1.0.97-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

isagoico · 2021-09-13T22:28:04Z

@parasharrajat @Jag96 is this change behind a beta? This is working for us in expensifail accounts only. Gmail accounts are giving us an error and the PR is a fail.

parasharrajat · 2021-09-14T08:50:02Z

I was using private email and I will test using Gmail email.

isagoico · 2021-09-14T14:43:27Z

@parasharrajat Should these tests pass with a gmail too? or is it expected that an error is shown there?

parasharrajat · 2021-09-14T16:06:26Z

@isagoico For me, I don't see any error till step 6.

On 7, I see which makes sense, I don't have any workspaces for this account.

Jag96 · 2021-09-14T21:52:54Z

@isagoico the free plan is still under beta, so if the user isn't on the beta they will see this error message. Gmail users won't be on the beta so it is normal for them to see this error.

isagoico · 2021-09-14T22:07:28Z

Ohh ok got it. Thanks! Looks like we cab check it off then 🎉

botify · 2021-09-15T20:05:02Z

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-22. 🎊

OSBotify · 2021-09-15T20:25:48Z

🚀 Deployed to production by @francoisl in version: 1.0.98-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

…y#5169

add validation to requestCall page and add policy check

943303a

parasharrajat requested a review from a team as a code owner September 9, 2021 19:20

MelvinBot requested review from Jag96 and removed request for a team September 9, 2021 19:20

update validations for requestCallPage

5fec12f

update translation

39de406

Jag96 reviewed Sep 10, 2021

View reviewed changes

form update

3a1ac0c

Jag96 suggested changes Sep 10, 2021

View reviewed changes

update

c7090f4

parasharrajat force-pushed the request-call branch from dcb5fba to c7090f4 Compare September 10, 2021 22:24

Jag96 approved these changes Sep 10, 2021

View reviewed changes

Jag96 merged commit 193ebc9 into Expensify:main Sep 10, 2021

OSBotify mentioned this pull request Sep 13, 2021

Deploy Checklist: New Expensify 2021-09-13 #5224

Closed

14 tasks

rayane-d added a commit to rayane-d/z that referenced this pull request Oct 8, 2023

bugfix/react-native-reanimated: Fix animated UI Props on Web Expensif…

6647b25

…y#5169

parasharrajat deleted the request-call branch November 20, 2023 13:07

Conversation

parasharrajat commented Sep 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests | QA Steps

Tested On

Screenshots

Web | Mobile Web | Desktop

iOS

Android

Uh oh!

parasharrajat commented Sep 9, 2021

Uh oh!

Jag96 commented Sep 9, 2021

Uh oh!

parasharrajat commented Sep 9, 2021

Uh oh!

Jag96 commented Sep 9, 2021

Uh oh!

parasharrajat commented Sep 9, 2021

Uh oh!

Jag96 commented Sep 9, 2021

Uh oh!

parasharrajat commented Sep 9, 2021

Uh oh!

parasharrajat commented Sep 9, 2021

Uh oh!

Jag96 left a comment

Choose a reason for hiding this comment

Uh oh!

parasharrajat commented Sep 10, 2021

Uh oh!

Jag96 commented Sep 10, 2021

Uh oh!

parasharrajat commented Sep 10, 2021

Uh oh!

Jag96 Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

parasharrajat Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

parasharrajat commented Sep 10, 2021

Uh oh!

Jag96 left a comment

Choose a reason for hiding this comment

Uh oh!

OSBotify commented Sep 10, 2021

Uh oh!

OSBotify commented Sep 13, 2021

Uh oh!

isagoico commented Sep 13, 2021

Uh oh!

parasharrajat commented Sep 14, 2021

Uh oh!

isagoico commented Sep 14, 2021

Uh oh!

parasharrajat commented Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jag96 commented Sep 14, 2021

Uh oh!

isagoico commented Sep 14, 2021

Uh oh!

botify commented Sep 15, 2021

Uh oh!

OSBotify commented Sep 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

parasharrajat commented Sep 9, 2021 •

edited

Loading

parasharrajat commented Sep 14, 2021 •

edited

Loading