Flutter-permission-handler: Make API more intuitive

[marcelgarus]

First of all: Thanks for this great package! 😊
It's widely adopted for good reasons and I want to congratulate you on that.

I understand how the API of this package got designed but Dart's introduction of extension methods makes it possible to design a much tighter, more readable API. Here are just some of my ideas:

Enums:
I think the PermissionStatus and ServiceStatus should be enums so that users can switch over the result. Values and other methods can still be defined using extension methods.

PermissionHandler:
I know this is what gives the package its name, but I'm not sure a PermissionHandler is needed at all. Currently, it mainly provides a namespace for all the functions of this package. But they could be defined just as well on the PermissionGroups themselves, which seems much more intuitive to me.
status could be a getter, because it doesn't perform an action that the developer or user perceives. Developers still know that some work is involved, because it returns a Future.
Also, most of the time you only want to request a single permission, so it makes sense to also provide a method for that.

current | proposed
--- | ---
await PermissionHandler() .checkPermissionStatus(PermissionGroup.camera) | await PermissionGroup.camera.status
(await PermissionHandler() .requestPermissions([PermissionGroup.camera]))[PermissionGroup.camera] | await PermissionGroup.camera.request()
PermissionHandler() .requestPermissions([PermissionGroup.camera, PermissionGroup.storage])) | await [PermissionGroup.camera, PermissionGroup.storage].request()

Intuitive getters:
Sure, users can always check someStatus == PermissionStatus.granted, but someStatus.isGranted seems more intuitive, allowing for (await PermissionGroup.camera.status).isGranted.
These getters could not only get implemented on PermissionStatus, but also on Future<PermissionStatus>, allowing even terser syntax: await PermissionGroup.camera.checkStatus().isGranted.
To make the API more declarative, maybe these getters could also get directly implemented on PermissionGroups as syntactic sugar for implicitly calling the check() function: await PermissionGroup.camera.isGranted

Support common patterns:
Very often, permissions are checked and if they are denied, a request is made. That could be supported using some requestIfDenied method, that returns the status if the status is not PermissionStatus.denied and otherwise requests the permission from the user and then returns the new status: PermissionGroup.camera.requestIfDenied()

PermissionGroups?
I understand that some of the permissions resemble multiple OS-level permissions, but I think that's an implementation detail that most developers using this API don't really care about.
Maybe, PermissionGroup could get renamed to Permission?

Putting it all together
Most of this was trivial syntactic sugar, but putting it all together, it makes for much more terse code.
What before looked like this:

var handler = PermissionHandler();
var permission = await handler.checkPermissionStatus(PermissionGroup.storage);
if (permission != PermissionStatus.granted) {
  var permissions = await handler.requestPermissions([PermissionGroup.storage]);
  if (permissions[PermissionGroup.storage] != PermissionStatus.granted) {
    // Do stuff.
  }
}

could now be as easy as this:

if (await Permission.storage.requestIfDenied().isGranted) {
  // Do stuff.
}

Of course, these are breaking changes to the api and require a major version update to 5.0.0. I do think though that the simplicity of that API would be worth it.
What do you think about these changes? If you decide on redesigning the API, I'm happy to help to implement the changes.

[mvanbeusekom]

Thank you for the kind words, those are really appreciated.

I really like your proposal and would be really interesting to have a go at implementing it. The only concern I have is with the openAppSettings method which is not really related to a permission group or status. I am sure however we will find a solution for it.

So if you are able to make a start, that would be awesome.

[marcelgarus]

Great 😊 I'll get started on a first prototype.
I have a question regarding the current architecture: There's a permission_handler and a permission_handler_platform_interface package. What's the reasoning behind that?

[mvanbeusekom]

That is what Flutter has called the “federated plugin” structure and is invented to make it easier to implement plug-ins for different platforms. You can find a detailed explanation in this article: https://medium.com/flutter/how-to-write-a-flutter-web-plugin-part-2-afdddb69ece6

This is something that we would like to stick to, since the Flutter team also see’s this now as the de facto standard for their plug-ins.

[mvanbeusekom]

Maybe also good to know, we are aiming to become a Flutter Favorite plugin. At the moment the only thing that is stopping us is that the API (the Dart side) should be covered by unit-tests. So in you refactoring it is still important that we should be able to cover the methods using unit-tests.

[marcelgarus]

Okay, so on my fork I left the federated architecture as-is. The enum file in the permission_handler_platform_interface got replaced by several files: permission_status.dart, service_status.dart and permissions.dart. The PermissionHandler in the other package got completely rewritten to a bunch of extension methods.

Now, there are status and shouldShowRequestRationale getters on permissions as well as a request() and a requestIfDenied() method. You can also ask for the permission status directly on the permission, so Permission.camera.isGranted works. You can also call request() on a list of permissions.

