[HOLD Expensify/react-native-onyx/pull/581] Expensify Card - Toggle for Expensify Card is only locked after refreshing the page

IuliiaHerets commented 4 days ago

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.35-0 Reproducible in staging?: Y Reproducible in production?: N Issue was found when executing this PR: https://github.com/Expensify/App/pull/49173 Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Go to staging.new.expensify.com
Create a new workspace.
Go to workspace settings > More features.
Enable Expensify Card.
Go to Expensify Card.
Click Issue new card.
Go through bank account set up.
Issue new card to yourself.
After issuing new card, go to More features page.

Expected Result:

The toggle for Expensify Card should be immediately locked (production behavior).

Actual Result:

The toggle for Expensify Card is not immediately locked. It is only locked after refreshing the page.

Manual refresh is also required to unlock the toggle after deactivating Expensify Card.

Workaround:

Unknown

Platforms:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/fcac0eb9-7b95-4ceb-a0bc-ebeea88e3b4b

View all open jobs on GitHub

melvin-bot[bot] commented 4 days ago

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

melvin-bot[bot] commented 4 days ago

Triggered auto assignment to @NikkiWines (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 4 days ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

Nodebrute commented 4 days ago

Edited by proposal-police: This proposal was edited at 2024-09-14 19:27:58 UTC.

Proposal

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

Toggle for Expensify Card is only locked after refreshing the page

What is the root cause of that problem?

The issue is that adding or deleting a card doesn’t trigger a re-render because there’s no change in policy?.workspaceAccountID or policy?.areExpensifyCardsEnabled. https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx#L113

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

We need to ensure that adding or deleting a card triggers a re-render. One approach to achieve this is by using allCardsList in WorkspaceMoreFeaturesPage.tsx

    const [allCardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}`);

And then we can pass it as third parameter here and then we can remove [this].(https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/libs/CardUtils.ts#L29-L36)

What alternative solutions did you explore? (Optional)

Or We could move this logic to the Workspace More Features page and use !isEmptyObject(cardsList) here.

dominictb commented 4 days ago

Edited by proposal-police: This proposal was edited at 2023-10-10T10:00:00Z.

Proposal

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

The toggle for Expensify Card is not immediately locked. It is only locked after refreshing the page.

What is the root cause of that problem?

We use this condition to disable the toggle button: Link to code, but the function shouldExpensifyCardToggleBeDisabled is not re-triggered when its dependencies change.
As a result, CardUtils.shouldExpensifyCardToggleBeDisabled(policy?.workspaceAccountID, policy?.areExpensifyCardsEnabled) becomes outdated when those dependencies are updated

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

We need to create a new variable, named expensifyCards in WorkspaceMOreFeaturesPage:


    const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policy.id);
    const [expensifyCards] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`);

then update condition: https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx#L113


            disabled: Object.keys(expensifyCards).length > 0,

or


            disabled: Object.values(expensifyCards).map((i)=>i.pendingAction !== 'delete').length > 0,

What alternative solutions did you explore? (Optional)

dominictb commented 4 days ago

Proposal updated

s77rt commented 4 days ago

cc @VickyStash seems related to https://github.com/Expensify/App/pull/49173

mountiny commented 3 days ago

@dominictb @Nodebrute we cannot do that as that would reintroduce the bug we had before.

I think this is ok and we can revert to using the cardList once we merge the deploy the onyx fix https://github.com/Expensify/react-native-onyx/pull/581

Expensify / App