sparrowcode / PermissionsKit

Universal API for request permission and get its statuses.
https://x.com/sparrowcode_ios
MIT License
5.64k stars 462 forks source link

New iOS 17 requestFullAccessToEvents returns always notDetermined on first run #326

Closed ostatnicky closed 1 year ago

ostatnicky commented 1 year ago

New iOS 17 requestFullAccessToEvents returns always notDetermined on first run of an app.

@ivanvorobei @alexanderpuchta is it working to you?

We need a fix there https://github.com/sparrowcode/PermissionsKit/pull/325 but we need to have a return value in func request(completion: @escaping () -> Void) because requestFullAccessToEvents returns accessGranted = true but status or EKEventStore.authorizationStatus(for: EKEntityType.event) returns .notDetermined until you run your app again.

So this library still doesn't work well on iOS 17.

alexanderpuchta commented 1 year ago

but this sounds more like an issue in general with this calendar permissions, right? did you check if there is already a reported bug on apple side available?

ostatnicky commented 1 year ago

Does it matter if it’s Apple’s bug or not? The current flow doesn’t work and you’re responsible to react on it and provide a solution for it (and it exists and I told you it).

To be honest, I’m removing this pod from my project because it seems out-dated and you don’t care about it, you don’t test PRs. Someone told you in July that there are changes in iOS 17 but you didn’t even think about it. So it’s better to have my own solution than always debug why PermissionKit doesn’t work :/

alexanderpuchta commented 1 year ago

Not sure why you are so rude? This isn't even my package, I just provided a solution for iOS 17 which was recommended by apples migration guide. 😅

ostatnicky commented 1 year ago

Ah, you’re different guy :D Sorry both, I was very upset and spent whole day to make it work and expected just to update the pod…

ivanvorobei commented 1 year ago

@ostatnicky I see you upset with bug, but its open source. Its just happen.

ivanvorobei commented 1 year ago

PR merged, new release pushed.

alexanderpuchta commented 1 year ago

@ivanvorobei @ostatnicky to fix the issue with initial result is always .notDetermined we could add an @available(iOS 17.0, *) func request() async -> Status to class Permission.

this could look like this:


    @available(iOS 17.0, *)
    public override func request() async -> Status {

        let eventStore = EKEventStore()

        return await withCheckedContinuation { continuation in
            switch kind {
            case .calendar(let access):
                switch access {
                case .full:
                    eventStore.requestFullAccess { (isGranted: Bool, error: Error?) in
                        continuation.resume(returning: isGranted ? .authorized : .denied)
                    }

                case .write:
                    eventStore.requestWriteOnlyAccess { (isGranted: Bool, error: Error?) in
                        continuation.resume(returning: isGranted ? .authorized : .denied)
                    }
                }

            default:
                continuation.resume(returning: .notDetermined)
            }
        }
    }

this could then be used within apps like:

let status = await Permission.calendar(access: .full).request()

helper methods for EKEventStore can clean up code, so you don't have to write those requestFullAccess or requestWriteOnlyAccess again and again.

private extension EKEventStore {

    typealias EventStoreAction = ((isGranted: Bool, error: Error?)) -> Void

    func requestFullAccess(completion: @escaping EventStoreAction) {
        if #available(iOS 17.0, *) {
            self.requestFullAccessToEvents { (accessGranted: Bool, error: Error?) in
                DispatchQueue.main.async {
                    completion((isGranted: accessGranted, error: error))
                }
            }
        } else {
            self.requestAccess(to: EKEntityType.event) { (accessGranted: Bool, error: Error?) in
                DispatchQueue.main.async {
                    completion((isGranted: accessGranted, error: error))
                }
            }
        }
    }

    func requestWriteOnlyAccess(completion: @escaping EventStoreAction) {
        if #available(iOS 17.0, *) {
            self.requestWriteOnlyAccessToEvents { (accessGranted: Bool, error: Error?) in
                DispatchQueue.main.async {
                    completion((isGranted: accessGranted, error: error))
                }
            }
        }
    }
}

the default request(completion: @escpaing () ->Void)) could then be refactored like this:

    public override func request(completion: @escaping () -> Void) {

        let eventStore = EKEventStore()

        if #available(iOS 17.0, *) {
            switch kind {
            case .calendar(let access):
                switch access {
                case .full:
                    eventStore.requestFullAccess { (isGranted: Bool, error: Error?) in
                        completion()
                    }

                case .write:
                    eventStore.requestWriteOnlyAccess { (isGranted: Bool, error: Error?) in
                        completion()
                    }
                }

            default:
                completion()
            }
        } else {
            eventStore.requestFullAccess { (isGranted: Bool, error: Error?) in
                completion()
            }
        }
    }
alexanderpuchta commented 1 year ago

you can find the changes here: https://github.com/alexanderpuchta/PermissionsKit/commit/e0e9c617b47f5c4cfdc38b0fed22f641362c7de1

ostatnicky commented 1 year ago

Yes, it looks really promising! Thanks for the effort, Alex.

alexanderpuchta commented 1 year ago

yup, but this would mean an entire update of that framework. not sure if @ivanvorobei will do this at the moment

ivanvorobei commented 1 year ago

yup, but this would mean an entire update of that framework. not sure if @ivanvorobei will do this at the moment

I will check it today, I see it's important to achieve

ostatnicky commented 1 year ago

Alex's code works well, I've tested. Let's take a look on the response:

continuation.resume(returning: isGranted ? .authorized : .denied)

I'm not sure if we should return Permission.Status. Because the status could be really different. Sometimes the access is not granted just because it's not determined and not because of it was denied.

So I suggest to have something like this:

public override func request() async -> (isGranted: Bool, error: Error?)

Yeah, the solution is not straightforward... :D

alexanderpuchta commented 1 year ago

I wouldn't like that, to return a tuple.

I would recommend func request() async throw -> Status or change continuation.resume to

let status: Status = {
    if let error != nil {
        return .notDetermined
    }

    return isGranted ? .authorized : .denied
}()

continuation.resume(returning: status)
ivanvorobei commented 1 year ago

@ostatnicky thanks for you debug. Honestly I don't see clear your point with opensource and its look little negative. But anyway, we are community.

I did some research with it and for sure it's apple's bug. But good news - I have clean solution. I resecah and got result of state wrong only first time. I just added empty call which not return for state. Next call response with right value.

let _ = EKEventStore.authorizationStatus(for: EKEntityType.event)

I add this line and now its work well. New release preparing and hope its looks better than use async call. Anyway @alexanderpuchta thanks for your reseach.

If my solution not work let me know please.

ivanvorobei commented 1 year ago

Release 9.2.2 available.