I also added a ServicePermission, which extends Permission and doesn't contain any new fields but indicates that a service is associated with that permission. ServicePermissions have an additional serviceStatus getter, which returns the status of the associated service.

Things that still need to be resolved

openAppSettings()

For now, openAppSettings() is a global method. To be honest, I'm okay with that because it's unlikely that it'll conflict with any user-defined methods.

Declare ServicePermissions

I'm not really well-versed with the native permission models and I'm really not sure which permissions have associated services, so, for now, I just made the location permissions to ServicePermissions. Someone with more experience with permissions should go through all permissions and change the right ones to ServicePermissions.

Impossible ServiceStatus.notApplicable

With the new API change, users are only able to call serviceStatus on ServicePermission, i.e. permissions, that have an associated service. The notApplicable status is therefore never used. I'm not sure how the native code interacts with these statuses, so I left the status there, but we can probably get rid of it.
Also, in which cases do PermissionStatus.unknown and ServiceStatus.unknown occur? Maybe we can get rid of them too.

Naming of getters

The naming of some getters on the permission and their statuses is a bit off. isGranted, isDenied and isRestricted work well, but isUnknown and isNeverAskAgain don't feel natural.
isUnknown could get renamed to isStatusUnknown, but isNeverAskAgain is tricky.
Maybe we could rename it to isNeverAskedAgain, implying that the OS will never ask the user for this permission even if you call request() in your code? Also it uses a different terminology than the rest of the code ("ask" instead of "request"), so maybe refactoring it to use a totally custom getter name like doNotRequestAgain or isPermanentlyDenied could work?

Adding of getters for service permissions

ServicePermissions have a serviceStatus, but maybe we can enable getters on them too? Thsi could make code easier: Permission.location.isServiceEnabled instead of Permission.location.serviceStatus.isEnabled

Code duplication

The getters and especially the doc comments are mostly just plain copies throughout all abstraction layers.
I'm okay with some code duplication in regards to getters, because they provide different semantics at these different abstraction layers.
But the comments should probably be adjusted to match the abstraction. For example, on the PermissionHandlerInterface, we don't need to explain use cases for the Permission.phone, because it's not relevant for developers looking at these methods (supporting a new platform).

[mvanbeusekom]

Really nice work so far, I went through the changes on your fork and it looks really good. Also I am really impressed by the clear and elaborate explanation you wrote in this issue, so compliments for that as well.

I do have a question regarding the request() and requestIfDenied() methods. Personally I do not really see any added value in adding two different methods. Could you explain a bit on why you created the requestIfDenied() method? I see that it only tries to request permissions when the current status is "denied", this however is currently also checked on the platform side when requesting permissions. Meaning when you currently request permissions the Android or iOS code will first check if permissions are already granted. If so it will simply return that permissions are granted. When permissions are not granted it will continue requesting the actual permissions. So basically both methods will now result in the same logic being run.

Here some of my feedback regarding the items in the "Things that still need to be resolved" section.

openAppSettings()

I agree with you that we can leave this a global method.

Declare ServicePermissions

This is for me also a trial and error situation, as in I haven't read all the documentation. But currently this is supported for the following permissions:

Android

• All location permissions (meaning: PermissionGroup.location, PermissionGroup.locationWhenInUse and PermissionGroup.locationAlways);
• Phone permissions (meaning: PermissionGroup.phone);
• Permissions to ignore battery optimizations (meaning: PermissionGroup.ignoreBatteryOptimizations).

iOS

• All location permissions (same as Android so: PermissionGroup.location, PermissionGroup.locationWhenInUse and PermissionGroup.locationAlways);
• Phone permissions (meaning: PermissionGroup.phone). This is a bit of an odd implementation since it indicates if the device is able to make phone calls (e.g. for iPad or iPhone without a sim-card it will return disabled meaning the device is not able to make a phone call);
• Permissions to access the Sensors (meaning: PermissionGroup.sensor);

Impossible ServiceStatus.notApplicable

As you can see in the "Declare ServicePermissions" section, it differs a bit per platform which permissions are backed by a separate service and have a service status behind them. So it might still be viable to keep this state around in case someone would request this on one of the service-permissions that is only supported on one of the platforms (e.g. calling ServicePermission.sensors.isServiceEnabled on Android).

Regarding the PermissionStatus.unknown, this only occurs on iOS when you check permissions before you have requested permissions for the first time. In this case iOS will return the unknown status. Meaning iOS makes an explicit distinction between:

1. Permissions have never been requested before (results in PermissionStatus.unknown);
2. Permissions have been granted (results in PermissionStatus.granted);
3. Permissions have been denied (results in PermissionStatus.denied);
4. Restricted permissions have been granted (results in PermissionStatus.restricted).

On Android the permission system defaults to denied. Meaning if you haven't requested permissions before consider permissions being denied.

The ServiceStatus.unknown is actually never explicitly used but is implemented as a default fallback in case of an exception while checking for the service status. This could happen on Android when we could not access a valid Activity instance (on iOS it is actually never used).

Naming of getters

I agree that the isUnknown and isNeverAskAgain are a bit off. For the unknown state I think isStatusUnknown, isUnknownState or isUnknownStatus would fit. For the "neverAskAgain" I personally like isPermantlyDenied especially since this is also closer to the way it is implemented. Since you are free to try and request permissions, Android however will not show the permission dialog when the end-user has selected the don't ask again option. This is important because if the user changes his mind and updates the permission settings on his device the App is not informed, so the App needs to actively keep on asking for permissions.

Adding getter for service permissions

I am not sure about this at the moment. I like that now it is explicitly separated since they are really two different things. Implementing it would also mean we might need to keep the extra notApplicable state around so developers get feedback if it is supported or not for that particular permission. Maybe for now we should leave this out and only implement it upon (high) request from other developers.

Code duplication

The way the code is structured now I agree with you that the duplication of the code is not really an issue (like you said "they provide different semantics at these different abstraction layers").

Regarding the comments, this is something we need to check. For the scoring on pub.dev it is important to have all public members covered with comments. If we don't we get a penalty and points will be deducted from the overall score. This is also a hard requirement to become a Flutter Favorite package. In other words when we run flutter analyze it should not report that there are public members that lack documentation. When I run flutter analyze on your repo at the moment I get the following output (the warning regarding the public member documentation is listed al the way at the bottom: "104 issues found. (ran in 4.0s; 2 public members lack documentation)").

warning • The include file package:effective_dart/analysis_options.1.2.0.yaml in
       /Users/maurits/Projects/oss/marcelgarus/permission-handler/permission_handler/analysis_options.yaml cannot be found •
       permission_handler/analysis_options.yaml:1:11 • include_file_not_found
warning • The include file package:effective_dart/analysis_options.1.2.0.yaml in
       /Users/maurits/Projects/oss/marcelgarus/permission-handler/permission_handler/analysis_options.yaml cannot be found •
       permission_handler/analysis_options.yaml:1:11 • include_file_not_found
  error • Target of URI doesn't exist: 'package:flutter/material.dart' • permission_handler/example/lib/main.dart:3:8 •
         uri_does_not_exist
  error • Target of URI doesn't exist: 'package:permission_handler/permission_handler.dart' •
         permission_handler/example/lib/main.dart:4:8 • uri_does_not_exist
  error • The function 'runApp' isn't defined • permission_handler/example/lib/main.dart:6:16 • undefined_function
  error • Classes can only extend other classes • permission_handler/example/lib/main.dart:10:21 • extends_non_class
  error • Undefined class 'StatelessWidget' • permission_handler/example/lib/main.dart:10:21 • undefined_class
  error • Undefined class 'Widget' • permission_handler/example/lib/main.dart:12:3 • undefined_class
   info • The method doesn't override an inherited method • permission_handler/example/lib/main.dart:12:10 •
          override_on_non_overriding_member
  error • Undefined class 'BuildContext' • permission_handler/example/lib/main.dart:12:16 • undefined_class
  error • The method 'MaterialApp' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:13:12 •
         undefined_method
  error • The method 'Scaffold' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:14:13 • undefined_method
  error • The method 'AppBar' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:15:17 • undefined_method
  error • Undefined class 'Text' • permission_handler/example/lib/main.dart:16:24 • undefined_class
  error • The name 'Widget' isn't a type so it can't be used as a type argument • permission_handler/example/lib/main.dart:17:21 •
         non_type_as_type_argument
  error • The method 'IconButton' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:18:13 •
         undefined_method
  error • Undefined class 'Icon' • permission_handler/example/lib/main.dart:19:27 • undefined_class
  error • Undefined name 'Icons' • permission_handler/example/lib/main.dart:19:32 • undefined_identifier
  error • The method 'openAppSettings' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:21:33 •
         undefined_method
  error • The method 'debugPrint' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:22:17 •
         undefined_method
  error • The method 'Center' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:27:15 • undefined_method
  error • The method 'ListView' isn't defined for the type 'MyApp' • permission_handler/example/lib/main.dart:28:18 • undefined_method
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:29:25 • undefined_identifier
  error • Undefined class 'Permission' • permission_handler/example/lib/main.dart:30:27 • undefined_class
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:32:44 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:33:41 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:34:41 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:35:41 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:36:41 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:38:44 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:39:41 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:40:41 • undefined_identifier
  error • Undefined name 'Permission' • permission_handler/example/lib/main.dart:41:41 • undefined_identifier
  error • Classes can only extend other classes • permission_handler/example/lib/main.dart:54:32 • extends_non_class
  error • Undefined class 'StatefulWidget' • permission_handler/example/lib/main.dart:54:32 • undefined_class
  error • Undefined class 'Permission' • permission_handler/example/lib/main.dart:58:9 • undefined_class
   info • The method doesn't override an inherited method • permission_handler/example/lib/main.dart:61:20 •
          override_on_non_overriding_member
  error • Classes can only extend other classes • permission_handler/example/lib/main.dart:64:32 • extends_non_class
  error • Undefined class 'State' • permission_handler/example/lib/main.dart:64:32 • undefined_class
  error • Undefined class 'Permission' • permission_handler/example/lib/main.dart:67:9 • undefined_class
  error • Undefined class 'PermissionStatus' • permission_handler/example/lib/main.dart:68:3 • undefined_class
  error • Undefined name 'PermissionStatus' • permission_handler/example/lib/main.dart:68:40 • undefined_identifier
   info • The method doesn't override an inherited method • permission_handler/example/lib/main.dart:71:8 •
          override_on_non_overriding_member
  error • The method 'initState' isn't defined in a superclass of '_PermissionState' • permission_handler/example/lib/main.dart:72:11
         • undefined_super_method
  error • The method 'setState' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:79:5 •
         undefined_method
  error • Undefined class 'Color' • permission_handler/example/lib/main.dart:82:3 • undefined_class
  error • Case expressions must be constant • permission_handler/example/lib/main.dart:84:12 • non_constant_case_expression
  error • Undefined name 'PermissionStatus' • permission_handler/example/lib/main.dart:84:12 • undefined_identifier
  error • Undefined name 'Colors' • permission_handler/example/lib/main.dart:85:16 • undefined_identifier
  error • Case expressions must be constant • permission_handler/example/lib/main.dart:86:12 • non_constant_case_expression
  error • Undefined name 'PermissionStatus' • permission_handler/example/lib/main.dart:86:12 • undefined_identifier
  error • Undefined name 'Colors' • permission_handler/example/lib/main.dart:87:16 • undefined_identifier
  error • Undefined name 'Colors' • permission_handler/example/lib/main.dart:89:16 • undefined_identifier
  error • Undefined class 'Widget' • permission_handler/example/lib/main.dart:94:3 • undefined_class
   info • The method doesn't override an inherited method • permission_handler/example/lib/main.dart:94:10 •
          override_on_non_overriding_member
  error • Undefined class 'BuildContext' • permission_handler/example/lib/main.dart:94:16 • undefined_class
  error • The method 'ListTile' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:95:12 •
         undefined_method
  error • The method 'Text' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:96:14 •
         undefined_method
  error • The method 'Text' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:97:17 •
         undefined_method
  error • The method 'TextStyle' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:99:16 •
         undefined_method
  error • The method 'IconButton' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:101:17 •
         undefined_method
  error • Undefined class 'Icon' • permission_handler/example/lib/main.dart:102:23 • undefined_class
  error • Undefined name 'Icons' • permission_handler/example/lib/main.dart:102:28 • undefined_identifier
  error • Undefined class 'BuildContext' • permission_handler/example/lib/main.dart:112:27 • undefined_class
  error • Undefined class 'Permission' • permission_handler/example/lib/main.dart:112:49 • undefined_class
  error • Undefined name 'Scaffold' • permission_handler/example/lib/main.dart:113:5 • undefined_identifier
  error • The method 'SnackBar' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:113:39 •
         undefined_method
  error • The method 'Text' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:114:16 •
         undefined_method
  error • Undefined class 'Permission' • permission_handler/example/lib/main.dart:118:34 • undefined_class
  error • The method 'setState' isn't defined for the type '_PermissionState' • permission_handler/example/lib/main.dart:121:5 •
         undefined_method
  error • Target of URI doesn't exist: 'package:permission_handler_platform_interface/permission_handler_platform_interface.dart' •
         permission_handler/lib/permission_handler.dart:3:8 • uri_does_not_exist
  error • Target of URI doesn't exist: 'package:permission_handler_platform_interface/permission_handler_platform_interface.dart' •
         permission_handler/lib/permission_handler.dart:5:8 • uri_does_not_exist
  error • Undefined class 'PermissionHandlerPlatform' • permission_handler/lib/permission_handler.dart:8:1 • undefined_class
  error • Undefined name 'PermissionHandlerPlatform' • permission_handler/lib/permission_handler.dart:8:43 • undefined_identifier
  error • Undefined class 'Permission' • permission_handler/lib/permission_handler.dart:16:32 • undefined_class
  error • The name 'PermissionStatus' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:18:10 • non_type_as_type_argument
  error • The name 'PermissionStatus' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:35:10 • non_type_as_type_argument
  error • The name 'PermissionStatus' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:42:10 • non_type_as_type_argument
  error • Undefined class 'Permission' • permission_handler/lib/permission_handler.dart:53:39 • undefined_class
  error • Undefined name 'status' • permission_handler/lib/permission_handler.dart:55:32 • undefined_identifier
  error • Undefined name 'status' • permission_handler/lib/permission_handler.dart:58:33 • undefined_identifier
  error • Undefined name 'status' • permission_handler/lib/permission_handler.dart:64:36 • undefined_identifier
  error • Undefined name 'status' • permission_handler/lib/permission_handler.dart:67:33 • undefined_identifier
  error • Undefined name 'status' • permission_handler/lib/permission_handler.dart:72:41 • undefined_identifier
  error • Undefined class 'ServicePermission' • permission_handler/lib/permission_handler.dart:76:39 • undefined_class
  error • The name 'ServiceStatus' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:97:10 • non_type_as_type_argument
  error • The name 'Permission' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:101:41 • non_type_as_type_argument
  error • The name 'Permission' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:106:14 • non_type_as_type_argument
  error • The name 'PermissionStatus' isn't a type so it can't be used as a type argument •
         permission_handler/lib/permission_handler.dart:106:26 • non_type_as_type_argument
  error • Target of URI doesn't exist: 'package:plugin_platform_interface/plugin_platform_interface.dart' •
         permission_handler_platform_interface/lib/permission_handler_platform_interface.dart:4:8 • uri_does_not_exist
  error • Target of URI doesn't exist: 'package:flutter/services.dart' •
         permission_handler_platform_interface/lib/src/method_channel/method_channel_permission_handler.dart:3:8 • uri_does_not_exist
  error • Undefined class 'MethodChannel' •
         permission_handler_platform_interface/lib/src/method_channel/method_channel_permission_handler.dart:8:7 • undefined_class
  error • Const variables must be initialized with a constant value •
         permission_handler_platform_interface/lib/src/method_channel/method_channel_permission_handler.dart:9:5 •
         const_initialized_with_non_constant_value
  error • The function 'MethodChannel' isn't defined •
         permission_handler_platform_interface/lib/src/method_channel/method_channel_permission_handler.dart:9:5 • undefined_function
  error • Classes can only extend other classes •
         permission_handler_platform_interface/lib/src/permission_handler_platform_interface.dart:11:50 • extends_non_class
  error • Undefined class 'PlatformInterface' •
         permission_handler_platform_interface/lib/src/permission_handler_platform_interface.dart:11:50 • undefined_class
  error • The named parameter 'token' isn't defined •
         permission_handler_platform_interface/lib/src/permission_handler_platform_interface.dart:13:39 • undefined_named_parameter
  error • Undefined name 'PlatformInterface' •
         permission_handler_platform_interface/lib/src/permission_handler_platform_interface.dart:27:5 • undefined_identifier
   info • Extension methods weren't supported until version 2.6.0, but this code is required to be able to run on earlier versions •
          permission_handler_platform_interface/lib/src/permission_status.dart:26:1 • sdk_version_extension_methods
   info • Extension methods weren't supported until version 2.6.0, but this code is required to be able to run on earlier versions •
          permission_handler_platform_interface/lib/src/permission_status.dart:55:1 • sdk_version_extension_methods
   info • Extension methods weren't supported until version 2.6.0, but this code is required to be able to run on earlier versions •
          permission_handler_platform_interface/lib/src/permission_status.dart:77:1 • sdk_version_extension_methods
   info • Extension methods weren't supported until version 2.6.0, but this code is required to be able to run on earlier versions •
          permission_handler_platform_interface/lib/src/service_status.dart:17:1 • sdk_version_extension_methods
   info • Extension methods weren't supported until version 2.6.0, but this code is required to be able to run on earlier versions •
          permission_handler_platform_interface/lib/src/service_status.dart:43:1 • sdk_version_extension_methods
   info • Extension methods weren't supported until version 2.6.0, but this code is required to be able to run on earlier versions •
          permission_handler_platform_interface/lib/src/service_status.dart:57:1 • sdk_version_extension_methods

