Standardize per-row action links across Data Health checks (#1405)

website/admin/data_health/checks/duplicate_people.py

@@ -31,6 +31,7 @@ class DuplicatePeopleCheck(HealthCheck):
         'delete shell.'
     )
     group = 'People'
+    link_model = 'person'
     columns = [
         'cluster_key', 'id', 'first_name', 'middle_name', 'last_name',
         'url_name', 'email', 'personal_website', 'github', 'linkedin',

website/admin/data_health/checks/news_health.py

@@ -18,6 +18,7 @@ class NewsHealthCheck(HealthCheck):
     title = 'News health'
     description = "News items missing a slug or an author."
     group = 'People'
+    link_model = 'news'
     columns = ['id', 'title', 'date', 'missing_slug', 'has_author']
 
     def get_rows(self):

website/admin/data_health/checks/position_integrity.py

@@ -22,6 +22,8 @@ class PositionIntegrityCheck(HealthCheck):
         'themselves.'
     )
     group = 'People'
+    link_model = 'person'
+    link_id_key = 'person_id'
     columns = ['person_id', 'name', 'issue', 'detail']
 
     def get_rows(self):

website/admin/data_health/checks/project_health.py

@@ -23,6 +23,7 @@ class ProjectHealthCheck(HealthCheck):
         'site), with no currently-active members, or with no umbrella.'
     )
     group = 'Projects'
+    link_model = 'project'
     columns = [
         'id', 'name', 'short_name', 'is_visible', 'has_thumbnail',
         'has_publication', 'active_member_count', 'has_umbrella', 'issues',

website/admin/data_health/checks/project_leadership.py

@@ -30,6 +30,7 @@ class ProjectLeadershipCheck(HealthCheck):
         'have all ended (no currently-active PI).'
     )
     group = 'Projects'
+    link_model = 'project'
     columns = [
         'id', 'name', 'short_name', 'is_visible', 'has_ended',
         'pi_count', 'active_pi_count', 'copi_count', 'pis', 'issues',

website/admin/data_health/checks/publication_quality.py

@@ -27,6 +27,7 @@ class PublicationQualityCheck(HealthCheck):
         'and publications that share a normalized title (possible duplicates).'
     )
     group = 'Artifacts'
+    link_model = 'publication'
     columns = ['id', 'title', 'date', 'missing_fields', 'dup_title']
 
     def get_rows(self):

website/admin/data_health/checks/url_name_collisions.py

@@ -8,6 +8,9 @@
 """
 
 from collections import defaultdict
+from urllib.parse import urlencode
+
+from django.urls import reverse
 
 from website.admin.data_health.registry import HealthCheck, register_check
 from website.models import Person
@@ -45,3 +48,11 @@ def get_rows(self):
         # Worst offenders first, then alphabetical.
         rows.sort(key=lambda r: (-r['count'], r['url_name']))
         return rows
+
+    def row_link(self, row):
+        """Each row is a cluster of people (not a single object), so link to the
+        Person changelist searched for the offending url_name — surfacing the
+        rows to renumber/merge. (``url_name`` is in PersonAdmin.search_fields.)
+        """
+        url = reverse('admin:website_person_changelist')
+        return ('Open in admin →', f"{url}?{urlencode({'q': row['url_name']})}")

website/admin/data_health/registry.py

@@ -13,6 +13,7 @@
 import io
 
 from django.http import HttpResponse
+from django.urls import reverse
 from django.utils import timezone
 
 
@@ -34,18 +35,36 @@ class HealthCheck:
     group = 'Other'
     #: Ordered column keys; used as table headers and the CSV header row.
     columns = []
+    #: Lowercase ``website`` model name (e.g. ``"publication"``) whose admin
+    #: change page each row links to. Leave ``None`` for checks that build
+    #: their own link (or none) by overriding :meth:`row_link`.
+    link_model = None
+    #: Row key holding the primary key used to build the default link.
+    link_id_key = 'id'
 
     def get_rows(self):
         """Return a list of row dicts (read-only). Override in subclasses."""
         raise NotImplementedError
 
     def row_link(self, row):
-        """Optional: a ``(label, url)`` pair to act on ``row`` from the detail
-        page, or ``None``. Lets a check wire its read-only findings to a fixer
-        elsewhere in the admin (e.g. the keyword-merge changelist). Default: no
-        link. Only affects the on-screen table — the CSV export is unchanged.
+        """Return an ``(label, url)`` pair to act on ``row`` from the detail
+        page, or ``None``.
+
+        Default behavior keeps every flagged row one click from its fix: when
+        the subclass sets :attr:`link_model` and the row carries a primary key
+        under :attr:`link_id_key`, deep-link to that object's admin change page.
+        Checks with a non-trivial target — clusters, per-row model types, or a
+        filtered changelist — override this instead (e.g. the keyword-merge
+        changelist). Only affects the on-screen table; the CSV export is
+        unchanged.
         """
-        return None
+        if not self.link_model:
+            return None
+        obj_id = row.get(self.link_id_key)
+        if not obj_id:
+            return None
+        url = reverse(f'admin:website_{self.link_model}_change', args=[obj_id])
+        return ('Open →', url)
 
     def count(self):
         """Number of flagged rows. Override for a cheaper count if available."""

website/admin/person_admin.py

@@ -175,8 +175,10 @@ def save_model(self, request, obj, form, change):
     # see: https://docs.djangoproject.com/en/1.11/ref/contrib/admin/#inlinemodeladmin-objects
     inlines = [PositionInline, ProjectRoleInline]
 
-    # We must define search_fields in order to use the autocomplete_fields option
-    search_fields = ['first_name', 'last_name',]
+    # We must define search_fields in order to use the autocomplete_fields option.
+    # url_name is included so the Data Health "url_name collisions" check can deep-link
+    # here with ?q=<url_name> to surface the colliding rows.
+    search_fields = ['first_name', 'last_name', 'url_name',]
 
     def get_search_results(self, request, queryset, search_term):
         """Role-filter the admin autocomplete results for advisor/mentor fields (#1126).

website/tests/test_data_health.py

@@ -14,7 +14,7 @@
 from django.test import SimpleTestCase, override_settings
 from django.urls import reverse
 
-from website.admin.data_health.registry import get_check
+from website.admin.data_health.registry import REGISTRY, HealthCheck, get_check
 from website.tests.base import DatabaseTestCase
 from website.utils.name_utils import normalize_person_name, is_default_person_image
 
@@ -331,6 +331,24 @@ def test_missing_file_row_links_to_admin_edit(self):
         )
 
 
+class ActionLinkStandardizationTests(SimpleTestCase):
+    """Every registered check must give its rows an action link (issue #1405),
+    keeping admins one click from the fix. A check qualifies by either declaring
+    ``link_model`` (default deep-link to the object's admin change page) or
+    overriding ``row_link`` (custom target). This pins the standardization so a
+    future check can't silently ship without one."""
+
+    def test_every_check_provides_an_action_link(self):
+        offenders = [
+            c.slug for c in REGISTRY
+            if c.link_model is None
+            and type(c).row_link is HealthCheck.row_link
+        ]
+        self.assertEqual(
+            offenders, [], f"checks lacking a per-row action link: {offenders}"
+        )
+
+
 class DataHealthReadOnlyTests(DatabaseTestCase):
     def test_get_rows_does_not_mutate_db(self):
         from website.models import Person, Publication