Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

iOS - Deeplink - Split bill with previous insert sum displayed if open split bill link #24474

Closed lanitochka17 closed 1 year ago

lanitochka17 commented 1 year ago

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


Issue found when executing PR https://github.com/Expensify/App/pull/24126

Action Performed:

  1. Open Expensify app
  2. Tap on "+" Fab menu
  3. Tap on "Split bill"
  4. Insert amount
  5. Tap "Next"
  6. Kill the app (completely close it so it doesn't run in background)
  7. Tap on deeplink https://staging.new.expensify.com/split/new (e.g. in Notes)

Expected Result:

User lands in split bill tab with amount set to 0

Actual Result:

User lands in split bill tab with amount inserted at step 4 displayed. To reproduce the issue on Desktop app log into browser and desktop app with same account as "precondition" and as step 8 tap on "open" in prompt in browser

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.53.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/78819774/160b2da8-6283-44be-aaec-752f18b59918

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

Talha345 commented 1 year ago

Proposal

Note: This issue is present not only for split bill but also for request money and send money(which is now hidden but functionality is still there).Also this is present on all platforms.

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

When opening split bill via deep link after entering a value already and pressing next, the previous value is shown.

What is the root cause of that problem?

The RC for this issue is that we are storing the IOU data in Onyx and when the user visits the page using deep link, the stored data is fetched and previous values are displayed. The function that is executed when the next button is clicked is as follows:

https://github.com/Expensify/App/blob/main/src/pages/iou/steps/NewRequestAmountPage.js#L145-L156

Here we can see, the methods IOU.setMoneyRequestAmount(amountInSmallestCurrencyUnits); and IOU.setMoneyRequestCurrency(currency); are updating the data in Onyx.

On the otherhand, if the user comes again via FAB Split Bill, the IOU data is reset via resetMoneyRequestInfo('${iouType}${reportID}');using the following function before the component is rendered.

https://github.com/Expensify/App/blob/main/src/libs/actions/IOU.js#L1468-L1471

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

We can reset the IOU data (similar to when the user comes again via FAB Split Bil) if the page is visited using a deeplink.In order to do that we need to make the following changes:

  1. We can use an useEffect hook to check if the page is accessed via a deeplink as follows:
    useEffect(() => {
        const handleDeepLink = () => {
            // Get the initial URL when the app was launched
            Linking.getInitialURL().then((initialUrl) => {
                if (!Url.addTrailingForwardSlash(initialUrl).includes(ROUTES.getMoneyRequestRoute(iouType))) {
                    return;
                }
                resetMoneyRequestInfo();
            });
        };
        handleDeepLink();
    }, []);

We can add this to MoneyRequestSelectorPage as the NewRequestAmountPage page is rendered from this component.Alternatively, we can also add the hook to NewRequestAmountPage component itself but I would prefer to keep it in the parent component to keep things neat.

  1. Secondly, we need to make changes to saveAmountToState because if we set the amount as 0, the guard clause returns and the value in the form is not updated to ''.We can update this function like:

    const saveAmountToState = (currencyCode, amountInCurrencyUnits) => {
        if (!currencyCode || typeof amountInCurrencyUnits !== 'number') {
            return;
        }
        const amountAsStringForState = amountInCurrencyUnits ? CurrencyUtils.convertToWholeUnit(currency, amount).toString() : '';
        setCurrentAmount(amountAsStringForState);
        setSelection({
            start: amountAsStringForState.length,
            end: amountAsStringForState.length,
        });
    };

What alternative solutions did you explore? (Optional)

N/A

Result:

Before:

https://github.com/Expensify/App/assets/36226639/5fa61e1b-45b7-4751-b1f7-e6d4802abcf6

After:

https://github.com/Expensify/App/assets/36226639/083c0071-49a9-49e6-b6d3-37cdd4878043

JmillsExpensify commented 1 year ago

I don't consider this is a bug. Going to bring up for discussion internally.

JmillsExpensify commented 1 year ago

Closing, as this isn't an issue. You can always change the amount if you want.