[#3688] Use Documents URLs as fileupload variables values #4032
Conversation
Viicos
force-pushed
the
feature/3688-attachments
branch
3 times, most recently
from
162d93f
to
7d4112c
Compare
| try: | ||
| ObjectsAPISubmissionAttachment.objects.get( | ||
| submission_file_attachment=attachment | ||
| ) | ||
| except ObjectsAPISubmissionAttachment.DoesNotExist: | ||
| ObjectsAPISubmissionAttachment.objects.create( | ||
| submission_file_attachment=attachment, | ||
| documents_url=register_submission_attachment( | ||
| submission, attachment, options, documents_client | ||
| ), | ||
| ) |
There was a problem hiding this comment.
If I understand correctly, this prevents that if the Objects API registration fails, the attachments are sent again to the Documents API during the retry flow?
src/openforms/registrations/contrib/objects_api/submission_registration.py
Outdated
Show resolved
Hide resolved
src/openforms/registrations/contrib/objects_api/submission_registration.py
Outdated
Show resolved
Hide resolved
src/openforms/registrations/contrib/objects_api/submission_registration.py
Outdated
Show resolved
Hide resolved
Viicos
force-pushed
the
feature/3688-attachments
branch
3 times, most recently
from
a6f6552
to
3799efc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4032 +/- ##
==========================================
+ Coverage 96.06% 96.07% +0.01%
==========================================
Files 728 728
Lines 22854 22867 +13
Branches 2650 2654 +4
==========================================
+ Hits 21954 21970 +16
+ Misses 637 636 -1
+ Partials 263 261 -2 ☔ View full report in Codecov by Sentry. |
| for attachment in submission.attachments: | ||
| if attachment not in existing: | ||
| objs.append( | ||
| ObjectsAPISubmissionAttachment( | ||
| submission_file_attachment=attachment, | ||
| document_url=register_submission_attachment( | ||
| submission, attachment, options, documents_client | ||
| ), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
(please create a follow up issue for this)
This can still cause (repeated) failures - if you have two documents and the first succeeds, while the second does not, the for-loop will crash out and it will never be persisted that the first document creation actually succeeded.
you'll probably want to try: around this to make sure that you can create the model instances for whatever's in objs in the finally block:
try:
for attachmenet in ...:
# do the stuff
except:
raise
finally:
ObjectsAPISubmissionAttachment.objects.bulk_create(objs)| for key in dynamic_values.keys(): | ||
| if key in urls_map: | ||
| variable = state.get_variable(key) | ||
| is_multiple = variable.form_variable.form_definition.configuration_wrapper.component_map[ | ||
| key | ||
| ].get( | ||
| "multiple", False | ||
| ) | ||
| dynamic_values[key] = ( | ||
| urls_map[key][0] if not is_multiple else urls_map[key] | ||
| ) |
There was a problem hiding this comment.
I did some reasoning about the form_variable FK potentially being None and how this works if registration is run after a file-upload component is removed from the form, and I concluded it's not an issue, because:
stateis used, which ensures that we only consider currently available form variables (through thecollect_variablesmethod)dynamic_valuesis used, which relies on the above state considering only currently available form variables
So there may be extra SubmissionFileAttachment records for the submission, and those will exist in our DB but not sent along to the objects API. And this behaviour is as (currently) intended.
Necessary for tests making use of `freezegun`
For the multiple file component, create a single file attachment to make sure the component multiplicity is taken into account Adapt submission date after update to the 'now' static variable
Accessing dotted-keys like foo.bar in the underlying data structure used by FormioData does not work, the point of FormioData is to abstract this access.
sergei-maertens
force-pushed
the
feature/3688-attachments
branch
from
3799efc
to
4700eb1
Compare
Part of #3688, #4019