[HOLD for payment 2024-11-11] [$250] Distance - Receipt disappears when dismissing distance editor after receipt is generated

[izarutskaya]

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: 9.0.29-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: applausetester+kh010901@applause.expensifail.com Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Go offline.
4. Submit a distance expense.
5. Go to transaction thread.
6. Click Distance.
7. Go online.
8. After the receipt in the background is generated, dismiss the distance editor by clicking RHP back button.

Expected Result:

The receipt in the transaction thread will remain generated.

Actual Result:

The receipt in the transaction thread turns to placeholder thumbnail. Sometimes, the map loads, but the map is not clickable (it is still in the loading state and not generating the actual receipt).

This issue also happens when opening distance editor right after the distance expense is submitted before the receipt is generated.

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/a07d59b4-5b40-41e4-809a-fd5156c25484

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021832157502208534261
• Upwork Job ID: 1832157502208534261
• Last Price Increase: 2024-10-18
• Automatic offers:
• ikevin127 | Reviewer | 104586775
• wildan-m | Contributor | 104586777

Issue Owner

Current Issue Owner: @isabelastisser

[melvin-bot[bot]]

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

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-09-09 05:24:30 UTC.

Proposal

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

The receipt in the transaction thread turns to placeholder thumbnail. Sometimes, the map loads, but the map is not clickable (it is still in the loading state and not generating the actual receipt).

What is the root cause of that problem?

When we open the distance page, the transaction backup data has no route data. After BE returns the data, MoneyRequestView is loaded transaction data from BE. If we close the distance page without saving any thing, the transaction data is reverted to the transaction backup data which is outdated data.

https://github.com/Expensify/App/blob/d3c995a1314f6b16c2b6adbb463ed85e705ac044/src/pages/iou/request/step/IOURequestStepDistance.tsx#L186-L192

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

We should check if at the time we open the distance page, we don't have the route data and at the time we close the page, we have the route data, and we will not revert the transaction to transaction backup data

