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

Battletest the ZGW API-connection between Rxmission and Open Formulieren #3649

Closed
2 tasks
joeribekker opened this issue Nov 30, 2023 · 10 comments · Fixed by #3673
Closed
2 tasks

Battletest the ZGW API-connection between Rxmission and Open Formulieren #3649

joeribekker opened this issue Nov 30, 2023 · 10 comments · Fixed by #3673
Assignees
Milestone

Comments

@joeribekker
Copy link
Contributor

joeribekker commented Nov 30, 2023

Thema / Theme

Other

Omschrijving / Description

There are few unexpected issues when using Rxmission as ZGW API registration backend.

Where we typically read the OAS to adjust our calls to the backend, this concept came unexpected to Visma Roxit. The OAS was not readily available for retrieval. After manually adding the OAS, some other issues came to light, like passing an API-version header attribute bestandsomvang that didn't match the upload size (for documents).

These issues needs to be tracked, analysed and fixed. Since the ZGW API in Rxmission is also subject to changes, we might ask them to fix things or workaround issues for now.

Added value / Toegevoegde waarde

Harden the ZGW API registration backend to be used in more infra setups.

Aanvullende opmerkingen / Additional context

No claims are made who is correct or not, we just want things to work :)

Tasks

  • Add version selection option to Service. This value can be empty. If filled, it should have an effect on the API-version header that is sent.
  • Figure out why the error doesn't show up in request-logging

By setting API-version to 1.0 for Documenten API, this should work on Rxmission.

@joeribekker joeribekker added triage Issue needs to be validated. Remove this label if the issue considered valid. enhancement labels Nov 30, 2023
@sergei-maertens
Copy link
Member

Since 2.4 we no longer use the API spec to figure out where to send the request, the URL paths are now hardcoded, e.g. https://github.com/open-formulieren/open-forms/blob/master/src/openforms/contrib/zgw/clients/zaken.py#L51

@joeribekker
Copy link
Contributor Author

Refinement: We should document the explicit versions that we support in Open Forms and which version Rxmission is using. We could change our behaviour based on version header.

@joeribekker joeribekker added enhancement and removed enhancement triage Issue needs to be validated. Remove this label if the issue considered valid. labels Dec 4, 2023
@joeribekker joeribekker added this to the Release 2.5.0 milestone Dec 4, 2023
@joeribekker
Copy link
Contributor Author

Discussion with Roxit. Sending the "bestandsomvang" attribute is required for their Documenten API in 1.1 and up. It is not like that in the standard... so meh.

@sergei-maertens
Copy link
Member

Standard allows for null, so can we just send that? In older versions this field is a calculated value, so Open Zaak will ignore it if its sent.

@SilviaAmAm
Copy link
Contributor

@sergei-maertens Unfortunately, when setting the value to null, the API still gives a 400 with this error:

Error converting value {null} to type \'System.Int64\'. Path \'bestandsomvang\', line 1, position 534.

😞

@sergei-maertens
Copy link
Member

Let's at least inform them that their implementation does not conform to the Documenten API 1.1 standard :)

@joeribekker
Copy link
Contributor Author

@SilviaAmAm I added tasks in the description. The idea is to add Version selection and that we sent the API-version header. This solves the immediate issue AND is a nice feature overall I think, that we can extend later.

If we sent the 1.0 version, everything works. If we sent the 1.1 version, bestandsgrootte is required (and it shouldn't) but anyway, we can then set it to 1.0 to make it work.

I already informed them and they created a bug report internally.

@SilviaAmAm
Copy link
Contributor

Discussed:

  • Add bestandsomvang field
  • Document compatibility of Documenten API only version 1.1+

@SilviaAmAm SilviaAmAm removed the blocked label Dec 8, 2023
sergei-maertens added a commit that referenced this issue Dec 8, 2023
@sergei-maertens
Copy link
Member

Re-opening since I'm not sure that's all there is to do, but feel free to close it if it's really done.

@CharString CharString mentioned this issue Dec 11, 2023
@joeribekker
Copy link
Contributor Author

Refinement: Silvia tested this and it worked without errors so this should wrap up the integration issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants