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.

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

[HOLD for payment 2024-12-19] [$150] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you #52959

Closed
4 of 8 tasks
lanitochka17 opened this issue Nov 22, 2024 · 18 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 22, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.65.1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #52232

Action Performed:

precondition: user has enabled Expensify card

  1. Open the app and log in
  2. Navigate to the account settings > Security >Copilot
  3. Continue with the flow
  4. On "Let's make sure it is you" step, tap the back icon
  5. Create a new workspace and open the workspace settings
  6. Select More feature and enable Workflows
  7. Open Workflows > toggle on "Make or track payments" option
  8. Tap Connect bank account > any option
  9. On "Let's make sure it is you" step, tap the back icon
  10. Navigate to the workspace with Expensify card enabled
  11. Start issuing a new card
  12. On "Let's make sure it is you" step, tap the back icon
  13. Navigate to account settings > Profile > Contact method
  14. Start adding a new contact method
  15. On "Let's make sure it is you" step, tap the back icon

Expected Result:

User is navigated to the previous screen after tapping the back icon on "Let's make sure it is you" in the flows where code validation is required

Actual Result:

When tapping the back icon on "Let's make sure it is you" step in the Add copilot and Add bank account flows (steps 4 and 9), the keyboard gets dismissed while in the other flows (Issue a new card, Add new contact method, Reveal card details) user is returned to the previous screen immediately (steps 12 and 15)

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6672434_1732229158372.video_2024-11-21_17-41-09.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021861312036312016837
  • Upwork Job ID: 1861312036312016837
  • Last Price Increase: 2024-11-26
Issue OwnerCurrent Issue Owner: @VictoriaExpensify
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 22, 2024
Copy link

melvin-bot bot commented Nov 22, 2024

Triggered auto assignment to @VictoriaExpensify (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.

@NJ-2020
Copy link
Contributor

NJ-2020 commented Nov 22, 2024

Proposal

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

IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you

What is the root cause of that problem?

It's because the ValidateCodeActionModal is wrapped inside ScrollView component

<ValidateCodeActionModal
title={translate('contacts.validateAccount')}
descriptionPrimary={translate('contacts.featureRequiresValidate')}
descriptionSecondary={translate('contacts.enterMagicCode', {contactMethod})}
isVisible={!!isValidateCodeActionModalVisible}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
validatePendingAction={loginData?.pendingFields?.validateCodeSent}
sendValidateCode={() => User.requestValidateCodeAction()}
handleSubmitForm={(validateCode) => User.validateSecondaryLogin(loginList, contactMethod, validateCode)}
validateError={!isEmptyObject(validateLoginError) ? validateLoginError : ErrorUtils.getLatestErrorField(loginData, 'validateCodeSent')}
clearError={() => User.clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')}
onClose={() => toggleValidateCodeActionModal?.(false)}
/>
</View>
</ScrollView>
</View>

I can see like inside Profile page the ValidateCodeActionModal got dismissed properly when we click the back button because the ValidateCodeActionModal is not wrapped by ScrollView

<ValidateCodeActionModal
validatePendingAction={pendingContactAction?.pendingFields?.actionVerified}
validateError={validateLoginError}
handleSubmitForm={addNewContactMethod}
clearError={() => {
if (!loginData) {
return;
}
User.clearContactMethodErrors(addSMSDomainIfPhoneNumber(pendingContactAction?.contactMethod ?? contactMethod), 'addedLogin');
}}
onClose={() => {
if (loginData?.errorFields && pendingContactAction?.contactMethod) {
User.clearContactMethod(pendingContactAction?.contactMethod);
User.clearUnvalidatedNewContactMethodAction();
}
setIsValidateCodeActionModalVisible(false);
}}
isVisible={isValidateCodeActionModalVisible}
hasMagicCodeBeenSent={!!loginData?.validateCodeSent}
title={translate('delegate.makeSureItIsYou')}
sendValidateCode={() => User.requestValidateCodeAction()}
descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod})}
/>
</ScreenWrapper>
);

But in the other side like inside Bank account page the ValidateCodeActionModal is not get dismissed properly because it's wrapped by ScrollView, example here:
BankAccountPage

