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

Implement Objects API sets #4332

Closed
wants to merge 16 commits into from
Closed

Implement Objects API sets #4332

wants to merge 16 commits into from

Conversation

Viicos
Copy link
Contributor

@Viicos Viicos commented May 27, 2024

For #4267

Changes

This is best reviewed commit per commit.

  • Add a Objects API group model
  • The ObjectsAPIConfig model now holds a reference to a default group
  • Data migrations to update existing instances
  • Update the Objects API registration configuration to be able to specify a group to use (fallback to the default group)

Checklist

Check off the items that are completed or not relevant.

  • Impact on features

    • Checked copying a form
    • Checked import/export of a form
    • Config checks in the configuration overview admin page
    • Problem detection in the admin email digest is handled
  • Release management

    • I have labelled the PR as "needs-backport" accordingly
  • I have updated the translations assets (you do NOT need to provide translations)

    • Ran ./bin/makemessages_js.sh.sh
    • Ran ./bin/compilemessages_js.sh
  • Commit hygiene

    • Commit messages refer to the relevant Github issue
    • Commit messages explain the "why" of change, not the how

Tests still need to be added to check that an explicitly provided
group is being used.
It is assumed that the ZGW registration will use the default Objects API.
This is currently not configurable, but might be in the future.
It now takes a new `objects_api_group` parameter, similar to zgw.
`registration_backend` is now made required, as it doesn't really
make sense to have it optional.

Tests were adapted/refactored.
Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 94.35484% with 7 lines in your changes missing coverage. Please review.

Project coverage is 96.24%. Comparing base (69b95dc) to head (4e05ba2).
Report is 773 commits behind head on master.

Files with missing lines Patch % Lines
...rms/registrations/contrib/objects_api/api/views.py 77.77% 2 Missing and 2 partials ⚠️
src/openforms/registrations/api/filters.py 95.23% 0 Missing and 1 partial ⚠️
...nforms/registrations/contrib/objects_api/models.py 92.85% 1 Missing ⚠️
...nforms/registrations/contrib/objects_api/plugin.py 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4332      +/-   ##
==========================================
- Coverage   96.25%   96.24%   -0.02%     
==========================================
  Files         731      731              
  Lines       23741    23789      +48     
  Branches     2800     2807       +7     
==========================================
+ Hits        22853    22895      +42     
- Misses        617      620       +3     
- Partials      271      274       +3     

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

@Viicos Viicos marked this pull request as ready for review May 29, 2024 14:13
Comment on lines +502 to +503
objects_api_config = ObjectsAPIConfig.get_solo()
assert isinstance(objects_api_config, ObjectsAPIConfig)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed we use the default objects API group when registering to the Objects API from the ZGW registration.

Should we add support for group selection as well for ZGW?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, selection seems best and consistent, but this can be a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened #4344

@sergei-maertens sergei-maertens self-requested a review May 30, 2024 11:50
src/openforms/registrations/api/filters.py Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

this can be dropped entirely - and the zgw config/api group should get the same treatment.

@sergei-maertens
Copy link
Member

Closing to declutter :)

@Viicos Viicos deleted the feature/objects-api-sets branch June 17, 2024 10:20
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.

2 participants