[$4000] Deleting an image file which is still uploading displays empty image file and disappears after adding a reaction

[kavimuru]

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

---

Action Performed:

1. Go to Web chrome
2. Go to any chat
3. Upload an image file with 2MB+ size
4. Immediately delete the file when it says ‘Uploading attachment’ and wait for a moment.
5. First bug : You’ll see that the even if the file has already been deleted , a blank image file is displayed on the chat for some time. Additionally, ‘Attachment’ is displayed on the right side chat menu.
6. Second bug: Next, repeat the same steps and now react with any emoji in the empty image file. Notice that the image file goes away but the ‘Attachment’ is still displayed on the right hand side chat menu for some time.

Expected Result:

Deleting an image file that hasn't yet uploaded should not display an empty image file on the chat and should not display ‘Attachment’ on the chat menu

Actual Result:

Deleting an image file displays an empty image file on the chat and displays ‘Attachment’ on the chat menu.

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.82-3
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Recording.1674.mp4

image-2023-03-10_10.35.33.mp4

Expensify/Expensify Issue URL:
Issue reported by: @avi-shek-jha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1678424033842019

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014d0fc7ae438c05c4
• Upwork Job ID: 1636152686846541824
• Last Price Increase: 2023-04-05

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[MelvinBot]

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

[sonialiap]

I was able to reproduce it. Started the upload, deleted. The blank box popped up in chat and "[Attachment]" in LHB. Seconds later both disappeared and the chat moved down to where it was before I attempted to send the message. Same behavior if I reacji to the blank box, the blank box disappears and the "[Attachment]" also disappears within a few seconds.

Looks like it's some kind of delay

[deetergp]

@kavimuru Just to confirm, this only happens in web? Regardless, I'm sure it's can be dealt with in the front end by a contributor. Setting as External.

[MelvinBot]

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

[MelvinBot]

Current assignee @sonialiap is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @stitesExpensify (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[sonialiap]

Just FYI, I'll be OOO Mar 17-25

[usenbekov]

Proposal

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

Deleting an image file which is still uploading displays empty image file and disappears after adding a reaction

What is the root cause of that problem?

SequentialQueue processes finalization order is LIFO, but the order-sensitive operations that should be executed after every SequentialQueue process (like Onyx.update) should be FIFO order.

Details:
User.js -> PusherUtils.subscribeToPrivateUserChannelEvent(ONYX_API_UPDATE) -> SequentialQueue.getCurrentRequest().then -> Onyx.update(pushJSON)

Here Onyx.update(pushJSON) will be called when the SequentialQueue current request is finished. But when we delete the attachment which is still uploading the Onyx.update(AddAttachment) won't be called until the delete process is finished, so it will be called last.

The visualization looks like this:

(AddAttachment)  process start {
    (DeleteComment) process start {
        ...next process in the SequentialQueue
    } then {
        Onyx.update(DeleteComment)
    }
} then {
    Onyx.update(AddAttachment)
}

So the Onyx.update(AddAttachment) will be called only after the next processes are finished, including the DeleteComment process.

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

We can keep another list of promises and for SequentialQueue.getCurrentRequest() we can return the last promise from this list. And on SequentialQueue -> on process finalization we can resolve these promises in the correct order.

The result is:

Screen.Recording.2023-03-30.at.11.36.27.AM.mp4

What alternative solutions did you explore? (Optional)

Another solution would be:
At User.js -> subscribeToUserEvents -> PusherUtils.subscribeToPrivateUserChannelEvent
we can wait for the SequentialQueue finalization and execute all pushJSON objects in the correct order.

[MelvinBot]

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

[thesahindia]

If I am not wrong we have plans to remove "uploading attachment" text, so I think we should do nothing here.

[0xmiros]

If I am not wrong we have plans to remove "uploading attachment" text, so I think we should do nothing here.

@stitesExpensify can you please confirm this?

[MelvinBot]

@sonialiap @stitesExpensify @0xmiroslav 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!

[stitesExpensify]

If I am not wrong we have plans to remove "uploading attachment" text, so I think we should do nothing here.

@stitesExpensify can you please confirm this?

This is the first I've heard of this, but it's definitely possible. @thesahindia do you have the context for your comment?

[Pujan92]

I think we should not allow deleting/any action while the attachment is uploading. Same I have proposed here and for inspiration, we can consider a similar pattern in the slack.

[thesahindia]

Here's the thread for more info.

[usenbekov]

Proposal

Updated

[MelvinBot]

@sonialiap @stitesExpensify @0xmiroslav this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[MelvinBot]

@sonialiap, @stitesExpensify, @0xmiroslav Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[hoangzinh]

Are we going to close this GH issue or open for proposals?

[0xmiros]

Are we going to close this GH issue or open for proposals?

The discussion was 6 months ago but it seems no progress yet as low priority.
So I am not sure if we should "close this" or this is "worth fixing and go ahead".
@stitesExpensify will decide.

[MelvinBot]

@sonialiap, @stitesExpensify, @0xmiroslav Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[thesahindia]

I believe we should just close this issue because we have plans to remove "uploading attachment" text (Previous discussion thread)

[sonialiap]

@stitesExpensify what do you think of closing this one in favor of #9402?

[0xmiros]

This will be the expected result:

The image should be grayed (opacity 0.6 i.e., chatItemUnsentMessage) until its uploaded and once its uploaded it should not show the white thumbnail with loader
and instead continue showing the local thumbnail or show the final image immediately.

So uploading text will be replaced with grayed image or local thumbnail and it will still be reactionable.
So it makes sense to hold this issue and revisit after that PR merged.

[MelvinBot]

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

[stitesExpensify]

I think that closing it makes sense @sonialiap. If we're getting rid of this functionality, we don't really need to fix it.