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.

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

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

Closed
1 of 6 tasks
lanitochka17 opened this issue Aug 12, 2023 · 5 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 12, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Issue found when executing PR #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?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

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

Bug6161824_splitbill.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 12, 2023

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

@Talha345
Copy link
Contributor

Talha345 commented Aug 13, 2023

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:

before.mov

After:

after.mov

@melvin-bot melvin-bot bot added the Overdue label Aug 14, 2023
@JmillsExpensify
Copy link

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 16, 2023
@JmillsExpensify
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
None yet
Development

No branches or pull requests

3 participants