Merge pull request #1427 from makeabilitylab/1426-api-position-title-school

Expose school on people and position-during-role on project people (#1426)

Author: jonfroehlich

docs/API.md

@@ -71,8 +71,16 @@ sub-resources are keyed by `short_name`:
 - `GET /api/v1/projects/<short_name>/publications/` — the project's pubs.
 - `GET /api/v1/projects/<short_name>/grants/` — grants funding the project.
 - `GET /api/v1/projects/<short_name>/people/` — everyone with a role on the
-  project, each as a `{ person, role, lead_project_role, start_date, end_date,
+  project, each as a `{ person, role, lead_project_role, position_title,
+  position_school, position_school_abbreviated, start_date, end_date,
   is_active }` record (a person may appear more than once for multiple roles).
+  `role` is the editor-written free-text description of what they did; the
+  `position_*` fields are what they **were at the time** — the title and school
+  from the Position they held when the role started, so a 2015 stint reads
+  `"Undergrad"` / `"UMD"` even if that person is a professor today. Where the
+  role's start date falls outside every recorded position, the most recent
+  position already begun by then is used — or, for a role predating them all,
+  the earliest. All three are `null` for someone with no Position on record.
 - `GET /api/v1/projects/<short_name>/leadership/` — **all** leadership across
   all time (current *and* past), grouped:
   `{ pis, co_pis, student_leads, postdoc_leads, research_scientist_leads }`,
@@ -91,8 +99,14 @@ Funding amounts are intentionally **not** exposed by the API.
 
 Actual lab members (people with at least one Position); external co-authors are
 not listed here even though they appear as publication `authors`. Detail by
-`url_name`: `GET /api/v1/people/<url_name>/` — name, current title, bio,
-thumbnail, and public social/web links (ORCID, Google Scholar, GitHub, etc.).
+`url_name`: `GET /api/v1/people/<url_name>/` — name, `current_title`,
+`current_school`, `current_department`, bio, thumbnail, and public social/web
+links (ORCID, Google Scholar, GitHub, etc.).
+
+> **Note:** the `current_*` fields come from the person's *latest* Position, so
+> for an alum they describe their last lab position, not their present-day
+> employer. For what someone was during a specific project stint, use the
+> `position_*` fields on `/projects/<short_name>/people/`.
 
 > **Note:** `email` is intentionally **not** exposed by the API to avoid making
 > it an email-harvesting surface, even where it appears on a member page.

makeabilitylab/settings.py

@@ -86,8 +86,8 @@
     SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https')
 
 # Makeability Lab Global Variables, including Makeability Lab version
-ML_WEBSITE_VERSION = "2.27.4" # Keep this updated with each release and also change the short description below
-ML_WEBSITE_VERSION_DESCRIPTION = "Infrastructure-only release: hardens the auto-deploy webhook (auto-deploy/index.php, #1422) after the test server spent Jul 18-22 rebuilding on every push while serving source frozen at Jul 18. Root cause was a one-character bug -- the deploy condition read `$OPERATION = 'TAG'` (assignment) instead of `==`, so ANY tag push satisfied it on EVERY configured host and reassigned $OPERATION for the rest of the request; a tag push therefore drove the branch-tracking test host down the tag code path and left its checkout detached at that tag. It could not recover, because `git pull` aborts from a detached HEAD and the pull ran before the branch checkout. Compounding it, the container build ran regardless of whether the pull or checkout succeeded, so a failed update still produced a successful-looking rebuild -- which is why it went unnoticed for four days. Fixed: `==`; branch hosts now check out the branch before pulling (tags keep fetch-first, since the tag is not local until after the fetch); and the container build is gated on the checked-out HEAD actually matching the pushed commit, aborting with a log line instead of silently shipping stale source. The gate peels annotated tags with ^{commit} (our release tags are annotated, and `after` carries the tag object's sha, not the commit's) and fails safe -- if the sha cannot be resolved locally it deploys as before. Also fixed three more instances of the same =/== footgun (_trace/_debug were always on), a misspelled $reqest variable that made the branch fast-path dead code, and the incoming ref is now validated against refs/(heads|tags)/[A-Za-z0-9._/-]+ before it reaches a shell. No application code changed."
+ML_WEBSITE_VERSION = "2.28.0" # Keep this updated with each release and also change the short description below
+ML_WEBSITE_VERSION_DESCRIPTION = "Adds five fields to the public v1 REST API so Project Sidewalk's new About page can render its team section straight from this site instead of hand-maintaining a roster (#1426). /api/v1/people/<url_name>/ gains current_school and current_department alongside the existing current_title; /api/v1/projects/<short_name>/people/ gains position_title, position_school, and position_school_abbreviated -- what the person WAS when they joined the project, not what they are now, so a 2015 stint reads 'Undergrad, UMD' even though that person may be a professor today. (Jon's own 2012 Sidewalk role now correctly reports Assistant Professor / UMD rather than Professor / UW.) The new fields are named position_* rather than a bare 'title' so they don't read as part of 'role', the editor-written free-text description sitting beside them in the same record. Which position gets picked is an explicit, documented rule on ProjectRole.position_during_role: the position containing the role's start date, else the latest one already begun (the role started in a gap between positions), else the earliest (the role predates every recorded position -- common in older imported data), and null only for someone with no Position at all; ties on start date break on pk so two positions beginning the same day can't let DB row order decide the answer from one request to the next. Both project sub-resources now prefetch person__position_set and the people list prefetches position_set, so resolving a position per row rides the prefetch instead of costing a query each -- that matters at Sidewalk's ~150-person roster, and it also fixes a pre-existing N+1 behind current_title; a regression test pins the query count as flat as the roster grows. Separately, get_school_abbreviated is tightened twice: it no longer acronyms one-word affiliations (not every school is a university, and 'Easterseals' -> 'E' (or 'Cornell' -> 'C') is worse than the name itself), and its UW/UMD special cases now match the phrase 'university of washington/maryland' rather than a bare 'washington'/'maryland' substring, which used to hand UW's initials to Washington State and Washington University in St. Louis, and UMD's to UMBC. That is a visible fix on the public People page too, not just in the API. No model changes and no migration; v1 stays additive-only, so existing consumers are unaffected."
 DATE_MAKEABILITYLAB_FORMED = datetime.date(2012, 1, 1)  # Date Makeability Lab was formed
 MAX_BANNERS = 7 # Maximum number of banners on a page
 

website/api/serializers.py

@@ -75,18 +75,27 @@ def get_thumbnail(self, obj):
 class PersonSerializer(PersonSummarySerializer):
     """Full person representation for the people detail/list endpoints.
 
-    Extends the summary with bio, current title, and the public social/web
+    Extends the summary with bio, current affiliation, and the public social/web
     links. ``email`` is intentionally omitted (see module docstring).
+
+    ``current_title`` / ``current_school`` / ``current_department`` all come from
+    the person's *latest* Position, so for an alum they describe their last lab
+    position, not their present-day employer. For what someone was during a
+    specific project stint, use ``ProjectRoleSerializer``'s ``position_*`` fields.
     """
 
     current_title = serializers.SerializerMethodField()
+    current_school = serializers.SerializerMethodField()
+    current_department = serializers.SerializerMethodField()
 
     class Meta(PersonSummarySerializer.Meta):
         fields = PersonSummarySerializer.Meta.fields + [
             "first_name",
             "middle_name",
             "last_name",
             "current_title",
+            "current_school",
+            "current_department",
             "bio",
             "personal_website",
             "github",
@@ -103,6 +112,14 @@ def get_current_title(self, obj):
         # Person.get_current_title is a cached_property, not a method.
         return obj.get_current_title
 
+    def get_current_school(self, obj):
+        # Person.get_current_school is a cached_property, not a method.
+        return obj.get_current_school
+
+    def get_current_department(self, obj):
+        # Person.get_current_department is a cached_property, not a method.
+        return obj.get_current_department
+
 
 class ProjectSummarySerializer(serializers.ModelSerializer):
     """Compact project representation, used when nested in publications/grants."""
@@ -262,21 +279,50 @@ def get_bibtex(self, obj):
 
 
 class ProjectRoleSerializer(serializers.ModelSerializer):
-    """A person's role on a project (start/end, lead type, active flag)."""
+    """A person's role on a project (start/end, lead type, active flag).
+
+    The ``position_*`` fields describe what the person *was* when they joined the
+    project -- title and school from the Position held at the role's start date
+    (#1426) -- so a 2015 stint reads "Undergrad, UMD" even if that person is a
+    professor today. They're ``null`` for someone with no Position on record.
+    Named ``position_*`` (not bare ``title``) so they don't read as part of
+    ``role``, which is the editor-written free-text description of the work.
+    """
 
     person = PersonSummarySerializer(read_only=True)
     is_active = serializers.SerializerMethodField()
+    position_title = serializers.SerializerMethodField()
+    position_school = serializers.SerializerMethodField()
+    position_school_abbreviated = serializers.SerializerMethodField()
 
     class Meta:
         model = ProjectRole
         fields = [
             "person",
             "role",
             "lead_project_role",
+            "position_title",
+            "position_school",
+            "position_school_abbreviated",
             "start_date",
             "end_date",
             "is_active",
         ]
 
     def get_is_active(self, obj):
         return obj.is_active()
+
+    # ProjectRole.position_during_role is a cached_property, so the three fields
+    # below resolve it once per row (and ride the view's prefetch -- see the
+    # model helper).
+    def get_position_title(self, obj):
+        position = obj.position_during_role
+        return position.title if position else None
+
+    def get_position_school(self, obj):
+        position = obj.position_during_role
+        return position.school if position else None
+
+    def get_position_school_abbreviated(self, obj):
+        position = obj.position_during_role
+        return position.get_school_abbreviated() if position else None

website/api/views.py

@@ -122,8 +122,12 @@ class PersonViewSet(ReadOnlyModelViewSet):
     lookup_field = "url_name"
 
     def get_queryset(self):
+        # position_set is prefetched because current_title/current_school/
+        # current_department all funnel through Person.get_latest_position,
+        # which resolves in Python off position_set.all().
         return (
             Person.objects.filter(position__isnull=False)
+            .prefetch_related("position_set")
             .distinct()
             .order_by("last_name", "first_name")
         )
@@ -199,6 +203,7 @@ def people(self, request, short_name=None):
         qs = (
             ProjectRole.objects.filter(project=project)
             .select_related("person")
+            .prefetch_related("person__position_set")
             .order_by("person__last_name", "person__first_name", "start_date")
         )
         return self._paginated(qs, ProjectRoleSerializer)
@@ -218,6 +223,7 @@ def leadership(self, request, short_name=None):
                 project=project, lead_project_role__in=list(_LEAD_BUCKETS)
             )
             .select_related("person")
+            .prefetch_related("person__position_set")
             .order_by("-start_date")
         )
         context = self.get_serializer_context()

website/models/project_role.py

@@ -1,4 +1,5 @@
 from django.db import models
+from django.utils.functional import cached_property
 from datetime import date, datetime, timedelta
 
 class LeadProjectRoleTypes(models.TextChoices):
@@ -70,6 +71,62 @@ def get_date_range_as_str(self):
         else:
             return f"{self.start_date.year}-{self.end_date.year}"
 
+    @cached_property
+    def position_during_role(self):
+        """The Position this person held when they started this project role.
+
+        This is deliberately *not* the person's current title: a 2015 undergrad
+        who is now a professor should read "Undergrad" next to a 2015 project
+        stint (see issue #1426). Where someone's title changed mid-stint (an
+        undergrad who stayed on for an MS), we report what they were when they
+        joined the project.
+
+        Resolution order, most to least faithful:
+
+        1. The latest-starting Position whose date range contains this role's
+           ``start_date``.
+        2. Failing that, the latest-starting Position that had already begun by
+           ``start_date`` (the role started in a gap between positions).
+        3. Failing that, the earliest Position (the role predates every recorded
+           position -- data drift, common for older imported roles).
+        4. ``None`` only if the person has no Positions at all.
+
+        Ties on ``start_date`` (a person holding two positions that begin the
+        same day -- concurrent Member/Collaborator rows, or a duplicate entry)
+        break on ``pk``, so the answer is stable across requests: ``position_set``
+        has no ``Meta.ordering``, so without an explicit tie-break the winner
+        would follow whatever order the DB happened to return.
+
+        A ``cached_property`` (like :meth:`Person.get_current_title`) because the
+        API reads it three times per row -- for title, school, and abbreviated
+        school. Filters ``position_set`` in Python rather than issuing a query,
+        so a caller with ``prefetch_related('person__position_set')`` (e.g. the
+        API's project-people endpoint) resolves it with zero extra queries. This
+        mirrors :meth:`Person.get_latest_position`; positions-per-person is tiny.
+
+        Example:
+            >>> role.position_during_role.title
+            'Undergrad'
+        """
+        positions = list(self.person.position_set.all())
+        if not positions:
+            return None
+
+        # Sorts rather than max()/min() so the pk tie-break is stated once.
+        positions.sort(key=lambda p: (p.start_date, p.pk))
+
+        containing = [p for p in positions
+                      if p.start_date <= self.start_date
+                      and (p.end_date is None or p.end_date >= self.start_date)]
+        if containing:
+            return containing[-1]
+
+        already_started = [p for p in positions if p.start_date <= self.start_date]
+        if already_started:
+            return already_started[-1]
+
+        return positions[0]
+
     def get_pi_status_index(self):
         if self.lead_project_role is not None and self.lead_project_role in self.LEAD_PROJECT_ROLE_MAPPING:
             return self.LEAD_PROJECT_ROLE_MAPPING[self.lead_project_role]