Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.29k stars 2.73k forks source link

Thread - App flickers & returns to main chat when leaving and reentering thread #45834

Closed m-natarajan closed 21 hours ago

m-natarajan commented 1 month ago

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: 9.0.10-2 Reproducible in staging?: y Reproducible in production?: no new feature 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): 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 to workspace chat.
  3. Send a message.
  4. Open the message in thread > Send a message in thread.
  5. Leave the thread via thread header.
  6. Click on the thread subtitle to return to main chat.
  7. Open the thread again from the main chat.
  8. Note that app auto returns to main chat.

DM issue

  1. Go to staging.new.expensify.com
  2. Go to DM.
  3. Send a message.
  4. Open the message in thread > Send a message in thread.
  5. Leave the thread via thread header.
  6. Click on the thread subtitle to return to main chat.
  7. Open the thread again from the main chat.
  8. Note that thread flickers briefly.

Expected Result:

No flicker issue and app will remain on the same thread when leaving and reentering thread.

Actual Result:

Screenshots/Videos

https://github.com/user-attachments/assets/accd65e3-fef7-4d64-b6ee-c56e462fe08c

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @chiragsalian
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @trjExpensify (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.

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @srikarparsi (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 1 month ago

:wave: 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.
tsa321 commented 1 month ago

This seems related to my PR regarding loading the skeleton of the report screen (I believe only related to report action list). However, the loading is also displayed for the composer and header of the report, which are different components from the the report action list. I will investigate further.

tsa321 commented 1 month ago

It seems like this is a backend issue. When user leaves the thread and returns to the parent report, and then goes back to the thread again, GetMissingOnyxMessages sets the report and report_actions to null.

ss-2

mountiny commented 1 month ago

Thanks, @tsa321. I don't think this is major enough to be a blocker, as it's a rare scenario for the user to do, and it's just a minor UX issue. @tsa321, do you, however, know if this is not happening in the production backend? Maybe its a mix of frontend and backend change

mountiny commented 1 month ago

@trjExpensify @srikarparsi please feel free to make this a blocker if you disagree with my reasoning above

tsa321 commented 1 month ago

@mountiny, in production, the 'leave' button doesn't exist. So, I think this is a new feature?

mountiny commented 1 month ago

@tsa321 not completely, it used to be in the three dot menu, but it was omitted by mistake when we moved all the actions to RHP so its not new new, just new :tm:

trjExpensify commented 1 month ago

Haha, so are we seeing this reproduced on prod?

mountiny commented 1 month ago

Well not in the current prod as you were not able to leave. Its hard to say if this was the case couple weeks back before the details revamp

trjExpensify commented 1 month ago

This shows the option to Leave in details, has that not made its way to prod?

image
mountiny commented 1 month ago

@trjExpensify we have just deployed so now it will be in production too

melvin-bot[bot] commented 1 month ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

trjExpensify commented 1 month ago

Hm okay, so what are we doing here? I'm a little lost on this issue.

tsa321 commented 1 month ago

@trjExpensify I think we should label this issue as internal to correct the backend response of GetMissingOnyxMessages here.

srikarparsi commented 1 month ago

Ok I'm able to reproduce both of the issues from the issue body on production and agree with @tsa321's analysis. We do eventually get to the right report both times but there is a "glitch". My plate's really full right now so I don't think I'll be able to look into this until late next week, do you think we could see if there's anyone who could look into this sooner?

trjExpensify commented 1 month ago

Sounds good! @muttmuure is this janky enough in a core chat flow to go into #newdot-quality?

muttmuure commented 1 month ago

Probably as a LOW?

trjExpensify commented 1 month ago

Sounds good!

melvin-bot[bot] commented 1 month ago

@trjExpensify @srikarparsi this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

srikarparsi commented 1 month ago

I'm still working on wave-collect and instant-smartscan and don't have the bandwidth to take this one on. I'm going to un-assign as per this slack post but please re-assign me if anything has changed since then.

trjExpensify commented 1 month ago

@muttmuure, do we add the #newdot-quality auto assigner here now? Is that the process?

muttmuure commented 1 month ago

Yes, exactly

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @chiragsalian (AutoAssignerNewDotQuality)

muttmuure commented 1 month ago

https://stackoverflowteams.com/c/expensify/questions/19512

chiragsalian commented 2 weeks ago

oh whoops i thought this was an external issue. I'll try to catch up on this when i get the chance.

muttmuure commented 1 week ago

Oh, I think I misunderstood what this issue was, I'm going to add it to UX reliability

chiragsalian commented 2 days ago

Okay i finally got some free time to check this out today. I tested on main and staging and I'm unable to reproduce the issue. After i leave the thread, when i click into the thread again i only see an OpenReport network request. I do not see a GetMissingOnyxMessages request. Maybe this was resolved but a change in flow elsewhere.

Can someone else retest and confirm if this issue still persists? @m-natarajan @tsa321 @muttmuure. If not we can close out this issue.

tsa321 commented 2 days ago

I think this issue has been fixed. I am unable to reproduce it in staging.

muttmuure commented 21 hours ago

OK seems to be fixed on staging for me

muttmuure commented 21 hours ago

Going to close

muttmuure commented 21 hours ago

Thanks!