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

[MOB-10624] add anonymous user id to keychain #868

Conversation

evantk91
Copy link
Contributor

@evantk91 evantk91 commented Jan 21, 2025

🔹 Jira Ticket(s) if any

✏️ Description

This pull request adds anonymous user id to keychain and migrator. This was necessary after merging master into the feature branch.

Copy link
Contributor

@sumeruchat sumeruchat left a comment

Choose a reason for hiding this comment

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

Why remove the failure handleer

private boolean encryptionEnforced = false;
private boolean enableAnonActivation = false;
private boolean enableEmbeddedMessaging = false;
private int eventThresholdLimit = 100;
private IterableIdentityResolution identityResolution = new IterableIdentityResolution();
private IterableAnonUserHandler iterableAnonUserHandler;
private IterableDecryptionFailureHandler decryptionFailureHandler;
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a duplicate. See line 157. I added two by accident in the merge conflict.

Choose a reason for hiding this comment

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

right i didnt notice the one on top my bad
]

@@ -357,6 +355,8 @@ public Builder setEnableEmbeddedMessaging(boolean enableEmbeddedMessaging) {

public Builder setIdentityResolution(IterableIdentityResolution identityResolution) {
this.identityResolution = identityResolution;
return this;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it necessary to return this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I am just following the other config values. It was deleted when resolving the merge conflicts for some reason.

Choose a reason for hiding this comment

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

makes sense if other config values are also doing it

} else {
IterableLogger.w(TAG, "No anon user ID found to migrate.")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Since there was no KEY_ANON_USER_ID stored in encrypted shared prefs before we dont need to migrate it either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm good point I will remove.

Copy link
Member

@Ayyanchira Ayyanchira left a comment

Choose a reason for hiding this comment

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

lgtm

@evantk91 evantk91 merged commit 12f0cc4 into feature/itbl-track-anon-user Jan 23, 2025
3 of 4 checks passed
@evantk91 evantk91 deleted the evan/MOB-10624-add-anonymous-user-id-to-keychain branch January 23, 2025 19:08
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.

4 participants