SDKS-4927 Rename AgreementCollector to ReadOnlyTextCollector to align with the DaVinci READ_ONLY_TEXT input type#195
Open
SDKS-4927 Rename AgreementCollector to ReadOnlyTextCollector to align with the DaVinci READ_ONLY_TEXT input type#195
AgreementCollector to ReadOnlyTextCollector to align with the DaVinci READ_ONLY_TEXT input type#195Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughRenames AgreementCollector → ReadOnlyTextCollector and updates all bindings, imports, sample composables, tests, and README references to use ReadOnlyTextCollector and READ_ONLY_TEXT factory registration. ChangesCollector Rename and Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
witrisna
requested review from
spetrov and
vibhorgoswami
and removed request for
forgerock-chris
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/DaVinciContinueNode.kt (1)
1-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCopyright header should use the year range 2025-2026 for new source files.
The copyright header must include both 2025 and 2026 per the coding guidelines for new source files (
.kt). Update the header fromCopyright (c) 2026toCopyright (c) 2025-2026 Ping Identity Corporation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/DaVinciContinueNode.kt` around lines 1 - 6, Update the file header in DaVinciContinueNode.kt to use the required year range; replace the single-year copyright line "Copyright (c) 2026 Ping Identity Corporation" with "Copyright (c) 2025-2026 Ping Identity Corporation" so the top-of-file comment conforms to the new-source-file guideline.
🧹 Nitpick comments (1)
davinci/README.md (1)
107-109: ⚡ Quick win
ReadOnlyTextCollectoris listed but lacks a dedicated API documentation section.Every other collector in the README has a
####section describing its accessible properties (e.g.,#### LabelCollector (LABEL),#### SingleSelectCollector (DROPDOWN, RADIO)).ReadOnlyTextCollectoris only mentioned in the summary sentence, leaving its API undocumented for SDK consumers.📝 Suggested documentation section to add after the `SingleSelectCollector` block
+#### ReadOnlyTextCollector (READ_ONLY_TEXT) + +```kotlin +readOnlyTextCollector.key // The form field key +readOnlyTextCollector.type // The type value from the server (e.g., "AGREEMENT") +readOnlyTextCollector.content // The read-only text content to display +readOnlyTextCollector.title // The optional title +readOnlyTextCollector.titleEnabled // Whether the title is shown +readOnlyTextCollector.enabled // Whether this collector is active +readOnlyTextCollector.agreementId // The associated agreement definition ID +readOnlyTextCollector.useDynamicAgreement // Whether content is loaded dynamically +```Would you like me to open a new issue to track adding documentation sections for any other undocumented collectors?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@davinci/README.md` around lines 107 - 109, Add a dedicated API docs section for ReadOnlyTextCollector in the README mirroring other collectors: create a "#### ReadOnlyTextCollector (READ_ONLY_TEXT)" block after the SingleSelectCollector section and document its accessible properties (e.g., readOnlyTextCollector.key, readOnlyTextCollector.type, readOnlyTextCollector.content, readOnlyTextCollector.title, readOnlyTextCollector.titleEnabled, readOnlyTextCollector.enabled, readOnlyTextCollector.agreementId, readOnlyTextCollector.useDynamicAgreement) with brief one-line descriptions for each to match the style used by LabelCollector and SingleSelectCollector.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/DaVinciContinueNode.kt`:
- Around line 1-6: Update the file header in DaVinciContinueNode.kt to use the
required year range; replace the single-year copyright line "Copyright (c) 2026
Ping Identity Corporation" with "Copyright (c) 2025-2026 Ping Identity
Corporation" so the top-of-file comment conforms to the new-source-file
guideline.
---
Nitpick comments:
In `@davinci/README.md`:
- Around line 107-109: Add a dedicated API docs section for
ReadOnlyTextCollector in the README mirroring other collectors: create a "####
ReadOnlyTextCollector (READ_ONLY_TEXT)" block after the SingleSelectCollector
section and document its accessible properties (e.g., readOnlyTextCollector.key,
readOnlyTextCollector.type, readOnlyTextCollector.content,
readOnlyTextCollector.title, readOnlyTextCollector.titleEnabled,
readOnlyTextCollector.enabled, readOnlyTextCollector.agreementId,
readOnlyTextCollector.useDynamicAgreement) with brief one-line descriptions for
each to match the style used by LabelCollector and SingleSelectCollector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c31c87f-facd-42ea-8bfb-0c8bd64bd611
📒 Files selected for processing (8)
davinci/README.mddavinci/src/main/kotlin/com/pingidentity/davinci/CollectorRegistry.ktdavinci/src/main/kotlin/com/pingidentity/davinci/collector/ReadOnlyTextCollector.ktdavinci/src/test/kotlin/com/pingidentity/davinci/ReadOnlyTextCollectorTest.ktsamples/app/src/main/java/com/pingidentity/samples/app/davinci/collector/ContinueNode.ktsamples/app/src/main/java/com/pingidentity/samples/app/davinci/collector/ReadOnlyText.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/DaVinciContinueNode.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/collector/ReadOnlyText.kt
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #195 +/- ##
=============================================
+ Coverage 44.27% 44.28% +0.01%
Complexity 1276 1276
=============================================
Files 310 310
Lines 9334 9334
Branches 1353 1353
=============================================
+ Hits 4133 4134 +1
Misses 4637 4637
+ Partials 564 563 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…lign with the DaVinci `READ_ONLY_TEXT` input type - Rename `AgreementCollector` to `ReadOnlyTextCollector` across the library and sample applications to better represent its function of displaying read-only content. - Update `CollectorRegistry` to map the `READ_ONLY_TEXT` factory key to the renamed `ReadOnlyTextCollector`. - Rename the `Agreement` Compose UI component to `ReadOnlyText` and update its references in both sample apps. - Update `ContinueNode` in sample applications to handle the `ReadOnlyTextCollector` type. - Migrate and rename unit tests from `AgreementCollectorTest.kt` to `ReadOnlyTextCollectorTest.kt`. - Update documentation in `davinci/README.md` to list `ReadOnlyTextCollector` as an available collector.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA Ticket
SDKS-4927
Description
AgreementCollectortoReadOnlyTextCollectoracross the library and sample applications to better represent its function of displaying read-only content.CollectorRegistryto map theREAD_ONLY_TEXTfactory key to the renamedReadOnlyTextCollector.AgreementCompose UI component toReadOnlyTextand update its references in both sample apps.ContinueNodein sample applications to handle theReadOnlyTextCollectortype.AgreementCollectorTest.kttoReadOnlyTextCollectorTest.kt.davinci/README.mdto listReadOnlyTextCollectoras an available collector.Summary by CodeRabbit
Refactor
Documentation
Tests