-
Notifications
You must be signed in to change notification settings - Fork 26
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
[#3688] Update Objects API registration options form #3861
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3861 +/- ##
=======================================
Coverage 96.34% 96.35%
=======================================
Files 714 714
Lines 22344 22343 -1
Branches 2563 2563
=======================================
+ Hits 21527 21528 +1
+ Misses 567 565 -2
Partials 250 250 ☔ View full report in Codecov by Sentry. |
9bc1db3
to
8b9c659
Compare
src/openforms/js/components/admin/form_design/registrations/objectsapi/ObjectsApiOptionsForm.js
Outdated
Show resolved
Hide resolved
b1d3913
to
f74bc8e
Compare
Of course, f74bc8e was too easy.. |
45726f9
to
19e2b68
Compare
I added test but they are probably meaningless for now as SB is getting crazy when running them I have no idea why |
}, | ||
}, | ||
form: ObjectsApiOptionsForm, | ||
onStepEdit: onStepEdit, |
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 onStepEdit
seems to be coded entirely on the structure of camunda registration options. Looks like zgw-create-zaak
has the same issue.
I'd expect some crashes due to undefined
not having an property find
🤔
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 confirmed the issue and patched straight to master: fac0afc
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.
Luckily the objects API does not require any changes on step edit?
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.
mapping of variable -> json path will need to be updated accordingly when a form variable is deleted/modified ;)
...orms/js/components/admin/form_design/registrations/objectsapi/ObjectsApiOptionsFormFields.js
Outdated
Show resolved
Hide resolved
...components/admin/form_design/registrations/objectsapi/ObjectsApiOptionsFormFields.stories.js
Outdated
Show resolved
Hide resolved
...components/admin/form_design/registrations/objectsapi/ObjectsApiOptionsFormFields.stories.js
Outdated
Show resolved
Hide resolved
690ae64
to
d9af27e
Compare
Wait what how are tests passing |
Use tabs to separate the legacy and new form Explicitly define the legacy form
Handling the effects when selecting an objecttype/version is hard, so lots of special casing
c923bc5
to
9ee405b
Compare
"aWwaAq": [ | ||
{ | ||
"type": 0, | ||
"value": "Maximum selected checkboxes" | ||
} | ||
], |
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.
Not sure why these where removed
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.
Some polishing and then this can be merged.
Part of #3688.
To be resolved:
I'm having an issue right now: as the selects are now required, a default objecttype gets selected as soon as the page loads. However, it does not trigger the
useAsync
for versions, resulting in an empty dropdown for versions.