Skip to content
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

Drop dependency on gemma-zds-client #4059

Merged
merged 16 commits into from
Mar 28, 2024

Conversation

sergei-maertens
Copy link
Member

Closes #4057

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 98.55072% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 96.08%. Comparing base (1203022) to head (cbe57e9).
Report is 4 commits behind head on master.

Files Patch % Lines
...penforms/registrations/contrib/zgw_apis/options.py 98.36% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4059   +/-   ##
=======================================
  Coverage   96.07%   96.08%           
=======================================
  Files         728      729    +1     
  Lines       22892    22917   +25     
  Branches     2659     2662    +3     
=======================================
+ Hits        21994    22020   +26     
  Misses        636      636           
+ Partials      262      261    -1     

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

@sergei-maertens sergei-maertens force-pushed the chore/4057-drop-gemma-zds-client branch from e89d855 to bc38084 Compare March 26, 2024 09:39
@sergei-maertens sergei-maertens force-pushed the chore/4057-drop-gemma-zds-client branch from bc38084 to b53be20 Compare March 26, 2024 09:45
@sergei-maertens sergei-maertens force-pushed the chore/4057-drop-gemma-zds-client branch 2 times, most recently from 381ed39 to b27de95 Compare March 26, 2024 10:12
Configuration of the fields (defaults) from zgw api group needs to be
stored on the registration backend options so that it's made
explicit.
From zgw api group instance to form backend configuration, making
the fields explicit.
Instead, make the fields required on the options serializer, ensuring
they're specified for each registration backend.

The previous commit handles the data migration for the instances
without explicit configuration.
Dropping gemma-zds-client means that we can't use any of its
API anymore.
Which drops gemma-zds-client as dependency.
@sergei-maertens sergei-maertens force-pushed the chore/4057-drop-gemma-zds-client branch from b27de95 to 11a645a Compare March 26, 2024 14:02
The import was a left-over import that didn't have any effect.
@sergei-maertens sergei-maertens force-pushed the chore/4057-drop-gemma-zds-client branch from 4296951 to 19b6158 Compare March 26, 2024 16:41
@sergei-maertens
Copy link
Member Author

Test failures:

  • openforms.registrations.contrib.zgw_apis.tests.test_validators
  • openforms.submissions.tests.test_on_completion_retry_chain.OnCompletionRetryFailedRegistrationTests.test_backend_registration_succeeds
  • openforms.submissions.tests.test_on_completion_retry_chain.OnCompletionRetryFailedUpdatePaymentStatusTests.test_payment_status_update_now_succeeds
  • openforms.submissions.tests.test_on_completion_retry_chain.OnCompletionRetryFailedUpdatePaymentStatusTests.test_payment_status_update_still_fails

Discussed with the team on Slack, extensive input validation
where relations between configuration aspects are verified
for consistency and integrity is required, but only during
configuration of the form.

At registration time itself, only the validations that don't
touch external services are performed, so that:

* tests are simpler to write without having to set up mocks
  or VCR, distracting from the actual tested behaviour
* performance during registration is not affected because
  of additional IO. A validation error on this aspect
  would manifest as a runtime error anyway, which is the
  same outcome as the remote API not accepting the
  configured options.
@sergei-maertens sergei-maertens force-pushed the chore/4057-drop-gemma-zds-client branch from 17ea68d to 0e6e787 Compare March 28, 2024 07:52
The updated validation would require adding more mocks to the existing
tests, or instead, we can extend the fixture and use VCR for the Open
Zaak API interaction.
@sergei-maertens sergei-maertens marked this pull request as ready for review March 28, 2024 09:42
@Viicos Viicos self-requested a review March 28, 2024 11:37
@sergei-maertens sergei-maertens merged commit 01694d6 into master Mar 28, 2024
28 checks passed
@sergei-maertens sergei-maertens deleted the chore/4057-drop-gemma-zds-client branch March 28, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to zgw-consumers 0.32.0
2 participants