From 92c83b2bc5d47cbfe7ee13784186383cfca2a409 Mon Sep 17 00:00:00 2001 From: Jim Laney Date: Thu, 11 Apr 2024 08:30:43 -0700 Subject: [PATCH] improved caseload search --- compass/dao/person.py | 22 +++++++++++----------- compass/dao/photo.py | 32 +++++++++++--------------------- compass/tests/dao/test_person.py | 20 ++++++++++++++++---- compass/urls.py | 7 ++++--- compass/views/api/adviser.py | 18 ++++++++---------- compass/views/api/photo.py | 6 +++--- compass/views/api/student.py | 6 +++++- compass_vue/pages/caseload.vue | 6 +++--- docker/settings.py | 2 +- 9 files changed, 62 insertions(+), 57 deletions(-) diff --git a/compass/dao/person.py b/compass/dao/person.py index a0e4ae54..3b1ec710 100644 --- a/compass/dao/person.py +++ b/compass/dao/person.py @@ -1,8 +1,8 @@ # Copyright 2024 UW-IT, University of Washington # SPDX-License-Identifier: Apache-2.0 -from django.db.models import OuterRef, Subquery, F -from django.db.models.functions import JSONObject +from django.db.models import CharField, OuterRef, Subquery, Value, F +from django.db.models.functions import JSONObject, Concat from django.conf import settings from django.core.cache import cache from uw_person_client.models import ( @@ -10,6 +10,7 @@ from uw_person_client.exceptions import ( PersonNotFoundException, AdviserNotFoundException) from uw_pws import PWS, InvalidNetID, DataFailureException +from compass.dao.photo import PhotoDAO import re system_key_re = re.compile(r'^\d{9}$') @@ -99,6 +100,7 @@ def get_appuser_by_uwnetid(uwnetid): def get_adviser_caseload(uwnetid): adviser = Adviser.objects.get_adviser_by_uwnetid(uwnetid) + photo_key = PhotoDAO().generate_photo_key() return Student.objects.annotate(latest_transcript=Subquery( Transcript.objects.filter( @@ -110,10 +112,9 @@ def get_adviser_caseload(uwnetid): Degree.objects.filter( student=OuterRef('pk')).values(json=JSONObject( degree_status_desc='degree_status_desc') - ).order_by('-degree_date')[:1]) + ).order_by('-degree_index')[:1]) ).filter(advisers__in=[adviser]).values( 'student_number', - 'gender', 'class_desc', 'academic_term__year', 'academic_term__quarter', @@ -123,19 +124,18 @@ def get_adviser_caseload(uwnetid): 'special_program_code', 'special_program_desc', 'enroll_status_code', - 'enroll_status_request_code', - 'enroll_status_desc', - 'person__display_name', 'person__uwnetid', 'person__uwregid', 'person__pronouns', - 'person__surname', + 'person__display_name', 'latest_transcript', 'latest_degree' ).annotate( - display_name=F('person__display_name'), uwnetid=F('person__uwnetid'), uwregid=F('person__uwregid'), pronouns=F('person__pronouns'), - surname=F('person__surname') - ).order_by('person__surname') + display_name=F('person__display_name'), + photo_url=Concat( + Value('/api/internal/photo/'), F('person__uwregid'), + Value(f'/{photo_key}/'), output_field=CharField()) + ).order_by('person__surname', 'person__first_name') diff --git a/compass/dao/photo.py b/compass/dao/photo.py index 344f7a3e..59caaf03 100644 --- a/compass/dao/photo.py +++ b/compass/dao/photo.py @@ -6,40 +6,30 @@ from django.conf import settings from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist -from django.urls import reverse from uw_pws import PWS from urllib.parse import urlparse, urlunparse class PhotoDAO(): - - def __init__(self): - super().__init__() - def cache_key(self, key): return 'idphoto-key-{}'.format(key) - def get_photo(self, photo_key): + def generate_photo_key(self, image_size='medium'): + photo_key = ''.join(random.SystemRandom().choice( + string.ascii_lowercase + string.digits) for _ in range(16)) + + data = {'image_size': image_size} + expires = getattr(settings, 'IDCARD_TOKEN_EXPIRES', 60 * 5) + cache.set(self.cache_key(photo_key), data, timeout=expires) + return photo_key + + def get_photo(self, uwregid, photo_key): data = cache.get(self.cache_key(photo_key)) - cache.delete(self.cache_key(photo_key)) if data is None: raise ObjectDoesNotExist() - return PWS().get_idcard_photo( - data.get('reg_id'), size=data.get('image_size')) - - def get_photo_url(self, reg_id, image_size="medium"): - """ Returns a url for the IDPhoto - """ - if PWS().valid_uwregid(reg_id): - photo_key = ''.join(random.SystemRandom().choice( - string.ascii_lowercase + string.digits) for _ in range(16)) - - data = {'reg_id': reg_id, 'image_size': image_size} - expires = getattr(settings, 'IDCARD_TOKEN_EXPIRES', 60 * 60) - cache.set(self.cache_key(photo_key), data, timeout=expires) - return reverse('photo', kwargs={'photo_key': photo_key}) + return PWS().get_idcard_photo(uwregid, size=data.get('image_size')) def get_avatar_url(self, url, image_size): """ Modifies the passed url based on image_size diff --git a/compass/tests/dao/test_person.py b/compass/tests/dao/test_person.py index d2036597..bbcb2ed0 100644 --- a/compass/tests/dao/test_person.py +++ b/compass/tests/dao/test_person.py @@ -2,9 +2,11 @@ # SPDX-License-Identifier: Apache-2.0 +from django.urls import reverse from compass.tests import CompassTestCase from compass.dao.person import * from uw_pws.util import fdao_pws_override +import mock @fdao_pws_override @@ -129,15 +131,25 @@ def test_get_adviser_caseload_not_found(self): self.assertRaises( AdviserNotFoundException, get_adviser_caseload, 'javerage') - def test_get_adviser_caseload(self): + @mock.patch.object(PhotoDAO, 'generate_photo_key') + def test_get_adviser_caseload(self, mock_photo_key): + key = 'testtesttesttest' + mock_photo_key.return_value = key + students = get_adviser_caseload('jadviser') self.assertEqual(len(students), 3) s1 = students[0] - self.assertEqual(s1['surname'], 'Average') + self.assertEqual(s1['display_name'], 'Jamesy McJamesy') + self.assertEqual(s1['photo_url'], reverse('photo', kwargs={ + 'uwregid': s1['uwregid'], 'photo_key': key})) s2 = students[1] - self.assertEqual(s2['surname'], 'Simpson') + self.assertEqual(s2['display_name'], 'Lisa Simpson') + self.assertEqual(s2['photo_url'], reverse('photo', kwargs={ + 'uwregid': s2['uwregid'], 'photo_key': key})) s3 = students[2] - self.assertEqual(s3['surname'], 'Student') + self.assertEqual(s3['display_name'], 'James Bothell Student') + self.assertEqual(s3['photo_url'], reverse('photo', kwargs={ + 'uwregid': s3['uwregid'], 'photo_key': key})) diff --git a/compass/urls.py b/compass/urls.py index 7eb7e505..8b50d19c 100644 --- a/compass/urls.py +++ b/compass/urls.py @@ -70,11 +70,11 @@ StudentView.as_view(), ), re_path( - r"^api/internal/student/(?P[-@:\w]+)/schedules/$", + r"^api/internal/student/(?P[a-fA-F0-9]{32})/schedules/$", StudentSchedulesView.as_view(), ), re_path( - r"^api/internal/student/(?P[-@:\w]+)/transcripts/$", + r"^api/internal/student/(?P[a-fA-F0-9]{32})/transcripts/$", StudentTranscriptsView.as_view(), ), re_path( @@ -138,7 +138,8 @@ AdviserCaseloadView.as_view(), ), re_path( - r"^api/internal/photo/(?P[a-z0-9]*)/$", + r"^api/internal/photo/(?P[a-fA-F0-9]{32})/" + r"(?P[a-z0-9]{16})/$", PhotoView.as_view(), name="photo", ), diff --git a/compass/views/api/adviser.py b/compass/views/api/adviser.py index b65f6f07..5d14cb39 100644 --- a/compass/views/api/adviser.py +++ b/compass/views/api/adviser.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 +from django.urls import reverse from compass.views.api import BaseAPIView from compass.dao.person import ( valid_uwnetid, get_person_by_system_key, get_adviser_caseload, @@ -22,13 +23,16 @@ def get(self, request, adviser_netid): return self.response_badrequest('Invalid uwnetid') contacts = [] - photo_dao = PhotoDAO() + photo_key = PhotoDAO().generate_photo_key() for contact in Contact.objects.by_adviser(adviser_netid): contact_dict = ContactReadSerializer(contact, many=False).data try: person = get_person_by_system_key(contact.student.system_key) - person.photo_url = photo_dao.get_photo_url(person.uwregid) - contact_dict["student"] = person.to_dict() + person_dict = person.to_dict() + person_dict['photo_url'] = reverse('photo', kwargs={ + 'uwregid': person.uwregid, + 'photo_key': photo_key}) + contact_dict["student"] = person_dict contacts.append(contact_dict) except PersonNotFoundException: pass @@ -50,10 +54,4 @@ def get(self, request, adviser_netid): except AdviserNotFoundException: queryset = [] - photo_dao = PhotoDAO() - students = [] - for row in queryset: - row['photo_url'] = photo_dao.get_photo_url(row['uwregid']) - students.append(row) - - return self.response_ok(students) + return self.response_ok(list(queryset)) diff --git a/compass/views/api/photo.py b/compass/views/api/photo.py index 338ff1b3..0a3cc4ab 100644 --- a/compass/views/api/photo.py +++ b/compass/views/api/photo.py @@ -14,13 +14,13 @@ class PhotoView(BaseAPIView): date_format = '%a, %d %b %Y %H:%M:%S GMT' def get(self, request, *args, **kwargs): + uwregid = kwargs.get('uwregid') photo_key = kwargs.get('photo_key') now = datetime.utcnow() expires = now + timedelta(seconds=self.cache_time) try: - photo = PhotoDAO().get_photo(photo_key) - response = StreamingHttpResponse(photo, - content_type='image/jpeg') + photo = PhotoDAO().get_photo(uwregid, photo_key) + response = StreamingHttpResponse(photo, content_type='image/jpeg') response['Cache-Control'] = 'public,max-age={}'.format( self.cache_time) response['Expires'] = expires.strftime(self.date_format) diff --git a/compass/views/api/student.py b/compass/views/api/student.py index 29a384b3..80037441 100644 --- a/compass/views/api/student.py +++ b/compass/views/api/student.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 +from django.urls import reverse from compass.views.api import BaseAPIView from compass.dao.photo import PhotoDAO from compass.dao.person import ( @@ -52,7 +53,10 @@ def get(self, request, identifier): return self.response_badrequest('Invalid student identifier') person_dict = person.to_dict() - person_dict['photo_url'] = PhotoDAO().get_photo_url(person.uwregid) + photo_key = PhotoDAO().generate_photo_key() + person_dict['photo_url'] = reverse('photo', kwargs={ + 'uwregid': person.uwregid, + 'photo_key': photo_key}) return self.response_ok(person_dict) except PersonNotFoundException: return self.response_notfound() diff --git a/compass_vue/pages/caseload.vue b/compass_vue/pages/caseload.vue index d358f858..1b93c3f5 100644 --- a/compass_vue/pages/caseload.vue +++ b/compass_vue/pages/caseload.vue @@ -347,9 +347,9 @@ export default { }, loadAdviserCaseload: function (netid) { this.getAdviserCaseload(netid).then((response) => { - this.persons = response.data.sort(function (a, b) { - return a.surname > b.surname ? 1 : -1; - }); + if (response.data) { + this.persons = response.data; + } this.isLoading = false; }); }, diff --git a/docker/settings.py b/docker/settings.py index 61639b1b..ad3e5e9a 100644 --- a/docker/settings.py +++ b/docker/settings.py @@ -105,7 +105,7 @@ ALLOW_USER_OVERRIDE_FOR_WRITE = (os.getenv('ENV', 'localdev') != 'prod') # IDCard photo config -IDCARD_TOKEN_EXPIRES = 60 * 60 +IDCARD_TOKEN_EXPIRES = 60 * 5 # PDS config, default values are for localdev DATABASES['uw_person'] = {