-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Hold for payment 2024-07-31] [Dupe Detection] Merge Duplicates - Confirm Page #39810
Comments
Triggered auto assignment to @sakluger ( |
Callstack will be taking this series of tasks |
Hi, I'm Jakub from Callstack - expert contributor group - and I would like to take a look at this issue. |
Hi, just wanted to inform that I will be OOO from 29.04 to 05.05. |
In progress! |
In progress! |
WIP! |
WIP! |
This issue has not been updated in over 15 days. @pecanoro, @kubabutkiewicz eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
Still WIP! |
I would like to review this page as well given that I reviewed other two and has more context. |
@parasharrajat @pecanoro @JmillsExpensify PR is ready to review 😄 |
Did the automation not run here? Seems like it hit prod 3 days ago: #42571 (comment) |
Hmm, yes, I think it failed |
cc @sakluger this is ready for payment soon. |
Sorry, everyone, I just noticed that this issue slipped through the cracks. Summarizing payment on this issue: Contributor: @kubabutkiewicz $250 - contractor, no payment required @parasharrajat do we need any regression tests for this one? Maybe something similar to the tests laid out in the PR? |
Regression Test Steps
Do you agree 👍 or 👎 ? |
Thanks! Those steps are pretty good, I edited them to be a bit more specific.
|
Payment requested as per #39810 (comment) |
$250 approved for @parasharrajat |
Part of the Merge Duplicates project
Main issue: https://github.com/Expensify/Expensify/issues/307591
Doc section: Surfacing Potential Duplicates
Project: [Wave: Collect Approvers] Dupe Detection
Part of a series of issues:
Feature Description
Context for ReviewDuplicates
All of these screens will exist in their own navigator so they can be pushed (and later popped off once complete) as a group.
All of this will use an Onyx form key of ONYXKEYS.FORMS.REVIEW_DUPLICATES_FORM so we can store the data selected by the user so far when resolving the duplicates. It also lets us keep track of which transaction to keep and which ones are duplicates when the user clicks on Keep this one.
This key will follow a structure like this:
Confirm Page
Our last confirm page will show a similar non-editable version of MoneyRequestView:
Some notes about the above request:
Since we are using pattern B, we need to set the optimistic and failure data. When we merge two or more transactions, we basically need to get rid of the ones that we don’t want to keep and update the one that we want to keep.
transactions_
key in Onyx with the updated fields (merchant, tags, category…) for the transactionID.transactions_
key in Onyx to remove the duplicate violation violation within the comment.transactionViolations_
key in Onyx to remove the duplicate violation. We’ll update theViolationUtils.getViolationsOnyxData
method that calculates the new transaction violations (by adding/removing those that have been addressed with the current user changes), and returns an object that we can save into Onyx, to work with duplicate violations as well.We will navigate the user directly back to the MoneyRequestView of the expenseReport we were keeping.
Issue Owner
Current Issue Owner: @kubabutkiewiczThe text was updated successfully, but these errors were encountered: