[$250] Settings - Take photo option is missing for uploading a profile avatar image

lanitochka17 commented 3 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: 9.0.19-10 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4852296&group_by=cases:section_id&group_id=229064&group_order=asc Issue reported by: Applause - Internal Team

Action Performed:

Navigate to https://staging.new.expensify.com/
Log in with a new Gmail account
Tap on "Account settings"
Tap on the avatar

Expected Result:

I should be able to take a photo for the avatar

Actual Result:

Take photo option is missing for uploading a profile avatar image. The gallery opens Samsung Galaxy A52s 5G

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/8cf8dfd9-aea1-49f7-b38e-0466fe3b3286

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011cd8972e087510c8
Upwork Job ID: 1826240108874477245
Last Price Increase: 2024-08-28

Issue Owner

Current Issue Owner: @mollfpr

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 3 months ago

@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 commented 3 months ago

We think that this bug might be related to #vip-vsp

daledah commented 2 months ago

Proposal

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

Take photo option is missing for uploading a profile avatar image. The gallery opens

What is the root cause of that problem?

In AvatarWithImagePicker component, we are using AttachmentPicker with type is CONST.ATTACHMENT_PICKER_TYPE.IMAGE: https://github.com/Expensify/App/blob/f2dad3a524b408f45dd74ea2c49b2c70015b2f38/src/components/AvatarWithImagePicker.tsx#L338
With this, we use use input html tag with accept prop is image/*: https://github.com/Expensify/App/blob/f2dad3a524b408f45dd74ea2c49b2c70015b2f38/src/components/AttachmentPicker/index.tsx#L14
And in android chrome, using input with accept: "image/*" will not show "Take photo" option.

What changes do you think we should make in order to solve the problem?
In general, we should use the input element without the accept attribute on Android Chrome to ensure that the "Take photo" option is available. However, this approach can lead to the "Choose a file" option displaying unexpected file types. To address this, we need to implement a mechanism that throws an error if the user selects a file type that is not expected. Detail below.

Return undefined if it is android chrome: https://github.com/Expensify/App/blob/f2dad3a524b408f45dd74ea2c49b2c70015b2f38/src/components/AttachmentPicker/index.tsx#L10
```
if (type !== CONST.ATTACHMENT_PICKER_TYPE.IMAGE || Browser.isMobileChrome()) {
```

Adding a validation logic to: https://github.com/Expensify/App/blob/f2dad3a524b408f45dd74ea2c49b2c70015b2f38/src/components/AttachmentPicker/index.tsx#L40

                // Check if the selected file is an image
                if (type === CONST.ATTACHMENT_PICKER_TYPE.IMAGE && file && !file.type.startsWith('image/')) {
                    // Show an alert if the file is not an image
                    alert('Please select an image file.');
                    // Reset the file input value
                    if (fileInput.current) {
                        fileInput.current.value = '';
                    }
                    return;
                }

What alternative solutions did you explore? (Optional)

In validation logic, when throw error, we can display a modal instead of alert.

melvin-bot[bot] commented 2 months ago

@JmillsExpensify Huh... This is 4 days overdue. Who can take care of this?

JmillsExpensify commented 2 months ago

Will open up the issue for proposals since this is possible on native mobile

melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~011cd8972e087510c8

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

@JmillsExpensify, @mollfpr Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 2 months ago

@JmillsExpensify @mollfpr 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!

JmillsExpensify commented 2 months ago

Still open for proposals. I'll keep it open though I don't think it's worth a higher price so keeping that the same.

melvin-bot[bot] commented 2 months ago

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

mollfpr commented 2 months ago

I can reproduce the issue with a physical device, not with the emulator.

The RCA from @daledah seems correct and it's a known bug that was reported https://issuetracker.google.com/issues/317289301.

@daledah Is the step 2 in your solution is a must? I think we already have the validation with the file type for avatar.

daledah commented 2 months ago

Is the step 2 in your solution is a must? I think we already have the validation with the file type for avatar

I believe we need to keep it. Without it, an error message appears below the avatar field whenever an incorrect file is selected.

mollfpr commented 2 months ago

I believe we need to keep it. Without it, an error message appears below the avatar field whenever an incorrect file is selected.

@daledah I think it's okay for now, as long the incorrect file didn't get uploaded.

Let's go with @daledah proposal without validation.

🎀 👀 🎀 C+ reviewed!

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

📣 @daledah You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

daledah commented 2 months ago

@mollfpr PR is ready.

melvin-bot[bot] commented 1 month ago

This issue has not been updated in over 15 days. @JmillsExpensify, @puneetlath, @mollfpr, @daledah eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

puneetlath commented 1 month ago

@mollfpr @daledah what's the status of this? Is it done?

mollfpr commented 1 month ago

It seems the automation has failed to update the status. The PR is deployed to the production on 6th September, and there's no deploy blocker linked to the issue. So it's ready for the payment.

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

No offending PR

[@mollfpr] 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:

The regression step should be good!

[@mollfpr] Determine if we should create a regression test for this bug. [@mollfpr] 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.

Login to the App
Navigate to "Account settings"
Tap on the avatar
Verify that: Take photo option is shown
👍 or 👎

puneetlath commented 1 month ago

Payment summary:

C - $250 - @daledah - to be paid via Upwork
C+ - 250 - @mollfpr - to be paid via NewDot

@daledah I don't see a contract for you for this issue. Can you link me your Upwork profile?

garrettmknight commented 1 month ago

$250 approved for @mollfpr

daledah commented 1 month ago

Can you link me your Upwork profile?

@puneetlath Here's mine https://www.upwork.com/freelancers/~0138d999529f34d33f

puneetlath commented 1 month ago

@daledah offer sent here: https://www.upwork.com/nx/wm/offer/104289314

Please ping me on this issue when you've accepted.

puneetlath commented 1 month ago

@daledah bump!

daledah commented 1 month ago

@puneetlath Sorry for not letting you know here, I accepted it

puneetlath commented 1 month ago

All paid. Thanks y'all!

Expensify / App