zgw-consumers refactor #38

SonnyBA · 2024-10-24T14:41:18Z

Fixes #7
Partially fixes maykinmedia/open-api-framework#66

Upgrading zgw-consumers to ensure we can upgrade zgw consumers in Open Zaak: open-zaak/open-zaak#1800

docs/ref/client.rst

setup.cfg

vng_api_common/migrations/0006_delete_apicredential.py

vng_api_common/models.py

vng_api_common/oas.py

vng_api_common/models.py

vng_api_common/notifications/models.py

vng_api_common/utils.py

SonnyBA · 2024-10-25T08:39:14Z

Also tackles #7

notifications-api-common has changed its underlying client to ape_pie.APIClient due to a zgw-consumers update (maykinmedia/notifications-api-common#15)

To make them match with notifications-api-common>=0.3.0

This can be useful for Openzaak instances where these paths are used

These definitions are included in `notifications-api-common`

AuthorizationsConfig model Removed models should be imported from notifications-api-common

AuthorizationsConfig

codecov-commenter · 2024-11-08T09:40:45Z

Codecov Report

Attention: Patch coverage is 62.13592% with 39 lines in your changes missing coverage. Please review.

Project coverage is 55.51%. Comparing base (9563533) to head (d73f24c).
Report is 30 commits behind head on main.

Files with missing lines	Patch %	Lines
vng_api_common/client.py	53.84%	16 Missing and 2 partials ⚠️
vng_api_common/authorizations/middleware.py	16.66%	10 Missing ⚠️
vng_api_common/notifications/handlers.py	0.00%	6 Missing ⚠️
vng_api_common/routers.py	0.00%	3 Missing ⚠️
vng_api_common/mocks.py	0.00%	1 Missing ⚠️
vng_api_common/views.py	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
+ Coverage   54.95%   55.51%   +0.55%     
==========================================
  Files          79       80       +1     
  Lines        3632     3599      -33     
  Branches      590      470     -120     
==========================================
+ Hits         1996     1998       +2     
+ Misses       1533     1499      -34     
+ Partials      103      102       -1

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

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

vng_api_common/migrations/0006_delete_apicredential.py

SonnyBA

Github won't let me approve (because I'm the owner of the PR), but this looks good 👍

docs/ref/client.rst

vng_api_common/models.py

annashamray · 2024-11-11T16:15:47Z

vng_api_common/models.py

-        return auth
-
-
-class ClientConfig(SingletonModel):


Why is this abstract model deleted? I'm pretty sure it's used in Open Zaak. Maybe just change api_root to be a FK to Service model instead of the url

I've replaced the usage of it in Open Zaak with a regular SingletonModel with a service, it feels a bit unnecessary to have an abstract model that only contains a FK to Service imo

vng_api_common/notifications/constants.py

vng_api_common/notifications/models.py

vng_api_common/validators.py

vng_api_common/utils.py

vng_api_common/oas.py

annashamray · 2024-11-11T16:38:17Z

vng_api_common/authorizations/models.py

+        blank=True,
+        null=True,


I do think it should not be nullable.

I've added a migration to remove null=True

Hmm upon second thought, this does make calling .get_solo() problematic (in tests for example), because you cannot add a default value for this, so we might have to keep it nullable :/

Used for the ResourceValidator, which verifies if objects have the correct shape as defined in an OAS.

SonnyBA · 2024-11-13T11:49:47Z

@stevenbal I added 44a4032 today, as I noticed the configuration view was broken (incorrectly stated services were reachable) by the changes made in this PR. They were not picked up by the tests though, so I also added/updated some tests.

annashamray

Looks good!

setup.cfg

vng_api_common/views.py

Handle duplicate service slugs

this checked for vng_api_common.notifications, but should check notifications_api_common since the notifications code moved there

SonnyBA requested a review from stevenbal October 24, 2024 14:41

SonnyBA self-assigned this Oct 24, 2024

SonnyBA mentioned this pull request Oct 24, 2024

