Skip to content

Commit

Permalink
👌 [#4980] Process PR feedback
Browse files Browse the repository at this point in the history
* Revise result in register_submission: submission.registration_result automatically gets written with the return value of register_submission, see https://github.com/open-formulieren/open-forms/blob/794bbdae3c62eb4e4ce429661b402b57f24f4605/src/openforms/registrations/tasks.py#L338
* Remove explicit boolean assignments to properties in Variables: this is not necessary in JSX
* Add the json_dump app to the pyright.pyproject.toml, so it is type checked in CI. Also update the types where necessary
  • Loading branch information
viktorvanwijk committed Jan 21, 2025
1 parent 2500948 commit 29b11fa
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 19 deletions.
1 change: 1 addition & 0 deletions pyright.pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ include = [
# Registrations
"src/openforms/registrations/tasks.py",
"src/openforms/registrations/contrib/email/",
"src/openforms/registrations/contrib/json_dump/",
"src/openforms/registrations/contrib/stuf_zds/options.py",
"src/openforms/registrations/contrib/stuf_zds/plugin.py",
"src/openforms/registrations/contrib/stuf_zds/typing.py",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const Variables = () => {
isMulti
required
closeMenuOnSelect={false}
includeStaticVariables={true}
includeStaticVariables
/>
</Field>
</FormRow>
Expand Down
26 changes: 12 additions & 14 deletions src/openforms/registrations/contrib/json_dump/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
SubmissionFileAttachment,
SubmissionValueVariable,
)
from openforms.typing import JSONObject
from openforms.typing import JSONObject, JSONValue
from openforms.variables.constants import FormVariableSources

from ...base import BasePlugin # openforms.registrations.base
Expand All @@ -26,6 +26,7 @@ class JSONDumpRegistration(BasePlugin):
verbose_name = _("JSON dump registration")
configuration_options = JSONDumpOptionsSerializer

# TODO: add JSONDumpResult typed dict to properly indicate return value
def register_submission(
self, submission: Submission, options: JSONDumpOptions
) -> dict:
Expand Down Expand Up @@ -62,17 +63,14 @@ def register_submission(
# Send to the service
data = json.dumps({"values": values, "schema": schema}, cls=DjangoJSONEncoder)
service = options["service"]
submission.registration_result = result = {}
with build_client(service) as client:
if ".." in (path := options["path"]):
raise SuspiciousOperation("Possible path traversal detected")

res = client.post(path, json=data)
res.raise_for_status()
result = client.post(path, json=data)
result.raise_for_status()

result["api_response"] = res.json()

return result
return {"api_response": result.json()}

def check_config(self) -> None:
# Config checks are not really relevant for this plugin right now
Expand Down Expand Up @@ -101,7 +99,7 @@ def process_variables(submission: Submission, values: JSONObject):
if component is None or component["type"] != "file":
continue

encoded_attachments = [
encoded_attachments: list[JSONValue] = [
{
"file_name": attachment.original_name,
"content": encode_attachment(attachment),
Expand All @@ -111,19 +109,19 @@ def process_variables(submission: Submission, values: JSONObject):
]

match (
multiple := component.get("multiple", False),
n_attachments := len(encoded_attachments)
component.get("multiple", False),
n_attachments := len(encoded_attachments),
):
case False, 0:
values[key] = None
case False, 1:
values[key] = encoded_attachments[0]
case True, _:
case True, int():
values[key] = encoded_attachments
case _:
case False, int(): # pragma: no cover
raise ValueError(
f"Combination of multiple ({multiple}) and number of "
f"attachments ({n_attachments}) is not allowed."
f"Cannot have multiple attachments ({n_attachments}) for a "
"single file component."
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_submission_happy_flow(self):

SubmissionFileAttachmentFactory.create(
form_key="file",
submission_step=submission.submissionstep_set.get(),
submission_step=submission.steps[0],
file_name="test_file.txt",
original_name="test_file.txt",
content_type="application/text",
Expand Down Expand Up @@ -96,6 +96,7 @@ def test_submission_happy_flow(self):
}

result = json_plugin.register_submission(submission, options)
assert result is not None

self.assertEqual(result["api_response"], expected_response)

Expand Down Expand Up @@ -152,7 +153,7 @@ def test_multiple_file_uploads(self):

SubmissionFileAttachmentFactory.create(
form_key="file",
submission_step=submission.submissionstep_set.get(),
submission_step=submission.steps[0],
file_name="file1.txt",
original_name="file1.txt",
content_type="application/text",
Expand All @@ -163,7 +164,7 @@ def test_multiple_file_uploads(self):

SubmissionFileAttachmentFactory.create(
form_key="file",
submission_step=submission.submissionstep_set.get(),
submission_step=submission.steps[0],
file_name="file2.txt",
original_name="file2.txt",
content_type="application/text",
Expand Down Expand Up @@ -191,6 +192,7 @@ def test_multiple_file_uploads(self):
]

result = json_plugin.register_submission(submission, options)
assert result is not None

self.assertEqual(
result["api_response"]["data"]["values"]["file"], expected_files
Expand All @@ -215,7 +217,7 @@ def test_one_file_upload_for_multiple_files_component(self):

SubmissionFileAttachmentFactory.create(
form_key="file",
submission_step=submission.submissionstep_set.get(),
submission_step=submission.steps[0],
file_name="file1.txt",
original_name="file1.txt",
content_type="application/text",
Expand All @@ -232,6 +234,7 @@ def test_one_file_upload_for_multiple_files_component(self):
json_plugin = JSONDumpRegistration("json_registration_plugin")

result = json_plugin.register_submission(submission, options)
assert result is not None

self.assertEqual(
result["api_response"]["data"]["values"]["file"],
Expand Down Expand Up @@ -261,6 +264,7 @@ def test_no_file_upload_for_single_file_component(self):
json_plugin = JSONDumpRegistration("json_registration_plugin")

result = json_plugin.register_submission(submission, options)
assert result is not None

self.assertEqual(result["api_response"]["data"]["values"]["file"], None)

Expand All @@ -282,6 +286,7 @@ def test_no_file_upload_for_multiple_files_component(self):
json_plugin = JSONDumpRegistration("json_registration_plugin")

result = json_plugin.register_submission(submission, options)
assert result is not None

self.assertEqual(result["api_response"]["data"]["values"]["file"], [])

Expand Down

0 comments on commit 29b11fa

Please sign in to comment.