-
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
[$250] [Dupe detection] Tax field does not show the "Default" label on the confirmation page #47975
Comments
Triggered auto assignment to @stephanieelliott ( |
We think that this bug might be related to #wave-control |
@stephanieelliott FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
ProposalPlease re-state the problem that we are trying to solve in this issue.Dupe detect - Tax field does not show the "Default" label on the confirmation page What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?When we modify the transaction, we need to store the remaining properties of the comment object. To do this we need to make few changes.
if (allCommentsAreEqual || allCommentsAreEmpty) {
keep[fieldName] = firstTransaction?.comment?.comment ?? firstTransaction?.comment;
keep.comment = firstTransaction?.comment;
comment: {...reviewDuplicateTransaction?.comment, comment: reviewDuplicateTransaction?.description},
const comment = compareResult.change.description?.find((d) => d?.comment === data.value);
setReviewDuplicatesKey({description: data.value, comment}); Test BranchWhat alternative solutions did you explore? (Optional)Alternatively we can store the comment object with updated comment instead of description text: App/src/libs/TransactionUtils/index.ts Lines 987 to 992 in 0c84552
NOTE: We also need to make changes in few other files. The changes are bit similar to solution above. Please checkout the text branch below for a better understanding. Test Branch - Alternative SolutionResultMonosnap.screencast.2024-08-25.06-52-08.mp4 |
updated the repro steps slightly -- actual result shows |
Job added to Upwork: https://www.upwork.com/jobs/~017284a9effe70faa8 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov ( |
and another one! CC: @dylanexpensify @pecanoro @parasharrajat @kubabutkiewicz |
@alitoshmatov Could you review the proposals when you have some time? |
I can do that @pecanoro if needed. |
Putting a HOLD here for now since it seems to be the same as #47974 |
@pecanoro, I have proposals for both issues, and I can assure you that they are different. In this issue, we aren't persisting all the properties of the comment object, whereas in issue #47974, the comparison of the I think we can remove the hold. |
I still think we should wait since the proposals are really similar, and they will need to be updated accordingly. |
@pecanoro, @stephanieelliott, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this? |
Still holding on #47974 |
1 similar comment
Still holding on #47974 |
Still holding |
Still holding on #47974 |
Im not able to reproduce this anymore -- can anyone else repro? |
@stephanieelliott, issue is still reproducible. Monosnap.screencast.2024-10-04.08-32-51.mp4 |
Ok @Krishna2323 can you make any necessary updates to your proposal, then let us know when it's ready so @alitoshmatov can review? |
Bump on the above @Krishna2323, did still want to work on this? |
@stephanieelliott, I will update my proposal/test branch today. |
Hey @Krishna2323 have you had a chance to update the proposal? |
@stephanieelliott sorry for the delay, I have updated the main solution's test branch. @alitoshmatov can start reviewing. @alitoshmatov Let me know if you need a test branch for alternative solution too. |
Hey @alitoshmatov can you review the updated proposal? #47975 (comment) |
Bump @alitoshmatov |
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.24-1
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Action Performed:
Precondition:
Expected Result:
Tax field will show the "Default" label on the confirmation page.
Actual Result:
Tax field does not show the "Default" label on the confirmation page and instead shows "None"
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6581494_1724513644363.20240824_232902.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: