Skip to content
This repository has been archived by the owner on Feb 27, 2019. It is now read-only.

Add Health permission #17

Closed
bre7 opened this issue May 2, 2015 · 8 comments
Closed

Add Health permission #17

bre7 opened this issue May 2, 2015 · 8 comments
Milestone

Comments

@bre7
Copy link
Collaborator

bre7 commented May 2, 2015

Extra optional parameter (Health category type) needed in PermissionConfig, similar to what happened with Notifications. Using associated values would be better

In progress: PermissionScope-wip/healthkit

@bre7 bre7 changed the title Add HealthKit Add Health permission May 2, 2015
@nickoneill nickoneill mentioned this issue Aug 21, 2015
6 tasks
@bre7 bre7 added this to the 1.0 milestone Aug 21, 2015
@bre7
Copy link
Collaborator Author

bre7 commented Aug 26, 2015

@nickoneill: HK permission working (Don't use the example since you need to add the entitlement and link it to your account) on my last commit to the wip/health-cloud-kit branch. There's something we need to consider first:

  • What to do when the user asks for x types to share and y types to read ? Return .Authorized if all of them were ? What about SharingDenied and NotDetermined ? Link to the code

Also, that branch has a prototype of #71 by using PermissionConfig as a protocol (example here). I still think that we should improve decoupling. PermissionScope.swift shouldn't contain the request/success methods imo. A protocol based approach would be better (We still can't use structs because of ObjC...Unless someone wants to maintain a separate branch for ObjC compatibility)

And one other thing, if the user taps "Don't Allow" on the HealthKit panel (not the system alert per se but the system modal vc to enable/disable each requested permission type) then it won't appear in the App's settings, by design (maybe a bug..There are radars about it). The user will have to go to Settings > Privacy > Health > App_Name to enable/disable each object, which we can't deeplink.

@bre7 bre7 mentioned this issue Aug 26, 2015
4 tasks
@nickoneill
Copy link
Owner

I really like the look of the PermissionConfig stuff, feels much nicer. Let's get this stuff in before we play with more decoupling, I want to evaluate those changes separately.

For the HealthKit stuff... I'm OK with sending users to the same App Settings page for the moment, even if HealthKit isn't listed there. It's better than the alternative of just saying "HealthKit needs to be turned on, go here and here and here" in an alert. Can you link to any radars about this? It would be nice for each of us to file duplicates so they know we'd like this feature.

@nickoneill
Copy link
Owner

Can you be more clear on your first issue? I'm not certain I understand what you're saying.

@bre7
Copy link
Collaborator Author

bre7 commented Aug 28, 2015

This one ?

What to do when the user asks for x types to share and y types to read ? Return .Authorized if all of them were ? What about SharingDenied and NotDetermined ? Link to the code

Well, when requesting HK access one needs to specify 2 sets: elements that the app will share (aka write) and read. So, when we want to return HK's status what should we do ?

let typesAuthorized = statusArray
    .filter { $0 == .SharingAuthorized }
let typesDenied = statusArray
    .filter { $0 == .SharingDenied }
let typesNotDetermined = statusArray
    .filter { $0 == .NotDetermined }

if typesNotDetermined.count == statusArray.count || statusArray.isEmpty {
    return .Unknown
} else if !typesDenied.isEmpty {
    return .Unauthorized
} else {
    return .Authorized
}

As the code shows, at the moment Authorized is only returned when all of them are.

@nickoneill
Copy link
Owner

Ah, I see. Let me play with the API a bit.

@bre7
Copy link
Collaborator Author

bre7 commented Aug 28, 2015

I guess one solution could be to add an extra flag to HealthPermissionConfig called strictMode. If strict is enabled, then Authorized would only be returned when all of them are. Otherwise, if at least one has SharingAuthorized, then return Authorized

Code:

public func statusHealthKit(...) -> PermissionStatus {
        guard HKHealthStore.isHealthDataAvailable() else { return .Disabled }

        var statusArray:[HKAuthorizationStatus] = []
        typesToShare?.forEach {
            statusArray.append(HKHealthStore().authorizationStatusForType($0))
        }
        typesToRead?.forEach {
            statusArray.append(HKHealthStore().authorizationStatusForType($0))
        }

        let typesNotDetermined = statusArray
            .filter { $0 == .NotDetermined }

        if typesNotDetermined.count == statusArray.count || statusArray.isEmpty {
            return .Unknown
        }

        let typesAuthorized = statusArray
            .first { $0 == .SharingAuthorized }
        let typesDenied = statusArray
            .first { $0 == .SharingDenied }

        if strict {
            if let _ = typesDenied {
                return .Unauthorized
            } else {
                return .Authorized
            }
        } else {
            if let _ = typesAuthorized {
                return .Authorized
            } else {
                return .Unauthorized
            }
        }
    }

@nickoneill
Copy link
Owner

Ugh, such granularity.

Your approach might be a good first shot. Some alternatives I was thinking about are:
• breaking the health items up by read / share (one config, shows as two items)
• breaking the health items up into individual data points to be requested (needs some other design for the buttons, they would still all be requested at once)
• providing HealthKit as a convenience API only, not allowing it to be requested in the dialog with other items (it already provides a granular interface, little need to do the same thing again)

@bre7
Copy link
Collaborator Author

bre7 commented Aug 28, 2015

I'll commit strictMode for now. Then we'll think about improvements

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

No branches or pull requests

2 participants