diff --git a/cmsmanage/settings/base.py b/cmsmanage/settings/base.py index 807a00e..8639a4f 100644 --- a/cmsmanage/settings/base.py +++ b/cmsmanage/settings/base.py @@ -82,10 +82,6 @@ DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" WSGI_APPLICATION = "cmsmanage.wsgi.application" -DATABASE_ROUTERS = [ - "membershipworks.routers.MembershipWorksRouter", -] - # Default URL to redirect to after authentication LOGIN_REDIRECT_URL = "/" LOGIN_URL = "/auth/login/" diff --git a/membershipworks/routers.py b/membershipworks/routers.py deleted file mode 100644 index aae1652..0000000 --- a/membershipworks/routers.py +++ /dev/null @@ -1,23 +0,0 @@ -class MembershipWorksRouter: - app_label = "membershipworks" - db = "membershipworks" - - def db_for_read(self, model, **hints): - if model._meta.app_label == self.app_label: - return self.db - return None - - def db_for_write(self, model, **hints): - if model._meta.app_label == self.app_label: - return self.db - return None - - def allow_migrate(self, db, app_label, model_name=None, **hints): - if app_label == self.app_label: - return db == self.db - return None - - def allow_relation(self, obj1, obj2, **hints): - if self.app_label in (obj1._meta.app_label, obj2._meta.app_label): - return True - return None diff --git a/paperwork/api.py b/paperwork/api.py index 1c6c478..7dcd6f4 100644 --- a/paperwork/api.py +++ b/paperwork/api.py @@ -1,4 +1,4 @@ -from django.db.models import Prefetch, Q +from django.db.models import Q from rest_framework import routers, serializers, viewsets from rest_framework.decorators import action @@ -33,10 +33,6 @@ class DepartmentViewSet(viewsets.ModelViewSet): departments = self.queryset.prefetch_related( "children", "shop_lead_flag__members", - Prefetch( - "certificationdefinition_set__certificationversion_set__certification_set__member", - queryset=Member.objects.with_is_active(), - ), ) lists = {} for department in departments.filter(has_mailing_list=True): @@ -48,14 +44,12 @@ class DepartmentViewSet(viewsets.ModelViewSet): else: moderator_emails = [] - # TODO: this could be done in SQL instead if - # membershipworks was in the same database active_certified_members = { - member_cert.member.sanitized_mailbox() - for certification in department.certificationdefinition_set.all() - for version in certification.certificationversion_set.all() - for member_cert in version.certification_set.all() - if member_cert.member and member_cert.member.is_active + member.sanitized_mailbox() + for member in Member.objects.with_is_active().filter( + is_active=True, + certification__certification_version__definition__department=department, + ) } lists[department.list_name] = { diff --git a/paperwork/models.py b/paperwork/models.py index fbea35a..3c2d6b3 100644 --- a/paperwork/models.py +++ b/paperwork/models.py @@ -71,11 +71,11 @@ class CmsRedRiverVeteransScholarship(models.Model): class DepartmentQuerySet(models.QuerySet): def filter_by_shop_lead(self, member: Member) -> models.QuerySet["Department"]: """Get departments for which `member` is a shop lead""" - # TODO: could be a lot simpler if membershipworks was in the same database - # TODO: should also select children - member_flags = list(member.flags.all().values_list("pk", flat=True)) - return self.prefetch_related("shop_lead_flag__members").filter( - shop_lead_flag__in=member_flags + # TODO: should select children recursively, instead of specific levels + return self.filter( + Q(shop_lead_flag__members=member) + | Q(parent__shop_lead_flag__members=member) + | Q(parent__parent__shop_lead_flag__members=member) ) diff --git a/paperwork/views.py b/paperwork/views.py index 69fbc95..10d24eb 100644 --- a/paperwork/views.py +++ b/paperwork/views.py @@ -3,7 +3,17 @@ from django.contrib import staticfiles from django.contrib.auth.decorators import login_required from django.contrib.auth.mixins import PermissionRequiredMixin from django.db import models -from django.db.models import Case, Count, Max, Q, Value, When +from django.db.models import ( + Case, + Count, + Exists, + Max, + OuterRef, + Q, + Subquery, + Value, + When, +) from django.db.models.functions import Concat from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseNotFound from django.shortcuts import get_object_or_404, render @@ -236,49 +246,24 @@ class AccessVerificationReport( export_formats = ("csv", "xlsx", "ods") def get_queryset(self): - # TODO: could be done with subqueries if membershipworks was not a separate DB def shop_error(access_field: str, shop_name: str): - member_list = list( - CertificationVersion.objects.filter( - is_current=True, - definition__department__name=shop_name, - certification__member__pk__isnull=False, - ) - .values_list( - "certification__member__pk", - flat=True, - ) - .distinct() - ) + member_list = CertificationVersion.objects.filter( + is_current=True, + definition__department__name=shop_name, + certification__member__pk__isnull=False, + ).values("certification__member__pk") return Case( When( - Q(**{access_field: True}) & ~Q(uid__in=member_list), + Q(**{access_field: True}) & ~Q(uid__in=Subquery(member_list)), Value("Has access but no cert"), ), When( - Q(**{access_field: False}) & Q(uid__in=member_list), + Q(**{access_field: False}) & Q(uid__in=Subquery(member_list)), Value("Has cert but no access"), ), default=None, ) - # TODO: could be a lot cleaner if membershipworks was not a separate DB - storage_closet_members = ( - Member.objects.filter( - Member.objects.has_flag("label", "Volunteer: Desker") - | Q(billing_method__startswith="Desker") - ) - .union( - *[ - department.shop_lead_flag.members.all() - for department in Department.objects.filter( - shop_lead_flag__isnull=False - ) - ] - ) - .values_list("pk", flat=True) - ) - qs = ( Member.objects.with_is_active() .filter(is_active=True) @@ -307,7 +292,17 @@ class AccessVerificationReport( storage_closet_error=Case( When( Q(access_storage_closet=True) - & ~Q(uid__in=storage_closet_members), + & ~( + Member.objects.has_flag("label", "Volunteer: Desker") + | Q(billing_method__startswith="Desker") + | Q( + Exists( + Department.objects.filter( + shop_lead_flag__members=OuterRef("pk") + ) + ) + ) + ), Value("Has access but not shop lead or desker"), ), default=None,