Control the flow based on fse theme or not, to avoid accidently using…#38
Control the flow based on fse theme or not, to avoid accidently using…#38
Conversation
… event archives on non event archives
Walkthrough
Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
src/classes/class-se-template-loader.php (2)
58-71: This condition hijacks all archives; use the proper taxonomy check.is_archive() ignores its argument and returns true for any archive type, causing this branch to run on unrelated archives and return an events template. This is the likely source of “accidentally using event archives on non-event archives.”
Apply this diff:
- if ( is_archive( 'se-event-category' ) ) { + if ( is_tax( 'se-event-category' ) ) { $theme_templates = array( 'archive-se-event.php', ); // Determine if any of these templates is available in the theme. $archive_event_template = get_query_template( 'archive', $theme_templates ); if ( $archive_event_template ) { return $archive_event_template; } else { return self::locate_template( array( 'archive.php' ) ); } }
108-120: Use is_post_type_archive for the se-event-date archive branch.Same issue as above: is_archive('se-event-date') doesn’t target the intended CPT archive and can run on any archive view.
Apply this diff:
- if ( is_archive( 'se-event-date' ) ) { + if ( is_post_type_archive( 'se-event-date' ) ) { $theme_templates = array( 'archive-se-event.php', ); // Determine if any of these templates is available in the theme. $archive_event_template = get_query_template( 'archive', $theme_templates ); if ( $archive_event_template ) { return $archive_event_template; } $fallback_template = 'archive.php'; }
🧹 Nitpick comments (2)
src/template-functions.php (2)
640-649: Add explicit index fallbacks to both hierarchies.If neither archive-se-event.* nor archive.* exists, having index.* as a final fallback is safer and mirrors WP’s standard hierarchy.
Apply this diff:
- $custom_hierarchy_fse = array( + $custom_hierarchy_fse = array( 'archive-se-event.html', - 'archive.html', + 'archive.html', + 'index.html', ); - $custom_hierarchy_legacy = array( + $custom_hierarchy_legacy = array( 'archive-se-event.php', - 'archive.php', + 'archive.php', + 'index.php', );
637-649: Prefer is_post_type_archive check over reading post_type query var.Using the conditional API is more robust than get_query_var('post_type'), especially with complex queries or rewritten URLs.
Outside the selected range, consider:
if ( is_post_type_archive( 'se-event-date' ) ) { // existing hierarchy logic... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/classes/class-se-template-loader.php(1 hunks)src/template-functions.php(1 hunks)
🔇 Additional comments (1)
src/template-functions.php (1)
640-649: Good separation of FSE vs legacy hierarchies.Choosing between .html and .php hierarchies based on wp_is_block_theme() is the right direction and aligns with WP’s template resolution paths.
| if ( wp_is_block_theme() && str_contains( $template, 'template-canvas.php' ) ) { | ||
| return $template; | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid PHP 8-only str_contains and broaden the FSE guard to fully skip PHP template logic.
- str_contains requires PHP 8; use strpos for broader compatibility.
- Per the PR summary (“ignore PHP template logic when an FSE theme is active”), short-circuiting on any block theme is safer than only when template-canvas.php is present. The current guard is too narrow and may still hijack non-event archives via downstream logic.
Two options:
- Option A (recommended): Bail early for all block themes.
- if ( wp_is_block_theme() && str_contains( $template, 'template-canvas.php' ) ) {
- return $template;
- }
+ if ( wp_is_block_theme() ) {
+ return $template;
+ }- Option B (minimal): Keep the specific guard but switch to strpos for PHP 7.4 compatibility.
- if ( wp_is_block_theme() && str_contains( $template, 'template-canvas.php' ) ) {
+ if ( wp_is_block_theme() && false !== strpos( $template, 'template-canvas.php' ) ) {
return $template;
}Note: If you keep Option B, consider making the guard broader (additional known PHP-based canvas names) or adding a filter to let sites opt-out of the loader entirely on block themes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( wp_is_block_theme() && str_contains( $template, 'template-canvas.php' ) ) { | |
| return $template; | |
| } | |
| if ( wp_is_block_theme() ) { | |
| return $template; | |
| } |
🤖 Prompt for AI Agents
In src/classes/class-se-template-loader.php around lines 33 to 36, the current
guard uses PHP 8-only str_contains and only short-circuits when
'template-canvas.php' is present; update this to either (recommended)
immediately return $template whenever wp_is_block_theme() is true to fully skip
any PHP template logic on FSE/block themes, or (minimal) replace str_contains
with a PHP 7.4-compatible strpos check (e.g. strpos($template,
'template-canvas.php') !== false) and consider expanding the needle list or
adding a filter for opt-out; implement the chosen change so the loader bails out
earlier and avoids hijacking non-event archives on block themes.
| $custom_hierarchy_fse = array( | ||
| 'archive-se-event.html', | ||
| 'archive.html', | ||
| ); | ||
|
|
||
| return $custom_hierarchy; | ||
| $custom_hierarchy_legacy = array( | ||
| 'archive-se-event.php', | ||
| 'archive.php', | ||
| ); | ||
| return wp_is_block_theme() ? $custom_hierarchy_fse : $custom_hierarchy_legacy; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify this function is actually hooked for both PHP and block template hierarchies.
Returning .html entries only helps if the function is tied to the appropriate hierarchy filter(s). Ensure you hook this for archives in both classic and block contexts (e.g., archive_template_hierarchy for PHP; verify whether your block theme path is reached by filters you control).
Run this script to confirm hooks:
🏁 Script executed:
#!/bin/bash
# Find where se_fix_se_events_fse_archive_template is hooked.
rg -n "se_fix_se_events_fse_archive_template" -A 3
# Check for archive hierarchy filters or similar
rg -n "add_filter\([^)]*archive_template_hierarchy" -A 2
# Look for block template related hooks (if any)
rg -n "add_filter\([^)]*block_template" -A 2
# Also check if se_event_archive_template() is hooked anywhere (to avoid conflicting paths).
rg -n "se_event_archive_template\s*\(" -A 2Length of output: 1844
Hook se_fix_se_events_fse_archive_template into the block template hierarchy
You’re only filtering archive_template_hierarchy (classic PHP). To ensure your .html FSE archive templates are picked up in block themes, also register this callback on block_template_hierarchy.
• src/template-hooks.php—add alongside the existing hook:
add_filter( 'archive_template_hierarchy', 'se_fix_se_events_fse_archive_template' );
+add_filter( 'block_template_hierarchy', 'se_fix_se_events_fse_archive_template', 10, 2 );• src/template-functions.php—accept the extra parameter when hooked into block hierarchy:
- function se_fix_se_events_fse_archive_template( $templates ) {
+ function se_fix_se_events_fse_archive_template( $templates, $type = '' ) {
if ( 'se-event-date' === get_query_var( 'post_type' ) ) {
$custom_hierarchy_fse = array(
'archive-se-event.html',
'archive.html',
);
$custom_hierarchy_legacy = array(
'archive-se-event.php',
'archive.php',
);
return wp_is_block_theme() ? $custom_hierarchy_fse : $custom_hierarchy_legacy;
}
}🤖 Prompt for AI Agents
In src/template-functions.php around lines 640 to 649, the function returns
different hierarchies for block themes (.html) vs classic themes but is only
hooked into the classic PHP filter; update the hook registration in
src/template-hooks.php to also add this callback to the block_template_hierarchy
filter, and modify the function signature (or add handling) so it accepts the
extra parameter passed by block_template_hierarchy (typically $templates, $type
or similar) when used for block themes; ensure the function still returns the
same arrays but can accept and ignore the extra argument when invoked via
block_template_hierarchy.
1 participant
… event archives on non event archives
Changes proposed in this Pull Request
Testing instructions
Mentions #
Summary by CodeRabbit