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

➕ [#120] changed drf-yasg to drf-spectacular #123

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

bart-maykin
Copy link
Contributor

@bart-maykin bart-maykin commented Dec 14, 2023

fixes #120

@bart-maykin bart-maykin force-pushed the feature/120-drf-spectacular branch 2 times, most recently from 1ceb16d to a61853a Compare December 19, 2023 11:44
@bart-maykin bart-maykin marked this pull request as ready for review December 19, 2023 11:47
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3f5d569) 96.62% compared to head (18dfd29) 96.96%.
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
+ Coverage   96.62%   96.96%   +0.33%     
==========================================
  Files         184      181       -3     
  Lines        7701     7731      +30     
==========================================
+ Hits         7441     7496      +55     
+ Misses        260      235      -25     

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

@joeribekker joeribekker self-requested a review January 10, 2024 12:10
Copy link
Contributor

@CharString CharString left a comment

Choose a reason for hiding this comment

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

Needs to be rebased and some changes.

And what is the story behind the JWT tokens?

bin/generate_schema_for_component.sh Outdated Show resolved Hide resolved
src/openklant/components/contactgegevens/api/urls.py Outdated Show resolved Hide resolved
Comment on lines +26 to +18
summary="Alle organisaties opvragen.",
description="Alle organisaties opvragen.",
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the value of a description with the exact same contents as the summary? description is an optional field that MAY contain a longer marked up description of the object it describes. I'd remove them all or fill them with the text from the reference VNG spec.

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 added the description so that the api spec is consistant with the legacy api specs. Because they also used the same description as summary if there wasn't additional information.

Comment on lines +38 to +37
path(
"jwtsecret/", CreateJWTSecretView.as_view(), name="jwtsecret-create"
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this replace something in the original
path("", include("vng_api_common.api.urls")),?
Or does this add new behaviour?
How does it relate to the JWTDummyAuthentication elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vng_api_common.api.urls only has one endpoint which we don't want to include in the schema, so I extended the class to add the extend_schema decorator so I could set exclude to True.

src/openklant/components/contactgegevens/api/urls.py Outdated Show resolved Hide resolved
Copy link
Contributor

@CharString CharString left a comment

Choose a reason for hiding this comment

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

django.conf.urls.url is deprecated and will be removed in Django 4.x, please replace with path or re_path

src/openklant/conf/api.py Show resolved Hide resolved
@bart-maykin bart-maykin force-pushed the feature/120-drf-spectacular branch 2 times, most recently from 60a2464 to af49c47 Compare January 18, 2024 14:58
@bart-maykin bart-maykin force-pushed the feature/120-drf-spectacular branch 2 times, most recently from 18dfd29 to 4ab07ce Compare January 23, 2024 15:55
@CharString CharString merged commit d0fb64a into master Jan 24, 2024
14 checks passed
@CharString CharString deleted the feature/120-drf-spectacular branch January 24, 2024 15:40
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.

Switch from yasg to spectacular
4 participants