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

fix(mobile): check and request permissions when saving files to prevent hard crash #14919

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hurdofdaniel
Copy link

Description

Hi all,

This is my first PR so please be patient (I am also a web dev by trade, not a flutter dev so I am open to feedback on my code)

I noticed an issue when trying to save images when the permissions were "None" or "Add Photos Only"
The app would act normally until it had written them to the phone. It was still saving however the app would crash (close and do a complete re-open when clicking the icon)

My change implements an alert dialog if the user has denied permissions, to prompt for permissions, or if the permission is permanently denied, it will ask the user to provide more permissions in settings (The screenshot shows the permanently denied dialog)

Please let me know if you have any suggestions or changes

Thanks

How Has This Been Tested?

  • Set permissions to "None" and try saving a file, the alert dialog appears and no crashes
  • Set permissions to "Add Photos Only" and try saving a file, the alert dialog appears and no crashes
  • Set permissions to "Limited Access" and try saving a file, the file saves with the notifications and no crashes
  • Set permissions to "Full Access" and try saving a file, the file saves with the notifications and no crashes

Screenshots (if appropriate):

Screenshot 2024-12-25 at 5 03 38 PM Screenshot 2024-12-25 at 5 05 19 PM

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable

@hurdofdaniel
Copy link
Author

changelog:bugfix

@hurdofdaniel hurdofdaniel marked this pull request as ready for review December 25, 2024 05:20
@bo0tzz
Copy link
Member

bo0tzz commented Dec 25, 2024

The label is added by a maintainer. For future reference, no need to open a new PR if you have issues; a PR can pretty much always be fixed up to what you want it to be, even if you've completely mangled it :P

@hurdofdaniel
Copy link
Author

The label is added by a maintainer. For future reference, no need to open a new PR if you have issues; a PR can pretty much always be fixed up to what you want it to be, even if you've completely mangled it :P

Cheers, I'm not super familiar with GitHub, I use different Git Hosting providers at work

Always good to learn new things

Thanks

Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. A few feedback.

As we transition to the repository pattern, additional work will be needed.

We will need a permission.service.dart calls to permission.repository.dart, in which the library Permission API will be invoked.

The download.provider.dart will call the check permission in permission.service.dart; the return value will be used to either or not calling the showDialog, which belongs to the UI's logic layer.

Let me know if you want to take on this, otherwise, I am working on some permission related tasks and can add this in as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants