From 7c46400a3ec244ff681ba7d6a79f2a48eb6d24a8 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Sun, 21 Dec 2025 16:20:07 -0800 Subject: [PATCH 1/3] Makes a number of bug fixes (still need to be tested more), including: 1. Fixes a missing null check on latest_position.has_started, which should address https://github.com/makeabilitylab/makeabilitylabwebsite/issues/1239 2. Adds additional support for auto-generated bios of non-members, which should fix: https://github.com/makeabilitylab/makeabilitylabwebsite/issues/1238 3. Redid logic of recent project listing on member pages, which should fix: https://github.com/makeabilitylab/makeabilitylabwebsite/issues/1168 --- docker-compose-local-dev.yml | 10 +- website/models/person.py | 134 +++++++++---------- website/models/position.py | 44 +++++- website/static/website/css/design-tokens.css | 1 + website/static/website/css/project.css | 10 +- website/templates/website/project.html | 4 +- website/views/member.py | 58 ++++---- 7 files changed, 150 insertions(+), 111 deletions(-) diff --git a/docker-compose-local-dev.yml b/docker-compose-local-dev.yml index 1f98c0d1..2e99894d 100644 --- a/docker-compose-local-dev.yml +++ b/docker-compose-local-dev.yml @@ -9,13 +9,13 @@ # 3. a11y - An accessibility testing container using Pa11y + Axe # # Usage: -# docker-compose -f docker-compose-local-dev.yml up +# docker compose -f docker-compose-local-dev.yml up # -# Access checks: -# docker-compose -f docker-compose-local-dev.yml --profile testing run --rm a11y +# To run accessibility checks, you can edit .pa11yci.json and then run: +# docker compose -f docker-compose-local-dev.yml --profile testing run --rm a11y # # Access check with report generation: -# docker-compose -f docker-compose-local-dev.yml --profile testing run --rm a11y sh -c " +# docker compose -f docker-compose-local-dev.yml --profile testing run --rm a11y sh -c " # npm install -g pa11y-ci && # pa11y-ci --config /workspace/.pa11yci.json --json | tee /workspace/a11y-report.json # " @@ -23,7 +23,7 @@ # After running, the website is available at: http://localhost:8571 # # To stop: -# docker-compose down +# docker compose down # # ============================================================================= diff --git a/website/models/person.py b/website/models/person.py index 1bf6219b..a11fe88b 100644 --- a/website/models/person.py +++ b/website/models/person.py @@ -1,13 +1,16 @@ from django.db import models from django.dispatch import receiver -from django.db.models.signals import pre_delete, post_save, m2m_changed, post_delete +from django.db.models.signals import pre_delete from website.models.publication import PubType from website.models.position import Role, Title from website.models.project_role import ProjectRole from django.core.files import File import website.utils.fileutils as ml_fileutils -from django.db.models import F, Q, Sum, ExpressionWrapper, fields +from django.db.models.functions import Coalesce +from django.conf import settings + +from django.db.models import Count, Max, Value, F, Q, Sum, ExpressionWrapper, fields from django.utils import timezone from django.db.models.functions import Coalesce @@ -20,7 +23,7 @@ from uuid import uuid4 # for generating unique filenames import re -from datetime import date, datetime, timedelta +from datetime import date, timedelta from image_cropping import ImageRatioField @@ -234,11 +237,11 @@ def get_total_time_on_project(self, project): Returns the total time as a timedelta this person has worked on the given project. If the end_date is None, the current date/time is used. """ - print(f"person {self}, project {project}") + _logger.debug(f"person {self}, project {project}") # Get the roles this person has had on the given project roles = ProjectRole.objects.filter(person=self, project=project) - print(f"person {self}, roles {roles}") + _logger.debug(f"person {self}, roles {roles}") # If there are no roles, return a timedelta of zero duration if not roles.exists(): return timedelta() @@ -261,7 +264,7 @@ def get_total_time_on_project(self, project): ) ).aggregate(total_time=Sum('time_worked'))['total_time'] - print(f"person {self}, total_time_worked", total_time_worked) + _logger.debug(f"person {self}, total_time_worked {total_time_worked}") # Returns a timedelta object, which is part of Python’s datetime module and it represents # a duration, or the difference between two dates or times. @@ -306,8 +309,12 @@ def is_active(self): @cached_property def has_started(self): """Returns True if person has started in the lab. False otherwise.""" - return self.get_latest_position.has_started() - + latest_position = self.get_latest_position + if latest_position is None: + return False + else: + return latest_position.has_started() + def get_total_time_in_role(self, role): """Returns the total time as in the specified role across all positions as a DurationField""" duration = ExpressionWrapper(Coalesce(F('end_date'), date.today()) - F('start_date'), output_field=fields.DurationField()) @@ -406,11 +413,9 @@ def get_earliest_position_in_role(self, role, contiguous_constraint=True): next_position = cur_position elif (next_position.start_date - cur_position.end_date) <= max_time_gap: time_gap = (next_position.start_date - cur_position.end_date) - # print("Met minimum time gap: gap= {} max_gap={}".format(time_gap, max_time_gap)) next_position = cur_position else: time_gap = (next_position.start_date - cur_position.end_date) - # print("Exceeded minimum time gap: gap= {} max_gap={}".format(time_gap, max_time_gap)) break return next_position @@ -474,9 +479,9 @@ def get_full_name(self, include_middle=True): :return: the person's full name as a string """ if self.middle_name and include_middle: - return u"{0} {1} {2}".format(self.first_name, self.middle_name, self.last_name) + return f"{self.first_name} {self.middle_name} {self.last_name}" else: - return u"{0} {1}".format(self.first_name, self.last_name) + return f"{self.first_name} {self.last_name}" get_full_name.short_description = "Full Name" get_full_name.admin_order_field = 'first_name' # Allows column order sorting based on full name @@ -540,12 +545,12 @@ def get_projects(self): def get_mentees(self, randomize=False): """ - Returns a list of all students this person has mentored + Returns a QuerySet of all students this person has mentored. + Uses the reverse relation from Position.grad_mentor. """ - grad_mentors = Position.objects.filter(grad_mentor=self).values('person') - mentees = Person.objects.filter(id__in=grad_mentors) + # Use the reverse relation 'Grad_Mentor' from Position model + mentees = Person.objects.filter(position__grad_mentor=self).distinct() - # Randomize the order of the mentees if randomize is True if randomize: mentees = mentees.order_by('?') @@ -553,63 +558,54 @@ def get_mentees(self, randomize=False): def get_grad_mentors(self): """ - Retrieve a list of grad mentors for the current person instance. - - This method filters the Person objects to find those who are listed as - grad mentors for the current person in the Position model. It ensures - that each mentor is listed only once using the distinct() method. - + Retrieve a QuerySet of grad mentors for the current person instance. + Returns: - QuerySet: A QuerySet of Person objects who are grad mentors for the current person. + QuerySet: Person objects who are grad mentors for the current person. """ - positions = Position.objects.filter(person=self) - grad_mentors = positions.values('grad_mentor') - return Person.objects.filter(id__in=grad_mentors) + # Get all grad mentors from this person's positions + return Person.objects.filter( + id__in=self.position_set.exclude( + grad_mentor__isnull=True + ).values_list('grad_mentor', flat=True) + ).distinct() def get_projects_sorted_by_contrib(self, filter_out_projs_with_zero_pubs=True): - """Returns a set of all projects this person is involved in ordered by number of pubs""" - map_project_name_to_tuple = dict() # tuple is (count, most_recent_pub_date, project) - #publications = self.publication_set.order_by('-date') - - # Go through all the projects by this person and track how much - # they've contributed to each one (via publication) - #print("******{}*******".format(self.get_full_name())) - for pub in self.publication_set.all(): - for proj in pub.projects.all(): - #print("pub", pub, "proj", proj) - if proj.name not in map_project_name_to_tuple: - most_recent_date = proj.start_date - if most_recent_date is None: - most_recent_date = pub.date - if most_recent_date is None: - most_recent_date = datetime.date(2012, 1, 1) # when the lab was founded - - map_project_name_to_tuple[proj.name] = (0, most_recent_date, proj) - - tuple_cnt_proj = map_project_name_to_tuple[proj.name] - most_recent_date = tuple_cnt_proj[1] - if pub.date is not None and pub.date > most_recent_date: - most_recent_date = pub.date - - map_project_name_to_tuple[proj.name] = (tuple_cnt_proj[0] + 1, # pub cnt - most_recent_date, # most recent pub date - tuple_cnt_proj[2]) # project - - list_tuples = list([tuple_cnt_proj for tuple_cnt_proj in map_project_name_to_tuple.values()]) - list_tuples_sorted = sorted(list_tuples, key=lambda t: (t[0], t[1]), reverse=True) - - #print("list_tuples_sorted", list_tuples_sorted) - - ordered_projects = [] - if len(list_tuples_sorted) > 0: - list_cnts, list_dates, ordered_projects = zip(*list_tuples_sorted) - - if len(ordered_projects) <= 0 and not filter_out_projs_with_zero_pubs: - # if a person hasn't published but is still on projects - # default to this - ordered_projects = self.get_projects() - - return ordered_projects + """ + Returns projects involving this person, sorted by the number of + publications and the date of the most recent publication. + """ + Project = apps.get_model('website', 'Project') + + # Start with projects where this person has a role + projects_qs = Project.objects.filter( + projectrole__person=self + ).annotate( + # Count publications by this person on each project + pub_count=Count( + 'publication', + filter=Q(publication__authors=self), + distinct=True + ), + # Get most recent publication date by this person + most_recent_pub_date=Max( + 'publication__date', + filter=Q(publication__authors=self) + ) + ).annotate( + # Fallback date logic: pub date > project start > lab founding + sort_date=Coalesce( + 'most_recent_pub_date', + 'start_date', + Value(settings.DATE_MAKEABILITYLAB_FORMED) + ) + ).order_by('-pub_count', '-sort_date').distinct() + + # Filter out projects with zero publications if requested + if filter_out_projs_with_zero_pubs: + projects_qs = projects_qs.filter(pub_count__gt=0) + + return projects_qs def __str__(self): diff --git a/website/models/position.py b/website/models/position.py index e5e83331..9fce1d57 100644 --- a/website/models/position.py +++ b/website/models/position.py @@ -78,6 +78,10 @@ class Position(models.Model): Title.UNKNOWN: 11 } + # BETTER - Use class constant: + PROFESSOR_TITLES = {Title.FULL_PROF, Title.ASSOCIATE_PROF, Title.ASSISTANT_PROF} + GRAD_STUDENT_TITLES = {Title.MS_STUDENT, Title.PHD_STUDENT} + def save(self, *args, **kwargs): # Save the Position instance first super(Position, self).save(*args, **kwargs) @@ -150,15 +154,12 @@ def is_member(self): return self.role == Role.MEMBER def is_professor(self): - """Returns true if professor""" - return (self.title == Title.FULL_PROF or - self.title == Title.ASSOCIATE_PROF or - self.title == Title.ASSISTANT_PROF) + """Returns True if this position is a professor.""" + return self.title in self.PROFESSOR_TITLES def is_grad_student(self): - """Returns true if grad student""" - return (self.title == Title.MS_STUDENT or - self.title == Title.PHD_STUDENT) + """Returns True if this position is a grad student.""" + return self.title in self.GRAD_STUDENT_TITLES def is_high_school(self): """Returns true if high school student""" @@ -207,6 +208,35 @@ def __str__(self): return "Name={}, Role={}, Title={}, Start={} End={}".format( self.person.get_full_name(), self.role, self.title, self.start_date, self.end_date) + # In position.py, add to the Position class: + + @staticmethod + def get_indefinite_article_for_title(title): + """ + Returns the appropriate indefinite article ('a' or 'an') for a given title. + + Args: + title: Title enum value or string + + Returns: + 'a' or 'an' depending on whether the title starts with a vowel sound + + Example: + >>> Position.get_indefinite_article(Title.UGRAD) + 'an' + >>> Position.get_indefinite_article(Title.PHD_STUDENT) + 'a' + """ + # Titles that require "an" (start with vowel sound) + titles_needing_an = { + Title.UGRAD, # "Undergrad" + Title.MS_STUDENT, # "MS Student" (M sounds like "em") + Title.ASSISTANT_PROF, # "Assistant Professor" + Title.ASSOCIATE_PROF, # "Associate Professor" + } + + return "an" if title in titles_needing_an else "a" + @staticmethod def get_sorted_abstracted_titles(): """Static method returns a sorted list of abstracted title names""" diff --git a/website/static/website/css/design-tokens.css b/website/static/website/css/design-tokens.css index 554d55c2..3e1c6fe1 100644 --- a/website/static/website/css/design-tokens.css +++ b/website/static/website/css/design-tokens.css @@ -165,6 +165,7 @@ --space-4: 1rem; /* 16px */ --space-5: 1.25rem; /* 20px */ --space-6: 1.5rem; /* 24px */ + --space-7: 1.75rem; /* 28px */ --space-8: 2rem; /* 32px */ --space-10: 2.5rem; /* 40px */ --space-12: 3rem; /* 48px */ diff --git a/website/static/website/css/project.css b/website/static/website/css/project.css index 0e758fc1..481c0dad 100644 --- a/website/static/website/css/project.css +++ b/website/static/website/css/project.css @@ -69,7 +69,7 @@ @media (min-width: 992px) { .project-layout { grid-template-columns: 1fr 320px; - gap: var(--space-8); + gap: var(--space-12); } } @@ -261,7 +261,7 @@ @media (min-width: 992px) { .project-sidebar-section { - margin-bottom: var(--space-6); + margin-bottom: var(--space-7); } } @@ -271,7 +271,7 @@ ============================================================================= */ .project-sidebar-header { - margin: 0 0 var(--space-2) 0; + margin: var(--space-7) 0 var(--space-2) 0; font-family: var(--font-family-primary); font-size: var(--font-size-sm); font-weight: var(--font-weight-semibold); @@ -280,6 +280,10 @@ letter-spacing: 0.03em; } +.project-sidebar-header.first-section{ + margin-top: var(--space-5); +} + /* ============================================================================= SIDEBAR TEXT & LINKS diff --git a/website/templates/website/project.html b/website/templates/website/project.html index 103a5db0..06e5026a 100644 --- a/website/templates/website/project.html +++ b/website/templates/website/project.html @@ -54,7 +54,7 @@ - has_videos_beyond_featured_video: Boolean - talks: QuerySet of Talk objects -@version 2.0.0 - Accessibility and design token refactor +@version 2.0.1 - Updated sidebar spacing @author Makeability Lab ================================================================================ {% endcomment %} @@ -284,7 +284,7 @@

Project Information

-

Date

+

Date

{{ date_str }}

diff --git a/website/views/member.py b/website/views/member.py index e777700e..7ffbdf9f 100644 --- a/website/views/member.py +++ b/website/views/member.py @@ -1,5 +1,5 @@ from django.conf import settings # for access to settings variables, see https://docs.djangoproject.com/en/4.0/topics/settings/#using-settings-in-python-code -from website.models import Banner, Person, News, Talk, Video, Publication +from website.models import Person, News, Video, Position import website.utils.ml_utils as ml_utils from django.shortcuts import render, get_object_or_404, redirect from django.db.models import Q @@ -17,17 +17,11 @@ def member(request, member_name=None, member_id=None): func_start_time = time.perf_counter() _logger.debug(f"Starting views/member member_id={member_id} and member_name={member_name} at {func_start_time:0.4f}") - # person = None # This code block gets a person object either from the member id or the url_name. # If the member_id is a digit, it's assumed to be the primary key (pk) of the Person object # The get_object_or_404 function is then used to retrieve the Person object with this pk. # If no such Person object exists, the function will raise a 404 error. # If the member_id is not a digit, it's assumed to be the url-friendly name (url_name). - # if (member_id.isdigit()): - # person = get_object_or_404(Person, pk=member_id) - # else: - # person = get_object_or_404(Person, url_name__iexact=member_id) - person = None if member_id is not None: _logger.debug(f"Found a member_id={member_id}, checking for a person with that id") @@ -69,9 +63,22 @@ def member(request, member_name=None, member_id=None): publications = person.publication_set.order_by('-date') talks = person.talk_set.order_by('-date') videos = get_videos_by_author(person) - project_roles = person.projectrole_set.order_by('start_date') + project_roles = person.projectrole_set.order_by('-start_date') projects = person.get_projects + # Sort projects: active first (not ended), then by most recent start_date + projects = sorted( + projects, + key=lambda proj: ( + # First sort key: active projects first + # has_ended() returns False for active, True for ended + # Since False < True, active projects come first + proj.has_ended(), + # Second sort key: most recent start_date first (descending) + -(proj.start_date.toordinal() if proj.start_date else 0) + ) + ) + left_align_headers = (len(projects) <= 4 and len(publications) <= 3 and len(talks) <= 3 and len(videos) <= 3) @@ -119,23 +126,15 @@ def member(request, member_name=None, member_id=None): def get_videos_by_author(person): """Returns a queryset of videos that the given person is an author on""" - # Get all Publications where the given person is an author - publication_videos = Publication.objects.filter(authors=person).values_list('video', flat=True) - - # Get all Talks where the given person is an author - talk_videos = Talk.objects.filter(authors=person).values_list('video', flat=True) - - # Combine the two querysets and order by date - videos = Video.objects.filter(Q(id__in=publication_videos) | - Q(id__in=talk_videos)).order_by('-date') - - return videos + return Video.objects.filter( + Q(publication__authors=person) | Q(talk__authors=person) + ).distinct().order_by('-date') def auto_generate_bio(person): - """Auto-generates a bio using list construction to prevent grammar errors.""" + """Auto-generates a bio using stored info about person.""" # 1. Generate the Role Sentence - role_parts = [person.get_full_name()] + role_parts = [] # Calculate duration safely total_time = person.get_total_time_in_lab() @@ -143,19 +142,28 @@ def auto_generate_bio(person): if not person.has_started: date_str = f" on {person.get_latest_position.start_date}" if person.get_latest_position and person.get_latest_position.start_date else "" - role_parts.append(f"will be joining the Makeability Lab{date_str}.") + role_parts.append(f"{person.get_full_name()} will be joining the Makeability Lab{date_str}.") elif person.is_current_member: - role_parts.append(f"is currently a {person.get_current_title} in the Makeability Lab.") + article = Position.get_indefinite_article_for_title(person.get_current_title) + role_parts.append(f"{person.get_full_name()} is currently {article} {person.get_current_title} in the Makeability Lab.") if duration_str: role_parts.append(f"{person.first_name} has been in the lab for {duration_str}.") elif person.is_alumni_member: - role_parts.append(f"was a {person.get_current_title} in the Makeability Lab") + article = Position.get_indefinite_article_for_title(person.get_current_title) + role_parts.append(f"{person.get_full_name()} was {article} {person.get_current_title} in the Makeability Lab") if duration_str: role_parts.append(f"for {duration_str}") start = person.get_start_date.strftime("%b %Y") end = person.get_end_date.strftime("%b %Y") if person.get_end_date else "present" role_parts.append(f"({start} to {end}).") + elif person.is_current_collaborator: + role_parts.append(f"{person.get_full_name()} is a collaborator with the Makeability Lab.") + elif person.is_past_collaborator: + role_parts.append(f"{person.get_full_name()} was a collaborator with the Makeability Lab.") + else: + # Fallback for edge cases + role_parts.append(f"{person.get_full_name()} has published with the Makeability Lab.") # Combine the introductory sentences bio_sentences = [" ".join(role_parts).replace(" .", ".")] @@ -169,7 +177,7 @@ def auto_generate_bio(person): pub_count = person.publication_set.count() if proj_count > 0 or pub_count > 0: - contrib_str = f"{person.first_name} contributed to" + contrib_str = f"They contributed to" # --- Build Project String --- if proj_count > 0: From 846c84e656f67a095d3ceae68a56aaf890b830f9 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Sun, 21 Dec 2025 16:39:01 -0800 Subject: [PATCH 2/3] added more access urls --- .pa11yci.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.pa11yci.json b/.pa11yci.json index c600a0e0..e5c47e3e 100644 --- a/.pa11yci.json +++ b/.pa11yci.json @@ -16,9 +16,10 @@ "http://website:8000/people/", "http://website:8000/talks/", "http://website:8000/news/", - "http://website:8000/member/jonfroehlich/" + "http://website:8000/member/jonfroehlich/", + "http://website:8000/project/sidewalk/" ], "urls": [ - "http://website:8000/news/" + "http://website:8000/project/sidewalk/" ] } \ No newline at end of file From 625a4dea5edd4004df64d303eed65860284fc152 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Sun, 21 Dec 2025 16:39:47 -0800 Subject: [PATCH 3/3] bumped version to 2.1.2 --- makeabilitylab/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/makeabilitylab/settings.py b/makeabilitylab/settings.py index 47e8e70e..67def47f 100644 --- a/makeabilitylab/settings.py +++ b/makeabilitylab/settings.py @@ -72,8 +72,8 @@ ALLOWED_HOSTS = ['*'] # Makeability Lab Global Variables, including Makeability Lab version -ML_WEBSITE_VERSION = "2.1.1" # Keep this updated with each release and also change the short description below -ML_WEBSITE_VERSION_DESCRIPTION = "Updated project page accessibility" +ML_WEBSITE_VERSION = "2.1.2" # Keep this updated with each release and also change the short description below +ML_WEBSITE_VERSION_DESCRIPTION = "Fixed a number of bugs, including project ordering on member pages" DATE_MAKEABILITYLAB_FORMED = datetime.date(2012, 1, 1) # Date Makeability Lab was formed MAX_BANNERS = 7 # Maximum number of banners on a page