Skip to content

Commit 025b69b

Browse files
jonfroehlichclaude
andcommitted
Add an artifact-type suffix to standardized filenames (#1404)
Standardized artifact filenames gain a trailing type segment for the two types whose file is ambiguous once downloaded: a talk's exported slides and a poster generate the same Author_TitleInTitleCase_VenueYear name as the paper itself. Talks now end in "_Talk", posters in "_Poster"; publications and grants stay unsuffixed (a bare paper PDF is the default expectation, and that name is also the .bib download name). Trailing rather than the existing mid-name suffix slot, so everything before it stays byte-identical to the old scheme and the "-<timestamp>" uniquifier still appends cleanly. The label comes from a class attribute (FILENAME_TYPE_SUFFIX), not from data -- Poster has no type field and Talk.talk_type is nullable and editor-editable, so deriving it would rename files on a metadata-only edit. Applied retroactively via restandardize_artifact_filenames, which is already an idempotent every-container-start step. It ships here with --dry-run so the scope is reviewable in the logs before any file moves. The load-bearing subtlety is ordering: backfill_original_filenames runs BEFORE that step, and its "already standardized?" guard is what stops it recording a renamed file's current name as its "Originally uploaded as" provenance. The moment the scheme changed, every already-renamed talk and poster would have failed that guard and had its old standardized name written in as a fake original -- so the guard now accepts both schemes. Also extracts the uniquifier-tolerant name comparison that three commands had each copy-pasted into one tested helper (matches_standardized_basename), and teaches repair_diverged_artifact_filenames to look for orphans under the pre-#1404 base too (that bug predates the scheme change). Known and accepted: /media/talks/ and /media/posters/ 404s are answered by Apache and never reach Django, so unlike publications (which serve_pdf can redirect via the recorded original filename) external links to renamed talk/poster PDFs cannot be rescued from this repo. Tracked separately. Tests: 661 OK. New coverage for the trailing segment, per-class labels, the shared matcher, the retroactive rename of a pre-#1404 talk (pdf + raw + thumbnail) and its idempotency, publications not churning, and the backfill guard for both a legacy name and its uniquified variant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 54c63b6 commit 025b69b

11 files changed

Lines changed: 390 additions & 54 deletions

docker-entrypoint.sh

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,15 @@ echo "******************************************"
164164
python manage.py setup_admin_groups
165165

166166
echo "****************** STEP 4.10b/5: docker-entrypoint.sh ************************"
167-
echo "4.10b Running 'python manage.py restandardize_artifact_filenames' to rename legacy talk/poster/pub files to the standardized scheme (#1401)"
168-
echo "******************************************"
169-
python manage.py restandardize_artifact_filenames
167+
echo "4.10b Running 'python manage.py restandardize_artifact_filenames' to rename legacy talk/poster/pub files to the standardized scheme (#1401/#1404)"
168+
echo "******************************************"
169+
# TEMPORARY (#1404): the scheme just gained a trailing artifact-type segment
170+
# ("..._CHI2024_Talk"), so this step would re-rename EVERY already-standardized
171+
# talk and poster in one unattended pass. --dry-run logs what WOULD be renamed,
172+
# touching nothing on disk or in the DB, so the scope can be reviewed on the
173+
# test server (and then prod) first. REMOVE --dry-run and redeploy to perform
174+
# the rename; after that this step is idempotent again and stays in place.
175+
python manage.py restandardize_artifact_filenames --dry-run
170176

171177
echo "****************** STEP 4.10c/5: docker-entrypoint.sh ************************"
172178
echo "4.10c Running 'python manage.py repair_diverged_artifact_filenames' to recover artifacts whose files were renamed on disk but not in the DB (#1390 dotted-name bug)"

makeabilitylab/settings.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@
8686
SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https')
8787

8888
# Makeability Lab Global Variables, including Makeability Lab version
89-
ML_WEBSITE_VERSION = "2.28.1" # Keep this updated with each release and also change the short description below
90-
ML_WEBSITE_VERSION_DESCRIPTION = "Adds two quick links to the admin header so a pair of easy-to-forget internal pages stop being invisible: Project People (/view-project-people/, the project-team roster, which nothing in the main site nav links to) and Activity Log (the site-wide admin action log added in #1413). The Activity Log link is wrapped in {% if user.is_superuser %} so it matches the gate on LogEntryAdmin itself -- editors and contributors never see a link to a page they cannot open, and the log's existence is not advertised to them. The roster link is labeled 'Project People' to match that page's own heading rather than inventing a second name for the same page, and both new-tab links now carry a visually-hidden '(opens in a new tab)' cue built on Django admin's own screen-reader helper class, so no new CSS ships. Also fixes a NoReverseMatch this change would otherwise have shipped: website/urls.py sets app_name = 'website', so the roster reverses as 'website:view_project_people', and the bare name raised inside the userlinks block of base_site.html -- the parent template of EVERY admin page -- which would have 500'd the whole admin for any logged-in staff user rather than merely breaking one link (and, on -test where DEBUG is True, shown a public traceback). It slipped past manual testing because hitting /view-project-people/ exercises the URL path rather than the reverse name, and an unauthenticated admin request 302s to login without ever rendering the template. A new regression test renders the admin index as both a superuser and a staff non-superuser, pinning both reverses, the superuser gate, and the new-tab cue, so a future URL-name rename fails a test instead of taking down the admin."
89+
ML_WEBSITE_VERSION = "2.29.0" # Keep this updated with each release and also change the short description below
90+
ML_WEBSITE_VERSION_DESCRIPTION = "Standardized filenames for talks and posters now end in '_Talk' and '_Poster' (#1404), so a downloaded slide deck or poster is no longer indistinguishable from the paper PDF of the same work. Publication filenames are unchanged. Existing talk/poster files are renamed once, on deploy."
9191
DATE_MAKEABILITYLAB_FORMED = datetime.date(2012, 1, 1) # Date Makeability Lab was formed
9292
MAX_BANNERS = 7 # Maximum number of banners on a page
9393

website/management/commands/backfill_original_filenames.py

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from django.core.management.base import BaseCommand
55

66
from website.models import Artifact, Talk, Poster, Publication
7+
from website.utils import fileutils as ml_fileutils
78

89
# This retrieves a Python logging instance (or creates it)
910
_logger = logging.getLogger(__name__)
@@ -122,19 +123,26 @@ def _backfill_row(self, model, artifact, file_attr, original_attr, dry_run):
122123

123124
current_basename = os.path.basename(file_field.name)
124125
current_no_ext = os.path.splitext(current_basename)[0]
125-
standardized_no_ext = Artifact.generate_filename(artifact)
126-
127-
# Treat the file as already-standardized when its name equals the
128-
# standardized scheme OR is a uniquified variant of it. When a
129-
# standardized name collides on disk, ensure_filename_is_unique()
130-
# (fileutils.py) appends "-<timestamp>" — e.g.
131-
# "Lee_Talk_CHI2021-1782399772.42.pdf" — so the on-disk name still
132-
# STARTS WITH the standardized base. Matching only on exact equality
133-
# would misread those as never-renamed and record the standardized+
134-
# suffix name as the "original" — a false positive.
135-
already_standardized = (
136-
current_no_ext == standardized_no_ext
137-
or current_no_ext.startswith(standardized_no_ext + "-")
126+
127+
# Treat the file as already-standardized when its name matches the
128+
# standardized scheme. matches_standardized_basename also accepts the
129+
# "-<timestamp>" that ensure_filename_is_unique appends on a disk
130+
# collision (e.g. "Lee_Talk_CHI2021-1782399772.42.pdf"), so a renamed-
131+
# then-uniquified file isn't misread as an original upload.
132+
#
133+
# BOTH schemes count (#1404). This command runs at container start
134+
# BEFORE restandardize_artifact_filenames, so on the deploy that
135+
# introduces the artifact-type segment ("..._Talk") every already-renamed
136+
# talk and poster still carries its pre-#1404 name. Checking only the
137+
# current scheme would read the whole corpus as never-renamed and record
138+
# those old standardized names as originals — false provenance, shown to
139+
# editors as "Originally uploaded as".
140+
already_standardized = any(
141+
ml_fileutils.matches_standardized_basename(
142+
current_no_ext,
143+
Artifact.generate_filename(
144+
artifact, include_type_suffix=include_type_suffix))
145+
for include_type_suffix in (True, False)
138146
)
139147
if already_standardized:
140148
# Already renamed — the original upload name is gone.

website/management/commands/repair_diverged_artifact_filenames.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from django.core.management.base import BaseCommand
55

66
from website.models import Artifact, Talk, Poster, Publication
7+
from website.utils import fileutils as ml_fileutils
78

89
# This retrieves a Python logging instance (or creates it)
910
_logger = logging.getLogger(__name__)
@@ -150,9 +151,15 @@ def _repair_field(self, artifact, field_name, dry_run):
150151
# base, possibly with a "-<timestamp>" uniqueness suffix from colliding
151152
# with the sibling files. Match those, then confirm by content type so we
152153
# never mis-pair a pdf with a pptx/thumbnail (they share the base name).
154+
# The pre-#1404 base (no trailing "_Talk"/"_Poster") is searched too: the
155+
# bug predates that scheme change, so an orphan it left behind on disk
156+
# carries the old base even though the repair target uses the new one.
157+
legacy_base = get_valid_filename(
158+
Artifact.generate_filename(artifact, include_type_suffix=False))
153159
candidates = []
154160
for entry in os.listdir(directory):
155-
if entry == valid_base or entry.startswith(valid_base + "-"):
161+
if any(ml_fileutils.matches_standardized_basename(entry, base)
162+
for base in {valid_base, legacy_base}):
156163
full = os.path.join(directory, entry)
157164
if os.path.isfile(full):
158165
candidates.append(entry)

website/management/commands/restandardize_artifact_filenames.py

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,28 @@
44
from django.core.management.base import BaseCommand
55

66
from website.models import Artifact, Talk, Poster, Publication
7+
from website.utils import fileutils as ml_fileutils
78

89
# This retrieves a Python logging instance (or creates it)
910
_logger = logging.getLogger(__name__)
1011

1112

1213
class Command(BaseCommand):
1314
help = (
14-
"Re-standardizes legacy talk/poster/publication filenames that were "
15-
"never renamed to the Author_TitleInTitleCase_VenueYear scheme (issue "
16-
"#1401). Production has many such rows (bulk-imported, so they never "
17-
"went through an authored Artifact.save()). This reuses the existing, "
18-
"now-correct rename path: when Artifact.do_filenames_need_updating() is "
19-
"True it calls artifact.save(), which renames the pdf_file, raw_file, "
20-
"and thumbnail on disk AND in the DB together. The original upload name "
21-
"is preserved (it was captured into original_*_filename by "
22-
"backfill_original_filenames / #1391 before this runs). Idempotent: "
23-
"once a row is standardized the check returns False, so re-runs do "
24-
"nothing. Safe to run on every container start."
15+
"Re-standardizes talk/poster/publication filenames that don't match the "
16+
"current scheme. Two populations: files that were never renamed at all "
17+
"(issue #1401 — bulk-imported rows that never went through an authored "
18+
"Artifact.save()), and files standardized under a superseded scheme "
19+
"(issue #1404 added the trailing artifact-type segment, e.g. "
20+
"'..._CHI2024_Talk'), which this migrates in one pass. It reuses the "
21+
"existing, now-correct rename path: when Artifact.do_filenames_need_"
22+
"updating() is True it calls artifact.save(), which renames the "
23+
"pdf_file, raw_file, and thumbnail on disk AND in the DB together. The "
24+
"original upload name is preserved (it was captured into "
25+
"original_*_filename by backfill_original_filenames / #1391 before this "
26+
"runs). Idempotent: once a row matches the current scheme the check "
27+
"returns False, so re-runs do nothing. Safe to run on every container "
28+
"start."
2529
)
2630

2731
# The concrete artifact models this covers. Posters are already
@@ -156,9 +160,12 @@ def _needs_restandardizing(artifact):
156160
standardized name collided on disk and got a ``-<timestamp>`` suffix
157161
(``ensure_filename_is_unique``) reads as "needs updating" forever and
158162
would be re-renamed on every run — churning duplicate-name artifacts'
159-
filenames on every deploy. Here a name that equals the standardized
160-
base OR is a ``-<suffix>`` variant of it counts as already standardized,
161-
which keeps the command idempotent.
163+
filenames on every deploy. ``matches_standardized_basename`` accepts
164+
those variants, which keeps the command idempotent.
165+
166+
Only the CURRENT scheme counts as standardized: a file named under the
167+
pre-#1404 scheme (no trailing "_Talk"/"_Poster") is deliberately flagged,
168+
which is what migrates the existing corpus in a single pass.
162169
"""
163170
standardized = Artifact.generate_filename(artifact)
164171
for file_attr in ("pdf_file", "raw_file"):
@@ -167,10 +174,7 @@ def _needs_restandardizing(artifact):
167174
continue
168175
current_no_ext = os.path.splitext(
169176
os.path.basename(file_field.name))[0]
170-
is_standardized = (
171-
current_no_ext == standardized
172-
or current_no_ext.startswith(standardized + "-")
173-
)
174-
if not is_standardized:
177+
if not ml_fileutils.matches_standardized_basename(
178+
current_no_ext, standardized):
175179
return True
176180
return False

website/models/artifact.py

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ class Artifact(models.Model):
2323
2. also hook up an authors_changed signal in signals.py. For example, add the line "@receiver(m2m_changed, sender=Grant.authors.through)"
2424
to signals.py's def authors_changed(sender, instance, action, reverse, **kwargs):
2525
"""
26+
# The artifact-type segment appended to standardized filenames (#1404), so a
27+
# downloaded file says what kind of artifact it is. Set on the subclasses
28+
# whose files are otherwise ambiguous — a talk's exported slides and a poster
29+
# generate the same Author_Title_VenueYear name as the paper itself. Left
30+
# None here (and on Publication/Grant): a bare paper PDF is the default
31+
# expectation, and that same name is also the .bib download name.
32+
FILENAME_TYPE_SUFFIX = None
33+
2634
title = models.CharField(max_length=255, blank=True, null=True)
2735
authors = SortedManyToManyField('Person', blank=True)
2836
date = models.DateField(null=True)
@@ -473,32 +481,44 @@ def do_filenames_need_updating(artifact):
473481
return False
474482

475483
@staticmethod
476-
def generate_filename(artifact, file_extension=None, max_pub_title_length = -1):
484+
def generate_filename(artifact, file_extension=None, max_pub_title_length = -1,
485+
include_type_suffix=True):
477486
"""
478487
Generates a filename for the given artifact.
479488
480-
This method generates a filename based on the artifact's first author's last name, title, forum name, and date.
489+
This method generates a filename based on the artifact's first author's last name, title, forum name, and date,
490+
plus the artifact-type segment of its class (``FILENAME_TYPE_SUFFIX``, e.g. "_Talk" — see #1404).
481491
If a file extension is provided, it is appended to the filename. Otherwise, a filename without extension is returned.
482492
483493
Parameters:
484494
artifact (Artifact): The artifact for which the filename is to be generated.
485495
file_extension (str, optional): The file extension to be appended to the filename. Defaults to None.
496+
include_type_suffix (bool, optional): Pass False for the pre-#1404 name (no type segment). Only the
497+
filename management commands need this, to recognize a file that was standardized under the old
498+
scheme; everything else wants the current scheme. Defaults to True.
486499
487500
Returns:
488501
str: The generated filename.
489502
490503
Example:
491-
>>> artifact = Artifact(first_author_last_name="Froehlich", title="Research Artifact Title", forum_name="CHI", date="2023-12-16")
492-
>>> generate_filename(artifact, file_extension=".pdf")
504+
>>> talk = Talk(title="Research Artifact Title", forum_name="CHI", date="2023-12-16") # first author: Froehlich
505+
>>> generate_filename(talk, file_extension=".pdf")
506+
'Froehlich_ResearchArtifactTitle_CHI2023_Talk.pdf'
507+
>>> generate_filename(talk, file_extension=".pdf", include_type_suffix=False)
493508
'Froehlich_ResearchArtifactTitle_CHI2023.pdf'
494509
"""
495-
510+
511+
# Read off the class, not the instance, so the type segment is a property of the model
512+
# (Poster has no type field, and Talk.talk_type is nullable and editor-editable — deriving
513+
# the segment from data would rename files on a metadata-only edit).
514+
type_suffix = type(artifact).FILENAME_TYPE_SUFFIX if include_type_suffix else None
515+
496516
# An empty string or a string with only whitespace characters is considered False in a boolean context.
497517
if not file_extension or not file_extension.strip():
498518
return ml_fileutils.get_filename_without_ext_for_artifact(
499-
artifact.get_first_author_last_name(), artifact.title,
500-
artifact.forum_name, artifact.date)
519+
artifact.get_first_author_last_name(), artifact.title,
520+
artifact.forum_name, artifact.date, type_suffix=type_suffix)
501521
else:
502522
return ml_fileutils.get_filename_for_artifact(
503-
artifact.get_first_author_last_name(), artifact.title,
504-
artifact.forum_name, artifact.date, file_extension)
523+
artifact.get_first_author_last_name(), artifact.title,
524+
artifact.forum_name, artifact.date, file_extension, type_suffix=type_suffix)

website/models/poster.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ class Poster(Artifact):
66
UPLOAD_DIR = 'posters/'
77
THUMBNAIL_DIR = os.path.join(UPLOAD_DIR, 'images/')
88

9+
# Standardized filenames end in "_Poster" (#1404) so a downloaded poster
10+
# isn't indistinguishable from the paper's PDF of the same work.
11+
FILENAME_TYPE_SUFFIX = 'Poster'
12+
913
external_slides_url = models.URLField(blank=True, null=True)
1014
external_slides_url.help_text = (
1115
"Optional link to the source design (e.g., Figma, Canva, Illustrator "

website/models/talk.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ class Talk(Artifact):
2323
UPLOAD_DIR = 'talks/'
2424
THUMBNAIL_DIR = os.path.join(UPLOAD_DIR, 'images/')
2525

26+
# Standardized filenames end in "_Talk" (#1404) so a downloaded slide deck
27+
# isn't indistinguishable from the paper's PDF of the same work.
28+
FILENAME_TYPE_SUFFIX = 'Talk'
29+
2630
external_slides_url = models.URLField(blank=True, null=True)
2731
external_slides_url.help_text = (
2832
"Optional link to the source slide deck (e.g., Figma, Google Slides, "

0 commit comments

Comments
 (0)