[HOLD for payment 2024-10-29] [$250] Import categories - Dropdown button does not match the selection in the dropdown list

[IuliiaHerets]

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: 9.0.45-2 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Categories.
3. Click 3-dot menu > Import spreadsheet.
4. Upload a csv containing two column data.
5. Click on the dropdown of the first item.

Expected Result:

The selection in the dropdown should match what is displayed on the dropdown button.

Actual Result:

The dropdown button shows "Enabled", but the selection in the dropdown is "Ignore".

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/48118e97-396a-46ef-a88e-e1e98b26c949

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021843456236485457533
• Upwork Job ID: 1843456236485457533
• Last Price Increase: 2024-10-08
• Automatic offers:
• ZhenjaHorbach | Reviewer | 104404493
• Nodebrute | Contributor | 104404495

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot[bot]]

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

[IuliiaHerets]

We think that this bug might be related to #wave-control

[IuliiaHerets]

@kadiealexander 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

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-10-06 20:13:28 UTC.

Proposal

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

Dropdown button does not match the selection in the dropdown list

What is the root cause of that problem?

This issue occurs not only with categories but also with tags. In this case, the defaultSelectedIndex will return -1 if the index is not found. https://github.com/Expensify/App/blob/6d8e739f7b1c286ae65565a6689b27b5983fe48a/src/components/ImportColumn.tsx#L165 And then we pass this defaultSelectedIndex here, which is incorrect we should pass index 0 which is index of ignore https://github.com/Expensify/App/blob/6d8e739f7b1c286ae65565a6689b27b5983fe48a/src/components/ImportColumn.tsx#L204

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

In cases where the defaultSelectedIndex is -1 we should pass 0 here https://github.com/Expensify/App/blob/6d8e739f7b1c286ae65565a6689b27b5983fe48a/src/components/ImportColumn.tsx#L204

We can do something like this (pseudo-code)

    const defaultSelectedIndex = columnRoles.findIndex((item) => item.value === colName);
    const finalIndex = defaultSelectedIndex !== -1 ? defaultSelectedIndex : 0;

and then we can pass this finalIndex here https://github.com/Expensify/App/blob/6d8e739f7b1c286ae65565a6689b27b5983fe48a/src/components/ImportColumn.tsx#L204

Some cleanup may be required, which we can address in the PR. Result

https://github.com/user-attachments/assets/19886efd-dad6-4280-aaf7-56a0888522a5

[!NOTE]
There are multiple ways to achieve the same result, but I'm keeping the proposal simple by avoiding excessive options.

What alternative solutions did you explore? (Optional)

Even though the index of 'ignore' is consistently 0 across all pages, we can implement additional checks to make this more error-proof.

    const defaultSelectedIndex = columnRoles.findIndex((item) => item.value === colName);
    const finalIndex = defaultSelectedIndex !== -1 ? defaultSelectedIndex : columnRoles.findIndex(item => item.value === 'ignore');

[Nodebrute]

Proposal Updated Added POC

[twilight2294]

Proposal

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

Dropdown button does not match the selection in the dropdown list

What is the root cause of that problem?

The dropdown button highlight is selected based on the isSelected prop:

https://github.com/Expensify/App/blob/134bbd0faf3171e30c0d7bd1349e05de5d5adbba/src/components/ImportColumn.tsx#L155-L160

But when we get the defaultSelectedIndex, we are essentially comparing against the colName which is derived from column hence we always get -1 as the return value. This will display the last option in the list as the header value of the dropdown.

https://github.com/Expensify/App/blob/6d8e739f7b1c286ae65565a6689b27b5983fe48a/src/components/ImportColumn.tsx#L165

the reason this check fails is that the values of colName are:

Hence they will never match with the values of columnRoles:

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

We should instead get the value from options/ columnRoles itself by checking if isSelected is set to true and if yes them we should show that option as the header:

    const defaultSelectedIndex = options.findIndex((item) => item.isSelected === true)

We should fix this in other places where csv import is used, for cases where none is selected we shouldn't display any value for the header, we should consult with design team for the exact behaviour for this case

https://github.com/user-attachments/assets/d44301c9-3c50-44f9-84d6-859610b5a587

What alternative solutions did you explore? (Optional)

[mkzie2]

Proposal

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

Import categories - Dropdown button does not match the selection in the dropdown list

What is the root cause of that problem?

This bug happens with the column that returns the columnName as empty in the default case here

It leads the defaultSelectedIndex to be -1 which causes the display text in the dropdown button to be different from the selected option in the drop-down menu https://github.com/Expensify/App/blob/7981265dc14bc9cb99b512baddbb027446568fcf/src/components/ImportColumn.tsx#L165

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

For the default case, we should return CONST.CSV_IMPORT_COLUMNS.IGNORE here instead of an empty string.

default:
    attribute = CONST.CSV_IMPORT_COLUMNS.IGNORE;
    break;

https://github.com/Expensify/App/blob/7981265dc14bc9cb99b512baddbb027446568fcf/src/components/ImportColumn.tsx#L114-L116

What alternative solutions did you explore? (Optional)

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[trjExpensify]

This is a wave-control project, moving.

[ZhenjaHorbach]

I will check proposals today or tomorrow !

[ZhenjaHorbach]

@Nodebrute @twilight294 @mkzie2 Thanks for proposals !

@Nodebrute @mkzie2 Your proposals look similar (Have the same result) But I'm not sure that we should stick to a specific value Because if this value will be missing in a list, issue will be reproduced I know that now we have Ignore on every Options list at the moment (But I won't be surprised if something changes in the future) So I would prefer some more flexible solution

@twilight294 Your proposal looks promising But unfortunately it doesn't work for me Could you please provide more code?

[Nodebrute]

@ZhenjaHorbach, @mkzie2's solution will not work. You can try importing this file in the tags import section, and you'll see that the error will still be reproducible with the GL code value. Categories-2024-09-10 17_10_20.413.csv

[Nodebrute]

@mkzie2's solution will not work because, in this case, the colName will return glcode. https://github.com/Expensify/App/blob/d6d300798d9430b89d9ac11dea01b10f7c1ff779/src/components/ImportColumn.tsx#L34 And in this case, when we don't pass 'glcode' in the tags, it will return -1, thus reproducing the error. https://github.com/Expensify/App/blob/d6d300798d9430b89d9ac11dea01b10f7c1ff779/src/components/ImportColumn.tsx#L165

@ZhenjaHorbach I have proposed two solutions. The first is to fallback to the 0 index. Let’s assume we don’t have ignore in the future, which I believe will never happen😅, but at the very least, we will have a default value at the 0 index. So, instead of falling back to ignore, we can always fallback to the 0 index.

My second solution, where we can find the index of ignore instead of falling back to 0, is quite reliable. When we set the spreadsheet data, we use ignore, so ignore will remain our default for a long time. https://github.com/Expensify/App/blob/d6d300798d9430b89d9ac11dea01b10f7c1ff779/src/libs/actions/ImportSpreadsheet.ts#L13

[Nodebrute]

@ZhenjaHorbach Sorry for the message overload! I just checked OD as well, and it seems ignore is the default for everything there too. Let me know what you think about the points above.

[mkzie2]

@ZhenjaHorbach Here is why I propose adding the ignore as a fallback in the findColumnName function.

When we import a CSV file, we save the spreadsheet data, and all column names are ignore by default. Then the column name will be updated based on the value in findColumnName function.

https://github.com/Expensify/App/blob/f0be8a731dc0832827dbd14d23fd49b332ff275e/src/libs/actions/ImportSpreadsheet.ts#L12-L15

It will not update in the case empty string is returned because we already have the check here. But the defaultSelectedIndex is also used to focus and display the correct column value then it makes sense to return ignore by default https://github.com/Expensify/App/blob/f0be8a731dc0832827dbd14d23fd49b332ff275e/src/components/ImportColumn.tsx#L167-L174

[twilight2294]

@twilight294 Your proposal looks promising But unfortunately it doesn't work for me Could you please provide more code?

oh my bad, actually we have to add extra type prop to DropdownOption here, Below is the test branch and a test csv with categories, do let me know if you have any more doubts:

CSV: Categories-2024-10-10 05_30_44.389.csv

Test PR: - https://github.com/Expensify/App/pull/50542

[Nodebrute]

@twilight294 in your test branch you are using 0 as fallback, similar to my proposal.

Cc: @ZhenjaHorbach

[ZhenjaHorbach]

@mkzie2's solution will not work because, in this case, the colName will return glcode.

https://github.com/Expensify/App/blob/d6d300798d9430b89d9ac11dea01b10f7c1ff779/src/components/ImportColumn.tsx#L34

And in this case, when we don't pass 'glcode' in the tags, it will return -1, thus reproducing the error. https://github.com/Expensify/App/blob/d6d300798d9430b89d9ac11dea01b10f7c1ff779/src/components/ImportColumn.tsx#L165

@ZhenjaHorbach I have proposed two solutions. The first is to fallback to the 0 index. Let’s assume we don’t have ignore in the future, which I believe will never happen😅, but at the very least, we will have a default value at the 0 index. So, instead of falling back to ignore, we can always fallback to the 0 index.

My second solution, where we can find the index of ignore instead of falling back to 0, is quite reliable. When we set the spreadsheet data, we use ignore, so ignore will remain our default for a long time.

https://github.com/Expensify/App/blob/d6d300798d9430b89d9ac11dea01b10f7c1ff779/src/libs/actions/ImportSpreadsheet.ts#L13

Hmmmm Yes Your alternative solution looks interesting And I think it makes sense to use ignore as the default value in case of something

[ZhenjaHorbach]

I think this proposal make sense in which many details are taken into account

So I'm happy to go with this proposal 🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[twilight2294]

@ZhenjaHorbach we should always compare with options and not columnRoles, as outlined in my proposal here.

c.c. @mjasikowski

[mjasikowski]

@ZhenjaHorbach let's go ahead with @Nodebrute's proposal

[melvin-bot[bot]]

📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @Nodebrute 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[mkzie2]

@ZhenjaHorbach Can you please check again? We should update findColumnName function by returning the default value correctly instead of having a fallback like the selected proposal, it doesn't fix the RCA. I also have an explanation here

const defaultSelectedIndex = columnRoles.findIndex((item) => item.value === colName);
const finalIndex = defaultSelectedIndex !== -1 ? defaultSelectedIndex : columnRoles.findIndex(item => item.value === 'ignore');

[Nodebrute]

@mkzie2, your proposal didn't work.

@ZhenjaHorbach, @mkzie2's solution will not work. You can try importing this file in the tags import section, and you'll see that the error will still be reproducible with the GL code value. Categories-2024-09-10 17_10_20.413.csv

Here are the steps to test your proposal. You'll see that it doesn't fix the problem.

[Nodebrute]

Pr will be ready in few hours

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/50726

If no regressions arise, payment will be issued on 2024-10-29. :confetti_ball:

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

• @ZhenjaHorbach requires payment automatic offer (Reviewer)
• @Nodebrute requires payment automatic offer (Contributor)