Remove Django comments (#1130) — phase 1: code removal + queue-safety scrub

[rdhyee]

Removes Django comments functionality per #1130 — Eric-approved (2026-04-23, re-confirmed 2026-08-03): comments are a spam magnet with low-value content, and removing the app before the Django 5.2 upgrade eliminates the django_comments.0004 index migration from the 5.2 path entirely.

Phase 1 of 2 — code removal + one non-destructive migration

This PR removes every runtime reference to django_comments; the DB tables and notice-type rows stay behind, orphaned and unreferenced. A follow-up migration-only PR (phase 2, comments-drop-migration-1130) deletes the notice types and drops the tables — it merges only after this release is deployed and all processes restarted, which makes the destructive step safe under every deploy path (no restart/migrate ordering to get wrong; Codex R2 finding).

The one migration here (core.0031_scrub_comment_notice_batches) deletes pending notification-queue batches that carry comment notices. It must ship with this release (Codex R3 HIGH): those batches embed pickled Comment instances that the comment-free code can neither unpickle nor render, and notification.engine.send_all() blocks on the first failing batch — one stale spam-comment notification would wedge the entire queue. The scrub is non-destructive and safe under any deploy ordering (old code is indifferent to deleted pending batches); phase 2 re-runs it to catch anything created in the migrate→restart window.

⛔ Merge order

Merge after #1213. This branch's migration core.0031 depends on #1213's core.0030_retire_pledge_b2u — the agreed sequencing (#1213 → #1130 → 5.2) is encoded structurally, and migrate fails loudly (NodeNotFoundError) if mis-ordered.

What's removed

• App wiring: django_comments out of INSTALLED_APPS; /comments/ URL includes (both the django_comments URLs and the site's latest-comments page)
• Work page: Comments tab gone. Tab numbering deliberately stays 1/3/4 — tabs4.js and the CSS bind by fixed class names (tabs2, #tabs-2), and empty jQuery selections no-op, so no JS/CSS churn. Inbound ?tab=2 links (old notification emails) degrade to the Description tab via the view's whitelist.
• Homepage activity feed: comment events out of the pledges/wishes/comments chain (feed logic and template branch)
• Notifications: comment_was_posted handler and the 3 comment notice types (comment_on_commented, wishlist_comment, wishlist_official_comment); their template dirs deleted; ?tab=2 CTAs stripped from surviving notices; the now-empty "Comment Notifications" section removed from settings
• Model plumbing: Comment cleanup in work-delete (bibmodels) and comment re-pointing in work-merge (bookloader)
• Explore sidebar: Latest Comments link
• Tests / deps: comment fixtures out of the merge test and booktests helper; django-contrib-comments out of requirements.txt and Pipfile (16 template files deleted; ~470 lines net removed)

Deploy sequence (per box; test first, then prod)

1. Deploy-guide §5 chained checkout → deps → migrate (runs the order-tolerant core.0031 scrub)
2. deploy.yml restart onto the same SHA — comment ingress ends here, permanently
3. manage.py scrub_comment_notice_batches — definitive post-restart scrub (a poisoned batch created in the step-1→2 window would otherwise wedge the notification queue; failed batches aren't consumed, so this also un-wedges)
4. Verify: the command prints examined/scrubbed/remaining counts (expect remaining → 0); then smoke-test homepage, a work page (tabs), explore, notification settings

run_pip=true not strictly required (dependency removed, not added — pip won't uninstall; the package sits inert in existing venvs until a rebuild).

Review

Codex review: 5 rounds (R1: 2 HIGH — deploy-window table drops, notification-queue wedge via stale pickled batches; R2: two-phase split adopted, scrub precision upgraded to label-slot inspection; R3: scrub moved into phase 1 + interphase settings filter; R4: post-restart scrub command closes the ingress race; R5: final). Scrub logic verified against synthetic pickle fixtures (unimportable embedded class, label-in-prose false-positive case, protocol 0).

🤖 Generated with Claude Code

https://claude.ai/code/session_01DD8dp3nkAyH2KeZymEgW5W

[rdhyee]

[Codex via CC] Codex (gpt-5.4) review — 5 rounds, final verdict: LGTM (2026-08-06)

Round 1 (NOT-LGTM): 2 HIGH — (a) dropping tables in the same deploy as the code swap creates a 500 window for old in-memory workers (and celery isn't restarted by every path); (b) pending NoticeQueueBatch pickles carrying comment labels permanently wedge send_all() once NoticeTypes are deleted (it only tolerates missing users; any other exception escapes past batch.delete()). Plus leftovers (Pipfile, _template_map.txt, empty notification-settings section, stale contenttypes).

Round 2 (NOT-LGTM): documentation alone can't enforce restart-before-migrate on every deploy path (full provision runs migrate as a task while restarts are deferred handlers) → two-phase release split adopted. Also flagged the byte-scan scrub's false-positive risk (a comment label appearing in unrelated message text) → upgraded to restricted-unpickler label-slot inspection.

Round 3 (NOT-LGTM): phase 1 alone could still wedge the queue — existing comment batches become unprocessable the moment the app leaves INSTALLED_APPS → scrub migration moved into phase 1; interphase notification-settings filter added.

Round 4 (NOT-LGTM): the migrate→restart ingress window can still admit one poisoned batch → post-restart manage.py scrub_comment_notice_batches step added (definitive: ingress has structurally ended; failed batches aren't consumed, so the scrub also un-wedges).

Round 5: LGTM — "The R4 HIGH is closed. The post-restart command runs after comment ingress has permanently ended, removes any interphase straggler, and leaves no mechanism for re-poisoning the queue. No new findings." Codex additionally verified the restricted unpickler across pickle protocols 0–5 (precise label detection, no payload-text false positives) and the phase-2 cascade/FK-drop ordering.

Scrub logic was also verified empirically against synthetic fixtures (comment batch embedding an unimportable class; label-in-prose false-positive case; protocol-0 pickle) — 4/4 as expected.

Still pending before the CC+Codex+LGTM label: Django suite + smoke on test.unglue.it.

🤖 Posted by Claude Code on Raymond's behalf

[rdhyee]

[CC] Test-box validation complete (2026-08-06, both phases rehearsed end-to-end on test.unglue.it at integration SHA 5e4a21ca = retire-pledge-b2u-1195 + this PR + #1218):

Phase 1 (this PR):

• §5 chained migrate → core.0031 scrub applied cleanly (migration graph resolved against Retire Pledge & Buy-to-Unglue campaigns (#1195) #1213's core.0030_retire_pledge_b2u — hence this PR's merge-after-Retire Pledge & Buy-to-Unglue campaigns (#1195) #1213 gate)
• deploy.yml restart → apache2/celeryd/celerybeat all active
• manage.py scrub_comment_notice_batches → batches examined: 1, scrubbed: 0, remaining: 1 — correctly preserved a legit pending rights_holder_claim batch while scrubbing nothing (no comment batches present)
• Live queue-drain proof: that pending batch (stranded since 8/1 — emit had been dormant on test since the 8/1 deploys, pre-existing condition) emitted successfully under comment-free code on the next beat cycle: 1 batches, 2 sent, queue now 0 and cycling 0 batches, 0 sent every 10 min
• Smoke: / 200, work page 200 with tabs 1/3/4 and zero comment artifacts, ?tab=2 degrades to Description (200), /comments/ 404 as intended, /free/ 200, notification settings 200
• Full Django suite: 117 tests, no new reds vs baseline. The 17 pre-existing failures/errors all trace to box-config or environment causes (LIBRARYTHING_KEY missing ×6, expired Stripe test key, assertRegexpMatches removed in py3.12, booxtream bytes/tempfile drift, LibraryThing 403, OpenLibrary data drift, RhPage csrf ×3, libraryauth registration) — none touch comments, notifications, or merge paths, and 7 of them were red on the pre-integration SHA as well.

Phase 2 (#1218) rehearsed in its intended order (restart first, then migrate):

• core.0032 applied → comment tables gone, 3 NoticeTypes gone (38 others intact), ContentTypes gone for both django_comments and the ancient pre-1.6 comments app label (the latter discovered during rehearsal — 6 stale permissions attached; migration extended and re-verified via rollback-reapply), all comment permissions 0, queue 0
• Post-migration smoke: all 200s

Applying CC+Codex+LGTM: Codex LGTM (5 rounds, verdict above) + CC LGTM now grounded in the live rehearsal.

🤖 Posted by Claude Code on Raymond's behalf