Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.87k forks source link

[HOLD for payment 2024-09-24] [$125] Import categories - Error message for multiple GL code mappings displays "glCode" #48530

Closed IuliiaHerets closed 1 month ago

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

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to Control workspace settings > Categories.
  3. Click 3-dot menu.
  4. Click Import spreadsheet.
  5. Import a CSV file that has multiple columns.
  6. Map Name, Enabled.
  7. Map GL code a few times.
  8. Click Import.

Expected Result:

The error message for multiple GL code mappings will display "GL code".

Actual Result:

The error message for multiple GL code mappings displays "glCode".

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/ce245714-74c6-407c-818c-3e63ac70e8f5

Bug6592371_1725411660406!Independent.-.Multi.Level.tags.csv

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016b3b7ee11227810d
  • Upwork Job ID: 1831261318645362164
  • Last Price Increase: 2024-09-04
Issue OwnerCurrent Issue Owner: @mkhutornyi
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @greg-schroeder (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 2 months ago

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

github-actions[bot] commented 2 months 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:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
IuliiaHerets commented 2 months ago

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

daledah commented 2 months ago

Proposal

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

The error message for multiple GL code mappings displays "glCode".

What is the root cause of that problem?

In Onyx, columns are stored as column values, not column text:

Screenshot 2024-09-04 at 16 16 10

Then in

https://github.com/Expensify/App/blob/562d7b48226247253625b9ac1fd45266cb53eda8/src/pages/workspace/categories/ImportedCategoriesPage.tsx#L56

and

https://github.com/Expensify/App/blob/562d7b48226247253625b9ac1fd45266cb53eda8/src/pages/workspace/categories/ImportedCategoriesPage.tsx#L66-L73

We use column value to display duplicate error instead of column text, which result in the error displayed as glCode, meanwhile when displaying missing column error in

https://github.com/Expensify/App/blob/562d7b48226247253625b9ac1fd45266cb53eda8/src/pages/workspace/categories/ImportedCategoriesPage.tsx#L59-L65

we use column text, which results in correct message.

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

Get column text from the column value using columnRoles here

Change this: https://github.com/Expensify/App/blob/562d7b48226247253625b9ac1fd45266cb53eda8/src/pages/workspace/categories/ImportedCategoriesPage.tsx#L69

to:

        const duplicateColumn = columnRoles.find((role) => role.value === duplicate);
        errors.duplicates = translate('spreadsheet.singleFieldMultipleColumns', duplicateColumn?.text ?? '');

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

madmax330 commented 2 months ago

@daledah can you find the offending PR?

shubham1206agra commented 2 months ago

@madmax330 It is https://github.com/Expensify/App/pull/47827 cc @filip-solecki @rushatgabhane.

I think this feature is behind beta. Can someone confirm?

mountiny commented 2 months ago

cc @filip-solecki @rushatgabhane @rlinoz coming from the import spreadsheet PR

mountiny commented 2 months ago

This feels super minor so demoting

melvin-bot[bot] commented 2 months ago

Upwork job price has been updated to $125

filip-solecki commented 2 months ago

I think @daledah proposal looks good!

daledah commented 2 months ago

@madmax330 Any updates on this issue?

madmax330 commented 2 months ago

@rushatgabhane seems like you are familiar with this, can you chime in here?

greg-schroeder commented 1 month ago

Waiting to see what Rushat has to say!

rushatgabhane commented 1 month ago

yeah this seems very minor.

@daledah's proposal should work

rushatgabhane commented 1 month ago

cc: @madmax330

melvin-bot[bot] commented 1 month ago

📣 @daledah You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

daledah commented 1 month ago

@rushatgabhane PR is ready.

trjExpensify commented 1 month ago

CSV import is a #wave-control project, moving this over.

greg-schroeder commented 1 month ago

Oops. sorry, thanks @trjExpensify

greg-schroeder commented 1 month ago

PR is merged and has been deployed to staging. awaiting deploy to prod

greg-schroeder commented 1 month ago

Prod automation didn't work, updated title/payment date

greg-schroeder commented 1 month ago

Payment summary:

Contributor: @daledah - $125 - Will be paid via Upwork on 9/24 C+: @rushatgabhane - $125 - Can make a manual request on ND on 9/24

garrettmknight commented 1 month ago

$125 approved for @rushatgabhane

melvin-bot[bot] commented 1 month ago

Payment Summary

Upwork Job

BugZero Checklist (@greg-schroeder)

greg-schroeder commented 1 month ago

@daledah can you please follow the requested steps here so I can pay you? thanks

greg-schroeder commented 1 month ago

NVM, offer sent via Upwork, I believe I found your profile. Will pay ASAP once you accept

daledah commented 1 month ago

Will pay ASAP once you accept

@greg-schroeder Done that thanks

greg-schroeder commented 1 month ago

paid