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

[$250] Onboarding - Infinite loop in onboarding modal when coming from OD via Support #51481

Closed
1 of 8 tasks
lanitochka17 opened this issue Oct 25, 2024 · 28 comments
Closed
1 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 25, 2024

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


Version Number: 9.0.54
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5121161
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to https://staging.expensify.com/
  2. Get started with “Organize my own expenses” option
  3. Verify you redirect to NewDot and Onboarding modal appears
  4. Click on "Chat and split expenses with friends" option on the Onboarding modal
  5. Enter user name and click Continue button

Expected Result:

The user can complete the onboarding flow and is brought to the NewDot concierge chat

Actual Result:

The user remains stuck in an infinite loop in the "What do you want to do today?" modal when he chooses the "Chat and split expenses with friends" option.
By selecting any other option, for instance, "Get paid back by my employer" the loop ends.

Workaround:

Unknown

Platforms:

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

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6645146_1729868484024.Infinite_loop_in_onboarding.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021850366174167695381
  • Upwork Job ID: 1850366174167695381
  • Last Price Increase: 2024-11-24
Issue OwnerCurrent Issue Owner: @fedirjh
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 25, 2024
Copy link

melvin-bot bot commented Oct 25, 2024

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@lanitochka17
Copy link
Author

@jliexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@NJ-2020
Copy link
Contributor

NJ-2020 commented Oct 26, 2024

Edited by proposal-police: This proposal was edited at 2024-10-26 04:14:21 UTC.

Proposal

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

Onboarding - Infinite loop in onboarding modal when coming from OD via Support

What is the root cause of that problem?

When we navigate to concierge via support, the concierge chat doesn't exist yet so we will create new optimistic chat and generator new report id for the concierge chat

App/src/libs/actions/Report.ts

Lines 1025 to 1035 in 9508b17

