Conversation
- Add reset() to clear all local SDK state including device ID - Deprecate removeUserId() in favor of reset()
WalkthroughRelease 1.5.0: adds a public Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3320ae49f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 15-19: The CHANGELOG and API are out of sync: update the
implementation in Clix.kt so setUserId accepts a nullable type (change
setUserId(userId: String) to setUserId(userId: String?) and handle null by
clearing the stored user id), mark removeUserId() as deprecated with `@Deprecated`
and make it delegate to setUserId(null) (or remove its body in favor of
delegation), and update any KDoc/comments for setUserId/removeUserId to reflect
the new behavior.
In `@clix/src/main/kotlin/so/clix/core/Clix.kt`:
- Around line 155-171: reset() currently removes local storage keys but fails to
clear backend user properties and the in-memory device state; update reset() to
call deviceService.removeProjectUserId() (await/handle errors) to remove the
project-level user ID on the backend and also clear the in-memory
Clix.environment.device (remove device id and token) so no residual device
identity remains; alternatively, if backend removal must be explicit, add a
clear of Clix.environment.device and update docs/comments to state that
deviceService.removeProjectUserId() must be invoked separately for backend
cleanup (reference: reset(), deviceService.removeProjectUserId(),
Clix.environment.device, and removeUserId()).
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@clix/src/main/kotlin/so/clix/core/Clix.kt`:
- Around line 155-172: The KDoc for Clix.reset() is ambiguous about backend user
identity removal and may mislead callers given the deprecated removeUserId() had
server-side effects; update the KDoc on the `@JvmStatic` fun reset() in Clix.kt to
explicitly state that reset() only clears local SDK state (device ID, local
session) and does NOT make any server calls or clear backend user properties,
and add guidance that callers who need backend cleanup should call
removeUserId() (or the corresponding server-clearing API) before removing that
deprecated method or changing its ReplaceWith hint so callers are not silently
left with backend identity data.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
clix/src/test/kotlin/so/clix/core/ClixTest.kt (1)
132-145: Consider verifying that SDK requires re-initialization afterreset().The
reset()implementation correctly sets the privateisInitializedflag to false, and the test covers the service resets. However, the initialization state reset itself is not verified. SinceisInitializedis private, either expose a publicisInitialized()getter or add a behavioral test that verifies the SDK requires re-initialization (e.g., attempt to use a method requiring initialization and confirm it fails or re-initializes).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clix/src/test/kotlin/so/clix/core/ClixTest.kt` around lines 132 - 145, After calling Clix.reset() the test should assert the SDK is no longer initialized; either add a public isInitialized() accessor on Clix and assert it returns false after Clix.reset(), or add a behavioral check by invoking a method that requires initialization (e.g., a method like Clix.initialize() or any public API that should fail when uninitialized such as calling session-related API) and assert it either throws an IllegalStateException or forces re-initialization; update the test to call Clix.reset() then use the chosen approach to verify the private isInitialized flag was effectively cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@clix/src/test/kotlin/so/clix/core/ClixTest.kt`:
- Around line 132-145: After calling Clix.reset() the test should assert the SDK
is no longer initialized; either add a public isInitialized() accessor on Clix
and assert it returns false after Clix.reset(), or add a behavioral check by
invoking a method that requires initialization (e.g., a method like
Clix.initialize() or any public API that should fail when uninitialized such as
calling session-related API) and assert it either throws an
IllegalStateException or forces re-initialization; update the test to call
Clix.reset() then use the chosen approach to verify the private isInitialized
flag was effectively cleared.
Summary
Add
reset()method for logout support and deprecateremoveUserId().Changes
reset()method that clears all local SDK state (device ID, session) without server callsremoveUserId()— usereset()insteadreset()Spec
https://docs.google.com/document/d/1ET-KqAFQlODNEm08YDXehzfWuqKZsOhnr4NJG-f0P9k/edit?tab=t.0#bookmark=id.9awmqjhnpii6
Summary by CodeRabbit
Release
New Features
Deprecations
Tests