Fixed address clear detected & modified the state #5954

Santhosh-Sellavel · 2021-10-19T23:24:28Z

Details

If the address was cleared in VBA Company step but address states was not cleared technically, updated the code that will clear the address states when the address is modified.

Fixed Issues

$ #5920

Tests & QA Steps

Go to https://staging.new.expensify.com
Log in with expensifail account
Go to Workspace and Click on Connect Bank account
Log in with Chase credentials
In the company info page select any autofill address and clear the address
Fill in rest of the fields and hit save and continue
Will show error now.

Tested On

Screenshots

Web

Mobile Web

Desktop

DESKTOP.mov

iOS

iOS_01.mp4

Android

Santhosh-Sellavel · 2021-10-19T23:31:29Z

@stitesExpensify had made some additional changes apart from the proposed changes. To ensure everything works well.

Adding onChangeText: () => saveLocationDetails({}), this alone creates few unexpected behaviour
resets saved address while reloading the page going back to previous step coming back to current step.

roryabraham · 2021-10-20T01:05:45Z

Something weird happened here and I think @stitesExpensify should've been assigned as a reviewer here because he's the CME on the linked issue. Also since this fixes a deploy blocker I'm assigning the CP Staging label.

github-actions · 2021-10-20T01:06:31Z

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

roryabraham · 2021-10-20T01:20:39Z

src/components/AddressSearch.js

