Skip to content
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.

Sign up for GitHub

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

Fix blank map preview on employee side #51840

Merged
merged 8 commits into from
Nov 7, 2024
Merged

Fix blank map preview on employee side #51840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f9dd9b9
update getUpdatedTransaction
nkdengineer Oct 30, 2024
da13f9e
Merge branch 'main' into fix/51285
nkdengineer Nov 1, 2024
bfd6b16
fix lint
nkdengineer Nov 1, 2024
aa0076a
remove console.log
nkdengineer Nov 1, 2024
b4b10a1
remove unused type
nkdengineer Nov 1, 2024
a5227dd
Merge branch 'main' into fix/51285
nkdengineer Nov 4, 2024
018b156
Merge branch 'main' into fix/51285
nkdengineer Nov 5, 2024
7e9ec7b
remove useless code
nkdengineer Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals

const distanceRequestFields = canUseP2PDistanceRequests ? (
<>
<OfflineWithFeedback pendingAction={getPendingFieldAction('waypoints')}>
<OfflineWithFeedback pendingAction={getPendingFieldAction('waypoints') ?? getPendingFieldAction('merchant')}>
<MenuItemWithTopDescription
description={translate('common.distance')}
title={distanceToDisplay}
Expand Down
6 changes: 4 additions & 2 deletions src/libs/TransactionUtils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashDeepClone from 'lodash/cloneDeep';
import lodashHas from 'lodash/has';
import lodashIsEqual from 'lodash/isEqual';
import lodashSet from 'lodash/set';
Expand Down Expand Up @@ -247,7 +248,7 @@ function areRequiredFieldsEmpty(transaction: OnyxEntry<Transaction>): boolean {
*/
function getUpdatedTransaction(transaction: Transaction, transactionChanges: TransactionChanges, isFromExpenseReport: boolean, shouldUpdateReceiptState = true): Transaction {
// Only changing the first level fields so no need for deep clone now
const updatedTransaction = {...transaction};
const updatedTransaction = lodashDeepClone(transaction);
let shouldStopSmartscan = false;

// The comment property does not have its modifiedComment counterpart
Expand Down Expand Up @@ -299,7 +300,7 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra
const conversionFactor = existingDistanceUnit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? CONST.CUSTOM_UNITS.MILES_TO_KILOMETERS : CONST.CUSTOM_UNITS.KILOMETERS_TO_MILES;
const distance = NumberUtils.roundToTwoDecimalPlaces((transaction?.comment?.customUnit?.quantity ?? 0) * conversionFactor);
lodashSet(updatedTransaction, 'comment.customUnit.quantity', distance);
lodashSet(updatedTransaction, 'pendingFields.waypoints', CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE);
lodashSet(updatedTransaction, 'pendingFields.merchant', CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE);
}
}

Expand Down Expand Up @@ -339,6 +340,7 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra
}

updatedTransaction.pendingFields = {
...(updatedTransaction?.pendingFields ?? {}),
...(Object.hasOwn(transactionChanges, 'comment') && {comment: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}),
...(Object.hasOwn(transactionChanges, 'created') && {created: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}),
...(Object.hasOwn(transactionChanges, 'amount') && {amount: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}),
Expand Down
10 changes: 8 additions & 2 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2506,7 +2506,7 @@ function getUpdateMoneyRequestParams(
const failureData: OnyxUpdate[] = [];

// Step 1: Set any "pending fields" (ones updated while the user was offline) to have error messages in the failureData
const pendingFields = Object.fromEntries(Object.keys(transactionChanges).map((key) => [key, CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE]));
let pendingFields: OnyxTypes.Transaction['pendingFields'] = Object.fromEntries(Object.keys(transactionChanges).map((key) => [key, CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE]));
const errorFields = Object.fromEntries(Object.keys(pendingFields).map((key) => [key, {[DateUtils.getMicroseconds()]: Localize.translateLocal('iou.error.genericEditFailureMessage')}]));

const allReports = ReportConnection.getAllReports();
Expand All @@ -2519,6 +2519,13 @@ function getUpdateMoneyRequestParams(
let updatedTransaction: OnyxEntry<OnyxTypes.Transaction> = transaction ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) : undefined;
const transactionDetails = ReportUtils.getTransactionDetails(updatedTransaction);

if (updatedTransaction?.pendingFields) {
pendingFields = {
...pendingFields,
...updatedTransaction?.pendingFields,
};
}

Comment on lines +2522 to +2528
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nkdengineer @ishpaul777, could you please explain this change?

The only place where the pendingFields is used later is here:

App/src/libs/actions/IOU.ts

Lines 2669 to 2678 in 17cae11

optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
...updatedTransaction,
pendingFields,
isLoading: hasPendingWaypoints,
errorFields: null,
},
});

Doesn't it do basically the same thing you've introduced here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paultsimura The problem is getUpdatedTransaction function can return another pendingFields

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and they will be passed to optimisticData because we use ...updatedTransaction (which includes the new pendingFields) as the first line in the code I've referenced.
Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and they will be passed to optimisticData because we use ...updatedTransaction (which includes the new pendingFields) as the first line in the code I've referenced.

Yes, that's correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see it now.

if (transactionDetails?.waypoints) {
// This needs to be a JSON string since we're sending this to the MapBox API
transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints);
Expand Down Expand Up @@ -4982,7 +4989,6 @@ function completeSplitBill(chatReportID: string, reportAction: OnyxTypes.ReportA
const splitParticipants: Split[] = updatedTransaction?.comment?.splits ?? [];
const amount = updatedTransaction?.modifiedAmount;
const currency = updatedTransaction?.modifiedCurrency;
console.debug(updatedTransaction);

// Exclude the current user when calculating the split amount, `calculateAmount` takes it into account
const splitAmount = IOUUtils.calculateAmount(splitParticipants.length - 1, amount ?? 0, currency ?? '', false);
Expand Down
Loading