[HOLD for payment 2024-12-07] [$250] [Dupe detection] Tax field does not show the "Default" label on the confirmation page

[IuliiaHerets]

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:

• Track tax is enabled in Workspace settings > Distance rates > Settings.
• Distance rate has Tax rate and Tax reclaimable fields populated.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit two same distance expenses with the same distance rate that has tax rate and tax reclaimable on.
4. Go to transaction thread of any submitted expense.
5. Note that Tax field has the "Default" label.
6. Click Review duplicates.
7. Click Keep this on (any).
8. Proceed to confirmation page.

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:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6581494_1724513644363.20240824_232902.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017284a9effe70faa8
• Upwork Job ID: 1828590697351581643
• Last Price Increase: 2024-11-13
• Automatic offers:
• Krishna2323 | Contributor | 104974640

Issue Owner

Current Issue Owner: @stephanieelliott

[melvin-bot]

Triggered auto assignment to @stephanieelliott (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.

[IuliiaHerets]

We think that this bug might be related to #wave-control

[IuliiaHerets]

@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

[Krishna2323]

Proposal

Please 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?

• To get the default tax rate we need comment?.customUnit?.customUnitRateID?.toString(); in the transactionID.
  

  App/src/libs/TransactionUtils.ts

  Lines 748 to 769 in 73f97c9

  	/**
  	* Get rate ID from the transaction object
  	*/
  	function getRateID(transaction: OnyxInputOrEntry<Transaction>): string | undefined {
  	return transaction?.comment?.customUnit?.customUnitRateID?.toString();
  	}
  	/**
  	* Gets the tax code based on the type of transaction and selected currency.
  	* If it is distance request, then returns the tax code corresponding to the custom unit rate
  	* Else returns policy default tax rate if transaction is in policy default currency, otherwise foreign default tax rate
  	*/
  	function getDefaultTaxCode(policy: OnyxEntry<Policy>, transaction: OnyxEntry<Transaction>, currency?: string | undefined): string | undefined {
  	if (isDistanceRequest(transaction)) {
  	const customUnitRateID = getRateID(transaction) ?? '';
  	const customUnitRate = getCustomUnitRate(policy, customUnitRateID);
  	return customUnitRate?.attributes?.taxRateExternalID ?? '';
  	}
  	const defaultExternalID = policy?.taxRates?.defaultExternalID;
  	const foreignTaxDefault = policy?.taxRates?.foreignTaxDefault;
  	return policy?.outputCurrency === (currency ?? getCurrency(transaction)) ? defaultExternalID : foreignTaxDefault;
  	}

• But when we call buildNewTransactionAfterReviewingDuplicates on confirmation page the comment object is only left with a comment property, all other properties are removed.
  

  App/src/libs/TransactionUtils.ts

  Lines 952 to 963 in 73f97c9

  	function buildNewTransactionAfterReviewingDuplicates(reviewDuplicateTransaction: OnyxEntry<ReviewDuplicates>): Partial<Transaction> {
  	const originalTransaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${reviewDuplicateTransaction?.transactionID}`] ?? undefined;
  	const {duplicates, ...restReviewDuplicateTransaction} = reviewDuplicateTransaction ?? {};
  	return {
  	...originalTransaction,
  	...restReviewDuplicateTransaction,
  	modifiedMerchant: reviewDuplicateTransaction?.merchant,
  	merchant: reviewDuplicateTransaction?.merchant,
  	comment: {comment: reviewDuplicateTransaction?.description},
  	};
  	}

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.

• We first need to store the comment object in keep object.
  

  App/src/libs/TransactionUtils/index.ts

  Lines 991 to 993 in 0c84552

  	if (allCommentsAreEqual || allCommentsAreEmpty) {
  	keep[fieldName] = firstTransaction?.comment?.comment ?? firstTransaction?.comment;
  	} else {

                if (allCommentsAreEqual || allCommentsAreEmpty) {
                    keep[fieldName] = firstTransaction?.comment?.comment ?? firstTransaction?.comment;
                    keep.comment = firstTransaction?.comment;

• In buildNewTransactionAfterReviewingDuplicates, we need to merge the remaining properties of the comment object.
  

  App/src/libs/TransactionUtils/index.ts

  Line 1030 in 0c84552

  comment: {comment: reviewDuplicateTransaction?.description},

comment: {...reviewDuplicateTransaction?.comment, comment: reviewDuplicateTransaction?.description},

• When updating the description in ReviewDescription, we need to find the correct comment object and update it in the REVIEW_DUPLICATES ONYX object.
  

  App/src/pages/TransactionDuplicate/ReviewDescription.tsx

  Lines 34 to 39 in 0c84552

  	const setDescription = (data: FieldItemType<'description'>) => {
  	if (data.value !== undefined) {
  	setReviewDuplicatesKey({description: data.value});
  	}
  	navigateToNextScreen();
  	};

            const comment = compareResult.change.description?.find((d) => d?.comment === data.value);
            setReviewDuplicatesKey({description: data.value, comment});

Test Branch

What 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

	if (fieldName === 'description') {
	const allCommentsAreEqual = areAllCommentsEqual(transactions, firstTransaction);
	const allCommentsAreEmpty = isFirstTransactionCommentEmptyObject && transactions.every((item) => item?.comment === undefined);
	if (allCommentsAreEqual || allCommentsAreEmpty) {
	keep[fieldName] = firstTransaction?.comment?.comment ?? firstTransaction?.comment;

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 Solution

Result

Monosnap.screencast.2024-08-25.06-52-08.mp4

[stephanieelliott]

updated the repro steps slightly -- actual result shows None instead of the expected default rate.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017284a9effe70faa8

[melvin-bot]

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

[trjExpensify]

and another one! CC: @dylanexpensify @pecanoro @parasharrajat @kubabutkiewicz

[pecanoro]

@alitoshmatov Could you review the proposals when you have some time?

[parasharrajat]

I can do that @pecanoro if needed.

[pecanoro]

Putting a HOLD here for now since it seems to be the same as #47974

[Krishna2323]

@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 comment.comment string is incorrect.

I think we can remove the hold.

[melvin-bot]

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

[Krishna2323]

Will raise PR tomorrow. Thanks for waiting🙇🏻

[Krishna2323]

@alitoshmatov PR ready for review ^

[stephanieelliott]

Hey @alitoshmatov the PR is blocked on your review, can you take a look? #52928

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.68-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: [Dupe detection] Tax field does not show the "Default" label on the confirmation page #52928

If no regressions arise, payment will be issued on 2024-12-07. 🎊

For reference, here are some details about the assignees on this issue:

• @alitoshmatov requires payment through NewDot Manual Requests
• @Krishna2323 requires payment automatic offer (Contributor)

[melvin-bot]

@alitoshmatov @stephanieelliott @alitoshmatov 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]

[alitoshmatov]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 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:

• 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
• 3z. Other:

• [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/42571/files#r1878801476

• [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: No discussion started

• [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.

  I don't think regression test is needed

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @Krishna2323 $250 via Upwork - PAID!
• Contributor+: @alitoshmatov $250 via ND - Please request!

Upwork job is here: https://www.upwork.com/jobs/~017284a9effe70faa8

[JmillsExpensify]

$250 approved for @alitoshmatov