Clean up organization mixins

judge/forms.py

@@ -636,7 +636,6 @@ def clean(self) -> None:
 
 class BlogPostForm(ModelForm):
     def __init__(self, *args, **kwargs):
-        kwargs.pop('org_pk', None)
         self.user = kwargs.pop('user', None)
         super(BlogPostForm, self).__init__(*args, **kwargs)
 

judge/views/organization.py

@@ -1,3 +1,5 @@
+from functools import cached_property
+
 from django import forms
 from django.conf import settings
 from django.contrib import messages
@@ -8,7 +10,7 @@
 from django.db.models.expressions import F, Value
 from django.db.models.functions import Coalesce
 from django.forms import Form, modelformset_factory
-from django.http import Http404, HttpResponsePermanentRedirect, HttpResponseRedirect
+from django.http import Http404, HttpResponseRedirect
 from django.shortcuts import get_object_or_404, render
 from django.urls import reverse
 from django.utils import timezone
@@ -36,59 +38,103 @@
 
 
 class OrganizationMixin(object):
-    context_object_name = 'organization'
     model = Organization
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
-        context['logo_override_image'] = self.object.logo_override_image
-        context['meta_description'] = self.object.about[:settings.DESCRIPTION_MAX_LENGTH]
+        context['organization'] = self.organization
+        context['logo_override_image'] = self.organization.logo_override_image
+        context['meta_description'] = self.organization.about[:settings.DESCRIPTION_MAX_LENGTH]
         return context
 
+    @cached_property
+    def organization(self):
+        return get_object_or_404(Organization, slug=self.kwargs['slug'])
+
     def dispatch(self, request, *args, **kwargs):
+        if 'slug' not in kwargs:
+            raise ImproperlyConfigured('Must pass a slug')
+
         try:
+            self.object = self.organization
             return super(OrganizationMixin, self).dispatch(request, *args, **kwargs)
         except Http404:
-            key = kwargs.get(self.slug_url_kwarg, None)
-            if key:
+            slug = kwargs.get('slug', None)
+            if slug:
                 return generic_message(request, _('No such organization'),
-                                       _('Could not find an organization with the key "%s".') % key)
+                                       _('Could not find an organization with the key "%s".') % slug)
             else:
                 return generic_message(request, _('No such organization'),
                                        _('Could not find such organization.'))
 
     def can_edit_organization(self, org=None):
         if org is None:
-            org = self.object
+            org = self.organization
         if not self.request.user.is_authenticated:
             return False
         return org.is_admin(self.request.profile) or self.request.user.has_perm('judge.edit_all_organization')
 
 
-class BaseOrganizationListView(OrganizationMixin, ListView):
-    model = None
-    context_object_name = None
-    slug_url_kwarg = 'slug'
+# Use this mixin to mark a view is public for all users, including non-members
+class PublicOrganizationMixin(OrganizationMixin):
+    pass
 
-    def get_object(self):
-        return get_object_or_404(Organization, slug=self.kwargs.get('slug'))
 
-    def get_context_data(self, **kwargs):
-        return super().get_context_data(organization=self.object, **kwargs)
+# Use this mixin to mark a view is private for members only
+class PrivateOrganizationMixin(OrganizationMixin):
+    # If the user has at least one of the following permissions,
+    # they can access the private data even if they are not in the org
+    permission_bypass = []
 
