[HOLD for payment 2024-08-19] [$500] No feedback and delay when uploading a large attachment #38478

m-natarajan · 2024-03-18T15:19:00Z

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

Version Number:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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
Expensify/Expensify Issue URL:
Issue reported by: @neil-marcellini
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1710524006372329

Action Performed:

Use iOS on a slow network connection
Go to any chat
Click the plus button
Add an attachment
Select a video larger than the current limit of 24mb

Expected Result:

"Attachment too large Attachment size is larger than 25 MB limit" popup shows immediately

Actual Result:

Nothing happens for quite a while, then you receive a popover explaining that the attachment is too large

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

RPReplay_Final1710521379.MP4

IMG_6440.MOV

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01398c6a10dbc0b325
Upwork Job ID: 1770333731300958208
Last Price Increase: 2024-07-17
Automatic offers:

hoangzinh | Reviewer | 0
ShridharGoel | Contributor | 0

Issue Owner

Current Issue Owner: @Christinadobrzyn / @Christinadobrzyn

Issue Owner

Current Issue Owner: @puneetlath / @Christinadobrzyn

melvin-bot · 2024-03-18T15:19:12Z

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

ShridharGoel · 2024-03-18T17:48:58Z

Proposal

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

When uploading a large attachment there is a delay on iOS.

What is the root cause of that problem?

Image picker library is taking time to fetch the result on IOS.

To test it out, I had added logs to check time inside imageLibrary method in the native.ts file of the library.

The time difference was coming between call to launchImageLibrary and getting the result.

Github issue for some discussion on the same: react-native-image-picker/react-native-image-picker#2117

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

Add assetRepresentationMode as current.

const imagePickerOptions = {
    includeBase64: false,
    saveToPhotos: false,
    selectionLimit: 1,
    includeExtra: false,
+  assetRepresentationMode: 'current'
};

melvin-bot · 2024-03-20T06:17:09Z

Job added to Upwork: https://www.upwork.com/jobs/~01398c6a10dbc0b325

Christinadobrzyn · 2024-03-20T06:17:11Z

I can reproduce this. Here's the GH about increasing the size from 24 MB - #32702

I think this can be external

melvin-bot · 2024-03-20T06:17:14Z

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

ShridharGoel · 2024-03-20T06:46:48Z

Proposal

Updated

ShridharGoel · 2024-03-21T13:39:02Z

@hoangzinh Kindly have a look at my proposal when possible.

hoangzinh · 2024-03-21T14:36:46Z

Hi @ShridharGoel I don't think you have a correct RCA. When I put console log in that isValidFile function, it already had size value, so it just read from the object data.

ShridharGoel · 2024-03-21T17:52:14Z

@hoangzinh If you are using a Simulator, are you able to replicate the issue?

hoangzinh · 2024-03-22T05:15:19Z

ah nope I can't. Is it only reproducible in a real iphone?

ShridharGoel · 2024-03-22T09:00:18Z

@hoangzinh It seems like that. I've analysed the issue more - it's related to the image picker taking time. I'll update my proposal.

ShridharGoel · 2024-03-22T09:03:24Z

Proposal

Updated

hoangzinh · 2024-03-22T15:14:48Z

@Christinadobrzyn @ShridharGoel what is the file size you used to test? I used a video file with 128MB but it shows up file size error almost immediately

ShridharGoel · 2024-03-22T15:21:19Z

@hoangzinh Do you have access to a physical iPhone to check?

hoangzinh · 2024-03-22T15:29:49Z

Yes I do @ShridharGoel

ShridharGoel · 2024-03-22T16:38:27Z

@hoangzinh Can you check with a 500 MB video maybe?

@situchan Do you have an iPhone? If possible, can you check if you are able to replicate this?

situchan · 2024-03-22T16:45:40Z

@ShridharGoel I can test on my iPhone. Can you attach or share link to video file you tested?

ShridharGoel · 2024-03-22T17:10:18Z

@situchan Can you try this? Link

situchan · 2024-03-22T17:19:09Z

I am seeing "Attachment too large" modal immediately

Christinadobrzyn · 2024-03-25T02:06:32Z

testing this again, I am also seeing the "attachment too large" prompt immediately. So I think this is resolved but I'm just double-checking with @neil-marcellini to make sure this is what is expected.

