Conversation
…tion process to the new format
Created the new post type to handle dates and started the basic migra…
pulled in changes where meta is editable and hideen from editor meta …
…an up, but its mostly there
Feature/track event dates
…etup Date rendering and grouping added
…cluding reverting timezones
…in-js Feaure/remove old meta date calls in js
Feature/hook up next previous support
…previous. Also patched the issue where you cant change the timezone correctly
…d-from-feed You can exclude dates form the feed, so are hidden from the next and …
…locks to work wiht the new setups too
Feature/remove old meta date useage
Added in the migration process and clean up all orphaned dates
… not refleting in the editor, deal with live versions sometimes using unsaved attributes in view. These should only be used for previews
Feature/v2 fix linter jul3
did some suggestion from AI Rabbit
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/event-functions.php (1)
349-390: Complete the array key fixes for backward compatibilityAdditional array key inconsistencies in the deprecated function:
-$opening_li = $date['id'] === $event_date_id ? '<li class="active" style="text-decoration: underline;">' : '<li>'; +$opening_li = ( isset( $date['id'] ) && $date['id'] === $event_date_id ) ? '<li class="active" style="text-decoration: underline;">' : '<li>'; // Check if start and end times are on the same day. -$same_day = wp_date( 'Y-m-d', $date['start_date'], $timezone ) === wp_date( 'Y-m-d', $date['end_date'], $timezone ); +$same_day = wp_date( 'Y-m-d', $date['datetime_start'], $timezone ) === wp_date( 'Y-m-d', $date['datetime_end'], $timezone ); // Get start and end times. -$time_start = ( $hide_start_time ) ? '' : wp_date( get_option( 'time_format' ), $date['start_date'], $timezone ); -$time_end = ( $hide_end_time ) ? '' : wp_date( get_option( 'time_format' ), $date['end_date'], $timezone ); +$time_start = ( $hide_start_time ) ? '' : wp_date( get_option( 'time_format' ), $date['datetime_start'], $timezone ); +$time_end = ( $hide_end_time ) ? '' : wp_date( get_option( 'time_format' ), $date['datetime_end'], $timezone ); // Start the output string. -$single_date_output = wp_date( get_option( 'date_format' ), $date['start_date'], $timezone ); +$single_date_output = wp_date( get_option( 'date_format' ), $date['datetime_start'], $timezone ); // Return early if we only want the date. if ( $date_only ) { - $end_date = wp_date( get_option( 'date_format' ), $date['end_date'], $timezone ); + $end_date = wp_date( get_option( 'date_format' ), $date['datetime_end'], $timezone ); $single_date_output .= sprintf( ' %s – %s %s', $time_start, - wp_date( get_option( 'date_format' ), $date['end_date'], $timezone ), + wp_date( get_option( 'date_format' ), $date['datetime_end'], $timezone ), $time_end );src/classes/class-se-event-post-type.php (1)
540-633: Remove or clarify the developer comment.Line 626 contains a comment
// what is this?that appears to be a developer note. This should be removed or replaced with a meaningful comment explaining the comparison logic.- 'compare' => '>=', // what is this? + 'compare' => '>=', // Show events that haven't ended yet
♻️ Duplicate comments (6)
src/classes/class-date-display-formatter.php (1)
393-398: Handle null date IDs properly in HTML output.When
$date['id']is null, this results in an empty ID attribute. The ID attribute should be omitted entirely when null.-// If we dont have an id on date, set as null. -if ( ! isset( $date['id'] ) ) { - $date['id'] = null; -} -$item_class = array( 'se-event-date-list-item', $date['id'] === $this->event_date_id ? 'se-event-date-list-item__active' : '' ); -$existing_output .= sprintf( '<li id="se-event-date-list-item-%s" class="%s">%s</li>', $date['id'], implode( ' ', $item_class ), $this->render_single_date( $date ) ); +$date_id = isset( $date['id'] ) ? $date['id'] : null; +$item_class = array( 'se-event-date-list-item' ); +if ( $date_id === $this->event_date_id ) { + $item_class[] = 'se-event-date-list-item__active'; +} +$id_attr = $date_id ? sprintf( ' id="se-event-date-list-item-%s"', esc_attr( $date_id ) ) : ''; +$existing_output .= sprintf( '<li%s class="%s">%s</li>', $id_attr, esc_attr( implode( ' ', $item_class ) ), $this->render_single_date( $date ) );src/event-functions.php (5)
191-196: Review the future date filtering logicThe current logic filters dates where both start AND end are in the future. This would exclude events that have already started but haven't ended yet. Typically, "future events" include ongoing events.
$event_dates = array_filter( $event_dates, function ( $date ) use ( $now ) { - return $date['start_date'] > $now && $date['end_date'] > $now; + return $date['end_date'] > $now; } );
663-711: Improve validation and fix meta key namesThe function has validation issues:
- Only validates
start_datebut notend_date- Uses incorrect meta key names (should be prefixed with
se_event_date_)// Validate the event dates, start date should be a timestamp and end date should be a timestamp. if ( ! is_numeric( $event_dates['start_date'] ) ) { return null; } +if ( ! is_numeric( $event_dates['end_date'] ) ) { + return null; +} // Update the post meta for the event date. update_post_meta( $event_date_id, 'se_event_date_start', esc_attr( $event_dates['start_date'] ) ); update_post_meta( $event_date_id, 'se_event_date_end', esc_attr( $event_dates['end_date'] ) ); -update_post_meta( $event_date_id, 'se_event_all_day', boolval( $event_dates['all_day'] ) ); -update_post_meta( $event_date_id, 'se_event_hide_from_calendar', boolval( $event_dates['hide_from_calendar'] ) ); -update_post_meta( $event_date_id, 'se_event_hide_from_feed', boolval( $event_dates['hide_from_feed'] ) ); +update_post_meta( $event_date_id, 'se_event_date_all_day', boolval( $event_dates['all_day'] ) ); +update_post_meta( $event_date_id, 'se_event_date_hide_from_calendar', boolval( $event_dates['hide_from_calendar'] ) ); +update_post_meta( $event_date_id, 'se_event_date_hide_from_feed', boolval( $event_dates['hide_from_feed'] ) );
746-764: Fix inconsistent meta key usage.The function uses inconsistent meta key names that don't match the registered meta keys in the post type class:
$dates = array_map( function ( $date_id ) { $start_date = get_post_meta( $date_id, 'se_event_date_start', true ); $end_date = get_post_meta( $date_id, 'se_event_date_end', true ); - $all_day = get_post_meta( $date_id, 'se_event_all_day', true ); - $hide_from_calendar = get_post_meta( $date_id, 'se_event_hide_from_calendar', true ); - $hide_from_feed = get_post_meta( $date_id, 'se_event_hide_from_feed', true ); + $all_day = get_post_meta( $date_id, 'se_event_date_all_day', true ); + $hide_from_calendar = get_post_meta( $date_id, 'se_event_date_hide_from_calendar', true ); + $hide_from_feed = get_post_meta( $date_id, 'se_event_date_hide_from_feed', true );
788-795: Remove redundant setTimestamp callThe timestamp is already set on line 790. The second call on line 795 is redundant.
try { $date_time_object = new DateTime( 'now', new DateTimeZone( $timezone ) ); + $date_time_object->setTimestamp( $timestamp ); } catch ( Exception $e ) { $date_time_object = new DateTime(); + $date_time_object->setTimestamp( $timestamp ); // todo handle exception } -return $date_time_object->setTimestamp( $timestamp ); +return $date_time_object;
803-808: Add validation for settings arrayThe
get_optionfunction can returnfalseif the option doesn't exist. Add validation before usingarray_key_exists.function se_event_treat_each_date_as_own_event(): bool { $settings = get_option( 'se_options' ); - return is_array( $settings ) - && array_key_exists( 'treat_each_date_as_own_event', $settings ) - && 'on' === $settings['treat_each_date_as_own_event']; + return is_array( $settings ) + && array_key_exists( 'treat_each_date_as_own_event', $settings ) + && 'on' === $settings['treat_each_date_as_own_event']; }
🧹 Nitpick comments (5)
src/blocks/event-info/components/DateTimeGroup.js (2)
134-136: Use optional chaining for cleaner code.The conditional checks can be simplified using optional chaining.
Apply this diff to use optional chaining:
-if (dateManagerInstance && dateManagerInstance.upsertDate) { - dateManagerInstance.upsertDate(newEventDateTime); -} +dateManagerInstance?.upsertDate?.(newEventDateTime);This same pattern applies to lines 338-340, 357-359, and 376-378.
Also applies to: 338-340, 357-359, 376-378
109-112: Consider extracting the time buffer as a constant.The 1-hour buffer (3600 seconds) is hardcoded in multiple places. Consider extracting it as a constant for better maintainability.
Add a constant at the top of the file:
+const TIME_BUFFER_SECONDS = 3600; // 1 hour buffer between start and end times + const DateTimeGroupNew = ({Then use it in the calculations:
-newEventDateTime.end_date = String( - parseInt(newEventDateTime.start_date) + - 3600 -); +newEventDateTime.end_date = String( + parseInt(newEventDateTime.start_date) + + TIME_BUFFER_SECONDS +);Also applies to: 123-125
src/classes/class-se-block-variations.php (2)
124-124: Fix incomplete comment.The comment is incomplete and should be fixed.
-// Return back the +// Return the parent posts with updated dates
134-139: Optimize timezone handling.The timezone fallback and DateTimeZone object creation can be improved.
-// Get the event timezone. -$timezone = get_post_meta( $parent->ID, 'se_event_timezone', true ); -// use the timezone or default to the site timezone. -$timezone = $timezone ? $timezone : get_option( 'timezone_string' ); - -// Get the date im this format 2025-07-01 13:14:09 -$start_date = wp_date( 'Y-m-d H:i:s', $start_date_ts, new \DateTimeZone( $timezone ) ); -$start_date_gmt = wp_date( 'Y-m-d H:i:s', $start_date_ts, new \DateTimeZone( 'UTC' ) ); +// Get the event timezone with fallback to site timezone +$timezone = get_post_meta( $parent->ID, 'se_event_timezone', true ) ?: get_option( 'timezone_string' ); + +// Create timezone objects once +$tz_obj = new \DateTimeZone( $timezone ); +$utc_obj = new \DateTimeZone( 'UTC' ); + +// Get the date in this format: 2025-07-01 13:14:09 +$start_date = wp_date( 'Y-m-d H:i:s', $start_date_ts, $tz_obj ); +$start_date_gmt = wp_date( 'Y-m-d H:i:s', $start_date_ts, $utc_obj );src/classes/class-date-display-formatter.php (1)
492-492: Fix typo in method documentation.There's a typo in the docblock.
-* Join a string with differing separators a the then +* Join a string with differing separators at the end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/blocks/event-info/components/DateTimeGroup.js(1 hunks)src/classes/class-date-display-formatter.php(1 hunks)src/classes/class-se-block-variations.php(4 hunks)src/classes/class-se-calendar.php(3 hunks)src/classes/class-se-event-dates.php(1 hunks)src/classes/class-se-event-post-type.php(6 hunks)src/event-functions.php(9 hunks)src/templates/content-archive.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/templates/content-archive.php
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-calendar.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-se-block-variations.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-se-event-dates.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-se-event-post-type.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-date-display-formatter.php (3)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: chrisbellboy
PR: a8cteam51/simple-events#13
File: src/classes/class-se-blocks.php:316-316
Timestamp: 2025-06-10T16:47:40.589Z
Learning: In the Simple Events plugin, the external link label meta field `se_event_external_link_label` has a default value of "Tickets" set at the meta registration level. If an empty string reaches the `se_event_external_link_text` filter, it represents an intentional user choice (e.g., to show only an icon without text), so no fallback default should be applied in the filter.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/event-functions.php (3)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: chrisbellboy
PR: a8cteam51/simple-events#13
File: src/classes/class-se-blocks.php:316-316
Timestamp: 2025-06-10T16:47:40.589Z
Learning: In the Simple Events plugin, the external link label meta field `se_event_external_link_label` has a default value of "Tickets" set at the meta registration level. If an empty string reaches the `se_event_external_link_text` filter, it represents an intentional user choice (e.g., to show only an icon without text), so no fallback default should be applied in the filter.
src/blocks/event-info/components/DateTimeGroup.js (1)
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🧬 Code Graph Analysis (2)
src/classes/class-se-block-variations.php (3)
src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-716)src/classes/class-se-blocks.php (1)
filter_unique_parents_where(971-1010)src/event-functions.php (1)
se_event_treat_each_date_as_own_event(803-808)
src/event-functions.php (4)
src/classes/class-date-display-formatter.php (1)
SE_Date_Display_Formatter(14-671)src/classes/class-se-calendar.php (3)
SE_Calendar(6-459)get_instance(30-36)create_date_time(108-124)src/template-functions.php (1)
se_template_get_event_date_id(18-21)src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-716)
🪛 PHPMD (2.15.0)
src/classes/class-se-event-dates.php
272-272: Avoid unused parameters such as '$hide_from_calendar'. (Unused Code Rules)
(UnusedFormalParameter)
272-272: Avoid unused parameters such as '$hide_from_feed'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 Biome (1.9.4)
src/blocks/event-info/components/DateTimeGroup.js
[error] 134-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 338-338: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 357-357: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 376-376: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (10)
src/classes/class-se-calendar.php (2)
240-285: Excellent refactoring to use WP_Query!The migration from raw SQL to WP_Query is well-implemented with proper meta queries and follows WordPress best practices.
367-401: Well-structured refactoring using the new event dates API!The migration to use
SE_Event_Dates::get_event_dates_for_dateis clean and properly handles data enrichment.src/classes/class-se-event-dates.php (1)
272-332: Well-designed query logic for finding event dates!The meta query properly handles both exact timestamp matches and all-day events within date ranges. The filtering logic for calendar/feed visibility is correctly implemented.
src/classes/class-date-display-formatter.php (1)
14-671: Excellent formatter implementation!This is a well-architected formatter class that:
- Cleanly separates display logic from data management
- Handles timezone complexities properly
- Provides flexible display options for various use cases
- Follows WordPress coding standards
src/event-functions.php (3)
123-129: Deprecation notice properly implemented.The function correctly uses
__doing_it_wrong()to notify developers about the deprecated function and directs them to the new replacement function.
489-514: Well-implemented expiration check.The function correctly handles multiple event dates and properly adjusts end times for all-day events to ensure they expire at the end of the day rather than the beginning.
519-547: Legacy code acknowledgedBased on previous feedback, this function contains legacy code patterns which explains the redundant logic. No action needed.
src/classes/class-se-event-post-type.php (3)
144-200: Well-structured child post type registration.The
se-event-datepost type is properly configured as an internal child post type with appropriate restrictions (non-public, no UI, disabled creation) while maintaining REST API access for programmatic operations.
277-288: Robust meta field configuration with proper sanitization.The addition of default value and sanitize callback ensures consistent handling of the
se_event_datesmeta field, preventing potential type errors.
694-715: Good abstraction for event date deletion.The update to use
SE_Event_Dates::delete_all_event_dates()properly delegates the deletion logic to the dedicated event dates management class, improving maintainability.
| // Compare the date without time to see if the time or date was changed. | ||
| const isDateChange = | ||
| moment(currentDateTime).format('YYYY-MM-DD') === | ||
| moment(newDateTime).format('YYYY-MM-DD'); | ||
|
|
||
| if (isDateChange) { | ||
| setTempEventTime(newDateTime); | ||
| } else { | ||
| setTempEventDate(newDateTime); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix misleading variable name.
The variable isDateChange is confusing - it's true when dates are equal (indicating time change) and false when dates differ (indicating date change).
Rename the variable to better reflect its purpose:
-const isDateChange =
+const isSameDate =
moment(currentDateTime).format('YYYY-MM-DD') ===
moment(newDateTime).format('YYYY-MM-DD');
-if (isDateChange) {
+if (isSameDate) {
setTempEventTime(newDateTime);
} else {
setTempEventDate(newDateTime);
}📝 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.
| // Compare the date without time to see if the time or date was changed. | |
| const isDateChange = | |
| moment(currentDateTime).format('YYYY-MM-DD') === | |
| moment(newDateTime).format('YYYY-MM-DD'); | |
| if (isDateChange) { | |
| setTempEventTime(newDateTime); | |
| } else { | |
| setTempEventDate(newDateTime); | |
| } | |
| // Compare the date without time to see if the time or date was changed. | |
| const isSameDate = | |
| moment(currentDateTime).format('YYYY-MM-DD') === | |
| moment(newDateTime).format('YYYY-MM-DD'); | |
| if (isSameDate) { | |
| setTempEventTime(newDateTime); | |
| } else { | |
| setTempEventDate(newDateTime); | |
| } |
🤖 Prompt for AI Agents
In src/blocks/event-info/components/DateTimeGroup.js around lines 154 to 163,
the variable isDateChange is misleading because it is true when the dates are
equal, indicating a time change, and false when the dates differ, indicating a
date change. Rename isDateChange to something like isSameDate or isDateUnchanged
to clearly reflect that it is true when the dates are the same. Update the
conditional logic accordingly to maintain correct behavior.
| array( | ||
| 'methods' => 'GET', | ||
| 'callback' => array( $this, 'get_event_dates' ), | ||
| 'permission_callback' => '__return_true', |
There was a problem hiding this comment.
Consider adding permission check for GET endpoint.
The GET endpoint currently allows anyone to fetch event dates. Consider checking if the event is published or if the user has appropriate capabilities.
-'permission_callback' => '__return_true',
+'permission_callback' => function( $request ) {
+ $event_id = $request->get_param( 'event_id' );
+ $event = get_post( $event_id );
+ if ( ! $event ) {
+ return false;
+ }
+ // Allow access to published events or users with edit capability
+ return 'publish' === $event->post_status || current_user_can( 'edit_post', $event_id );
+},📝 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.
| 'permission_callback' => '__return_true', | |
| 'permission_callback' => function( $request ) { | |
| $event_id = $request->get_param( 'event_id' ); | |
| $event = get_post( $event_id ); | |
| if ( ! $event ) { | |
| return false; | |
| } | |
| // Allow access to published events or users with edit capability | |
| return 'publish' === $event->post_status | |
| || current_user_can( 'edit_post', $event_id ); | |
| }, |
🤖 Prompt for AI Agents
In src/classes/class-se-event-dates.php at line 59, the GET endpoint's
permission_callback currently allows unrestricted access by returning true.
Modify this callback to check if the event is published or verify that the
current user has the necessary capabilities to view the event dates. Implement a
function that performs these checks and use it as the permission_callback to
restrict access appropriately.
| // Filter out any dates that are in the past. (start and end) | ||
| $event_dates = array_filter( | ||
| $event_dates, | ||
| $event_timezone, | ||
| $hide_end_time, | ||
| $hide_start_time, | ||
| $display_timezone, | ||
| false, | ||
| false | ||
| function ( $date ) use ( $now ) { | ||
| return $date['start_date'] < $now && $date['end_date'] < $now; | ||
| } | ||
| ); |
There was a problem hiding this comment.
Consider the past date filtering logic
The current logic requires both start AND end dates to be in the past. This might exclude events that started in the past but are still ongoing. Consider if this is the intended behavior or if you want to include all events that have started.
🤖 Prompt for AI Agents
In src/event-functions.php around lines 244 to 250, the filtering logic excludes
events only if both start_date and end_date are in the past, which removes
ongoing events that started in the past but haven't ended. To fix this, change
the condition to filter out events only if the end_date is less than the current
time, so ongoing events with past start dates are included.
| $event_time_start = wp_date( get_option( 'time_format' ), $event_dates[0]['start_date'], $timezone ); | ||
| $event_time_end = wp_date( get_option( 'time_format' ), $event_dates[0]['end_date'], $timezone ); | ||
| $same_times = ( 1 < $dates_count ) ? true : false; |
There was a problem hiding this comment.
Fix array key usage for backward compatibility
The function uses 'start_date' and 'end_date' keys, but based on the legacy code patterns in this file (lines 141, 366-367, 389-390), it should use 'datetime_start' and 'datetime_end' for backward compatibility.
-$event_time_start = wp_date( get_option( 'time_format' ), $event_dates[0]['start_date'], $timezone );
-$event_time_end = wp_date( get_option( 'time_format' ), $event_dates[0]['end_date'], $timezone );
+$event_time_start = wp_date( get_option( 'time_format' ), $event_dates[0]['datetime_start'], $timezone );
+$event_time_end = wp_date( get_option( 'time_format' ), $event_dates[0]['datetime_end'], $timezone );📝 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.
| $event_time_start = wp_date( get_option( 'time_format' ), $event_dates[0]['start_date'], $timezone ); | |
| $event_time_end = wp_date( get_option( 'time_format' ), $event_dates[0]['end_date'], $timezone ); | |
| $same_times = ( 1 < $dates_count ) ? true : false; | |
| $event_time_start = wp_date( get_option( 'time_format' ), $event_dates[0]['datetime_start'], $timezone ); | |
| $event_time_end = wp_date( get_option( 'time_format' ), $event_dates[0]['datetime_end'], $timezone ); | |
| $same_times = ( 1 < $dates_count ) ? true : false; |
🤖 Prompt for AI Agents
In src/event-functions.php around lines 340 to 342, replace the array keys
'start_date' and 'end_date' with 'datetime_start' and 'datetime_end'
respectively to maintain backward compatibility with legacy code patterns
observed in this file. Update both instances where these keys are accessed in
the $event_dates array.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/classes/class-se-migrate-events.php (3)
6-6: Remove unused import statement.The
use function Crontrol\Event\get;import is not used anywhere in this class and should be removed.-use function Crontrol\Event\get;
256-256: Remove unused parameter from closure.The
$methodsparameter in the closure is not used and should be removed to address the static analysis warning.return array_filter( self::VERSION_UPGRADES, - function ( $methods, $min_version ) use ( $version ) { + function ( $min_version ) use ( $version ) { return version_compare( $version, $min_version, '<' ); }, - ARRAY_FILTER_USE_BOTH + ARRAY_FILTER_USE_KEY );
285-285: Remove unused loop variable.The
$keyvariable in the foreach loop is not used and should be removed.-foreach ( $dates as $key => $date ) { +foreach ( $dates as $date ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
composer.json(1 hunks)src/classes/class-se-block-variations.php(4 hunks)src/classes/class-se-migrate-events.php(1 hunks)src/templates/content-archive.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/templates/content-archive.php
- composer.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-block-variations.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-se-migrate-events.php (1)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.956Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
🪛 PHPMD (2.15.0)
src/classes/class-se-migrate-events.php
256-256: Avoid unused parameters such as '$methods'. (Unused Code Rules)
(UnusedFormalParameter)
285-285: Avoid unused local variables such as '$key'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (6)
src/classes/class-se-migrate-events.php (4)
117-167: Excellent error handling in the REST endpoint.The REST endpoint implementation demonstrates robust error handling with proper JSON validation, exception catching, and structured REST responses. The input validation and error messaging are well-implemented.
208-238: Well-structured migration logic with proper error handling.The single event migration method properly validates the event post type, handles version-based migration execution, and includes comprehensive error handling. The version update after successful migration is correctly implemented.
50-61: Add nonce verification for CSRF protection.The REST endpoint only checks
manage_optionscapability but lacks nonce verification for CSRF protection. Consider adding nonce validation to the permission callback.register_rest_route( $namespace, '/migrate-events', array( 'methods' => 'POST', 'callback' => array( __CLASS__, 'migrate_events_rest' ), 'permission_callback' => function () { - return current_user_can( 'manage_options' ); + return current_user_can( 'manage_options' ) && wp_verify_nonce( $_REQUEST['_wpnonce'] ?? '', 'se_migrate_events' ); }, ) );Likely an incorrect or invalid review comment.
75-75: No additional availability check needed for SE_Event_Post_TypeThe plugin’s bootstrap (
plugin.php) explicitlyrequire_once-sclass-se-event-post-type.phpbefore loadingclass-se-migrate-events.php, soSE_Event_Post_Type::$post_typeis guaranteed to be defined when used. You can safely remove the suggested class-existence guard.—
Likely an incorrect or invalid review comment.
src/classes/class-se-block-variations.php (2)
110-152: LGTM: Well-implemented post transformation logic.The
modify_event_postsmethod correctly converts event date posts to parent event posts with proper timezone handling and date formatting. The implementation properly:
- Checks for the correct query type
- Handles timezone conversion with fallback to site timezone
- Updates parent post dates with event date information
- Attaches event date ID for reference
181-184: LGTM: Proper meta key ordering logic.The meta key selection correctly uses end date for descending order and start date for ascending order, which aligns with the expected behavior for event ordering.
| $start = $date['datetime_start'] ?? ''; | ||
| $end = $date['datetime_end'] ?? ''; | ||
| $all_day = $date['all_day'] ?? false; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for date format.
The migration extracts date values from the old format but doesn't validate that they are in the expected format before passing to the helper function.
// Unpack
$start = $date['datetime_start'] ?? '';
$end = $date['datetime_end'] ?? '';
$all_day = $date['all_day'] ?? false;
+// Skip if essential date data is missing
+if ( empty( $start ) ) {
+ continue;
+}📝 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.
| $start = $date['datetime_start'] ?? ''; | |
| $end = $date['datetime_end'] ?? ''; | |
| $all_day = $date['all_day'] ?? false; | |
| // Unpack | |
| $start = $date['datetime_start'] ?? ''; | |
| $end = $date['datetime_end'] ?? ''; | |
| $all_day = $date['all_day'] ?? false; | |
| // Skip if essential date data is missing | |
| if ( empty( $start ) ) { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In src/classes/class-se-migrate-events.php around lines 287 to 289, the date
values extracted from the old format lack validation for their format before
use. Add validation checks to ensure $start and $end are in the expected date
format (e.g., using DateTime::createFromFormat or a regex) before passing them
to the helper function. If validation fails, handle the error or set default
values to prevent invalid data processing.
| se_event_create_event_date( | ||
| $event_id, | ||
| array( | ||
| 'all_day' => $all_day, | ||
| 'start_date' => $start, | ||
| 'end_date' => $end, | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for the helper function existence.
The code calls se_event_create_event_date() without verifying it exists. Add a function existence check to prevent fatal errors.
+if ( ! function_exists( 'se_event_create_event_date' ) ) {
+ return; // Skip if helper function doesn't exist
+}
se_event_create_event_date(
$event_id,
array(
'all_day' => $all_day,
'start_date' => $start,
'end_date' => $end,
)
);📝 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.
| se_event_create_event_date( | |
| $event_id, | |
| array( | |
| 'all_day' => $all_day, | |
| 'start_date' => $start, | |
| 'end_date' => $end, | |
| ) | |
| ); | |
| } | |
| if ( ! function_exists( 'se_event_create_event_date' ) ) { | |
| return; // Skip if helper function doesn't exist | |
| } | |
| se_event_create_event_date( | |
| $event_id, | |
| array( | |
| 'all_day' => $all_day, | |
| 'start_date' => $start, | |
| 'end_date' => $end, | |
| ) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/classes/class-se-migrate-events.php around lines 291 to 299, the call to
se_event_create_event_date() lacks a check for the function's existence, which
can cause fatal errors if the function is undefined. Add a conditional check
using function_exists('se_event_create_event_date') before calling it to ensure
the function is defined, and only call it if the check passes.
| add_action( 'rest_api_init', array( __CLASS__, 'register_rest_route' ) ); | ||
| add_action( 'se_migrate_events', array( __CLASS__, 'migrate_events' ) ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the 'se_migrate_events' action hook implementation.
The code registers a hook for 'se_migrate_events' action but there's no corresponding migrate_events method in this class. Only migrate_events_by_ids exists as a private method.
🏁 Script executed:
#!/bin/bash
# Check if there's a migrate_events method or if this action is triggered elsewhere
ast-grep --pattern 'function migrate_events($_) {
$$$
}'
# Also check if this action is triggered anywhere
rg -A 3 "do_action.*se_migrate_events"Length of output: 101
🏁 Script executed:
#!/bin/bash
# Locate the migration class file
file=$(fd 'class-se-migrate-events.php' | head -n1)
echo "Inspecting: $file"
# List all methods defined in that class
grep -nE 'function\s+' "$file"
# Search for where the 'se_migrate_events' hook is fired
echo
echo "Looking for do_action('se_migrate_events') in the repo:"
grep -R "do_action.*se_migrate_events" -n .Length of output: 1116
Define or Remove the migrate_events Hook
The call to
add_action( 'se_migrate_events', [ __CLASS__, 'migrate_events' ] );references a non-existent migrate_events method (and there are no do_action('se_migrate_events') calls in the codebase). You must either:
- Implement a
public static function migrate_events()that triggers your batch migration logic (e.g. callsget_events_to_migrate()andmigrate_events_by_ids()),
or - Remove this
add_actionline if the hook is unused or not yet needed.
Locations:
- src/classes/class-se-migrate-events.php
- Lines ~36–37: the two
add_actioncalls
- Lines ~36–37: the two
Suggested fix snippet (example of adding a wrapper):
diff --git a/src/classes/class-se-migrate-events.php b/src/classes/class-se-migrate-events.php
index abc1234..def5678 100644
--- a/src/classes/class-se-migrate-events.php
+++ b/src/classes/class-se-migrate-events.php
@@ public static function register_rest_route() {
- add_action( 'se_migrate_events', [ __CLASS__, 'migrate_events' ] );
+ add_action( 'se_migrate_events', [ __CLASS__, 'migrate_events' ] );
}
+
+/**
+ * Run all pending event migrations.
+ */
+public static function migrate_events(): void {
+ $ids = self::get_events_to_migrate();
+ if ( empty( $ids ) ) {
+ return;
+ }
+ self::migrate_events_by_ids( $ids );
+}🤖 Prompt for AI Agents
In src/classes/class-se-migrate-events.php around lines 36 to 37, the add_action
call for 'se_migrate_events' references a migrate_events method that does not
exist and the hook is not triggered anywhere. To fix this, either implement a
public static function migrate_events() that contains the batch migration logic
such as calling get_events_to_migrate() and migrate_events_by_ids(), or remove
the add_action line for 'se_migrate_events' if this hook is not currently used
or needed.
| $args['feed_order'] = $feed_order; // Store feed order for use in the WHERE filter | ||
|
|
||
| // Ensure we only get the correct event date for each parent. | ||
| add_filter( 'posts_where', array( $this, 'filter_unique_parents_where' ), 10, 2 ); |
There was a problem hiding this comment.
Remove the filter after use to prevent affecting other queries.
Similar to the modify_event_posts filter, the filter_unique_parents_where filter is added but never removed, which could cause it to affect other queries unintentionally.
Consider implementing a cleanup mechanism or using a more targeted filtering approach.
// Add this after the query is processed or in a cleanup method
-// Ensure we only get the correct event date for each parent.
-add_filter( 'posts_where', array( $this, 'filter_unique_parents_where' ), 10, 2 );
+// Ensure we only get the correct event date for each parent.
+add_filter( 'posts_where', array( $this, 'filter_unique_parents_where' ), 10, 2 );
+// TODO: Remove filter after query is processed to prevent affecting other queriesCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/classes/class-se-block-variations.php at line 219, the filter
'posts_where' is added with the method 'filter_unique_parents_where' but never
removed, which can unintentionally affect other queries. To fix this, remove the
filter immediately after the query that requires it by calling remove_filter
with the same parameters. This ensures the filter only applies to the intended
query and prevents side effects on subsequent queries.
| $query['post_type'] = SE_Event_Post_Type::$event_date_post_type; | ||
|
|
||
| // Add filter to modify posts results | ||
| add_filter( 'the_posts', array( $this, 'modify_event_posts' ), 10, 2 ); |
There was a problem hiding this comment.
Remove the filter after use to prevent affecting other queries.
The modify_event_posts filter is added but never removed, which could cause it to affect other queries unintentionally.
Consider adding a mechanism to remove the filter after the query is processed, or use a more targeted approach to ensure it only affects the intended query.
// Add this after the query is processed or in a cleanup method
-// Add filter to modify posts results
-add_filter( 'the_posts', array( $this, 'modify_event_posts' ), 10, 2 );
+// Add filter to modify posts results
+add_filter( 'the_posts', array( $this, 'modify_event_posts' ), 10, 2 );
+// TODO: Remove filter after query is processed to prevent affecting other queriesCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/classes/class-se-block-variations.php at line 105, the filter 'the_posts'
is added with the method 'modify_event_posts' but never removed, which can
unintentionally affect other queries. To fix this, modify the code to remove the
filter immediately after it has been applied to the intended query, either by
calling remove_filter within the callback or by using a more targeted
conditional check to ensure the filter only runs once for the specific query.
|
|
||
| /** | ||
| * Filter posts to only include the correct event date for each parent. | ||
| * | ||
| * @param string $where The WHERE clause of the query. | ||
| * @param WP_Query $query The WP_Query instance. | ||
| * | ||
| * @return string | ||
| */ | ||
| public function filter_unique_parents_where( $where, $query ) { | ||
| // Check if this is our events query and unique parents is enabled | ||
| if ( ! isset( $query->query_vars['unique_parents'] ) || ! isset( $query->query_vars['feed_order'] ) ) { | ||
| return $where; | ||
| } | ||
|
|
||
| // Skip if treating each date as own event | ||
| if ( se_event_treat_each_date_as_own_event() ) { | ||
| return $where; | ||
| } | ||
|
|
||
| global $wpdb; | ||
|
|
||
| $feed_order = $query->query_vars['feed_order']; | ||
| $meta_key = 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start'; | ||
|
|
||
| // Subquery to get the correct post ID for each parent based on sort order | ||
| $subquery = " | ||
| AND {$wpdb->posts}.ID IN ( | ||
| SELECT p1.ID | ||
| FROM {$wpdb->posts} p1 | ||
| INNER JOIN {$wpdb->postmeta} pm1 ON p1.ID = pm1.post_id AND pm1.meta_key = '{$meta_key}' | ||
| WHERE p1.post_type = '" . SE_Event_Post_Type::$event_date_post_type . "' | ||
| AND p1.post_status = 'publish' | ||
| AND pm1.meta_value = ( | ||
| SELECT " . ( 'desc' === $feed_order ? 'MAX' : 'MIN' ) . "(pm2.meta_value) | ||
| FROM {$wpdb->posts} p2 | ||
| INNER JOIN {$wpdb->postmeta} pm2 ON p2.ID = pm2.post_id AND pm2.meta_key = '{$meta_key}' | ||
| WHERE p2.post_parent = p1.post_parent | ||
| AND p2.post_type = '" . SE_Event_Post_Type::$event_date_post_type . "' | ||
| AND p2.post_status = 'publish' | ||
| ) | ||
| GROUP BY p1.post_parent | ||
| ) | ||
| "; | ||
|
|
||
| $where .= $subquery; | ||
|
|
||
| return $where; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider performance implications of the complex subquery.
The SQL subquery is well-structured and appears correct for ensuring unique parents, but it involves multiple JOINs and subqueries that could be expensive on large datasets.
Consider adding database indexes on the meta keys used in the subquery and monitor query performance in production environments with large event datasets.
🏁 Script executed:
#!/bin/bash
# Check for potential performance bottlenecks in the subquery logic
rg -A 10 -B 5 "se_event_date_start|se_event_date_end" --type php | head -50Length of output: 2709
Evaluate and optimize performance of the subquery in filter_unique_parents_where
The nested JOINs and meta-value subqueries against wp_postmeta can lead to full scans on large datasets, potentially slowing down event feeds. To improve query performance and ensure scalability:
• File: src/classes/class-se-block-variations.php method filter_unique_parents_where (lines 230–278)
• Add a composite index on wp_postmeta(meta_key, meta_value) so joins and value lookups for se_event_date_start/se_event_date_end benefit from indexing
• Run EXPLAIN against the full generated SQL in staging to confirm the new index is used and identify any remaining table scans
• Verify that posts.post_parent is indexed (WordPress core typically adds this) or add a custom index if missing
• Consider caching the filtered parent IDs (e.g., with transients or object cache) for high-traffic feeds to avoid repeated expensive queries
• Monitor query timings in production once indexed and cached, adjusting as needed for large event datasets
🤖 Prompt for AI Agents
In src/classes/class-se-block-variations.php within the
filter_unique_parents_where method (lines 230–278), the subquery uses joins and
meta_value lookups on wp_postmeta that can cause slow full table scans. To fix
this, add a composite index on wp_postmeta(meta_key, meta_value) to speed up
these lookups, verify that posts.post_parent is indexed or add an index if
missing, and run EXPLAIN on the full query in staging to confirm index usage.
Additionally, implement caching of the filtered parent IDs using transients or
object cache to reduce repeated query load on high-traffic feeds, and monitor
query performance in production to adjust indexing or caching strategies as
needed.
| $subquery = " | ||
| AND {$wpdb->posts}.ID IN ( | ||
| SELECT p1.ID | ||
| FROM {$wpdb->posts} p1 | ||
| INNER JOIN {$wpdb->postmeta} pm1 ON p1.ID = pm1.post_id AND pm1.meta_key = '{$meta_key}' | ||
| WHERE p1.post_type = '" . SE_Event_Post_Type::$event_date_post_type . "' | ||
| AND p1.post_status = 'publish' | ||
| AND pm1.meta_value = ( | ||
| SELECT " . ( 'desc' === $feed_order ? 'MAX' : 'MIN' ) . "(pm2.meta_value) | ||
| FROM {$wpdb->posts} p2 | ||
| INNER JOIN {$wpdb->postmeta} pm2 ON p2.ID = pm2.post_id AND pm2.meta_key = '{$meta_key}' | ||
| WHERE p2.post_parent = p1.post_parent | ||
| AND p2.post_type = '" . SE_Event_Post_Type::$event_date_post_type . "' | ||
| AND p2.post_status = 'publish' | ||
| ) | ||
| GROUP BY p1.post_parent | ||
| ) | ||
| "; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify SQL query security and escaping.
The SQL subquery construction appears safe since $meta_key is derived from controlled values, but it's good practice to ensure proper escaping for security.
The $meta_key variable is set from controlled values ('se_event_date_start' or 'se_event_date_end'), which should be safe, but consider using $wpdb->prepare() or esc_sql() for additional security assurance.
🏁 Script executed:
#!/bin/bash
# Check how meta_key is used in SQL queries throughout the codebase
rg -A 5 -B 5 "meta_key.*=.*'" --type php | head -30Length of output: 1994
🏁 Script executed:
#!/bin/bash
# Show where $meta_key is defined or assigned in class-se-block-variations.php
rg -n "\$meta_key\s*=" -C3 src/classes/class-se-block-variations.php
# Dump lines 200–300 to see the context around the subquery and $meta_key usage
sed -n '200,300p' src/classes/class-se-block-variations.phpLength of output: 2759
Escape $meta_key before injecting into SQL
Although $meta_key is always 'se_event_date_start' or 'se_event_date_end', it’s best practice to harden this variable against injection. In filter_unique_parents_where() you can either wrap the assignment in esc_sql() or move the meta‐key check into a prepared statement.
• File: src/classes/class-se-block-variations.php
• Method: filter_unique_parents_where() (around line 264)
Example using esc_sql():
- $meta_key = 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start';
+ $meta_key = esc_sql( 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start' );Or switch to $wpdb->prepare() for the entire subquery:
$subquery = $wpdb->prepare(
"
AND {$wpdb->posts}.ID IN (
SELECT p1.ID
FROM {$wpdb->posts} p1
INNER JOIN {$wpdb->postmeta} pm1
ON p1.ID = pm1.post_id AND pm1.meta_key = %s
…
)",
$meta_key
);Either approach will ensure the meta key is safely escaped.
🤖 Prompt for AI Agents
In src/classes/class-se-block-variations.php around lines 256 to 273, the
variable $meta_key is directly injected into the SQL query without escaping,
which risks SQL injection. To fix this, either wrap $meta_key with esc_sql()
before including it in the query string or refactor the entire subquery to use
$wpdb->prepare() with %s placeholders for $meta_key, passing $meta_key as a
parameter to safely escape it.
…ype from the parent events query loop to the grandchild meta info block
…-to-children Patched issue where sometime we do not pass the sort order and feed t…
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/classes/class-se-block-variations.php (4)
105-105: Remove the filter after use to prevent affecting other queries.The
modify_event_postsfilter is added but never removed, which could cause it to affect other queries unintentionally.
221-221: Remove the filter after use to prevent affecting other queries.Similar to the
modify_event_postsfilter, thefilter_unique_parents_wherefilter is added but never removed, which could cause it to affect other queries unintentionally.
256-259: Escape$meta_keybefore injecting into SQLAlthough
$meta_keyis always'se_event_date_start'or'se_event_date_end', it's best practice to harden this variable against injection. Infilter_unique_parents_where()you can either wrap the assignment inesc_sql()or move the meta‐key check into a prepared statement.Also applies to: 287-287
282-303: Consider performance implications of the complex subquery.The SQL subquery is well-structured and appears correct for ensuring unique parents, but it involves multiple JOINs and subqueries that could be expensive on large datasets.
src/classes/class-se-blocks.php (2)
984-984: Escape$meta_keybefore injecting into SQLSame SQL injection concern as in
SE_Block_Variations::filter_unique_parents_where().Also applies to: 1013-1013
988-1033: Optimize complex SQL subquery for performanceThis SQL subquery with multiple joins and aggregation functions could be a performance bottleneck on large datasets. Consider adding database indexes or implementing caching.
🧹 Nitpick comments (2)
src/classes/class-se-block-variations.php (1)
124-124: Fix incomplete comment.The comment appears to be incomplete.
- // Return back the + // Return the parent posts with updated datessrc/classes/class-se-blocks.php (1)
796-806: Consider validating the dynamically selected function exists.While the switch statement covers all expected cases, it would be safer to verify the function exists before calling it.
// Based on the feed type, set the get_date_function switch ( $attributes['feedType'] ?? 'default' ) { case 'upcoming': $get_date_function = 'se_event_get_future_dates'; break; case 'past': $get_date_function = 'se_event_get_past_dates'; break; default: $get_date_function = 'se_event_get_formatted_dates'; break; } + + // Ensure the function exists + if ( ! function_exists( $get_date_function ) ) { + $get_date_function = 'se_event_get_formatted_dates'; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
simple-events.zipis excluded by!**/*.zip
📒 Files selected for processing (4)
src/blocks/loop-event-info/index.js(2 hunks)src/classes/class-se-block-variations.php(4 hunks)src/classes/class-se-blocks.php(10 hunks)src/variations/query-loop-events/block.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/blocks/loop-event-info/index.js
- src/variations/query-loop-events/block.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-blocks.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-se-block-variations.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🧬 Code Graph Analysis (1)
src/classes/class-se-block-variations.php (3)
src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-716)src/classes/class-se-blocks.php (1)
filter_unique_parents_where(970-1033)src/event-functions.php (1)
se_event_treat_each_date_as_own_event(803-808)
🔇 Additional comments (3)
src/classes/class-se-blocks.php (3)
124-128: LGTM!The addition of event version data to the localized script properly supports the new event date system.
244-291: Well-implemented date display formatting!The refactoring to use
SE_Date_Display_Formatterclass improves code organization and the active date header logic is well thought out.
555-561: Excellent filter management!The filters are properly added before the query and removed afterwards, preventing them from affecting other queries. This is the correct pattern that should be followed in
SE_Block_Variationsas well.Also applies to: 569-570, 602-605
| global $wpdb; | ||
|
|
||
| $feed_order = $query->query_vars['feed_order']; | ||
| $meta_key = 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start'; |
There was a problem hiding this comment.
Inconsistent case handling for feed order comparison.
Line 258 doesn't use strtolower() while line 291 does, creating inconsistency in how feed order is compared.
- $meta_key = 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start';
+ $meta_key = 'desc' === strtolower( $feed_order ) ? 'se_event_date_end' : 'se_event_date_start';Also applies to: 291-291
🤖 Prompt for AI Agents
In src/classes/class-se-block-variations.php at lines 258 and 291, the
comparison of the variable feed_order to the string 'desc' is inconsistent
because line 258 does not use strtolower() while line 291 does. To fix this,
apply strtolower() to feed_order in both lines before comparing it to 'desc' to
ensure consistent case-insensitive comparison.
| public static function filter_unique_parents_where( $where, $query ) { | ||
| // Check if this is our events query and unique parents is enabled | ||
| if ( ! isset( $query->query_vars['unique_parents'] ) || ! isset( $query->query_vars['feed_order'] ) ) { | ||
| return $where; | ||
| } | ||
|
|
||
| // Skip if treating each date as own event | ||
| if ( se_event_treat_each_date_as_own_event() ) { | ||
| return $where; | ||
| } | ||
|
|
||
| global $wpdb; | ||
|
|
||
| $feed_order = $query->query_vars['feed_order']; | ||
| $meta_key = 'desc' === strtolower( $feed_order ) ? 'se_event_date_end' : 'se_event_date_start'; | ||
|
|
||
| // Get the current time filtering from the main query's meta_query | ||
| $time_filter = ''; | ||
| $meta_query = $query->get( 'meta_query' ); | ||
| if ( ! empty( $meta_query ) && is_array( $meta_query ) ) { | ||
| foreach ( $meta_query as $meta_condition ) { | ||
| if ( isset( $meta_condition['key'] ) && 'se_event_date_end' === $meta_condition['key'] ) { | ||
| $compare = $meta_condition['compare']; | ||
| $value = $meta_condition['value']; | ||
|
|
||
| // Add the same time filtering to the subquery | ||
| if ( '>=' === $compare ) { | ||
| // For upcoming events | ||
| $time_filter = "AND pm3.meta_value >= {$value}"; | ||
| } elseif ( '<' === $compare ) { | ||
| // For past events | ||
| $time_filter = "AND pm3.meta_value < {$value}"; | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Subquery to get the correct post ID for each parent based on sort order | ||
| $subquery = " | ||
| AND {$wpdb->posts}.ID IN ( | ||
| SELECT p1.ID | ||
| FROM {$wpdb->posts} p1 | ||
| INNER JOIN {$wpdb->postmeta} pm1 ON p1.ID = pm1.post_id AND pm1.meta_key = '{$meta_key}' | ||
| WHERE p1.post_type = '" . SE_Event_Post_Type::$event_date_post_type . "' | ||
| AND p1.post_status = 'publish' | ||
| AND pm1.meta_value = ( | ||
| SELECT " . ( 'desc' === strtolower( $feed_order ) ? 'MAX' : 'MIN' ) . "(pm2.meta_value) | ||
| FROM {$wpdb->posts} p2 | ||
| INNER JOIN {$wpdb->postmeta} pm2 ON p2.ID = pm2.post_id AND pm2.meta_key = '{$meta_key}' | ||
| " . ( $time_filter ? "INNER JOIN {$wpdb->postmeta} pm3 ON p2.ID = pm3.post_id AND pm3.meta_key = 'se_event_date_end'" : '' ) . " | ||
| WHERE p2.post_parent = p1.post_parent | ||
| AND p2.post_type = '" . SE_Event_Post_Type::$event_date_post_type . "' | ||
| AND p2.post_status = 'publish' | ||
| {$time_filter} | ||
| ) | ||
| GROUP BY p1.post_parent | ||
| ) | ||
| "; | ||
|
|
||
| $where .= $subquery; | ||
|
|
||
| return $where; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication between classes.
These static methods duplicate the functionality of instance methods in SE_Block_Variations. This violates the DRY principle and creates maintenance burden.
Consider refactoring to:
- Create a shared trait or helper class for the common filtering logic
- Or have
SE_Block_Variationsuse these static methods fromSE_Blocks - Or move all the filtering logic to a dedicated service class
Example approach using static methods from SE_Blocks:
// In SE_Block_Variations::build_query()
add_filter( 'the_posts', array( 'SE_Blocks', 'modify_event_posts_for_blocks' ), 10, 2 );
// In SE_Block_Variations::set_event_query_args()
add_filter( 'posts_where', array( 'SE_Blocks', 'filter_unique_parents_where' ), 10, 2 );Also applies to: 1043-1077
🤖 Prompt for AI Agents
In src/classes/class-se-blocks.php around lines 970 to 1033, the static method
filter_unique_parents_where duplicates functionality found in instance methods
of SE_Block_Variations, violating DRY principles. Refactor by extracting the
shared filtering logic into a common trait or helper class, or alternatively
have SE_Block_Variations call these static methods from SE_Blocks instead of
duplicating code. This will centralize the logic, reduce maintenance overhead,
and ensure consistency. Apply the same refactoring approach to the similar code
block at lines 1043 to 1077.
…ry-loop Feature/add per page to event query loop
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/classes/class-se-block-variations.php (3)
307-326: Monitor and optimize the performance of this complex subquery.The nested JOINs and meta lookups could impact performance on large datasets.
Consider these optimizations:
- Add composite index on
wp_postmeta(meta_key, meta_value)for the date lookups- Implement caching of filtered parent IDs using transients for high-traffic feeds
- Monitor query execution time in production and adjust as needed
218-224: Implement filter cleanup to prevent affecting other queries.The filters added here are never removed, which could unintentionally affect subsequent queries. This was flagged in previous reviews and remains unaddressed.
Consider implementing a cleanup mechanism:
// Store the filters that need cleanup private $active_filters = []; // In set_event_query_args, track filters $this->active_filters[] = ['posts_where', [$this, 'filter_unique_parents_where'], 10]; add_filter('posts_where', [$this, 'filter_unique_parents_where'], 10, 2); // Add a cleanup method public function cleanup_filters() { foreach ($this->active_filters as $filter) { remove_filter($filter[0], $filter[1], $filter[2]); } $this->active_filters = []; } // Hook cleanup to appropriate action add_action('loop_end', [$this, 'cleanup_filters']);
283-283: Apply SQL escaping for defense in depth.While
$meta_keyis derived from controlled values, it's best practice to escape it.- $meta_key = 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start'; + $meta_key = esc_sql( 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start' );
🧹 Nitpick comments (4)
src/variations/query-loop-events/block.js (1)
110-129: Remove redundant case clause.The
case 'default':is unnecessary since thedefault:clause returns the same result.Apply this diff to remove the redundant case:
- case 'default': default: return [ { label: 'Oldest to Newest', value: 'asc' }, { label: 'Newest to Oldest', value: 'desc' }, ];src/classes/class-se-block-variations.php (3)
121-121: Fix incomplete comment.The comment appears to be cut off.
- // Return back the + // Return the parent events instead of event dates
162-162: Remove trailing # character.There's a stray
#character at the end of the line.- $feed_order = $request->get_param( 'order' );# + $feed_order = $request->get_param( 'order' );
273-273: Remove commented debug code.- // adump( $query->query_vars['unique_parents'] ); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/classes/class-se-block-variations.php(5 hunks)src/variations/query-loop-events/block.js(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-block-variations.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/variations/query-loop-events/block.js (1)
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🪛 GitHub Actions: PHP Coding Standards
src/classes/class-se-block-variations.php
[warning] 273-273: PHP_CodeSniffer: This comment is 67% valid code; is this commented out code? (Squiz.PHP.CommentedOutCode.Found)
🪛 Biome (1.9.4)
src/variations/query-loop-events/block.js
[error] 122-122: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (6)
src/variations/query-loop-events/block.js (4)
17-43: LGTM! Well-structured Redux store implementation.The Redux store for managing query data across blocks is implemented correctly with proper actions and selectors.
69-72: Good implementation of query caching and event info display.The addition of
_cacheBusterensures queries are refreshed when parameters change, and the switch tosimple-events/loop-event-infoaligns with the new event date architecture.Also applies to: 77-77
88-103: Clean implementation of query data synchronization.The component correctly dispatches query data to the Redux store, enabling child blocks to access feed type and order information. The effect dependencies are properly configured.
169-192: Well-implemented pagination control.The RangeControl provides intuitive control over the number of events displayed, with appropriate limits and clear labeling. The
_cacheBusterensures queries refresh when the value changes.src/classes/class-se-block-variations.php (2)
101-103: Correct implementation of event date post type querying.The change to query event date posts instead of event posts aligns with the new architecture where dates are managed as child posts.
236-256: Good fix for editor sorting inconsistency.The method correctly handles the edge case where the editor forces ASC ordering, ensuring the intended sort order is preserved.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/classes/class-se-block-variations.php (3)
213-225: Remove filters after use to prevent side effects.The filters are added but never removed, which can unintentionally affect other queries throughout the request lifecycle.
Consider implementing a cleanup mechanism or using a more targeted approach:
// add the arg to denote unique parents. $args['unique_parents'] = true; $args['feed_order'] = $feed_order; // Store feed order for use in the WHERE filter // Ensure we only get the correct event date for each parent. add_filter( 'posts_where', array( $this, 'filter_unique_parents_where' ), 10, 2 ); // Add a filter to modify the posts results. add_filter( 'the_posts', array( $this, 'modify_event_posts' ), 10, 2 ); // Add a custom order by. add_filter( 'posts_orderby', array( $this, 'fix_editor_sort_order' ), 10, 2 ); +// TODO: Remove these filters after the query is processed to prevent affecting other queries +// Consider implementing a cleanup mechanism in the modify_event_posts method
281-281: Inconsistent case handling for feed order comparison.Line 281 doesn't use
strtolower()while line 313 does, creating inconsistency in how feed order is compared.-$meta_key = 'desc' === $feed_order ? 'se_event_date_end' : 'se_event_date_start'; +$meta_key = 'desc' === strtolower( $feed_order ) ? 'se_event_date_end' : 'se_event_date_start';
304-329: Escape meta_key and optimize complex subquery.The subquery logic is sound but has security and performance concerns.
Security: Escape the
$meta_keyvariable:-$meta_key = 'desc' === strtolower( $feed_order ) ? 'se_event_date_end' : 'se_event_date_start'; +$meta_key = esc_sql( 'desc' === strtolower( $feed_order ) ? 'se_event_date_end' : 'se_event_date_start' );Performance: The complex subquery with multiple JOINs could be expensive on large datasets. Consider:
- Adding database indexes on
meta_keyandmeta_valuecolumns- Implementing caching for the filtered parent IDs
- Monitoring query performance in production environments
#!/bin/bash # Check for other uses of meta_key in SQL queries for consistency rg -A 3 -B 3 "meta_key.*=.*\\\$" --type php
🧹 Nitpick comments (1)
src/classes/class-se-block-variations.php (1)
107-149: Verify timezone handling and add error handling.The method correctly transforms event date posts to their parent events with proper timezone conversion. However, consider adding error handling for cases where the parent post or meta data might not exist.
public function modify_event_posts( $posts, $query ) { // Check if this is our events query if ( ! isset( $query->query_vars['sub-type'] ) || self::QUERY_LOOP_EVENTS !== $query->query_vars['sub-type'] ) { return $posts; } // Return back the return array_map( function ( $post ) { $parent = get_post( $post->post_parent ); + if ( ! $parent ) { + return $post; // Fallback to original post if parent not found + } // Get the start date from the event. $start_date_ts = get_post_meta( $post->ID, 'se_event_date_start', true ); + if ( ! $start_date_ts ) { + return $parent; // Return parent without date modification if no start date + } // Get the event timezone. $timezone = get_post_meta( $parent->ID, 'se_event_timezone', true ); // use the timezone or default to the site timezone. $timezone = $timezone ? $timezone : wp_timezone_string(); // Get the date im this format 2025-07-01 13:14:09 $start_date = wp_date( 'Y-m-d H:i:s', $start_date_ts, new \DateTimeZone( $timezone ) ); $start_date_gmt = wp_date( 'Y-m-d H:i:s', $start_date_ts, new \DateTimeZone( 'UTC' ) ); // update the parent posts post date $parent->post_date = $start_date; $parent->post_date_gmt = $start_date_gmt; $parent->post_modified = $start_date; $parent->post_modified_gmt = $start_date_gmt; $parent->event_date_id = $post->ID; return $parent; }, $posts ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/classes/class-se-block-variations.php(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-block-variations.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🧬 Code Graph Analysis (1)
src/classes/class-se-block-variations.php (3)
src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-716)src/classes/class-se-blocks.php (1)
filter_unique_parents_where(970-1033)src/event-functions.php (1)
se_event_treat_each_date_as_own_event(803-808)
🔇 Additional comments (3)
src/classes/class-se-block-variations.php (3)
101-102: Post type change looks correct.The change to query event date posts instead of events aligns with the new architecture described in the PR objectives.
178-184: Ordering logic is correct.The meta_key selection based on feed_order is appropriate - using end date for descending order and start date for ascending order makes sense for event chronology.
236-256: Fix for sort order issue looks appropriate.The method correctly addresses the admin/editor sort order bug by replacing the default ASC with the intended feed order.
Fixes issues with calendar dates and also hiding passed events
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/classes/class-se-event-dates.php (1)
59-59: Consider adding permission check for GET endpoint.The GET endpoint currently allows anyone to fetch event dates. Consider checking if the event is published or if the user has appropriate capabilities.
src/classes/class-se-blocks.php (1)
988-1051: Eliminate code duplication between classes.These static methods duplicate the functionality of instance methods in
SE_Block_Variations. This violates the DRY principle and creates maintenance burden.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/classes/class-se-blocks.php(10 hunks)src/classes/class-se-event-dates.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-blocks.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
src/classes/class-se-event-dates.php (2)
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🧬 Code Graph Analysis (1)
src/classes/class-se-blocks.php (4)
src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-716)src/classes/class-date-display-formatter.php (5)
SE_Date_Display_Formatter(14-671)modify_timezone(170-174)render_active_date(265-290)render_date_list(301-349)is_treating_each_date_as_own_event(190-192)src/event-functions.php (3)
se_event_get_event_dates(730-770)se_create_date_time_from_timestamp(780-796)se_event_treat_each_date_as_own_event(803-808)src/classes/class-se-block-variations.php (1)
filter_unique_parents_where(267-329)
🪛 PHPMD (2.15.0)
src/classes/class-se-event-dates.php
272-272: Avoid unused parameters such as '$hide_from_calendar'. (Unused Code Rules)
(UnusedFormalParameter)
272-272: Avoid unused parameters such as '$hide_from_feed'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (2)
src/classes/class-se-blocks.php (2)
277-280: Arrow function syntax is compatible with the plugin’s PHP requirementThe plugin header specifies
Requires PHP: 8.0, and arrow functions (fn() => …) have been supported since PHP 7.4. No changes needed here.
802-808: Add null check for global $post variable.The code accesses the global
$postvariable without checking if it's set, which could cause errors in certain contexts.Apply this diff to add proper validation:
public static function loop_event_info_render( $attributes, $content, $block ): string { global $post; $output = ''; $prefix = ''; $post_ID = ( isset( $attributes['thePostId'] ) && $attributes['thePostId'] > 0 ) ? $attributes['thePostId'] : $block->context['postId']; - $event_date_id = $post instanceof \WP_Post && property_exists( $post, 'event_date_id' ) && is_numeric( $post->event_date_id ) && se_event_treat_each_date_as_own_event() + $event_date_id = isset( $post ) && $post instanceof \WP_Post && property_exists( $post, 'event_date_id' ) && is_numeric( $post->event_date_id ) && se_event_treat_each_date_as_own_event() ? absint( $post->event_date_id ) : null;⛔ Skipped due to learnings
Learnt from: gin0115 PR: a8cteam51/simple-events#21 File: src/event-functions.php:544-547 Timestamp: 2025-07-07T09:19:18.986Z Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
| return array_filter( | ||
| $mapped, | ||
| function ( $event_date ) use ( $hide_from_calendar, $hide_from_feed ) { | ||
| return ! $event_date['event_hide_from_calendar'] && ! $event_date['event_hide_from_feed']; | ||
| } | ||
| ); |
There was a problem hiding this comment.
Fix filter logic for hide_from_calendar and hide_from_feed parameters.
The filter logic appears incorrect. The method parameters indicate what content to hide, but the filter only checks the event properties without considering these parameters.
The current filter excludes events that have either hide_from_calendar or hide_from_feed set to true, regardless of the method parameters. Apply this diff to fix the logic:
return array_filter(
$mapped,
- function ( $event_date ) use ( $hide_from_calendar, $hide_from_feed ) {
- return ! $event_date['event_hide_from_calendar'] && ! $event_date['event_hide_from_feed'];
+ function ( $event_date ) use ( $hide_from_calendar, $hide_from_feed ) {
+ // Include event if:
+ // - We're showing calendar content AND event is not hidden from calendar
+ // - OR we're showing feed content AND event is not hidden from feed
+ $show_in_calendar = ! $hide_from_calendar && ! $event_date['event_hide_from_calendar'];
+ $show_in_feed = ! $hide_from_feed && ! $event_date['event_hide_from_feed'];
+ return $show_in_calendar || $show_in_feed;
}
);📝 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.
| return array_filter( | |
| $mapped, | |
| function ( $event_date ) use ( $hide_from_calendar, $hide_from_feed ) { | |
| return ! $event_date['event_hide_from_calendar'] && ! $event_date['event_hide_from_feed']; | |
| } | |
| ); | |
| return array_filter( | |
| $mapped, | |
| function ( $event_date ) use ( $hide_from_calendar, $hide_from_feed ) { | |
| // Include event if: | |
| // - We're showing calendar content AND event is not hidden from calendar | |
| // - OR we're showing feed content AND event is not hidden from feed | |
| $show_in_calendar = ! $hide_from_calendar && ! $event_date['event_hide_from_calendar']; | |
| $show_in_feed = ! $hide_from_feed && ! $event_date['event_hide_from_feed']; | |
| return $show_in_calendar || $show_in_feed; | |
| } | |
| ); |
🤖 Prompt for AI Agents
In src/classes/class-se-event-dates.php around lines 327 to 332, the filter
logic incorrectly ignores the method parameters hide_from_calendar and
hide_from_feed when filtering events. Update the filter callback to consider
these parameters by excluding events only if the corresponding parameter is true
and the event property indicates it should be hidden. This means the filter
should return false for events where hide_from_calendar is true and
event_hide_from_calendar is true, or hide_from_feed is true and
event_hide_from_feed is true, otherwise return true.
| $existing_date_ids = array_map( | ||
| function ( $date ) { | ||
| return $date['id']; | ||
| }, | ||
| se_event_get_event_dates( $event_id ) | ||
| ); |
There was a problem hiding this comment.
Add error handling for se_event_get_event_dates call.
The call to se_event_get_event_dates can throw an exception but lacks error handling here, unlike the proper handling in delete_all_event_dates.
Apply this diff to add error handling:
// Get the existing dates.
+try {
$existing_date_ids = array_map(
function ( $date ) {
return $date['id'];
},
se_event_get_event_dates( $event_id )
);
+} catch ( \Exception $e ) {
+ return new WP_REST_Response(
+ array(
+ 'code' => 'server_error',
+ 'message' => __( 'An error occurred while fetching existing event dates.', 'simple-events' ),
+ ),
+ 500
+ );
+}📝 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.
| $existing_date_ids = array_map( | |
| function ( $date ) { | |
| return $date['id']; | |
| }, | |
| se_event_get_event_dates( $event_id ) | |
| ); | |
| // Get the existing dates. | |
| try { | |
| $existing_date_ids = array_map( | |
| function ( $date ) { | |
| return $date['id']; | |
| }, | |
| se_event_get_event_dates( $event_id ) | |
| ); | |
| } catch ( \Exception $e ) { | |
| return new WP_REST_Response( | |
| array( | |
| 'code' => 'server_error', | |
| 'message' => __( 'An error occurred while fetching existing event dates.', 'simple-events' ), | |
| ), | |
| 500 | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/classes/class-se-event-dates.php around lines 165 to 170, the call to
se_event_get_event_dates($event_id) can throw an exception but currently lacks
error handling. Wrap this call in a try-catch block to catch any exceptions
thrown, and handle the error appropriately, such as logging the error or
returning early, similar to the error handling approach used in the
delete_all_event_dates method.
…ts-are-also-migrated Fix/ensure draft and trashed events are also migrated
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/classes/class-se-migrate-events.php (3)
36-37: Define the missingmigrate_eventsmethod.The action hook references a non-existent
migrate_eventsmethod. This will cause fatal errors when the hook is triggered.+ /** + * Run all pending event migrations. + * + * @return void + */ + public static function migrate_events(): void { + $events = self::get_events_to_migrate(); + if ( empty( $events ) ) { + return; + } + self::migrate_events_by_ids( wp_list_pluck( $events, 'ID' ) ); + }
343-345: Add validation for date format.The migration extracts date values but doesn't validate their format before use, which could lead to invalid data being processed.
// Unpack $start = $date['datetime_start'] ?? ''; $end = $date['datetime_end'] ?? ''; $all_day = $date['all_day'] ?? false; +// Skip if essential date data is missing +if ( empty( $start ) ) { + continue; +}
347-355: Add validation for helper function existence.The code calls
se_event_create_event_date()without verifying it exists, which could cause fatal errors.+if ( ! function_exists( 'se_event_create_event_date' ) ) { + return; // Skip if helper function doesn't exist +} se_event_create_event_date( $event_id, array( 'all_day' => $all_day, 'start_date' => $start, 'end_date' => $end, ) );
🧹 Nitpick comments (5)
src/classes/class-se-migrate-events.php (5)
6-6: Remove unused import statement.The
use function Crontrol\Event\get;import is not used anywhere in the class and should be removed to avoid confusion.-use function Crontrol\Event\get;
131-131: Remove unused parameter.The
$requestparameter is not used in the method body and generates a static analysis warning.- public static function migrate_events_rest_all( $request ) { // phpcs:ignore + public static function migrate_events_rest_all( $request ) {Or better yet, use an underscore to indicate the parameter is intentionally unused:
- public static function migrate_events_rest_all( $request ) { // phpcs:ignore + public static function migrate_events_rest_all( $_request ) {
312-312: Remove unused parameter from closure.The
$methodsparameter in the closure is not used and generates a static analysis warning.return array_filter( self::VERSION_UPGRADES, - function ( $methods, $min_version ) use ( $version ) { + function ( $_methods, $min_version ) use ( $version ) { return version_compare( $version, $min_version, '<' ); }, ARRAY_FILTER_USE_BOTH );
341-341: Remove unused loop variable.The
$keyvariable is not used in the foreach loop and generates a static analysis warning.- foreach ( $dates as $key => $date ) { + foreach ( $dates as $date ) {
268-270: Use constant instead of hardcoded string.The post type check uses a hardcoded string instead of the class constant, which reduces maintainability.
- if ( ! $event_post || 'se-event' !== $event_post->post_type ) { + if ( ! $event_post || SE_Event_Post_Type::$post_type !== $event_post->post_type ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
simple-events.zipis excluded by!**/*.zip
📒 Files selected for processing (2)
src/assets/js/admin.js(1 hunks)src/classes/class-se-migrate-events.php(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-migrate-events.php (2)
Learnt from: gin0115
PR: #21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The se_event_get_calendar_link function in src/event-functions.php contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: #17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🪛 PHPMD (2.15.0)
src/classes/class-se-migrate-events.php
131-131: Avoid unused parameters such as '$request'. (Unused Code Rules)
(UnusedFormalParameter)
312-312: Avoid unused parameters such as '$methods'. (Unused Code Rules)
(UnusedFormalParameter)
341-341: Avoid unused local variables such as '$key'. (Unused Code Rules)
(UnusedLocalVariable)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/assets/js/admin.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gin0115
PR: a8cteam51/simple-events#17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
Learnt from: gin0115
PR: a8cteam51/simple-events#21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The `se_event_get_calendar_link` function in `src/event-functions.php` contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
src/classes/class-se-migrate-events.php (2)
Learnt from: gin0115
PR: #21
File: src/event-functions.php:544-547
Timestamp: 2025-07-07T09:19:18.986Z
Learning: The se_event_get_calendar_link function in src/event-functions.php contains legacy code, which explains why it may have redundant or suboptimal implementation patterns.
Learnt from: gin0115
PR: #17
File: src/blocks/event-info/index.js:82-98
Timestamp: 2025-06-17T12:10:45.329Z
Learning: In src/blocks/event-info/index.js, when all event dates are in the past, the fallback logic intentionally takes the latest start date and latest end date independently (even from different events) to handle overlapping events correctly. This approach captures the full scope of recent event activity rather than constraining to a single event's date range.
🪛 PHPMD (2.15.0)
src/classes/class-se-migrate-events.php
131-131: Avoid unused parameters such as '$request'. (Unused Code Rules)
(UnusedFormalParameter)
312-312: Avoid unused parameters such as '$methods'. (Unused Code Rules)
(UnusedFormalParameter)
341-341: Avoid unused local variables such as '$key'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (1)
src/classes/class-se-migrate-events.php (1)
23-23: Fix PHPDoc syntax error.The
@paramtag is incomplete and should specify the parameter name and description.- * @param array + * @var array<string, array<string>>Likely an incorrect or invalid review comment.
| 'methods' => array( 'POST', 'GET' ), | ||
| 'callback' => array( __CLASS__, 'migrate_events_rest_all' ), | ||
| 'permission_callback' => function () { | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
Restrict permissions for migrate-all-events endpoint.
The /migrate-all-events endpoint has 'permission_callback' => function () { return true; }, allowing anyone to trigger migrations. This is a serious security vulnerability.
'permission_callback' => function () {
- return true;
+ return current_user_can( 'manage_options' );
},📝 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.
| 'methods' => array( 'POST', 'GET' ), | |
| 'callback' => array( __CLASS__, 'migrate_events_rest_all' ), | |
| 'permission_callback' => function () { | |
| return true; | |
| }, | |
| 'methods' => array( 'POST', 'GET' ), | |
| 'callback' => array( __CLASS__, 'migrate_events_rest_all' ), | |
| 'permission_callback' => function () { | |
| return current_user_can( 'manage_options' ); | |
| }, |
🤖 Prompt for AI Agents
In src/classes/class-se-migrate-events.php around lines 67 to 71, the
permission_callback for the /migrate-all-events endpoint currently returns true,
allowing unrestricted access. Replace this with a proper permission check, such
as verifying the current user has an appropriate capability or role, to restrict
access only to authorized users and prevent unauthorized triggering of
migrations.
| // Cast to an array of integers from JSON | ||
| $event_ids = array_map( 'intval', $event_ids ); | ||
| // Validate the event IDs. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for event IDs array.
After JSON decoding, the code should validate that the result is actually an array before processing.
// Cast to an array of integers from JSON
-$event_ids = array_map( 'intval', $event_ids );
+if ( ! is_array( $event_ids ) ) {
+ return new WP_REST_Response(
+ array(
+ 'message' => esc_html__( 'Event IDs must be provided as an array.', 'simple-events' ),
+ ),
+ 400
+ );
+}
+
+$event_ids = array_map( 'intval', $event_ids );
// Validate the event IDs.📝 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.
| // Cast to an array of integers from JSON | |
| $event_ids = array_map( 'intval', $event_ids ); | |
| // Validate the event IDs. | |
| // Cast to an array of integers from JSON | |
| if ( ! is_array( $event_ids ) ) { | |
| return new WP_REST_Response( | |
| array( | |
| 'message' => esc_html__( 'Event IDs must be provided as an array.', 'simple-events' ), | |
| ), | |
| 400 | |
| ); | |
| } | |
| $event_ids = array_map( 'intval', $event_ids ); | |
| // Validate the event IDs. |
🤖 Prompt for AI Agents
In src/classes/class-se-migrate-events.php around lines 199 to 201, after
decoding JSON into $event_ids, add a check to confirm $event_ids is an array
before applying array_map. If it is not an array, handle the error or initialize
$event_ids as an empty array to prevent runtime errors during processing.
1 participant
Changes proposed in this Pull Request
andse_event_date_end`Testing instructions
Mentions #
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores
Documentation