-    def get(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        return super().get(request, *args, **kwargs)
+    # Override this method to customize the permission check
+    def can_access_this_view(self):
+        if self.request.user.is_authenticated:
+            if self.request.profile in self.organization:
+                return True
+            if any(self.request.user.has_perm(perm) for perm in self.permission_bypass):
+                return True
+        return False
 
+    def generate_error_message(self, request):
+        return generic_message(request,
+                               _("Cannot view organization's private data"),
+                               _('You must join the organization to view its private data.'))
+
+    def dispatch(self, request, *args, **kwargs):
+        if not self.can_access_this_view():
+            return self.generate_error_message(request)
+
+        return super(PrivateOrganizationMixin, self).dispatch(request, *args, **kwargs)
 
-class OrganizationDetailView(OrganizationMixin, DetailView):
-    def get(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        if self.object.slug != kwargs['slug']:
-            return HttpResponsePermanentRedirect(reverse(
-                request.resolver_match.url_name, args=(self.object.id, self.object.slug)))
-        context = self.get_context_data(object=self.object)
-        return self.render_to_response(context)
+
+# Use this mixin to ensure that the user is an admin of the organization
+class AdminOrganizationMixin(PrivateOrganizationMixin):
+    def can_access_this_view(self):
+        return self.can_edit_organization()
+
+    def generate_error_message(self, request):
+        return generic_message(request, _("Can't edit organization"),
+                               _('You are not allowed to edit this organization.'), status=403)
+
+
+class BaseOrganizationListView(PublicOrganizationMixin, ListView):
+    model = None
+    context_object_name = None
+
+    def get_object(self, queryset=None):
+        if queryset is None:
+            return self.organization
+        return super(BaseOrganizationListView, self).get_object(queryset)
+
+# this is unused, comment out to avoid confusion
+# class OrganizationDetailView(OrganizationMixin, DetailView):
+#     def get(self, request, *args, **kwargs):
+#         self.object = self.get_object()
+#         if self.object.slug != kwargs['slug']:
+#             return HttpResponsePermanentRedirect(reverse(
+#                 request.resolver_match.url_name, args=(self.object.id, self.object.slug)))
+#         context = self.get_context_data(object=self.object)
+#         return self.render_to_response(context)
 
 
 class OrganizationList(TitleMixin, ListView):
@@ -126,7 +172,7 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class OrganizationMembershipChange(LoginRequiredMixin, OrganizationMixin, SingleObjectMixin, View):
+class OrganizationMembershipChange(LoginRequiredMixin, PublicOrganizationMixin, SingleObjectMixin, View):
     def post(self, request, *args, **kwargs):
         org = self.get_object()
         response = self.handle(request, org, request.profile)
@@ -353,7 +399,7 @@ def dispatch(self, request, *args, **kwargs):
                                    _('You are not allowed to create new organizations.'), status=403)
 
 
-class EditOrganization(LoginRequiredMixin, TitleMixin, OrganizationMixin, UpdateView):
+class EditOrganization(LoginRequiredMixin, TitleMixin, AdminOrganizationMixin, UpdateView):
     template_name = 'organization/edit.html'
     model = Organization
     form_class = OrganizationForm
@@ -373,20 +419,10 @@ def form_valid(self, form):
             revisions.set_user(self.request.user)
             return super(EditOrganization, self).form_valid(form)
 
-    def dispatch(self, request, *args, **kwargs):
-        try:
-            return super(EditOrganization, self).dispatch(request, *args, **kwargs)
-        except PermissionDenied:
-            return generic_message(request, _("Can't edit organization"),
-                                   _('You are not allowed to edit this organization.'), status=403)
-
 
-class KickUserWidgetView(LoginRequiredMixin, OrganizationMixin, SingleObjectMixin, View):
+class KickUserWidgetView(LoginRequiredMixin, AdminOrganizationMixin, SingleObjectMixin, View):
     def post(self, request, *args, **kwargs):
-        organization = self.get_object()
-        if not self.can_edit_organization(organization):
-            return generic_message(request, _("Can't edit organization"),
-                                   _('You are not allowed to kick people from this organization.'), status=403)
+        organization = self.organization
 
         try:
             user = Profile.objects.get(id=request.POST.get('user', None))
@@ -408,69 +444,11 @@ def post(self, request, *args, **kwargs):
         return HttpResponseRedirect(organization.get_users_url())
 
 
-# This is almost the same as the OrganizationMixin
-# However, I need to write a new class because the
-# current mixin is for the DetailView.
-class CustomOrganizationMixin(object):
-    # If true, all user can view the current page
-    # even if they are not in the org
-    allow_all_users = False
-
-    # If the user has at least one of the following permissions,
-    # they can access the private data even if they are not in the org
-    permission_bypass = []
-
-    def get_context_data(self, **kwargs):
-        context = super().get_context_data(**kwargs)
-        context['organization'] = self.organization
-        context['logo_override_image'] = self.organization.logo_override_image
-        context['meta_description'] = self.organization.about[:settings.DESCRIPTION_MAX_LENGTH]
-        return context
-
-    def dispatch(self, request, *args, **kwargs):
-        if 'slug' not in kwargs:
-            raise ImproperlyConfigured('Must pass a slug')
-        self.organization = get_object_or_404(Organization, slug=kwargs['slug'])
-        self.object = self.organization
-
-        if not self.allow_all_users and \
-           self.request.profile not in self.organization and \
-           not any(self.request.user.has_perm(perm) for perm in self.permission_bypass):
-            return generic_message(request,
-                                   _("Cannot view organization's private data"),
-                                   _('You must join the organization to view its private data.'))
-
-        return super(CustomOrganizationMixin, self).dispatch(request, *args, **kwargs)
-
-    def can_edit_organization(self, org=None):
-        if org is None:
-            org = self.organization
-        if not self.request.user.is_authenticated:
-            return False
-        return org.is_admin(self.request.profile) or self.request.user.has_perm('judge.edit_all_organization')
-
-
-class CustomAdminOrganizationMixin(CustomOrganizationMixin):
-    def dispatch(self, request, *args, **kwargs):
-        if 'slug' not in kwargs:
-            raise ImproperlyConfigured('Must pass a slug')
-        self.organization = get_object_or_404(Organization, slug=kwargs['slug'])
-        if self.can_edit_organization():
-            return super(CustomAdminOrganizationMixin, self).dispatch(request, *args, **kwargs)
-        raise PermissionDenied
-
-    def get_form_kwargs(self):
-        kwargs = super(CustomAdminOrganizationMixin, self).get_form_kwargs()
-        kwargs['org_pk'] = self.organization.pk
-        return kwargs
-
-
-class OrganizationHome(TitleMixin, CustomOrganizationMixin, PostListBase):
+# using PublicOrganizationMixin to allow user to view org's public information
+# like name, request join org, ...
+# However, they cannot see the organization private blog
+class OrganizationHome(TitleMixin, PublicOrganizationMixin, PostListBase):
     template_name = 'organization/home.html'
-    # Need to set this to true so user can view the org's public
-    # information like name, request join org, ...
-    # However, they cannot see the org blog
-    allow_all_users = True
 
     def get_queryset(self):
         queryset = BlogPost.objects.filter(organization=self.organization)
@@ -543,7 +521,7 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class ProblemListOrganization(CustomOrganizationMixin, ProblemList):
+class ProblemListOrganization(PrivateOrganizationMixin, ProblemList):
     context_object_name = 'problems'
     template_name = 'organization/problem-list.html'
     permission_bypass = ['judge.see_organization_problem', 'judge.edit_all_problem']
@@ -582,7 +560,7 @@ def get_filter(self):
         return _filter & Q(organizations=self.organization)
 
 
-class ContestListOrganization(CustomOrganizationMixin, ContestList):
+class ContestListOrganization(PrivateOrganizationMixin, ContestList):
     template_name = 'organization/contest-list.html'
     permission_bypass = ['judge.see_private_contest', 'judge.edit_all_contest']
     hide_private_contests = None
@@ -598,7 +576,7 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class SubmissionListOrganization(CustomOrganizationMixin, SubmissionsListBase):
+class SubmissionListOrganization(PrivateOrganizationMixin, SubmissionsListBase):
     template_name = 'organization/submission-list.html'
     permission_bypass = ['judge.view_all_submission']
 
@@ -614,7 +592,7 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class ProblemCreateOrganization(CustomAdminOrganizationMixin, ProblemCreate):
+class ProblemCreateOrganization(AdminOrganizationMixin, ProblemCreate):
     permission_required = 'judge.create_organization_problem'
 
     def get_initial(self):
@@ -625,9 +603,7 @@ def get_initial(self):
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
-        kwargs.update({
-            'user': self.request.user,
-        })
+        kwargs['org_pk'] = self.organization.pk
         return kwargs
 
     def form_valid(self, form):
@@ -649,7 +625,7 @@ def form_valid(self, form):
         return HttpResponseRedirect(self.get_success_url())
 
 
-class BlogPostCreateOrganization(CustomAdminOrganizationMixin, PermissionRequiredMixin, BlogPostCreate):
+class BlogPostCreateOrganization(AdminOrganizationMixin, PermissionRequiredMixin, BlogPostCreate):
     permission_required = 'judge.edit_organization_post'
 
     def get_initial(self):
@@ -672,7 +648,7 @@ def form_valid(self, form):
         return HttpResponseRedirect(post.get_absolute_url())
 
 
-class ContestCreateOrganization(CustomAdminOrganizationMixin, CreateContest):
+class ContestCreateOrganization(AdminOrganizationMixin, CreateContest):
     permission_required = 'judge.create_private_contest'
 
     def get_initial(self):
@@ -681,6 +657,11 @@ def get_initial(self):
         initial['key'] = ''.join(x for x in self.organization.slug.lower() if x.isalpha()) + '_'
         return initial
 
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        kwargs['org_pk'] = self.organization.pk
+        return kwargs
+
     def save_contest_form(self, form):
         self.object = form.save()
         self.object.authors.add(self.request.profile)