Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2022-11-29] [$2000][Image] Attachments uploaded via camera lose their rotation, appears sideways. #11854

Closed
kavimuru opened this issue Oct 14, 2022 · 57 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review

Comments

@kavimuru
Copy link

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. Go to a DM
  2. Tap plus in chat
  3. Choose “add an attachment”
  4. Chose “take a photo”
  5. Take your photo, save

Expected Result:

Uploaded image should be in the same orientation

Actual Result:

All uploaded photos lose their orientation and are rotated 90 degrees to the left

Workaround:

unknown

Platform:

Where is this issue occurring?

  • iOS

Version Number: 1.2.15-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/195900439-cb259355-2f1b-45a5-bf87-3096a4b86252.MP4

Expensify/Expensify Issue URL:
Issue reported by: @JmillsExpensify
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1665704504033759

View all open jobs on GitHub

@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 14, 2022

Triggered auto assignment to @arielgreen (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 14, 2022
@kavimuru kavimuru changed the title Bug: Attachments uploaded via the camera lose their rotation, meaning that every upload photo via the camera appears sideways. Bug: Attachments uploaded via the camera lose their rotation, appears sideways. Oct 14, 2022
@kavimuru kavimuru changed the title Bug: Attachments uploaded via the camera lose their rotation, appears sideways. Bug: Attachments uploaded via camera lose their rotation, appears sideways. Oct 14, 2022
@JmillsExpensify
Copy link

@arielgreen I'm going to take this one from you if you don't mind! 😄

@JmillsExpensify
Copy link

Having trouble accessing Upwork at the moment, though I'll get to this one asap.

@melvin-bot melvin-bot bot removed the Overdue label Oct 17, 2022
@JmillsExpensify
Copy link

Actually I'm getting ahead of myself. I think this should be external but I'll quickly confirm with an Engineer to ensure that others agree.

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@JmillsExpensify
Copy link

@NikkiWines I imagine we can look for the orientation someone in the image metadata, so it's simply a matter of making sure we respect that orientation post upload. Do you agree?

@NikkiWines
Copy link
Contributor

I'd imagine it varies depending on what time of image but from looking at a couple of different types (.heic, .tiff, .jpg, screenshot) it looks like the orientation is occasionally provided:

.heic
image

.tiff
image

.jpeg
image

taking a screenshot of another image
image

Wouldn't be a fool proof method to identifying orientation but we could most likely check to see if the orientation is present and modify the image based on that. I think this is a fine issue for External

@NikkiWines NikkiWines removed their assignment Oct 17, 2022
@NikkiWines NikkiWines added NewFeature Something to build that is a new item. External Added to denote the issue can be worked on by a contributor and removed NewFeature Something to build that is a new item. labels Oct 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@melvin-bot melvin-bot bot changed the title Bug: Attachments uploaded via camera lose their rotation, appears sideways. [$250] Bug: Attachments uploaded via camera lose their rotation, appears sideways. Oct 17, 2022
@mountiny mountiny removed the Reviewing Has a PR in review label Nov 14, 2022
@mountiny
Copy link
Contributor

Sorry that must have been fat-fingering, still waiting for @hellohublot for the PR. Feel free to link a draft as well 🙌

@JmillsExpensify
Copy link

@JmillsExpensify - I can't seem to access this job on Upwork to view proposals/offers/hires etc. Can you take care of hiring @hellohublot & @sobitneupane, please?

Huh, not sure how to explain that. I've gone ahead and sent an offer to @sobitneupane since we've worked together before. @hellohublot to make sure we hire the right profile in Upwork, can you apply here: https://www.upwork.com/jobs/~01a08d6a4623670c83.

@sobitneupane
Copy link
Contributor

@sobitneupane do you have iPhone to test this?

@mountiny Yes, I have.

@mountiny
Copy link
Contributor

@hellohublot It has been two days now without linked PR, can you please link a draft PR to show the progress. Or update us on why you havent been able to start the PR yet. Thank you very much!

@hellohublot
Copy link
Contributor

@mountiny
Sorry to keep you waiting, these two days, I'm thinking about whether writing some unit tests, to avoid repeating problems like last time, or some potential problems of 4.9.0 or 4.10.0. I'm more cautious just like submitting a final, perfect proposal. Yes, I'd better do some basic testing first, I will submit a draft pull request tomorrow. Thank you!

@mountiny
Copy link
Contributor

@hellohublot I think unit tests are not required here unless this would be easy to achieve, not sure how one does unit tests about taking a picture.

We will add a regression test specifically for this so QA will always check the camera features work as expected.

@hellohublot
Copy link
Contributor

hellohublot commented Nov 16, 2022

Hi. I have created a PR: #12765, please help me to check, Thanks

@mountiny mountiny added the Reviewing Has a PR in review label Nov 16, 2022
@mountiny
Copy link
Contributor

PR is in a review.

@melvin-bot
Copy link

melvin-bot bot commented Nov 16, 2022

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:

@mountiny
Copy link
Contributor

The offending PR has been this #10297, which has bumped the version f the library with upstream fix for different issue but the solution has introduced this bug in the package.

@mountiny mountiny added the Awaiting Payment Auto-added when associated PR is deployed to production label Nov 22, 2022
@mountiny mountiny changed the title [$2000][Image] Attachments uploaded via camera lose their rotation, appears sideways. [HOLD for payment 2022-11-29] [$2000][Image] Attachments uploaded via camera lose their rotation, appears sideways. Nov 22, 2022
@trjExpensify
Copy link
Contributor

@hellohublot - quick bump on this please to apply to the job on Upwork so we're ready to issue payment.

@hellohublot
Copy link
Contributor

hellohublot commented Nov 23, 2022

@trjExpensify OK, I have submitted it on upwork, my username is also hellohublot, Thanks

@trjExpensify
Copy link
Contributor

I can't see that on this upwork job, are you sure you sent it on the right one?

image

@hellohublot
Copy link
Contributor

@trjExpensify
Copy link
Contributor

Thanks, but can you click this link and apply to the job? When I search for your profile, I can't find you.

@hellohublot
Copy link
Contributor

@trjExpensify imageBut I have already applied and cannot reapply

@hellohublot
Copy link
Contributor

hellohublot commented Nov 25, 2022

@trjExpensify Maybe you can try to search "hublot", if you still can't find me, maybe this job can only hire one person, please help to try to create another job, Thanks

@melvin-bot melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Nov 29, 2022
@trjExpensify
Copy link
Contributor

Okay, payment is due today. @hellohublot you've been hired for another job since, so I've been able to locate you via that. Not sure why your application for this one hasn't been received - but I've sent you an offer. I've also applied the 50% bonus for merging the PR within 3 days.

@sobitneupane - settled up for C+ 👍

@trjExpensify
Copy link
Contributor

As for the checklist:

  • The PR has been identified
  • I've commented on it
  • We don't have a regression test for uploading an attachment via Take Photo which would have caught this one far sooner than we did. Created an issue to add one here.

@trjExpensify
Copy link
Contributor

Cool, I've now settled up with @hellohublot. Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests