[HOLD for payment 2023-02-23] [$1000] Allow avatar image uploads: bmp on iOS

[Beamanator]

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. Upload .bmp image as avatar on iOS

Expected Result:

The image should upload no problem, and is converted to jpg file type

Actual Result:

iOS throws errors when uploading BMP

Note: Some investigation was done #12549 (comment), indicating this may be a problem with a library we use, RNImageManipulator

Workaround:

Yes, just can't upload this file type on iOS devices

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: Latest
Reproducible in staging?: Y
Reproducible in production?: Y

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d9fdd76b47328f10
• Upwork Job ID: 1623995536797614080
• Last Price Increase: 2023-02-10

[MelvinBot]

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

[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]

Job added to Upwork: https://www.upwork.com/jobs/~01d9fdd76b47328f10

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[Beamanator]

@Santhosh-Sellavel there's already some proposals in this issue: #14751

I created this one to track the iOS fix, while the old issue used to track both but will now only track the Android fix

[s77rt]

Proposal

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

Uploading bmp images on iOS fails

What is the root cause of that problem?

This is a bug in react-native-image-picker where it fails to detect .bmp files as images.

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

Update react-native-image-picker once this PR gets merged.

What alternative solutions did you explore? (Optional)

None

[Santhosh-Sellavel]

@s77rt Proposal sound's good to me, let's wait and hear from the maintainer

cc: @Beamanator

[Santhosh-Sellavel]

Bumped the maintainer, and they responded hopefully we would get reviewed this weekend, else let's follow up next week!

[trjExpensify]

Awaiting a review of the PR upstream. Let's reach back out in a couple of days.

[s77rt]

The upstream PR has been merged 🎉

[MelvinBot]

📣 @s77rt You have been assigned to this job by @Beamanator!
Please apply to this job in Upwork 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 📖

[Beamanator]

Wow that's great the maintainers are quick & have already merged the fix!! 🚀

[s77rt]

Applied in Upwork.
PR is ready.

[MelvinBot]

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

[MelvinBot]

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

• Fix: Detect .bmp files as images on iOS #15096

If no regressions arise, payment will be issued on 2023-02-23. 🎊

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.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

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:

• [@Santhosh-Sellavel / @Beamanator] The PR that introduced the bug has been identified. Link to the PR:
• [@Santhosh-Sellavel / @Beamanator] 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:
• [@Santhosh-Sellavel / @Beamanator] 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:
• [@s77rt] Propose regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

Regression Test Proposal

1. Login to any account
2. Go to Settings -> Profile
3. Upload a .bmp image as avatar
4. Verify that the image is uploaded with success

[trjExpensify]

Okay, we're ready for payment here. Sent the following offers:

• $1,500 @s77rt for the fix incl. efficiency bonus
• $1,500 @Santhosh-Sellavel for the C+ review incl. efficiency bonus

[trjExpensify]

[@Santhosh-Sellavel / @Beamanator] The PR that introduced the bug has been identified. Link to the PR:
[@Santhosh-Sellavel / @Beamanator] 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:
[@Santhosh-Sellavel / @Beamanator] 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:

Any thoughts on these @Santhosh-Sellavel @Beamanator? This was a bug upstream, right?

[trjExpensify]

As for the regression test.. I'm not convinced we need to test BMP on iOS specifically really on every regression run. It's primarily a windows based format, isn't it? Given that, I'm voting we don't proceed with adding it to the test suite. Let me know what you think!

[Santhosh-Sellavel]

Yeah, it was an upstream bug. we can check the first 3 off as N/A. Let me know your thoughts @Beamanator

[trjExpensify]

Dope. Settled up with everyone!

[trjExpensify]

@Beamanator if you happen to disagree that a regression test for BMP on iOS is not needed, feel free to reopen.