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

feat: use property syntax all the properties that are persisted #114

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

1abhishekpandey
Copy link
Contributor

@1abhishekpandey 1abhishekpandey commented Jan 31, 2025

Description

  • We have provided getters for all properties, including userId, traits, and anonymousId, that persist in storage (excluding externalId as in future we are going to remove its persistence). Also, we've added a setter to set only the anonymousId.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactor/optimization

Implementation Details

  • We have added getters for all the properties that are persisted and setters only for the anonymousId, as userId and traits can be set while making the identify event. I've not provided the option to get the externalId, as we've decided to remove the persistence for this value in future PR.
  • I've added them as an extension function in the UserIdentity class itself, where those variables have been defined.
  • They can make a call like this:
// Settter
RudderAnalyticsUtils.analytics.anonymousId = "Custom Anonymous ID"
// Getter
val anonymousId = RudderAnalyticsUtils.analytics.anonymousId
val userId = RudderAnalyticsUtils.analytics.userId
val traits = RudderAnalyticsUtils.analytics.traits
  • I've added buttons to set and fetch these properties.
  • Added unit tests.

Checklist

  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation (if appropriate).
  • I have ensured that my code follows the project's code style.
  • I have checked for potential performance impacts and optimized if necessary.
  • I have checked the code for security issues.
  • I have updated the changelog (if required).

How to test?

  • Use the above code snippet or buttons provided in the sample app and you can see the value being fetched and set.
  • anonymousId:
    • After SDK init, get the anonymousId: It'll return the UUID v4.
    • Call shutdown, and then get the anonymousId: It'll return null.
    • Set the anonymousId, and then get the anonymousId: It'll return the custom anonymousId.
  • userId and traits:
    • After SDK init, get the userId and traits: It'll return empty values.
    • Make and identify the event and set both the values, and then try to get them: It'll return the set values.
    • Call shutdown, and then get them: It'll return null.
image

Breaking Changes

  • We have removed the getAnonymousId and setAnonymousId APIs defined in the Analytics class. Now, the user needs to access this as a property. Now, this is no longer valid:
val anonymousId = analytics.getAnonymousId()
analytics.setAnonymousId("<custom_anonymousId>")

Maintainers Checklist

  • The code has been reviewed.
  • CI tests have passed.
  • All necessary documentation has been updated.

Screenshots (if applicable)

Before:
image

After:
image

Additional Context

NOTE: We've not added getters for externalId, as we've planned to remove storage support for that property.

SDK persists `userId`, `anonymousId` and `traits`. In future, we are not going to persist the `externalId`. Therefore, I omitted that.
@1abhishekpandey 1abhishekpandey self-assigned this Jan 31, 2025
@1abhishekpandey 1abhishekpandey changed the title Feat/sdk 2877 use property syntax in useridentity members feat: use property syntax for UserIdentity values that are persisted Jan 31, 2025
@1abhishekpandey 1abhishekpandey changed the title feat: use property syntax for UserIdentity values that are persisted feat: use property syntax for UserIdentity properties that are persisted Jan 31, 2025
@1abhishekpandey 1abhishekpandey changed the title feat: use property syntax for UserIdentity properties that are persisted feat: use property syntax all the properties that are persisted Jan 31, 2025
@1abhishekpandey 1abhishekpandey marked this pull request as ready for review January 31, 2025 14:33
* val anonymousId = analyticsInstance.anonymousId
* ```
*/
var Analytics.anonymousId: String?
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that when we have extension property like this, we are unable to access getter and setter for this field in a java class making it java incompatible. We have to define them as class level property in this case.


val anonymousId = analytics.anonymousId

assertTrue(anonymousId == CUSTOM_ANONYMOUS_ID)
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: we can use assertEquals instead of assertTrue at some places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants