Skip to content

One field catalogue for every fund page - #102

Open
taavipertman-glitch wants to merge 3 commits into
masterfrom
fund-document-catalogue
Open

One field catalogue for every fund page#102
taavipertman-glitch wants to merge 3 commits into
masterfrom
fund-document-catalogue

Conversation

@taavipertman-glitch

@taavipertman-glitch taavipertman-glitch commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces #69 and #86, and carries everything they had that master lacks, so both can be closed. Step 1 of TODO — Fund page document publishing.md (private tuleva repo, work/investeerimistegevus/docs/, raised as TulevaEE/tuleva#511).

All four fund pages now take their documents — and the CO2 intensity figure published beside them — from one catalogue in helpers/acf/fund-disclosures.php, with a scope table declaring per page template whether each is required, optional, or absent.

Field names are identical everywhere a field exists, so nothing downstream branches per fund: fund → page slug, document → field name. What differs per fund is whether the field exists at all.

Why a rebuild rather than a rebase of #69

#69 predates ~19 commits of master and moved one field. Master has since added upcoming prospectus, terms and NAV procedure rows to all four pages — every one of them as a hardcoded template URL, including on TKF100, the fund that is supposed to be ACF-driven (#88, #93, c9bcf9b). Wiring a new ACF field by hand was slower than editing the template, so the template kept winning. Generating the field set from a catalogue removes that incentive.

Absent is a real state

Two states are not enough. if ($url) cannot tell "no CO2 intensity is calculated for TKF100" apart from "TUK75's Põhiteave link silently vanished".

wp-admin REST Page Gap check
absent no field key absent nothing silent
optional field key present renders when set silent
required field key present renders when set reported by tuleva_fund_disclosures_missing()

TKF100 is a UCITS fund: it carries investor_rights_file and its own nav_procedure_file. The pension funds carry neither and share one NAV procedure document. The pension funds carry fund_co2_intensity; TKF100 does not. A field outside a fund's scope returns '' and never reaches the fallback — an empty pension field must not borrow TKF100's document. There are tests for exactly that.

Turning a field off for one fund is one line in the scope table. Each fund has its own page template and ACF binds groups by template, so the granularity is per fund, not per fund type. The savings template still carries the CO2 markup, so publishing a TKF100 figure one day is a scope-table edit and nothing else.

What came from #86

The CO2 intensity figure, as a catalogue entry rather than its own Sustainability field group. It is not a document, but it has the same problem — published on the fund page, updated on a cadence, not the same set for every fund — so the catalogue is renamed from documents to disclosures rather than growing a member its name excludes.

Text, not number: 133.80 must not render as 133.8. The three literals were inconsistent about this (83.68 and 83.73 bare floats, '133.80' quoted); all three are now strings through the same path.

group_fund_savings_details does not get show_in_rest as it did in #86 — with CO2 in the catalogue, nothing writes to that group over REST. Exposing the fee fields there is a separate decision, and FEE-UPDATES.md is a separate process.

What came from #69

The generate_report_link() rewrite and its 171 lines of tests. Output is identical for the three live report URLs — verified by running both versions — but the function no longer emits unescaped HTML, keeps an absolute URL's own host instead of discarding it and re-prefixing get_site_url(), reads the reporting period from the YYYY-MM in the filename rather than the upload folder, and no longer produces labels like 12.-1. from intval() on an unmatched preg_match. All four matter more now that the URL arrives from an ACF field: FundReportMapping.buildPdfFilename() always embeds the period, while the upload folder is just the month the file happened to be uploaded, and those diverge on a re-upload — see 2bf6f27, where exactly that broke a URL.

Bugs this fixes

  • show_in_rest was missing on every field group, so ACF dropped the acf key from every REST write and returned 200 anyway. GET /wp-json/wp/v2/pages/35292 and /17533 both return "acf": [] on production right now — TKF100's automated path has never worked, and neither scripts/update_acf.py nor onboarding-service's WordPressMediaClient would have noticed.
  • nav_procedure_upcoming_file was read but registered nowhere. fund-savings-details.php read it; no field group defined it. It always returned null, so that row could only ever come from the hardcoded URL.
  • Two conventions for the same effective date — a hardcoded string on the pension pages, the hand-typed media library title on TKF100. Both now read …alates-DD.MM.YYYY from the filename, which the publishing script controls. TUK75-tingimused-kehtivad-alates-01.01.2027-1.pdf (a media re-upload suffix) is in the test cases.

No visible change

Rendered markup is identical on all four templates while the fields are empty, which they are in production. Verified by rendering each component with WordPress stubbed out, from master and from this branch, and comparing whitespace-normalised output — identical for all four, CO2 figures included. Rendering again with fields set shows the ACF value winning, the effective date following the new filename, and an out-of-scope field staying invisible.

Field keys TKF100's values are stored under are pinned in the catalogue, so moving those fields into the generated group does not orphan them. A test asserts each of the ten.

Notes for review

  • tuleva_fund_disclosures_missing() has no caller yet — it is what makes required mean something, and the admin notice in step 5 of the plan is its consumer. It is tested.
  • The three pension templates still render the four firm-wide documents (sustainability, adverse impacts, remuneration, pension NAV procedure) from helpers/extras.php constants. Those are one document for all four funds and move to the options page in step 3.
  • Fallback URLs stay in the templates until the fields are populated (step 2) and verified. Once a page's field is set, editing the literal changes nothing visible — CLAUDE.md now says so in as many words.

