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 get_schema_operation_parameters() for Open API 3 generateschema #1086

Merged

Conversation

n2ygk
Copy link
Contributor

@n2ygk n2ygk commented May 31, 2019

Fixes #1083

Implements Open API 3 (formerly known as Swagger) schema generation by upstream DRF 3.9+ generateschema management command.

@codecov-io
Copy link

codecov-io commented May 31, 2019

Codecov Report

Merging #1086 into master will decrease coverage by 0.23%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1086      +/-   ##
=========================================
- Coverage   98.34%   98.1%   -0.24%     
=========================================
  Files          15      15              
  Lines        1208    1216       +8     
=========================================
+ Hits         1188    1193       +5     
- Misses         20      23       +3
Impacted Files Coverage Δ
django_filters/rest_framework/backends.py 91.46% <62.5%> (-3.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c08dee...0aac3dd. Read the comment docs.

@n2ygk n2ygk changed the title Issue 1083/base filter backend add get_schema_operation_parameters() for Open API 3 generateschema May 31, 2019
@carltongibson carltongibson self-assigned this Jun 3, 2019
@carltongibson
Copy link
Owner

Hi @n2ygk. Super. Thanks for this.

Copy link
Contributor Author

@n2ygk n2ygk left a comment

Choose a reason for hiding this comment

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

@carltongibson If I change it to queryset = view.queryset, the only test that fails is this one, because the get_queryset() override is ignored:

def test_filterset_fields_list_with_bad_get_queryset(self):
"""
See:
* https://github.com/carltongibson/django-filter/issues/551
"""
class BadGetQuerySetView(FilterFieldsRootView):
filterset_fields = ['decimal', 'date']
def get_queryset(self):
raise AttributeError("I don't have that")
backend = DjangoFilterBackend()
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
fields = backend.get_schema_fields(BadGetQuerySetView())
self.assertEqual(fields, [], "get_schema_fields should handle AttributeError")

======================================================================
FAIL: test_filterset_fields_list_with_bad_get_queryset (tests.rest_framework.test_backends.GetSchemaFieldsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/alan/src/django-filter/tests/rest_framework/test_backends.py", line 150, in test_filterset_fields_list_with_bad_get_queryset
    self.assertEqual(fields, [], "get_schema_fields should handle AttributeError")
AssertionError: Lists differ: [Field(name='decimal', required=False, loc[266 chars]one)] != []

First list contains 2 additional elements.
First extra element 0:
Field(name='decimal', required=False, location='query', schema=<coreschema.schemas.Number object at 0x10e10a9b0>, description=None, type=None, example=None)

django_filters/rest_framework/backends.py Show resolved Hide resolved
@carltongibson
Copy link
Owner

“If I change...”

Why would you do that? 🙂

It’s just not right to access the queryset directly.

(That’s what I don’t follow...)

@n2ygk
Copy link
Contributor Author

n2ygk commented Jun 10, 2019

Why would you do that? 🙂

No good reason? Because it's there? Because it's named queryset not _queryset so it must be a "public" member? Because lots of bad examples out there like this: https://stackoverflow.com/questions/19707237/use-get-queryset-method-or-set-queryset-variable

@carltongibson
Copy link
Owner

I guess see rest_framework.generics.

tl;dr: Client code should always call get_queryset().

@n2ygk
Copy link
Contributor Author

n2ygk commented Jun 11, 2019

I guess see rest_framework.generics.

tl;dr: Client code should always call get_queryset().

Yep, learn something new every day. Sometimes for the second or third time. :-)

@n2ygk
Copy link
Contributor Author

n2ygk commented Jun 24, 2019

@carltongibson just pinging to see if you are OK with merging this?

Copy link
Owner

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Super. Thanks.

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.

get_schema_operation_parameters()
3 participants