<ValidateCodeActionModal
title={translate('contacts.validateAccount')}
descriptionPrimary={translate('contacts.featureRequiresValidate')}
descriptionSecondary={translate('contacts.enterMagicCode', {contactMethod})}
isVisible={!!isValidateCodeActionModalVisible}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
validatePendingAction={loginData?.pendingFields?.validateCodeSent}
sendValidateCode={() => User.requestValidateCodeAction()}
handleSubmitForm={(validateCode) => User.validateSecondaryLogin(loginList, contactMethod, validateCode)}
validateError={!isEmptyObject(validateLoginError) ? validateLoginError : ErrorUtils.getLatestErrorField(loginData, 'validateCodeSent')}
clearError={() => User.clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')}
onClose={() => toggleValidateCodeActionModal?.(false)}
/>
</View>
</ScrollView>
</View>

Copilot
return (
<ValidateCodeActionModal
clearError={clearError}
onClose={onBackButtonPress}

<DelegateMagicCodeModal
login={login}
role={role}
onClose={() => {
if (!showValidateActionModal) {
setIsValidateCodeActionModalVisible(false);
return;
}
Navigation.navigate(ROUTES.SETTINGS_SECURITY);
}}
isValidateCodeActionModalVisible={isValidateCodeActionModalVisible}
/>
</HeaderPageLayout>

<ScrollView contentContainerStyle={[safeAreaPaddingBottomStyle, style, scrollViewContainerStyles]}>
{!Browser.isSafari() && <View style={styles.overscrollSpacer(backgroundColor ?? theme.appBG, windowHeight)} />}
<View style={[styles.alignItemsCenter, styles.justifyContentEnd, StyleUtils.getBackgroundColorStyle(backgroundColor ?? theme.appBG), headerContainerStyles]}>
{headerContent}
</View>
<View style={[styles.pt5, appBGColor, childrenContainerStyles]}>{children}</View>
</ScrollView>

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

We should put the ValidateCodeActionModal outside the ScrollView

<ValidateCodeActionModal
title={translate('contacts.validateAccount')}
descriptionPrimary={translate('contacts.featureRequiresValidate')}
descriptionSecondary={translate('contacts.enterMagicCode', {contactMethod})}
isVisible={!!isValidateCodeActionModalVisible}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
validatePendingAction={loginData?.pendingFields?.validateCodeSent}
sendValidateCode={() => User.requestValidateCodeAction()}
handleSubmitForm={(validateCode) => User.validateSecondaryLogin(loginList, contactMethod, validateCode)}
validateError={!isEmptyObject(validateLoginError) ? validateLoginError : ErrorUtils.getLatestErrorField(loginData, 'validateCodeSent')}
clearError={() => User.clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')}
onClose={() => toggleValidateCodeActionModal?.(false)}
/>
</View>
</ScrollView>
</View>

And we can do the same approach for other pages also

Result

Screen.Recording.2024-11-22.at.07.02.17.mov

What alternative solutions did you explore? (Optional)

@bernhardoj
Copy link
Contributor

Proposal

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

Inconsistent behavior on some pages that use the magic code validation modal.

What is the root cause of that problem?

For the copilot and bank account validation modal, the validation modal is wrapped inside a ScrollView.

<DelegateMagicCodeModal
login={login}
role={role}
onClose={() => {
if (!showValidateActionModal) {
setIsValidateCodeActionModalVisible(false);
return;
}
Navigation.navigate(ROUTES.SETTINGS_SECURITY);
}}
isValidateCodeActionModalVisible={isValidateCodeActionModalVisible}
/>
</HeaderPageLayout>

<ScrollView contentContainerStyle={[safeAreaPaddingBottomStyle, style, scrollViewContainerStyles]}>
{!Browser.isSafari() && <View style={styles.overscrollSpacer(backgroundColor ?? theme.appBG, windowHeight)} />}
<View style={[styles.alignItemsCenter, styles.justifyContentEnd, StyleUtils.getBackgroundColorStyle(backgroundColor ?? theme.appBG), headerContainerStyles]}>
{headerContent}
</View>
<View style={[styles.pt5, appBGColor, childrenContainerStyles]}>{children}</View>
</ScrollView>

<ValidateCodeActionModal
title={translate('contacts.validateAccount')}
descriptionPrimary={translate('contacts.featureRequiresValidate')}
descriptionSecondary={translate('contacts.enterMagicCode', {contactMethod})}
isVisible={!!isValidateCodeActionModalVisible}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
validatePendingAction={loginData?.pendingFields?.validateCodeSent}
sendValidateCode={() => User.requestValidateCodeAction()}
handleSubmitForm={(validateCode) => User.validateSecondaryLogin(loginList, contactMethod, validateCode)}
validateError={!isEmptyObject(validateLoginError) ? validateLoginError : ErrorUtils.getLatestErrorField(loginData, 'validateCodeSent')}
clearError={() => User.clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')}
onClose={() => toggleValidateCodeActionModal?.(false)}
/>
</View>
</ScrollView>

ScrollView by default will dismiss the keyboard first when tapping on it, so we need to press the back button again to close the modal. Compared to the issue new card confirm page, the modal is outside the scroll view, so it doesn't have this problem.

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

In a normal case, we won't have this problem because a header is usually not wrapped in a scroll view. But in this case, it's a bit different because the modal is put inside the scroll view, so the whole modal is affected. To fix this, we can keyboardShouldPersistTaps="handled" to the scroll view.

<ScrollView keyboardShouldPersistTaps="handled">

It's the same as what we did on the contact method details page.

<ScrollView keyboardShouldPersistTaps="handled">
{isFailedAddContactMethod && (
<ErrorMessageRow
errors={ErrorUtils.getLatestErrorField(loginData, 'addedLogin')}
errorRowStyles={[themeStyles.mh5, themeStyles.mv3]}
onClose={() => {
User.clearContactMethod(contactMethod);
User.clearUnvalidatedNewContactMethodAction();
Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo));
}}
canDismissError
/>
)}
<ValidateCodeActionModal
title={formattedContactMethod}
onModalHide={() => {}}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
isVisible={isValidateCodeActionModalVisible && !loginData.validatedDate && !!loginData}
validatePendingAction={loginData.pendingFields?.validateCodeSent}
handleSubmitForm={(validateCode) => User.validateSecondaryLogin(loginList, contactMethod, validateCode)}
validateError={!isEmptyObject(validateLoginError) ? validateLoginError : ErrorUtils.getLatestErrorField(loginData, 'validateCodeSent')}
clearError={() => User.clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')}
onClose={() => {
Navigation.goBack(ROUTES.SETTINGS_CONTACT_METHODS.getRoute(backTo));
setIsValidateCodeActionModalVisible(false);
}}
sendValidateCode={() => User.requestContactMethodValidateCode(contactMethod)}
descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod})}
/>