+                onChangeText: (text) => {
+                    // Ensure whether an address is selected already has address value initialized.
+                    if (!_.isEmpty(googlePlacesRef.current.getAddressText())) {
+                        if (_.isEmpty(text) || !_.isEqual(text, props.value)) {


Sorry, I'm confused why we're clearing the value if text is not the same as props.value. Can you help me understand?

Also NAB because this is personal preference, but I'm not a fan of the nested if statements – I think something like this would be slightly clearer:

const isTextValid = !_.isEmpty(text) && _.isEqual(text, props.value); if (!_.isEmpty(googlePlacesRef.current.getAddressText()) && !isTextValid) { saveLocationDetails({}); }

I'm confused why we're clearing the value if text is not the same as props.value. Can you help me understand?

Because it text will change whenever you modify the address input & props.value will update only if the address is selected in the autocomplete list.

Yes, it makes sense I'll update the suggested changes here.

Santhosh-Sellavel · 2021-10-20T06:57:15Z

@roryabraham it happened to couple more times, when @stitesExpensify was CME, his account was not getting assigned as reviewer.

roryabraham · 2021-10-20T18:15:47Z

Retrying E2E tests for ya

Santhosh-Sellavel · 2021-10-20T18:21:28Z

It's failing for all recently updated PRs. @roryabraham

roryabraham · 2021-10-20T18:24:13Z

Agree it's probably unrelated

stitesExpensify · 2021-10-20T18:51:16Z

Something weird happened here and I think @stitesExpensify should've been assigned as a reviewer here because he's the CME on the linked issue. Also since this fixes a deploy blocker I'm assigning the CP Staging label.

Probably because I'm not on the contributor management team technically?

roryabraham · 2021-10-20T18:55:50Z

Probably because I'm not on the contributor management team technically?

Join us 🙃

stitesExpensify · 2021-10-21T19:47:29Z

@Santhosh-Sellavel can you pull main and push it up to fix e2e tests?

Santhosh-Sellavel · 2021-10-21T19:54:34Z

@stitesExpensify if possible can you restart the iOS Tests again? I already merged once from the main. In my other PRs also, I faced the same issue it is fixed now.

Pull from main

Santhosh-Sellavel · 2021-10-21T19:58:18Z

Merged again now let's wait and check thanks!

stitesExpensify · 2021-10-21T23:03:36Z

Looks like it passed! CC: @roryabraham

roryabraham

Tested well on web:

(cherry picked from commit f5cff3f)

roryabraham · 2021-10-22T23:05:05Z

Verified on staging web:

isagoico · 2021-10-22T23:39:47Z

Testing this atm on the following environments:

Android
Desktop
iOS
mWeb

OSBotify · 2021-10-22T23:54:12Z

🚀 Cherry-picked to staging by @roryabraham in version: 1.1.8-9 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

roryabraham · 2021-10-22T23:58:26Z

Passed QA on iOS:

roryabraham · 2021-10-23T00:06:09Z

Passed QA on desktop:

roryabraham · 2021-10-23T00:16:07Z

QA passed on iOS Safari:

OSBotify · 2021-10-23T00:40:38Z

🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀

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

OSBotify · 2021-10-23T01:31:24Z

🚀 Deployed to staging by @roryabraham in version: 1.1.8-10 🚀

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

roryabraham · 2021-10-23T16:29:39Z

Something weird happened here – This was CP'd to staging in version 1.1.8-9 (and we verified it there). But for some reason that deploy comment didn't land on this PR. Then it says it was deployed to staging in 1.1.8-10, which I think is a symptom of this issue.

OSBotify · 2021-10-28T20:56:16Z

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

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

Fixed address clear detected & modified the state

475a9a6

Santhosh-Sellavel requested a review from a team as a code owner October 19, 2021 23:24

MelvinBot requested review from danieldoglas and removed request for a team October 19, 2021 23:24

roryabraham self-assigned this Oct 20, 2021

roryabraham requested a review from stitesExpensify October 20, 2021 01:05

roryabraham assigned Santhosh-Sellavel and unassigned roryabraham Oct 20, 2021

roryabraham added the CP Staging label Oct 20, 2021

roryabraham reviewed Oct 20, 2021

View reviewed changes

Santhosh-Sellavel added 2 commits October 20, 2021 19:58

Refactored as per suggestion

21b963c

Merge branch 'main' into address_clear_vba_fix

c0c7cdb

isagoico mentioned this pull request Oct 20, 2021

Contributor PRs are not being consistently assigned to the correct CME #5962

Closed

Merge branch 'main' into address_clear_vba_fix

b16dd83

Merge pull request #2 from Santhosh-Sellavel/main

51a60d2

Pull from main

roryabraham approved these changes Oct 22, 2021

View reviewed changes

roryabraham merged commit f5cff3f into Expensify:main Oct 22, 2021

roryabraham mentioned this pull request Oct 22, 2021

Deploy Checklist: New Expensify 2021-10-15 #5879

Closed

github-actions bot pushed a commit that referenced this pull request Oct 22, 2021

Merge pull request #5954 from Santhosh-Sellavel/address_clear_vba_fix

708d447

(cherry picked from commit f5cff3f)

OSBotify mentioned this pull request Oct 22, 2021

🍒 Cherry pick PR #5954 to staging 🍒 #6010

Merged

botify mentioned this pull request Oct 23, 2021

[HOLD for payment 2021-11-04] [HOLD for payment 2021-10-30] VBA - User can proceed to personal info after clearing address in company info page #5920

Closed

OSBotify mentioned this pull request Oct 23, 2021

Deploy Checklist: New Expensify 2021-10-23 #6017

Closed

OSBotify mentioned this pull request Aug 16, 2022

Deploy Checklist: New Expensify 2022-08-04 #10253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed address clear detected & modified the state #5954

Fixed address clear detected & modified the state #5954

Santhosh-Sellavel commented Oct 19, 2021 •

edited

Loading

Santhosh-Sellavel commented Oct 19, 2021

roryabraham commented Oct 20, 2021

github-actions bot commented Oct 20, 2021

roryabraham Oct 20, 2021

Santhosh-Sellavel Oct 20, 2021

Santhosh-Sellavel commented Oct 20, 2021 •

edited

Loading

roryabraham commented Oct 20, 2021

Santhosh-Sellavel commented Oct 20, 2021

roryabraham commented Oct 20, 2021

stitesExpensify commented Oct 20, 2021

roryabraham commented Oct 20, 2021

stitesExpensify commented Oct 21, 2021

Santhosh-Sellavel commented Oct 21, 2021 •

edited

Loading

Santhosh-Sellavel commented Oct 21, 2021

stitesExpensify commented Oct 21, 2021

roryabraham left a comment

roryabraham commented Oct 22, 2021

isagoico commented Oct 22, 2021 •

edited by roryabraham

Loading

OSBotify commented Oct 22, 2021

roryabraham commented Oct 22, 2021

roryabraham commented Oct 23, 2021

roryabraham commented Oct 23, 2021

OSBotify commented Oct 23, 2021

OSBotify commented Oct 23, 2021

roryabraham commented Oct 23, 2021

OSBotify commented Oct 28, 2021

Fixed address clear detected & modified the state #5954

Fixed address clear detected & modified the state #5954

Conversation

Santhosh-Sellavel commented Oct 19, 2021 • edited Loading

Details

Fixed Issues

Tests & QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Santhosh-Sellavel commented Oct 19, 2021

roryabraham commented Oct 20, 2021

github-actions bot commented Oct 20, 2021

roryabraham Oct 20, 2021

Choose a reason for hiding this comment

Santhosh-Sellavel Oct 20, 2021

Choose a reason for hiding this comment

Santhosh-Sellavel commented Oct 20, 2021 • edited Loading

roryabraham commented Oct 20, 2021

Santhosh-Sellavel commented Oct 20, 2021

roryabraham commented Oct 20, 2021

stitesExpensify commented Oct 20, 2021

roryabraham commented Oct 20, 2021

stitesExpensify commented Oct 21, 2021

Santhosh-Sellavel commented Oct 21, 2021 • edited Loading

Santhosh-Sellavel commented Oct 21, 2021

stitesExpensify commented Oct 21, 2021

roryabraham left a comment

Choose a reason for hiding this comment

roryabraham commented Oct 22, 2021

isagoico commented Oct 22, 2021 • edited by roryabraham Loading

OSBotify commented Oct 22, 2021

roryabraham commented Oct 22, 2021

roryabraham commented Oct 23, 2021

roryabraham commented Oct 23, 2021

OSBotify commented Oct 23, 2021

OSBotify commented Oct 23, 2021

roryabraham commented Oct 23, 2021

OSBotify commented Oct 28, 2021

Santhosh-Sellavel commented Oct 19, 2021 •

edited

Loading

Santhosh-Sellavel commented Oct 20, 2021 •

edited

Loading

Santhosh-Sellavel commented Oct 21, 2021 •

edited

Loading

isagoico commented Oct 22, 2021 •

edited by roryabraham

Loading