-
Notifications
You must be signed in to change notification settings - Fork 3
SDKS-4146: Implement Protect collector in DaVinci client #340
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
🦋 Changeset detectedLatest commit: 0fa522a The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
View your CI Pipeline Execution ↗ for commit 0fa522a
☁️ Nx Cloud last updated this comment at |
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (26.15%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #340 +/- ##
==========================================
- Coverage 61.61% 60.46% -1.15%
==========================================
Files 33 33
Lines 1985 2044 +59
Branches 287 291 +4
==========================================
+ Hits 1223 1236 +13
- Misses 762 808 +46
🚀 New features to boost your workflow:
|
Deployed 5c631e5 to https://ForgeRock.github.io/ping-javascript-sdk/pr-340/5c631e598c89cd69bc9845fabac5f4587b457bf3 branch gh-pages in ForgeRock/ping-javascript-sdk |
export type ReadOnlyFields = ReadOnlyField; | ||
export type RedirectFields = RedirectField; | ||
export type SingleValueFields = StandardField | ValidatedField | SingleSelectField; | ||
export type SingleValueFields = StandardField | ValidatedField | SingleSelectField | ProtectField; |
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.
Do we need a ProtectField here? I think we're using the field directly in the AutoCollector, right?
could be missing something
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.
A few lines above I define the ProtectField which I understood as an interface for the response we get from DaVinci. I wasn't sure if it should be categorized as a ComplexValueFields
or SingleValueFIelds
or even it's own category. But either way yeah I think it has to be defined here and included as a DaVinciField
type in order for the node.next
reducer to be able to return a Protect collector.
280e951
to
816ff82
Compare
816ff82
to
0fa522a
Compare
📦 Bundle Size Analysis📦 Bundle Size Analysis📊 Minor Changes📈 @forgerock/davinci-client - 34.3 KB (+0.6 KB) ➖ No Changes➖ @forgerock/sdk-types - 5.6 KB 11 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
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.
This looks good to me.
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-4146
Description
Implements Protect collector in DaVinci client. Will write e2e tests in another ticket because we do not have a working e2e flow yet. Manual testing only for now.
Changeset added.