Uh oh!
There was an error while loading. Please reload this page.
feat(theme): Jinja theming engine with bundled themes - #322
Draft
Rl0007 wants to merge 29 commits into
Draft
Conversation
Ports the frappe_themes engine (page renderer, Jinja theme loader, theme inheritance chain, route table) into buzz/theme/. DocTypes are renamed to Buzz Theme / Buzz Theme Settings / Buzz Themed Route because DocType names are globally unique in Frappe - a module does not namespace them - so the old names would clash wherever frappe_themes is also installed.
Theme resolution becomes: ?preview_theme= (dev) -> Buzz Event.theme on event-scoped routes -> Buzz Settings.default_theme -> nothing. can_render() now matches the route before resolving the theme, because the event is only known once the event_route group has been captured. The resolved context is stashed on frappe.local so theme_asset_url() and theme_config() cannot diverge from it and serve another theme's assets. Drops is_active and the active_theme.json manifest: Buzz Settings is the single source of truth for the site default, and the manifest's only reader was the Vite plugin, which is not part of this port.
Moves buzz_events_theme, sketchbook_theme and events_theme into buzz, with the public/themes symlinks that make their assets servable. The route table previously existed only in one dev site's database, so the engine shipped inert; a patch now seeds it idempotently. importable_doctypes is what makes the theme records themselves import on migrate - the exported folder is otherwise only an export target, never an import source. Theme scripts join the existing vendored-JS exclusions: they are Alpine browser globals and minified vendor bundles, like buzz/public/js/lib.
Jinja methods are registered in one global namespace keyed by function name, so buzz's theme_asset_url silently lost to the identically-named helper in frappe_themes wherever both apps are installed. Every themed page rendered with empty asset URLs. Renaming to buzz_theme_asset_url/buzz_theme_config gives the doctype rename the co-installability it was chosen for. The not-found guard was an output node in a template that extends another, and Jinja discards those - only statements run. The throw never fired, so a missing event fell through to get_cached_doc(..., None), which raised DoesNotExistError carrying a doctype and got converted to 403 Not Permitted for guests instead of 404.
Ports the sticker-pack prototype onto the live theme engine: dot-grid ground, 3px borders with offset shadows, accent-cycled rotated cards and date stickers. The prototype's accent_bg/accent_fg/rot_class were Python globals registered in its render.py, which Frappe's Jinja has no equivalent of, so they are reimplemented as macros in components/macros/accents.html. Tailwind is built locally rather than pulled from the Play CDN. Events Theme is removed: it shipped two pages that extend a base.html it did not have, so it could never render standalone.
Drops Buzz Event.theme. Theming an individual event let a listing page and the events it linked to render in different design languages, so clicking a card visibly changed the design mid-journey. Hosted event platforms attach the theme to the organizer or workspace for this reason and let an event vary only its content, never its templates. With nothing request-dependent left to resolve, can_render() checks the theme first and returns early when none is set, and the frappe.local hand-off that kept the Jinja helpers in step with the renderer is no longer needed - both now read the same cached lookup.
Removing a field drops its DocField but never the column, so the stale values survived every migrate. Also invalidates the cached column list the DDL does not touch - otherwise running workers keep building SQL for a column the table no longer has.
…ndling Split bundled themes into private templates (buzz/themes/<slug>) and public assets (buzz/public/themes/<slug>), replacing the committed symlinks that exposed every page/component template, its Jinja queries and any sibling .py page controller at /assets/buzz/themes/<slug>/... Also: - reject theme names outside ^[A-Za-z0-9 _-]+$ and containment-check every path before rename/rmtree/makedirs; frappe.scrub keeps path separators - never remove a standard theme's shipped folders on record delete - gate after_insert scaffolding on developer_mode so a read-only apps volume cannot break the insert - make scaffold_theme_settings POST-only and check write permission - refuse reserved first path segments for dynamic pages so a theme cannot shadow login/desk/api - stop the seed patch re-enabling dynamic_pages_enabled on every re-run - batch the sponsor tier lookup on the event detail page - log boot-data failures, memoize compiled routes per request, drop dead code
Module Def names are globally unique the same way DocType names are, and 'Theme' was too generic to own. The pre-model-sync patch creates the new module and repoints the doctypes before their JSONs sync, because DocType.module is a Link and the sync would otherwise fail to import them.
Covers the regressions this engine has already had: asset URLs resolving to the wrong theme (which fails as a broken stylesheet, not an error), path traversal, on_trash deleting shipped app source, and reserved paths being hijacked by a theme page. Also renames the asset fallback from base_ to active_: names is child-first, so the fallback URL is the active theme, not the root ancestor.
Contributor
Too many files changed for review (111 files, 100 file limit). Bypass the limit by tagging |
Rl0007
marked this pull request as draft
August 8, 2026 10:45
The three bundled themes each shipped a byte-identical copy of lucide.min.js and alpine.min.js, plus a shared theme.js/components.js pair duplicated between sketchbook and stickerpack. buzz_theme_asset_url() already walks the parent_theme chain child-first and serves the first theme that has the file, so a new asset-only "Buzz Base Theme" parent removes the duplicates with no engine change. buzz_events_theme keeps its own theme.js/components.js, which genuinely differ, and still overrides the parent. The three per-theme package.json/yarn.lock become one at buzz/themes/, so there is a single node_modules instead of three. Each build keeps its own working directory so Tailwind's automatic source detection stays scoped to one theme. The separate lockfiles had drifted apart (4.3.0/4.3.0/4.3.3); consolidating normalises all three on 4.3.3, which only shifts Tailwind's own preflight defaults — the emitted utility class set is unchanged for every theme.
The template reader reaches open() only through find_theme_file or the loader's own containment check, so the flagged path is already proven to sit inside a theme folder; annotated with the reasoning semgrep asks for. frappe-ui beta.37 declares Combobox's emit as (...args: unknown[]), which a narrowly typed handler is not assignable to, so PhoneInput narrows at the boundary instead.
Sketchbook's footer row was a non-wrapping flex row, so it pushed the page 36px wider than a 390px viewport. Stickerpack's theme toggle is fixed to the top-right, which sits in the gutter beside the centred shell on desktop but lands on top of the header's sign-in link once the gutter is gone.
staticmap.openstreetmap.de is NXDOMAIN - the service was withdrawn - so every venue with coordinates rendered a broken image in all three themes. The official openstreetmap.org embed needs a bounding box rather than a centre point, derived here from the venue's latitude and longitude.
Every bundled theme hardcoded one customer's identity (brand text, og tags, mailto, social links), so any site installing buzz rendered someone else's name. Themed pages now get frappe's own website context and read the brand, og image, footer links, copyright and address from Website Settings, falling back cleanly when a field is unset. Also wires up the per-theme settings mechanism that already existed but was unused: Stickerpack Theme Settings drives the homepage hero and featured heading through buzz_theme_config().
Ports the site's frappe/builder home, event and category pages into the Jinja theme system as a fourth bundled theme, with light and dark modes. Colours are frappe-ui's own semantic tokens (surface / ink / outline), resolved from frappe-ui/tailwind/generated/colors.json, so themed pages and the Vue dashboard share one grey ramp. Adds Buzz Theme Nav Link so header links are editable from Desk; its validate() allowlists http/https because the URL is rendered straight into href. Component CSS is wrapped in @layer components: tokens.css is imported unlayered, where it would otherwise outrank every Tailwind utility regardless of specificity.
Frappe writes every patch into Patch Log as applied WITHOUT running it when a site is installed, so a fresh site came up with the theme records but zero themed routes, and the log entry then blocked the patch from ever re-running. Only /home resolved, by accident, through dynamic pages. Moves the route list and seeding into buzz_theme_settings as DEFAULT_ROUTES and seed_default_routes(), called from after_install, on_migrate and the patch alike. Seeding is idempotent on url_pattern, so custom routes survive. Drops the dead ^events$ default: no bundled theme ships pages/events.html.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renders themes with jinja autoescape on: frappe's shared env has it off, which
made every {{ }} in a theme an xss sink for fields frappe does not sanitize on
save. Deliberate html now says |safe.
- reject dot-dot and non-word segments in dynamic page paths, so an encoded
..%2f can no longer walk out of pages/ and render any partial
- inherit requires_auth from the route that owns a template, so a gated page
is not reachable by its file path once dynamic pages are on
- never render venue.google_maps_embed_code or category.icon_svg: both are Code
fields, which frappe deliberately leaves unsanitized. maps validate the embed
url, categories name an icon
- keep route captures out of form_dict, let website settings failures raise
rather than log per request, refresh the cached jinja overlay's globals
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a site-wide Jinja theming engine for the public event pages, ported in from the standalone
frappe_themes+buzz_themesapps so buzz stands alone.Buzz Settings.default_theme. Empty means the renderer declines and nothing changes — the engine is inert until a theme is chosen.DocTypes and the module are
Buzz *-prefixed because both names are globally unique in Frappe and would otherwise clash wherever the old apps are co-installed.Theme templates are private; only built CSS and scripts are published under
public/themes/. An earlier symlink layout served the raw Jinja — data model, filters and all — at/assets/.