[#4396] Objects API prefill plugin #4620

vaszig · 2024-08-26T11:03:48Z

Closes #4396 partially

Changes

Added necessary model field prefill_options to the FormVariable model (for storing all the needed configuration) and updated constraints according to new field
Moved public prefill functions to prefill.service
Added prefill functionality for ObjectsAPI

Checklist

Check off the items that are completed or not relevant.

Impact on features
- Checked copying a form
- Checked import/export of a form
- Config checks in the configuration overview admin page
- Problem detection in the admin email digest is handled
Release management
- I have labelled the PR as "needs-backport" accordingly
I have updated the translations assets (you do NOT need to provide translations)
- Ran ./bin/makemessages_js.sh
- Ran ./bin/compilemessages_js.sh
Commit hygiene
- Commit messages refer to the relevant Github issue
- Commit messages explain the "why" of change, not the how

codecov · 2024-08-26T11:16:52Z

Codecov Report

Attention: Patch coverage is 93.21267% with 15 lines in your changes missing coverage. Please review.

Project coverage is 96.55%. Comparing base (35b0af6) to head (b15f080).
Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
...c/openforms/forms/api/serializers/form_variable.py	47.05%	7 Missing and 2 partials ⚠️
src/openforms/prefill/service.py	90.76%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4620      +/-   ##
==========================================
- Coverage   96.57%   96.55%   -0.02%     
==========================================
  Files         746      748       +2     
  Lines       25271    25399     +128     
  Branches     3332     3354      +22     
==========================================
+ Hits        24406    24525     +119     
- Misses        602      609       +7     
- Partials      263      265       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/openforms/formio/tests/test_component_translations.py

src/openforms/forms/models/form_variable.py

src/openforms/forms/api/serializers/form_variable.py

src/openforms/prefill/base.py

src/openforms/prefill/contrib/objects_api/plugin.py

src/openforms/forms/api/serializers/form_variable.py

src/openforms/prefill/service.py

src/openforms/prefill/sources/user_defined.py

src/openforms/prefill/utils.py

src/openforms/submissions/models/submission_value_variable.py

sergei-maertens · 2024-10-23T07:28:58Z

src/openforms/submissions/tests/test_start_submission.py

+        # we are creating a new step below-no need for two steps
+        self.form.formstep_set.get().delete()
+        FormStepFactory.create(
            form=self.form,
-            form_definition=self.step.form_definition,
-            prefill_plugin="demo",
-            prefill_attribute="random_string",
+            form_definition__configuration={
+                "components": [
+                    {
+                        "type": "textfield",
+                        "key": "test-key",
+                        "label": "Test label",
+                        "prefill": {"plugin": "demo", "attribute": "random_string"},
+                    }
+                ]
+            },


does this result in the same variable being created?

I realized that in the way we test things here (getting the submission variables by SubmissionValueVariable.objects.filter(submission=submission) )gives us one submission variable when the form variables are two. I would do it with submission.load_submission_value_variables_state() tbh which gives 2. I am a bit confused here..

I traced back why the additional variable was created and patched that behaviour, so the test changes are reverted too :)

sergei-maertens · 2024-10-25T15:11:47Z

src/openforms/prefill/service.py

+        if not (prefill := component.get("prefill")):
+            continue
+        if not prefill.get("plugin"):
+            continue
+        if not prefill.get("attribute"):
+            continue


Lines were not covered before the code was moved, accepting this as-is and I'm ignoring the codecov annotations.

…lugin for ObjectsApi

Seems to pass the tests and simplifies the code a little. The try-except should not happen because of the way form variables are created/synced from formio definitions, and if it happens, we want to be notified of it via error monitoring. Additionally, we can apply the post-processing special treatment for component sources, and leave the default behaviour for everything else, rather than potentially missing some cases if additional sources get added in the future. The variable itself is already mutated during the saving of prefill data, so just updating the dict holding the [key, value] pairs is sufficient.

sergei-maertens · 2024-10-25T15:21:31Z

src/openforms/prefill/service.py

+            try:
+                component = total_config_wrapper.component_map[variable_key]
+            except KeyError:
+                return None


This looks a bit weird - if something is wrong we seem to discard everything. Does that make sense? The lack of coverage on the error handling also makes this seem like a what-if.

This could occur if someone manually creates a FormVariable in the admin/shell without it being backed by an actual form definition component, which shouldn't happen. The API endpoint synchronizes the components and derived variables, so I'm leaning towards removing the try-except and if it crashes, it crashes and at least it will become visible in Sentry instead of failing silently, no?

In the old situation this wouldn't happen anyway because we looped only over the prefill variables.

sergei-maertens · 2024-10-25T15:58:59Z

src/openforms/submissions/tests/test_start_submission.py

+        # we are creating a new step below-no need for two steps
+        self.form.formstep_set.get().delete()
+        FormStepFactory.create(
            form=self.form,
-            form_definition=self.step.form_definition,
-            prefill_plugin="demo",
-            prefill_attribute="random_string",
+            form_definition__configuration={
+                "components": [
+                    {
+                        "type": "textfield",
+                        "key": "test-key",
+                        "label": "Test label",
+                        "prefill": {"plugin": "demo", "attribute": "random_string"},
+                    }
+                ]
+            },


I traced back why the additional variable was created and patched that behaviour, so the test changes are reverted too :)

vaszig marked this pull request as draft August 26, 2024 11:41

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from 66ed5a5 to 482bc20 Compare August 27, 2024 14:30

vaszig force-pushed the task/4396-objectsapi-prefill-views-and-urls branch from 0ee299f to 2b992bf Compare August 27, 2024 14:36

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 4 times, most recently from 2b6a3f0 to 8eb07c9 Compare August 28, 2024 08:51

vaszig force-pushed the task/4396-objectsapi-prefill-views-and-urls branch from 2b992bf to 50c9377 Compare August 28, 2024 10:20

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from 8eb07c9 to 0d9f00c Compare August 28, 2024 10:23

vaszig force-pushed the task/4396-objectsapi-prefill-views-and-urls branch from 50c9377 to 70f0382 Compare August 28, 2024 12:44

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from 0d9f00c to b7e03db Compare August 28, 2024 13:02

vaszig requested a review from stevenbal August 28, 2024 13:21

vaszig marked this pull request as ready for review August 28, 2024 13:21

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 2 times, most recently from 7ac1670 to 71cc7e8 Compare September 2, 2024 06:12

vaszig mentioned this pull request Sep 4, 2024

[#4396] Prefill variables by using ObjectsAPI #4633

Closed

9 tasks

sergei-maertens linked an issue Sep 6, 2024 that may be closed by this pull request

Create a prefill plugin to prefill with values from an external reference (initially only for Objects API) #4396

Closed

vaszig force-pushed the task/4396-objectsapi-prefill-views-and-urls branch from 70f0382 to 0dfe280 Compare September 6, 2024 09:31

vaszig marked this pull request as draft September 6, 2024 12:39

vaszig removed the request for review from stevenbal September 6, 2024 12:39

vaszig force-pushed the task/4396-objectsapi-prefill-views-and-urls branch from 7b222b9 to 59304cb Compare September 13, 2024 13:41

Base automatically changed from task/4396-objectsapi-prefill-views-and-urls to master September 16, 2024 12:21

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 3 times, most recently from a7c3df4 to e60e61c Compare September 23, 2024 14:43

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 3 times, most recently from 0b5b2cd to 1a4c02c Compare October 2, 2024 07:18

vaszig marked this pull request as ready for review October 2, 2024 07:19

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from 1a4c02c to 110283f Compare October 2, 2024 11:26

sergei-maertens mentioned this pull request Oct 8, 2024

✨ [#4398] Check object ownership when creating submission #4696

Merged

9 tasks

sergei-maertens reviewed Oct 10, 2024

View reviewed changes

src/openforms/formio/tests/test_component_translations.py Show resolved Hide resolved

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 2 times, most recently from 050232d to cf2bb25 Compare October 10, 2024 14:55

sergei-maertens force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from cf2bb25 to 13938ae Compare October 11, 2024 09:21

sergei-maertens reviewed Oct 11, 2024

View reviewed changes

vaszig mentioned this pull request Oct 15, 2024

Migrate FormVariable's attribute field into prefill_options #4756

Open

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 4 times, most recently from 25a6f91 to 2890501 Compare October 18, 2024 12:29

vaszig requested a review from sergei-maertens October 18, 2024 12:50

sergei-maertens force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from 2890501 to ac6c7a4 Compare October 23, 2024 06:47

sergei-maertens reviewed Oct 23, 2024

View reviewed changes

vaszig force-pushed the task/4396-objectsapi-prefill-implement-plugin branch 3 times, most recently from dd39aa2 to e9de058 Compare October 25, 2024 07:58

sergei-maertens reviewed Oct 25, 2024

View reviewed changes

vaszig and others added 4 commits October 25, 2024 17:17

[#4396] Added prefill_options field to the form variable

89b4992

[#4396] Moved public functions to prefill.service and added prefill p…

9590a47

…lugin for ObjectsApi

🎨 [#4396] Only create the variables that are affected by prefill

b15f080

sergei-maertens force-pushed the task/4396-objectsapi-prefill-implement-plugin branch from e9de058 to b15f080 Compare October 25, 2024 15:58

sergei-maertens reviewed Oct 25, 2024

View reviewed changes

sergei-maertens approved these changes Oct 25, 2024

View reviewed changes

sergei-maertens merged commit 2cfc111 into master Oct 25, 2024
29 of 34 checks passed

sergei-maertens deleted the task/4396-objectsapi-prefill-implement-plugin branch October 25, 2024 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#4396] Objects API prefill plugin #4620

[#4396] Objects API prefill plugin #4620

vaszig commented Aug 26, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading

sergei-maertens Oct 23, 2024

vaszig Oct 24, 2024

sergei-maertens Oct 25, 2024

sergei-maertens Oct 25, 2024

sergei-maertens Oct 25, 2024

sergei-maertens Oct 25, 2024

[#4396] Objects API prefill plugin #4620

[#4396] Objects API prefill plugin #4620

Conversation

vaszig commented Aug 26, 2024 • edited Loading

codecov bot commented Aug 26, 2024 • edited Loading

Codecov Report

sergei-maertens Oct 23, 2024

Choose a reason for hiding this comment

vaszig Oct 24, 2024

Choose a reason for hiding this comment

sergei-maertens Oct 25, 2024

Choose a reason for hiding this comment

sergei-maertens Oct 25, 2024

Choose a reason for hiding this comment

sergei-maertens Oct 25, 2024

Choose a reason for hiding this comment

sergei-maertens Oct 25, 2024

Choose a reason for hiding this comment

vaszig commented Aug 26, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading