Skip to content

Commit f497eea

Browse files
jonfroehlichclaude
andcommitted
feat(admin): flag missing project PI/Co-PI in data health; fix get_pis/get_co_pis (#1182)
Add a read-only "project-leadership" data-health check that flags projects with no PI on record, or ongoing projects whose PI role(s) have all ended (no currently-active PI). Co-PI absence is surfaced as context but not flagged, since many projects legitimately have only a PI. Also fix Project.get_pis()/get_co_pis(), which filtered on a nonexistent "pi_member" field (the real field is lead_project_role) and so raised FieldError on any call. They now filter on lead_project_role and return a deduplicated QuerySet of Person objects as their docstrings promise. No callers existed, so changing the return type from person IDs to Person objects is safe. Adds regression tests for the new check and for get_pis/get_co_pis. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 30901d5 commit f497eea

5 files changed

Lines changed: 234 additions & 11 deletions

File tree

website/admin/data_health/checks/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
media_integrity,
1010
publication_quality,
1111
project_health,
12+
project_leadership,
1213
position_integrity,
1314
news_health,
1415
)
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""
2+
Data-health check: projects with missing PI/Co-PI leadership info (#1182).
3+
4+
PI/Co-PI tracking lives on ``ProjectRole.lead_project_role`` (values ``PI`` /
5+
``Co-PI``), not on the ``Project`` itself. This check flags two gaps:
6+
7+
- **no PI** — the project has no ``ProjectRole`` marked PI at all. Per the model
8+
help text, most projects should have Jon Froehlich as PI, so a project with
9+
zero PI roles is almost always an oversight.
10+
- **no active PI** — the project hasn't ended, but every PI role on it has an
11+
end date in the past, so nobody is currently the PI of a live project.
12+
13+
Co-PI absence is intentionally *not* flagged: many projects legitimately have
14+
only a PI. The Co-PI count is surfaced as context. Read-only.
15+
"""
16+
17+
from datetime import date
18+
19+
from website.admin.data_health.registry import HealthCheck, register_check
20+
from website.models import Project
21+
from website.models.project_role import LeadProjectRoleTypes
22+
23+
24+
@register_check
25+
class ProjectLeadershipCheck(HealthCheck):
26+
slug = 'project-leadership'
27+
title = 'Project leadership (PI/Co-PI)'
28+
description = (
29+
'Projects with no PI on record, or ongoing projects whose PI role(s) '
30+
'have all ended (no currently-active PI).'
31+
)
32+
group = 'Projects'
33+
columns = [
34+
'id', 'name', 'short_name', 'is_visible', 'has_ended',
35+
'pi_count', 'active_pi_count', 'copi_count', 'pis', 'issues',
36+
]
37+
38+
def get_rows(self):
39+
today = date.today()
40+
rows = []
41+
projects = Project.objects.all().prefetch_related('projectrole_set__person')
42+
for project in projects:
43+
roles = list(project.projectrole_set.all())
44+
pi_roles = [
45+
r for r in roles
46+
if r.lead_project_role == LeadProjectRoleTypes.PI
47+
]
48+
copi_roles = [
49+
r for r in roles
50+
if r.lead_project_role == LeadProjectRoleTypes.CO_PI
51+
]
52+
53+
def _is_active(role):
54+
return (
55+
role.start_date and role.start_date <= today
56+
and (role.end_date is None or role.end_date >= today)
57+
)
58+
59+
active_pi_count = sum(1 for r in pi_roles if _is_active(r))
60+
has_ended = project.has_ended()
61+
62+
issues = []
63+
if not pi_roles:
64+
issues.append('no PI')
65+
elif not has_ended and active_pi_count == 0:
66+
issues.append('no active PI')
67+
68+
if not issues:
69+
continue
70+
71+
pi_names = ', '.join(
72+
r.person.get_full_name() for r in pi_roles
73+
)
74+
rows.append({
75+
'id': project.pk,
76+
'name': project.name,
77+
'short_name': project.short_name,
78+
'is_visible': bool(project.is_visible),
79+
'has_ended': has_ended,
80+
'pi_count': len(pi_roles),
81+
'active_pi_count': active_pi_count,
82+
'copi_count': len(copi_roles),
83+
'pis': pi_names,
84+
'issues': ', '.join(issues),
85+
})
86+
87+
rows.sort(key=lambda r: r['name'].lower())
88+
return rows

website/models/project.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,18 +335,26 @@ def get_thumbnail_alt_text(self):
335335
return self.thumbnail_alt_text
336336

337337
def get_pis(self):
338-
"""Returns the PIs for the project as a QuerySet of Person objects"""
339-
pis_queryset = (self.projectrole_set
340-
.filter(pi_member=LeadProjectRoleTypes.PI)
341-
.values_list('person', flat=True))
342-
return pis_queryset
338+
"""Returns the PIs for the project as a QuerySet of Person objects.
339+
340+
PI status is tracked on ``ProjectRole.lead_project_role`` (not on the
341+
Project itself), so we filter the project's roles by that field.
342+
"""
343+
return Person.objects.filter(
344+
projectrole__project=self,
345+
projectrole__lead_project_role=LeadProjectRoleTypes.PI,
346+
).distinct()
343347

344348
def get_co_pis(self):
345-
"""Returns the Co-PIs for this project as a QuerySet of Person objects"""
346-
copis_queryset = (self.projectrole_set
347-
.filter(pi_member=LeadProjectRoleTypes.CO_PI)
348-
.values_list('person', flat=True))
349-
return copis_queryset
349+
"""Returns the Co-PIs for this project as a QuerySet of Person objects.
350+
351+
Co-PI status is tracked on ``ProjectRole.lead_project_role`` (not on the
352+
Project itself), so we filter the project's roles by that field.
353+
"""
354+
return Person.objects.filter(
355+
projectrole__project=self,
356+
projectrole__lead_project_role=LeadProjectRoleTypes.CO_PI,
357+
).distinct()
350358

351359
def has_award(self):
352360
"""

website/tests/test_data_health.py

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,74 @@ def test_flags_incomplete_project(self):
173173
self.assertIn("no publication", rows["Lonely Project"]["issues"])
174174

175175

176+
class ProjectLeadershipCheckTests(DatabaseTestCase):
177+
def _add_role(self, project, lead_role, start_date, end_date=None):
178+
from website.models import ProjectRole
179+
180+
person = self.make_person(first_name="Lead", last_name="Person")
181+
return ProjectRole.objects.create(
182+
person=person,
183+
project=project,
184+
lead_project_role=lead_role,
185+
start_date=start_date,
186+
end_date=end_date,
187+
)
188+
189+
def test_flags_project_with_no_pi(self):
190+
proj = self.make_project(name="PI-less Project")
191+
rows = {r["name"]: r for r in get_check("project-leadership").get_rows()}
192+
self.assertIn("PI-less Project", rows)
193+
self.assertEqual(rows["PI-less Project"]["issues"], "no PI")
194+
self.assertEqual(rows["PI-less Project"]["pi_count"], 0)
195+
196+
def test_ongoing_project_with_only_an_ended_pi_flagged_no_active_pi(self):
197+
from datetime import date, timedelta
198+
from website.models.project_role import LeadProjectRoleTypes
199+
200+
proj = self.make_project(name="Stale Lead Project") # no end_date => ongoing
201+
self._add_role(
202+
proj,
203+
LeadProjectRoleTypes.PI,
204+
start_date=date.today() - timedelta(days=400),
205+
end_date=date.today() - timedelta(days=30),
206+
)
207+
rows = {r["name"]: r for r in get_check("project-leadership").get_rows()}
208+
self.assertIn("Stale Lead Project", rows)
209+
self.assertEqual(rows["Stale Lead Project"]["issues"], "no active PI")
210+
self.assertEqual(rows["Stale Lead Project"]["pi_count"], 1)
211+
self.assertEqual(rows["Stale Lead Project"]["active_pi_count"], 0)
212+
213+
def test_project_with_active_pi_not_flagged(self):
214+
from datetime import date, timedelta
215+
from website.models.project_role import LeadProjectRoleTypes
216+
217+
proj = self.make_project(name="Well-Led Project")
218+
self._add_role(
219+
proj,
220+
LeadProjectRoleTypes.PI,
221+
start_date=date.today() - timedelta(days=30),
222+
)
223+
names = {r["name"] for r in get_check("project-leadership").get_rows()}
224+
self.assertNotIn("Well-Led Project", names)
225+
226+
def test_ended_project_with_ended_pi_not_flagged(self):
227+
from datetime import date, timedelta
228+
from website.models.project_role import LeadProjectRoleTypes
229+
230+
proj = self.make_project(
231+
name="Wrapped-Up Project",
232+
end_date=date.today() - timedelta(days=10),
233+
)
234+
self._add_role(
235+
proj,
236+
LeadProjectRoleTypes.PI,
237+
start_date=date.today() - timedelta(days=400),
238+
end_date=date.today() - timedelta(days=20),
239+
)
240+
names = {r["name"] for r in get_check("project-leadership").get_rows()}
241+
self.assertNotIn("Wrapped-Up Project", names)
242+
243+
176244
class PositionIntegrityCheckTests(DatabaseTestCase):
177245
def test_no_position_and_self_advisor(self):
178246
from datetime import date as _date
@@ -243,7 +311,10 @@ def test_get_rows_does_not_mutate_db(self):
243311
self.make_person(first_name="Jane", last_name="Doe")
244312
self.make_person(first_name="Jane", last_name="Doe")
245313
before = (Person.objects.count(), Publication.objects.count())
246-
for slug in ("duplicate-people", "url-name-collisions", "position-integrity"):
314+
for slug in (
315+
"duplicate-people", "url-name-collisions", "position-integrity",
316+
"project-leadership",
317+
):
247318
get_check(slug).get_rows()
248319
after = (Person.objects.count(), Publication.objects.count())
249320
self.assertEqual(before, after)

website/tests/test_project.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,58 @@ def test_long_completed_role_outranks_short_ongoing_role(self):
101101
result = list(project.get_people())
102102
self.assertEqual(result[0], long_done)
103103
self.assertEqual(result[1], short_ongoing)
104+
105+
106+
# --- get_pis / get_co_pis regression --------------------------------------
107+
108+
109+
class ProjectGetPisCoPisTests(DatabaseTestCase):
110+
"""
111+
Regression for Project.get_pis / get_co_pis (models/project.py).
112+
113+
Both methods filtered on a nonexistent ``pi_member`` field (the real field
114+
is ``lead_project_role``), so any call raised FieldError. The fix filters
115+
on ``lead_project_role`` and returns a QuerySet of Person objects as the
116+
docstring promises (#1182).
117+
"""
118+
119+
def _add_role(self, person, project, lead_role):
120+
from website.models import ProjectRole
121+
return ProjectRole.objects.create(
122+
person=person, project=project,
123+
lead_project_role=lead_role, start_date=date.today(),
124+
)
125+
126+
def test_get_pis_and_co_pis_return_correct_people(self):
127+
from website.models import Project
128+
from website.models.project_role import LeadProjectRoleTypes
129+
130+
project = Project.objects.create(name="Lab Project", short_name="lab")
131+
pi = self.make_person(first_name="Jon", last_name="Froehlich")
132+
co_pi = self.make_person(first_name="Co", last_name="Investigator")
133+
student = self.make_person(first_name="Grad", last_name="Student")
134+
self._add_role(pi, project, LeadProjectRoleTypes.PI)
135+
self._add_role(co_pi, project, LeadProjectRoleTypes.CO_PI)
136+
self._add_role(student, project, LeadProjectRoleTypes.STUDENT_LEAD)
137+
138+
self.assertEqual(list(project.get_pis()), [pi])
139+
self.assertEqual(list(project.get_co_pis()), [co_pi])
140+
141+
def test_no_pi_returns_empty_queryset(self):
142+
from website.models import Project
143+
144+
project = Project.objects.create(name="Empty Project", short_name="empty")
145+
self.assertEqual(list(project.get_pis()), [])
146+
self.assertEqual(list(project.get_co_pis()), [])
147+
148+
def test_duplicate_pi_roles_deduplicated(self):
149+
"""A person with two PI roles on one project appears once."""
150+
from website.models import Project
151+
from website.models.project_role import LeadProjectRoleTypes
152+
153+
project = Project.objects.create(name="Dup Project", short_name="dup")
154+
pi = self.make_person(first_name="Repeat", last_name="Lead")
155+
self._add_role(pi, project, LeadProjectRoleTypes.PI)
156+
self._add_role(pi, project, LeadProjectRoleTypes.PI)
157+
158+
self.assertEqual(list(project.get_pis()), [pi])

0 commit comments

Comments
 (0)