From abd8fdb35515f0224f936b2f72c4043b247add88 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Mon, 22 Jun 2026 14:18:46 -0700 Subject: [PATCH 1/2] Preserve admin uploads on validation error + drag-and-drop (#248) The Django admin change form renders with novalidate, so the browser does not enforce the required attributes Django emits. A submit that is missing a required field round-trips to the server, fails validation, and the re-rendered form silently drops any file the user had selected -- the re-upload pain reported in #248. Add a progressive-enhancement guard on the Talk/Poster/Publication admin forms (website/static/website/js/admin_artifact_form.js): - Block the file-losing submit: if any required field is empty, stop the POST, show an accessible error summary, and scroll to the first offender, so the round-trip never happens and selected files stay attached. - Pre-check files on selection: extension (from the field's accept list) and a %PDF- magic-byte check for the PDF field, mirroring the server validators so a bad file is caught before it costs a round-trip. - Drag-and-drop onto each file field via the DataTransfer API, plus a selected-filename readout. The native input stays the accessible control; the drop zone is a mouse-only enhancement hidden from assistive tech. Staying in sync with the backend: required-ness is read from the DOM ([required]), and allowed extensions flow from the PDF_EXTENSIONS / RAW_FILE_EXTENSIONS constants in upload_validators.py via ArtifactAdmin. get_form, which sets each file input's accept attribute. Python stays the single source of truth and the server validators remain authoritative, so any drift degrades gracefully. Regression test pins the server-side contract the JS depends on: the guard assets load on all three artifact add forms and the accept attributes match the validator allowlists. Co-Authored-By: Claude Opus 4.8 (1M context) --- website/admin/artifact_admin.py | 40 ++- .../website/css/admin_artifact_form.css | 93 +++++ .../static/website/js/admin_artifact_form.js | 333 ++++++++++++++++++ .../tests/test_artifact_admin_upload_guard.py | 55 +++ 4 files changed, 520 insertions(+), 1 deletion(-) create mode 100644 website/static/website/css/admin_artifact_form.css create mode 100644 website/static/website/js/admin_artifact_form.js create mode 100644 website/tests/test_artifact_admin_upload_guard.py diff --git a/website/admin/artifact_admin.py b/website/admin/artifact_admin.py index 2f119856..c9175597 100644 --- a/website/admin/artifact_admin.py +++ b/website/admin/artifact_admin.py @@ -2,13 +2,32 @@ from website.models import Artifact from django.contrib.admin import widgets from sortedm2m_filter_horizontal_widget.forms import SortedFilteredSelectMultiple +from website.utils.upload_validators import PDF_EXTENSIONS, RAW_FILE_EXTENSIONS import logging # This retrieves a Python logging instance (or creates it) _logger = logging.getLogger(__name__) + +def _accept_attr(extensions): + """Build an HTML ``accept`` value (e.g. ``.pdf,.pptx``) from an extension + allowlist. Used to seed the file inputs so both the OS file picker and the + client-side check in ``admin_artifact_form.js`` read the allowed types + straight from the markup — keeping them in sync with the server validators + (issue #248).""" + return ",".join(f".{ext}" for ext in extensions) + + class ArtifactAdmin(admin.ModelAdmin): + # Loaded on every artifact add/change form (Talk/Poster/Publication, which + # all subclass this). Guards against losing selected files when the form is + # submitted with a missing required field, plus drag-and-drop (issue #248). + # PublicationAdmin defines its own Media; Django merges this base in. + class Media: + css = {"all": ("website/css/admin_artifact_form.css",)} + js = ("website/js/admin_artifact_form.js",) + # The list display lets us control what is shown in the default talk table at Home > Website > Talk # See: https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.list_display list_display = ('title', 'date', 'get_first_author_last_name', 'forum_name', 'location') @@ -27,9 +46,28 @@ class ArtifactAdmin(admin.ModelAdmin): ('Keyword Info', {'fields': ['keywords']}), ] + def get_form(self, request, obj=None, **kwargs): + """ + Seed the ``accept`` attribute on the file inputs from the same extension + allowlists the server validators enforce (``PDF_EXTENSIONS`` / + ``RAW_FILE_EXTENSIONS`` in ``website.utils.upload_validators``). This gives + the OS file picker a native type filter and is the single source of truth + the client-side check in ``admin_artifact_form.js`` reads back from the + DOM, so the JS can't drift from the Python rules (issue #248). + + Subclasses (Talk/Publication) call ``super().get_form()`` first and then + layer their own widget tweaks, so this runs for all artifact admins. + """ + form = super().get_form(request, obj, **kwargs) + if "pdf_file" in form.base_fields: + form.base_fields["pdf_file"].widget.attrs["accept"] = _accept_attr(PDF_EXTENSIONS) + if "raw_file" in form.base_fields: + form.base_fields["raw_file"].widget.attrs["accept"] = _accept_attr(RAW_FILE_EXTENSIONS) + return form + def formfield_for_manytomany(self, db_field, request, **kwargs): """ - Overrides the formfield_for_manytomany method of the parent ModelAdmin class to customize the widgets + Overrides the formfield_for_manytomany method of the parent ModelAdmin class to customize the widgets used for ManyToMany fields in the admin interface. Parameters: diff --git a/website/static/website/css/admin_artifact_form.css b/website/static/website/css/admin_artifact_form.css new file mode 100644 index 00000000..41be8df2 --- /dev/null +++ b/website/static/website/css/admin_artifact_form.css @@ -0,0 +1,93 @@ +/* + * Styles for the artifact (Talk / Poster / Publication) admin upload guard and + * drag-and-drop. See website/static/website/js/admin_artifact_form.js (issue #248). + * + * Accessibility: error state is never signaled by color alone — it always pairs + * with an outline plus text (the inline message and the top-of-form summary), so + * it remains perceivable for low-vision and color-blind users (WCAG 2.0 AA). + */ + +/* --- Drag-and-drop zone (mouse-only enhancement; aria-hidden in the JS) --- */ +.artifact-dropzone { + margin-top: 6px; + padding: 10px 12px; + border: 2px dashed #b0b0b0; + border-radius: 6px; + color: #555; + font-size: 12px; + text-align: center; + cursor: pointer; + max-width: 420px; + background: #fafafa; + transition: border-color 0.12s ease, background-color 0.12s ease; +} + +.artifact-dropzone:hover { + border-color: #79aec8; /* Django admin accent */ +} + +.artifact-dropzone-active { + border-color: #417690; + border-style: solid; + background: #eef6fb; + color: #205067; +} + +/* --- Per-field readout / error (under each file input) --- */ +.artifact-file-note { + margin-top: 4px; + font-size: 12px; +} + +.artifact-file-note .artifact-file-name { + display: block; + color: #2a2a2a; +} + +.artifact-file-note .artifact-file-error { + display: block; + margin-top: 2px; + color: #ba2121; /* Django admin error red */ + font-weight: 600; +} + +/* Invalid control: outline + the accompanying text above (never color alone). */ +.artifact-field-invalid { + outline: 2px solid #ba2121; + outline-offset: 1px; +} + +/* --- Top-of-form error summary --- */ +.artifact-error-summary { + margin: 0 0 16px 0; + padding: 12px 16px; + border: 1px solid #ba2121; + border-left-width: 6px; + border-radius: 4px; + background: #fff4f4; +} + +.artifact-error-summary:focus { + outline: 2px solid #417690; + outline-offset: 2px; +} + +.artifact-error-summary-heading { + margin: 0 0 8px 0; + font-weight: 700; + color: #6b1414; +} + +.artifact-error-summary ul { + margin: 0; + padding-left: 20px; +} + +.artifact-error-summary li { + margin: 2px 0; +} + +.artifact-error-summary a { + color: #ba2121; + text-decoration: underline; +} diff --git a/website/static/website/js/admin_artifact_form.js b/website/static/website/js/admin_artifact_form.js new file mode 100644 index 00000000..ad0fb046 --- /dev/null +++ b/website/static/website/js/admin_artifact_form.js @@ -0,0 +1,333 @@ +/** + * Client-side guard + drag-and-drop for the artifact (Talk / Poster / Publication) + * admin add/change forms. Addresses issue #248. + * + * Why this exists + * --------------- + * The Django admin renders its change form with the `novalidate` attribute, which + * disables the browser's native HTML5 constraint validation. So even though Django + * emits `required` on every form-required field, the browser does NOT block a submit + * that's missing one. The POST goes to the server, validation fails, the form is + * re-rendered with errors — and any file the user had selected is silently dropped + * (the browser never re-sends a file input's value, and Django can't repopulate it). + * Re-uploading the lost PDF/PPTX after fixing an unrelated required field is the + * exact pain reported in #248. + * + * What this does (all progressive enhancement — the form still works without JS): + * 1. Required-field guard: on submit, blocks the POST if any required field is + * empty, shows an accessible summary, and scrolls to the first offender — so + * the round-trip that loses files never happens. + * 2. File pre-checks: on selection, validates extension (against the field's + * `accept` list) and, for PDF-only fields, the `%PDF-` signature — mirroring + * the server validators so a bad file is caught before it costs a round-trip. + * 3. Drag-and-drop onto each file field, plus a selected-filename readout. + * + * Staying in sync with the backend (see also website/utils/upload_validators.py): + * - Required-ness is read from the DOM (`[required]`), which Django derives from + * each model field's `blank=`. No field list is hardcoded here. + * - Allowed extensions are read from each file input's `accept` attribute, which + * ArtifactAdmin.get_form sets from the same PDF_EXTENSIONS / RAW_FILE_EXTENSIONS + * constants the server validators use. Python is the single source of truth. + * - The `%PDF-` signature is a frozen part of the PDF spec (mirrors + * _looks_like_pdf in upload_validators.py), so there is nothing to keep in sync. + * + * The server validators remain authoritative; everything here is a convenience + * pre-check, so any drift degrades gracefully (at worst a redundant round-trip). + */ +(function () { + "use strict"; + + // How many leading bytes to sniff for the PDF signature (matches the server's + // _read_header default in upload_validators.py). + var PDF_HEADER_BYTES = 1024; + + /** + * The main artifact form: the multipart add/change form. Returns null on any + * other admin page (this script is only loaded on the artifact admins anyway). + * @returns {HTMLFormElement|null} + */ + function getArtifactForm() { + return document.querySelector('#content-main form[enctype="multipart/form-data"]'); + } + + /** + * Human-readable label for a form control, taken from its