const hasRouteRef = useRef(TransactionUtils.hasRoute(transaction));
hasRouteRef.current = TransactionUtils.hasRoute(transaction);
useEffect(() => {
    if (isCreatingNewRequest) {
        return () => {};
    }

    // On mount, create the backup transaction.
    TransactionEdit.createBackupTransaction(transaction);
    const hasRoute = TransactionUtils.hasRoute(transaction);
    const pendingRoute = transaction?.pendingAction ?? transaction?.pendingFields?.waypoints;

    return () => {
        // If the user cancels out of the modal without without saving changes, then the original transaction
        // needs to be restored from the backup so that all changes are removed.
        if (transactionWasSaved.current || (!hasRoute && !!pendingRoute && hasRouteRef.current)) {
            TransactionEdit.removeBackupTransaction(transaction?.transactionID ?? '-1');
            return;
        }
        TransactionEdit.restoreOriginalTransactionFromBackup(transaction?.transactionID ?? '-1', IOUUtils.shouldUseTransactionDraft(action));
    };
    // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [transaction?.pendingAction]);

https://github.com/Expensify/App/blob/d3c995a1314f6b16c2b6adbb463ed85e705ac044/src/pages/iou/request/step/IOURequestStepDistance.tsx#L186-L192

What alternative solutions did you explore? (Optional)

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ikevin127]

♻️ Will start reviewing the proposal in ~12h.

[ikevin127]

@nkdengineer Thanks for the proposal. I was able to reproduce the issue but I'm struggling with applying the solution, specifically the changes suggested at step 3 and without that the issue is still reproducible therefore I cannot verify the proposal.

Are you able to provide a testing branch with the working solution ? That would help verifying the solution and if the RCA matches the working solution then I would be able to finalize the proposal review. Thanks!

[nkdengineer]

@ikevin127 Ahh, sorry I proposed another bug. Updated my proposal for the original bug.

[ikevin127]

@nkdengineer Thanks for the update, I tested the solution and while it seems to fix the issue at first - the solution is not reliable because is breaking functionality that was working as expected before the change:

• check out the attached video where I show the discrepancy between local dev (proposed solution) vs current staging

Video

https://github.com/user-attachments/assets/51b3554d-ca20-4fad-952c-7310c683715f

Additionally, while going back online with the RHP open I had instances where the map receipt in the background report would show 404 not found:

I think a solution that would be acceptable for this issue should not break functionality that currently works well.

In addition, if proposing changes to currently existing code logic, some steps should be included for the reviewers to verify that current functionality is not affected by the proposed changes.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[isabelastisser]

Waiting for new proposals!

[ikevin127]

💰 Looking for proposals! Please keep in mind the previous proposal's pitfalls detailed in https://github.com/Expensify/App/issues/48630#issuecomment-2342426929 review.

[melvin-bot[bot]]

@isabelastisser, @ikevin127 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[isabelastisser]

Still waiting for proposals.

[ikevin127]

💰 Still looking for proposals! Please keep in mind the previous proposal's pitfalls detailed in https://github.com/Expensify/App/issues/48630#issuecomment-2342426929 review.

[melvin-bot[bot]]

@isabelastisser @ikevin127 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[wildan-m]

Proposal

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

Receipt disappears when dismissing distance editor after it is generated.

What is the root cause of that problem?

TransactionBackup routes are not being updated when online, causing the receipt route to disappear when the modal is dismissed.

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

This solution will require backend modifications.

Calculate transaction backup routes when there are pending waypoints, valid routes, and the system is online.

Create new API endpoint similar to GET_ROUTE_FOR_DRAFT but for backup.

    GET_ROUTE_FOR_BACKUP: 'GetRouteForBackup',

Add new type to define transactionStates

    TRANSACTION_STATE: {
        CURRENT: 'current',
        DRAFT: 'draft',
        BACKUP: 'backup',
    }

modify useFetchRoute to accept new parameter transactionState

export default function useFetchRoute(transaction: OnyxEntry<Transaction>, waypoints: WaypointCollection | undefined, action: IOUAction, transactionState: TransactionState = CONST.TRANSACTION_STATE.CURRENT) {
...............
    useEffect(() => {
        if (isOffline || !shouldFetchRoute || !transaction?.transactionID) {
            return;
        }

        TransactionAction.getRoute(transaction.transactionID, validatedWaypoints, transactionState);
    }, [shouldFetchRoute, transaction?.transactionID, validatedWaypoints, isOffline, action]);

Modify getOnyxDataForRouteRequest and getRoute to accept new transactionState param

function getOnyxDataForRouteRequest(transactionID: string, transactionState: TransactionState = CONST.TRANSACTION_STATE.CURRENT): OnyxData {
    let keyPrefix;
    switch (transactionState) {
        case CONST.TRANSACTION_STATE.DRAFT:
            keyPrefix = ONYXKEYS.COLLECTION.TRANSACTION_DRAFT;
            break;
        case CONST.TRANSACTION_STATE.BACKUP:
            keyPrefix = ONYXKEYS.COLLECTION.TRANSACTION_BACKUP;
            break;
        case CONST.TRANSACTION_STATE.CURRENT:
        default:
            keyPrefix = ONYXKEYS.COLLECTION.TRANSACTION;
            break;
    }

....

function getRoute(transactionID: string, waypoints: WaypointCollection, routeType: TransactionState = CONST.TRANSACTION_STATE.CURRENT) {
    if (routeType === CONST.TRANSACTION_STATE.BACKUP)
    {
        mockGetBackupRoute(transactionID);
    }

    const parameters: GetRouteParams = {
        transactionID,
        waypoints: JSON.stringify(waypoints),
    };

    let command;
    switch (routeType) {
        case 'draft':
            command = READ_COMMANDS.GET_ROUTE_FOR_DRAFT;
            break;
        case 'current':
            command = READ_COMMANDS.GET_ROUTE;
            break;
        case 'backup':
            command = READ_COMMANDS.GET_ROUTE_FOR_BACKUP;
            break;
        default:
            throw new Error('Invalid route type');
    }
.......

Adjust useFetchRoute usage: src/pages/iou/request/step/IOURequestStepConfirmation.tsx

    useFetchRoute(transaction, transaction?.comment?.waypoints, action, IOUUtils.shouldUseTransactionDraft(action) ? CONST.TRANSACTION_STATE.DRAFT : CONST.TRANSACTION_STATE.CURRENT);

call useFetchRoute for backup in addition to current fetch, we can't use current transaction waypoints since that waypoints can be modified while offline. src/pages/iou/request/step/IOURequestStepDistance.tsx

    const backupWaypoints = !!transactionBackup?.pendingFields?.waypoints ? transactionBackup?.comment?.waypoints : undefined;

    const { shouldFetchRoute, validatedWaypoints } = useFetchRoute(transaction, waypoints, action, IOUUtils.shouldUseTransactionDraft(action) ? CONST.TRANSACTION_STATE.DRAFT : CONST.TRANSACTION_STATE.CURRENT);
    useFetchRoute(transactionBackup, backupWaypoints, action, CONST.TRANSACTION_STATE.BACKUP);

Branch for this solution

What alternative solutions did you explore? (Optional)

N/A

[ikevin127]

♻️ Reviewing this and asking the team what's the procedure when proposals request BE changes - will get back with feedback next week.

[melvin-bot[bot]]

@isabelastisser, @ikevin127 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[isabelastisser]

Hey @ikevin127, can you please provide an update? Thanks!

[ikevin127]

Sure, in the meantime I asked on #contributor-plus Slack channel thread about how we should proceed in cases like the latest proposal given that it suggests BE changes.

♻️ Will get back with an answer by EOD (PST).

[ikevin127]

Without actually testing it, since it's missing the BE part: based on reviewing the technical aspects of the proposal and based on the response received in the Slack thread mentioned above, @wildan-m's proposal looks good to me as the RCA makes sense and the proposed solution seems valid based on the fact that we do something similar for GET_ROUTE_FOR_DRAFT.

Therefore I'll defer this to the assigned CME for the BE part and further validation of the proposal.

cc @Gonals

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[Gonals]

It'll be a few days until I get to the BE of this

[isabelastisser]

Thanks for the update, @Gonals!

[melvin-bot[bot]]

@Gonals, @isabelastisser, @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ikevin127]

Not overdue, we're working on FE / BE pull requests.

[melvin-bot[bot]]

@Gonals @isabelastisser @ikevin127 this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[ikevin127]

Not overdue, we're still working on FE / BE pull requests.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@Gonals, @isabelastisser, @ikevin127 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[ikevin127]

I think we're still working on FE / BE pull requests. @Gonals Any updates from the BE side ?

[Gonals]

Any updates from the BE side ?

Travel stuff is taking priority, so it might still be a few days. Sorry!

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[ikevin127]

Not overdue, we're working on the PRs 👍

[melvin-bot[bot]]

@Gonals, @isabelastisser, @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ikevin127]

Not overdue, still working on the PRs 👍

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[isabelastisser]

@ikevin127, can you please share an update? thanks!

[ikevin127]

Travel stuff is taking priority, so it might still be a few days. Sorry!

@Gonals Any updates on the BE side of this issue ? If not maybe we can put the issue on HOLD for Gonals so we can avoid Overdue.

[Gonals]

Getting to it today!

[Gonals]

@wildan-m, how will GET_ROUTE_FOR_BACKUP differ from GET_ROUTE_FOR_DRAFT? Do you expect to pass the same parameters and receive the same response? If so, why not calling GET_ROUTE_FOR_DRAFT in this flow?

[wildan-m]

@Gonals the only difference is target onyx key from the server. it should target backup onyx key

[melvin-bot[bot]]

@Gonals, @isabelastisser, @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ikevin127]

Not overdue, we're working on the BE side after which the FE pull request will follow.

[wildan-m]

If so, why not calling GET_ROUTE_FOR_DRAFT in this flow?

@Gonals calling that API will update the key for draft not for back up

[Gonals]

Gotcha. Backend PR incoming

[Gonals]

Backend PR in review

[ikevin127]

Not overdue, BE pull request is in review after which the FE pull request will follow.