Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

customEntitlementsComputation: new method to switch users #2437

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Apr 24, 2023

Adds a new method to switch users without posting to the backend for Custom Entitlements Computation mode.

@aboedo aboedo added the pr:feat A new feature label Apr 24, 2023
@aboedo aboedo self-assigned this Apr 24, 2023
Copy link
Contributor

@NachoSoto NachoSoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Maybe we can link to this method in the new Purchases.configure method.

Sources/Identity/IdentityManager.swift Outdated Show resolved Hide resolved
Sources/Identity/IdentityManager.swift Outdated Show resolved Hide resolved
Sources/Purchasing/Purchases/Purchases.swift Outdated Show resolved Hide resolved
Sources/Purchasing/Purchases/Purchases.swift Outdated Show resolved Hide resolved
Sources/Identity/IdentityManager.swift Outdated Show resolved Hide resolved
Sources/Purchasing/Purchases/Purchases.swift Show resolved Hide resolved
@tonidero
Copy link
Contributor

Just a heads up that I'm taking over this PR. Will address current comments first then test it manually and add unit tests.

@tonidero tonidero assigned tonidero and unassigned aboedo Apr 25, 2023
/// with the newAppUserID.
///
@objc(switchUserTo:)
func switchUser(to newAppUserID: String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely feels some users might inadvertently use this method and it will error silently for them. I wonder if we should just crash if they are not in custom entitlement computation mode... Also, moving this to a different package could avoid this confusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved by #2440.

@tonidero
Copy link
Contributor

Tested this manually and seems to work fine. Also added some unit tests though if we end up going with the approach of building a different package, we might have to move them as well... In any case, this is mostly final now.

@tonidero tonidero marked this pull request as ready for review April 25, 2023 17:17
@tonidero tonidero requested a review from NachoSoto April 25, 2023 17:17
@tonidero tonidero force-pushed the andy/sdk-3082-update-login-so-that-it-doesnt-issue-a branch from 72e6157 to 41edbe0 Compare April 25, 2023 17:40
@tonidero tonidero changed the base branch from andy/sdk-3068-dangeroussetting-that-disables to andy/sdk-3068-dangeroussetting-that-disables_rebased April 25, 2023 17:40
Copy link
Contributor

@NachoSoto NachoSoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment about the Obj-C method

Sources/Purchasing/Purchases/Purchases.swift Outdated Show resolved Hide resolved
@tonidero tonidero merged commit ea7b5d9 into andy/sdk-3068-dangeroussetting-that-disables_rebased Apr 25, 2023
@tonidero tonidero deleted the andy/sdk-3082-update-login-so-that-it-doesnt-issue-a branch April 25, 2023 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:feat A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants