Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] Taxes - RBR is present in expense preview but no error or violation when tax rate is invalid #39798

Closed
6 tasks done
izarutskaya opened this issue Apr 7, 2024 · 18 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Apr 7, 2024

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.60-13
Reproducible in staging?: Y
Reproducible in production?: Y
Found when validating PR : #39707
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

  • Admin and employee are in the same Collect workspace.
  • The Collect workspace has several tax rates.
  1. Go to staging.new.expensify.com
  2. [Employee] Go to workspace chat.
  3. [Employee] Create manual request with a tax rate in workspace chat.
  4. [Admin] On Old Dot, delete the tax rate selected in Step 3.
  5. [Employee] Go to Settings > About > Troubleshoot > Clear cache and restart > Reset and refresh (relogin can also be done).
  6. [Employee] Navigate to the workspace chat and transaction thread with invalid tax rate.

Expected Result:

There will be an error or violation under tax rate to indicate that the tax rate is no longer valid.

Actual Result:

There is RBR in the expense preview in the workspace chat, but there is no error message or violation for the invalid tax rate in the transaction thread

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

Bug6439975_1712384377676.bandicam_2024-04-06_14-10-20-712.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~018e4229ab6da1ceae
  • Upwork Job ID: 1779881866618400768
  • Last Price Increase: 2024-04-15
  • Automatic offers:
    • s77rt | Reviewer | 0
    • nkdengineer | Contributor | 0
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 7, 2024
Copy link

melvin-bot bot commented Apr 7, 2024

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@izarutskaya
Copy link
Author

We think this issue might be related to the #collect project.

@nkdengineer
Copy link
Contributor

nkdengineer commented Apr 8, 2024

Proposal

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

There is RBR in the expense preview in the workspace chat, but there is no error message or violation for the invalid tax rate in the transaction thread

What is the root cause of that problem?

In report preview, we show the RBR if any transaction has error or has violation

const hasErrors = hasMissingSmartscanFields || (canUseViolations && ReportUtils.hasViolations(iouReportID, transactionViolations)) || ReportUtils.hasActionsWithErrors(iouReportID);

But in MoneyRequestView, we don't get the error for tax field and pass it to MenuItemWithTopDescription as we do for other fields like tag, ...

<OfflineWithFeedback pendingAction={getPendingFieldAction('taxCode')}>

brickRoadIndicator={getErrorForField('tag', {tagListIndex: index, tagListName: name}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('tag', {tagListIndex: index, tagListName: name})}

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

We should get the error for tax field via getErrorField function and pass brickRoadIndicator and error prop into
MenuItemWithTopDescription for tax field here

brickRoadIndicator={getErrorForField('tax') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('tax')}

<OfflineWithFeedback pendingAction={getPendingFieldAction('taxCode')}>

OPTIONAL: We can show the error the both tax code and tax amount.

What alternative solutions did you explore? (Optional)

NA

Result

Screenshot 2024-04-08 at 11 22 25

@melvin-bot melvin-bot bot added the Overdue label Apr 9, 2024
Copy link

melvin-bot bot commented Apr 10, 2024

@muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Apr 12, 2024

@muttmuure Huh... This is 4 days overdue. Who can take care of this?

@muttmuure muttmuure added the External Added to denote the issue can be worked on by a contributor label Apr 15, 2024
@melvin-bot melvin-bot bot changed the title Taxes - RBR is present in expense preview but no error or violation when tax rate is invalid [$250] Taxes - RBR is present in expense preview but no error or violation when tax rate is invalid Apr 15, 2024
Copy link

melvin-bot bot commented Apr 15, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 15, 2024
Copy link

melvin-bot bot commented Apr 15, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Apr 15, 2024
@s77rt
Copy link
Contributor

s77rt commented Apr 16, 2024

@nkdengineer Thanks for the proposal. Your RCA is correct and the solution looks good to me, let's fix the bug for all affected fields.

🎀 👀 🎀 C+ reviewed
Link to proposal

Copy link

melvin-bot bot commented Apr 16, 2024

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

@stitesExpensify
Copy link
Contributor

LGTM!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 16, 2024
Copy link

melvin-bot bot commented Apr 16, 2024

📣 @s77rt 🎉 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

Copy link

melvin-bot bot commented Apr 16, 2024

📣 @nkdengineer 🎉 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 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Apr 17, 2024
@nkdengineer
Copy link
Contributor

@s77rt The PR is here.

@muttmuure
Copy link
Contributor

Looks like this needs to be paid out?

@s77rt
Copy link
Contributor

s77rt commented May 2, 2024

Yes but let's give this another test since QA team reported this as still reproducible in the PR #40334 (comment)

cc @nkdengineer

@nkdengineer
Copy link
Contributor

@s77rt Replied in the PR. It's fixed for me on the latest main. For staging, my account doesn't have violation permission so I can't test.

@muttmuure
Copy link
Contributor

Going to close this as fixed

@muttmuure
Copy link
Contributor

Everyone is paid out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Archived in project
Development

No branches or pull requests

5 participants