Web - Expense - Fields are blank and header shows $0.00 request when changing date

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.13.0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go to created manual expense details page
3. Click Date
4. Change the date

Expected Result:

Date is saved without having other areas of the app going blank

Actual Result:

When date is saved, the other fields go blank, the header turns $0.00 request and the date reverts to today's date briefly

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

Bug6313370_1702574868668.bandicam_2023-12-15_00-28-41-162.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @francoisl (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[abzokhattab]

Proposal

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

Fields are blank and when changing money request date

What is the root cause of that problem?

when chaning the money request date we set onlyIncludeChangedFields param as true here That is why it only returns the changed
param:

    const {params, onyxData} = getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, false);
    API.write('UpdateDistanceRequest', params, onyxData);
}

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

we should set it to false in order not to affect other fields

[paultsimura]

Proposal

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

When updating Money Request date, the transaction goes blank. It's easily noted if we update the date offline.

What is the root cause of that problem?

We are building the Onyx data that contains only updated fields, and use SET operation that fully replaces the transaction:

App/src/libs/actions/IOU.js

Lines 1016 to 1022 in 052c5cc

	function updateMoneyRequestDate(transactionID, transactionThreadReportID, val) {
	const transactionChanges = {
	created: val,
	};
	const {params, onyxData} = getUpdateMoneyRequestParams(transactionID, transactionThreadReportID, transactionChanges, true);
	API.write('UpdateMoneyRequestDate', params, onyxData);
	}

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

We should call getUpdateMoneyRequestParams with onlyIncludeChangedFields=false.

What alternative solutions did you explore? (Optional)

I see there is a bigger issue with the way we use the onlyIncludeChangedFields param in IOU.getUpdateMoneyRequestParams:

First, the Onyx SET operation:
The intention for using the onlyIncludeChangedFields param was to either build a partial transaction change (which should be used with the MERGE operation), or a full updated transaction (this will be used in the SET operation).

However, we only return the SET operation no matter what parameter was passed:

App/src/libs/actions/IOU.js

Lines 929 to 939 in 052c5cc

	optimisticData.push({
	// We need to use SET method to save updated waypoint instead MERGE method to avoid wrong update of waypoints. More detail: https://github.com/Expensify/App/issues/30290#issuecomment-1778957070
	onyxMethod: Onyx.METHOD.SET,
	key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
	value: {
	...optimisticTransaction,
	pendingFields,
	isLoading: _.has(transactionChanges, 'waypoints'),
	errorFields: null,
	},
	});

This will result in fully replacing the transaction with only the updated fields when the onlyIncludeChangedFields is set to true.

I would say we need to use MERGE if only updated fields were requested:

onyxMethod: onlyIncludeChangedFields ? Onyx.METHOD.MERGE : Onyx.METHOD.SET,

Second, the way we build the optimisticTransaction for the onlyIncludeChangedFields case:

App/src/libs/actions/IOU.js

Line 928 in 052c5cc

const optimisticTransaction = onlyIncludeChangedFields ? _.pick(updatedTransaction, _.keys(transactionChanges)) : updatedTransaction;

We plainly use the _.pick(updatedTransaction, _.keys(transactionChanges)), and here is the discrepancy:

We pass the transaction changes as { created: "..." } (as it should be), but the updatedTransaction contains 2 fields:

created: "original date",
modifiedCreated: "modified date"

We never get the updated value here, because using pick will lead to always getting the original date, which is under created.

So we should modify this approach as well, to match the way we build the updatedTransaction.

Having fixed that, we should keep calling IOU.getUpdateMoneyRequestParams with onlyIncludeChangedFields=true to use the MERGE operation, which is more safe and acceptable than SET.

---

If we're OK with using SET operation and just limit the data we send to the BE, the only change we should make is remove this line, and use updatedTransaction regardless of onlyIncludeChangedFields param:

App/src/libs/actions/IOU.js

Line 928 in 052c5cc

const optimisticTransaction = onlyIncludeChangedFields ? _.pick(updatedTransaction, _.keys(transactionChanges)) : updatedTransaction;