Fix content_width timing, remove dead code, add transient expiry - #112
Conversation
1. flash_content_width() ran on after_setup_theme priority 0 and called
flash_get_layout(), which depends on is_page()/is_single()/$post -
none of which are reliable before the main query is parsed. Verified
live: a full-width page still got content_width=780 instead of 1200,
because the layout lookup always saw the query in its unparsed
pre-bootstrap state. Moved the hook to template_redirect, where those
conditional tags are guaranteed to work.
2. Removed flash_plugin_version_compare() (inc/extras.php) and
flash_is_woocommerce_pages() (inc/woocommerce.php) - confirmed zero
call sites anywhere in this theme. flash_plugin_version_compare() IS
used in Flash Pro's own inc/admin/tdi-notice.php, so left untouched
there; flash_is_woocommerce_pages() doesn't exist in Pro at all.
Two more confirmed-dead files couldn't be removed in this commit
(sandboxed permission denial on file deletion in this session) -
flagging for manual removal: inc/class-flash-site-library.php (zero
references anywhere in the theme; fetches an external JSON URL and
echoes title/homepage/array keys unescaped - a latent XSS if it were
ever wired in, but it isn't loaded at all today) and
inc/customizer/class-flash-upsell-custom-control.php (0 bytes).
3. set_transient('flash_categories', ...) had no expiry, so it never
expired and effectively became a permanent autoloaded option. Added a
1-day expiry. Its flusher was also only hooked to edit_category and
save_post - neither fires when a category is created or deleted, so
the cached "how many categories" count went stale exactly when
categories were added or removed. Verified live: creating a new
category left the stale count in place before this fix; now
create_category/delete_category flush it immediately.
QA suite — refused, no regression specThis PR changes product source but adds no spec, so the suite was Source files changed with no matching spec |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A moderate legacy-transient issue and cleanup/reference nits remain unresolved.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR fixes layout-dependent content_width, improves category transient invalidation, and removes unused helpers.
Changes:
- Defers content-width calculation until
template_redirect. - Adds transient expiry and category create/delete flushing.
- Removes unused helper code.
| File | Summary |
|---|---|
inc/woocommerce.php |
Removes an unused helper. |
inc/template-tags.php |
Adds transient expiry and invalidation; legacy transient migration remains needed (moderate). |
inc/extras.php |
Removes an unused helper; empty dead files still require removal (nit). |
functions.php |
Updates content-width timing; tracking reference needs correction (nit). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $all_the_cool_cats = count( $all_the_cool_cats ); | ||
|
|
||
| set_transient( 'flash_categories', $all_the_cool_cats ); | ||
| set_transient( 'flash_categories', $all_the_cool_cats, DAY_IN_SECONDS ); |
| // template_redirect, not after_setup_theme: flash_get_layout() relies on | ||
| // is_page()/is_single()/$post, which aren't reliable until the main query | ||
| // has been parsed. | ||
| add_action( 'template_redirect', 'flash_content_width' ); |
|
Same regression a reviewer caught on the Pro sibling PR (flash-pro#124) exists here too, same code: moving the whole Fixed proactively in 4943bc8 the same way: |
Fixes #95
Changes proposed in this Pull Request:
Four independent hygiene items, all verified live rather than taken on the report's word:
content_width ignoring per-page layout - flash_content_width() ran on after_setup_theme priority 0 and called flash_get_layout(), which depends on is_page()/is_single()/$post - none of which are reliable before the main query is parsed. Verified live with a debug probe: a full-width page still got content_width=780 instead of 1200. Moved the hook to template_redirect, where those conditional tags actually work.
Dead code - inc/class-flash-site-library.php and inc/customizer/class-flash-upsell-custom-control.php (0 bytes) have zero references anywhere in the theme, confirmed by grep. flash_plugin_version_compare() and flash_is_woocommerce_pages() are also unused here. Removed the two functions in this PR; the two files could not be deleted in this session (a local tooling restriction on file deletion, unrelated to the theme itself) and are flagged below for manual removal.
Transient without expiry - set_transient('flash_categories', ...) had no expiry, so it never expired. Added one. Its flusher was also only hooked to edit_category/save_post - neither fires on category create or delete - so the cached category count went stale exactly when categories were added or removed. Verified live: creating a category used to leave the stale cached count in place; now create_category/delete_category flush it immediately.
Files still needing manual deletion (grep-confirmed zero references, listed in the commit message too):
Note: item B from the issue (undefined $end in flash_remove_more_jump_link()) is Pro-only per the report - fixed over there: themegrill/flash-pro#124
How to test the changes in this Pull Request:
Types of changes:
Other information:
Changelog entry
Fix - content_width now reflects the actual page layout; added expiry and create/delete flushing to the category-count transient; removed unused code.