Skip to content

[$500] Login - App crashes when going through task creation flow while the app is still loading #30539

Closed

Description

@lanitochka17

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: 1.3.92-2
**Reproducible in staging?:**Y
**Reproducible in production?:**Y
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to staging.new.expensify.com
  2. Log in with a high traffic account
  3. While the app is still loading, click + on the composer > Assign task
  4. Enter a title and hit ENTER to proceed to the next page

Expected Result:

App does not crash

Actual Result:

App crashes

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • Windows: Chrome
  • MacOS: Desktop

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
Windows: Chrome
Bug6253615_1698428410905.20231027_200501.mp4
MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01949234f35124ab07
  • Upwork Job ID: 1718005348020469760
  • Last Price Increase: 2023-10-27
  • Automatic offers:
    • jjcoffee | Reviewer | 27445256
    • DylanDylann | Contributor | 27445258

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Oct 27, 2023
  2. changed the title [-]Login - App crashes when going through task creation flow while the app is still loading[/-] [+][$500] Login - App crashes when going through task creation flow while the app is still loading[/+] on Oct 27, 2023
  3. melvin-bot commented on Oct 27, 2023

    @melvin-bot

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

  4. melvin-bot commented on Oct 27, 2023

    @melvin-bot
  5. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Oct 27, 2023
  6. melvin-bot commented on Oct 27, 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
  7. melvin-bot commented on Oct 27, 2023

    @melvin-bot

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

  8. mkhutornyi commented on Oct 27, 2023

    @mkhutornyi
    Contributor

    Proposal

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

    App crashes on assign task confirmation page when opened quickly after login

    What is the root cause of that problem?

    function getShareDestination(reportID, reports, personalDetails) {
    const report = lodashGet(reports, `report_${reportID}`, {});
    const participantAccountIDs = lodashGet(report, 'participantAccountIDs');
    const isMultipleParticipant = participantAccountIDs.length > 1;

    When open NewTaskPage before loading reports data, participantAccountIDs is undefined so in line 676, it calls undefined.length which leads to crash.

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

    Add fallback to lodashGet - const participantAccountIDs = lodashGet(report, 'participantAccountIDs', []);

  9. DylanDylann commented on Oct 28, 2023

    @DylanDylann
    Contributor

    Proposal

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

    • App crashes on assign task confirmation page when opened quickly after login

    What is the root cause of that problem?

    • When the report is undefined, so accessing length of participantAccountID leads to the error.
      const isMultipleParticipant = participantAccountIDs.length > 1;
    • The RCA is that when the reportID that we get from route is '0', then clicking on "Assign task" it will call clearOutTaskInfoAndNavigate(reportID) with reportID is '0':
      function clearOutTaskInfoAndNavigate(reportID) {
      clearOutTaskInfo();
      setParentReportID(reportID);
      Navigation.navigate(ROUTES.NEW_TASK_DETAILS);
      }
    • Then it will navigate to the new assign task flow. In here, Task.getShareDestination is called with reportID is '0':
      const displayDetails = Task.getShareDestination(props.task.shareDestination, props.reports, props.personalDetails);
    • So the report returned from
      const report = lodashGet(reports, `report_${reportID}`, {});
      is undefined, then accessing participantAccountIDs from undefined will leads to the bug.

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

    Solution 1: If the reportID is still '0', we can consider the "Assign task" action in the report is the same the "Assign task" in the FAB by updating:

    function clearOutTaskInfoAndNavigate(reportID) {
    clearOutTaskInfo();
    setParentReportID(reportID);
    Navigation.navigate(ROUTES.NEW_TASK_DETAILS);
    }

    to

    function clearOutTaskInfoAndNavigate(reportID) {
        ...
        if(reportID !== '0'){
            setParentReportID(reportID);
        }
        ...
    }
    

    Solution 2:

    • Also we should disable the action create task once the report is loading as we did with the message composer.
    • We already have a logic to display ReportFooter in case isReportReadyForDisplay is false:
      <ReportFooter isReportReadyForDisplay={false} />
    • So just need to use isReportReadyForDisplay to check should we prevent user from clicking on "Assign task" or not

    What alternative solutions did you explore? (Optional)

    • Also we can add the method that will auto-set the share destination once the reportID is loaded correctly. We can do that by using withCurrentReportID:
    • Firstly, update
      if (props.task.parentReportID) {
      Task.setShareDestinationValue(props.task.parentReportID);
      }

      to
     if (props.task.parentReportID && props.task.parentReportID !== '0') {
                Task.setShareDestinationValue(props.task.parentReportID);
            }
      if (props.task.parentReportID === '0' && props.currentReportID !== '0') {
                Task.setShareDestinationValue(currentReportID);
            }
    
  10. suneox commented on Oct 28, 2023

    @suneox
    Contributor

    Proposal

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

    Login - App crashes when going through task creation flow while the app is still loading

    What is the root cause of that problem?

    When login with account high traffic account or slow network report data still loading but create task use a report data so when click next button will crash at this line due to participantAccountIDs is empty
    It can reproduce if we change network status on login

    30539.mp4

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

    We should subscribe ONYXKEYS.IS_LOADING_APP for NewTaskPage at withOnyx

        isLoadingApp: {
            key: ONYXKEYS.IS_LOADING_APP,
        },

    then and render a loading state at this line

        {props.isLoadingApp ? (
            <FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />
            ) : (
            <ScrollView contentContainerStyle={styles.flexGrow1}>
            ....
            </ScrollView>
        )}

    And set a default value for this line

        const participantAccountIDs = lodashGet(report, 'participantAccountIDs', []);

    after fix

    30539_after_fix_2.mp4

    What alternative solutions did you explore? (Optional)

    We can hide the ReportFooter if isLoaddingApp due to we don't have the context to create a task and avoid other issues
    Subscribe app loading at

        withOnyx({
            shouldShowComposeInput: {
                key: ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT,
                initialValue: false,
            },
    +       isLoadingApp: {
    +           key: ONYXKEYS.IS_LOADING_APP,
    +       },
        }),

    and check before render

        if(props.isLoadingApp) {
            return null;
        }
        return (
            ...
        )
  11. maxconnectAbhi commented on Oct 28, 2023

    @maxconnectAbhi

    Proposal

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

    Login - App crashes when going through task creation flow while the app is still loading

    What is the root cause of that problem?

    When logging in with a high-traffic account or on a slow network, the report data is still loading. However, when opening the NewTaskPage before the report data has loaded, the participantAccountIDs variable is undefined, resulting in a crash.

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

    I should use a Shimmer as an input container placeholder to prevent users from assigning a task before the report data loads.

    What alternative solutions did you explore? (Optional)

    N/A

  12. 17 remaining items

  13. removed
    Help WantedApply this label when an issue is open to proposals by contributors
    on Oct 30, 2023
  14. melvin-bot commented on Oct 30, 2023

    @melvin-bot

    📣 @jjcoffee 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

    Offer link
    Upwork job

  15. melvin-bot commented on Oct 30, 2023

    @melvin-bot

    📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

    Offer link
    Upwork job
    Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
    Keep in mind: Code of Conduct | Contributing 📖

  16. added
    ReviewingHas a PR in review
    and removed on Nov 1, 2023
  17. DylanDylann commented on Nov 1, 2023

    @DylanDylann
    Contributor

    @jjcoffee PR #30670 is ready for review

  18. abekkala commented on Nov 13, 2023

    @abekkala
    Contributor

    Deployed to Prod - 3 days ago 🎉

  19. jjcoffee commented on Nov 20, 2023

    @jjcoffee
    Contributor

    @abekkala I think this one's due for payment?

  20. jjcoffee commented on Nov 21, 2023

    @jjcoffee
    Contributor
    • The PR that introduced the bug has been identified. Link to the PR: N/A - behaviour was always there as far as I can tell
    • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
    • A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
    • Determine if we should create a regression test for this bug. Yes
    • If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

    Regression Test Proposal

    1. Open the app, ideally on a slow network
    2. Before the report chat finishes loading, click + on the composer > Assign task
    3. Enter a title and proceed to the next page
    4. Verify that the app does not crash

    Do we agree 👍 or 👎

  21. jjcoffee commented on Nov 21, 2023

    @jjcoffee
    Contributor

    @abekkala Completed the checklist manually since automation went a bit wonky here! This has been in production since Nov 10, so is overdue payment.

  22. abekkala commented on Nov 22, 2023

    @abekkala
    Contributor

    Reviewer: Applause
    Fix: @DylanDylann [$500]
    PR Review: @jjcoffee [$500]

  23. abekkala commented on Nov 22, 2023

    @abekkala
    Contributor

    @DylanDylann and @jjcoffee payments made and contracts ended - thank you! 🎉

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

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorReviewingHas a PR in reviewWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions