Skip to content

Don't allow empty string for appUserID during initialization#995

Merged
NachoSoto merged 5 commits into
RevenueCat:mainfrom
NachoSoto:empty-app-user-id
Nov 30, 2021
Merged

Don't allow empty string for appUserID during initialization#995
NachoSoto merged 5 commits into
RevenueCat:mainfrom
NachoSoto:empty-app-user-id

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Fixes [sc-10159].

Based on top of #974 because it contains a significant simplification of IdentityManager that makes this a lot easier.

Note that Purchases.logIn will still fail with an error when trying to use an empty string. This PR does not change that behavior.

@NachoSoto NachoSoto requested review from a team and taquitos November 27, 2021 03:34
@shortcut-integration

Copy link
Copy Markdown

@NachoSoto NachoSoto changed the title Don t allow empty string for AppUserID during initialization Don t allow empty string for appUserID during initialization Nov 27, 2021
Comment thread Purchases/Identity/IdentityManager.swift Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made the conscious choice to not trim whitespaces for a not-empty user ID.
For example, " nacho with spaces " still remains the same, but " " is considered empty.

Not sure why somebody would want leading or trailing whitespaces, but I figured we wouldn't want to change that behavior.

Comment thread PurchasesTests/Identity/IdentityManagerTests.swift Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lol

Comment thread Purchases/Identity/IdentityManager.swift Outdated

@aboedo aboedo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks great! Left a couple of minor comments but otherwise good to go

Comment thread Purchases/Public/Purchases.swift Outdated
@NachoSoto NachoSoto changed the title Don t allow empty string for appUserID during initialization Don't allow empty string for appUserID during initialization Nov 29, 2021
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Still waiting to hear back from support before merging #974, which this PR depends on.

NachoSoto added a commit to RevenueCat/purchases-android that referenced this pull request Nov 29, 2021
@NachoSoto NachoSoto force-pushed the identity-manager-configure-idempotent branch from 135c711 to af1f3a0 Compare November 29, 2021 23:07
@NachoSoto NachoSoto force-pushed the empty-app-user-id branch 4 times, most recently from f3e3cd8 to d3e288e Compare November 29, 2021 23:10
NachoSoto added a commit to RevenueCat/purchases-android that referenced this pull request Nov 29, 2021
@NachoSoto NachoSoto changed the base branch from identity-manager-configure-idempotent to main November 30, 2021 01:10
@NachoSoto NachoSoto changed the base branch from main to identity-manager-configure-idempotent November 30, 2021 01:10
@NachoSoto NachoSoto changed the base branch from identity-manager-configure-idempotent to main November 30, 2021 01:11
@NachoSoto NachoSoto merged commit 641e8b5 into RevenueCat:main Nov 30, 2021
@NachoSoto NachoSoto deleted the empty-app-user-id branch November 30, 2021 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants