Merge pull request #401 from uw-it-aca/bug/student-adviser-vue

improved caseload search

compass/dao/person.py

@@ -1,15 +1,16 @@
 # 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 (
     Person, Adviser, Student, Transcript, Degree)
 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')

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

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}))

compass/urls.py

@@ -70,11 +70,11 @@
         StudentView.as_view(),
     ),
     re_path(
-        r"^api/internal/student/(?P<uwregid>[-@:\w]+)/schedules/$",
+        r"^api/internal/student/(?P<uwregid>[a-fA-F0-9]{32})/schedules/$",
         StudentSchedulesView.as_view(),
     ),
     re_path(
-        r"^api/internal/student/(?P<uwregid>[-@:\w]+)/transcripts/$",
+        r"^api/internal/student/(?P<uwregid>[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<photo_key>[a-z0-9]*)/$",
+        r"^api/internal/photo/(?P<uwregid>[a-fA-F0-9]{32})/"
+        r"(?P<photo_key>[a-z0-9]{16})/$",
         PhotoView.as_view(),
         name="photo",
     ),

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))

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)

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()

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;
       });
     },

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'] = {