newChat = ReportUtils.buildOptimisticChatReport(
[...participantAccountIDs, currentUserAccountID],
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,

So when we refresh the page or navigate to concierge chat again via support (like in the video), onboarding modal show's up and when completing the onboarding the onboarding show's up again and causes infinite loop

It's because the BE returns error saying invalid data, because we pass invalid report id which is optimistic concierge report id that we just created for the targetChatReport for introduction message and text message

App/src/libs/actions/Report.ts

Lines 3341 to 3348 in 9508b17

const targetChatReport = ReportUtils.getChatByParticipants([actorAccountID, currentUserAccountID]);
const {reportID: targetChatReportID = '', policyID: targetChatPolicyID = ''} = targetChatReport ?? {};
// Introductory message
const introductionComment = ReportUtils.buildOptimisticAddCommentReportAction(CONST.ONBOARDING_INTRODUCTION, undefined, actorAccountID);
const introductionCommentAction: OptimisticAddCommentReportAction = introductionComment.reportAction;
const introductionMessage: AddCommentOrAttachementParams = {
reportID: targetChatReportID,

We get targetChatReportID for the concierge report id by using ReportUtils.getChatByParticipants function and pass the user and concierge participants for the participants value

And the getChatByParticipants function will return the first report chat which is the optimistic report chat

But in this case why the optimistic chat report doesn't get removed? even we handle it right here and the BE handling correctly returning the preexistingReportID when the optimistic report chat is opened ...

App/src/libs/actions/Report.ts

Lines 1369 to 1376 in 9508b17

// It is possible that we optimistically created a DM/group-DM for a set of users for which a report already exists.
// In this case, the API will let us know by returning a preexistingReportID.
// We should clear out the optimistically created report and re-route the user to the preexisting report.
if (report?.reportID && report.preexistingReportID) {
let callback = () => {
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, null);
};

It's because almost every API write requests the onyx updates will be queued, so first OpenApp API command get called and the onyx updates get flush which will apply the onyx updates from BE to Onyx, and when it's done applying it will clear the queuedOnyxUpdates

function queueOnyxUpdates(updates: OnyxUpdate[]): Promise<void> {
queuedOnyxUpdates = queuedOnyxUpdates.concat(updates);
return Promise.resolve();
}
function flushQueue(): Promise<void> {
return Onyx.update(queuedOnyxUpdates).then(() => {
queuedOnyxUpdates = [];
});
}

But when we still applying the data for OpenApp, the OpenReport get called for optimistic concierge report chat and returns the preexistingReportID onyx updates and will push the onyxUpdates to queuedOnyxUpdates variable

But as we can see in the above the queuedOnyxUpdates will get cleared after the Onyx applied the onyxUpdates for for OpenApp

function flushQueue(): Promise<void> {
return Onyx.update(queuedOnyxUpdates).then(() => {
queuedOnyxUpdates = [];
});
}

so the OpenReport onyx updates response will get dismiss because it's got cleared after the Onyx applied the onyx updates for OpenApp

Demo:

Screen.Recording.2024-10-25.at.19.50.08.1.mov

As we can see in the video the OpenReport for concierge optimistic report chat onyx updates (preexistingReportID) got cleared inside queuedOnyxUpdates variable after Onyx applied the onyxUpdates for OpenApp

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

We should clear the queuedOnyxUpdates immediately without waiting until the Onyx applied the onyx updates for OpenApp

function flushQueue(): Promise<void> {
    const savedQueuedOnyxUpdates = queuedOnyxUpdates;
    queuedOnyxUpdates = [];
    return Onyx.update(savedQueuedOnyxUpdates);
}

Result

Screen.Recording.2024-10-25.at.21.12.38.mov

What alternative solutions did you explore? (Optional)

@jliexpensify
Copy link
Contributor

Yikes, this one is pretty bad - this is what I see in my LHN when I select the split option:

image

I can also confirm the Track option works fine and gets you out of the loop:

image

@jliexpensify jliexpensify added the External Added to denote the issue can be worked on by a contributor label Oct 27, 2024
@melvin-bot melvin-bot bot changed the title Onboarding - Infinite loop in onboarding modal when coming from OD via Support [$250] Onboarding - Infinite loop in onboarding modal when coming from OD via Support Oct 27, 2024
Copy link

melvin-bot bot commented Oct 27, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021850366174167695381

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 27, 2024
Copy link

melvin-bot bot commented Oct 27, 2024

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

Copy link

melvin-bot bot commented Oct 30, 2024

@jliexpensify, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick!

@jliexpensify
Copy link
Contributor

Bump @fedirjh we have a proposal for you to review.

@jliexpensify
Copy link
Contributor

DM-ed Fedi on Slack

@fedirjh
Copy link
Contributor

fedirjh commented Nov 1, 2024

I am unable to replicate the bug on staging v9.0.56-3 . It seems like the duplicate concierge is removed instantly for me :

CleanShot.2024-11-01.at.12.52.10-trimmed.mp4
CleanShot.2024-11-01.at.12.52.10.mp4

@melvin-bot melvin-bot bot removed the Overdue label Nov 1, 2024
@NJ-2020
Copy link
Contributor

NJ-2020 commented Nov 1, 2024

@fedirjh Yes it seems not reproducible on staging

But i just tested again with the latest main on dev mode and the issue is still reproducible

Screen.Recording.2024-11-01.at.05.11.47.1.mov

Copy link

melvin-bot bot commented Nov 3, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Nov 3, 2024
@fedirjh
Copy link
Contributor

fedirjh commented Nov 4, 2024

@NJ-2020 This is what I got on latest main (0647baf)

CleanShot.2024-11-04.at.10.59.23.mp4

@melvin-bot melvin-bot bot removed the Overdue label Nov 4, 2024
@NJ-2020
Copy link
Contributor

NJ-2020 commented Nov 4, 2024

@fedirjh You need to refresh to show the onboarding modal

@NJ-2020
Copy link
Contributor

NJ-2020 commented Nov 4, 2024

@fedirjh I tried again reproducing on the latest main, seems the issue is not reproducible both staging and dev

@fedirjh
Copy link
Contributor

fedirjh commented Nov 4, 2024

@NJ-2020 Thanks for re-testing 🙌🏼

@jliexpensify Can you please add the re-test label ?

@muttmuure muttmuure moved this from CRITICAL to LOW in [#whatsnext] #quality Nov 5, 2024
@muttmuure muttmuure moved this from LOW to MEDIUM in [#whatsnext] #quality Nov 5, 2024
@muttmuure
Copy link
Contributor

Removing due to low reproducibility

@jliexpensify jliexpensify added retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2 and removed Daily KSv2 labels Nov 6, 2024
@jliexpensify
Copy link
Contributor

Thanks @fedirjh - added!

Copy link

melvin-bot bot commented Nov 10, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Nov 16, 2024
Copy link

melvin-bot bot commented Nov 17, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@jliexpensify
Copy link
Contributor

Nice, will wait another week of testing before closing.

@fedirjh
Copy link
Contributor

fedirjh commented Nov 18, 2024

No overdue.

@melvin-bot melvin-bot bot removed the Overdue label Nov 18, 2024
Copy link

melvin-bot bot commented Nov 22, 2024

@jliexpensify @fedirjh this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@jliexpensify
Copy link
Contributor

Waiting on a final test from Applause

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

Copy link

melvin-bot bot commented Nov 24, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@jliexpensify
Copy link
Contributor

Great, lets close!

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. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

6 participants