[$250] Contact method - There is no feature to delete unverified contact methods. #54626

IuliiaHerets · 2024-12-27T16:49:16Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: v9.0.79-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5288103
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: macOS Sequoia 15.1.1 / Web
App Component: undefined

Action Performed:

Open the URL: https://staging.new.expensify.com
Loggin
Click on 'Settings'
Click on 'Profile'
Click on 'Contact methods'
Click on 'New contact method'
Enter a valid email & click on 'Add'
Paste the code from mail and click on 'Verify'
On the contact methods screen, click on the recently added unverified contact method.

Expected Result:

The user should be able to delete the contact method as long as it is unverified

Actual Result:

It is not possible to delete a univerified contact methods because there is no feature for that

Workaround:

Unknown

Platforms:

Screenshots/Videos

Bug6682518_1733188311399.There_is_no_feature_for_delete_unverified_contact_methods.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021874053836834663264
Upwork Job ID: 1874053836834663264
Last Price Increase: 2024-12-31

Issue Owner

Current Issue Owner: @parasharrajat

melvin-bot · 2024-12-27T16:49:26Z

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

Shahidullah-Muffakir · 2024-12-27T19:23:20Z

Edited by proposal-police: This proposal was edited at 2024-12-30 19:29:21 UTC.

Proposal

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

User is not able to remove an unverified contact method

What is the root cause of that problem?

New feature

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

We can accept the children prop in the ValidateCodeActionModal component as:

<View style={[themeStyles.borderTop, themeStyles.mv4]} />
{children}

Then, we simply pass the "Remove" button to it in the ContactMethodDetailsPage component as shown below:
here: ContactMethodDetailsPage

<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: formattedContactMethod})}
               >
+                <OfflineWithFeedback
+                   pendingAction={loginData.pendingFields?.deletedLogin}
+                   errors={ErrorUtils.getLatestErrorField(loginData, 'deletedLogin')}
+                    errorRowStyles={[themeStyles.mt6, themeStyles.ph5]}
+                    onClose={() => User.clearContactMethodErrors(contactMethod, 'deletedLogin')}
+                >
+                    <MenuItem
+                       title={translate('common.remove')}
+                        icon={Expensicons.Trashcan}
+                        iconFill={theme.danger}
+                       onPress={() => toggleDeleteModal(true)}
+                    />
+               </OfflineWithFeedback>
               </ValidateCodeActionModal>

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

We can pass the ThreeDotsMenu as rightComponent to the Secondary emails, we need design confirmation for this:

Screen.Recording.2024-12-31.at.12.58.16.AM.mov

Screen.Recording.2024-12-28.at.12.15.37.AM.mov

huult · 2024-12-29T08:33:42Z

Edited by proposal-police: This proposal was edited at 2024-12-29 08:39:48 UTC.

Proposal

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

There is no feature to delete unverified contact methods.

What is the root cause of that problem?

We are able to remove the contact method when it is verified and not the default contact.

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 154 in 6bdeffb

setIsValidateCodeActionModalVisible(!loginData?.validatedDate);

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 195 in 6bdeffb

{isDefaultContactMethod ? (

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 278 in 6bdeffb

{!isValidateCodeActionModalVisible && getMenuItems()}

And in the case of this ticket, the contact method is unable to be removed. This is a new feature

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

If we want to remove the contact method when it is not yet verified, we need to add getMenuItems to the footer props of ValidateCodeActionModal and update the parameters of getMenuItems to suit this case.

1 update ValidateCodeActionModal

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 261 in 6bdeffb

<ValidateCodeActionModal

update to:

    <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: formattedContactMethod})}
        footer={() => getMenuItems(true)} // Add this line
    />

2 update getMenuItems

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 180 in 6bdeffb

const getMenuItems = () => (

const getMenuItems = (onlyRemove = false) => {
        if (onlyRemove) { // Add this block
            if (isDefaultContactMethod) {
                return <View />;
            }
            return (
                <>
                    <OfflineWithFeedback
                        pendingAction={loginData.pendingFields?.deletedLogin}
                        errors={ErrorUtils.getLatestErrorField(loginData, 'deletedLogin')}
                        errorRowStyles={[themeStyles.mt6, themeStyles.ph5]}
                        onClose={() => User.clearContactMethodErrors(contactMethod, 'deletedLogin')}
                    >
                        <MenuItem
                            title={translate('common.remove')}
                            icon={Expensicons.Trashcan}
                            iconFill={theme.danger}
                            onPress={() => toggleDeleteModal(true)}
                        />
                    </OfflineWithFeedback>
                    <ConfirmModal
                        title={translate('contacts.removeContactMethod')}
                        onConfirm={confirmDeleteAndHideModal}
                        onCancel={() => toggleDeleteModal(false)}
                        onModalHide={() => {
                            InteractionManager.runAfterInteractions(() => {
                                validateCodeFormRef.current?.focusLastSelected?.();
                            });
                        }}
                        prompt={translate('contacts.removeAreYouSure')}
                        confirmText={translate('common.yesContinue')}
                        cancelText={translate('common.cancel')}
                        isVisible={isDeleteModalOpen && !isDefaultContactMethod}
                        danger
                    />
                </>
            );
        }

        ...
    };

POC

Screen.Recording.2024-12-29.at.15.47.31.mov

Note: I will test the other case and optimize the code during the pull request phase

Test branch

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

This issue updates the logic to allow removing the contact method when it is not verified, so I think we don’t need to test here

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

laurenreidexpensify · 2024-12-30T16:53:30Z

I'm not sure I fully follow the last repro steps:

Paste the code from mail and click on 'Verify'
On the contact methods screen, click on the recently added unverified contact method.

It sounds like we're verifying the contact, but then looking to remove an unverified contact?

Shahidullah-Muffakir · 2024-12-30T16:59:50Z

It sounds like we're verifying the contact, but then looking to remove an unverified contact?

The first code we paste from the email is sent to the main email of the current user. The secondary email we are trying to add remains unverified until we enter the second code sent to that secondary email. So, until the code is entered for the secondary email, it is still considered unverified.

parasharrajat · 2024-12-30T19:27:31Z

Ok, it looks like a valid issue. I can see that it was caused by #51882.

parasharrajat · 2024-12-30T19:33:24Z

@Shahidullah-Muffakir I don't see how your proposal will open the Delete Modal.

Shahidullah-Muffakir · 2024-12-30T19:35:45Z

@Shahidullah-Muffakir I don't see how your proposal will open the Delete Modal.

@parasharrajat Do you mean in the alternative method?"

parasharrajat · 2024-12-30T19:37:23Z

No, on the main solution.

Shahidullah-Muffakir · 2024-12-30T19:39:45Z

No, on the main solution.

Oh, okay. Since we're passing the Remove button as a child, clicking on it triggers the
onPress={() => toggleDeleteModal(true)} function, which opens the delete modal.

parasharrajat · 2024-12-30T19:41:34Z

But where are we rendering the modal?

parasharrajat · 2024-12-30T19:43:10Z

@laurenreidexpensify I can help review the proposals here as I have the context of the problem.

Shahidullah-Muffakir · 2024-12-30T19:45:09Z

But where are we rendering the modal?

We're already rendering the Confirmation modal here:

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Lines 220 to 233 in a5feee8

    
           <ConfirmModal 
        
               title={translate('contacts.removeContactMethod')} 
        
               onConfirm={confirmDeleteAndHideModal} 
        
               onCancel={() => toggleDeleteModal(false)} 
        
               onModalHide={() => { 
        
                   InteractionManager.runAfterInteractions(() => { 
        
                       validateCodeFormRef.current?.focusLastSelected?.(); 
        
                   }); 
        
               }} 
        
               prompt={translate('contacts.removeAreYouSure')} 
        
               confirmText={translate('common.yesContinue')} 
        
               cancelText={translate('common.cancel')} 
        
               isVisible={isDeleteModalOpen && !isDefaultContactMethod} 
        
               danger

It’s on the same page, and the isVisible prop becomes true when isDeleteModalOpen && !isDefaultContactMethod is true, which happens when you press the button.

melvin-bot · 2024-12-31T11:23:49Z

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

melvin-bot · 2024-12-31T11:23:53Z

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

parasharrajat · 2024-12-31T11:38:59Z

@Shahidullah-Muffakir But that Modal is rendered conditionally. It will be rendered when the ValidateCode modal is not rendered on the UI.

About your proposal, you chose to add the remove option via the children prop. Why didn't you use a footer prop for that?

huult · 2024-12-31T11:42:21Z

@parasharrajat I included it in my proposal

parasharrajat · 2024-12-31T11:44:07Z

I saw that, just trying to understand the proposal.

Shahidullah-Muffakir · 2024-12-31T11:46:31Z

@Shahidullah-Muffakir But that Modal is rendered conditionally. It will be rendered when the ValidateCode modal is not rendered on the UI.

No, I think it can still be displayed even if the ValidateCode modal is rendered, as I showed in the video. This is because we're only checking isVisible={isDeleteModalOpen && !isDefaultContactMethod}

About your proposal, you chose to add the remove option via the children prop. Why didn't you use a footer prop for that?

The footer prop would place the Remove button at the bottom of the screen, but the design team hasn't confirmed where the Remove button should go yet. Using the children prop allows us to display the Remove button anywhere we need."

parasharrajat · 2024-12-31T11:52:45Z

But if you look at the code the Modal is mounted at

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 278 in c4b3297

{!isValidateCodeActionModalVisible && getMenuItems()}

where mounting is controlled conditionally.

The footer prop would place the Remove button at the bottom of the screen, but the design team hasn't confirmed where the Remove button should go yet. Using the children prop allows us to display the Remove button anywhere we need."

I was expecting a slightly different motive so I asked. Thanks.

parasharrajat · 2024-12-31T12:04:56Z

Here, we can't use the footer prop as it will force the verify button to float above the screen, which is against our design guidelines. That's why it was removed in #51882.

But the remove button should not have been removed from here. The correct location to show the Remove button would be the empty area like we have done in status page for clear status.

Shahidullah-Muffakir · 2024-12-31T12:10:17Z

But if you look at the code the Modal is mounted at

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx

Line 278 in c4b3297

{!isValidateCodeActionModalVisible && getMenuItems()}

where mounting is controlled conditionally.

Oh, sorry, I missed that. We can move the ConfirmModal out of the getMenuItems function and place it directly below {!isValidateCodeActionModalVisible && getMenuItems()}

parasharrajat · 2024-12-31T13:17:54Z

After reviewing both proposals I can say that both lack some details. But following the reason in #54626 (comment), we can't use footer prop. We need a new prop or use children to achieve it. Tweaking both proposals will result in desired result, so I am going with the first in line and it does consider the placement of the remove button.

Let's go with @Shahidullah-Muffakir proposal for this one.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-12-31T13:18:07Z

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

huult · 2024-12-31T13:59:19Z

@parasharrajat If the design team wants the button for unverified contact methods to be located at the bottom and under the save button, then my proposal is valid.

parasharrajat · 2024-12-31T14:04:02Z

That's a valid point @huult. Let me tag @Expensify/design team to suggest the placement of the delete button.

IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 27, 2024

melvin-bot bot assigned laurenreidexpensify Dec 27, 2024

melvin-bot bot added the Overdue label Dec 30, 2024

melvin-bot bot removed the Overdue label Dec 30, 2024

laurenreidexpensify added the External Added to denote the issue can be worked on by a contributor label Dec 31, 2024

melvin-bot bot changed the title ~~Contact method - There is no feature to delete unverified contact methods.~~ [$250] Contact method - There is no feature to delete unverified contact methods. Dec 31, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 31, 2024

melvin-bot bot assigned thesahindia Dec 31, 2024

laurenreidexpensify assigned parasharrajat and unassigned thesahindia Dec 31, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 31, 2024

laurenreidexpensify added this to [#whatsnext] #quality Dec 31, 2024

laurenreidexpensify moved this to MEDIUM in [#whatsnext] #quality Dec 31, 2024

melvin-bot bot assigned tgolen Dec 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$250] Contact method - There is no feature to delete unverified contact methods. #54626

[$250] Contact method - There is no feature to delete unverified contact methods. #54626

IuliiaHerets commented Dec 27, 2024 •

edited by parasharrajat

Loading

melvin-bot bot commented Dec 27, 2024

Shahidullah-Muffakir commented Dec 27, 2024 •

edited by github-actions bot

Loading

huult commented Dec 29, 2024 •

edited

Loading

laurenreidexpensify commented Dec 30, 2024

Shahidullah-Muffakir commented Dec 30, 2024

parasharrajat commented Dec 30, 2024

parasharrajat commented Dec 30, 2024

Shahidullah-Muffakir commented Dec 30, 2024

parasharrajat commented Dec 30, 2024 •

edited

Loading

Shahidullah-Muffakir commented Dec 30, 2024

parasharrajat commented Dec 30, 2024 •

edited

Loading

parasharrajat commented Dec 30, 2024

Shahidullah-Muffakir commented Dec 30, 2024

melvin-bot bot commented Dec 31, 2024

melvin-bot bot commented Dec 31, 2024

parasharrajat commented Dec 31, 2024

huult commented Dec 31, 2024

parasharrajat commented Dec 31, 2024 •

edited

Loading

Shahidullah-Muffakir commented Dec 31, 2024

parasharrajat commented Dec 31, 2024 •

edited

Loading

parasharrajat commented Dec 31, 2024 •

edited

Loading

Shahidullah-Muffakir commented Dec 31, 2024

parasharrajat commented Dec 31, 2024

melvin-bot bot commented Dec 31, 2024

huult commented Dec 31, 2024

parasharrajat commented Dec 31, 2024

[$250] Contact method - There is no feature to delete unverified contact methods. #54626

[$250] Contact method - There is no feature to delete unverified contact methods. #54626

Comments

IuliiaHerets commented Dec 27, 2024 • edited by parasharrajat Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 27, 2024

Shahidullah-Muffakir commented Dec 27, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

huult commented Dec 29, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

Test branch

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

laurenreidexpensify commented Dec 30, 2024

Shahidullah-Muffakir commented Dec 30, 2024

parasharrajat commented Dec 30, 2024

parasharrajat commented Dec 30, 2024

Shahidullah-Muffakir commented Dec 30, 2024

parasharrajat commented Dec 30, 2024 • edited Loading

Shahidullah-Muffakir commented Dec 30, 2024

parasharrajat commented Dec 30, 2024 • edited Loading

parasharrajat commented Dec 30, 2024

Shahidullah-Muffakir commented Dec 30, 2024

melvin-bot bot commented Dec 31, 2024

melvin-bot bot commented Dec 31, 2024

parasharrajat commented Dec 31, 2024

huult commented Dec 31, 2024

parasharrajat commented Dec 31, 2024 • edited Loading

Shahidullah-Muffakir commented Dec 31, 2024

parasharrajat commented Dec 31, 2024 • edited Loading

parasharrajat commented Dec 31, 2024 • edited Loading

Shahidullah-Muffakir commented Dec 31, 2024

parasharrajat commented Dec 31, 2024

melvin-bot bot commented Dec 31, 2024

huult commented Dec 31, 2024

parasharrajat commented Dec 31, 2024

IuliiaHerets commented Dec 27, 2024 •

edited by parasharrajat

Loading

Shahidullah-Muffakir commented Dec 27, 2024 •

edited by github-actions bot

Loading

huult commented Dec 29, 2024 •

edited

Loading

parasharrajat commented Dec 30, 2024 •

edited

Loading

parasharrajat commented Dec 30, 2024 •

edited

Loading

parasharrajat commented Dec 31, 2024 •

edited

Loading

parasharrajat commented Dec 31, 2024 •

edited

Loading

parasharrajat commented Dec 31, 2024 •

edited

Loading