Skip to content

SDKS-5169 Add Image component for DaVinci#214

Open
vibhorgoswami wants to merge 1 commit into
developfrom
SDKS-5169
Open

SDKS-5169 Add Image component for DaVinci#214
vibhorgoswami wants to merge 1 commit into
developfrom
SDKS-5169

Conversation

@vibhorgoswami

@vibhorgoswami vibhorgoswami commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

JIRA Ticket

SDKS-5169

Description

Add ImageCollector to display Image for DaVinci Forms.
image

Summary by CodeRabbit

  • New Features

    • Added image display capabilities to DaVinci authentication flows with support for image URLs, descriptions, and optional hyperlinks.
    • Images render as interactive cards in the sample application.
  • Tests

    • Added comprehensive test coverage for image collector initialization and field validation.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@vibhorgoswami, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 34 minutes and 34 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: aa6d3873-b0fd-4667-b74d-0864c5bf9de2

📥 Commits

Reviewing files that changed from the base of the PR and between ded523e and 0331291.

📒 Files selected for processing (5)
  • davinci/src/main/kotlin/com/pingidentity/davinci/CollectorRegistry.kt
  • davinci/src/main/kotlin/com/pingidentity/davinci/collector/ImageCollector.kt
  • davinci/src/test/kotlin/com/pingidentity/davinci/ImageCollectorTest.kt
  • samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/DaVinciContinueNode.kt
  • samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt
📝 Walkthrough

Walkthrough

Adds a new ImageCollector to the DaVinci SDK that parses key, description, imageUrl, and optional hyperlinkUrl from a JSON object. The collector is registered under "IMAGE" in CollectorRegistry. Unit tests cover field parsing and defaults. A Jetpack Compose ImageCard composable in the sample app renders the image via Coil with SVG support, a placeholder icon, and optional hyperlink/description text.

Changes

ImageCollector SDK and Sample App Rendering

Layer / File(s) Summary
ImageCollector model and registry registration
davinci/src/main/kotlin/com/pingidentity/davinci/collector/ImageCollector.kt, davinci/src/main/kotlin/com/pingidentity/davinci/CollectorRegistry.kt
Defines ImageCollector as a Collector<Nothing> with key, description, imageUrl, and nullable hyperlinkUrl parsed from a JsonObject via init(). Registers it under "IMAGE" in CollectorRegistry.
ImageCollector unit tests
davinci/src/test/kotlin/com/pingidentity/davinci/ImageCollectorTest.kt
Verifies init() populates all four fields correctly from a full JSON object, that id() returns key, that init() returns the collector instance for chaining, and that all fields default to empty strings or null when JSON entries are absent.
Compose ImageCard and DaVinciContinueNode wiring
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt, samples/.../DaVinciContinueNode.kt
Adds Image and ImageCard composables that render an ImageCollector as a bordered card with a Coil/SVG async image, a placeholder icon when loading/erroring, and conditional description and hyperlink text. Extends DaVinciContinueNode's when dispatch to render Image(it) for ImageCollector instances.

Sequence Diagram(s)

sequenceDiagram
  participant DaVinciContinueNode
  participant Image
  participant ImageCard
  participant CoilImageLoader
  participant AsyncImage

  DaVinciContinueNode->>Image: Image(ImageCollector)
  Image->>ImageCard: ImageCard(imageUrl, description, hyperlinkUrl)
  ImageCard->>CoilImageLoader: build with SvgDecoder
  ImageCard->>AsyncImage: render(imageUrl, imageLoader)
  AsyncImage-->>ImageCard: painter state (Loading/Success/Failure)
  ImageCard-->>Image: bordered card with placeholder or image, description, hyperlink
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • ForgeRock/ping-android-sdk#182: Uses the same pattern of registering a new collector in CollectorRegistry and adding a when branch in DaVinciContinueNode, applied there for AgreementCollector.

Suggested reviewers

  • spetrov
  • witrisna

Poem

🐇 Hop hop, a new image appears on the screen,
Coil loads SVGs, the prettiest you've seen!
A JSON key maps to a card so bright,
With placeholder bunnies when images aren't right.
Hyperlinks clickable, descriptions below—
The DaVinci flow just stole the show! 🖼️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title clearly summarizes the main change: adding an Image component for DaVinci, which aligns with the substantial addition of ImageCollector implementation and related UI components across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description includes the required JIRA ticket link and provides a brief description with a screenshot, but lacks detailed explanation of implementation, testing, and review considerations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch SDKS-5169

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.20%. Comparing base (ffc82b5) to head (0331291).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop     #214      +/-   ##
=============================================
+ Coverage      44.12%   44.20%   +0.07%     
- Complexity      1393     1405      +12     
=============================================
  Files            316      317       +1     
  Lines           9760     9773      +13     
  Branches        1495     1497       +2     
=============================================
+ Hits            4307     4320      +13     
  Misses          4891     4891              
  Partials         562      562              
Flag Coverage Δ
integration-tests 28.22% <7.69%> (-0.04%) ⬇️
unit-tests 26.13% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt (1)

96-99: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Remember ImageLoader to avoid rebuilding on recomposition.

ImageLoader is rebuilt during recompositions; this is avoidable allocation/config work in a hot UI path.

♻️ Proposed refactor
-        val imageLoader = ImageLoader.Builder(ContextProvider.context)
-            .components { add(SvgDecoder.Factory()) }
-            .build()
+        val imageLoader = remember {
+            ImageLoader.Builder(ContextProvider.context)
+                .components { add(SvgDecoder.Factory()) }
+                .build()
+        }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt`
around lines 96 - 99, The ImageLoader instance is being recreated on every
recomposition, causing unnecessary allocations and configuration work in a hot
UI path. Wrap the ImageLoader.Builder initialization and build() call within a
remember block so that the imageLoader is created only once and reused across
recompositions. Ensure that ContextProvider.context or any other dependencies
are included as key parameters to the remember call if they can change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@davinci/src/main/kotlin/com/pingidentity/davinci/collector/ImageCollector.kt`:
- Around line 48-51: The code accesses `jsonPrimitive` directly on JSON values
in the init block for the key, description, imageUrl, and hyperlinkUrl fields,
which throws an IllegalArgumentException when those fields contain non-primitive
JSON values like objects or arrays. Replace the direct `jsonPrimitive` access
with safe casting that gracefully handles non-primitive values without throwing
exceptions. Use a safer approach such as a try-catch block or safe casting
method to prevent crashes when malformed payloads contain JSON objects or arrays
instead of primitive values.

In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt`:
- Around line 86-90: The clickable modifier block currently validates
hyperlinkUrl only for null, but blank values and invalid URL schemes can still
pass through to startActivity, creating a security issue. Replace the null check
for hyperlinkUrl with isNullOrBlank() to exclude blank strings, and add
additional validation to ensure the URL only contains http or https schemes
before attaching the click behavior. This ensures consistency with the display
logic and prevents unintended deep links from launching via invalid or untrusted
URL schemes.

---

Nitpick comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt`:
- Around line 96-99: The ImageLoader instance is being recreated on every
recomposition, causing unnecessary allocations and configuration work in a hot
UI path. Wrap the ImageLoader.Builder initialization and build() call within a
remember block so that the imageLoader is created only once and reused across
recompositions. Ensure that ContextProvider.context or any other dependencies
are included as key parameters to the remember call if they can change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eccabb30-a046-4ffb-b51f-cfcd48c70ef3

📥 Commits

Reviewing files that changed from the base of the PR and between ffc82b5 and ded523e.

📒 Files selected for processing (5)
  • davinci/src/main/kotlin/com/pingidentity/davinci/CollectorRegistry.kt
  • davinci/src/main/kotlin/com/pingidentity/davinci/collector/ImageCollector.kt
  • davinci/src/test/kotlin/com/pingidentity/davinci/ImageCollectorTest.kt
  • samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/DaVinciContinueNode.kt
  • samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/Image.kt

Comment thread davinci/src/main/kotlin/com/pingidentity/davinci/collector/ImageCollector.kt Outdated
@vibhorgoswami vibhorgoswami marked this pull request as ready for review June 23, 2026 18:12

@spetrov spetrov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

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

Development

Successfully merging this pull request may close these issues.

3 participants