Add critical alerts notification permission v3

Simon-ent commented 1 year ago

I first of all tried adding a critical alerts flag at the top level however this introduces issues if both notifications and critical alerts are enabled by a user (depending on the order the user sets them they get different results). Therefore as critical alerts cannot be enabled without notifications I've added the critical alerts flag directly to the notifications permission setting.

Changes:

New flag added to notifications for critical alerts which enables both critical alerts and notifications at the same time
Included the flag in the README
Added the required entitlement to the sample/PermissionsTest

Testing:

Tests run on a local build of ASU as per steps in the contributing guide (setting launch arguments on scheme and running). To repeat the tests you will need to rebuild PermissionsTest with the new entitlement ensuring the iOS is greater than 12.0.

Critical Alerts Enabled

Arguments passed --booted --byType "iPhone 14 Pro" --bundle "com.wix.PermissionsTest" --setPermissions "notifications=critical" After running ASU check the settings show critical alerts and notifications enabled for PermissionsTest app.

Notifications enabled

Arguments passed --booted --byType "iPhone 14 Pro" --bundle "com.wix.PermissionsTest" --setPermissions "notifications=YES" After running ASU check the settings show notifications enabled for PermissionsTest app.

Notifications disabled

Arguments passed --booted --byType "iPhone 14 Pro" --bundle "com.wix.PermissionsTest" --setPermissions "notifications=NO" After running ASU check the settings show notifications disabled for PermissionsTest app.

asafkorem commented 1 year ago

Hey @Simon-ent, thanks for the contribution. I will review this shortly.

Simon-ent commented 1 year ago

Hey @asafkorem, thanks for the feedback, I've made the requested changes in the latest commit (and retested them). I also changed the criticalAlertSetting value for when isCriticalEnabled == False, from disabled (@1) to notSupported (@0) as I think this will help to avoid potential issues in the future if the critical alerts entitlement isn't added to other users apps. I did try and create the issue with the PermissionsTest app but couldn't get it to raise an error for it though.

asafkorem commented 1 year ago

Hey @asafkorem, thanks for the feedback, I've made the requested changes in the latest commit (and retested them). I also changed the criticalAlertSetting value for when isCriticalEnabled == False, from disabled (@1) to notSupported (@0) as I think this will help to avoid potential issues in the future if the critical alerts entitlement isn't added to other users apps. I did try and create the issue with the PermissionsTest app but couldn't get it to raise an error for it though.

SGTM :+1:

wix / AppleSimulatorUtils