🚧 [maykinmedia/django-setup-configuration#16] Setup config fixes open-zaak/open-notificaties#193

Closed

1 task

stevenbal requested changes Oct 25, 2024

View reviewed changes

stevenbal and others added 25 commits October 25, 2024 17:04

👽 Update NotificationsConfig client usage

c55cae7

notifications-api-common has changed its underlying client to ape_pie.APIClient due to a zgw-consumers update (maykinmedia/notifications-api-common#15)

Update project requirements

dffe1e6

To make them match with notifications-api-common>=0.3.0

Add ape-pie & zgw-consumers requirements

1c8c547

Use to do external API calls

d1f6a09

Remove APICredentials model

f24ee8f

Remove zds_client usage

e131840

Run formatting

15e7d3b

Update tox configuration

4a628a1

Update documentation

d201498

Add reverse migration path

ab92b11

Update docs

b93738e

Update CI configuration

c79f786

Describe APICredentials to Service model migration

8cd3bea

Add missing field for Service's

fd16c37

Try to determine api_type from api_root

4933a19

This can be useful for Openzaak instances where these paths are used

Reuse client.auth to retrieve token

82786bf

Remove oas related validators

726b28a

Fix build_client usage

0649f9c

Remove request_object_attribute

4794536

Fix import error

9e3498a

Remove notifications-api-common related definitions

9760dbb

These definitions are included in `notifications-api-common`

Remove notifications-api-common related models & update

e35bfbd

AuthorizationsConfig model Removed models should be imported from notifications-api-common

Fix migrations, use simple get_client function, migrate

417ab78

AuthorizationsConfig

Update JWTAuth error handling

91744ee

Fix import errors

f8857cc

✅ Add migration tests for service migrations

f3fe4ee

stevenbal force-pushed the feature/update-notifs-client branch from 23015e8 to f3fe4ee Compare November 8, 2024 10:21

stevenbal reviewed Nov 8, 2024

View reviewed changes

vng_api_common/migrations/0006_delete_apicredential.py Show resolved Hide resolved

stevenbal marked this pull request as ready for review November 8, 2024 10:40

stevenbal requested review from annashamray and Coperh November 8, 2024 13:15

SonnyBA commented Nov 11, 2024

View reviewed changes

annashamray requested changes Nov 11, 2024

View reviewed changes

SonnyBA added 2 commits November 12, 2024 12:29

Add zgw-consumers-oas to requirements

120062f

Used for the ResourceValidator, which verifies if objects have the correct shape as defined in an OAS.

call raise_for_status when testing AC & NRC connections

44a4032

stevenbal requested a review from annashamray November 14, 2024 13:51

stevenbal added 2 commits November 14, 2024 15:39

👌 [maykinmedia/open-api-framework#66] PR feedback

4517d90

➕ Add missing requests-mock dependency

5d331a6

stevenbal force-pushed the feature/update-notifs-client branch from 6bd8cfe to 5d331a6 Compare November 14, 2024 15:23

SonnyBA added 2 commits November 18, 2024 10:29

Handle duplicate service slugs

df70763

Apply formatting

d83ca95

annashamray approved these changes Nov 18, 2024

View reviewed changes

annashamray requested changes Nov 19, 2024

View reviewed changes

setup.cfg Outdated Show resolved Hide resolved

Move zgw-consumers-oas to test requirements

b420445

annashamray approved these changes Nov 19, 2024

View reviewed changes

annashamray reviewed Nov 20, 2024

View reviewed changes

vng_api_common/views.py Show resolved Hide resolved

stevenbal and others added 2 commits November 22, 2024 10:05

Merge pull request #40 from maykinmedia/slug-migrations

e7ad23a

Handle duplicate service slugs

🐛 Check for correct installed notifs app in nrc config check

d73f24c

this checked for vng_api_common.notifications, but should check notifications_api_common since the notifications code moved there

stevenbal self-requested a review November 22, 2024 10:18

stevenbal approved these changes Nov 22, 2024

View reviewed changes

stevenbal merged commit 9e81e1e into main Nov 22, 2024
6 checks passed

stevenbal deleted the feature/update-notifs-client branch November 22, 2024 10:22

SonnyBA mentioned this pull request Nov 22, 2024

🔖 Release version 2.0.0 #41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zgw-consumers refactor #38

zgw-consumers refactor #38

SonnyBA commented Oct 24, 2024 •

edited by stevenbal

Loading

SonnyBA commented Oct 25, 2024

codecov-commenter commented Nov 8, 2024 •

edited

Loading

SonnyBA left a comment

annashamray Nov 11, 2024

stevenbal Nov 14, 2024 •

edited

Loading

annashamray Nov 11, 2024

stevenbal Nov 14, 2024

stevenbal Nov 14, 2024 •

edited

Loading

SonnyBA commented Nov 13, 2024

annashamray left a comment

zgw-consumers refactor #38

zgw-consumers refactor #38

Conversation

SonnyBA commented Oct 24, 2024 • edited by stevenbal Loading

SonnyBA commented Oct 25, 2024

codecov-commenter commented Nov 8, 2024 • edited Loading

Codecov Report

SonnyBA left a comment

Choose a reason for hiding this comment

annashamray Nov 11, 2024

Choose a reason for hiding this comment

stevenbal Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

annashamray Nov 11, 2024

Choose a reason for hiding this comment

stevenbal Nov 14, 2024

Choose a reason for hiding this comment

stevenbal Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

SonnyBA commented Nov 13, 2024

annashamray left a comment

Choose a reason for hiding this comment

SonnyBA commented Oct 24, 2024 •

edited by stevenbal

Loading

codecov-commenter commented Nov 8, 2024 •

edited

Loading

stevenbal Nov 14, 2024 •

edited

Loading

stevenbal Nov 14, 2024 •

edited

Loading