API: project roster titles track the latest position overlapping the role (#1435)

docs/API.md

@@ -76,12 +76,18 @@ sub-resources are keyed by `short_name`:
   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.
+  `position_*` fields describe them **over the span of that role** — the title
+  and school from the latest Position overlapping the role's window
+  (`start_date` → `end_date`, or today for an open role; never past today). So
+  an ongoing role reads as what the person is *now* (`"Professor"` / `"UW"`),
+  while a stint that ended in 2016 still reads `"Undergrad"` / `"UMD"` even if
+  that person is a professor today. Where a role's window falls outside every
+  recorded position, the most recent position already begun by its end is used —
+  or, for a role predating them all, the earliest. All three are `null` for
+  someone with no Position on record.
+  > Use these rather than `/people/<url_name>/`'s `current_*` fields for a roster:
+  > they're scoped to the project, so a past contributor reads as what they were
+  > on the project, not their last lab position.
 - `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 }`,
@@ -107,8 +113,10 @@ etc.). See *Images* below.
 
 > **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/`.
+> employer. For someone's title during a specific project stint — including a
+> current one — use the `position_*` fields on
+> `/projects/<short_name>/people/`; they're project-scoped, and for an active
+> role they carry the same present-day title anyway.
 
 > **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.
@@ -145,6 +153,13 @@ disk and will 404. Ask for a different size in an issue instead.
     derivative described above. A field named `thumbnail` handing out an 11 MB
     original was a bug, not a contract — 13 headshots cost one consumer 33.5 MB.
     The raw file is still available as `image_original`.
+  - A second, in **2.31.0** (#1435): the `position_*` fields on
+    `/projects/<short_name>/people/` were pinned to the Position held on the
+    role's *start* date, so a role open since 2012 still reported a title from
+    2012. They now report the latest Position overlapping the role, which only
+    changes values for roles spanning a title change — and changes them to the
+    ones a roster actually wants. Finished stints are unaffected unless the
+    person was promoted mid-stint.
 - Don't hardcode pagination page sizes as a proxy for "all" — page through
   `next`, or set `page_size` explicitly (≤100).
 - URLs in responses (PDFs, thumbnails, page links) are absolute and safe to use

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.30.0" # Keep this updated with each release and also change the short description below
-ML_WEBSITE_VERSION_DESCRIPTION = "The public API now serves cropped, sized photos (#1432): a person or project thumbnail honors the crop box you set in the admin instead of handing out the raw upload, which cost one consumer 33.5 MB for 13 headshots. The full-resolution file is still available."
+ML_WEBSITE_VERSION = "2.31.0" # Keep this updated with each release and also change the short description below
+ML_WEBSITE_VERSION_DESCRIPTION = "A project's roster in the public API now reports each person's title over the span of their role (#1435), so someone on a project since 2012 reads as what they are today rather than the title they held back then. Finished stints still read as what that person was at the time."
 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

@@ -340,12 +340,13 @@ def get_bibtex(self, obj):
 class ProjectRoleSerializer(serializers.ModelSerializer):
     """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.
+    The ``position_*`` fields describe the person *over the span of this role* --
+    title and school from the latest Position overlapping it (#1426, #1435). An
+    ongoing role reports what they are today ("Professor, UW"); a stint that
+    ended in 2016 still reads "Undergrad, UMD" even if that person is a professor
+    now. 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)

website/models/project_role.py

@@ -73,24 +73,47 @@ def get_date_range_as_str(self):
 
     @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.
+        """The Position that best describes this person over this project role.
+
+        Specifically, the *latest* Position overlapping the role's window --
+        ``[start_date, end_date or today]``, with the end never past today. That
+        gives the two answers a roster wants (#1426, refined in #1435):
+
+        - An **ongoing** role reports what the person is *now*. Someone who has
+          led a project since 2012 and was promoted along the way reads
+          "Professor, University of Washington", not the assistant professorship
+          they held when the stint began.
+        - A **finished** stint stays frozen in its own time. A 2015 undergrad who
+          is a professor today still reads "Undergrad" next to that stint, because
+          the professorship never overlapped it. Where their title changed
+          mid-stint (an undergrad who stayed on for an MS), the later title wins.
+
+        This is why it is *not* :meth:`Person.get_current_title`, which is the
+        person's latest Position full stop -- for an alum, their last lab position
+        rather than either of the above. The one case where the two converge is a
+        role left open (``end_date`` null) after the person has actually left: its
+        window runs to today, so it reports their last lab position, same as
+        ``get_current_title`` would. That's a data-entry gap, not a rule -- both
+        :meth:`Position.save` (which closes a departing member's open roles) and
+        the ``auto_close_project_roles`` command exist to keep it from arising.
 
         Resolution order, most to least faithful:
 
-        1. The latest-starting Position whose date range contains this role's
-           ``start_date``.
+        1. The latest-starting Position overlapping the role's window.
         2. Failing that, the latest-starting Position that had already begun by
-           ``start_date`` (the role started in a gap between positions).
+           the window's end (the whole role sits 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.
 
+        The window end is clamped twice. Up top, to today, so a Position starting
+        in the future (a promotion entered ahead of time) never reaches today's
+        payload -- including on a role carrying a future ``end_date``, which an
+        editor may enter for a planned wrap-up. Down below, to at least the start
+        date, so a typo'd ``end_date`` earlier than ``start_date`` degrades to
+        "position at role start" instead of inverting the range and matching
+        nothing.
+
         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``
@@ -105,8 +128,10 @@ def position_during_role(self):
         mirrors :meth:`Person.get_latest_position`; positions-per-person is tiny.
 
         Example:
+            >>> role.end_date is None          # still on the project
+            True
             >>> role.position_during_role.title
-            'Undergrad'
+            'Professor'
         """
         positions = list(self.person.position_set.all())
         if not positions:
@@ -115,13 +140,19 @@ def position_during_role(self):
         # 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]
+        today = date.today()
+        window_start = self.start_date
+        # min() first so a future end_date can't admit a not-yet-started
+        # position; max() second so a typo'd end_date can't invert the window.
+        window_end = max(min(self.end_date or today, today), window_start)
+
+        overlapping = [p for p in positions
+                       if p.start_date <= window_end
+                       and (p.end_date is None or p.end_date >= window_start)]
+        if overlapping:
+            return overlapping[-1]
 
-        already_started = [p for p in positions if p.start_date <= self.start_date]
+        already_started = [p for p in positions if p.start_date <= window_end]
         if already_started:
             return already_started[-1]
 

website/tests/test_api.py

@@ -225,7 +225,7 @@ def test_project_people_subresource(self):
         lead_roles = {r["lead_project_role"] for r in body["results"]}
         self.assertEqual(lead_roles, {"PI", "Co-PI", "Student Lead"})
 
-    # ---- position held during a project role (#1426) ------------------------
+    # ---- position held during a project role (#1426, #1435) -----------------
     #
     # These pin the *wire contract* -- that the resolved Position reaches the
     # payload, and at what cost. The resolution rules themselves (which Position
@@ -271,8 +271,8 @@ def _role_record(self, person):
         return matches[0]
 
     def test_project_people_exposes_position_during_role(self):
-        """The roll call wants what someone *was* at the time, not their current
-        title: an undergrad who later did an MS reads 'Undergrad', 'UMD'."""
+        """A finished stint reports the latest position that overlapped it -- an
+        undergrad who moved to an MS mid-stint reads 'MS Student', 'UW'."""
         person = self._make_role_holder(
             "Multi",
             positions=[
@@ -285,9 +285,28 @@ def test_project_people_exposes_position_during_role(self):
             role_end=date(2018, 1, 1),
         )
         record = self._role_record(person)
-        self.assertEqual(record["position_title"], "Undergrad")
-        self.assertEqual(record["position_school"], "University of Maryland")
-        self.assertEqual(record["position_school_abbreviated"], "UMD")
+        self.assertEqual(record["position_title"], "MS Student")
+        self.assertEqual(record["position_school"], "University of Washington")
+        self.assertEqual(record["position_school_abbreviated"], "UW")
+
+    def test_project_people_active_role_reports_present_day_position(self):
+        """#1435: a still-open role reports what the person is *today*, not what
+        they were when the role began -- the case that had consumers fetching
+        /people/<url_name>/ per roster row just to get a current title."""
+        person = self._make_role_holder(
+            "Promoted",
+            positions=[
+                (Title.ASSISTANT_PROF, "University of Maryland",
+                 date(2012, 1, 1), date(2017, 8, 31)),
+                (Title.FULL_PROF, "University of Washington",
+                 date(2017, 9, 1), None),
+            ],
+            role_start=date(2012, 2, 1),
+        )
+        record = self._role_record(person)
+        self.assertEqual(record["position_title"], "Professor")
+        self.assertEqual(record["position_school"], "University of Washington")
+        self.assertEqual(record["position_school_abbreviated"], "UW")
 
     def test_project_people_position_null_without_position(self):
         """self.past_lead has a project role but no Position at all."""