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.36k stars 2.78k forks source link

[HOLD for payment 2023-07-26] [$1000] Attachments not shown for sender, stuck at 'Uploading attachment...' #22750

Closed Julesssss closed 1 year ago

Julesssss commented 1 year 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:

Expected Result:

Actual Result:

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

Version Number: 1.3.39-5 and 1.3.38-7 Reproducible in staging?: Yes Reproducible in production?: Yes Issue reported by: Me Slack conversation: N/A

Uploader (jules) sees:

Screenshot 2023-07-12 at 14 51 29

Other user sees:

image (5)

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019a63d2e48d9abd42
  • Upwork Job ID: 1679142267668922368
  • Last Price Increase: 2023-07-12
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

Julesssss commented 1 year ago

Hi @JmillsExpensify, do you recall anything unusual about the profile image file you used? I have only ever seen this issue on messages from you 😅

Pujan92 commented 1 year ago

Proposal

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

Attachments not shown for sender, stuck at 'Uploading attachment...'

What is the root cause of that problem?

Seems we are passing wrong value(undefined) here for prevVisibleMessageText Localize.translateLocal in the latest PR which seems breaking the flow for attachment. https://github.com/Expensify/App/blob/29ae9421d61c5a05e12408c258f64d4b65c3c2c4/src/libs/actions/Report.js#L248-L250 https://github.com/Expensify/App/commit/1a7bbcf0251a76c234b326b1fee2936e02d99d3e#diff-8afe5b71ee0436c21364148c86dadd640f2bff3e3d916addbb1f1f6f7e5b6a43R248-R250

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

We need to correct the value we are passing. cc: @roryabraham prevVisibleMessageText = Localize.translateLocal(lastVisibleMessage.lastMessageTranslationKey);

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~019a63d2e48d9abd42

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

Julesssss commented 1 year ago

FFS. Not again. Going back to Conor as he was the original assignee

melvin-bot[bot] commented 1 year ago

📣 @rushatgabhane Please request via NewDot manual requests for the Reviewer role ($1000)

melvin-bot[bot] commented 1 year ago

📣 @rushatgabhane We're missing your Upwork ID to automatically send you an offer for the Reviewer role. Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

melvin-bot[bot] commented 1 year ago

📣 @Pujan92 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

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 📖

roryabraham commented 1 year ago

My mistake, good find @Pujan92

Pujan92 commented 1 year ago

@rushatgabhane PR is ready for review!

melvin-bot[bot] commented 1 year ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 year ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.42-26 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-26. :confetti_ball:

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

melvin-bot[bot] commented 1 year ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

sakluger commented 1 year ago

Hey @conorpendergrast - @gadhiyamanan reported a similar issue (https://github.com/Expensify/App/issues/22762) 6-7 hours before this GH issue was created, and the PR here ended up fixing the issue he reported. I think that we should pay @gadhiyamanan the reporting bonus since he reportedthe issue first. Could you please send him an offer on this job for $250?

conorpendergrast commented 1 year ago

Sent! I'll pay that alongside everything else in this issue

conorpendergrast commented 1 year ago

Ok, it's two days past payday, sorry! Here's what I'm going to do:

conorpendergrast commented 1 year ago

@rushatgabhane Final step is the checklist, please! Then I'll do a regression test (if necessary) and I'll close this out

conorpendergrast commented 1 year ago

@rushatgabhane Bump on the checklist please. I'll be able to close this out after that!

rushatgabhane commented 1 year ago
  1. The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/22724

  2. 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/22724#discussion_r1262931017

  3. 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: Not required because it's a typo-ish.

  4. Determine if we should create a regression test for this bug. No

  5. 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 - N.A.

rushatgabhane commented 1 year ago

Created a manual request https://staging.new.expensify.com/r/8593074213496497

This issue can be closed 🙇

conorpendergrast commented 1 year ago

Woo, thanks!

JmillsExpensify commented 1 year ago

Reviewed details for @rushatgabhane. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot.