[$250] IOU - Category briefly reselects itself when removed

[lanitochka17]

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: 1.4.86-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): betlihemasfaw14@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. FAB > Submit Expense
2. Add amount and choose workspace
3. Add category and merchant
4. Go to IOU > Category > click to remove

Expected Result:

The category should not briefly reselect itself

Actual Result:

The category briefly reselects itself when clicked to remove

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6519586_1718901363324.Screen_Recording_2024-06-20_at_9.31.55_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c03aa025a5c5f612
• Upwork Job ID: 1807555103945888117
• Last Price Increase: 2024-07-21

Issue Owner

Current Issue Owner: @fedirjh

[melvin-bot]

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.

[lanitochka17]

@jliexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[jliexpensify]

Yep, I can reproduce this one - after selecting and deselecting multiple times (e.g. 2 times) the Category reappears briefly again.

NOTE: Technically, IOU functionality is under #billpay, but I think this would affect all users so adding it under #collect

[melvin-bot]

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

[melvin-bot]

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

[pankajsoftyoi]

Proposal

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

Category briefly reselects itself when removed

What is the root cause of that problem?

There isn't any condition in place which prevents getting reselected itself in onSelectRow(item);.

App/src/components/SelectionList/BaseListItem.tsx

Lines 79 to 82 in 57622ff

	if (shouldPreventEnterKeySubmit && e && 'key' in e && e.key === CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey) {
	return;
	}
	onSelectRow(item);

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

Need to set the following condition for already selected item. With this change it will prevent to reselect itself.

 if (!canSelectMultiple && !item.isSelected && !rightHandSideComponent) {
      onSelectRow(item);
}

What alternative solutions did you explore? (Optional)

N/A

[jliexpensify]

Bump @fedirjh for a review!

[fedirjh]

Still awaiting more proposals.

[fedirjh]

@pankajsoftyoi Your proposal looks like a reverse solution statement. Can you elaborate more on the root cause of the problem? Can you explain why this bug occurs after multiple selecting and deselecting?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[fedirjh]

Still awaiting more proposals.

[wlegolas]

Proposal

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

IOU - Category briefly reselects itself when removed

What is the root cause of that problem?

When the user selects and deselects multiple times (e.g. 2 times) the Category reappears briefly again because the previous request finished successfully and updates the Onyx state with a new transaction that has the category information before the request to update the transaction with deselected category has not yet been completed.

For example:

1. The user clicks on the "Category" label to open the modal to select the Category.
2. The user selects the category "Advertising"
3. App closes the modal
4. The request UpdateMoneyRequestCategory is started to update the transaction with the selected category "Advertising".
5. The user clicks on the "Category" label to open the modal to deselect the Category.
6. User deselects the category "Advertising"
7. App closes the modal
8. The request UpdateMoneyRequestCategory is started to update the transaction with an empty string in the category property.
9. The user clicks on the "Category" label to open the modal
10. The request started at step 4 finished and updates the Onyx state with a new transaction with the category "Advertising"
11. User sees the "Advertising" as selected category
12. The request started at step 8 finished and updates the Onyx state with a new transaction with the category with an empty string
13. User sees the "Advertising" being deselected

The issue is caused because when the user deselects the Category (CategoryPiker component) and opens the modal again (MoneyRequestView component) the previous request (to select the Category "Advertising") finished and updates the state with a new transaction with the category "Advertising" and the user can see the "Advertising" category as selected and after a while the last request finishes and updates the state with an empty category.

In the following video, you can see the steps above, in the Network tab you can see when each request finishes.

issue-44649-evidence-request-not-finished.mp4

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

We should change the IOURequestStepCategory component to show the loading message meanwhile the request to update the category is not completed yet. With this solution when the user opens the modal they will see a loading indicator and when the last request finishes the user will be able to proceed.

The loading indicator here should be changed to use the isLoadingReportData information from Onyx state to show the loading indicator if the isLoadingReportData property is true. For example:

function IOURequestStepCategory({
    report: reportReal,
    reportDraft,
    route: {
        params: {transactionID, backTo, action, iouType, reportActionID},
    },
    transaction,
    splitDraftTransaction,
    policy: policyReal,
    policyDraft,
    policyTags,
    policyCategories: policyCategoriesReal,
    policyCategoriesDraft,
    reportActions,
    session,
    isLoadingReportData,
}: IOURequestStepCategoryProps) {
    ...
    const {isOffline} = useNetwork({onReconnect: fetchData});
    const isLoading = !isOffline && (policyCategories === undefined || isLoadingReportData); // Here we should change
}

...

const IOURequestStepCategoryWithOnyx = withOnyx<IOURequestStepCategoryProps, IOURequestStepCategoryOnyxProps>({
    ...
    session: {
        key: ONYXKEYS.SESSION,
    },
    isLoadingReportData: {
        key: ONYXKEYS.IS_LOADING_REPORT_DATA,
    },
})(IOURequestStepCategory);

What alternative solutions did you explore? (Optional)

N/A

POC

poc-issue-44649.mp4

[fedirjh]

@wlegolas Your proposal doesn’t look good to me :

• Breaking the offline flow: I believe that implementing your proposal may disrupt the offline functionality. Users won't be able to update the category while in offline mode. The loading indicator will be displayed until they are connected again.
• Loading indicator on form: Introducing a loading indicator slowly to this form would deviate from the standard behavior observed on other pages. It would make the behavior inconsistent, as only this view would have a loading indicator while others wouldn't.

[wlegolas]

Hi @fedirjh thank you for sharing your points.

About your point about "Breaking the offline flow" the loading indicator won't be shown because the first condition is to be online:

const isLoading = !isOffline && (policyCategories === undefined || isLoadingReportData);

I recorded the video below to show the offline functionality when the CategoryPicker is shown and the user is offline, you can see the behavior is the same that we have currently on Staging.

offline-experience.mp4

Related to your point "Loading indicator on form", I suggested putting a loading indicator because the application state is not up to date and the CategoryPicker component needs to show feedback to the user about the previous process still occurring. I checked in other forms but didn't find a form with the same behavior that I found in the CategoryPicker, maybe other forms don't have this problem with the app state not being up to date.

If you know another form that had this same problem I can take a look at to find out another solution to solve this problem.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[jliexpensify]

Oh interesting, @fedirjh can you repro this? I wasn't able to.

[wlegolas]

I could see this problem when I changed my network connection to a slow one because the main problem was related to the time that the previous request didn't finish with the time the user opened the modal (CategoryPicker).

[jliexpensify]

Interesting - @fedirjh since selecting a Category is pretty integral to the product, it would be great to ensure that this is rock-solid. Are you able to confirm this and determine if we can fix it? Thanks!

[melvin-bot]

@jliexpensify @fedirjh this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[jliexpensify]

Waiting on @fedirjh

[fedirjh]

@jliexpensify I am still able to reproduce the bug on staging.

[fedirjh]

@wlegolas I noticed something in the server response. For other fields, we have two properties: the original one and the modified one. However, for the category field (i.e we have amount and modifiedAmount), we don't have such property. This probably explains why other fields are working properly.

[wlegolas]

Hi @fedirjh I agree with your point about the category field does not have a related modified field.

I simulated locally adding the modifiedCategory field and looks like worked as expected.

I can change my proposal to have the implementation to fix it but in my opinion, we have two changes, one in the Frontend project (I can put the details in my current proposal) and another change on the Backend side because the transaction response when updating the Money Request Category needs to return this new field modifiedCategory to update the state with the correct information.

If you agree I can change my proposal to have the information to use the new field modifiedCategory to fix this problem.

[fedirjh]

we have two changes, one in the Frontend project (I can put the details in my current proposal)

@wlegolas Can you share details about front end changes?

[wlegolas]

we have two changes, one in the Frontend project (I can put the details in my current proposal)

@wlegolas Can you share details about front end changes?

Sure, below I'm putting a brief details about the front end changes that we should do to have the modifiedCategory information in the Transaction object and also solve this issue.

• Add the new property modifiedCategory in the Transaction type
• Change the getCategory method to retrieve the category value first using the modifiedCategory this looks like the implementation we have in the getMerchant
• Improve the getUpdateMoneyRequestParams method to add a state in the successData and failureData to have the correct information before the request is finished. For example:

function getUpdateMoneyRequestParams(
    transactionID: string,
    transactionThreadReportID: string,
    transactionChanges: TransactionChanges,
    policy: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy>,
    policyTagList: OnyxTypes.OnyxInputOrEntry<OnyxTypes.PolicyTagList>,
    policyCategories: OnyxTypes.OnyxInputOrEntry<OnyxTypes.PolicyCategories>,
    onlyIncludeChangedFields: boolean,
): UpdateMoneyRequestData {
    const optimisticData: OnyxUpdate[] = [];
    const successData: OnyxUpdate[] = [];
    const failureData: OnyxUpdate[] = [];
    ...

    // Update recently used categories if the category is changed
    if ('category' in transactionChanges) {
        const optimisticPolicyRecentlyUsedCategories = Category.buildOptimisticPolicyRecentlyUsedCategories(iouReport?.policyID, transactionChanges.category);
        if (optimisticPolicyRecentlyUsedCategories.length) {
            optimisticData.push({
                onyxMethod: Onyx.METHOD.SET,
                key: `${ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_CATEGORIES}${iouReport?.policyID}`,
                value: optimisticPolicyRecentlyUsedCategories,
            });
        }

        // Update the state with the modified category
        successData.push({
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
            value: {
                modifiedCategory: transactionChanges.category,
            },
        });

        // Revert the transaction's modified category to the original value on failure.
        if (transaction) {
            failureData.push({
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
                value: {
                    modifiedCategory: transaction.modifiedCategory,
                },
            });
        }
    }

    ...
}

• Add some Unit Tests here to ensure that the getCategory method is returning the correct information across transaction scenarios.

This solution will work properly If the back end returns the modifiedCategory when the UpdateMoneyRequestCategory request is completed.

If you have any questions or suggestions, please let me know.

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[jliexpensify]

Waiting on @fedirjh

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[fedirjh]

I am still able to replicate on staging.

[fedirjh]

@jliexpensify @wlegolas After reconsidering, I believe that the current behavior is expected. I can replicate this behavior with other fields that have a list, such as tags and taxes. To reliably reproduce the behavior:

1. Submit new expense
2. Go offline
3. Update the category field several times (select and deselect or select different categories)
4. Open category selector again
5. Go online

CleanShot.2024-07-22.at.16.45.08.mp4

[fedirjh]

@jliexpensify I think we can just close this for now.

[jliexpensify]

Ok, that's fair - thanks for doing some additional testing @fedirjh , closing!