Skip to content

Add a mixin class for dashboard views on models with project relations #2353

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

Merged
merged 2 commits into from
Aug 9, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions readthedocs/projects/views/mixins.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""Mixin classes for project views"""

from django.shortcuts import get_object_or_404

from readthedocs.projects.models import Project


class ProjectRelationMixin(object):

"""Mixin class for constructing model views for project dashboard

This mixin class is used for model views on models that have a relation
to the :py:cls:`Project` model.

:cvar project_lookup_url_kwarg: URL kwarg to use in project lookup
:cvar project_lookup_field: Query field for project relation
:cvar project_context_object_name: Context object name for project
"""

project_lookup_url_kwarg = 'project_slug'
project_lookup_field = 'project'
project_context_object_name = 'project'

def get_project_queryset(self):
return Project.objects.for_admin_user(user=self.request.user)

def get_project(self):
if self.project_lookup_url_kwarg not in self.kwargs:
return None
return get_object_or_404(
self.get_project_queryset(),
slug=self.kwargs[self.project_lookup_url_kwarg]
)

def get_queryset(self):
return self.model.objects.filter(
**{self.project_lookup_field: self.get_project()}
)

def get_context_data(self, **kwargs):
context = {}
Copy link
Member

Choose a reason for hiding this comment

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

If we need the try...except block, I'd say move context = {} to

...
except AttributeError:
    context = {}

try:
context = super(ProjectRelationMixin, self).get_context_data(**kwargs)
except AttributeError:
Copy link
Member

Choose a reason for hiding this comment

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

Is this check really needed? get_context_data() comes from ContentMixin and it's defined in almost all class based views except View and RedirectView.

Copy link
Contributor Author

@agjohnson agjohnson Aug 8, 2016

Choose a reason for hiding this comment

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

Only because this was written as a mixin class independent of any base class. But I think perhaps basing this class on ContextMixin is the most correct thing to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, MRO here means the ContextMixin.get_context_data returns context data, but this should instead be able to fallback to vanilla view's get_context_data. I altered the call based on this feedback to instead expect get_context_data to always be implemented on a super class.

pass
context[self.project_context_object_name] = self.get_project()
return context
27 changes: 26 additions & 1 deletion readthedocs/rtd_tests/tests/test_project_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@
from readthedocs.rtd_tests.base import (WizardTestCase, MockBuildTestCase,
RequestFactoryTestMixin)
from readthedocs.projects.exceptions import ProjectSpamError
from readthedocs.projects.models import Project
from readthedocs.projects.models import Project, Domain
from readthedocs.projects.views.private import ImportWizardView
from readthedocs.projects.views.mixins import ProjectRelationMixin


@patch('readthedocs.projects.views.private.trigger_build', lambda x, basic: None)
Expand Down Expand Up @@ -330,3 +331,27 @@ def test_delete_project(self):
remove_dir.apply_async.assert_called_with(
queue='celery',
args=[project.doc_path])


class TestPrivateMixins(MockBuildTestCase):

def setUp(self):
self.project = get(Project, slug='kong')
self.domain = get(Domain, project=self.project)

def test_project_relation(self):
"""Class using project relation mixin class"""

class FoobarView(ProjectRelationMixin):
model = Domain

def get_project_queryset(self):
# Don't test this as a view with a request.user
return Project.objects.all()


view = FoobarView()
view.kwargs = {'project_slug': 'kong'}
self.assertEqual(view.get_project(), self.project)
self.assertEqual(view.get_queryset().first(), self.domain)
self.assertEqual(view.get_context_data(), {'project': self.project})