Skip to content

feat: simplify handleEmbeddedClick API and add KDoc (#969)#1039

Closed
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/969-handleEmbeddedClick-simplify-api
Closed

feat: simplify handleEmbeddedClick API and add KDoc (#969)#1039
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/969-handleEmbeddedClick-simplify-api

Conversation

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor

@franco-zalamena-iterable franco-zalamena-iterable commented Apr 7, 2026

Summary

IterableEmbeddedManager.handleEmbeddedClick had two unused parameters (message and buttonIdentifier) that made the API confusing and required extra work from callers without providing any benefit.

Changes

  • New method: handleEmbeddedClick(clickedUrl: String) — clean API without unnecessary parameters
  • Deprecated: old handleEmbeddedClick(message, buttonIdentifier, clickedUrl) with @Deprecated annotation pointing to the new method
  • KDoc: Added documentation to IterableEmbeddedManager class and all public methods

Migration

Before:

iterableEmbeddedManager.handleEmbeddedClick(message, null, url)

After:

iterableEmbeddedManager.handleEmbeddedClick(url)

Test plan

  • Verify new handleEmbeddedClick(url) correctly handles click events
  • Verify old method still works (no breaking change)
  • Verify deprecation warning appears in IDE when using old method
  • Verify KDoc is visible in IDE autocomplete

Made with Cursor

#969)

Add new handleEmbeddedClick(clickedUrl: String) that removes unnecessary
parameters (message, buttonIdentifier) that were never used internally.
Deprecate the old overload with a ReplaceWith suggestion. Add KDoc
documentation to IterableEmbeddedManager for better discoverability.

Made-with: Cursor
@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

PR Analysis

Problem: IterableEmbeddedManager.handleEmbeddedClick(message, buttonIdentifier, clickedUrl) requires callers to supply message and buttonIdentifier parameters that are never used inside the method, making the API unnecessarily cumbersome. The class also lacks documentation.

Ideal fix plan:

  • Add a new overload handleEmbeddedClick(clickedUrl: String) containing the actual logic
  • Deprecate the old three-parameter method, delegating to the new one
  • Add KDoc to the class and its public surface
  • Update the internal caller in IterableEmbeddedView to use the new overload
  • Add unit tests for the new method covering the three URL-scheme branches (action://, itbl://, regular URL) and the empty-string guard

What the PR did:

  • Added handleEmbeddedClick(clickedUrl: String) with the extracted logic
  • Deprecated the old method with @Deprecated annotation and delegated to the new one
  • Added KDoc to the class and all public methods

Assessment:

  • Root cause identified: yes -- the unused parameters are correctly identified as the issue
  • Fix correctness: partially correct -- the new overload and deprecation are correct, but there are gaps (see below)
  • Missed:
    • No tests added. The new method has three distinct code paths (action://, itbl://, regular URL) plus an empty-string early return. All four branches should have unit tests. The old deprecated method's delegation should also be tested.
    • IterableEmbeddedView not updated. The SDK's own IterableEmbeddedView.kt calls the old three-parameter method in two places (setDefaultAction and the button click listener). These should be migrated to the new overload so the SDK itself does not trigger its own deprecation warning. This is also a good signal to verify the new API works in practice.
    • ReplaceWith is fragile. The suggested replacement handleEmbeddedClick(clickedUrl ?: return) will not compile at most call sites because return inside a lambda or non-local context behaves differently. A safer ReplaceWith would be handleEmbeddedClick(clickedUrl ?: "") or simply drop the ReplaceWith and let the KDoc guide migration.
  • Wrong assessment: none -- the analysis of unused parameters is accurate
  • Tests: needed but missing

@franco-zalamena-iterable franco-zalamena-iterable deleted the fix/969-handleEmbeddedClick-simplify-api branch April 8, 2026 14:43
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.

1 participant