[HOLD #39616][$250] [Track Tax] - Tax does not show up in the Expense report when created offline

[lanitochka17]

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: 1.4.62-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Issue found when executing PR #39723

Action Performed:

Prerequisites:
Create WS collect, include Tax, add 5% rate.

1. Open https://staging.new.expensify.com/
2. Navigate to a workspace room
3. Switch on offline
4. Create and send to the IOU room with 5% tax.
5. Navigate to the IOU details

Expected Result:

The tax should appear in the IOU report when created offline

Actual Result:

Tax does not show up in the IOU report when created offline

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6445180_1712786974295.Recording__34.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d25bb36f4e169f12
• Upwork Job ID: 1779966508736303104
• Last Price Increase: 2024-04-15

[melvin-bot]

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

[lanitochka17]

@abekkala 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

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[allgandalf]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Tax amount is not stored optimistically when we create IOU request offline

What is the root cause of that problem?

When we create the request offline, we call IOU.requestMoney and in requestMoney, we build out a report optimistically :

App/src/libs/TransactionUtils.ts

Lines 86 to 103 in 8ae5491

	function buildOptimisticTransaction(
	amount: number,
	currency: string,
	reportID: string,
	comment = '',
	created = '',
	source = '',
	originalTransactionID = '',
	merchant = '',
	receipt: Receipt = {},
	filename = '',
	existingTransactionID: string | null = null,
	category = '',
	tag = '',
	billable = false,
	pendingFields: Partial<{[K in TransactionPendingFieldsKey]: ValueOf<typeof CONST.RED_BRICK_ROAD_PENDING_ACTION>}> | undefined = undefined,
	reimbursable = true,
	): Transaction {

But in here we do not save the tax rate and tax amount optimistically , we only send them to the BE:

App/src/libs/actions/IOU.ts

Lines 2338 to 2352 in 8ae5491

	taxCode,
	taxAmount,
	billable,
	// This needs to be a string of JSON because of limitations with the fetch() API and nested objects
	gpsPoints: gpsPoints ? JSON.stringify(gpsPoints) : undefined,
	transactionThreadReportID,
	createdReportActionIDForThread,
	};
	API.write(WRITE_COMMANDS.REQUEST_MONEY, parameters, onyxData);
	resetMoneyRequestInfo();
	Navigation.dismissModal(activeReportID);
	if (activeReportID) {
	Report.notifyNewAction(activeReportID, payeeAccountID);
	}

but here we forgot to consider the case when IOU is created Offline at this case when we call buildOptimisticTransaction we do not pass and store both values optimistically.

What changes do you think we should make in order to solve the problem?

We need to send additional props for tax rate and tax amount, this will be passed down to buildOptimisticTransaction via getMoneyRequestInformation, and then we will return the value of tax amount and rate stored optimistically so that we can see the rate and tax amount even in offline mode

App/src/libs/actions/IOU.ts

Line 2297 in 8ae5491

} = getMoneyRequestInformation(

Result Video

Screen.Recording.2024-04-11.at.7.03.14.AM.mov

Note: Same approach would be followed for tax rate as well, in the video I have only implemented taxamount

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• Tax - Tax does not show up in the IOU report when created offline

What is the root cause of that problem?

• In requestMoney the onyxData does not contain the taxCode and taxAmount, that leads to there is no tax data shown in offline mode.

What changes do you think we should make in order to solve the problem?

• We need to call getMoneyRequestInformation with additional params, taxAmount and taxCode.

• Then, in getMoneyRequestInformation, call buildOptimisticTransaction with these params, taxAmount and taxCode.

• Then in buildOptimisticTransaction, add these two params to here

• One note is that we need to make sure that the optimistic taxAmount is the same as the taxAmount returned by BE. To do that, we need to format the taxAmount before passing it to buildOptimisticTransaction like below:

    let optimisticTransaction = TransactionUtils.buildOptimisticTransaction(
        ReportUtils.isExpenseReport(iouReport) ? -amount : amount,
        ...
        taxCode,
        ReportUtils.isExpenseReport(iouReport) ? -taxAmount : taxAmount,
    );

• An additional bug is that, we do not have correct optimistic tax data when editing taxCode. When editing taxCode, we do not update the taxAmount accordingly. Solution: we can add check if the transactionChanges in here contains taxCode field. If it is, update the taxAmount as well. To do it, we can add the below logic to this:

if ('taxCode' in transactionChanges && !('taxAmount' in transactionChanges)) {
        const taxRates = policy?.taxRates;
        const getTaxAmount = (taxRates, selectedTaxRate: string, amount: number) => {
            const percentage = Object.values(OptionsListUtils.transformedTaxRates(taxRates)).find((taxRate) => taxRate.code?.includes(selectedTaxRate))?.value;
            if (percentage) {
                return TransactionUtils.calculateTaxAmount(percentage, amount);
            }
        };
        const taxAmount = getTaxAmount(taxRates, transactionChanges.taxCode, transaction?.amount);
        const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(taxAmount);
        optimisticData.push({
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
            value: {
                taxAmount: amountInSmallestCurrencyUnits,
            },
        });
    }

• Then we need to add the failureData and successData properly.

What alternative solutions did you explore? (Optional)

• NA

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01d25bb36f4e169f12

[melvin-bot]

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

[abekkala]

@jayeshmangwani it looks like we've already received a couple proposals for review! 🎉

[allgandalf]

Note for @jayeshmangwani : I have added the result video during the time of proposal itself rather than providing you with long lengthy code lines as in our proposal guidelines it states that:

Please use plain English, be brief and avoid jargon. Feel free to use images.
Do not post large multi-line diffs or write walls of text.

Code is something which is considered as implementation details and not the core idea, I have clearly stated where the changes would be implemented and what changes are required, I hope that helps you in making the decision, thanks 🥂

[jayeshmangwani]

Will review proposals today

[jayeshmangwani]

Thanks @GandalfGwaihir and @nkdengineer for the proposals

Both proposals are correct, and RCA is on point. @nkdengineer has added some extra details than @GandalfGwaihir for editing taxCode, but that is additional to the original issue. @GandalfGwaihir proposed a working solution first for the described issue, so in this case, we can go with user @GandalfGwaihir 's Proposal

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @arosiclair, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[trjExpensify]

👋 Just a heads up to make sure we distinguish between iouReport and expenseReport in the title and OP of the issue. A request to a workspace goes onto an expense report. Tax tracking is a feature of workspaces. You can't track tax on requests to individuals, that go on iou Reports.

[trjExpensify]

CC: @MonilBhavsar as it's Track Tax related.

[nkdengineer]

@arosiclair More about this C+ comment: We have addtional bugs related to the tax flow in offline mode (mentioned in my proposal), should we plan to fix it in this issue? If not, I think we can go with the selected proposal

[MonilBhavsar]

This is dupe of implementation issue - #39616

[arosiclair]

This is dupe of implementation issue - #39616

Should we just close this then?

[MonilBhavsar]

Let's close it 👍