✨ [#4608] Modal for Objects API prefill options #4610
Merged
Conversation
This file contains 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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4610 +/- ##
=========================================
Coverage ? 96.56%
=========================================
Files ? 746
Lines ? 25184
Branches ? 3312
=========================================
Hits ? 24319
Misses ? 601
Partials ? 264 ☔ View full report in Codecov by Sentry. |
src/openforms/js/components/admin/form_design/variables/prefill/PrefillConfigurationForm.js
Outdated
Show resolved
Hide resolved
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
7 times, most recently
from
c2bba39 to
9424c20
Compare
|
Currently looks like this: |
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
from
5e67b5b to
17f010b
Compare
stevenbal
changed the title
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
3 times, most recently
from
c9901b2 to
5affccb
Compare
9 tasks
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
from
5affccb to
0e6353a
Compare
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
2 times, most recently
from
a8f7deb to
30cbead
Compare
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
from
30cbead to
6b51149
Compare
sergei-maertens
requested changes
Sep 23, 2024
src/openforms/js/components/admin/form_design/logic/actions/dmn/DMNParametersForm.js
Outdated
Show resolved
Hide resolved
src/openforms/js/components/admin/form_design/logic/actions/dmn/VariableMapping.js
Outdated
Show resolved
Hide resolved
src/openforms/js/components/admin/form_design/logic/actions/dmn/VariableMapping.js
Outdated
Show resolved
Hide resolved
src/openforms/js/components/admin/form_design/logic/actions/dmn/VariableMapping.js
Outdated
Show resolved
Hide resolved
src/openforms/js/components/admin/form_design/logic/actions/dmn/VariableMapping.js
Outdated
Show resolved
Hide resolved
src/openforms/js/components/admin/form_design/variables/prefill/PrefillConfigurationForm.js
Outdated
Show resolved
Hide resolved
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
2 times, most recently
from
3e37de2 to
cff46cf
Compare
stevenbal
force-pushed
the
feature/4608-product-prefill-modal
branch
3 times, most recently
from
ae346a9 to
237dae0
Compare
sergei-maertens
force-pushed
the
feature/4608-product-prefill-modal
branch
from
e0e3b97 to
f8b09c3
Compare
|
@stevenbal given your availability and how long this PR has been going on, I've taken some liberty in making the The main takeway is that the JSON serialization/deserialization is now an implementation detail, so you can use whatever value type for the dropdown values, while ensure the labels are something that can be rendered. It's a refinement of your implementation, but a bit more bold, and I made sure to fix all the prop types warnings/violations (those caused me to look into it in more detail to begin with). I'll continue polishing this so that it can hopefully be merged today or tomorrow :) |
sergei-maertens
force-pushed
the
feature/4608-product-prefill-modal
branch
from
1272c0c to
2fec7dc
Compare
this is the initial implementation, it is not yet fully functional, since it relies on backend changes that are not merged yet. It will be connected with the backend in #4693
* make VariableMapping generic by moving it outside of DMN directory * rename extraData to configurationContext * add test for plugin configurationContext * rename prefillAttribute to prefillProperty for Objects API prefill * move VariableMapping styling to its own file * define mapping for prefill plugins to components
because the prefill properties endpoint also specifies `targetPath`, which are lists to distinguish between nested values and values with `.` in them. Also ensure alreadyMapped works for Objects API prefill mapping
previously these were defined as part of registrations, but since they are now also used by prefill, their location should reflect that they are generic * ObjectsAPIGroup * ObjectTypeSelect * ObjectTypeVersionSelect
previously it was only possible to specify a prefix and the objecttype, objecttypeVersion and objectsApiGroup names were fixed. Now those are configurable and the prefix has been removed
sergei-maertens
force-pushed
the
feature/4608-product-prefill-modal
branch
from
2122125 to
e656a09
Compare
Now the VariableMapping component interface is determined, we need to make sure that the DMN configuration form can still properly use it after making it more generic.
After changing the API of the VariableMapping, the tests and implementation in the prefill configuration modal needed to be updated to be compatible.
Since there are two columns for source/target in the variable mapping component, it makes sense to display the direction of assignment with an arrow.
The hook is actually more broadly usable than in the objects_api, but for the time being it's only used there.
After moving the ObjectsAPIGroup component to a shared field layer where the form data structure is unknown at the component level, the code to reset dependent fields was no longer properly 'generic' and the approach with a prefix was not the most elegant: * it assumes the other (dependent) field names are the same in registration and prefill (they are, but deliberately, though there's no guarantee they will be in the future) * it assumes a certain structure, but dependent fields may live in another namespace or the reset behaviour could require additional changes in the form state to be made Instead, the reset behaviour can now be passed in as a prop, so that the caller/user of the ObjectsAPIGroup component is in control of which state exactly gets reset. This already reflects nicely since the variableMapping reset in the Legacy Objects API registration configuration didn't make any sense, it's a V2 option, so now we have tailored reset functions to the expected data structure.
Renamed and documented the props for more consistency with the rest of the codebase.
Renamed and documented the props for more consistency with the rest of the codebase.
Once we start re-using bits and pieces and have plugin-specific code/ configuration, it's better to stick to the one-component-per-file approach since (some of) the components are now re-usable and semi- public API.
This way, plugin-specific configuration forms cannot accidentally make it impossible to change the plugin type again when a mistake is made, and it reduces the code repetition.
sergei-maertens
force-pushed
the
feature/4608-product-prefill-modal
branch
from
8095b9d to
d31b31b
Compare
sergei-maertens
approved these changes
Oct 8, 2024
There was a problem hiding this comment.
@stevenbal it might be interesting to go over my commit messages that finetuned this PR!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #4608
Changes
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh./bin/compilemessages_js.shCommit hygiene