Support Calendar Access Levels (iOS17+) #1151

MilosKarakas · 2023-09-12T12:49:34Z

I don't know if you agree with my approach, I can change it if you have any suggestions.

Pre-launch Checklist

codecov · 2023-09-12T12:51:58Z

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (d8882f1) 100.00% compared to head (d6fd0e9) 71.42%.
Report is 24 commits behind head on main.

Files	Patch %	Lines
permission_handler/lib/permission_handler.dart	47.36%	10 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              main    #1151       +/-   ##
============================================
- Coverage   100.00%   71.42%   -28.58%     
============================================
  Files            1        1               
  Lines           17       35       +18     
============================================
+ Hits            17       25        +8     
- Misses           0       10       +10

Flag	Coverage Δ
unittests	`71.42% <47.36%> (-28.58%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

JeroenWeener

Thank you for your contribution! We are very happy to have you onboard and are impressed by your PR 😄

I have some remarks regarding the architecture. I would appreciate it if you could resolve these issues. If you do not have time/motivation please let us know so we can take over and make sure your works makes it in.

Instead of an add only permission, we would prefer a full access permission. This would change the permission approach such that add only is the default (by requesting Permission.calendar) and that for full access, you'd need Permission.calendarFullAccess.

Because there will be two separate permissions for calendar that users of the plugins can use, we do not need extra permission statuses such as 'write only'. Typically, we want to avoid adding more statuses, as this corrodes the experience of our users. In this case, a user that gets 'granted' on Permission.calendar knows that they have write-only access, so there is no need for additional statuses.

This PR requires changes to all platforms, so the new permission is implemented everywhere. In this case, the new permission on platforms other than iOS can just use the same logic as the current Permission.calendar, as there is no distinction on those platforms between full access and write-only access. There should be updates to the different platform packages (permission_handler_android, permission_handler_windows, permission_handler_web and permission_handler_linux). If it is more convenient, we can also make these changes.

JeroenWeener · 2023-09-26T10:07:29Z

permission_handler_platform_interface/lib/src/permissions.dart

+  /// Events can only be added. If you want to read them as well, use the
+  /// [calendar] permission instead.
+  /// iOS: Calendar (17+ read & write access level)
+  static const calendarAddOnly = Permission._(1);


We prefer adding new permissions at the end of the list and giving them a higher id, instead of shuffling ids around. While your approach is nicer to the maintainers eye, it is usually a source of bugs because we forget to sync the ids between dart and the platform code. Also, we'd need to update all the platform packages too.

JeroenWeener · 2023-09-26T10:11:50Z

permission_handler_platform_interface/pubspec.yaml

@@ -3,7 +3,7 @@ description: A common platform interface for the permission_handler plugin.
 homepage: https://github.com/baseflow/flutter-permission-handler/tree/master/permission_handler_platform_interface
 # NOTE: We strongly prefer non-breaking changes, even at the expense of a
 # less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
-version: 3.11.5
+version: 3.11.6


Suggested change

version: 3.11.6

version: 3.12.0

JeroenWeener · 2023-09-26T10:12:21Z

permission_handler_apple/pubspec.yaml

@@ -2,7 +2,7 @@ name: permission_handler_apple
 description: Permission plugin for Flutter. This plugin provides the iOS API to request and check permissions.
 repository: https://github.com/baseflow/flutter-permission-handler
 issue_tracker: https://github.com/Baseflow/flutter-permission-handler/issues
-version: 9.1.4
+version: 9.1.5


Suggested change

version: 9.1.5

version: 9.2.0

permission_handler/CHANGELOG.md

MilosKarakas · 2023-09-26T11:04:52Z

@JeroenWeener Thank you for kind words, I've been using your package for a long time so it made sense to give something back.

All of requested changes make sense. I have time this week to readjust the PR, but if I run into any issues while adjusting the packages for platforms other then iOS I'll reach out for help.

MilosKarakas · 2023-09-27T05:36:15Z

@JeroenWeener Does it make sense to mark this in the changelog as a breaking change since requesting calendar now degrades the permission to a write-only, while calendarFullAccess works as calendar used to?

JeroenWeener · 2023-09-27T07:06:24Z

@MilosKarakas That makes sense 👍

dawsonbristow20 · 2023-09-28T20:33:59Z

Need this update soon! We are pending app store release due to this issue😓

MilosKarakas · 2023-10-02T06:28:15Z

@dawsonbristow20 I had a lot of work during last week so I didn't manage to complete this PR, I'm working on this today.

MilosKarakas · 2023-10-02T11:44:39Z

@JeroenWeener Hi, I think I've adjusted everything as requested, you can review it again when you find time. I wasn't sure only about this part, can you give me an example of what you expect here:

There should be updates to the different platform packages (permission_handler_android, permission_handler_windows, permission_handler_web and permission_handler_linux). If it is more convenient, we can also make these changes.

parker-sherrill · 2023-10-02T14:36:18Z

@MilosKarakas Thank you for this!

MilosKarakas · 2023-10-02T15:55:41Z

@parker-sherrill Hey, no problem.. Just make sure to wait until it's reviewed and officially released on pub.dev before using it, I can't guarantee everything is 100% right before @JeroenWeener or someone other who's authorised confirms it.

parker-sherrill · 2023-10-06T14:20:01Z

Any update on review? @JeroenWeener

JeroenWeener · 2023-10-10T10:35:27Z

Apologies for the radio silence. We had an internal discussion about splitting read-only and full access calendar permissions. Since Android natively also supports this split, we want to deprecate the calendar permission and introduce two new permissions: calendarReadOnly and calendarFullAccess. I will put up a PR for the platform interface and the Android package later today or tomorrow. Then, this PR can be updated, adhering to the new platform implementation.

JeroenWeener · 2023-10-10T10:43:35Z

Supporting issues: #1181, #1182.

JeroenWeener · 2023-10-10T11:46:20Z

The platform interface has been updated (see #1181).

@MilosKarakas could you update your dependency to permission_handler_platform_interface to versions 3.12.0?

There are now three permissions to take into account; calendar, calendarReadOnly and calendarFullAccess. The calendar permission will be deprecated in the future. For now, it will function identical to calendarFullAccess.

This means that the changes in this PR are no longer breaking. If everything went well, you should be able to dismiss your changes to the platform interface. Regarding my earlier comments about updating other platforms, don't worry about it. We will take care of Android and the other platforms.

If you have any questions, feel free to hit me up.

MilosKarakas · 2023-10-11T07:01:42Z

@JeroenWeener Hi, I'll be done with changes by the end of the week, as soon as I catch some time from work.

I'll let you know if any questions arise.

# Conflicts: # permission_handler/CHANGELOG.md # permission_handler/README.md # permission_handler/pubspec.yaml # permission_handler_platform_interface/lib/src/permissions.dart # permission_handler_platform_interface/test/src/permissions_test.dart

TimHoogstrate · 2023-11-09T10:37:50Z

One note: I've tested the implementation and when I run the permission_handler_apple Example app on iOS 17.1. And I request calendarWriteOnly. I am unable to request calendarFullAccess. I think it should show something else. Let's see how we can fix this before merging. When I click it i get an error:

[VERBOSE-2:dart_vm_initializer.cc(41)] Unhandled Exception: PlatformException(ERROR_ALREADY_REQUESTING_PERMISSIONS, A request for permissions is already running, please wait for it to finish before doing another request (note that you can request multiple permissions at the same time)., null, null)
#0      StandardMethodCodec.decodeEnvelope (package:flutter/src/services/message_codecs.dart:652:7)
#1      MethodChannel._invokeMethod (package:flutter/src/services/platform_channel.dart:310:18)
<asynchronous suspension>
#2      MethodChannelPermissionHandler.requestPermissions (package:permission_handler_platform_interface/src/method_channel/method_channel_permission_handler.dart:79:9)
<asynchronous suspension>
#3      _PermissionState.requestPermission (package:permission_handler_apple_example/main.dart:144:20)
<asynchronous suspension>

Calendar full access does not seem to work. I'll take a look.

TimHoogstrate · 2023-11-13T14:22:01Z

It seems to work correctly, The apple example project was not updated by adding the full access macro's. After I added the Full access macro's it was working as a charm.

MilosKarakas · 2023-11-13T14:32:00Z

@JeroenWeener Thanks, I hope it gets merged.

@TimHoogstrate Thanks for taking a look, I'm glad it works fine.

…nd podfile

JeroenWeener · 2023-11-20T08:46:59Z

Update: last week we merged updates to the different platforms. I just updated this PR which will expose these platform changes.

@TimHoogstrate could you provide another review?

TimHoogstrate

LGTM but i added some comments to make it more in line with other parts of the documentation

permission_handler/README.md

Co-authored-by: TimHoogstrate <[email protected]>

…er-permission-handler into Baseflow#1108

JeroenWeener · 2023-11-21T13:09:10Z

@MilosKarakas we merged your PR just now. The new calendar permissions for iOS should be up and running starting with permission_handler version 11.1.0. Once again thank you for contributing!

MilosKarakas · 2023-11-21T14:23:39Z

@JeroenWeener You're welcome. Thank you and @TimHoogstrate for reviewing and improving the code I submitted, it been a bit back and forth but it's done now. I'm glad I contributed to this great package.

milosKarakas added 3 commits September 12, 2023 12:52

- Split iOS calendar permission into write only and full access

93aec99

- Fixes + adjust readme and bump versions

7df9bd1

- Adjust Changelog.md

3a7da16

JeroenWeener changed the title ~~#1108~~ Support Calendar Access Levels (iOS17+) Sep 25, 2023

JeroenWeener suggested changes Sep 26, 2023

View reviewed changes

milosKarakas added 2 commits October 2, 2023 13:39

- Support iOS 17+ calendar permissions

ff4bdbe

- Update README.md to reflect iOS 17+ calendar permissions

c0812a1

MilosKarakas and others added 4 commits October 2, 2023 13:46

Merge branch 'Baseflow:main' into Baseflow#1108

4929c7f

- Fix PermissionManager.m issue

f987ed0

Merge remote-tracking branch 'origin/Baseflow#1108' into Baseflow#1108

12fc78d

- Fix dependencies

3590e97

info-trone mentioned this pull request Oct 3, 2023

Calendar permission not showing builttoroam/device_calendar#507

Closed

MilosKarakas mentioned this pull request Oct 3, 2023

[Bug]: #1178

Closed

5 tasks

milosKarakas added 3 commits October 13, 2023 15:13

Merge branch 'main' into Baseflow#1108

bffc413

# Conflicts: # permission_handler/CHANGELOG.md # permission_handler/README.md # permission_handler/pubspec.yaml # permission_handler_platform_interface/lib/src/permissions.dart # permission_handler_platform_interface/test/src/permissions_test.dart

Baseflow#1108 Adjusted for calendarReadOnly and calendarFullAccess

df9c1e3

Baseflow#1108 Remove breaking change annotation from changelog

71d5411

TimHoogstrate and others added 3 commits November 13, 2023 15:33

Added NSCalendarsFullAccessUsageDescription to apple example .plist a…

f0e8b8e

…nd podfile

Fix deprecated calendar permission logic

7055e43

Update EventPermissionStrategy.m

471aadf

JeroenWeener requested a review from TimHoogstrate November 14, 2023 12:54

JeroenWeener mentioned this pull request Nov 14, 2023

Support iOS 17 calendar permissions (iOS) #1220

Merged

10 tasks

JeroenWeener added 2 commits November 15, 2023 09:53

Merge remote-tracking branch 'upstream/main' into Baseflow#1108

665de10

Make small adjustments

b9f680a

JeroenWeener mentioned this pull request Nov 15, 2023

Support iOS 17 calendar permissions (Windows) #1223

Merged

10 tasks

JeroenWeener added 3 commits November 20, 2023 08:58

Merge remote-tracking branch 'upstream/main' into Baseflow#1108

6de76f9

Make small touch-ups

57f9059

Updates Runner.xcodeproj files

0f06363

JeroenWeener self-requested a review November 20, 2023 08:45

TimHoogstrate approved these changes Nov 20, 2023

View reviewed changes

permission_handler/README.md Outdated Show resolved Hide resolved

permission_handler/README.md Outdated Show resolved Hide resolved

JeroenWeener and others added 2 commits November 20, 2023 14:23

Update permission_handler/README.md

d480a4d

Co-authored-by: TimHoogstrate <[email protected]>

Update permission_handler/README.md

88239ca

Co-authored-by: TimHoogstrate <[email protected]>

JeroenWeener approved these changes Nov 20, 2023

View reviewed changes

JeroenWeener and others added 6 commits November 20, 2023 14:48

Replace calendar with contacts in tests

6c31afb

Merge branch 'Baseflow#1108' of https://github.com/MilosKarakas/flutt…

25a88eb

…er-permission-handler into Baseflow#1108

Update permission_handler_apple.yaml

22ef639

Update permission_handler.yaml

274ecb0

Update permission_handler.yaml

7bfe8a5

Update permission_handler_apple.yaml

d6fd0e9

JeroenWeener merged commit e3c92e3 into Baseflow:main Nov 21, 2023

JeroenWeener mentioned this pull request Nov 21, 2023

[Bug]: The Calendar Permission Popup Menu Never Shows Up #1188

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Calendar Access Levels (iOS17+) #1151

Support Calendar Access Levels (iOS17+) #1151

MilosKarakas commented Sep 12, 2023 •

edited

Loading

codecov bot commented Sep 12, 2023 •

edited

Loading

JeroenWeener left a comment

JeroenWeener Sep 26, 2023

JeroenWeener Sep 26, 2023

JeroenWeener Sep 26, 2023

MilosKarakas commented Sep 26, 2023

MilosKarakas commented Sep 27, 2023

JeroenWeener commented Sep 27, 2023

dawsonbristow20 commented Sep 28, 2023 •

edited

Loading

MilosKarakas commented Oct 2, 2023

MilosKarakas commented Oct 2, 2023

parker-sherrill commented Oct 2, 2023

MilosKarakas commented Oct 2, 2023

parker-sherrill commented Oct 6, 2023

JeroenWeener commented Oct 10, 2023

JeroenWeener commented Oct 10, 2023

JeroenWeener commented Oct 10, 2023

MilosKarakas commented Oct 11, 2023

TimHoogstrate commented Nov 9, 2023 •

edited

Loading

TimHoogstrate commented Nov 13, 2023

MilosKarakas commented Nov 13, 2023

JeroenWeener commented Nov 20, 2023

TimHoogstrate left a comment

JeroenWeener commented Nov 21, 2023

MilosKarakas commented Nov 21, 2023

Support Calendar Access Levels (iOS17+) #1151

Support Calendar Access Levels (iOS17+) #1151

Conversation

MilosKarakas commented Sep 12, 2023 • edited Loading

Pre-launch Checklist

codecov bot commented Sep 12, 2023 • edited Loading

Codecov Report

JeroenWeener left a comment

Choose a reason for hiding this comment

JeroenWeener Sep 26, 2023

Choose a reason for hiding this comment

JeroenWeener Sep 26, 2023

Choose a reason for hiding this comment

JeroenWeener Sep 26, 2023

Choose a reason for hiding this comment

MilosKarakas commented Sep 26, 2023

MilosKarakas commented Sep 27, 2023

JeroenWeener commented Sep 27, 2023

dawsonbristow20 commented Sep 28, 2023 • edited Loading

MilosKarakas commented Oct 2, 2023

MilosKarakas commented Oct 2, 2023

parker-sherrill commented Oct 2, 2023

MilosKarakas commented Oct 2, 2023

parker-sherrill commented Oct 6, 2023

JeroenWeener commented Oct 10, 2023

JeroenWeener commented Oct 10, 2023

JeroenWeener commented Oct 10, 2023

MilosKarakas commented Oct 11, 2023

TimHoogstrate commented Nov 9, 2023 • edited Loading

TimHoogstrate commented Nov 13, 2023

MilosKarakas commented Nov 13, 2023

JeroenWeener commented Nov 20, 2023

TimHoogstrate left a comment

Choose a reason for hiding this comment

JeroenWeener commented Nov 21, 2023

MilosKarakas commented Nov 21, 2023

MilosKarakas commented Sep 12, 2023 •

edited

Loading

codecov bot commented Sep 12, 2023 •

edited

Loading

dawsonbristow20 commented Sep 28, 2023 •

edited

Loading

TimHoogstrate commented Nov 9, 2023 •

edited

Loading