-
Notifications
You must be signed in to change notification settings - Fork 42
[SDK-149] add-logout-functionality #781
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
Conversation
|
Diff Coverage: The code coverage on the diff in this pull request is 100.0%. Total Coverage: This PR will increase coverage by 0.46%. File Coverage Changes
🛟 Help
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
Ayyanchira
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some suggestions in test file regarding email and userID flow
src/core/classes/Iterable.ts
Outdated
| Iterable.savedConfig.authHandler!() | ||
| .then((promiseResult) => { | ||
| // Promise result can be either just String OR of type AuthResponse. | ||
| // Promise result can be either just String OR of type AuthRespronse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spell mistake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha! I'm guessing that I pressed r to refresh the app, but was focused here
instead of the terminal
| IterableEventName.handleCustomActionCalled | ||
| ); | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleAuthCalled); | ||
| Iterable.removeAllEventListeners(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catching all in one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| static logout() { | ||
| Iterable.removeAllEventListeners(); | ||
| Iterable.setEmail(null); | ||
| Iterable.setUserId(null); | ||
| } | ||
|
|
||
| /** | ||
| * Removes all event listeners for the Iterable SDK. | ||
| */ | ||
| private static removeAllEventListeners() { | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleUrlCalled); | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleInAppCalled); | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleCustomActionCalled); | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleAuthCalled); | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleAuthSuccessCalled); | ||
| RNEventEmitter.removeAllListeners(IterableEventName.handleAuthFailureCalled); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
| Iterable.setEmail('[email protected]'); | ||
| Iterable.setUserId('user123'); | ||
| // WHEN Iterable.logout is called |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
email and userId usually dont go together.
When email is set and we call setUserId, it goes through logging out user with email id and then tries to log in.
So it should ideally be setEmail > logout > assernull for email
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I know.. this doesn't call the internal functionality for setting user id
and email, so I'm just doing this as a test to make sure that both are cleared
later. I'll add a comment to address this.
🔹 JIRA Ticket(s) if any
✏️ Description
Adds a logout helper to
Iterable