Remove the read-only "Originaltext" date field that confuses editors (#710) #712
Reference in New Issue
Block a user
Delete Branch "feat/issue-710-remove-originaltext"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #710.
What
Removes the read-only "Originaltext:" date field (the verbatim import cell,
metaDateRaw) from everywhere it was visible in the frontend. The data stays untouched in the backend — we only stop showing and re-submitting it.metaDateRawhad two roles; only the visible one is removed:seasonFromRaw) — kept, so no date label silently changes.Changes (frontend only — no backend change)
WhoWhenSection.svelte{#if rawDate}block (visible text + the hiddenmetaDateRawround-trip input) and therawDateprop.DocumentEditLayout.svelterawDate=…prop passed intoWhoWhenSection.DocumentDate.svelteshowRawprop, theshowRawLinederived, and the stale CWE-79/WCAG comment. Kept therawprop (feeds the season word).DocumentRow.svelteshowRaw={false}props and the comment justifying them.DocumentMetadataDrawer.svelte{#if documentDate || metaDateRaw}to{#if documentDate}so a raw-only/undated document shows "—" instead of leaking raw text.documentDate.tsmessages/{de,en,es}.jsondate_original_labelkey.Tests
WhoWhenSectiontest flipped to assert the raw block and the hiddeninput[name="metaDateRaw"]are absent — this guards the core "no round-trip" acceptance criterion.DocumentDatetests asserting on the deleted DOM sink (UNKNOWN secondary line + its XSS-escaping). DAY/MONTH coverage kept. No XSS surface is lost — the only survivingrawconsumer isseasonFromRaw, which matches a fixed allowlist and never emits raw text.DocumentMetadataDrawertest: an undated, raw-only document renders "—" and never surfaces the raw text.Verification
npm run lint✅ (clean)npm run check✅ (no new errors vs.mainbaseline)🤖 Generated with Claude Code
DocumentDate rendered an "Originaltext: <raw>" secondary line for UNKNOWN/SEASON/APPROX dates, gated by a showRaw prop. Drop the visible line, the showRaw prop, the showRawLine derived, and the now-unused date_original_label message import. The raw prop stays — it still feeds the SEASON word in formatDocumentDate, which only ever maps a fixed German season token (never emits raw text), so no XSS surface remains. Update both DocumentRow call sites to drop the now-gone showRaw={false} and the comment that justified it. Remove the two DocumentDate tests that asserted on the deleted DOM sink (the UNKNOWN secondary line and its XSS-escaping); the DAY/MONTH coverage stays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>The detail drawer's date cell rendered DocumentDate whenever a date OR a raw cell was present (`{#if documentDate || metaDateRaw}`). For an undated, raw-only document that meant the verbatim import text leaked into the view. Tighten the guard to `{#if documentDate}` so such a document shows "—". The raw prop is still passed through for the SEASON word on dated documents. Covered by a new test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ✅ Approved
Clean, focused deletion. The PR does exactly what it says: it removes a confusing UI element and its round-trip input while keeping the one legitimate internal consumer (
seasonFromRaw). No dead code left behind, comments updated rather than orphaned, and the TDD evidence is present (tests changed alongside the production code, asserting the absence of the removed DOM).What I checked
showRaw,showRawLine, andrawDateare removed everywhere they were defined and consumed (DocumentDate.svelte,WhoWhenSection.svelte,DocumentEditLayout.svelte,DocumentRow.svelte). No caller still passesshowRaw=orrawDate=. ✔rawprop kept correctly —DocumentDatestill acceptsrawand forwards it toformatDocumentDate(...), andDocumentMetadataDrawerstill passesraw={metaDateRaw}. The season word path is intact. ✔raw("used only to derive the SEASON word, never displayed") is exactly the kind of why-comment I want. The stale CWE-79/WCAG comments that described a now-deleted sink are correctly gone. ✔{#if documentDate || metaDateRaw}→{#if documentDate}is the right call: it removes the only path that surfaced raw text, and the{:else}em-dash branch handles the undated case cleanly. ✔date_original_labeldropped from all three locales (de/en/es) symmetrically. No dangling key, no orphaned usage. ✔Suggestions (non-blocking)
metaDateRawas a prop and threads it through purely as the season-word source. That's correct, but a one-word rename or a short comment at the prop site (mirroring the one you added inDocumentDate.svelte) would make it obvious to the next reader thatmetaDateRawis no longer a display field. Minor.Nothing blocks merge. This is the kind of small, honest cleanup I like.
🏛️ Markus Keller — Application Architect
Verdict: ✅ Approved
This is a frontend-only presentation-layer cleanup with no module-boundary, transport, schema, or data-flow implications. Nothing here changes who owns what.
What I checked
ErrorCode, orPermission. ThemetaDateRawcolumn and its API contract are untouched; only the display of that field is removed. ✔metaDateRawarrives in the drawer's$props()and is forwarded asraw). No client-side fetch introduced, no boundary crossed. ✔showRawboolean flag and the derivedshowRawLinereduces the component's surface. The remainingraw → seasonFromRawcoupling is a single, well-named, documented path. That's the right level of abstraction; I would not split it further.Note (not a blocker)
The data model now has a column (
metaDateRaw) whose only surviving consumer is a season-word derivation. That's a deliberate product decision, not architectural debt — the PR body documents it and the issue (#710) is the source of truth. No ADR needed; this is reversible UI scope, not a structural choice.Approved.
🔐 Nora "NullX" Steiner — Application Security Engineer
Verdict: ✅ Approved
This PR removes attack surface, it does not add any. From a security standpoint it's a net improvement, and the reasoning in the PR body is sound.
Threat-model analysis
The
metaDateRawcell is untrusted verbatim spreadsheet text — exactly the kind of input I treat as hostile. Before this PR it was reflected into the DOM in three rendered sinks (theDocumentDatesecondary line and theWhoWhenSection"Originaltext" block). All three were rendered via Svelte's default{...}interpolation, which HTML-escapes — so they were correctly not XSS (CWE-79), but they were still untrusted data on screen.After this PR the only surviving consumer of
rawisseasonFromRaw(raw)indocumentDate.ts. I checked that function's contract via the surrounding comment and the formatter docstring: it matches the raw cell against a fixed allowlist of German season tokens and emits a localized season word, never the raw string. So there is no path by whichmetaDateRawreaches the DOM anymore — escaped or otherwise. ✔<input type="hidden" name="metaDateRaw">also closes a (benign but real) mass-assignment-style round-trip where verbatim import text was re-submitted on every edit. Removing it is good hygiene. ✔DocumentMetadataDrawerguard tightening — narrowing{#if documentDate || metaDateRaw}to{#if documentDate}is a small but genuine data-exposure fix: previously a raw-only/undated document would enter theDocumentDatebranch; now it renders a neutral em-dash and the raw text never leaks. ✔{@html}, noinnerHTML, noevalanywhere in the diff. ✔Regression coverage (educational note, not a blocker)
The two deleted
DocumentDatetests included the XSS-escaping assertion (<img src=x onerror=...>rendered inert). Deleting them is justified — the sink they guarded no longer exists, so the test would be testing nothing. The newWhoWhenSectiontest (input[name="metaDateRaw"]is absent) and the new drawer test (raw text never surfaces) are good replacements that lock in the "no round-trip / no leak" property. If you want belt-and-suspenders, a single assertion inDocumentDate.svelte.test.tsthat a maliciousrawvalue produces no visible text would permanently document "raw is never a display sink" — but it's optional given the season path is allowlist-bound.Approved. Good defensive cleanup.
🧪 Sara Holt — Senior QA Engineer
Verdict: ⚠️ Approved with concerns
The test changes are well-structured and the deletions are defensible, but I want to be precise about what coverage moved where, and flag one assertion pattern worth double-checking.
What's good
WhoWhenSectiontest asserts both[data-testid="who-when-raw"]andinput[name="metaDateRaw"]aretoBeNull(). That's the right regression guard for the core acceptance criterion ("no round-trip"), and it reads as a sentence. ✔'shows an em-dash and never the raw cell for an undated, raw-only document'covers exactly the boundary the guard change introduced (undated + raw-only). One behavior, clear name. ✔DocumentDatetests is justified — they asserted on a DOM sink that no longer exists. Keeping them would be testing a deleted feature. DAY/MONTH label coverage is retained. ✔Concerns (please verify, not necessarily blocking)
getByText(...).not.toBeInTheDocument()for absence. In the new drawer test and theWhoWhenSectiontest you mix two styles:document.querySelector(...)+toBeNull()(robust for absence) andpage.getByText('Sommer 1916').not.toBeInTheDocument(). The locator-based negative assertion is the right vitest-browser idiom, but absence assertions are the classic spot for false greens — if the text were rendered slightly differently (e.g. wrapped, or with surrounding whitespace) a positive assertion would catch it but a negative one silently passes. Since these are browser-project (Playwright) tests that only run in CI, please confirm CI is green on this branch before merge — I can't validate them locally and won't trust "it compiles."Em-dash assertion is broad.
dashTexts.length > 0over alldd, pelements proves an em-dash exists somewhere, not that it's the date cell specifically. It's the same pattern as the existing undated test so it's consistent, but it would pass even if the date cell rendered raw text and some other field happened to show "—". Scoping the assertion to the date<dd>would make the failure mode unambiguous. Minor — the companionSommer 1916-is-absent assertion mostly covers the gap.Coverage note
Net branch coverage should be neutral-to-slightly-down (you removed the
showRawLinederived and its branches along with their tests). That's expected and fine — you deleted logic, not left it untested. Just don't let a coverage-ratchet gate flag this as a regression.No blockers. Confirm CI green on the browser project and I'm happy.
🎨 Leonie Voss — UX & Accessibility Lead
Verdict: ✅ Approved
Removing the "Originaltext:" line is the right UX call. For our dual audience — and especially the 60+ transcribers — a verbatim spreadsheet cell shown next to a clean, formatted date is noise: two competing date representations create exactly the "which one is real?" hesitation that Nielsen's aesthetic and minimalist design heuristic warns against. The screen now states the date once, honestly.
What I checked
seasonFromRaw; users don't lose the human-readable approximate date. Only the redundant raw echo is gone. ✔DocumentDate.svelteretains the neutral undated chip (#668): the calendar-with-question glyph is decorative (aria-hidden), and the visible "Datum unbekannt" text is the real, announced cue. Removing the raw line did not touch that pattern. An absence (undated) is still presented as neutral metadata, never as an error — no red/amber. ✔?? '—'. ✔DocumentRowalready suppressed the raw line (showRaw={false}); now that the line is gone everywhere, list rows and detail views are consistent rather than divergent. That's a small IA win — same date representation everywhere. ✔Suggestion (non-blocking)
None of real substance. If anything, this is a model "less is more" cleanup. I'd only note for the changelog/issue that editors transcribing handwritten dates now rely entirely on the structured precision controls — which is the intended workflow, and better than the old dual display.
Approved with enthusiasm.
📋 "Elicit" — Requirements Engineer / Business Analyst (Brownfield)
Verdict: ✅ Approved
This PR has an unusually clean requirements trace, which is exactly what I look for in a brownfield cleanup. It closes #710 (the source of truth), states the why in user terms, and is explicit about scope and non-scope.
Requirements assessment
metaDateRawcell anywhere in the UI, while continuing to derive the season word from it internally." Both halves are observable and are covered by tests (raw-line/hidden-input absent; season path retained). ✔Open questions / ambiguity (worth confirming, not blocking)
metaDateRawas data. The column now exists solely to feed a season word. That's fine today, but it's a latent requirements question: ismetaDateRawever expected to become user-visible again (e.g. an admin/provenance view), or is it permanently an internal derivation input? If the latter, consider a follow-up issue to document that decision in the glossary so a future editor doesn't "rediscover" the field and re-surface it. Logging this as OQ-710-01 — not a blocker.Both items are clarifications for the backlog, not defects. The change as specified is complete, testable, and well-scoped. Approved.
⚙️ Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved (LGTM)
Nothing in my domain to flag. I checked the diff for anything operational and found none:
docker-compose*.yml, no Caddyfile, no CI workflow, no Dockerfile, no env-var or secret changes. ✔pom.xml, no new dependency, no migration that would run on deploy. ✔The only thing that runs in CI for this PR is the frontend lint/check/unit + the browser-project component tests. As long as the pipeline is green (the new browser tests only execute in CI), there's nothing here that affects the VPS, the Compose stack, or the monthly bill.
Approved — no operational concerns.