🤖 Generated with Claude Code

taavipertman-glitch and others added 2 commits September 10, 2026 10:39
Fund documents were split three ways: hardcoded in the pension templates,
hardcoded in helpers for the firm-wide ones, and ACF fields on TKF100. The
ACF half never worked — no field group declared show_in_rest, so ACF dropped
the "acf" key from every REST write and returned 200 anyway.

All four pages now take their documents from one catalogue, with a scope table
saying per page template whether each document is required, optional, or
absent. Absent is a real state: TKF100 is a UCITS fund with a summary of
investor rights and its own NAV procedure, the pension funds have neither, and
a document outside a fund's scope has no field, no REST key, and never falls
back to another fund's document.

nav_procedure_upcoming_file was read by the savings template but registered in
no field group, so it always returned null and that row could only be changed
in code. It is in the catalogue now.

Upcoming documents take their effective date from the filename on all four
funds, replacing a hardcoded string on the pension pages and the hand-typed
media library title on TKF100.

Nothing changes on the pages: rendered markup is identical for all four
templates while the fields are empty, which they are in production today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports what #69 and #86 carried that master still lacks, so both can be closed.

From #86: the CO2 intensity figure. It is not a document, but it has the same
problem — published on the fund page, updated on a cadence, and not the same
set for every fund — so it becomes a catalogue entry rather than a field group
of its own. That makes "no CO2 intensity is calculated for TKF100" an absent
line in the scope table instead of an empty field nobody can explain, and
publishing one later a scope-table edit. The catalogue is renamed from
documents to disclosures to stop the name lying about what it holds.

From #69: the generate_report_link() rewrite and its tests. Output is
unchanged for the three live report URLs, but the function no longer emits
unescaped HTML, keeps an absolute URL's own host instead of re-prefixing
get_site_url(), reads the reporting period from the filename rather than the
upload folder, and stops producing labels like "12.-1." from an unmatched
preg_match. All four matter more now that the URL arrives from an ACF field
rather than a literal in the template.

Rendered markup stays identical on all four fund pages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@taavipertman-glitch taavipertman-glitch changed the title Give every fund page the same document fields One field catalogue for every fund page Sep 10, 2026
…values

Codex review of this branch. The first finding is a live regression, the
rest are latent.

The three pension templates use a non-breaking space inside the gettext
msgid ' (in Estonian)'. Retyping those rows replaced it with an ordinary
space in five strings per file, so the lookup missed and the Estonian
pages would have rendered the English msgid instead of its translation —
which for this string is a single space, i.e. the suffix is meant to be
invisible. Members would have seen a literal "(in Estonian)" and
"(in Estonian, effective from 01.01.2027)" on every pension fund page.

The render comparison that was supposed to catch this could not: it
normalised with \\s, and Python's \\s matches U+00A0, so the one character
that changed was the one being collapsed. The comparison now folds ASCII
whitespace only, and counts non-breaking spaces on both sides.

Three latent faults, all cases where an empty value produced markup
rather than nothing:

- paired prospectus/terms rows printed both anchors behind a single ||
  guard, so a missing half would render href="" once the fallbacks go
- the report row and the CO2 block rendered unconditionally, so an empty
  value would link to the site root and show a figure with no number
- the effective date was taken from the filename with no validation. The
  pension funds' NAV procedure is published as ...alates-02.03.3026.pdf,
  a typo for 2026 that is harmless only while nothing renders the date

Verified by rendering a pension template with every field out of scope:
no empty hrefs, no report row, no CO2 block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@taavipertman-glitch

Copy link
Copy Markdown
Contributor Author

Codex review found a live regression that my own verification was structurally unable to catch. Fixed in aade1fd.

The regression. These templates use a non-breaking space inside the gettext msgid (in Estonian). Retyping those rows replaced it with an ordinary space in five strings per pension template, so the lookup missed. In et.po that msgid translates to a single space — the suffix is meant to be invisible in Estonian — so the pages would have rendered a literal "(in Estonian)" and "(in Estonian, effective from 01.01.2027)" on all three pension fund pages.

Why the render comparison missed it. I compared master's and this branch's rendered markup with whitespace normalised via \s. Python's \s matches U+00A0, so the single character that changed was precisely the one being collapsed. The comparison now folds ASCII whitespace only and counts non-breaking spaces on both sides: 11 per pension template, matching master.

Three latent faults, also fixed — each a case where an empty value produced markup instead of nothing:

  • paired prospectus/terms rows printed both anchors behind one || guard, so a missing half renders href="" once the fallbacks come out
  • the report row and CO2 block rendered unconditionally, so an empty value would link to the site root and print a figure with no number
  • the effective date was read from the filename with no validation. The pension funds' NAV procedure is published as ...kehtib-alates-02.03.3026.pdf — a typo for 2026, harmless only while nothing renders the date. Now range- and calendar-checked, falling back when implausible.

Verified by rendering a pension template with every field out of scope: no empty hrefs, no report row, no CO2 block, only the five firm-wide rows. 69 tests pass; all four pages still render identically to master.

Codex's remaining finding is the one already tracked as step 2 of the plan: no ACF assertion here has been observed against a real WordPress — pinned keys preserving values, location rules binding, show_in_rest accepting the write are all reasoned, not watched. Codex could not check it either; ACF Pro is not in the checkout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant