Skip to content

[HOLD for payment 2023-08-28] [$1000] Offline mode: Crash after deleting Split Bill request #25481

Closed

Description

@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!


Action Performed:

  1. Log in to any account.
  2. Turn OFF the network connection to switch to OFFLINE mode.
  3. Create a Split Bill request with two other users.
  4. This will generate two sub-reports: report A and report B >> Click on report A to view its details.
  5. Hover over the report and select the Delete option >> report A will be deleted.
  6. Click on report B >> Observation: The app CRASHES.

Expected Result:

App should not crash

Actual Result:

App Crashes

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: v1.3.55-1

Reproducible in staging?: Y

Reproducible in production?: N

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

split-request-crash.mp4
Screen.Recording.20230817.183513.New.Expensify.mp4

Expensify/Expensify Issue URL:

Issue reported by: @tranvantoan-qn

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692279822439699

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e588feb8af6feae9
  • Upwork Job ID: 1692566105949474816
  • Last Price Increase: 2023-08-18
  • Automatic offers:
    • pradeepmdk | Contributor | 26243141
    • tranvantoan-qn | Reporter | 26243145

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Aug 18, 2023
  2. melvin-bot commented on Aug 18, 2023

    @melvin-bot

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

  3. melvin-bot commented on Aug 18, 2023

    @melvin-bot

    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
  4. OSBotify commented on Aug 18, 2023

    @OSBotify
    Contributor

    👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

    1. Identify the pull request that introduced this issue and revert it.
    2. Find someone who can quickly fix the issue.
    3. Fix the issue yourself.
  5. melvin-bot commented on Aug 18, 2023

    @melvin-bot

    Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

  6. tranvantoan-qn commented on Aug 18, 2023

    @tranvantoan-qn

    FYI: This crash happens on ALL platforms, not only Android as specified in the issue Title

  7. pradeepmdk commented on Aug 18, 2023

    @pradeepmdk
    Contributor

    Proposal

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

    Offline mode: Crash after deleting Split Bill request

    What is the root cause of that problem?

    Screenshot 2023-08-18 at 9 28 05 PM

    in offline after deleting transaction its updated to null because its completed removed from onyx
    Screenshot 2023-08-18 at 8 13 52 PM
    Screenshot 2023-08-18 at 8 18 19 PM

    return _.filter(allTransactions, (transaction) => transaction.reportID === reportID);

    App/src/libs/actions/IOU.js

    Lines 1109 to 1113 in a068ca5

    const optimisticData = [
    {
    onyxMethod: Onyx.METHOD.SET,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
    value: null,

    here when offline mode optimisticData will execute so it's set as a null. when onyx null it will remove from local database

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

    1. we need to check its null or not
    function getAllReportTransactions(reportID) {
        return _.filter(allTransactions, (transaction) => transaction && transaction.reportID === reportID);
    }

    And here we need update

    props.network.isOffline
    ) {
    shouldShowPendingConversionMessage = IOUUtils.isIOUReportPendingCurrencyConversion(props.reportActions, props.iouReport);
    }

    to

    IOUUtils.isIOUReportPendingCurrencyConversion(props.iouReport);
    

    because this isIOUReportPendingCurrencyConversion is expecting the iouReport param only

    App/src/libs/IOUUtils.js

    Lines 77 to 79 in a068ca5

    function isIOUReportPendingCurrencyConversion(iouReport) {
    const reportTransactions = TransactionUtils.getAllReportTransactions(iouReport.reportID);
    const pendingRequestsInDifferentCurrency = _.filter(reportTransactions, (transaction) => transaction.pendingAction && TransactionUtils.getCurrency(transaction) !== iouReport.currency);

    This Pr is modified the function
    https://github.com/Expensify/App/pull/24345/files

    result

    Screen.Recording.2023-08-18.at.8.41.45.PM.mov
  8. 68 remaining items

  9. roryabraham commented on Aug 22, 2023

    @roryabraham
    Contributor

    Agree @marcaaron that's the real root cause and we should fix it in Onyx. Didn't do that today because I wanted to get the deploy out as quickly as possible, but I made a follow-up issue here: Expensify/react-native-onyx#310

  10. Pujan92 commented on Aug 22, 2023

    @Pujan92
    Contributor
    • The subscriber to get the new complete collection with any null values removed
            if (_.isFunction(subscriber.callback)) {
                if (isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) {
                    const cachedCollection = getCachedCollection(subscriber.key);
                    cachedCollection[key] = data;
                    subscriber.callback(cachedCollection);
                    continue;
                }
    
                subscriber.callback(data, key);
                continue;
            }
    

    @marcaaron @roryabraham For non-collection also we are updating with a null value, so do we need to avoid that also?

  11. marcaaron commented on Aug 22, 2023

    @marcaaron
    Contributor

    No, we cannot do this. At least for now, it's the only way a subscriber can know that some data has been removed.

  12. MitchExpensify commented on Aug 29, 2023

    @MitchExpensify
    Contributor

    Paid and contracts ended #25481 (comment)

  13. MitchExpensify commented on Aug 29, 2023

    @MitchExpensify
    Contributor

    Bump BZ steps #25481 (comment)

  14. roryabraham commented on Sep 1, 2023

    @roryabraham
    Contributor

    I checked off my pieces in the BZ checklist. The main takeaway here is that the root cause was actually a bug in Onyx, and I'm working on fixing that in Expensify/react-native-onyx#311

  15. MitchExpensify commented on Sep 1, 2023

    @MitchExpensify
    Contributor

    Thanks @roryabraham! Looks like this can be closed in that case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2EngineeringExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions