Repository navigation
[Due for payment 2025-02-27] [Due for payment 2025-02-18] [Due for payment 2025-02-13] [$500] [Auth Violations] Support isDismissed on newDot to hide violations #53398
Description
Part of the Auth Violations project
Main issue: https://github.com/Expensify/Expensify/issues/422533
Project: https://github.com/Expensify/Expensify/issues/393838
Feature Description
Currently we have four violations that can be dismissed in old Expensify:
- DUPLICATED_TRANSACTION
- SMARTSCAN_FAILED
- RTER
- AUTOREPORTED_REJECTED_EXPENSE (Not implemented in newDot yet)
If the violation has been dismissed, we don't show the violation to the user. In PHP, we currently have a function like this:
/**
* Checks whether a violation has been dismissed, optionally by a particular email address.
*
* @param string $violationName check Policy_Violation::NAME_*
*/
public function isViolationDismissed(string $violationName, string $email = ''): bool
{
// If we don't provide an email, we just check if the violation has been dismissed by anyone
if (!$email) {
return count($this->getNVP('dismissedViolations')[$violationName] ?? []) > 0;
}
$timestamp = $this->getNVP('dismissedViolations')[$violationName][$email] ?? '';
return strlen(strval($timestamp)) > 0;
}Note: The NVP is stored in the transaction's "comment" along with the rest of the name-value pairs.
Ideally, we should add this logic in newDot similar to what we did for duplicate transactions here. My best guess is that we need to make that logic work in these functions here so we stop showing the violation in the LHN and the transaction itself. However, I might be missing other spots.
Take into account that checking for dismissed violations should be optional in case there are flows in which we want to show it regardless or block submission because of that.
Manual Test Steps
- Scan a receipt in which the smartscan will fail (illegible or not a receipt)
- Go to old Expensify, find the report and dismiss the violation.
- Go back to new Expensify and make sure the violation is not showing anymore.
Note: You might need to refresh the page, I am not sure if the BE is queueing these updates, let me know if not.
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021864348688314983900
- Upwork Job ID: 1864348688314983900
- Last Price Increase: 2024-12-11
- Automatic offers:
- rojiphil | Reviewer | 105371307
- Krishna2323 | Contributor | 105371309
Activity
melvin-bot commentedon Dec 4, 2024
Triggered auto assignment to @johncschuster (
NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.melvin-bot commentedon Dec 4, 2024
melvin-bot commentedon Dec 4, 2024
Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify (
NewFeature)[-][Auth Violations] Support isDismissed on newDot to hide violations[/-][+][$250] [Auth Violations] Support isDismissed on newDot to hide violations[/+]melvin-bot commentedon Dec 4, 2024
Job added to Upwork: https://www.upwork.com/jobs/~021864348688314983900
melvin-bot commentedon Dec 4, 2024
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rojiphil (
External)73 remaining items
melvin-bot commentedon Feb 18, 2025
@rojiphil, @johncschuster, @pecanoro, @Krishna2323 Eep! 4 days overdue now. Issues have feelings too...
johncschuster commentedon Feb 18, 2025
Payment has been issued to @Krishna2323. I'll issue payment to @rojiphil once the BZ Checklist is complete 👍
rojiphil commentedon Feb 19, 2025
BugZero Checklist:
Bug classification
Source of bug:
Where bug was reported:
Who reported the bug:
[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: No offending PR as this was a new feature/improvement.
[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: Not Required.
[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.
Regression Test Proposal
Precondition:
Test:
duplicatetransactions.Do we agree 👍 or 👎
rojiphil commentedon Feb 19, 2025
@johncschuster BZ Checklist done for adding test case to the test case repository. Otherwise, the checklist was not required as this was a new feature/improvement, not a bug. Thanks.
johncschuster commentedon Feb 19, 2025
Payment Summary
Contributor: @Krishna2323 paid $500 via Upwork
Contributor+: @rojiphil owed $500 via NewDot
@rojiphil please request payment via NewDot!
rojiphil commentedon Feb 19, 2025
Requested in ND
[-][Due for payment 2025-02-18] [Due for payment 2025-02-13] [$500] [Auth Violations] Support isDismissed on newDot to hide violations[/-][+][Due for payment 2025-02-27] [Due for payment 2025-02-18] [Due for payment 2025-02-13] [$500] [Auth Violations] Support isDismissed on newDot to hide violations[/+]melvin-bot commentedon Feb 20, 2025
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.1-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2025-02-27. 🎊
For reference, here are some details about the assignees on this issue:
melvin-bot commentedon Feb 20, 2025
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
garrettmknight commentedon Mar 1, 2025
$500 approved for @rojiphil