Skip to content

Add ACF investment report field for pension fund pages - #69

Open
taavipertman-glitch wants to merge 7 commits into
masterfrom
acf-investment-report-fields
Open

Add ACF investment report field for pension fund pages#69
taavipertman-glitch wants to merge 7 commits into
masterfrom
acf-investment-report-fields

Conversation

@taavipertman-glitch

@taavipertman-glitch taavipertman-glitch commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add the investment_report_file ACF field to TUK75, TUK00 and TUV100 fund pages, reusing the exact field name TKF100 already has
  • Enable show_in_rest on the field groups that carry it, so the field can be set over the REST API
  • Templates use the ACF field when set and fall back to the current hardcoded URL when not

What this enables

One ACF field name serves all four funds, so #1619 needs no per-fund branching.

Changes

  • helpers/acf/fund-pension.php — new field group for page templates page_fund-stocks.php, page_fund-bonds.php, page_fund-third.php
  • helpers/acf/init.php — include the new file
  • helpers/acf/fund-savings.php — add show_in_rest to group_fund_savings_documents
  • fund-stocks-details.php, fund-bonds-details.php, fund-third-details.php — read get_field('investment_report_file'), hardcoded fallback
  • helpers/extras.phpgenerate_report_link() rewrite (see below)
  • tests/helpers/ReportLinkTest.php — new coverage for generate_report_link()
  • CLAUDE.md — Step 4 no longer claims the ACF REST API is unavailable

show_in_rest is the part that actually makes automation work

onboarding-service posts {"acf": {"investment_report_file": <attachment id>}} to /wp-json/wp/v2/pages/{id}. ACF only honours the acf key when the field group declares 'show_in_rest' => 1; without it the key is dropped and the write silently no-ops. The theme has no acf/rest or register_rest_field filter that would enable this globally.

This gap also existed on group_fund_savings_documents, so TKF100's automated path has never worked either — that group is fixed here too. It is group-level, so all ten savings document fields become REST-readable; every one of them is already rendered publicly on the fund page, so nothing new is exposed. Writes still require edit_post.

Behaviour change in generate_report_link() — affects TKF100 too

This function is shared with fund-savings-details.php. It now derives the report period from a YYYY-MM in the filename, falling back to the old "month before the upload folder" rule only when the filename has no period.

For every URL in the repo today the two rules agree, so no label changes on merge. The rewrite is what makes the ACF path correct: FundReportMapping.buildPdfFilename() always embeds the report period in the filename, while the upload folder is the month the file happened to be uploaded. Those diverge on a media re-upload — see 2bf6f27, where exactly that broke a URL — and the old rule would then mislabel the report.

Also in the rewrite:

  • esc_url() / esc_html() on output, replacing an unescaped sprintf
  • absolute URLs keep their own host; previously the host was discarded and get_site_url() re-prefixed, which would break media served from another host
  • a URL with no date in either the filename or the path no longer produces labels like 12.-1. from intval() on an unmatched preg_match
  • guards basename() against parse_url() returning null for a pathless URL

Deploy notes

Safe to deploy. The three pension pages keep rendering their current hardcoded 2026-07 reports until someone sets the ACF field; TKF100 is unaffected until its field changes.

Before treating #1619 as unblocked, confirm in wp-admin that the pages onboarding-service targets by slug are actually on the matching page templates — ACF applies the field group by page_template, while findPageIdBySlug looks up tuleva-maailma-aktsiate-pensionifond, tuleva-maailma-volakirjade-pensionifond, tuleva-iii-samba-pensionifond and taiendav-kogumisfond. If a page is on a different template the write no-ops, which is the same failure mode as the missing show_in_rest.

Follow-up

Once the ACF fields are populated on all four pages, drop the hardcoded fallback URLs. Until then the monthly report update still touches these three templates.

🤖 Generated with Claude Code

taavipertman-glitch and others added 6 commits May 28, 2026 15:06
Add investment_report_file ACF field to the stocks, bonds, and third
pillar fund page templates, matching the pattern TKF100 already uses.

When the ACF field is set (via wp-admin or REST API), it overrides the
hardcoded report URL. When not set, the current hardcoded URL remains
as fallback — safe to deploy without setting the fields first.

This enables:
- Manual report updates from wp-admin (same as TKF100 today)
- Automated updates via WordPress REST API from onboarding-service

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add esc_url() and esc_html() to prevent potential XSS via
ACF-managed URLs. Guard preg_match before accessing $matches
to avoid PHP notices when URL has no /YYYY/MM/ date segment.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
generate_report_link() previously discarded the URL host and rebuilt the link
against get_site_url(), which breaks ACF-supplied media URLs served from a CDN
or dedicated media host. Absolute URLs now pass through with their host intact;
only site-relative paths are resolved against the site.

The date label was derived from the WordPress upload-folder month minus one,
which is wrong when a report is uploaded in a different month than the period it
covers. The investment report pipeline encodes the period directly in the
filename as YYYY-MM, so parse that when present and fall back to the legacy
folder-month heuristic only for the old hardcoded report URLs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rt-fields

# Conflicts:
#	src/wp-content/themes/tuleva/templates/components/fund-bonds-details.php
#	src/wp-content/themes/tuleva/templates/components/fund-stocks-details.php
#	src/wp-content/themes/tuleva/templates/components/fund-third-details.php
Resolve the three fund template conflicts by keeping master's current
2026-07 report URLs as the fallback, wrapped in the new ACF lookup.

Add 'show_in_rest' => 1 to the pension and savings document field
groups. Without it ACF drops the "acf" key from a REST page update, so
onboarding-service's updateAcfReportField write would silently no-op —
this is what #1619 is gated behind. The gap also meant TKF100's
automated path had never been exercised.

Use the existing 'Investment reports' string for the no-period, no-label
case instead of a new untranslated 'Report'.

Cover generate_report_link with tests: the filename period now takes
precedence over the upload-folder month for every caller, including
fund-savings-details.php.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parse_url() returns null for "https://tuleva.ee", and basename() has
rejected null since PHP 8.1. The rewrite introduced the basename() call,
so this path emitted a deprecation that phpunit.xml would fail on
(failOnWarning="true").

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
labelAndUrlAreEscaped pinned "&amp;b=2". esc_url() is stubbed here with
htmlspecialchars(), which writes & as &amp;; real WordPress writes
&#38;. So the assertion tested the stub -- it would have passed against
an implementation WordPress never produces, and told us nothing about
the escaping that actually ships.

Now it asserts the property instead: no raw & inside the href, whichever
entity form is used. Added a companion that fails if esc_url()/esc_html()
stop being called at all -- the markup has exactly two quoted attributes,
so a fifth double quote in the output is a payload that escaped its
attribute. Verified all four assertions pass under both the stub's
encoding and WordPress's, and fail with escaping removed.

Dropped the alert(1) assertion that came with the first draft of this:
the text survives escaping by design, inert, so asserting its absence
would have been wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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