[HOLD for payment 2024-12-05] [$250] Dupe detection - RBR is not shown when creating dupe expense with a Gmail account
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: v9.0.59-0 Reproducible in staging?: Y Reproducible in production?: Y Issue was found when executing this PR: https://github.com/Expensify/App/pull/48958 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team
Action Performed:
- Login to an account.
- Create a workspace
- Open the Workspace chat
- Submit an expense in the Workspace with a category,
- Submit another expense in the Workspace with a the same amount and merchant in the above but with no category.
Expected Result:
RBR(red dot) is shown in LHN and also in the report because of the dupe expense.
Actual Result:
RBR(red dot) is not shown in LHN and in the report. RBR is only shown inside the expense report
Workaround:
Unknown
Platforms:
- [ ] Android: Standalone
- [ ] Android: HybridApp
- [x] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/6067710d-aceb-409e-b71b-b12145014331
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021857074546459286027
- Upwork Job ID: 1857074546459286027
- Last Price Increase: 2024-11-21
- Automatic offers:
- ahmedGaber93 | Reviewer | 105026209
Issue Owner
Current Issue Owner: @muttmuure
Triggered auto assignment to @muttmuure (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.
Edited by proposal-police: This proposal was edited at 2024-11-21 13:28:28 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
RBR is not shown on the report preview when money request is dupe.
What is the root cause of that problem?
The RBR doesn't show on the report preview because it only shows if the dupe detection permission is enabled. IF the account doesn't have the permission, then RBR won't show. https://github.com/Expensify/App/blob/5e69bfc36ddfd82a113606257abb93f2fa78f7db/src/components/ReportActionItem/ReportPreview.tsx#L156-L160 https://github.com/Expensify/App/blob/5e69bfc36ddfd82a113606257abb93f2fa78f7db/src/libs/TransactionUtils/index.ts#L832-L842
However, on the individual expense preview, we just checks for the amount of the duplicates violation without checking whether the permission is enabled or not, which is different form hasWarningTypeViolations.
https://github.com/Expensify/App/blob/5e69bfc36ddfd82a113606257abb93f2fa78f7db/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L147-L149
So, I think there are 2 issues here. In ReportPreview, we check for warning type violation. In MoneyRequestPreviewContent, we don't check for warning type violation, but we check for duplicates, ignoring the dupe detection permission.
For the RBR doesn't show for workspace chat in the LHN issue, we already have a logic to do that actually. https://github.com/Expensify/App/blob/5955bd797eaaf60c2b02db95a35c18663d47effe/src/libs/ReportUtils.ts#L6352-L6356
It checks all the related report and see if there are violations. However, hasWarningTypeViolations always returns false. It happens because the warningTypeViolations here is empty. Even though the dupe violation is a warning type, the showInReview is not null.
https://github.com/Expensify/App/blob/5955bd797eaaf60c2b02db95a35c18663d47effe/src/libs/TransactionUtils/index.ts#L886-L891
It's because we pass nothing to showInReview. So, undefined !== null.
https://github.com/Expensify/App/blob/5955bd797eaaf60c2b02db95a35c18663d47effe/src/libs/ReportUtils.ts#L6355 https://github.com/Expensify/App/blob/5955bd797eaaf60c2b02db95a35c18663d47effe/src/libs/ReportUtils.ts#L6370-L6372
What changes do you think we should make in order to solve the problem?
Let's use hasWarningTypeViolations in MoneyRequestPreviewContent too.
const hasWarningTypeViolations = TransactionUtils.hasWarningTypeViolation(transaction?.transactionID ?? '-1', transactionViolations);
...
const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasWarningTypeViolations;
(MoneyRequestPreviewContent also checks for hasNoticeTypeViolation but ReportPreview don't. We can add it to ReportPreview too if needed by adding it to ReportUtils, just like the others, but I don't know how to trigger notice type violation, so not sure.)
To fix the LHN RBR issue, we should first update the showInReview type by removing the null type. Then, check for undefined instead of null.
https://github.com/Expensify/App/blob/5955bd797eaaf60c2b02db95a35c18663d47effe/src/libs/TransactionUtils/index.ts#L886-L891
(or we can just pass update showInReview to be a required param but keep it nullable)
@muttmuure Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!
@muttmuure Huh... This is 4 days overdue. Who can take care of this?
Job added to Upwork: https://www.upwork.com/jobs/~021857074546459286027
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ahmedGaber93 (External)
@bernhardoj Thanks for the proposal
Your proposal fix the issue on report preview but not on LHN. Can you also fix it?
https://github.com/user-attachments/assets/7a611cc4-d524-4546-9194-1bf974d19dae
I'm not sure if we should show the RBR on the workspace chat. In this PR, they mention that the RBR should show on the workspace chat, but this comment says otherwise.
NOTE: the previous logic before the PR also doesn't show expense report violations RBR on workspace chat.
they mention that the RBR should show on the workspace chat, but this https://github.com/Expensify/App/pull/51893#issuecomment-2468956992 says otherwise.
I think they said it should show on some cases only
@iwiznia Just for confirmation, in the duplication violations case, should we show RBR on the workspace chat on LHN?
Yes, RBRs in the LHN should only show in the workspace chat
Thanks for confirmation
@bernhardoj Bump for this change https://github.com/Expensify/App/issues/52243#issuecomment-2483225270
Ok, I got it now. It's supposed to work but there is a small bug. Updated my proposal to fix that.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Updated my proposal to fix that.
Great, I will review it tomorrow
@ahmedGaber93 @muttmuure 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!
Triggered auto assignment to @AndrewGable, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
π£ @ahmedGaber93 π An offer has been automatically sent to your Upwork account for the Reviewer role π Thanks for contributing to the Expensify app!
PR is ready
cc: @ahmedGaber93
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.67-9 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/52994
If no regressions arise, payment will be issued on 2024-12-05. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @ahmedGaber93 requires payment automatic offer (Reviewer)
- @bernhardoj requires payment through NewDot Manual Requests
@ahmedGaber93 @muttmuure @ahmedGaber93 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
BugZero Checklist:
- [x] [Contributor] Classify the bug:
Bug classification
Source of bug:
- [x] 1a. Result of the original design (eg. a case wasn't considered)
- [ ] 1b. Mistake during implementation
- [ ] 1c. Backend bug
- [ ] 1z. Other:
Where bug was reported:
- [x] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
- [ ] 2b. Reported on staging (eg. found during regression or PR testing)
- [ ] 2d. Reported on a PR
- [ ] 2z. Other:
Who reported the bug:
- [ ] 3a. Expensify user
- [ ] 3b. Expensify employee
- [ ] 3c. Contributor
- [ ] 3d. QA
- [x] 3z. Other: Applause Internal Team
-
[x] [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.
Link to comment: https://github.com/Expensify/App/pull/47056/files#r1876926777
-
[x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.
Link to discussion:N/A.
-
[x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
-
[ ] [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.
Link to issue:
There is no need for regression test because this issue related to dupe detection beta and it recently removed here https://github.com/Expensify/App/pull/53167
@muttmuure Could you please add the payment summary here? Also, please ignore the Upwork offer for me (I will request it on ND).
Requested in ND.
@AndrewGable, @ahmedGaber93, @muttmuure, @bernhardoj Eep! 4 days overdue now. Issues have feelings too...
Just waiting on payment to close
I need a payment summary to approve payment