From ec250a5de629e9079b69cecbc23429cc58aa2744 Mon Sep 17 00:00:00 2001 From: Filip Stenbacka Date: Wed, 30 Oct 2024 03:06:52 +0200 Subject: [PATCH] Enhanced the membersByMemberType endpoint by handling each MemberType separately. --- teknologr/api/filters.py | 2 +- teknologr/api/tests_members_by_mt.py | 233 ++++++++++++++++++++------- teknologr/api/views.py | 49 +++++- teknologr/members/models.py | 1 - 4 files changed, 220 insertions(+), 65 deletions(-) diff --git a/teknologr/api/filters.py b/teknologr/api/filters.py index 34d7b92d..4c5c0680 100644 --- a/teknologr/api/filters.py +++ b/teknologr/api/filters.py @@ -1,5 +1,5 @@ import django_filters -from django.db.models import Count +from django.db.models import Q, Count from members.models import * from functools import reduce from operator import and_ diff --git a/teknologr/api/tests_members_by_mt.py b/teknologr/api/tests_members_by_mt.py index 30c8827f..12f9a1c6 100644 --- a/teknologr/api/tests_members_by_mt.py +++ b/teknologr/api/tests_members_by_mt.py @@ -1,4 +1,6 @@ from django.contrib.auth.models import User +from django.utils import timezone +from datetime import timedelta from members.models import * from rest_framework import status from rest_framework.test import APITestCase @@ -8,51 +10,20 @@ def setUp(self): self.user = User.objects.create_user(username='svakar', password='teknolog') self.superuser = User.objects.create_superuser(username='superuser', password='teknolog') - self.m1 = Member.objects.create( - given_names='Sverker Svakar', - preferred_name='Svakar', - surname='von Teknolog', - student_id='123456', - username='vonteks1', - ) - self.m2 = Member.objects.create( - given_names='Svatta', - surname='von Teknolog', - student_id='654321', - username='vonteks2', - ) - self.m3 = Member.objects.create( - given_names='Simon', - surname='von Teknolog', - ) - - d1 = '2000-01-01' - d2 = '2010-01-01' - - MemberType.objects.create(member=self.m1, type='PH', begin_date=d1, end_date=d2) - MemberType.objects.create(member=self.m1, type='OM', begin_date=d2) - MemberType.objects.create(member=self.m1, type='JS', begin_date=d1) - - MemberType.objects.create(member=self.m2, type='PH', begin_date=d1, end_date=d2) - MemberType.objects.create(member=self.m2, type='OM', begin_date=d2) - MemberType.objects.create(member=self.m2, type='ST', begin_date=d1) - MemberType.objects.create(member=self.m2, type='ST', begin_date=d2) - - MemberType.objects.create(member=self.m3, type='JS', begin_date=d1) - MemberType.objects.create(member=self.m3, type='ST', begin_date=d2) - MemberType.objects.create(member=self.m3, type='ST', begin_date=d2) - def login_superuser(self): self.client.login(username='superuser', password='teknolog') class TestCases(): + def check_json(self, json, expected_members): + self.assertEqual([self.map(m) for m in expected_members], json) + def test_get_for_anonymous_users(self): - response = self.get('PH') + response = self.get('KE') self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_get_for_user(self): self.client.login(username='svakar', password='teknolog') - response = self.get('PH') + response = self.get('KE') self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_get_not_found(self): @@ -63,47 +34,195 @@ def test_get_not_found(self): def test_get_invalid(self): self.login_superuser() response = self.get('XX') + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {'detail': 'invalid membertype'}) + + def test_get_only_ongoing(self): + today = timezone.now().date() + past = today - timedelta(days=1) + future = today + timedelta(days=1) + + m1 = Member.objects.create(student_id='111', username='abc1') + m2 = Member.objects.create(student_id='222', username='abc2') + m3 = Member.objects.create(student_id='333', username='abc3') + m4 = Member.objects.create(student_id='444', username='abc4') + + MemberType.objects.create(member=m1, type='KE') + MemberType.objects.create(member=m2, type='KE', end_date=today) + MemberType.objects.create(member=m3, type='KE', end_date=past) + MemberType.objects.create(member=m4, type='KE', end_date=future) + + self.login_superuser() + response = self.get('KE') self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), []) + self.assertEqual(response.json(), [self.map(m1), self.map(m2), self.map(m4)]) + + def test_null_values(self): + m1 = Member.objects.create(student_id='123456', username='abc1') + m2 = Member.objects.create() + + MemberType.objects.create(member=m1, type='KE') + MemberType.objects.create(member=m2, type='KE') - def test_get_all_ended(self): self.login_superuser() - response = self.get('PH') + + response = self.get('KE') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), [self.map(m1)] + [None]) + + response = self.get('KE', True) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), []) + self.assertEqual(response.json(), [self.map(m1)]) + + def test_doubles(self): + m1 = Member.objects.create(student_id='123456', username='abc1') + + MemberType.objects.create(member=m1, type='KE') + MemberType.objects.create(member=m1, type='KE') + MemberType.objects.create(member=m1, type='KE') + + self.login_superuser() + response = self.get('KE') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), [self.map(m1)]) + + def test_OM(self): + m1 = Member.objects.create(student_id='111', username='abc1') + m2 = Member.objects.create(student_id='222', username='abc2') + m3 = Member.objects.create(student_id='333', username='abc3', graduated=True) + m4 = Member.objects.create(student_id='444', username='abc4', graduated_year=2023) + m5 = Member.objects.create(student_id='555', username='abc5') + m6 = Member.objects.create(student_id='666', username='abc6') + + MemberType.objects.create(member=m1, type='OM') + + # Ended + MemberType.objects.create(member=m2, type='OM', end_date="2010-01-01") + + # Graduated + MemberType.objects.create(member=m3, type='OM') + + # Graduated year + MemberType.objects.create(member=m4, type='OM') + + # Has 'ST' MemberType + MemberType.objects.create(member=m5, type='OM') + MemberType.objects.create(member=m5, type='ST') + + # Has 'EM' MemberType + MemberType.objects.create(member=m6, type='OM') + MemberType.objects.create(member=m6, type='EM') - def test_get_normal(self): self.login_superuser() response = self.get('OM') self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), self.normal) + self.assertEqual(response.json(), [self.map(m1)]) + + def test_JS(self): + m1 = Member.objects.create(student_id='111', username='abc1') + m2 = Member.objects.create(student_id='222', username='abc2') + m3 = Member.objects.create(student_id='333', username='abc3', graduated=True) + m4 = Member.objects.create(student_id='444', username='abc4', graduated_year=2023) + m5 = Member.objects.create(student_id='555', username='abc5') + m6 = Member.objects.create(student_id='666', username='abc6') + + MemberType.objects.create(member=m1, type='JS') + + # Ended + MemberType.objects.create(member=m2, type='JS', end_date="2010-01-01") + + # Graduated + MemberType.objects.create(member=m3, type='JS') + + # Graduated year + MemberType.objects.create(member=m4, type='JS') + + # Has 'ST' MemberType + MemberType.objects.create(member=m5, type='JS') + MemberType.objects.create(member=m5, type='ST') + + # Has 'EM' MemberType + MemberType.objects.create(member=m6, type='JS') + MemberType.objects.create(member=m6, type='EM') - def test_get_null(self): self.login_superuser() response = self.get('JS') self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), self.null) + self.assertEqual(response.json(), [self.map(m1)]) + + def test_ST(self): + m1 = Member.objects.create(student_id='111', username='abc1') + m2 = Member.objects.create(student_id='222', username='abc2') + m3 = Member.objects.create(student_id='333', username='abc3') + + MemberType.objects.create(member=m1, type='ST') + + # Ended + MemberType.objects.create(member=m2, type='ST', end_date="2010-01-01") + + # Has 'EM' MemberType + MemberType.objects.create(member=m3, type='ST') + MemberType.objects.create(member=m3, type='EM') - def test_get_double(self): self.login_superuser() response = self.get('ST') self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json(), self.double) + self.assertEqual(response.json(), [self.map(m1)]) + def test_PH(self): + m1 = Member.objects.create(student_id='111', username='abc1') + m2 = Member.objects.create(student_id='222', username='abc2') + m3 = Member.objects.create(student_id='333', username='abc3', graduated=True) + m4 = Member.objects.create(student_id='444', username='abc4', graduated_year=2023) + m5 = Member.objects.create(student_id='555', username='abc5') + m6 = Member.objects.create(student_id='666', username='abc6') + m7 = Member.objects.create(student_id='777', username='abc7') + + MemberType.objects.create(member=m1, type='PH') + + # Ended + MemberType.objects.create(member=m2, type='PH', end_date="2010-01-01") + + # Graduated + MemberType.objects.create(member=m3, type='PH') + + # Graduated year + MemberType.objects.create(member=m4, type='PH') + + # Has 'OM' MemberType + MemberType.objects.create(member=m5, type='PH') + MemberType.objects.create(member=m5, type='OM') + + # Has 'ST' MemberType + MemberType.objects.create(member=m6, type='PH') + MemberType.objects.create(member=m6, type='ST') + + # Has 'EM' MemberType + MemberType.objects.create(member=m7, type='PH') + MemberType.objects.create(member=m7, type='EM') + + self.login_superuser() + response = self.get('PH') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), [self.map(m1)]) class StudynumbersByMTTests(BaseClass, TestCases): - normal = ['123456', '654321'] - null = ['123456', None] - double = ['654321', None] + def get(self, type, skip_null=False): + url = f'/api/membersByMemberType/{type}/' + if skip_null: + url += "?skip_null=True" + return self.client.get(url) - def get(self, type): - return self.client.get(f'/api/membersByMemberType/{type}/') + def map(self, member): + return member.student_id class UsernamesByMTTests(BaseClass, TestCases): - normal = ['vonteks1', 'vonteks2'] - null = ['vonteks1', None] - double = ['vonteks2', None] - - def get(self, type): - return self.client.get(f'/api/membersByMemberType/{type}/usernames') + def get(self, type, skip_null=False): + url = f'/api/membersByMemberType/{type}/usernames' + if skip_null: + url += "?skip_null=True" + return self.client.get(url) + + def map(self, member): + return member.username diff --git a/teknologr/api/views.py b/teknologr/api/views.py index c2b6f22d..f9edc4b3 100644 --- a/teknologr/api/views.py +++ b/teknologr/api/views.py @@ -3,6 +3,7 @@ from django.db.models import Q from django.db.utils import IntegrityError from django.http import HttpResponse +from django.utils import timezone from django_filters import rest_framework as filters from rest_framework import viewsets, permissions from rest_framework.views import APIView @@ -10,8 +11,6 @@ from rest_framework.response import Response from rest_framework.decorators import api_view from ldap import LDAPError -from collections import defaultdict -from datetime import datetime from api.serializers import * from api.filters import * from api.ldap import LDAPAccountManager, LDAPError_to_string @@ -581,10 +580,48 @@ def member_types_for_member(request, mode, query): # Used by BILL and GeneriKey @api_view(['GET']) def members_by_member_type(request, membertype, field=None): - member_pks = MemberType.objects.filter(type=membertype, end_date=None).values_list("member", flat=True) - fld = "username" if field == "usernames" else "student_id" - members = Member.objects.filter(pk__in=member_pks).values_list(fld, flat=True) - return Response(members, status=200) + if membertype not in [a[0] for a in MemberType.TYPES]: + return Response({'detail': 'invalid membertype'}, status=404) + + # Need to be careful with 'OM' requests, because it is used to check for valid members. This need to be "foolproof" and account for (at least) the user error of leaving the 'OM' MemberType untouched when adding graduation date or the 'ST' MemberType. The same kind of logic can be applied to the other MemberTypes. + + def get_member_ids(mt): + today = timezone.now().date().strftime('%Y-%m-%d') + return MemberType.objects.filter(type=mt).exclude(end_date__lt=today).values_list('member', flat=True) + + pks = set(get_member_ids(membertype)) + + if membertype == 'PH': + pks.difference_update(get_member_ids('OM')) + pks.difference_update(get_member_ids('ST')) + pks.difference_update(get_member_ids('EM')) + + members = Member.objects.filter(pk__in=pks).exclude( + Q(graduated=True) | ~Q(graduated_year=None) + ) + + elif membertype in ['OM', 'JS']: + pks.difference_update(get_member_ids('ST')) + pks.difference_update(get_member_ids('EM')) + + members = Member.objects.filter(pk__in=pks).exclude( + Q(graduated=True) | ~Q(graduated_year=None) + ) + + elif membertype == 'ST': + pks.difference_update(get_member_ids('EM')) + + members = Member.objects.filter(pk__in=pks) + + else: + members = Member.objects.filter(pk__in=pks) + + fld = "username" if field and "username" in field else "student_id" + result = members.values_list(fld, flat=True) + print(request.GET) + if 'skip_null' in request.GET: + result = [r for r in result if r is not None] + return Response(result, status=200) # Data for HTK diff --git a/teknologr/members/models.py b/teknologr/members/models.py index 79499e13..7f28720d 100644 --- a/teknologr/members/models.py +++ b/teknologr/members/models.py @@ -790,7 +790,6 @@ class MemberType(SuperClass): ("KA", "Kanslist"), ("IM", "Inte medlem"), ("KE", "Kanslist emerita"), - ) member = models.ForeignKey("Member", on_delete=models.CASCADE, related_name='member_types') begin_date = models.DateField(null=True)