Disambiguate userInfoItemTypes and signUpPermissionTypes

[ghost]

Overview

Since APCOnboarding Manager now owns an APCPermissionsManager with a public properties of signUpPermissionTypes and userInfoItemTypes, it is no longer necessary for the Onboarding Manager to have a userProfileElements property.

Actions

• Remove references to userProfileElements from APCOnboardingManager and APCAppDelegate
• Remove userProfileElements from APCOnboardingManager initializers.
• Disambiguate userInfoItemTypes/userProfileElements and signUpPermissionTypes/requiredServiceTypes which refer to enums in APCConstants (APCUserInfoItemType and APCSignUpPermissionsType).
• Initialize APCPermissionsManager with signUpPermissionTypes.
• Ensure any methods enumerating profile types are of the correct type.

[ghost]

@p2, @YuanZhu-apple Please take a look

[p2]

I definitely agree that userProfileElements should be gone from app delegate, but I don't see where the permission manager's new userInfoItemTypes are actually used? These are the elements asked for during signup, so I'm not quite sure if the permissions manager should handle those. If the onboarding manager manages everything related to onboarding, i.e. the scenes to show, handling consent actions and defining which info to collect, it would be the right place.

The permissions manager could stay separated from onboarding, only handling the permissions it's told to handle. For example APCSettingsViewController does allocate a fresh permissions manager, it doesn't need to know what the signup-elements are. The permissions manager used during onboarding can then be configured accordingly by the onboarding manager, without creating a property specifically for onboarding. APCSignUpMedicalInfoViewController and APCSignUpPermissionsViewController then use the onboarding-configured permissions controller to ask for the right permissions.

Does this make sense?

[ghost]

@p2 permissionsManager.userInfoItemTypes being used here

[p2]

@chrisinsfo What's the difference between userInfoItemTypes and signUpPermissionTypes?

[ghost]

@p2 APCSignUpPermissionsTypeS are in defined in APCConstants.h and determine whether we ask the user for permission to access the camera, microphone, photo library etc. APCUserInfoItemTypeS are defined in APCUserInfoConstants.h and allow us to show an appropriate user interface in APCProfileViewController for each type. The APCUserInfoItemTypeS are an amalgamation of quantity types, characteristic types and other items.

[p2]

@chrisinsfo In the spots you highlighted, userInfoItemTypes is only used to determine what is to be shown, it's not used by the permissions manager itself, meaning the manager is just a vehicle for a configuration. I'd think the permissions manager should stay isolated from configuration and just handle permissions. This way you configure it to reflect the permissions you want – usually during onboarding, but also in settings – and it handles those permissions for you. This also allows you to instantiate a fresh permissions manager from within the onboarding manager, with onboarding manager setting it up properly, so you don't need to ask the app delegate for that information.

To specify what is to be asked during signup and what is to be shown in the profile view controller should be handled elsewhere IMHO. For the former the onboarding manager seems the best fit, for the latter this fact can be exploited or a new config vehicle could be added.

[ghost]

@p2 My understanding of your intent of adding an OnboardingManager was to handle which scenes are shown during on-boarding. This has a logical dependency on the permissionsManager.

APCPermissionsManager is effectively a global variable holding a representation of the configuration of permissions, returning the state of permissions, and requesting those permissions when necessary. While userInfoItemTypes behave like permissions, you are right to question whether they are permissions or configurations, and whether they belong on the Permissions Manager, .

Since userInfoItemTypes are used elsewhere in the app, I don't believe they belong on the OnboardingManager either, but because they are not expected to change throughout the lifetime of the app, and are mostly derived from permissions, this seems the best home for them until we decide how to handle differently. Since the userInfoItemTypes property is public on PermissionsManager, it can be set to different types as needed through the life cycle of the app.

I like your config vehicle idea. Do you have bandwidth to implement after this PR?

[p2]

@chrisinsfo Yes, I agree with that assessment.

My intention for the OnboardingManager was to be able to subclass and specify everything related to onboarding/signup. Then one can launch onboarding and the classes and view controllers would do the right thing. This is showing the scenes but also to configure what info to collect from the user (where the latter informs the former). As you say this also informs the permissions manager.

I saw that the PermissionsManager has facilities to query for permissions from different core frameworks. So, for me it is a class that can be used standalone by configuring which permissions you'd like and then you use the manager to determine if you got them and, if not, request them. Hence I made the onboarding manager create the permissions manager and set it up as it needs to be for onboarding.

Now this setup leaves two cases uncovered:

• Profile view controller
• Permissions view controller

In both cases the items to show and the items to ask for permission are still the same as during onboarding. These can indeed be covered by using the permissions manager and have the app delegate supply the permissions manager, properly set up. While not ideal it's probably more convenient than abusing the onboarding manager for these tasks, which is currently the case.

I won't have the bandwith in the near future and since I'm not using the profile view controller it's a little harder for me to implement nicely. I'll let you open an issue and hope somebody gets to it. ;)

[ghost]

@YuanZhu-apple I'll open an issue after the merge of this to deal with the configuration question.

[YuanZhu-apple]

Cool.