[$500] iOS - Profile - Photo upload options are missing

lanitochka17 commented 11 months 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!

Version Number: 1.4.6.1 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 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

Navigate to https://staging.new.expensify.com/
Log in
Tap on your profile picture
Tap on your profile picture again
Tap on the "Upload photo" button

Expected Result:

"Browse" option should be added so I can choose a photo from iCloud drive or from the iPhone internal storage

Actual Result:

Profile photo upload options are missing. Currently I can take a photo or choose one from the gallery

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[x] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/9bde98b8-5fef-404a-99c1-ddfe09b8c12b

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01cb01e156c8ab43dd
Upwork Job ID: 1730337464884461568
Last Price Increase: 2023-12-21
Automatic offers:

dukenv0307 | Contributor | 28118868
DylanDylann | Contributor | 28128145

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

Krishna2323 commented 11 months ago

Proposal

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

iOS - Profile - Photo upload options are missing

What is the root cause of that problem?

We disable the chooseDocument option whenever the type passed to AttachmentPicker is an image.

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

I think we can remove the prop type={CONST.ATTACHMENT_PICKER_TYPE.IMAGE} passed to AttachmentPicker since we validate the file type on upload.

Result

https://github.com/Expensify/App/assets/85894871/a179c198-ecae-4ffd-9b87-0e2e9c4505a6

neonbhai commented 11 months ago

Proposal

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

iOS - Profile - Photo upload options are missing

What is the root cause of that problem?

This happens as we remove the browse option when we are choosing an image in [AttachmentPicker/index.native.js]: https://github.com/Expensify/App/blob/4fc20d3f93a3693f91021901a760923cc5d913af/src/components/AttachmentPicker/index.native.js#L184-L188

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

We should remove this check from the code. This allows the user to choose an image from a path they remember in the interal storage or iCloud drive.

Result

https://github.com/Expensify/App/assets/64629613/829b8b41-910d-49e5-af29-065f2e305a48

What alternative solutions did you explore? (Optional)

We can rename the option to Choose File as this better align the purpose of this option.

This involves adding a translation key here: https://github.com/Expensify/App/blob/4fc20d3f93a3693f91021901a760923cc5d913af/src/languages/en.ts#L287

        chooseFile: 'Choose file',

Result

https://github.com/Expensify/App/assets/64629613/52f60f0a-6d8e-420b-91e7-61f2716ca883

neonbhai commented 11 months ago

Proposal

Updated

Added detailed links to where the problem is. With Alternative step to rename the option. Screencast showing how alt step will look like.

dukenv0307 commented 11 months ago

Proposal

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

Profile photo upload options are missing. Currently I can take a photo or choose one from the gallery

What is the root cause of that problem?

In here, we allow the "Choose document" options only if the picker type is not CONST.ATTACHMENT_PICKER_TYPE.IMAGE. This is not correct because it's normal that the user can select images from file, just that we should restrict the type of files they can pick instead of allowing all by the RNDocumentPicker.types.allFiles option here.

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

Make this a getDocumentPickerOptions which will accept the picker type, if the picker type is IMAGE, we should only allow the type RNDocumentPicker.types.images, otherwise allow type RNDocumentPicker.types.allFiles.
In here, use getDocumentPickerOptions(type) to get the correct document picker options based on the picker type
Remove the condition type !== CONST.ATTACHMENT_PICKER_TYPE.IMAGE here since we should allow the Choose from file option, we already restrict the file type in the steps above
We can consider rename the copy of the button from "Choose document" to something like "Choose from files" since it "document" is a bit vague and can mislead the user.

What alternative solutions did you explore? (Optional)

NA

Santhosh-Sellavel commented 11 months ago

@alexpensify This is a improvement not a bug, Can you confirm this internally, whether this is something we should fix or not, thanks!

alexpensify commented 11 months ago

Thanks! I'll start a discussion on Monday.

alexpensify commented 11 months ago

