Merged requesting and collecting tokens under the same section

[ajaysubra]

Description

⚠️ README ⚠️

Realized that the SDK doesn't refresh the push enablement status automatically and hence my prior instructions were incorrect. Update them to now set the push token only after requesting push permissions.

This won't allow for silent push to work in some cases as our SDK currently only allows setting token after requesting push permission. I can think of two options here -

1. Having a seperate method for setting the push enablement status so that we can get the token on app launch and then allow the user to request push permission at any point in their app as they see fit and update our SDK when they have the permissions (or not).
2. The SDK listens for app foregrounded life cycle events and gets the push enablement status and updates the register token endpoint whenever the enablement status has changed. I think I prefer this over the first approach.

[evan-masseau]

I don't understand why you have to merge the readme sections, and I think that is confusing because the two don't have to be done together. Can't we just update the code sample about requesting permission to include calling registerForRemoteNotifications from the callback, so that they will update token state after permission is set?

[ajaysubra]

think that is confusing because the two don't have to be done together.

Agree in theory but as it currently stands with our SDK they are tightly coupled since you can only set token after requesting permission else the token will never be refreshed.

Can't we just update the code sample about requesting permission to include calling registerForRemoteNotifications from the callback, so that they will update token state after permission is set?

registerForRemoteNotifications is now indeed called from the closure after requesting permissions. About keeping the two sections separate - I guess I could but when I think about how devs would implement this, and how closely they are tied it made sense to keep them together.

Having said all that, if we act on my suggestions in the description above I do think we can separate them out and I think we should since if we end up at some future point supporting silent push the tight coupling doesn't work since we'd need to token as soon as someone initializes our app and also allows users to request permission when they see fit.

[evan-masseau]

I'd prefer to keep the separate sections, rather than merge them now and separate them again later. And I think we'd be better served to write this readme as if we support silent push already, because when we do add silent push, customers would be better positioned to take advantage right away if they've already been collecting tokens prior to the permission request anyway.

[ajaysubra]

I'd prefer to keep the separate sections, rather than merge them now and separate them again later. And I think we'd be better served to write this readme as if we support silent push already, because when we do add silent push, customers would be better positioned to take advantage right away if they've already been collecting tokens prior to the permission request anyway.

💯 agree and kinda the sentiment when I wrote the description of the PR above. Just that we'd have to implement one of the suggestions I made for us to be able to do that. The reason to merge them now is because it easier for devs to implement it in one shot vs. jumping between sections since they are closely tied. registerForRemoteNotifications has to be called after permission is requested and that triggers didRegisterForRemoteNotificationsWithDeviceToken delegate method to be called. I'm flexible if you want us to seperate it but I do think as things are now with our SDK, it's easier for devs to do it in one go.

[evan-masseau]

They can call registerForRemoteNotifications twice (right?). If they do call them very close together, the SDK will consolidate to only one API call anyway. I think the readme change that I'd propose would be to add the part about calling registerForRemoteNotifications in their permission check callback, but keep the sections separated as they were before.

[evan-masseau]

If it helps I can make a separate PR with what I'm suggesting to see if it makes sense, or if I'm still missing something.

[ajaysubra]

Closing in favor of #157