IdentityManager.configure: warn on empty App User ID and convert it to anonymous#384
Conversation
|
Kotlin and Android are obviously not my areas of expertise, so please don't be easy on me :) |
There was a problem hiding this comment.
I imagine there's nothing weird with Kotlin here and this only evaluates to true if the result is not null and true.
There was a problem hiding this comment.
This is the right way of doing it :)
|
This pull request has been linked to Shortcut Story #10159: Don't allow empty string for AppUserId during Purchases initialization. |
…o anonymous Fixes [sc-10159]. See also RevenueCat/purchases-ios#995 for iOS equivalent.
7af5fa2 to
555396f
Compare
taquitos
left a comment
There was a problem hiding this comment.
🎈🐐 LGTM but I don't have enough experience to approve.
| return if (this.isBlank()) { | ||
| null | ||
| } else { | ||
| this |
There was a problem hiding this comment.
WHAT IS THIS? JAVASCRIPT? 🙃
There was a problem hiding this comment.
This can be written as:
this.ifBlank { null }
There was a problem hiding this comment.
This is the right way of doing it :)
| return if (this.isBlank()) { | ||
| null | ||
| } else { | ||
| this |
There was a problem hiding this comment.
This can be written as:
this.ifBlank { null }
| } | ||
|
|
||
| val appUserIDToUse = appUserID | ||
| ?.notEmpty |
There was a problem hiding this comment.
Instead of doing the extension, this can be written as "appUserID?.takeUnless { it.isBlank() }", which it feels more Kotlin to me.
I had to see what notEmpty was doing to understand what it meant. empty in Kotlin is typically used for "". Something is blank when it's either the empty string or if it consist solely of whitespace character. And notEmpty is actually checking if it's not blank.
Maybe calling the function nullIfBlank(). Also if you look into the standard library extension functions for string, they are normally extension functions and not extension properties.
There was a problem hiding this comment.
That's exactly the type of feedback I was looking for, thanks! I'll use appUserID?.takeUnless { it.isBlank() }
There was a problem hiding this comment.
I really like this. Seems like Kotlin's STL is wider than Swift's.
343c713 to
a990f95
Compare
### Description Attempt at dealing with [CSDK-531](https://revenuecats.atlassian.net/browse/CSDK-531) Back in #384, we added checks to make sure we don't allow blank user ids. However, before that was added, user ids could be empty and could have subscriber attributes. Those could remain in SharedPreferences unless users uninstall which could cause some issues when syncing user attributes. This is an attempt of solving that issue by just ignoring those old user ids when syncing subscriber attributes. [CSDK-531]: https://revenuecats.atlassian.net/browse/CSDK-531?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Fixes [sc-10159].
See also RevenueCat/purchases-ios#995 for iOS equivalent.