104 issues found. (ran in 4.0s; 2 public members lack documentation)

[marcelgarus]

Thank you 😊
Oh, I didn't actually think about what request() does if the permission is already granted.
This package initially came to my attention because I'm currently also developing a new API for the flutter_downloader package, which, in its example, explicitly checks for the storage permission not being granted before requesting it. Other packages published on pub.dev that depend on permission_handler, like alice, flutter_screenshot_callback and contacts_plugin (also from baseflow) all rely on the same pattern of checking whether the permission is granted and only requesting it if it is not.
Apparently, the current API is a little bit confusing in that regard — perhaps because requesting a permission doesn't align with what developers typically perceive as a request (which is, showing the dialog).
Come to think of it, the current behavior is reasonable. I would make it explicitly clear in the doc comment of request() as well as in the documentation on pub.dev that request() does nothing if the permission is not granted tough. That should help avoid misuses of the library (or rather reimplementations of already offered functionality) like those above.

Regarding ServicePermissions and ServiceStatus.notApplicable:

Again, I underestimated the amount of different behavior between platforms. Of course, all permissions that could possibly have an associated service should be turned into ServicePermissions and the ServiceStatus.notApplicable is here to stay.

About PermissionStatus.unknown

To be honest, I'm not entirely sure about this one.
The only downside I can think of is that, on iOS, developers get less information about the status of the permission than they theoretically could get. But currently, Flutter developers cannot treat PermissionStatus.denied as if the user actively denied the permission anyway, because the app probably needs to work on Android too.
Removing the enum value would also make the API behave more consistent across platforms and make the API smaller and therefore simpler and easier to learn.
That's why I'm tending towards removing that enum value.

About ServiceStatus.unknown

Are there any production-ready Android apps where no Activity can get accessed? I'm not too experienced in native Android development, but my guess is that this is something that shouldn't happen during runtime and is, therefore, a programmer error. In Dart, Errors should not be caught, so I believe we also don't need a ServiceStatus.unknown.

Both of these unknown statuses use the null object pattern, where a valid enum value is returned to represent an invalid internal state. This is difficult to catch during runtime (and is btw the exact reason why non-nullability by default is coming to Dart) so I'd be very careful with designing an API around that. Perhaps, we can throw an error if no Activity was found?

Naming of getters

Of all the unknown-getters, I prefer the isStatusUnknown getter — whether we actually need to implement it depends on whether you agree with getting rid of the unknown value as proposed above.
isPermanentlyDenied is also my favorite 😄 It's succinct and the relationship to isDenied is clear. Of course, it's not technically permanently denied, because users can always go to the settings and change permissions, but I believe that's clear to developers, so no harm is done by choosing this.

serviceStatus

I'm okay with hiding the serviceStatus behind a getter — perhaps it would be too confusing to have getters checking for the status of the permissions and other getters checking for the status of the service on the same object.

Duplicated comments

You're right, I'm going to fix my code in that regard. I'll definitely need to have another look at the doc comments before merging. It's going to take some time though because I'm currently a bit busy with other stuff.
Most of the errors probably occur because you didn't run flutter pub get in the example folder, I believe? Also, I apparently still need to configure the environment for the permission_handler_platform_interface to only run on Dart 2.6.0 and above.

[mvanbeusekom]

I fully understand your time is limited, this is no different for me ;). So these things take time and that is perfectly fine. It would be nice though if you could already create a work in progress pull-request and we could maybe collaborate on the necessary changes. In my response to the topics I mention that I can make some changes to the Android code so we could eliminate the need of the unknown status, I think it would be helpful if I commit these changes on your fork and we work together to the final solution.

Anyway things are starting to look really nice. So just to sum up, I think we can come to the following conclusions:

1. We will only implement the request() method and add more detailed documentation and comments to make sure everybody knows it will internally also check permissions and only request permissions when permissions hasn't been granted yet.
2. For now we only implement checking service status through the ServicePermissions enum. Thinking about it, maybe we should rename this enum to just Services since checking if the service is enabled or not has nothing to do with "permissions", so the name ServicePermission doesn't make sense;
3. We will use the isStatusUnkown (if we need it, I will get to that later) and isPermantlyDenied getter names;
4. We agreed that the Duplicated comments are a necessity and we will add them where needed.

This leaves the following points open:

About PermissionStatus.unknown

I just wanted to bring up the possibility to rename the PermissionStatus.unknown to PermissionStatus.notDetermined (to match with iOS recommendations) and then we can also update the Android implementation to support this status. This means once someone requests permissions we will record this in the shared preferences storage (this actually already happens to support the PermissionStatus.neverAskAgain). This means we can check for this key, if it doesn't exists permissions have never been requested before so we could return the PermissionStatus.notDetermined. This actually is something that developers have been requesting, I however always kept them at bay because it is not mentioned in the Android documentation.

My personal preference would be the above compared to removing the PermissionStatus.unknown completely. By renaming unknown to notDetermined we more clearly describe our intent and this will also make it easier for us to name the getter because isNotDetermined sounds pretty good to me ;).

About ServiceStatus.unknown

I agree we can remove this. Currently the situation that there is not activity can occur for example when an App is running in the background (meaning the App started a background service) or when an App wants to know information about one of the services before an activity is instantiated. This is actually a small bug on our side, since we should not care about the context being an Activity for checking service status, any context would do (on Android possible contexts are Activity, Application or Service). This bug originates from the past were we only supported requesting permissions. To request permissions we need to show a dialog, which means we really need to run on an Activity context.

Since the refactoring of version 4.4.0 however we can easily solve this and would be good to do so. Meaning we solve two problems, developers are able to check service status on other context then only an Activity and we can remove the ServiceStatus.unknown from the enum. Hope this all makes sense, if not please let me know ;).