RPReplay_Final1711332264.MP4

neil-marcellini · 2024-03-25T08:37:02Z

@Christinadobrzyn that looks like a slightly different flow you tested. I'm consistently able to reproduce with this video, regardless of network speed. I'm on iOS 17.3.1, New Expensify v1.4.50-5

IMG_6440.MOV

BrtqKr · 2024-07-22T21:28:46Z

@Christinadobrzyn I've been looking into this HEIC issue today. For now it seems like there's a mime type conversion happening for those images at some time. Conversion is a pretty standard approach for the HEIC format, but with assetRepresentationMode property used in there the transcoding is probably skipped and we end up with a broken file. I have an idea for a workaround - we might be able to convert it manually and skip the bug at the same time, but I'll try to verify it tomorrow morning.

BrtqKr · 2024-07-23T15:44:29Z

@Christinadobrzyn the PR is ready, I'll try to complete the recordings later this evening or tomorrow, but since the heic is being treated as a file attachment on other platforms, this should be ready for review. To sum up - the manual conversion worked and they indeed had an incorrect magic bytes signature in comparison to the file extension.

Christinadobrzyn · 2024-07-23T23:47:24Z

Awesome! Thank you so much @BrtqKr!

Christinadobrzyn · 2024-07-30T17:01:47Z

update for Melvin - PR is under review - @hoangzinh do you have a update on the PR review?

hoangzinh · 2024-07-31T00:04:12Z

The PR is still reviewing. I reported a bug in the PR and waiting for a fix. Hopefully, we can merge it this week.

Christinadobrzyn · 2024-08-06T21:56:22Z

monitoring PR #46025 - let us know if you have any other updates @hoangzinh

hoangzinh · 2024-08-07T09:56:24Z

PR has been merged and waiting deploy to staging

melvin-bot · 2024-08-12T09:52:54Z

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

melvin-bot · 2024-08-12T09:53:00Z

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

Fix image picker conversions #46025

If no regressions arise, payment will be issued on 2024-08-19. 🎊

For reference, here are some details about the assignees on this issue:

@hoangzinh requires payment automatic offer (Reviewer)
@ShridharGoel requires payment automatic offer (Contributor)
@BrtqKr does not require payment (Contractor)

melvin-bot · 2024-08-12T09:53:02Z

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:

[@hoangzinh] The PR that introduced the bug has been identified. Link to the PR:
[@hoangzinh] 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:
[@hoangzinh] 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:
[@hoangzinh] Determine if we should create a regression test for this bug.
[@hoangzinh] 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.
[@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/421516

Christinadobrzyn · 2024-08-13T20:29:15Z

Payouts due - can we confirm this is the correct payment for this job?

Contributor: $250 (50% less due to reverted PRs) @ShridharGoel (Paid in Upwork - https://www.upwork.com/nx/wm/offer/103513737)
Contributor+: $500 @hoangzinh (Paid in Upwork - https://www.upwork.com/nx/wm/offer/101646357)

Do we need a regression test for this @hoangzinh?

hoangzinh · 2024-08-14T09:44:22Z

@Christinadobrzyn, yep I think we need. I will complete BZ checklist soon

hoangzinh · 2024-08-14T10:21:14Z

Regression Test Proposal

On IOS/Android:

Sign in to any account
Go to any chat
Tap the plus button
Tap "Add an attachment" menu
Select "Chose from gallery"
Select a video larger than the current limit of 24MB
Verify that "Attachment size is larger than 24 MB limit" popup shows immediately

On Web/Desktop/mWeb:

Sign in to any account
Go to any chat
Tap the plus button
Tap "Add an attachment" menu
Select a video larger than the current limit of 24MB
Verify that "Attachment size is larger than 24 MB limit" popup shows immediately

Do we agree 👍 or 👎

hoangzinh · 2024-08-14T10:23:03Z

BugZero Checklist:

The PR that introduced the bug has been identified. Link to the PR: Prevent large attachment #4269
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: Prevent large attachment #4269 (comment)
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: N/A
Determine if we should create a regression test for this bug: ✅ Yes I think we should.
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. [HOLD for payment 2024-08-19] [$500] No feedback and delay when uploading a large attachment #38478 (comment)

Christinadobrzyn · 2024-08-19T16:50:40Z

Awesome! Thanks @hoangzinh.

Regression test - https://github.com/Expensify/Expensify/issues/421516

This is payment day and there are no regressions so paying this out based on this payment summary

Closing!

m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Mar 18, 2024

melvin-bot bot assigned Christinadobrzyn Mar 18, 2024

Christinadobrzyn added this to [#whatsnext] #vip-vsb Mar 20, 2024

Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Mar 20, 2024

melvin-bot bot changed the title ~~No feedback and delay when uploading a large attachment~~ [$500] No feedback and delay when uploading a large attachment Mar 20, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 20, 2024

melvin-bot bot assigned hoangzinh Mar 20, 2024

BrtqKr mentioned this issue Jul 23, 2024

Fix image picker conversions #46025

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jul 23, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 9, 2024

melvin-bot bot changed the title ~~[$500] No feedback and delay when uploading a large attachment~~ [HOLD for payment 2024-08-19] [$500] No feedback and delay when uploading a large attachment Aug 12, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Aug 12, 2024

hoangzinh mentioned this issue Aug 15, 2024

Prevent large attachment #4269

Merged

5 tasks

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 18, 2024

Christinadobrzyn closed this as completed Aug 19, 2024

[HOLD for payment 2024-08-19] [$500] No feedback and delay when uploading a large attachment #38478

[HOLD for payment 2024-08-19] [$500] No feedback and delay when uploading a large attachment #38478

Comments

m-natarajan commented Mar 18, 2024 • edited by puneetlath Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Mar 18, 2024

ShridharGoel commented Mar 18, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

melvin-bot bot commented Mar 20, 2024

Christinadobrzyn commented Mar 20, 2024

melvin-bot bot commented Mar 20, 2024

ShridharGoel commented Mar 20, 2024

Proposal

ShridharGoel commented Mar 21, 2024

hoangzinh commented Mar 21, 2024

ShridharGoel commented Mar 21, 2024

hoangzinh commented Mar 22, 2024

ShridharGoel commented Mar 22, 2024

ShridharGoel commented Mar 22, 2024

Proposal

hoangzinh commented Mar 22, 2024 • edited Loading

ShridharGoel commented Mar 22, 2024

hoangzinh commented Mar 22, 2024

ShridharGoel commented Mar 22, 2024

situchan commented Mar 22, 2024

ShridharGoel commented Mar 22, 2024

situchan commented Mar 22, 2024

Christinadobrzyn commented Mar 25, 2024 • edited Loading

neil-marcellini commented Mar 25, 2024

BrtqKr commented Jul 22, 2024 • edited Loading

BrtqKr commented Jul 23, 2024 • edited Loading

Christinadobrzyn commented Jul 23, 2024

Christinadobrzyn commented Jul 30, 2024 • edited Loading

hoangzinh commented Jul 31, 2024

Christinadobrzyn commented Aug 6, 2024

hoangzinh commented Aug 7, 2024

melvin-bot bot commented Aug 12, 2024

melvin-bot bot commented Aug 12, 2024

melvin-bot bot commented Aug 12, 2024 • edited by Christinadobrzyn Loading

Christinadobrzyn commented Aug 13, 2024 • edited Loading

hoangzinh commented Aug 14, 2024

hoangzinh commented Aug 14, 2024 • edited Loading

Regression Test Proposal

hoangzinh commented Aug 14, 2024 • edited Loading

BugZero Checklist:

Christinadobrzyn commented Aug 19, 2024

m-natarajan commented Mar 18, 2024 •

edited by puneetlath

Loading

ShridharGoel commented Mar 18, 2024 •

edited

Loading

hoangzinh commented Mar 22, 2024 •

edited

Loading

Christinadobrzyn commented Mar 25, 2024 •

edited

Loading

BrtqKr commented Jul 22, 2024 •

edited

Loading

BrtqKr commented Jul 23, 2024 •

edited

Loading

Christinadobrzyn commented Jul 30, 2024 •

edited

Loading

melvin-bot bot commented Aug 12, 2024 •

edited by Christinadobrzyn

Loading

Christinadobrzyn commented Aug 13, 2024 •

edited

Loading

hoangzinh commented Aug 14, 2024 •

edited

Loading

hoangzinh commented Aug 14, 2024 •

edited

Loading