I've asked here:

https://expensify.slack.com/archives/C01GTK53T8Q/p1701726386650909

alexpensify commented 11 months ago

I've bumped the 🧵 and there is feedback that it might be a dupe.

melvin-bot[bot] commented 11 months ago

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

alexpensify commented 11 months ago

I've asked for some more feedback in another Slack channel.

alexpensify commented 11 months ago

Still waiting for more feedback

alexpensify commented 11 months ago

No update, I've asked again for more feedback.

melvin-bot[bot] commented 11 months ago

@alexpensify @Santhosh-Sellavel 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!

melvin-bot[bot] commented 11 months ago

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

alexpensify commented 10 months ago

No update yet.

alexpensify commented 10 months ago

Based on the feedback so far, I need to identify if this fits into a wave. TBD.

melvin-bot[bot] commented 10 months ago

@alexpensify @Santhosh-Sellavel 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!

melvin-bot[bot] commented 10 months ago

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

alexpensify commented 10 months ago

I'm moving to Weekly. Due to other urgent GHs, I couldn't associate this one with a Wave before the holidays.

I will be offline from Friday, December 22, to Thursday, January 4, 2024. I will not be actively watching over this GitHub during that period. We can continue the review process in the new year-- thanks!

melvin-bot[bot] commented 10 months ago

@alexpensify @Santhosh-Sellavel this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

Decide whether any proposals currently meet our guidelines and can be approved as-is today
If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

melvin-bot[bot] commented 10 months ago

Current assignee @Santhosh-Sellavel is eligible for the Internal assigner, not assigning anyone new.

melvin-bot[bot] commented 10 months ago

@alexpensify, @Santhosh-Sellavel Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 10 months ago

@alexpensify, @Santhosh-Sellavel Still overdue 6 days?! Let's take care of this!

alexpensify commented 10 months ago

I'm back online again and will start working on the next steps.

melvin-bot[bot] commented 10 months ago

@alexpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

alexpensify commented 10 months ago

No update yet

alexpensify commented 10 months ago

No update, working on more urgent GHs and will review this one soon.

alexpensify commented 9 months ago

I should be updated to pitch this GH for a wave soon.

alexpensify commented 9 months ago

Pitched it to this VIP group:

https://expensify.slack.com/archives/C066HJM2CAZ/p1705518924933779

Waiting for confirmation on the next steps.

alexpensify commented 9 months ago

@Santhosh-Sellavel - alright, this one has been associated with a VIP. Can we review the proposals again to see if one will work? Thanks!

alexpensify commented 9 months ago

@Santhosh-Sellavel - any update here or let me know if you are unable to review this one? Thanks!

Santhosh-Sellavel commented 9 months ago

Sorry missed this one, I think we should have a proposal here already above, I'll update by tomorrow!

Santhosh-Sellavel commented 9 months ago

@dukenv0307's proposal looks good to me.

C+ Reviewed 🎀 👀 🎀

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

Current assignee @Santhosh-Sellavel is eligible for the External assigner, not assigning anyone new.

melvin-bot[bot] commented 9 months ago

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

Offer link 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 📖

alexpensify commented 9 months ago

Awesome, we are moving forward here.

Santhosh-Sellavel commented 9 months ago

@alexpensify PR is ready for review

I'm unavailable next week, so unassigning here please assign a new C+ here to move this forward

DylanDylann commented 9 months ago

I can review PR as C+

srikarparsi commented 9 months ago

Sounds good, assigning you to this and the PR @DylanDylann

DylanDylann commented 9 months ago

@srikarparsi Could you help to assign me to this issue again ?

melvin-bot[bot] commented 9 months ago

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

Offer link 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 📖

alexpensify commented 9 months ago

@dukenv0307 any update to address the PR feedback? Thanks!

alexpensify commented 8 months ago

PR is moving forward but looks like there is an inquiry that needs to be addressed.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Expensify / App