Report name is disappearing 5 seconds after page load

[tugbadogan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Expected Result:

Chat report name is not disappearing 5 seconds after chat page is loaded.

Actual Result:

Avatar and chat report name are disappearing 5 seconds after reload, avatar comes back, but name doesn't.

Action Performed:

Load a chat report on staging.expensify.cash and observe that report name is gone after 2-3 seconds.

Workaround:

Visit another report without loading page and header view is fixed.

Platform:

Where is this issue occurring?

[x] Web
[x] iOS
[x] Android
[x] Desktop App
[x] Mobile Web

Version Number: v1.0.11-2
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Screen.Recording.2021-04-05.at.16.43.00.mov

Expensify/Expensify Issue URL:

[tugbadogan]

This issue is caused by the following problem. When Onyx.mergeCollection is called, we should merge data with the modified data and update subscribers with merged data instead of updating them only with modified data.

Proposal

1. Modify updateIndividualSubscriber function to use data from prevState and use lodashMerge to merge objects, so we
2. Add unit tests to check if merge and mergeCollection functions are working as expected and we're updating subscribers with merged data instead of new data.

https://github.com/Expensify/react-native-onyx/blob/aa36bdb7b5cd2319c34b9ee1b4c73cb73e89f4a5/lib/Onyx.js#L160-L183

It looks like this function is regressed after @marcaaron 's changes in Expensify/react-native-onyx#57

[MelvinBot]

Triggered auto assignment to @slafortune (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[AndrewGable]

@tugbadogan - Can you write a failing test to show the regression in react-native-onyx? If so- We can make this into an Upwork job and pay you to fix it 👍

[tugbadogan]

Hi @AndrewGable
I've created a new test case to show the regression and it's failing here => https://github.com/Expensify/react-native-onyx/pull/60/checks?check_run_id=2274072407

Expensify/react-native-onyx#60

[marcaaron]

Thanks @tugbadogan. Finally got around to looking at all of this.

I think the issue stems from that merge() and mergeCollection() fundamentally work a bit differently - I missed this in my last change to Onyx. Calls to merge() are batched so there is no need to use prevState but calls to mergeCollection() are not batched.

Modify updateIndividualSubscriber function to use data from prevState and use lodashMerge to merge objects

While this will fix the issue and the failing test it feels incorrect since only in the case of mergeCollection() should we need to "use data from prevState and use lodashMerge to merge objects". If there a way to solve this without leaving this concern dangling then I am 👍

[Julesssss]

If there a way to solve this without leaving this concern dangling then I am 👍

@marcaaron Couldn't this simply be achieved via a flag passed to updateIndividualSubscriber()?. Is that the sort of solution you are suggesting, or something else?

[Julesssss]

@slafortune, it would be great if we could get the UpWork issue created for this one today, as it's a bit of a blocker for IOU/N5. Thanks in advanced