[marcelgarus]

I drafted a PR, so we can work together on the final solution.

I agree with almost all of the conclusions 😊 Regarding the ServicePermission, it's not an enum, but a special type of Permission (a permission with an associated service). So, maybe PermissionWithService would be a more appropriate name?

I didn't know the plugin already uses the shared preferences. I didn't know implementing PermissionStatus.notDetermined was possible on Android without a major rework. Of course, that working consistently across platforms would be the best option.

If removing ServiceStatus.unknown from the ServiceStatus enum is possible, that's great too. I'm looking forward to seeing this become a reality!

[marcelgarus]

Okay, so I implemented the changes:

• ServiceStatus.unknown is now gone.
• Comments everywhere have been updated to explicitly state that request only requests the permission if it hasn't already been granted.
• PermissionStatus.unknown got renamed to PermissionStatus.undetermined and all comments and getters got updated as well.
• PermissionStatus.notAskAgain got renamed to PermissionStatus.permanentlyDenied and all comments and getters got updated as well. Comments also state that the user can still manually change the status in the device's app settings.
• ServiceStatus.notApplicable now contains additional info that it only applies to the current platform.
• I rewrote the readme so it's easier to get started.

A question about shouldShowRationale: What exactly does this do and where is the rationale defined? The current readme contains the following code, which confused me:

bool isShown = await PermissionHandler().shouldShowRequestPermissionRationale(PermissionGroup.contacts);

So, shouldShowRequestPermissionRationale returns whether the rationale has been shown to the user?

[mvanbeusekom]

Apologies for the late reply, things got a bit hectic at work (even though we are forced to work from home, probably adding to the hectic).

The changes are starting to really look good. I have updated everything on the Android side:

• It will now correctly return the “undetermined” status the first time you check (but have not requested) the permission status (this means PermissionStatus.unknown is renamed to PermissionStatus.notDetermined, which doesn’t really effect the Dart side since the same int value is sent over);
• I have completely removed the ServiceStatus.unknown value;

I will get started on the iOS part removing the `ServiceStatus.unknown from its code base tomorrow (should be an easy enough change).

The shouldShowRequestPermissionRationale is an Android specific method that developer can call to check if the permission dialog will be shown when permissions are requested. It is Androids way to implement the “Never ask again” status. Problem is that it also returns false when permissions have not been requested before. Here is how Google explains it:

One approach you might use is to provide an explanation only if the user has already denied that permission request. Android provides a utility method, shouldShowRequestPermissionRationale(), that returns true if the user has previously denied the request, and returns false if a user has denied a permission and selected the Don't ask again option in the permission request dialog, or if a device policy prohibits the permission.

Thinking about it, now we have the `PermissionStatus.permanentlyDenied’ correctly implemented we have no need for this method anymore so we can remove it from the Dart code.

[mvanbeusekom]

I have also just pushed the necessary changes for iOS so that should now also work correctly (the example app works as intended).

[mvanbeusekom]

@marcelgarus do you have any open issues? I think we are ready to start releasing (maybe beta first) how do you feel about it?

[marcelgarus]

I don't have any issues anymore, so I think it's ready for a first release 😄

[mvanbeusekom]

@marcelgarus I have just released version 2.0.0 of the permission_handler_platform_interface and version 5.0.0 of the permission_handler plugins.

Thank you very much for this great contribution.

[marcelgarus]

Wonderful, I'm looking forward to using it 😊
Thanks for being so open to contributions

[mlemos]

@mvanbeusekom I'm having the following error when trying to use the example provided in the documentation:

My Code:

if (await Permission.location.request().isGranted) { ... }

Error:

The getter 'isGranted' isn't defined for the class 'Future<PermissionStatus>'.
Try importing the library that defines 'isGranted', correcting the name to the name of an existing getter, or defining a getter or field named 'isGranted'.

[marcelgarus]

Uh-oh 😲 Apparently, the permission_handler_interface package's extension methods don't get re-exported by the permission_handler package. That shouldn't have happened — I understand why Flutter Favorite has a hard requirement for Unit tests…

I just opened a PR to fix this problem 😇
In the meantime, you can work around this by also importing the permission_handler_interface package on the top of your file (you don't have to depend on it in your pubspec.yaml for this to work):

import 'package:permission_handler_interface/permission_handler_interface.dart';

[Zazo032]

Is there any migration guide users can follow to update their projects to 5.x?

[mvanbeusekom]

No there is not but the basics are as follows:

old way | new way
--- | ---
await PermissionHandler() .checkPermissionStatus(PermissionGroup.camera) | await PermissionGroup.camera.status
(await PermissionHandler() .requestPermissions([PermissionGroup.camera]))[PermissionGroup.camera] | await PermissionGroup.camera.request()
await PermissionHandler() .requestPermissions([PermissionGroup.camera, PermissionGroup.storage])) | await [PermissionGroup.camera, PermissionGroup.storage].request()
await PermissionHandler() .checkServiceStatus(PermissionGroup.location) | await Permission.location.serviceStatus.isEnabled

