From 7532429b0bdf4521b9868018a6ee0d77c51c3906 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Mon, 28 Jan 2019 05:18:33 -0400 Subject: [PATCH] Refactor common generic views Add keyword arguments. Sort arguments. Unify the ObjectListPermissionFilterMixin and ObjectPermissionCheckMixin into the RestrictedQuerysetMixin. Add MultipleObjectDownloadView. Update SingleObjectDownloadView to do queryset filtering. The method that returns the base queryset for views is now named get_source_queryset(). The views now use .get_object_list as a multi object homologous of get_object. The queryset returned by .get_object_list is restricted by access. Make MultipleObjectMixin a subclass of Django's SingleObjectMixin to reduce repeated code. All generic views are now imported from common.generics and not from common.views. Signed-off-by: Roberto Rosario --- mayan/apps/common/generics.py | 218 ++++++++++++++++---------- mayan/apps/common/mixins.py | 184 ++++++++++++---------- mayan/apps/common/models.py | 2 +- mayan/apps/common/tests/test_views.py | 2 +- mayan/apps/common/views.py | 9 +- mayan/apps/common/widgets.py | 1 - 6 files changed, 237 insertions(+), 179 deletions(-) diff --git a/mayan/apps/common/generics.py b/mayan/apps/common/generics.py index f7afb7f1f8..bdd2f04e2a 100644 --- a/mayan/apps/common/generics.py +++ b/mayan/apps/common/generics.py @@ -27,29 +27,29 @@ from .icons import ( icon_sort_up ) from .literals import ( - TEXT_CHOICE_ITEMS, TEXT_CHOICE_LIST, TEXT_LIST_AS_ITEMS_VARIABLE_NAME, - TEXT_LIST_AS_ITEMS_PARAMETER, TEXT_SORT_FIELD_PARAMETER, - TEXT_SORT_FIELD_VARIABLE_NAME, TEXT_SORT_ORDER_CHOICE_ASCENDING, - TEXT_SORT_ORDER_PARAMETER, TEXT_SORT_ORDER_VARIABLE_NAME + TEXT_SORT_FIELD_PARAMETER, TEXT_SORT_FIELD_VARIABLE_NAME, + TEXT_SORT_ORDER_CHOICE_ASCENDING, TEXT_SORT_ORDER_PARAMETER, + TEXT_SORT_ORDER_VARIABLE_NAME ) from .mixins import ( DeleteExtraDataMixin, DynamicFormViewMixin, ExtraContextMixin, FormExtraKwargsMixin, ListModeMixin, MultipleObjectMixin, - ObjectActionMixin, ObjectListPermissionFilterMixin, ObjectNameMixin, - ObjectPermissionCheckMixin, RedirectionMixin, ViewPermissionCheckMixin + ObjectActionMixin, ObjectNameMixin, RedirectionMixin, + RestrictedQuerysetMixin, ViewPermissionCheckMixin ) from .settings import setting_paginate_by __all__ = ( - 'AssignRemoveView', 'ConfirmView', 'FormView', 'MultiFormView', - 'MultipleObjectConfirmActionView', 'MultipleObjectFormActionView', + 'AssignRemoveView', 'ConfirmView', 'FormView', + 'MultiFormView', 'MultipleObjectConfirmActionView', + 'MultipleObjectFormActionView', 'MultipleObjectDownloadView', 'SingleObjectCreateView', 'SingleObjectDeleteView', - 'SingleObjectDetailView', 'SingleObjectEditView', 'SingleObjectListView', - 'SimpleView' + 'SingleObjectDetailView', 'MultipleObjectDownloadView', + 'SingleObjectEditView', 'SingleObjectListView', 'SimpleView' ) -class AssignRemoveView(ExtraContextMixin, ViewPermissionCheckMixin, ObjectPermissionCheckMixin, TemplateView): +class AssignRemoveView(ExtraContextMixin, ViewPermissionCheckMixin, RestrictedQuerysetMixin, TemplateView): decode_content_type = False left_list_help_text = _( 'Select entries to be added. Hold Control to select multiple ' @@ -73,7 +73,7 @@ class AssignRemoveView(ExtraContextMixin, ViewPermissionCheckMixin, ObjectPermis def generate_choices(choices): results = [] for choice in choices: - ct = ContentType.objects.get_for_model(choice) + ct = ContentType.objects.get_for_model(model=choice) label = force_text(choice) results.append(('%s,%s' % (ct.model, choice.pk), '%s' % (label))) @@ -108,21 +108,21 @@ class AssignRemoveView(ExtraContextMixin, ViewPermissionCheckMixin, ObjectPermis def get(self, request, *args, **kwargs): self.unselected_list = ChoiceForm( - prefix=self.LEFT_LIST_NAME, choices=self.left_list(), - help_text=self.get_left_list_help_text() + choices=self.left_list(), help_text=self.get_left_list_help_text(), + prefix=self.LEFT_LIST_NAME ) self.selected_list = ChoiceForm( - prefix=self.RIGHT_LIST_NAME, choices=self.right_list(), + choices=self.right_list(), disabled_choices=self.get_disabled_choices(), - help_text=self.get_right_list_help_text() + help_text=self.get_right_list_help_text(), + prefix=self.RIGHT_LIST_NAME ) return self.render_to_response(self.get_context_data()) def process_form(self, prefix, items_function, action_function): if '%s-submit' % prefix in self.request.POST.keys(): form = ChoiceForm( - self.request.POST, prefix=prefix, - choices=items_function() + self.request.POST, choices=items_function(), prefix=prefix ) if form.is_valid(): @@ -156,12 +156,12 @@ class AssignRemoveView(ExtraContextMixin, ViewPermissionCheckMixin, ObjectPermis def post(self, request, *args, **kwargs): self.process_form( - prefix=self.LEFT_LIST_NAME, items_function=self.left_list, - action_function=self.add + action_function=self.add, items_function=self.left_list, + prefix=self.LEFT_LIST_NAME ) self.process_form( - prefix=self.RIGHT_LIST_NAME, items_function=self.right_list, - action_function=self.remove + action_function=self.remove, items_function=self.right_list, + prefix=self.RIGHT_LIST_NAME ) return self.get(request, *args, **kwargs) @@ -200,12 +200,12 @@ class AssignRemoveView(ExtraContextMixin, ViewPermissionCheckMixin, ObjectPermis return data -class ConfirmView(ObjectListPermissionFilterMixin, ObjectPermissionCheckMixin, ViewPermissionCheckMixin, ExtraContextMixin, RedirectionMixin, TemplateView): +class ConfirmView(RestrictedQuerysetMixin, ViewPermissionCheckMixin, ExtraContextMixin, RedirectionMixin, TemplateView): template_name = 'appearance/generic_confirm.html' def post(self, request, *args, **kwargs): self.view_action() - return HttpResponseRedirect(self.get_success_url()) + return HttpResponseRedirect(redirect_to=self.get_success_url()) class FormView(ViewPermissionCheckMixin, ExtraContextMixin, RedirectionMixin, FormExtraKwargsMixin, DjangoFormView): @@ -216,6 +216,62 @@ class DynamicFormView(DynamicFormViewMixin, FormView): pass +class DownloadViewBase(VirtualDownloadView): + TextIteratorIO = TextIteratorIO + VirtualFile = VirtualFile + + +class MultipleObjectDownloadView(RestrictedQuerysetMixin, MultipleObjectMixin, DownloadViewBase): + """ + View that support receiving multiple objects via a pk_list query. + """ + def __init__(self, *args, **kwargs): + result = super(MultipleObjectDownloadView, self).__init__(*args, **kwargs) + + if self.__class__.mro()[0].get_queryset != MultipleObjectDownloadView.get_queryset: + raise ImproperlyConfigured( + '%(cls)s is overloading the get_queryset method. Subclasses ' + 'should implement the get_source_queryset method instead. ' % { + 'cls': self.__class__.__name__ + } + ) + + return result + + def get_queryset(self): + try: + return super(MultipleObjectDownloadView, self).get_queryset() + except ImproperlyConfigured: + self.queryset = self.get_source_queryset() + return super(MultipleObjectDownloadView, self).get_queryset() + + +class SingleObjectDownloadView(RestrictedQuerysetMixin, SingleObjectMixin, DownloadViewBase): + """ + View that provides a .get_object() method to download content from a + single object. + """ + def __init__(self, *args, **kwargs): + result = super(SingleObjectDownloadView, self).__init__(*args, **kwargs) + + if self.__class__.mro()[0].get_queryset != SingleObjectDownloadView.get_queryset: + raise ImproperlyConfigured( + '%(cls)s is overloading the get_queryset method. Subclasses ' + 'should implement the get_source_queryset method instead. ' % { + 'cls': self.__class__.__name__ + } + ) + + return result + + def get_queryset(self): + try: + return super(SingleObjectDownloadView, self).get_queryset() + except ImproperlyConfigured: + self.queryset = self.get_source_queryset() + return super(SingleObjectDownloadView, self).get_queryset() + + class MultiFormView(DjangoFormView): prefix = None prefixes = {} @@ -238,7 +294,7 @@ class MultiFormView(DjangoFormView): self.all_forms_valid(forms) - return HttpResponseRedirect(self.get_success_url()) + return HttpResponseRedirect(redirect_to=self.get_success_url()) def forms_invalid(self, forms): return self.render_to_response(self.get_context_data(forms=forms)) @@ -298,12 +354,12 @@ class MultiFormView(DjangoFormView): forms = self.get_forms(form_classes) if all([form.is_valid() for form in forms.values()]): - return self.forms_valid(forms) + return self.forms_valid(forms=forms) else: - return self.forms_invalid(forms) + return self.forms_invalid(forms=forms) -class MultipleObjectFormActionView(ObjectActionMixin, MultipleObjectMixin, FormExtraKwargsMixin, ViewPermissionCheckMixin, ExtraContextMixin, RedirectionMixin, DjangoFormView): +class MultipleObjectFormActionView(ObjectActionMixin, ViewPermissionCheckMixin, RestrictedQuerysetMixin, MultipleObjectMixin, FormExtraKwargsMixin, ExtraContextMixin, RedirectionMixin, DjangoFormView): """ This view will present a form and upon receiving a POST request will perform an action on an object or queryset @@ -316,7 +372,7 @@ class MultipleObjectFormActionView(ObjectActionMixin, MultipleObjectMixin, FormE if self.__class__.mro()[0].get_queryset != MultipleObjectFormActionView.get_queryset: raise ImproperlyConfigured( '%(cls)s is overloading the get_queryset method. Subclasses ' - 'should implement the get_object_list method instead. ' % { + 'should implement the get_source_queryset method instead. ' % { 'cls': self.__class__.__name__ } ) @@ -331,16 +387,36 @@ class MultipleObjectFormActionView(ObjectActionMixin, MultipleObjectMixin, FormE try: return super(MultipleObjectFormActionView, self).get_queryset() except ImproperlyConfigured: - self.queryset = self.get_object_list() + self.queryset = self.get_source_queryset() return super(MultipleObjectFormActionView, self).get_queryset() -class MultipleObjectConfirmActionView(ObjectActionMixin, MultipleObjectMixin, ObjectListPermissionFilterMixin, ViewPermissionCheckMixin, ExtraContextMixin, RedirectionMixin, TemplateView): +class MultipleObjectConfirmActionView(ObjectActionMixin, ViewPermissionCheckMixin, RestrictedQuerysetMixin, MultipleObjectMixin, ExtraContextMixin, RedirectionMixin, TemplateView): template_name = 'appearance/generic_confirm.html' + def __init__(self, *args, **kwargs): + result = super(MultipleObjectConfirmActionView, self).__init__(*args, **kwargs) + + if self.__class__.mro()[0].get_queryset != MultipleObjectConfirmActionView.get_queryset: + raise ImproperlyConfigured( + '%(cls)s is overloading the get_queryset method. Subclasses ' + 'should implement the get_source_queryset method instead. ' % { + 'cls': self.__class__.__name__ + } + ) + + return result + + def get_queryset(self): + try: + return super(MultipleObjectConfirmActionView, self).get_queryset() + except ImproperlyConfigured: + self.queryset = self.get_source_queryset() + return super(MultipleObjectConfirmActionView, self).get_queryset() + def post(self, request, *args, **kwargs): self.view_action() - return HttpResponseRedirect(self.get_success_url()) + return HttpResponseRedirect(redirect_to=self.get_success_url()) class SimpleView(ViewPermissionCheckMixin, ExtraContextMixin, TemplateView): @@ -377,7 +453,7 @@ class SingleObjectCreateView(ObjectNameMixin, ViewPermissionCheckMixin, ExtraCon } messages.error( - request=self.request, message=error_message + message=error_message, request=self.request ) return super( SingleObjectCreateView, self @@ -402,13 +478,13 @@ class SingleObjectCreateView(ObjectNameMixin, ViewPermissionCheckMixin, ExtraCon context = self.get_context_data() messages.success( - self.request, - _( + message=_( '%(object)s created successfully.' - ) % {'object': self.get_object_name(context=context)} + ) % {'object': self.get_object_name(context=context)}, + request=self.request ) - return HttpResponseRedirect(self.get_success_url()) + return HttpResponseRedirect(redirect_to=self.get_success_url()) def get_error_message_duplicate(self): return self.error_message_duplicate @@ -418,7 +494,7 @@ class SingleObjectDynamicFormCreateView(DynamicFormViewMixin, SingleObjectCreate pass -class SingleObjectDeleteView(ObjectNameMixin, DeleteExtraDataMixin, ViewPermissionCheckMixin, ObjectPermissionCheckMixin, ExtraContextMixin, RedirectionMixin, DeleteView): +class SingleObjectDeleteView(ObjectNameMixin, DeleteExtraDataMixin, ViewPermissionCheckMixin, RestrictedQuerysetMixin, ExtraContextMixin, RedirectionMixin, DeleteView): template_name = 'appearance/generic_confirm.html' def __init__(self, *args, **kwargs): @@ -427,7 +503,7 @@ class SingleObjectDeleteView(ObjectNameMixin, DeleteExtraDataMixin, ViewPermissi if self.__class__.mro()[0].get_queryset != SingleObjectDeleteView.get_queryset: raise ImproperlyConfigured( '%(cls)s is overloading the get_queryset method. Subclasses ' - 'should implement the get_object_list method instead. ' % { + 'should implement the get_source_queryset method instead. ' % { 'cls': self.__class__.__name__ } ) @@ -443,20 +519,19 @@ class SingleObjectDeleteView(ObjectNameMixin, DeleteExtraDataMixin, ViewPermissi result = super(SingleObjectDeleteView, self).delete(request, *args, **kwargs) except Exception as exception: messages.error( - self.request, - _('%(object)s not deleted, error: %(error)s.') % { + message=_('%(object)s not deleted, error: %(error)s.') % { 'object': object_name, 'error': exception - } + }, request=self.request ) raise exception else: messages.success( - self.request, - _( + message=_( '%(object)s deleted successfully.' - ) % {'object': object_name} + ) % {'object': object_name}, + request=self.request ) return result @@ -470,11 +545,11 @@ class SingleObjectDeleteView(ObjectNameMixin, DeleteExtraDataMixin, ViewPermissi try: return super(SingleObjectDeleteView, self).get_queryset() except ImproperlyConfigured: - self.queryset = self.get_object_list() + self.queryset = self.get_source_queryset() return super(SingleObjectDeleteView, self).get_queryset() -class SingleObjectDetailView(ViewPermissionCheckMixin, ObjectPermissionCheckMixin, FormExtraKwargsMixin, ExtraContextMixin, ModelFormMixin, DetailView): +class SingleObjectDetailView(ViewPermissionCheckMixin, RestrictedQuerysetMixin, FormExtraKwargsMixin, ExtraContextMixin, ModelFormMixin, DetailView): template_name = 'appearance/generic_form.html' def __init__(self, *args, **kwargs): @@ -483,7 +558,7 @@ class SingleObjectDetailView(ViewPermissionCheckMixin, ObjectPermissionCheckMixi if self.__class__.mro()[0].get_queryset != SingleObjectDetailView.get_queryset: raise ImproperlyConfigured( '%(cls)s is overloading the get_queryset method. Subclasses ' - 'should implement the get_object_list method instead. ' % { + 'should implement the get_source_queryset method instead. ' % { 'cls': self.__class__.__name__ } ) @@ -499,36 +574,11 @@ class SingleObjectDetailView(ViewPermissionCheckMixin, ObjectPermissionCheckMixi try: return super(SingleObjectDetailView, self).get_queryset() except ImproperlyConfigured: - self.queryset = self.get_object_list() + self.queryset = self.get_source_queryset() return super(SingleObjectDetailView, self).get_queryset() -class SingleObjectDownloadView(ViewPermissionCheckMixin, ObjectPermissionCheckMixin, VirtualDownloadView, SingleObjectMixin): - TextIteratorIO = TextIteratorIO - VirtualFile = VirtualFile - - def __init__(self, *args, **kwargs): - result = super(SingleObjectDownloadView, self).__init__(*args, **kwargs) - - if self.__class__.mro()[0].get_queryset != SingleObjectDownloadView.get_queryset: - raise ImproperlyConfigured( - '%(cls)s is overloading the get_queryset method. Subclasses ' - 'should implement the get_object_list method instead. ' % { - 'cls': self.__class__.__name__ - } - ) - - return result - - def get_queryset(self): - try: - return super(SingleObjectDownloadView, self).get_queryset() - except ImproperlyConfigured: - self.queryset = self.get_object_list() - return super(SingleObjectDownloadView, self).get_queryset() - - -class SingleObjectEditView(ObjectNameMixin, ViewPermissionCheckMixin, ObjectPermissionCheckMixin, ExtraContextMixin, FormExtraKwargsMixin, RedirectionMixin, UpdateView): +class SingleObjectEditView(ObjectNameMixin, ViewPermissionCheckMixin, RestrictedQuerysetMixin, ExtraContextMixin, FormExtraKwargsMixin, RedirectionMixin, UpdateView): template_name = 'appearance/generic_form.html' def form_valid(self, form): @@ -552,24 +602,22 @@ class SingleObjectEditView(ObjectNameMixin, ViewPermissionCheckMixin, ObjectPerm self.object.save(**save_extra_data) except Exception as exception: messages.error( - self.request, - _('%(object)s not updated, error: %(error)s.') % { + message=_('%(object)s not updated, error: %(error)s.') % { 'object': object_name, 'error': exception - } + }, request=self.request ) return super( SingleObjectEditView, self ).form_invalid(form=form) else: messages.success( - self.request, - _( + message=_( '%(object)s updated successfully.' - ) % {'object': object_name} + ) % {'object': object_name}, request=self.request ) - return HttpResponseRedirect(self.get_success_url()) + return HttpResponseRedirect(redirect_to=self.get_success_url()) def get_object(self, queryset=None): obj = super(SingleObjectEditView, self).get_object(queryset=queryset) @@ -585,7 +633,7 @@ class SingleObjectDynamicFormEditView(DynamicFormViewMixin, SingleObjectEditView pass -class SingleObjectListView(ListModeMixin, PaginationMixin, ViewPermissionCheckMixin, ObjectListPermissionFilterMixin, ExtraContextMixin, RedirectionMixin, ListView): +class SingleObjectListView(ListModeMixin, PaginationMixin, ViewPermissionCheckMixin, RestrictedQuerysetMixin, ExtraContextMixin, RedirectionMixin, ListView): template_name = 'appearance/generic_list.html' def __init__(self, *args, **kwargs): @@ -594,7 +642,7 @@ class SingleObjectListView(ListModeMixin, PaginationMixin, ViewPermissionCheckMi if self.__class__.mro()[0].get_queryset != SingleObjectListView.get_queryset: raise ImproperlyConfigured( '%(cls)s is overloading the get_queryset method. Subclasses ' - 'should implement the get_object_list method instead. ' % { + 'should implement the get_source_queryset method instead. ' % { 'cls': self.__class__.__name__ } ) @@ -635,7 +683,7 @@ class SingleObjectListView(ListModeMixin, PaginationMixin, ViewPermissionCheckMi try: queryset = super(SingleObjectListView, self).get_queryset() except ImproperlyConfigured: - self.queryset = self.get_object_list() + self.queryset = self.get_source_queryset() queryset = super(SingleObjectListView, self).get_queryset() self.field_name = self.get_sort_field() diff --git a/mayan/apps/common/mixins.py b/mayan/apps/common/mixins.py index 14ef15a44e..9de85100e1 100644 --- a/mayan/apps/common/mixins.py +++ b/mayan/apps/common/mixins.py @@ -4,11 +4,11 @@ from django.conf import settings from django.contrib import messages from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ImproperlyConfigured, PermissionDenied -from django.db.models.query import QuerySet from django.http import Http404, HttpResponseRedirect from django.shortcuts import get_object_or_404, resolve_url from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext +from django.views.generic.detail import SingleObjectMixin from mayan.apps.acls.models import AccessControlList from mayan.apps.permissions import Permission @@ -23,9 +23,8 @@ from .literals import ( __all__ = ( 'DeleteExtraDataMixin', 'DynamicFormViewMixin', 'ExtraContextMixin', 'FormExtraKwargsMixin', 'ListModeMixin', 'MultipleObjectMixin', - 'ObjectActionMixin', 'ObjectListPermissionFilterMixin', 'ObjectNameMixin', - 'ObjectPermissionCheckMixin', 'RedirectionMixin', - 'ViewPermissionCheckMixin' + 'ObjectActionMixin', 'ObjectNameMixin', 'RedirectionMixin', + 'RestrictedQuerysetMixin', 'ViewPermissionCheckMixin' ) @@ -204,39 +203,48 @@ class MultipleInstanceActionMixin(object): return HttpResponseRedirect(redirect_to=self.get_success_url()) -class MultipleObjectMixin(object): +class MultipleObjectMixin(SingleObjectMixin): """ Mixin that allows a view to work on a single or multiple objects. It can receive a pk, a slug or a list of IDs via an id_list query. The pk, slug, and ID list parameter name can be changed using the attributes: pk_url_kwargs, slug_url_kwarg, and pk_list_key. """ - model = None - object_permission = None pk_list_key = 'id_list' pk_list_separator = PK_LIST_SEPARATOR - pk_url_kwarg = 'pk' - queryset = None - slug_url_kwarg = 'slug' - def get_pk_list(self): - result = self.request.GET.get( - self.pk_list_key, self.request.POST.get(self.pk_list_key) - ) + def get(self, request, *args, **kwargs): + """ + Override BaseDetailView.get() + """ + return super(SingleObjectMixin, self).get(request, *args, **kwargs) - if result: - return result.split(self.pk_list_separator) - else: - return None + def get_context_data(self, **kwargs): + """ + Override SingleObjectMixin.get_context_data() + """ + return super(SingleObjectMixin, self).get_context_data(**kwargs) - def get_queryset(self): - if self.queryset is not None: - queryset = self.queryset - if isinstance(queryset, QuerySet): - queryset = queryset.all() - elif self.model is not None: - queryset = self.model._default_manager.all() + def get_object(self): + """ + Remove this method from the subclass + """ + raise AttributeError + def get_object_list(self, queryset=None): + """ + Returns the list of objects the view is displaying. + + By default this requires `self.queryset` and a `pk`, `slug` ro + `pk_list' argument in the URLconf, but subclasses can override this + to return any object. + """ + # Use a custom queryset if provided; this is required for subclasses + # like DateDetailView + if queryset is None: + queryset = self.get_queryset() + + # Next, try looking up by primary key. pk = self.kwargs.get(self.pk_url_kwarg) slug = self.kwargs.get(self.slug_url_kwarg) pk_list = self.get_pk_list() @@ -252,21 +260,37 @@ class MultipleObjectMixin(object): if pk_list is not None: queryset = queryset.filter(pk__in=self.get_pk_list()) + # If none of those are defined, it's an error. if pk is None and slug is None and pk_list is None: raise AttributeError( - 'Generic detail view %s must be called with ' - 'either an object pk, a slug or an id list.' + 'View %s must be called with ' + 'either an object pk, a slug or an pk list.' % self.__class__.__name__ ) - if self.object_permission: - return AccessControlList.objects.restrict_queryset( - permission=self.object_permission, queryset=queryset, - user=self.request.user + try: + # Get the single item from the filtered queryset + queryset.get() + except queryset.model.MultipleObjectsReturned: + # Queryset has more than one item, this is good. + return queryset + except queryset.model.DoesNotExist: + raise Http404( + _('No %(verbose_name)s found matching the query') % + {'verbose_name': queryset.model._meta.verbose_name} ) else: + # Queryset has one item, this is good. return queryset + def get_pk_list(self): + result = self.request.GET.get(self.pk_list_key) + + if result: + return result.split(self.pk_list_separator) + else: + return None + class ObjectActionMixin(object): """ @@ -311,36 +335,6 @@ class ObjectActionMixin(object): ) -class ObjectListPermissionFilterMixin(object): - """ - access_object_retrieve_method is used to have the entire view check - against an object permission and not the individual secondary items. - """ - access_object_retrieve_method = None - object_permission = None - - def dispatch(self, request, *args, **kwargs): - if self.access_object_retrieve_method and self.object_permission: - AccessControlList.objects.check_access( - obj=getattr(self, self.access_object_retrieve_method)(), - permissions=(self.object_permission,), user=request.user - ) - return super(ObjectListPermissionFilterMixin, self).dispatch( - request, *args, **kwargs - ) - - def get_queryset(self): - queryset = super(ObjectListPermissionFilterMixin, self).get_queryset() - - if not self.access_object_retrieve_method and self.object_permission: - return AccessControlList.objects.restrict_queryset( - permission=self.object_permission, queryset=queryset, - user=self.request.user - ) - else: - return queryset - - class ObjectNameMixin(object): def get_object_name(self, context=None): if not context: @@ -357,26 +351,6 @@ class ObjectNameMixin(object): return object_name -class ObjectPermissionCheckMixin(object): - """ - Filter the queryset of the view by the `object_permission` provided. - If no `object_permission` is provide the queryset will be returned - as is. - """ - object_permission = None - - def get_queryset(self): - queryset = super(ObjectPermissionCheckMixin, self).get_queryset() - - if self.object_permission: - return AccessControlList.objects.restrict_queryset( - permission=self.object_permission, queryset=queryset, - user=self.request.user - ) - - return queryset - - class RedirectionMixin(object): action_cancel_redirect = None post_action_redirect = None @@ -425,14 +399,56 @@ class RedirectionMixin(object): return self.next_url or self.previous_url +class RestrictedQuerysetMixin(object): + """ + Restrict the view's queryset against a permission via ACL checking. + Used to restrict the object list of a multiple object view or the source + queryset of the .get_object() method. + """ + model = None + object_permission = None + source_queryset = None + + def get_source_queryset(self): + if self.source_queryset is None: + if self.model: + return self.model._default_manager.all() + else: + raise ImproperlyConfigured( + "%(cls)s is missing a QuerySet. Define " + "%(cls)s.model, %(cls)s.source_queryset, or override " + "%(cls)s.get_source_queryset()." % { + 'cls': self.__class__.__name__ + } + ) + + return self.source_queryset.all() + + def get_queryset(self): + queryset = self.get_source_queryset() + + if self.object_permission: + return AccessControlList.objects.restrict_queryset( + permission=self.object_permission, queryset=queryset, + user=self.request.user + ) + else: + return queryset + + class ViewPermissionCheckMixin(object): + """ + Restrict access to the view based on the user's direct permissions from + roles. This mixing is used for views whose objects don't support ACLs or + for views that perform actions that are not related to a specify object or + object's permission like maintenance views. + """ view_permission = None def dispatch(self, request, *args, **kwargs): if self.view_permission: - Permission.check_permissions( - permissions=(self.view_permission,), - requester=self.request.user + Permission.check_user_permission( + permission=self.view_permission, user=self.request.user ) return super( diff --git a/mayan/apps/common/models.py b/mayan/apps/common/models.py index 148c3b1c98..de23671441 100644 --- a/mayan/apps/common/models.py +++ b/mayan/apps/common/models.py @@ -18,7 +18,7 @@ from .storages import storage_sharedupload logger = logging.getLogger(__name__) -#TODO: move outside of models.py +# TODO: move outside of models.py or as a static method of SharedUploadedFile def upload_to(instance, filename): return 'shared-file-{}'.format(uuid.uuid4().hex) diff --git a/mayan/apps/common/tests/test_views.py b/mayan/apps/common/tests/test_views.py index 2295e08fd4..865c7d782e 100644 --- a/mayan/apps/common/tests/test_views.py +++ b/mayan/apps/common/tests/test_views.py @@ -21,7 +21,7 @@ class CommonViewTestCase(GenericViewTestCase): def _create_error_log_entry(self): ModelPermission.register( - model=get_user_model(), permissions=(permission_error_log_view,) + model=get_user_model(), permission=permission_error_log_view ) ErrorLogEntry.objects.register(model=get_user_model()) diff --git a/mayan/apps/common/views.py b/mayan/apps/common/views.py index ca5e47fd2c..f7fc56e2ae 100644 --- a/mayan/apps/common/views.py +++ b/mayan/apps/common/views.py @@ -14,7 +14,6 @@ from django.utils.http import urlencode from django.utils.translation import ugettext_lazy as _ from django.views.generic import RedirectView, TemplateView -from mayan.apps.acls.models import AccessControlList from mayan.apps.common.mixins import ( ContentTypeViewMixin, ExternalObjectMixin ) @@ -24,12 +23,8 @@ from .forms import ( LicenseForm, LocaleProfileForm, LocaleProfileForm_view, PackagesLicensesForm ) -from .generics import ( # NOQA - AssignRemoveView, ConfirmView, FormView, MultiFormView, - MultipleObjectConfirmActionView, MultipleObjectFormActionView, SimpleView, - SingleObjectCreateView, SingleObjectDeleteView, SingleObjectDetailView, - SingleObjectDownloadView, SingleObjectDynamicFormCreateView, - SingleObjectDynamicFormEditView, SingleObjectEditView, SingleObjectListView +from .generics import ( + ConfirmView, SimpleView, SingleObjectEditView, SingleObjectListView ) from .icons import icon_object_error_list, icon_setup from .menus import menu_setup, menu_tools diff --git a/mayan/apps/common/widgets.py b/mayan/apps/common/widgets.py index 518b8ccda4..c6fe2d5851 100644 --- a/mayan/apps/common/widgets.py +++ b/mayan/apps/common/widgets.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals from django import forms from django.template import Context, Template from django.utils.encoding import force_text -from django.utils.html import format_html from django.utils.safestring import mark_safe from .icons import icon_fail as default_icon_fail