[$250] Assign Task - When changing Assignee, the page lags when closing modal

[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.81-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+emilio@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Open New Expensify App
2. Sign into a valid account (HT account is better for reproduction)
3. Create a Task and then click on the IOU
4. Change the Assignee and notice the page closing.

Expected Result:

User expects the page to close "clean"

Actual Result:

The page has a slight "lag/Delay" when closing, visible for a couple of seconds.

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

Bug6508417_1718040550665.When_changing_Assignee_the_page_lags.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01794355bb64abf4a9
• Upwork Job ID: 1800741657347973622
• Last Price Increase: 2024-07-10
• Automatic offers:
• allgandalf | Reviewer | 103151963
• truph01 | Contributor | 103151965

Issue Owner

Current Issue Owner: @allgandalf

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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]

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

[JmillsExpensify]

I'll open it up to the community for proposals.

[melvin-bot]

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

[melvin-bot]

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

[truph01]

Proposal

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

The page has a slight "lag/Delay" when closing, visible for a couple of seconds.

What is the root cause of that problem?

After selecting the assignee in

App/src/pages/tasks/TaskAssigneeSelectorModal.tsx

Line 182 in 8d11d0b

Navigation.dismissModal(report.reportID);

, some Onyx updates will occur that affect the JS thread. Those Onyx updates are from TaskActions.editTaskAssignee that was called here, they updates both the report and report actions which are heavy.

Right after that we goBack, so the navigation animation is affected by the Onyx updates.

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

Make use of useWaitForNavigation, we can navigate first, then handle the Onyx data update in the background, the Onyx data update is not user-visible so it's best done in the background so other user-visible things like navigation/animation is not obstructed

Navigation.dismissModal(report.reportID);

waitForNavigate(() => {
    // handle the data update here
});

The data update is this block

And do the same in the new task case

What alternative solutions did you explore? (Optional)

Run the navigation in InteractionManager.runAfterInteractions so it waits for the Onyx updates to complete before navigating

InteractionManager.runAfterInteractions(() => {
    Navigation.dismissModal(report.reportID);
});

And in here too

Another optional point to consider, in

App/src/pages/tasks/TaskAssigneeSelectorModal.tsx

Line 240 in 8d11d0b

reports: {

, it's connecting to the full reports collection even though only 1 report from the route param is needed. We can change to connect to only that 1 report by withOnyx or useOnyx, it will also help it to be more smooth.

Some other notes:

• After the changes, after pressing the button, the option looks like it's scrolling up, I believe this issue is being fixed in [$250] Web - User has to click twice to select currency if the currency is around the bottom #41922 (comment)
• We can freeze the sections by storing it by usePrevious, and use it instead of sections right after updating the data (before InteractionManager.runAfterInteractions for the navigation), so the sections will be freezed and is not updated while we're watiting for navigation.

[truph01]

Proposal has been updated to add an optional change

[truph01]

Proposal updated to add alternative fix

[allgandalf]

@truph01 , Your current RCA is:

some Onyx updates will occur that affect the JS thread. Right after that we goBack, so the navigation animation is affected by the Onyx updates.

Can you please point as to what Onyx updates you are referring to? it's necessary to have a clear RCA for us to proceed with solution,

We should know why first then we can think of possible solution to a bug , thanks and looking forward to your reply

[truph01]

Can you please point as to what Onyx updates you are referring to?

@allgandalf Appreciate the feedback! I added more details to the proposal.

[allgandalf]

Chill melvin, will review the updated proposal today/tomorrow 👍

[melvin-bot]

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

[allgandalf]

@JmillsExpensify Discussing something internally

[allgandalf]

Internal discussion on this issue was completed, will review the pending proposal tomorrow 👍

[melvin-bot]

@JmillsExpensify @allgandalf this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[allgandalf]

sorry was AFK, will review today/tomorrow, reviewing an urgent deploy blocker today

[allgandalf]

@truph01 , your RCA is correct, and according to the expected result:

User expects the page to close "clean"

I think your solution is good to go!

@truph01 's Main Soution looks good to me here

🎀👀🎀 C+ reviewed

@roryabraham , please find the initial summary here

[MonilBhavsar]

Okay, thanks for clarifying. Let's move ahead and we can update to use useOnyx

[melvin-bot]

📣 @allgandalf 🎉 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

[melvin-bot]

📣 @truph01 🎉 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 📖

[truph01]

PR #45691 can be reviewed

[allgandalf]

Left some comments over the PR @truph01 , thanks

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[allgandalf]

Woah, @truph01 can you verify on staging if this is still reproducible ?

[allgandalf]

friendly bump @truph01

[truph01]

@allgandalf I tested it on my end, and the bug is still reproducible, but not consistently.

[allgandalf]

how should we proceed here @MonilBhavsar ?, Qa said the bug is not reproducible and contributor says it is reproducible but not consistently

[truph01]

@allgandalf What about your end? Can you reproduce it?

[allgandalf]

bump @MonilBhavsar

[MonilBhavsar]

Sorry missed it somehow. i think we should still fix it and improve the code. I guess it will be constantly reproduced on high traffic accounts?

[allgandalf]

I guess it will be constantly reproduced on high traffic accounts?

Let me check on current staging

[allgandalf]

The issue is reproducible to me on staging, i will go forward with reviewing the PR

Screen.Recording.2024-07-31.at.11.09.56.AM.mov

[MonilBhavsar]

Yes! Thank you for checking

[allgandalf]

Still working on the PR, had one doubt, can you address that when you get time @MonilBhavsar

[allgandalf]

• The PR that introduced the bug has been identified. Link to the PR: 20354 task assignees list refactor #35841

• 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: https://github.com/Expensify/App/pull/35841/files#r1732183573

• 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

• 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 App
2. Create a Task and then click on the IOU
3. Change the Assignee and notice the page closing.

Verify that the page closes without delay/lag

Do we agree 👍 or 👎

[allgandalf]

@JmillsExpensify , this is ready for payment 🙇

[JmillsExpensify]

Payment summary:

• Contributor: @truph01 $250
• Reviewer: @allgandalf $250

[JmillsExpensify]

All contributors paid out. I'm going to past on creating the regression test given the scale/type of bug, so closing this out!