[petro-i]

What is the added value of this breaking change?

[Zazo032]

Less boilerplate, easier to use plugin

[marcelgarus]

@totalerex I initially started this PR just to make the API much easier to use, but we ended up releasing some other breaking changes with version 5.0.0, which reduce the complexity for users of this package:

• There's no ServiceStatus.unknown anymore, because on Android, this plugin now works with any Context, not just an Activity. The way I understand it, that means you can also request permissions when the app is running in the background or when it wants to know information about one of the services before an activity is instantiated.
• On Android, in the previous version, there was no distinction between a denied permission and permission that was never requested (because the OS doesn't have that functionality built-in). Now, Android also uses the undetermined status, so this works consistently across Android and iOS.

[awarmfastbear]

Hmm, is it just me or is the new API format and or docs not intuitive?

There is no 'PermissionGroup' documented here: https://pub.dev/documentation/permission_handler_platform_interface/latest/permission_handler_platform_interface/permission_handler_platform_interface-library.html

Trying to use PermissionGroup.camera.status in my package says PermissionGroup is undefined..

If I print:

print(Permission.microphone.status);

It just prints an instance of the future.

This works though:

      var microphonePermissionStatus = await Permission.microphone.status;
      print(microphonePermissionStatus);  // returns => PermissionStatus.granted

But that's not very usuable, this seems better:

      var microphonePermissionStatus = await Permission.microphone.status.isGranted;
      print(microphonePermissionStatus); // returns ==> true

Why not also include a readable generic function to check permission statuses that returns true or false?

e.g. checkPermission(microphone); which can return true or false.

If you're going to approve a new API format, provide docs for it? :)

[marcelgarus]

Thanks for your concern, I'm always happy to discuss about APIs.

Permissions statuses are more complicated than just true/false. A permission can be

• undetermined: We didn't ask the user yet.
• granted: The user granted the permission.
• denied: The user denied the permission.
• restricted: The operating system denied the permission, for example, because of parental controls on iOS.
• permanentlyDenied: The user permanently denied the permission, so we can't ask for granting it.

Developers should think about how to handle each of these cases.

Here are some reasons we went with the current API:

• No top-level functions are needed for checking a permission. That means the API is more explorable: Just by typing Permission., you can explore the API with autocomplete without needing to know about some magic top-level functions like checkPermission.
• The object relations match the intuitive understanding that a permission can be granted.
• It's more declarative.

That being said, it may be confusing that status returns a Future (because the permission is checked in the background). The readme has many examples right at the beginning of the "How to use" section, but maybe we should add another paragraph describing that status implicitly checks the permission. I think @mvanbeusekom would be glad to accept PRs in that regard.

[LinuCC]

We are just upgrading to the API of version 5, and I want to note that mocking this library has become significantly harder because of static constants in Permission, as well as its extension functions.
With version <=4 we could easily use a mocked PermissionHandler instance to mock return values, this can't be done anymore with mockito and Permission.
Another possibility we explored for mocking was setting the instance of PermissionHandlerPlatform in tests, but there is no setter exposed for that.

Until now, we were able skip writing a wrapper for this plugin because of that instance-based pattern; Now it seems we might need to write _more_ code in order to support this syntax style and still have tests for depending behaviour.

Do you know of an entry point to mock or set a mocked instance here?
I think adding a @visibleForTesting setter in the PermissionHandlerPlatform would already help us out here.

EDIT There _is_ a setter for the instance in PermissionHandlerPlatform, missed it.
With that we can mock most of the functionality we need.

[marcelgarus]

Interesting point. To be honest, I didn't think about testing too much, so that's a thing I'll keep in mind for future APIs.
Glad it still worked out though.

[josetiagobispo]

any idea how I can update using the new rule

static Future requestPermission(PermissionGroup permission) async {
final PermissionHandler _permissionHandler = PermissionHandler();
_permissionHandler.shouldShowRequestPermissionRationale(permission);
final result = await _permissionHandler.requestPermissions([permission]);
if (result[permission] == PermissionStatus.granted) {
return true;
}
return false;
}

[mvanbeusekom]

@josetiagobispo if you would like to reuse this code block you could write it as follows:

static Future requestPermission(Permission permission) async {
  final result = await permission.request();
  return result.isGranted;
}

Note that I removed the _permissionHandler.shouldShowRequestPermissionRationale(permission); since you don't use it's output meaning it doesn't do anything in your code. The shouldShowRequestPermissionRationale only returns a boolean value indicating (on Android only) if you should show an additional dialog to the user explaining why your App needs permissions. More information can be found here.