Skip to content

[$500] IOU - Request details page loads infinitely when request is created offline #37247

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.4.44-0
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go offline
  3. Go to workspace chat
  4. Create a manua; request
  5. Navigate to request details page

Expected Result:

The request details page will load when the request is created offline (production behavior)

Actual Result:

The request details page loads infinitely when the request is created offline (production behavior).
This issue also occurs when:

  • creating IOU with new user offline. The main chat loads infinitely.
  • Same issue when opening thread offline

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6393457_1708992632169.bandicam_2024-02-27_06-12-20-868.mp4

View all open jobs on GitHub

Activity

  1. added
    DeployBlockerCashThis issue or pull request should block deployment
    ExternalAdded to denote the issue can be worked on by a contributor
    on Feb 27, 2024
  2. changed the title [-] IOU - Request details page loads infinitely when request is created offline[/-] [+][$500] IOU - Request details page loads infinitely when request is created offline[/+] on Feb 27, 2024
  3. melvin-bot commented on Feb 27, 2024

    @melvin-bot
  4. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Feb 27, 2024
  5. melvin-bot commented on Feb 27, 2024

    @melvin-bot

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

  6. github-actions commented on Feb 27, 2024

    @github-actions
    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.
  7. melvin-bot commented on Feb 27, 2024

    @melvin-bot

    Auto-assign attempt failed, all eligible assignees are OOO.

  8. lanitochka17 commented on Feb 27, 2024

    @lanitochka17
    Author

    We think that this bug might be related to #vip-bills
    CC @davidcardoza

  9. dukenv0307 commented on Feb 27, 2024

    @dukenv0307
    Contributor

    Proposal

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

    The request details page loads infinitely when the request is created offline (production behavior).
    This issue also occurs when:

    What is the root cause of that problem?

    This is regression of this PR Expensify/react-native-onyx#470 since we always set value to mapping.initialValue without the check mapping.initialValue is undefined or not

    And then we have allowStaleData as true for report key here which makes cachedState wrong and then report data in Onyx is wrong.

    allowStaleData: true,

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

    We should add the check mapping.initialValue !== undefined here https://github.com/Expensify/react-native-onyx/blob/c9f22a3f655eb14b1cce7b98282fe21646c5d27d/lib/withOnyx.js#L62

    What alternative solutions did you explore? (Optional)

    We can remove allowStaleData here and then cachedState will be calculated correctly

    allowStaleData: true,

  10. melvin-bot commented on Feb 27, 2024

    @melvin-bot
  11. youssef-lr commented on Feb 27, 2024

    @youssef-lr
    Contributor

    cc @bernhardoj I think this is related to your PRs here & here

  12. bernhardoj commented on Feb 27, 2024

    @bernhardoj
    Contributor

    Checking

  13. bernhardoj commented on Feb 27, 2024

    @bernhardoj
    Contributor

    Okay, it's indeed reproducible after the Onyx version bump, but here is the real root cause.

    What the recent Onyx PR does is to immediately render the report screen by fixing some initialValue that are not applied when they are falsey (false or null). Btw, this PR is the first one that attempted to immediately render the report screen, but because we have the Onyx bug, it's not solved 100%.

    1. Now, when we press the money request preview, we create an optimistic transaction thread and navigate to it.
      if (!childReportID) {
      const thread = ReportUtils.buildTransactionThread(action, requestReportID);
      const userLogins = PersonalDetailsUtils.getLoginsByAccountIDs(thread.participantAccountIDs ?? []);
      Report.openReport(thread.reportID, userLogins, thread, action.reportActionID);
      Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(thread.reportID));
      return;

    Because the report screen is now immediately rendered, the new report Onyx data is not available yet, so the code below will call openReport.

    // It possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
    // is not stored locally yet. If report.reportID exists, then the report has been stored locally and nothing more needs to be done.
    // If it doesn't exist, then we fetch the report from the API.
    if (report.reportID && report.reportID === getReportID(route) && !isLoadingInitialReportActions) {
    return;
    }
    Report.openReport(reportIDFromPath);

    1. In openReport, we have a logic to use SET method (for the optimistic report) when the report is not in the Onyx yet,
      const report = ReportUtils.getReport(reportID);
      // If we open an exist report, but it is not present in Onyx yet, we should change the method to set for this report
      // and we need data to be available when we navigate to the chat page
      if (isEmptyObject(report)) {
      optimisticData[0].onyxMethod = Onyx.METHOD.SET;
      }

    The optimistic report looks like this

    const optimisticReport = reportActionsExist(reportID)
    ? {}
    : {
    reportName: allReports?.[reportID]?.reportName ?? CONST.REPORT.DEFAULT_REPORT_NAME,
    };

    So, the optimistic transaction thread is overwritten with the report above that only contains reportName, thus the infinite loading.

    How do we solve this?

    I would remove the logic to change the onyx method to SET (in point 2). It was added in this PR to solve #19364. The issue is that the not-found view is shown briefly (instead of skeleton loading) when opening a thread.

    If we read the root cause from the proposal, it's because when we don't have the report data yet, the default props of the report will have a value of isLoadingReportActions as false which makes the not found condition becomes true.

    report: {
    hasOutstandingIOU: false,
    isLoadingReportActions: false,
    },

    <FullPageNotFoundView
    shouldShow={(!this.props.report.reportID && !this.props.report.isLoadingReportActions && !isLoading) || shouldHideReport}

    isLoadingReportActions will become true when the openReport optimistic data is merged.

    function openReport(reportID, participantList = [], newReportObject = {}, parentReportActionID = '0', isFromDeepLink = false) {
    const optimisticReportData = {
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
    value: {
    isLoadingReportActions: true,

    Instead of using SET, we should just set the default value of isLoadingReportActions to true which is already the case today.
    isLoadingReportActions is isLoadingInitialReportActions in the latest code.

    reportMetadata: {
    key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`,
    initialValue: {
    isLoadingInitialReportActions: true,

    const shouldShowNotFoundPage = useMemo(
    () =>
    (!wasReportAccessibleRef.current &&
    !firstRenderRef.current &&
    !report.reportID &&
    !isOptimisticDelete &&
    !reportMetadata.isLoadingInitialReportActions &&

    @youssef-lr should the solution be reviewed first or should I just immediately create the follow-up PR to fix this?

  14. melvin-bot commented on Feb 27, 2024

    @melvin-bot
  15. youssef-lr commented on Feb 27, 2024

    @youssef-lr
    Contributor

    You can start on the PR, thanks!

  16. self-assigned this
    on Feb 27, 2024
  17. paultsimura commented on Feb 27, 2024

    @paultsimura
    Contributor

    Hey, @bernhardoj just a note: a part of the root cause is that the money report actions have no childReportID when created offline – this is being fixed in #37232

  18. bernhardoj commented on Feb 27, 2024

    @bernhardoj
    Contributor

    @paultsimura thanks, yep I noticed about that too, but not really sure whether it's intentional or not.

    Here is the PR to fix this issue #37269

  19. puneetlath commented on Feb 27, 2024

    @puneetlath
    Contributor

    Looks like this revert fixed it. Closing, but feel free to reopen if I'm mistaken.

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

Metadata

Metadata

Labels

EngineeringExternalAdded to denote the issue can be worked on by a contributorHelp WantedApply this label when an issue is open to proposals by contributorsReviewingHas a PR in reviewWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions