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

Edited by proposal-police: This proposal was edited at 2024-12-04 15:10:12 UTC.

Proposal

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

Expensify Card - Empty card details page can be opened after deactivating card

What is the root cause of that problem?

When card is deactivated card becomes undefined,

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

We should pass shouldBeBlocked={!card} to AccessOrNotFoundWrapper, or we can use isEmptyObject(card) optionally

        <AccessOrNotFoundWrapper
            accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
            policyID={policyID}
            featureName={CONST.POLICY.MORE_FEATURES.ARE_EXPENSIFY_CARDS_ENABLED}
            shouldBeBlocked={!card}

And we should close the modal when back button is clicked by passing navigation.dismissModal function to onBackButtonPress prop or fullPageNotFoundViewProps

fullPageNotFoundViewProps={{onBackButtonPress: () => Navigation.dismissModal()}}

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

What alternative solutions did you explore? (Optional)

expensify/ExpensiFlex Your proposal will be dismissed because you did not follow the proposal template.

Proposal

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

Expensify Card - Empty card details page can be opened after deactivating card

What is the root cause of that problem?

We haven't restricted access to the detail card page if a user tries to view an invalid card

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

In WorkspaceExpensifyCardDetailsPage we need to implement the same logic as we did in WorkspaceCompanyCardDetailsPage

    if (!card && !isLoadingOnyxValue(cardsListMetadata)) {
        return <NotFoundPage />;
    }

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

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.

@mountiny This seems like a lack of when implementing this feature. Could I take over this one?

Proposal

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

Empty card details page can be opened after deactivating card

What is the root cause of that problem?

There is no blocking screen in Expensify cards page in case the cardID doesn't exist in the list. hence we still see the details page for any cardID eg ABDC, 232, etc. with no actual data present.

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

We would need to make 2 changes here.

1. In WorkspaceExpensifyCardDetailsPage , first we need to introduce a new NotFoundPage page in case the card for that particular cardID is not present and the onyx updates are complete:

    if (!card && !isLoadingOnyxValue(cardsListMetadata)) {
        return <NotFoundPage />;
    }

2. We would also need to update the optimistic data for deactivateCard, this is because when we delete it optimistically, a Not found page would be immediately shown as we set the value to null in the optimistic data:

Note that fixing this is necessary else we will see a not found page everytime the user deactives the card before the RHP closes, so we will update optimistic data and introduce success data for deactivateCard:

    const optimisticData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`,
            value: {
                [cardID]: {
                    pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
                },
            },
        },
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.CARD_LIST,
            value: {
                [cardID]: {
                    pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
                },
            },
        },
    ];

    const successData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`,
            value: {
                [cardID]: null,
            },
        },
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.CARD_LIST,
            value: {
                [cardID]: null,
            },
        }
    ]

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

N/A this is a visual bug so no tests required

What alternative solutions did you explore? (Optional)

N/A

yeah if the cardId is not locally we should show not found page

@DylanDylann can you please review my proposal here

@thelullabyy's proposal looks good to me

@twilight2294 I noticed that you mentioned another bug in your proposal: "When deactivating the card, the card is removed before the modal is dismissed. It caused the empty value to be displayed in a moment". But I believe that we shouldn't add pendingAction to the deactivating card action because this action follows Offline Pattern C

@thelullabyy Let's also resolve this additional bug in your PR. One potential approach that we already used widely in our codebase is to deactivate the card after the modal is closed completely by using runAfterInteractions.

🎀 👀 🎀 C Reviewed

Current assignee @mountiny is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@DylanDylann we follow the pattern i suggested in company cards page when we deactive the card:

So i don't understand why cannot we use it in expensify cards page ? Can you please re-review my proposal ?

@mountiny can you also please check, my proposal already works the way company cards page works so i think it is fine to put the optimistic pending action

@twilight2294 I just checked again

• Company Card: Applying offline pattern B
• Expensify Card: Applying offline pattern C

It isn't the same.

@DylanDylann , we follow pattern B for updateExpensifyCardLimit, updateExpensifyCardTitle, updateExpensifyCardLimitType:

So i guess for the details page too, pattern B should be followed similar to company cards details page

@twilight2294 No, the company cards is a simple update in our database hence pattern B but for anything related to Expensify card we also need to communication with thirdparty api and that can fail for unknown reasons hence we only want users to be able to do take these critical actions online

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

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

@alexpensify, @mountiny, @DylanDylann, @thelullabyy Whoops! This issue is 2 days overdue. Let's get this updated quick!

@DylanDylann I created a draft PR, I just need to test and finish all videos before opening to review.

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

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

• Fix display not here page if card is deactivated #53835

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

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

• @DylanDylann requires payment (Needs manual offer from BZ)
• @thelullabyy requires payment (Needs manual offer from BZ)

@DylanDylann @alexpensify @DylanDylann 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]

Payment Summary

Upwork Job

• ROLE: @DylanDylann paid $250 via Upwork
• ROLE: @thelullabyy paid $250 via Upwork

BugZero Checklist (@alexpensify)

• 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/1865065766629964229/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

@thelullabyy - I'm having trouble finding you in Upwork. Please apply for the job in Upwork, and I can complete the payment process. Thanks!

BugZero Checklist:

• [Contributor] Classify the bug:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: Feature: Create Deactivate card logic #45703 (comment)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion:

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

• Expensify Card feature is enabled.
• Users have assigned themselves a virtual card.

Test:

1. Go to workspace settings > Expensify Card.
2. Click on the virtual card.
3. Click Deactivate card.
4. Deactivate the card.
5. Make sure the App navigates to the Company Card Page without displaying not found page
6. Click the browser forward button.
7. Make sure: The not here page opens because the card is deactivated.

Do we agree 👍 or 👎

@thelullabyy - I'm having trouble finding you in Upwork. Please apply for the job in Upwork, and I can complete the payment process. Thanks!

@alexpensify Applied!

Thanks, I've sent offers to @thelullabyy and @DylanDylann in Upwork. Please accept and I can complete the payment process.

@alexpensify Accepted, ty!

All set and closing, everyone has been paid via Upwork. Since I'm OOO until January 3, I'm going to close and work on the QA step when I'm back online.