This is especially useful for cases like the copilot confirm page because it uses HeaderPageLayout and the children is always wrapped with a scroll view. For other cases, if it's possible to put the modal outside the scroll view, then we put it outside the scroll view.

It's technically possible for the copilot confirm page too by doing it like this:

<>
    <HeaderPageLayout ... />
    <DelegateMagicCodeModal ... />
</>

But I think it would be cleaner to just apply keyboardShouldPersistTaps="handled" to the scroll view inside HeaderPageLayout (we can add a new prop to control it if needed).

Also, there are other pages that put the modal inside the scroll view too which we may need to fix too, such as the CodesStep.

<ValidateCodeActionModal
title={translate('contacts.validateAccount')}
descriptionPrimary={translate('contacts.featureRequiresValidate')}
descriptionSecondary={translate('contacts.enterMagicCode', {contactMethod})}
isVisible={!isUserValidated}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
validatePendingAction={loginData?.pendingFields?.validateCodeSent}
sendValidateCode={() => User.requestValidateCodeAction()}
handleSubmitForm={(validateCode) => User.validateSecondaryLogin(loginList, contactMethod, validateCode, true)}
validateError={!isEmptyObject(validateLoginError) ? validateLoginError : ErrorUtils.getLatestErrorField(loginData, 'validateCodeSent')}
clearError={() => User.clearContactMethodErrors(contactMethod, !isEmptyObject(validateLoginError) ? 'validateLogin' : 'validateCodeSent')}
onModalHide={() => {}}
onClose={() => TwoFactorAuthActions.quitAndNavigateBack(backTo)}
/>
</ScrollView>

@melvin-bot melvin-bot bot added the Overdue label Nov 25, 2024
@VictoriaExpensify
Copy link
Contributor

Hmm ok so I think this is a pretty minor issue, but because we do want everything to be consistent, we should fix it.

My feeling is that this is a pretty easy fix so I'm going to lower the bounty on this one. If I'm missing anything to do with the complexity here, please let me know.

@melvin-bot melvin-bot bot removed the Overdue label Nov 26, 2024
@VictoriaExpensify VictoriaExpensify added the External Added to denote the issue can be worked on by a contributor label Nov 26, 2024
Copy link

melvin-bot bot commented Nov 26, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021861312036312016837

@melvin-bot melvin-bot bot changed the title IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you [$250] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you Nov 26, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 26, 2024
Copy link

melvin-bot bot commented Nov 26, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

@VictoriaExpensify VictoriaExpensify changed the title [$250] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you [$150] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you Nov 26, 2024
Copy link

melvin-bot bot commented Nov 26, 2024

Upwork job price has been updated to $150

@thesahindia
Copy link
Member

@bernhardoj's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Nov 27, 2024

Triggered auto assignment to @danieldoglas, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot added Overdue and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Dec 2, 2024
@danieldoglas
Copy link
Contributor

assigned

@bernhardoj
Copy link
Contributor

PR is ready

cc: @thesahindia

@VictoriaExpensify btw, I think we can keep the price at a normal price (250) since we need to fix this at 3 pages, so we need to test more too.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 12, 2024
@melvin-bot melvin-bot bot changed the title [$150] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you [HOLD for payment 2024-12-19] [$150] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you Dec 12, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 12, 2024
Copy link

melvin-bot bot commented Dec 12, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Dec 12, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.74-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-12-19. 🎊

For reference, here are some details about the assignees on this issue:

  • @bernhardoj requires payment through NewDot Manual Requests
  • @thesahindia requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Dec 12, 2024

@thesahindia @VictoriaExpensify @thesahindia The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 18, 2024
Copy link

melvin-bot bot commented Dec 19, 2024

Payment Summary

Upwork Job

BugZero Checklist (@VictoriaExpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1861312036312016837/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@melvin-bot melvin-bot bot added the Overdue label Dec 20, 2024
@VictoriaExpensify
Copy link
Contributor

Payment summary:
Contributor: @bernhardoj owed $250 via Upwork
C+: @thesahindia owed $250 via Upwork

@bernhardoj
Copy link
Contributor

Requested in ND.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Development

No branches or pull requests

7 participants