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: do not refresh draft txn after redirect #44363

Merged
merged 15 commits into from
Jul 11, 2024
Merged

fix: do not refresh draft txn after redirect #44363

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d10afcb
fix: do not refresh draft txn after redirect
dominictb Jun 25, 2024
40bbcd7
Merge remote-tracking branch 'origin/main' into fix/40936-init-money-req
dominictb Jun 26, 2024
5cd9f11
fix: update explanation comment
dominictb Jun 26, 2024
da44443
fix: navigate to scan page instead of start page
dominictb Jul 1, 2024
c857885
fix: move the user back to start page if no receipt upload
dominictb Jul 5, 2024
26dd780
Revert "fix: move the user back to start page if no receipt upload"
dominictb Jul 10, 2024
fc9721d
Revert "fix: navigate to scan page instead of start page"
dominictb Jul 10, 2024
7cc38de
Revert "fix: update explanation comment"
dominictb Jul 10, 2024
e77ee51
Revert "fix: do not refresh draft txn after redirect"
dominictb Jul 10, 2024
78260a1
Merge remote-tracking branch 'origin/main' into fix/40936-init-money-req
dominictb Jul 10, 2024
e456298
fix: add a flag to keep the existing draft txn data
dominictb Jul 10, 2024
23493ff
fix: update lint and prettier
dominictb Jul 10, 2024
db23adf
Merge remote-tracking branch 'origin/main' into fix/40936-init-money-req
dominictb Jul 11, 2024
2f5cd9b
fix: update logic of checking shouldKeepData
dominictb Jul 11, 2024
07e9c6e
fix: remove redundant comment
dominictb Jul 11, 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
17 changes: 17 additions & 0 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,22 @@ function initMoneyRequest(reportID: string, policy: OnyxEntry<OnyxTypes.Policy>,
});
}

/**
* Reset the money request with a new report ID. Unlike the initMoneyRequest function, this function only
* set the new report ID (and isFromGlobalCreate flag) and reset the participants of the draft transaction
* @param transactionID
* @param reportID
* @param isFromGlobalCreate
*/
function resetMoneyRequestReportID(transactionID: string, reportID: string, isFromGlobalCreate: boolean) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {
reportID,
participants: [],
participantsAutoAssigned: false,
isFromGlobalCreate,
});
}

function createDraftTransaction(transaction: OnyxTypes.Transaction) {
if (!transaction) {
return;
Expand Down Expand Up @@ -6996,6 +7012,7 @@ export {
detachReceipt,
editMoneyRequest,
initMoneyRequest,
resetMoneyRequestReportID,
navigateToStartStepIfScanFileCannotBeRead,
payMoneyRequest,
payInvoice,
Expand Down
11 changes: 10 additions & 1 deletion src/pages/iou/request/IOURequestStartPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ function IOURequestStartPage({
if (transaction?.reportID === reportID) {
return;
}
// if transaction is from global create, the original behavior is that the user can change the recipient reportID
// without having to reset the whole transaction, hence we don't need to re-init the transaction in this case
if (transaction?.isFromGlobalCreate && transaction.transactionID) {
IOU.resetMoneyRequestReportID(transaction.transactionID, reportID, isFromGlobalCreate);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this anymore. Now the back button uses the transaction's report id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, what do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

When navigating back to the participants page we will use the transaction's report id in the uri param. Thus the condition above transaction?.reportID === reportID will be met and we won't init the money request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/Expensify/App/pull/44363/files#diff-5e5fc302dd5db871d26e30f810bc0606beccb7f50ae6ed6535cd93aaee98c280R193 in here, I only update this to fix the extra bug that I mentioned in the last PR

When you reload a scan IOU request in the confirmation step, it still use the route.params.reportID to navigate back to the start step if it couldn't load the image locally

Copy link
Contributor

Choose a reason for hiding this comment

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

In navigateToStartStepIfScanFileCannotBeRead can we navigate to the scan step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, tested it and confirmed that it still renders the IOURequestStartPage component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt feel free to re-test it and let me know. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

In navigateToStartStepIfScanFileCannotBeRead we navigate the user to ROUTES.MONEY_REQUEST_CREATE_TAB_SCAN

App/src/libs/actions/IOU.ts

Line 6968 in 937c418

IOUUtils.navigateToStartMoneyRequestStep(requestType, iouType, transactionID, reportID);
.

What I'm asking is to change the route here so the user is navigated to ROUTES.MONEY_REQUEST_STEP_SCAN and remove IOU.resetMoneyRequestReportID as it won't be needed anymore

Screen.Recording.2024-06-28.at.4.52.32.PM.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, let me test that and get back to you shortly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt updated in the latest commit!

IOU.initMoneyRequest(reportID, policy, isFromGlobalCreate, transactionRequestType.current);
}, [transaction, policy, reportID, iouType, isFromGlobalCreate]);

Expand All @@ -109,9 +115,12 @@ function IOURequestStartPage({

const resetIOUTypeIfChanged = useCallback(
(newIOUType: IOURequestType) => {
if (transaction?.iouRequestType === newIOUType) {
return;
}
IOU.initMoneyRequest(reportID, policy, isFromGlobalCreate, newIOUType);
},
[policy, reportID, isFromGlobalCreate],
[policy, reportID, isFromGlobalCreate, transaction?.iouRequestType],
);

if (!transaction?.transactionID) {
Expand Down
3 changes: 2 additions & 1 deletion src/pages/iou/request/step/IOURequestStepConfirmation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ function IOURequestStepConfirmation({
// If there is not a report attached to the IOU with a reportID, then the participants were manually selected and the user needs taken
// back to the participants step
if (!transaction?.participantsAutoAssigned) {
Navigation.goBack(ROUTES.MONEY_REQUEST_STEP_PARTICIPANTS.getRoute(iouType, transactionID, reportID, undefined, action));
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Navigation.goBack(ROUTES.MONEY_REQUEST_STEP_PARTICIPANTS.getRoute(iouType, transactionID, transaction?.reportID || reportID, undefined, action));
return;
}
IOUUtils.navigateToStartMoneyRequestStep(requestType, iouType, transactionID, reportID, action);
Expand Down
Loading