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

[#2804] Update headers for requests with Haal Centraal API client #1438

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

pi-sigma
Copy link
Contributor

@pi-sigma pi-sigma commented Oct 14, 2024

We need to pass additional headers for requests made with the Haal Centraal API client (these are not required by the API itself, but needed for integration of other services in connection with the API, e.g. PinkRoccade):

  • x-origin-oin
  • x-doelbinding
  • x-verwerking
  • x-gebruiker

Taiga: https://taiga.maykinmedia.nl/project/open-inwoner/task/2804

See also the implementation in Open Forms: open-formulieren/open-forms#3731

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 95.45455% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.58%. Comparing base (df9656f) to head (788a34d).

Files with missing lines Patch % Lines
src/open_inwoner/haalcentraal/validators.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1438   +/-   ##
========================================
  Coverage    94.58%   94.58%           
========================================
  Files         1069     1071    +2     
  Lines        39623    39665   +42     
========================================
+ Hits         37478    37518   +40     
- Misses        2145     2147    +2     

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

@pi-sigma pi-sigma force-pushed the task/2804-haalcentraal-headers branch from 418c06a to 81a5765 Compare October 14, 2024 13:40
@pi-sigma pi-sigma changed the title [#2804] Add legacy headers to Haal Centraal API client [#2805] Update headers for requests with Haal Centraal API client Oct 14, 2024
@pi-sigma pi-sigma changed the title [#2805] Update headers for requests with Haal Centraal API client [#2804] Update headers for requests with Haal Centraal API client Oct 14, 2024
@pi-sigma pi-sigma marked this pull request as ready for review October 14, 2024 13:58
@pi-sigma pi-sigma requested a review from swrichards October 14, 2024 13:59
@pi-sigma
Copy link
Contributor Author

@alextreme I added a field to the Haal Centraal configuration for the x-verwerking header, as well, in conformity with the Open Forms implementation (open-formulieren/open-forms#3731). Is this useful, or do we know for sure that we don't need this?

@alextreme
Copy link
Member

@pi-sigma I'm not sure what the purpose of x-verwerking is, so having this working the same as OF seems like a smart idea

@pi-sigma pi-sigma force-pushed the task/2804-haalcentraal-headers branch from 81a5765 to 294c41b Compare October 14, 2024 14:14
src/open_inwoner/haalcentraal/tests/test_client.py Outdated Show resolved Hide resolved
"Content-Type": "application/json",
"Accept": "application/json",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this is taken care of by using the json kwarg to the post below, though I am not sure about the Accept (perhaps they also deal in XML?), so we might want to keep that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right about "Content-Type", this is handled by the requests library. The "Accept" header is not, so I'm keeping this.

@pi-sigma pi-sigma force-pushed the task/2804-haalcentraal-headers branch from 294c41b to 654f194 Compare October 15, 2024 09:01
    - add headers for 'doelbinding', 'origin-oin' , 'verwerking' and
      'gebruiker' to requests maded with version 2.1 of the Haal
      Centraal client
@pi-sigma pi-sigma force-pushed the task/2804-haalcentraal-headers branch from 654f194 to 788a34d Compare October 15, 2024 09:11
@pi-sigma pi-sigma requested a review from swrichards October 15, 2024 09:42
@alextreme
Copy link
Member

Okay, it may have been a bit more flexible if GebruikerZelf could also have been made configurable

@alextreme alextreme merged commit eb8ff60 into develop Oct 15, 2024
20 checks passed
@alextreme alextreme deleted the task/2804-haalcentraal-headers branch October 15, 2024 10:02
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.

4 participants