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.49k stars 2.84k forks source link

Chat -Green line displays after send attachment/messages offline and come back online. #11237

Closed kbecciv closed 1 year ago

kbecciv commented 2 years 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!


Action Performed:

  1. Launch the app
  2. Log in with any account
  3. Turn off your internet connection
  4. Create a new room
  5. Tap on plus button
  6. Select Add Attachment
  7. Send Attachment
  8. Enable the internet connection

Expected Result:

There is no green line displayed after come back online

Actual Result:

New line displays after send attachment offline and come back online.

Workaround:

Unknown

Platform:

Where is this issue occurring?

Version Number: 1.2.5.0

Reproducible in staging?: Yes

Reproducible in production?: No, can not create room while offline

Email or phone of affected tester (no customers): any expensifail account

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/191973668-4c1fd8aa-ec33-4dac-beae-84b95db77c75.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

OSBotify commented 2 years 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.
melvin-bot[bot] commented 2 years ago

Triggered auto assignment to @sketchydroide (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

sketchydroide commented 2 years ago

is this only on Android, or does this happen in other platforms? It seems weird to be platform specific

sketchydroide commented 2 years ago

@kbecciv ? (sorry forgot to tag you in the previous comment)

sketchydroide commented 2 years ago

Also please correct the title, it's a bit confusing as New line means a empty line in programing language, aka return/enter, did you mean Green Line? thanks :)

sketchydroide commented 2 years ago

@marcaaron I think this change might have been the cause of it? As it's a relative new change, and I think we just added the QA testrail steps yesterday.

This said, I'm not 100% this should be a deploy blocker. Offline first stuff are a new feature, so it makes sense it's not in prod yet. Also this seems to be an edge case.

@kbecciv is this for attachements only, or would it be an issues with messages as wel:?

sketchydroide commented 2 years ago

maybe not that change, I think it might be on componentDidUpdate

kbecciv commented 2 years ago

@sketchydroide Let me try with messages as well.

kbecciv commented 2 years ago

The same issue with message

https://user-images.githubusercontent.com/93399543/191980433-67e86fb6-3ed9-457d-af6e-048801b0f470.mp4

sketchydroide commented 2 years ago

Ok, after some investigation I was able to reproduce the bug in Prod.

Here are the reproduction steps:

  1. Launch the app
  2. Log in with any account
  3. Create a new room
  4. Turn off your internet connection
  5. write a new comment
  6. Enable the internet connection

after a while the green line will appear.

I Don't think this is related to the Add Room Offline functionality. I'm inclined to remove the blocker, as I was able to reproduce something that I think it's the actual issue.

luacmartins commented 2 years ago

I couldn't reproduce this. This seems really similar to https://github.com/Expensify/App/issues/11216. Per this comment I'll tag @marcaaron so you can keep it in your radar.

I'll remove the blocker on this one since it seems like an inconsistent minor UI issue.

melvin-bot[bot] commented 2 years ago

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION 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 production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

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.

sketchydroide commented 2 years ago

aother similar GH opened here.

marcaaron commented 2 years ago

idk I think it can still be a Daily and added to the Whats App Quality milestone. No reason why someone can't look into this.

marcaaron commented 2 years ago

I'm gonna take this off the Unread Indicators milestone and move it to Whats App quality. Looks like tester is using an optimistic report and I have no idea whether unread indicators were taken into consideration for the optimistic report stuff.

sketchydroide commented 2 years ago

Ok I'll take a look at this today

sketchydroide commented 2 years ago

I can't replicate this on dev, none of the steps previously described, will try again on staging

sketchydroide commented 2 years ago

I'm having some trouble checking on this today, I think it's because I'm working from a coworking space today, will chack again on monday from home

sketchydroide commented 2 years ago

will try this again today

sketchydroide commented 2 years ago

was busy with other things, will take a look today

sketchydroide commented 2 years ago

I can no longer replicate this, neither in staging or dev. I do get duplicated Messages, but I think that is a different issue and the changes on the sequenceNumber

sketchydroide commented 2 years ago

Screenshot 2022-10-14 at 16 40 36

sketchydroide commented 2 years ago

demoting this to weekly, as I don't think this is happening anymore.

@kbecciv can your team take a look to see if it still happens?

sketchydroide commented 2 years ago

@kbecciv bump :)

kbecciv commented 2 years ago

@sketchydroide Checking, will update you shortly

kbecciv commented 2 years ago

@sketchydroide Issue is reproduced with latest build 1.2.16-4

https://user-images.githubusercontent.com/93399543/196197000-be125e5a-3bc3-416e-a42f-7faa4d4b74d4.MP4

sketchydroide commented 2 years ago

Thanks, I think it might be part of the process the login part. let me try again.

melvin-bot[bot] commented 2 years ago

@sketchydroide Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

sketchydroide commented 2 years ago

still haven't had the time to check, busy with the deployment, will check again

sketchydroide commented 1 year ago

Checking again this week. once I fix vagrant and php

sketchydroide commented 1 year ago

Vagrant fixed, I can look at this again

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@sketchydroide Huh... This is 4 days overdue. Who can take care of this?

sketchydroide commented 1 year ago

I'm removing myself, I haven't had the time to look at this, and as daily I should be focusing on it and have not been able to

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

jliexpensify commented 1 year ago

Just to confirm, this is what you're referring to @kbecciv ?

image

if so, I believe that's intended: since it's a new message you're sending, it makes sense to have a green line. I'm on v 1.2.23-0 and am seeing the same behaviour.

kbecciv commented 1 year ago

Yes, correct. I am referring to this green line above the attachment.

jliexpensify commented 1 year ago

Thanks for confirming - so again, my understanding is that this is intended behaviour, as when you're offline and a message is sent, you shouldn't see them until you get back online, which means they'd show up as "New".

Closing this one out, but please re-open if you feel like this is bug.

melvin-bot[bot] commented 1 year ago

Be sure to fill out the Contact List!