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

make sure all ListModelMixin views considered as list view #306

Merged
merged 3 commits into from Mar 31, 2019

Conversation

DimasInchidi
Copy link
Contributor

so i realize this weird behavior because i have a view that inherit generics.ListCreateAPIView and rendered swagger doesn't add pagination to the response sample for that view. Found that its because the URL end with parameter.

TLDR:
class GroupMemberListAPIView(generics.ListCreateAPIView) +
path('connections/group/<int:group_id>/', GroupMemberListAPIView.as_view()) = not considered as list view

@axnsan12
Copy link
Owner

axnsan12 commented Feb 5, 2019

Makes sense, thanks.

It would be great if you would also add a test for this somewhere, to show the intention and prevent future regressions.

@DimasInchidi
Copy link
Contributor Author

sure, then i'll need to add new view and the url. i'll add endpoint that retrieve list of all objects that related to certain user with url path('user/model/<int:user_id>/', views.ModelList.as_view()) and for the view, should i add it to snippets since it will use generics or to articles since it will need to have pagination_class?

@axnsan12
Copy link
Owner

axnsan12 commented Feb 5, 2019

You can add it anywhere you want, there's no real structure to the test project view, most of them don't even work.

@DimasInchidi
Copy link
Contributor Author

@axnsan12 added the test for this fix, please check again. thank you

@axnsan12
Copy link
Owner

Looks OK, thanks!

@DimasInchidi
Copy link
Contributor Author

@axnsan12 is this issue already covered by your last commit?
if yes, i'll close this PR, or do you want to keep test created in this PR?

@axnsan12 axnsan12 closed this Mar 29, 2019
@axnsan12 axnsan12 reopened this Mar 29, 2019
@axnsan12
Copy link
Owner

The last commit was targeted at #330. I'll be meeting this when I get the chance to rebase it on the latest master (or if you can do that).

Sorry for being so slow about this.

@axnsan12
Copy link
Owner

I meant #331

@DimasInchidi DimasInchidi reopened this Mar 29, 2019
@DimasInchidi
Copy link
Contributor Author

trigger too many tests, please re-run it. thanks

@axnsan12 axnsan12 merged commit 86c1675 into axnsan12:master Mar 31, 2019
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.

None yet

2 participants