Build the PACS viewer link from a deployment-level URL template when a report has no pacs_link #330

Motivation

RADIS advertises viewer deep linking (README.md:39, radis/core/templates/core/home.html:39, docs/user-docs/user-guide.md:69), but the only way to get a link today is for the importer to send a complete per-report URL in pacs_link. Every deployment must therefore reimplement "identifier -> viewer URL" in its ETL, the URL is frozen into each row (changing the viewer means re-upserting every report), and a 64-char StudyInstanceUID plus host and path can exceed the 200-char field. RADIS already stores the identifiers a viewer needs (accession_number, study_instance_uid, patient_id, pacs_aet), so it could render the link itself from one deployment-level template, retroactively for all existing reports.

Concrete case: in Bonn pacs_link is empty for every report and the viewer URL scheme is still being determined. The ETL route (send a full URL per report) works on today's main and needs no RADIS change; this issue proposes the generalisation so that the next deployment does not have to repeat it.

Current behaviour

• radis/reports/models.py:51 -- pacs_link = models.CharField(max_length=200, blank=True); no URL validation.
• Identifiers on Report: pacs_aet (16), pacs_name (64) at models.py:49-50; patient_id (64) at :52; study_instance_uid (64, blank allowed) and accession_number (32, blank allowed) at :77-78.
• pacs_link is writable through the report API (radis/reports/api/serializers.py:64-66, fields = "__all__"); bulk upsert copies it on create and update (radis/reports/api/viewsets.py:124-137,144-147). ReportViewSet requires IsAdminUser (viewsets.py:270).
• radis-client/radis_client/client.py:13-15 -- ReportData requires pacs_aet, pacs_name, pacs_link as str with no default, which is why importers send "".
• radis/reports/templates/reports/_report_buttons_panel.html:20-26 renders href="{{ report.pacs_link }}" unconditionally (see the companion bug issue). All six include sites pass a full Report (search passes document.full_report, radis/search/site.py:23-25), so a model property is picked up without touching the search provider or ReportDocument (site.py:14, radis/pgsearch/utils/document_utils.py:23).
• No viewer or URL-template setting exists. Deployment configuration is env-based via environs (radis/settings/base.py:17,43-44; documented in docs/user-docs/admin-guide.md:20-26). ReportsAppSettings exists (models.py:19-21, admin-registered at radis/reports/admin.py:16, auto-created in radis/reports/apps.py:13-17) but no AppSettings subclass carries any field beyond the inherited locked (adit_radis_shared/common/models.py:23-24), and TODO.md:6 plans to rename them.
• TODO.md:32 "Fetch favicon for pacs_link" treats the link as a first-class external-viewer link.

Proposal

1. New setting PACS_VIEWER_URL_TEMPLATE (env, default "") with str.format placeholders {accession_number}, {study_instance_uid}, {patient_id}, {pacs_aet}.
2. Report.viewer_link property (or a template tag in radis/reports/templatetags/reports_extras.py): return pacs_link if non-empty; else, if the template is set and every placeholder it references is non-empty on the report, the rendered template with percent-encoded values; else "". An explicit pacs_link always wins.
3. The buttons panel uses viewer_link and applies the empty-state handling from the companion bug issue.
4. Scheme safety: pacs_link is rendered straight into href, and autoescaping does not stop javascript: or data: URLs. Only admin tokens can set it today, so the exposure is small, but allow-listing schemes (http, https, plus a configurable list) for both the stored value and the rendered template is cheap. URLValidator is not a fit because it would reject custom viewer schemes such as protocol handlers.

Options, cheapest first:

• (a) No RADIS change; each ETL sends a full URL. Works today; duplicates logic per site, freezes URLs into rows, 200-char limit.
• (b1) Env template + viewer_link fallback as above. No migration, retroactive, a few dozen lines plus tests. Cannot differ per PACS.
• (b2) Same, stored as a field on ReportsAppSettings. One field, one migration, changeable without redeploy, but no precedent for config fields on AppSettings.
• (b3) Per-AET mapping (JSON env map or a small Pacs model keyed by pacs_aet). Most general; only needed for multi-PACS deployments.

Recommendation: (b1) now, (b3) as a follow-up if a deployment ingests from several AETs. (b2) is close to equal cost; maintainer preference.

Open questions

1. Which identifier should templates use? StudyInstanceUID is safer than AccessionNumber where several exams can share one accession number (the case in Bonn). {patient_id} puts PHI into browser history and viewer/proxy logs; rel="noopener noreferrer" already prevents Referer leakage from RADIS, but the placeholder choice is a data-protection decision.
2. Percent-encoding: mandatory, or configurable for custom-scheme parsers?
3. Env vs ReportsAppSettings (b1 vs b2).
4. Raise pacs_link max_length (ADIT uses 2000 for DicomServer.dicomweb_root_url, openradx/adit adit/core/models.py:148)? Only matters for route (a); widening a varchar is catalog-only in PostgreSQL but takes a brief exclusive lock on reports_report.
5. How does the favicon idea (TODO.md:32) interact with a template-derived link (fetch once from the template host vs per report)?

Implementation notes

• Files: radis/settings/base.py, example.env, docs/user-docs/admin-guide.md:20-26, radis/reports/models.py (or radis/reports/templatetags/reports_extras.py), _report_buttons_panel.html, docs/user-docs/user-guide.md:69, README.md:39.
• Tests: no anchor when pacs_link is empty and no template is set; template URL when set (pytest-django settings fixture, already used in radis/core/tests/test_llm_client.py:33); explicit pacs_link wins; missing identifier yields no link; values are encoded; disallowed schemes are rejected. ReportFactory defaults pacs_link to Faker("url") (radis/reports/factories.py:56), so override it.
• In-flight PRs: Allow admins to withdraw and restore reports #308 modifies radis/reports/models.py (the pacs_link line is at :68 on that branch); add a model property after it merges, or use a template tag to avoid the conflict. Convert report API to ADRF views (prep for async embedding trigger) #230 rewrites the bulk upsert in radis/reports/api/viewsets.py (the field list stays in that file) and adds radis/reports/api/operations.py; any change to which fields the API accepts should follow it. Make the FTS candidate query single-table with a search projection #292 touches only radis/pgsearch/, so no overlap.

Related

• Companion bug issue: PACS button in the report panel renders an anchor with an empty href when pacs_link is blank #329.
• TODO.md:32 (favicon), TODO.md:47 (export collections to ADIT -- same identifiers, different target).
• The ADIT -> RADIS deep link bullet from the brainstorming (open a RADIS report from an ADIT study by StudyInstanceUID) is the mirror image; both should agree on the identifier set; see Add a lookup URL that opens a report by Study Instance UID or Accession Number #310.
• Add a meaningful API for RADIS client #130 (radis-client API) -- pacs_link could become optional in ReportData.
• Allow admins to withdraw and restore reports #308, Convert report API to ADRF views (prep for async embedding trigger) #230 (in-flight, see above).