-
Notifications
You must be signed in to change notification settings - Fork 2
fix(davinci-client): event-name-and-formData #260
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: ba80bec The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 ba80bec.
☁️ Nx Cloud last updated this comment at |
ec4d5b7
to
b21784c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
==========================================
+ Coverage 47.35% 56.29% +8.94%
==========================================
Files 29 20 -9
Lines 1472 1295 -177
Branches 148 147 -1
==========================================
+ Hits 697 729 +32
+ Misses 775 566 -209
🚀 New features to boost your workflow:
|
Deployed 0b638d7 to https://ForgeRock.github.io/ping-javascript-sdk/pr-260/0b638d7fd2127c7351150c0714bce19dafadd6ae branch gh-pages in ForgeRock/ping-javascript-sdk |
// No default or existing value is passed | ||
}, | ||
output: | ||
field.key !== undefined |
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.
Why would the password field not have a key
?
defaults event name to continue when we don't have an event name (p1 forms) and ensures we don't add undefined to the formData keys
b21784c
to
ba80bec
Compare
@@ -41,7 +41,9 @@ export function transformSubmitRequest(node: ContinueNode): DaVinciRequest { | |||
const formData = collectors?.reduce<{ | |||
[key: string]: string | number | boolean | (string | number | boolean)[]; | |||
}>((acc, collector) => { | |||
acc[collector.input.key] = collector.input.value; | |||
if (collector.input.key !== undefined) { |
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.
I'm wondering why we need this. If we are filtering by category, and no collector within these categories should have an undefined
, did we get something wrong?
options: options, | ||
}, | ||
output: | ||
field.key !== undefined |
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.
I don't believe we should do this. If there's no key
property, there's something fundamentally wrong with the data. If this is a SingleValueCollector
, it requires a key in order to send the value back to the server. Without a key, we can't assign the collected value to something and the whole flow will break.
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-3992
https://pingidentity.atlassian.net/browse/SDKS-3994
Description
defaults event name to continue when we don't have an event name (p1 forms) and ensures we don't add undefined to the formData keys