Issue schema search console #638
Conversation
WalkthroughAdds a public Changes
Sequence Diagram(s)sequenceDiagram
participant Template as Event Template
participant Builder as Event_Builder
participant Helpers as Schema Helpers
participant DB as Calendar DB
Template->>Builder: parse_event_template_tags(template, event)
Builder->>Builder: run shortcodes first
alt template contains [schema-meta]
Builder->>Helpers: process_event_content(tag="schema-meta", event)
Helpers-->>Builder: hidden schema.org properties markup
Builder->>Template: inject markup at tag location
else template lacks [schema-meta]
Builder->>Helpers: get_missing_schema_properties(event)
Helpers-->>Builder: generated hidden schema.org properties
Builder->>Template: append generated markup to end
end
Note right of DB: maintenance function
DB->>DB: simcal_update_calendar_templates_for_schema()\n(scan calendars, append `[schema-meta]` where missing)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
includes/events/event-builder.php (3)
249-253: Auto-append schema: add an opt-out filter to avoid forced injection.Consider a filter to allow themes/sites to disable auto-append when they intentionally control schema in templates.
Apply this minimal change:
- if (strpos($template_tags, '[schema-meta]') === false) { - $result .= $this->get_missing_schema_properties($this->event); - } + if (strpos($template_tags, '[schema-meta]') === false + && apply_filters('simcal_append_schema_meta', true, $template_tags, $this->event, $this->calendar)) { + $result .= $this->get_missing_schema_properties($this->event); + }
1204-1207: Support MixedEventAttendanceMode when both venue and URL exist.Derive mode from both address and link to better reflect hybrid events.
- // Add eventAttendanceMode (default to OfflineEventAttendanceMode for physical events) - $attendance_mode = !empty($event->start_location['address']) ? 'OfflineEventAttendanceMode' : 'OnlineEventAttendanceMode'; + // Add eventAttendanceMode with Mixed support when both venue and URL exist. + $has_location = !empty($event->start_location['address']) || !empty($event->end_location['address']); + $has_url = !empty($event->link); + $attendance_mode = $has_location && $has_url + ? 'MixedEventAttendanceMode' + : ($has_location ? 'OfflineEventAttendanceMode' : 'OnlineEventAttendanceMode'); $schema_meta .= '<meta itemprop="eventAttendanceMode" content="https://schema.org/' . esc_attr($attendance_mode) . '" />';
1258-1277: Use event image first; parameter currently unused.Prefer event-specific cover image before site logo/icon; also resolves the “unused parameter $event” static hint.
private function get_event_image_url(Event $event) { + // Prefer event-specific image when available. + if (method_exists($event, 'get_cover_image')) { + $cover = $event->get_cover_image(); + // Accept array['url'] or HTML string with <img src="..."> + if (is_array($cover) && !empty($cover['url'])) { + return $cover['url']; + } + if (is_string($cover) && preg_match('~src=["\']([^"\']+)~', $cover, $m)) { + return $m[1]; + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/events/event-builder.php(4 hunks)includes/functions/shared.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
includes/events/event-builder.php (1)
includes/events/event.php (1)
Event(20-647)
includes/functions/shared.php (1)
includes/update.php (1)
get_posts(165-181)
🪛 GitHub Actions: build
includes/events/event-builder.php
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
includes/functions/shared.php
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🪛 PHPMD (2.15.0)
includes/events/event-builder.php
1258-1258: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (2)
includes/events/event-builder.php (1)
422-424: New [schema-meta] tag handler looks good.Straightforward passthrough to the generator.
includes/functions/shared.php (1)
296-296: Default template + [schema-meta]Good default; aligns with the new tag handler and avoids double-appending.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
includes/functions/shared.php (1)
301-339: Apply capability check, error handling, and cache invalidation as suggested in previous review.The function currently updates calendar posts without verifying edit permissions, checking for errors, or invalidating caches. The previous review comment provides a comprehensive diff addressing all three concerns that should be applied.
includes/events/event-builder.php (4)
1195-1202: Apply the endDate fallback improvement from previous review.The current +1 hour fallback doesn't account for whole-day events. The previous review provides a proper solution using
start_dtandendOfDayfor all-day events.
1214-1222: Apply the filterable offers pattern from previous review.Hardcoded free pricing and USD currency can misrepresent paid or non-USD events. The previous review provides a proper solution using
apply_filtersto make this optional.
1256-1262: Use get_organizer() method as suggested in previous review.Direct access to
$event->organizermay miss organizer data. The previous review correctly identifies that theget_organizer()method should be used instead.
1315-1322: Use get_organizer() method as suggested in previous review.Direct property access to
$event->organizerbypasses the proper accessor. The previous review correctly identifies this should useget_organizer().
🧹 Nitpick comments (2)
includes/events/event-builder.php (2)
1204-1209: Consider making attendance mode determination filterable.While the current logic (offline for physical locations, online otherwise) is reasonable, a filter would allow customization for hybrid events or special cases.
// Add eventAttendanceMode (default to OfflineEventAttendanceMode for physical events) $attendance_mode = !empty($event->start_location['address']) ? 'OfflineEventAttendanceMode' : 'OnlineEventAttendanceMode'; +$attendance_mode = apply_filters('simcal_event_attendance_mode', $attendance_mode, $event); $schema_meta .= '<meta itemprop="eventAttendanceMode" content="https://schema.org/' . esc_attr($attendance_mode) . '" />';
1272-1301: Unused$eventparameter - consider leveraging event-specific images.The method currently only checks theme logo and site icon. Consider using event attachments or cover image before falling back to site-wide defaults.
private function get_event_image_url(Event $event) { + // Try event-specific images first + $attachments = $event->get_attachments(); + if (!empty($attachments)) { + foreach ($attachments as $attachment) { + if (!empty($attachment['url']) && preg_match('/\.(jpg|jpeg|png|gif|webp)$/i', $attachment['url'])) { + return $attachment['url']; + } + } + } + // Try to get custom logo first $custom_logo_id = get_theme_mod('custom_logo');Based on static analysis
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/events/event-builder.php(4 hunks)includes/functions/shared.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
includes/events/event-builder.php (1)
includes/events/event.php (1)
Event(20-647)
includes/functions/shared.php (1)
includes/update.php (1)
get_posts(165-181)
🪛 PHPMD (2.15.0)
includes/events/event-builder.php
1282-1282: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (4)
includes/functions/shared.php (1)
296-296: LGTM! Schema-meta tag properly added to default template.The addition integrates cleanly with the existing template structure and aligns with the PR's objective to provide schema.org metadata by default.
includes/events/event-builder.php (3)
165-166: LGTM! Schema-meta tag properly declared.The tag is correctly added to the Meta Tags group with clear documentation.
249-252: Good fallback ensures schema properties are included even if tag is omitted.The check correctly uses strict equality (
=== false) to distinguish between "not found" and "found at position 0", ensuring schema properties are appended when the tag is absent from templates.
422-423: LGTM! Tag processing correctly delegates to helper method.The case statement cleanly handles the schema-meta tag by calling the appropriate helper function.
includes/events/event-builder.php
Outdated
| // Add event URL | ||
| if (!empty($event->link)) { | ||
| $schema_meta .= '<meta itemprop="url" content="' . esc_url($event->link) . '" />'; | ||
| } else { | ||
| // Fallback to calendar/page URL or current page URL | ||
| $fallback_url = ''; | ||
| if (!empty($event->calendar) && is_numeric($event->calendar)) { | ||
| $fallback_url = get_permalink($event->calendar); | ||
| } | ||
| if (empty($fallback_url)) { | ||
| global $wp; | ||
| $fallback_url = home_url($wp->request); | ||
| } | ||
| if (!empty($fallback_url)) { | ||
| $schema_meta .= '<meta itemprop="url" content="' . esc_url($fallback_url) . '" />'; | ||
| } | ||
| } |
There was a problem hiding this comment.
URL fallback may miss query parameters for filtered/paginated views.
The fallback uses home_url($wp->request) which excludes query strings. For calendar views with filters or pagination, this could generate incomplete URLs.
Consider including query parameters in the fallback:
if (empty($fallback_url)) {
global $wp;
- $fallback_url = home_url($wp->request);
+ $fallback_url = add_query_arg(array(), home_url($wp->request));
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In includes/events/event-builder.php around lines 1224 to 1240, the URL fallback
uses home_url($wp->request) which drops query parameters (breaking
filtered/paginated calendar views); modify the fallback so it preserves the
current query string (e.g. build the URL from home_url($wp->request) plus the
current query args or use add_query_arg($_GET, home_url($wp->request))) or, when
available, reconstruct from $_SERVER['QUERY_STRING'], then pass the resulting
URL through esc_url() before appending the <meta itemprop="url"> output.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
includes/events/event-builder.php (5)
1203-1211: Already flagged: endDate fallback logic issues.This issue was previously identified in past review comments. The current implementation:
- Uses arbitrary +1 hour fallback
- Doesn't account for whole-day events
- Recreates Carbon from timestamp instead of using
$event->start_dtPlease refer to the existing review comment for the suggested fix.
1280-1285: Already flagged: Use get_organizer() method instead of direct property access.This issue was previously identified in past review comments. The code checks
$event->organizerdirectly, which may miss organizer data exposed via theget_organizer()method.Please refer to the existing review comment for the suggested fix.
1222-1247: Already flagged: URL fallback drops query parameters.This issue was previously identified in past review comments. The fallback at line 1235 uses
home_url($wp->request)which excludes query strings, breaking filtered or paginated calendar views.Please refer to the existing review comment for the suggested fix.
1338-1345: Already flagged: Use get_organizer() accessor instead of direct property.This issue was previously identified in past review comments. Direct access to
$event->organizermay miss data exposed through theget_organizer()method.Please refer to the existing review comment for the suggested fix.
1249-1263: Already flagged: Hardcoded offers risk incorrect structured data.This issue was previously identified in past review comments. The hardcoded free/USD/InStock values may be incorrect for paid or non-USD events.
Additionally, the URL handling in the offers block (lines 1252-1256) is redundant:
Apply this diff to simplify:
// Add offers (default to free event) $schema_meta .= '<div itemprop="offers" itemscope itemtype="https://schema.org/Offer">'; $schema_meta .= '<meta itemprop="price" content="0" />'; -if (!empty($event_url)) { - $schema_meta .= '<meta itemprop="url" content="' . esc_url($event_url) . '" />'; -}else{ - $schema_meta .= '<meta itemprop="url" content="' . esc_url(home_url()) . '" />'; -} +$offer_url = !empty($event_url) ? $event_url : home_url(); +$schema_meta .= '<meta itemprop="url" content="' . esc_url($offer_url) . '" />'; $schema_meta .= '<meta itemprop="priceCurrency" content="USD" />';
🧹 Nitpick comments (1)
includes/events/event-builder.php (1)
1189-1293: Consider adding filters for customization and validation.The method generates schema properties without providing hooks for customization or validating the event parameter.
Consider these improvements:
- Add a filter to allow customization of all schema properties:
$schema_meta = apply_filters('simcal_event_schema_properties', $schema_meta, $event);
- Add type validation at the method start:
if (!$event instanceof Event) { return ''; }
- Consider breaking this large method into smaller, focused methods for each schema property type (e.g.,
get_schema_offers(),get_schema_attendance_mode(), etc.) to improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/events/event-builder.php(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
includes/events/event-builder.php (1)
includes/events/event.php (1)
Event(20-647)
🪛 GitHub Actions: build
includes/events/event-builder.php
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🪛 PHPMD (2.15.0)
includes/events/event-builder.php
1305-1305: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (4)
includes/events/event-builder.php (4)
165-166: LGTM! Well-documented public tag.The 'schema-meta' tag is correctly added to the Meta Tags section with a clear description.
366-366: LGTM! Correctly marks the event URL for schema.org.The additional
trueparameter appropriately addsitemprop="url"to the event link.
425-426: LGTM! Case handles the schema-meta tag correctly.The implementation follows the established pattern for processing meta tags.
945-996: LGTM! Schema.org URL markup correctly implemented.The new
$is_event_urlparameter and conditionalitemprop="url"addition follow WordPress and schema.org best practices. The trailing space in the itemprop string ensures proper HTML formatting.
| /** | ||
| * Get event image URL with fallbacks. | ||
| * | ||
| * @since 3.0.0 | ||
| * @access private | ||
| * | ||
| * @param Event $event | ||
| * | ||
| * @return string|false | ||
| */ | ||
| private function get_event_image_url(Event $event) | ||
| { | ||
| // Try to get custom logo first | ||
| $custom_logo_id = get_theme_mod('custom_logo'); | ||
| if ($custom_logo_id) { | ||
| $logo_url = wp_get_attachment_image_url($custom_logo_id, 'full'); | ||
| if ($logo_url) { | ||
| return $logo_url; | ||
| } | ||
| } | ||
|
|
||
| // Fallback to site icon | ||
| $site_icon_url = get_site_icon_url(); | ||
| if ($site_icon_url) { | ||
| return $site_icon_url; | ||
| } | ||
|
|
||
| // Fallback to a default image URL (you can customize this) | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Unused parameter and missing event-specific image sources.
The $event parameter is unused (flagged by PHPMD), and the method doesn't attempt to use event-specific images like attachments or cover images before falling back to site-level defaults.
Consider this enhancement to utilize event-specific images:
private function get_event_image_url(Event $event)
{
+ // Try event's cover image first
+ $cover_image = $event->get_cover_image();
+ if (!empty($cover_image)) {
+ // Extract URL from HTML if needed
+ if (preg_match('/src=["\']([^"\']+)["\']/', $cover_image, $matches)) {
+ return $matches[1];
+ }
+ }
+
+ // Try event attachments for images
+ $attachments = $event->get_attachments();
+ if (!empty($attachments)) {
+ foreach ($attachments as $attachment) {
+ if (!empty($attachment['icon']) && preg_match('/\.(jpg|jpeg|png|gif|webp)$/i', $attachment['icon'])) {
+ return $attachment['icon'];
+ }
+ }
+ }
+
// Try to get custom logo first
$custom_logo_id = get_theme_mod('custom_logo');This would make the $event parameter meaningful and provide more relevant images for schema markup.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 PHPMD (2.15.0)
1305-1305: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
includes/events/event-builder.php (1)
1204-1216: Fix endDate fallback to use existingstart_dtand respect whole-day events.The current code at lines 1210-1215:
- Unnecessarily recreates a Carbon instance from
$event->starttimestamp when$event->start_dtalready exists- Always adds 1 hour regardless of event type, which misrepresents whole-day events
Both
start_dtandwhole_dayproperties are used elsewhere in the file (lines 447, 504, 744, 535, 754). Replace with:} else { - // If no end date, use start date + 1 hour as fallback - $start = Carbon::createFromTimestamp($event->start, $event->timezone); - $end_date = $start->copy()->addHour(); + // Use existing start_dt and handle whole-day events appropriately + $start_dt = $event->start_dt->copy(); + $end_date = $event->whole_day ? $start_dt->endOfDay() : $start_dt->addHour(); $end_iso = $end_date->toIso8601String(); $schema_meta .= '<meta itemprop="endDate" content="' . esc_attr($end_iso) . '" />'; }
🧹 Nitpick comments (1)
includes/events/event-builder.php (1)
1305-1334: Enhance event-specific image resolution before site-level fallbacks.The
$eventparameter is currently unused (flagged by PHPMD), and the method doesn't attempt to use event-specific images. Based on the Event class interface,get_cover_image()andget_attachments()are available and should be checked before falling back to site-level defaults.private function get_event_image_url(Event $event) { + // Try event's cover image first + $cover_image = $event->get_cover_image(); + if (!empty($cover_image)) { + // Extract URL from HTML if needed + if (preg_match('/src=["\']([^"\']+)["\']/', $cover_image, $matches)) { + return $matches[1]; + } + } + + // Try event attachments for images + $attachments = $event->get_attachments(); + if (!empty($attachments)) { + foreach ($attachments as $attachment) { + if (!empty($attachment['icon']) && preg_match('/\.(jpg|jpeg|png|gif|webp)$/i', $attachment['icon'])) { + return $attachment['icon']; + } + } + } + // Try to get custom logo first $custom_logo_id = get_theme_mod('custom_logo');This would make the
$eventparameter meaningful and provide more relevant images for schema markup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/events/event-builder.php(10 hunks)includes/functions/shared.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/functions/shared.php
🧰 Additional context used
🧬 Code graph analysis (1)
includes/events/event-builder.php (1)
includes/events/event.php (2)
Event(20-647)get_organizer(643-646)
🪛 PHPMD (2.15.0)
includes/events/event-builder.php
1315-1315: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (6)
includes/events/event-builder.php (6)
165-166: LGTM! Schema-meta tag properly registered.The tag is correctly added to the public meta tags list with clear documentation.
249-260: LGTM! Improved schema property detection.The logic correctly processes shortcodes first and checks the actual output for schema properties rather than the template tag itself. This properly handles cases where conditional tags might remove
[schema-meta]during processing.
342-342: LGTM! Schema.org enhancements properly implemented.The changes correctly:
- Upgrade itemtype URLs to HTTPS for Place and Person entities
- Add
itemprop="url"marking for main event URLs via the new$is_event_urlparameter- Handle the schema-meta tag case to return missing properties
These improvements enhance schema.org compliance and help search engines identify event URLs.
Also applies to: 367-367, 426-427, 946-990, 1083-1083, 1185-1185
1255-1272: LGTM! Offers now properly filterable.The implementation correctly makes offers opt-in via
simcal_emit_default_offersfilter (defaulting to false), preventing incorrect pricing/currency data in structured markup. This addresses the previous concern about hardcoded "free, USD, InStock" values.
1280-1286: LGTM! Performer, organizer, and description fallbacks properly implemented.The helper methods correctly:
- Use
get_organizer()accessor to retrieve organizer data (Lines 1288-1295, 1349-1353)- Provide appropriate fallbacks (site name for performer/organizer, title for description)
- Check method existence before calling to avoid errors
Also applies to: 1288-1295, 1297-1300, 1346-1358
1228-1253: Review comment is based on incorrect assumptions about calendar filtering mechanism.The codebase does not use query parameters for calendar filtering. After comprehensive verification:
- No evidence of
?month=,?year=, or filter query parameters used anywhere in the codebase- Calendar navigation/filtering uses AJAX endpoints (
wp_ajax_simcal_default_calendar_draw_list,wp_ajax_simcal_default_calendar_draw_grid), not URL query strings- The URL at lines 1228-1253 is for schema.org Event structured data metadata, which should be canonical/stable, not include filtered view context
home_url($wp->request)without query parameters is the correct approach for schema metadataThe suggested fix (
add_query_arg($_GET, ...)) would actually break schema validation by creating dynamic URLs in structured data, which is incorrect for schema.org purposes.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Bug Fixes