Watch

Moved common header code into the included partial template #493

Merged

johnsturgeon merged 2 commits from figure-out-if-there-is-a-way-to-include-common-bits-of-the-three-base-jinja-templates-491 into main

2026-09-07 13:14:42 +02:00

johnsturgeon commented

2026-09-07 13:10:28 +02:00

Owner

Moved all common favicon setup to new file
Standardized on the cache-busting version of the CSS file
Added the Umami tracking script to the common new file

- Moved all common favicon setup to new file - Standardized on the cache-busting version of the CSS file - Added the Umami tracking script to the common new file

johnsturgeon added 2 commits

2026-09-07 13:10:28 +02:00

Migrated common header config to included header file 94868a126f

I moved all common favicon setup to new file
I standardized on the cache-busting version of the CSS file
I added the Umami tracking script to the common new file

Resolves #491

Claude Code Review / claude-review (pull_request) Successful in 1m21s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 46s

Details

Tests / pytest (pull_request) Successful in 27s

Details

Release on merge / release (pull_request) Successful in 0s

Details

427f37a97c

forgejo-actions commented

2026-09-07 13:11:50 +02:00

This is a clean, well-scoped refactor. I verified that config, request/url_for are present in the render context of every route that renders these templates (main.py:164, 215, 217, 303, 307, 451, 455, 519, 523 and the base.j2/allpicksbase.j2 consumers), and that UMAMI_JS_SCRIPT_URL/UMAMI_TRACKING_ID exist in config/op.env, so the extracted partial has everything it needs. No correctness or security regressions.

Review

Solid deduplication — four near-identical <head> blocks collapse into one _common_header.j2, and the previously non-cache-busted mainstyle.css in allpicksbase.j2 now uses the versioned URL like everyone else. Jinja {% include %} inherits the parent context by default, and every rendering route passes config, so the partial resolves correctly.

Worth fixing

app/templates/setting_up.j2 now fires Umami on a 60-second auto-refresh loop. setting_up.j2 keeps its <meta http-equiv="refresh" content="60"/> and now pulls in _common_header.j2:7, which adds the Umami tracking script it did not previously have. Every user parked on the "setting up" page will now generate a Umami pageview once a minute for as long as they wait, inflating analytics. If that page is meant to be untracked, consider gating the Umami script (e.g. a {% block %}/flag the partial honors) rather than unconditionally including it there.

Nits

_common_header.j2:2 places the stylesheet <link> before the favicon/manifest links, whereas the old base.j2 had favicons first. Purely cosmetic (head-element order is not significant here), just noting the reordering was silent.
Include position differs across files — base.j2 puts <title> before the include, setting_up.j2 puts the include before <title>. Harmless, but standardizing the order would make the partial's insertion point predictable.
The extracted Sentry loader still pins the 7.120.0 CDN bundle and sets replaysSessionSampleRate without loading a Replay integration — both pre-existing and out of scope for this PR, but now that it lives in one place it's a cheaper thing to revisit later.

No test coverage is expected for a pure template move; the offline suite doesn't render these. Looks good to merge once the setting_up Umami behavior is confirmed intentional.

This is a clean, well-scoped refactor. I verified that `config`, `request`/`url_for` are present in the render context of every route that renders these templates (`main.py:164, 215, 217, 303, 307, 451, 455, 519, 523` and the `base.j2`/`allpicksbase.j2` consumers), and that `UMAMI_JS_SCRIPT_URL`/`UMAMI_TRACKING_ID` exist in `config/op.env`, so the extracted partial has everything it needs. No correctness or security regressions. --- ## Review Solid deduplication — four near-identical `<head>` blocks collapse into one `_common_header.j2`, and the previously non-cache-busted `mainstyle.css` in `allpicksbase.j2` now uses the versioned URL like everyone else. Jinja `{% include %}` inherits the parent context by default, and every rendering route passes `config`, so the partial resolves correctly. ### Worth fixing - **`app/templates/setting_up.j2` now fires Umami on a 60-second auto-refresh loop.** `setting_up.j2` keeps its `<meta http-equiv="refresh" content="60"/>` and now pulls in `_common_header.j2:7`, which adds the Umami tracking script it did not previously have. Every user parked on the "setting up" page will now generate a Umami pageview once a minute for as long as they wait, inflating analytics. If that page is meant to be untracked, consider gating the Umami script (e.g. a `{% block %}`/flag the partial honors) rather than unconditionally including it there. ### Nits - **`_common_header.j2:2` places the stylesheet `<link>` before the favicon/manifest links**, whereas the old `base.j2` had favicons first. Purely cosmetic (head-element order is not significant here), just noting the reordering was silent. - **Include position differs across files** — `base.j2` puts `<title>` before the include, `setting_up.j2` puts the include before `<title>`. Harmless, but standardizing the order would make the partial's insertion point predictable. - The extracted Sentry loader still pins the 7.120.0 CDN bundle and sets `replaysSessionSampleRate` without loading a Replay integration — both pre-existing and out of scope for this PR, but now that it lives in one place it's a cheaper thing to revisit later. No test coverage is expected for a pure template move; the offline suite doesn't render these. Looks good to merge once the `setting_up` Umami behavior is confirmed intentional.

johnsturgeon merged commit 7807df9416 into main

2026-09-07 13:14:42 +02:00

johnsturgeon deleted branch figure-out-if-there-is-a-way-to-include-common-bits-of-the-three-base-jinja-templates-491

2026-09-07 13:14:42 +02:00

johnsturgeon referenced this pull request from a commit

2026-09-07 13:14:44 +02:00

Merge pull request 'Moved common header code into the included partial template' (#493) from figure-out-if-there-is-a-way-to-include-common-bits-of-the-three-base-jinja-templates-491 into main