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

Add Product to the ZGW API registration #4803

Merged
merged 8 commits into from
Nov 20, 2024

Conversation

robinmolen
Copy link
Contributor

@robinmolen robinmolen commented Oct 30, 2024

Closes #4796

Changes

Added product as a configuration option to the ZGW API registration. The product select gets populated from the selected case type. If there are multiple versions of the case type, then the case type version that is active at the moment of configuring is chosen.

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
    • 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

@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 3 times, most recently from 9e24349 to 135ff90 Compare October 30, 2024 16:16
Copy link

codecov bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 91.07143% with 5 lines in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (77e2cd6) to head (a50050b).
Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
src/openforms/contrib/zgw/products_and_services.py 75.00% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4803      +/-   ##
==========================================
- Coverage   96.58%   96.57%   -0.01%     
==========================================
  Files         748      749       +1     
  Lines       25477    25531      +54     
  Branches     3368     3376       +8     
==========================================
+ Hits        24606    24656      +50     
- Misses        608      612       +4     
  Partials      263      263              

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


🚨 Try these New Features:

@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 2 times, most recently from 63d8e50 to f3c7954 Compare October 31, 2024 08:57
@robinmolen robinmolen marked this pull request as ready for review October 31, 2024 09:19
@robinmolen robinmolen marked this pull request as draft November 8, 2024 13:27
@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 2 times, most recently from 1b0eee6 to 3574ee1 Compare November 12, 2024 15:30
@robinmolen robinmolen marked this pull request as ready for review November 12, 2024 15:32
@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 2 times, most recently from 03d0f12 to 4e229c2 Compare November 14, 2024 09:41
Copy link
Member

@sergei-maertens sergei-maertens left a comment

Choose a reason for hiding this comment

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

I'd suggest renaming product -> product_url everywhere so that it's clear we're dealing with a URL. Especially because we're moving literal URL-configuration options to their semantic 'description' and resolve URLs dynamically, I suspect we will go through this at some point too (where maybe a product slug/description will be stored instead of the exact URL).

docker/docker-compose.open-zaak.yml Outdated Show resolved Hide resolved
docker/open-zaak/app.py Outdated Show resolved Hide resolved
docker/open-zaak/app.py Outdated Show resolved Hide resolved
src/openforms/contrib/zgw/clients/catalogi.py Outdated Show resolved Hide resolved
src/openforms/contrib/zgw/clients/catalogi.py Outdated Show resolved Hide resolved
src/openforms/registrations/contrib/zgw_apis/api/views.py Outdated Show resolved Hide resolved
src/openforms/registrations/contrib/zgw_apis/options.py Outdated Show resolved Hide resolved
src/openforms/registrations/contrib/zgw_apis/typing.py Outdated Show resolved Hide resolved
@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 3 times, most recently from 1d5d05f to a280f43 Compare November 18, 2024 14:23
docker/docker-compose.rx-mission.yml Outdated Show resolved Hide resolved
docker/rx-mission/app.py Outdated Show resolved Hide resolved
docker/rx-mission/app.py Outdated Show resolved Hide resolved
src/openforms/contrib/zgw/clients/catalogi.py Outdated Show resolved Hide resolved
src/openforms/registrations/contrib/zgw_apis/api/views.py Outdated Show resolved Hide resolved
src/openforms/registrations/contrib/zgw_apis/plugin.py Outdated Show resolved Hide resolved
@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 2 times, most recently from 29da83e to fbd9f70 Compare November 20, 2024 13:13
products.append(
Product(
url=case_type_product["url"],
description=case_type_product["description"],
Copy link
Member

Choose a reason for hiding this comment

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

Can you address this type checker error? I would opt to make the key required in the product, if it's missing from the upstream API, you can default it to an empty string.

Copy link
Member

@sergei-maertens sergei-maertens left a comment

Choose a reason for hiding this comment

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

I'm not 100% happy with how the catalogueUrl now is passed around/calculated, but I won't let it block this PR since my #4606 UX ticket will touch that code anyway, so I'll play around with it in my future PR.

Once the type checking error is addressed, we can merge this.

@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch from fbd9f70 to bdc3ec4 Compare November 20, 2024 13:33
@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch 5 times, most recently from 4079598 to 73d4b26 Compare November 20, 2024 15:11
@robinmolen robinmolen force-pushed the feature/4796-zgw-configuration-select-product branch from 73d4b26 to a50050b Compare November 20, 2024 15:37
@robinmolen robinmolen merged commit 2c83208 into master Nov 20, 2024
32 of 34 checks passed
@robinmolen robinmolen deleted the feature/4796-zgw-configuration-select-product branch November 20, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants