Conversation
… in next/previous links
… in next/previous links
|
Caution Review failedThe pull request is closed. WalkthroughInitializes a local $fallback_template = '' in SE_Template_Loader::template_include to ensure the variable is defined. Adds se_get_date_ids_for_non_published_events() to collect and per-request cache IDs of event-date posts whose parent events are not published. se_event_get_next_event() and se_event_get_previous_event() are updated to set post__not_in using that helper and, when grouping is disabled, to merge the current event’s date IDs into the exclusion list. No public function signatures changed. Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/template-functions.php (2)
441-444: Deduplicate and simplify ID extraction when grouping is disabledMinor improvement: avoid duplicate IDs and simplify extraction using
wp_list_pluck.Apply this diff:
- $args['post__not_in'] = array_merge( - $args['post__not_in'], - array_map( fn( array $date ): int => $date['id'], se_event_get_event_dates( $event_id ) ) - ); + $args['post__not_in'] = array_unique( + array_merge( + $args['post__not_in'], + wp_list_pluck( se_event_get_event_dates( $event_id ), 'id' ) + ) + );
510-513: Deduplicate and simplify ID extraction when grouping is disabled (previous-event)Same improvement as for next-event.
Apply this diff:
- $args['post__not_in'] = array_merge( - $args['post__not_in'], - array_map( fn( array $date ): int => $date['id'], se_event_get_event_dates( $event_id ) ) - ); + $args['post__not_in'] = array_unique( + array_merge( + $args['post__not_in'], + wp_list_pluck( se_event_get_event_dates( $event_id ), 'id' ) + ) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/classes/class-se-template-loader.php(1 hunks)src/template-functions.php(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/template-functions.php (2)
src/event-functions.php (1)
se_event_get_event_dates(718-758)src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-735)
🔇 Additional comments (3)
src/classes/class-se-template-loader.php (1)
74-76: Good fix: initialize $fallback_template to avoid undefined-variable noticesInitializing
$fallback_templateprevents notices when neither singular nor archive fallbacks are set by earlier branches. The later guard on Line 121 ensures we only return a resolved fallback when non-empty.src/template-functions.php (2)
436-438: Correctly excludes non-published events from next-event querySetting
$args['post__not_in']usingse_get_date_ids_for_non_published_events()aligns with the PR objective and prevents non-published parent events from appearing in pagination.
505-507: Correctly excludes non-published events from previous-event queryMirrors the next-event fix and prevents non-published parents from appearing in "previous" pagination. Matches the test scenarios described in the PR.
| function se_get_date_ids_for_non_published_events() { | ||
| static $dates = null; | ||
| if ( is_array( $dates ) ) { | ||
| return $dates; | ||
| } | ||
|
|
||
| // Get all events that not published (draft or pending or private). | ||
| $args = array( | ||
| 'post_type' => SE_Event_Post_Type::$post_type, | ||
| 'post_status' => array_diff( get_post_stati(), array( 'publish' ) ), | ||
| 'posts_per_page' => -1, | ||
| 'fields' => 'ids', | ||
| ); | ||
| $draft_dates = get_posts( $args ); | ||
|
|
||
| $dates = array(); | ||
|
|
||
| foreach ( $draft_dates as $draft_date ) { | ||
| // Get all dates for this event. | ||
| $event_dates = se_event_get_event_dates( $draft_date ); | ||
| if ( ! empty( $event_dates ) ) { | ||
| foreach ( $event_dates as $date ) { | ||
| $dates[] = $date['id']; | ||
| } | ||
| } | ||
| } | ||
| return $dates; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential gap: private parent events may still leak without sufficient capabilities; consider a capability-agnostic approach
The current helper relies on get_posts() for parent events with post_status != 'publish'. Depending on the current user's capabilities, queries for post_status = 'private' can return no results, which would miss those parents and allow their published date children to slip into pagination.
Proposed robust approach (avoids capability checks and is a single, efficient JOIN):
function se_get_date_ids_for_non_published_events() {
- static $dates = null;
- if ( is_array( $dates ) ) {
- return $dates;
- }
-
- // Get all events that not published (draft or pending or private).
- $args = array(
- 'post_type' => SE_Event_Post_Type::$post_type,
- 'post_status' => array_diff( get_post_stati(), array( 'publish' ) ),
- 'posts_per_page' => -1,
- 'fields' => 'ids',
- );
- $draft_dates = get_posts( $args );
-
- $dates = array();
-
- foreach ( $draft_dates as $draft_date ) {
- // Get all dates for this event.
- $event_dates = se_event_get_event_dates( $draft_date );
- if ( ! empty( $event_dates ) ) {
- foreach ( $event_dates as $date ) {
- $dates[] = $date['id'];
- }
- }
- }
- return $dates;
+ static $dates = null;
+ if ( is_array( $dates ) ) {
+ return $dates;
+ }
+
+ global $wpdb;
+ $event_pt = SE_Event_Post_Type::$post_type;
+ $date_pt = SE_Event_Post_Type::$event_date_post_type;
+
+ // Fetch IDs of published event-date posts whose parent events are not published.
+ $sql = "
+ SELECT d.ID
+ FROM {$wpdb->posts} AS d
+ INNER JOIN {$wpdb->posts} AS e ON e.ID = d.post_parent
+ WHERE d.post_type = %s
+ AND d.post_status = 'publish'
+ AND e.post_type = %s
+ AND e.post_status <> %s
+ ";
+ // Capability-agnostic: direct SQL ignores current user's read_private_* caps.
+ $prepared = $wpdb->prepare( $sql, $date_pt, $event_pt, 'publish' );
+ $dates = array_map( 'intval', $wpdb->get_col( $prepared ) );
+
+ return $dates;
}Follow-up: If you prefer to avoid SQL, an alternative is to query all published se-event-date IDs and filter by parent status with get_post_status( $parent_id ) !== 'publish'. This is simpler but can be heavier on large datasets.
To verify the private-event case in a non-admin session:
- Create E1 (Private), with published date(s).
- Create E2 (Publish) dated after E1.
- As a logged-out user, view E2 and confirm E1 never appears in next/previous links.
🌐 Web query:
How does WP_Query handle private post_status for users without read_private_posts capability?
💡 Result:
If a user does not have the read_private_posts capability, WP_Query will exclude posts with the private post_status from query results for that user. Only users with the correct capability (such as Administrators and Editors by default) will see private posts; other users and guests cannot see them at all[1][2][8].
Key Behavior:
- WP_Query will only return private posts to users who have
read_private_postsfor the queried post type[2][8]. - For users without this capability, private posts are omitted from both frontend and backend queries and are effectively invisible to those users[2][1].
- When querying multiple post types or using
anyfor post types, older versions of WordPress (pre-5.9) had issues where private posts were excluded indiscriminately; the correct behavior is to check the user’s capability per post type and only include private posts for types where the user has permission[3].
Technical Details:
- WP_Query checks capabilities for the logged-in user and post type in the query. If
post_statusis set toprivate, it confirms the user canread_private_posts; if not, those posts are excluded from results[8][3]. - The
pre_get_postsfilter can be used to customize which users see private posts, but you must check the user capability first—usually withcurrent_user_can('read_private_posts')for the relevant post type[4]. - This capability mapping also extends to custom post types, where the developer must ensure the right capabilities are associated and mapped for private posts[5][8].
Summary Table:
| User Role | Can See Private Posts (private status) |
|---|---|
| Administrator / Editor | Yes |
| Author | Only their own |
| Contributor, Subscriber, Guest | No |
So, WP_Query enforces the visibility of private posts strictly based on user capabilities. Users without read_private_posts will never see private posts in query results, regardless of query arguments, unless custom query manipulation circumvents the check (which is not standard practice)[1][2][8][4].
Citations:
- 1: https://publishpress.com/knowledge-base/read_private_posts/
- 2: https://publishpress.com/blog/wordpress-publishing/private-posts/
- 3: https://core.trac.wordpress.org/ticket/48556
- 4: https://wordpress.org/support/topic/include-private-posts-from-specific-categories-to-the-main-query/
- 5: https://www.greengeeks.com/glossary/capability/
- 6: https://developer.wordpress.org/reference/functions/wp_count_posts/
- 7: https://publishpress.com/knowledge-base/edit_private_posts/
- 8: https://wpmayor.com/roles-capabilities-wordpress/
- 9: https://wpvip.com/blog/wp-query-performance/
- 10: https://www.billerickson.net/code/wp_query-arguments/
Ensure Private Events’ Dates Are Always Excluded, Regardless of User Capabilities
WP_Query (and thus get_posts()) will automatically omit private posts for users without the read_private_posts capability. As a result, private parent events may be skipped by your draft query, yet their published date children will still be returned—leaking those dates into pagination.
What to do:
- Replace the two-step get_posts() + se_event_get_event_dates() loop with a single capability-agnostic query.
- You can either use direct SQL (ignoring WP_Query’s capability checks) or stick to core functions by filtering published date posts by their parent’s status.
Suggested SQL-based refactor in src/template-functions.php:
function se_get_date_ids_for_non_published_events() {
- static $dates = null;
- if ( is_array( $dates ) ) {
- return $dates;
- }
-
- // Get all events that not published (draft or pending or private).
- $args = array(
- 'post_type' => SE_Event_Post_Type::$post_type,
- 'post_status' => array_diff( get_post_stati(), array( 'publish' ) ),
- 'posts_per_page' => -1,
- 'fields' => 'ids',
- );
- $draft_dates = get_posts( $args );
-
- $dates = array();
-
- foreach ( $draft_dates as $draft_date ) {
- // Get all dates for this event.
- $event_dates = se_event_get_event_dates( $draft_date );
- if ( ! empty( $event_dates ) ) {
- foreach ( $event_dates as $date ) {
- $dates[] = $date['id'];
- }
- }
- }
- return $dates;
+ static $dates = null;
+ if ( is_array( $dates ) ) {
+ return $dates;
+ }
+
+ global $wpdb;
+ $event_pt = SE_Event_Post_Type::$post_type;
+ $date_pt = SE_Event_Post_Type::$event_date_post_type;
+
+ // Get published dates whose parent events are not published—ignores WP_Query caps.
+ $sql = "
+ SELECT d.ID
+ FROM {$wpdb->posts} AS d
+ INNER JOIN {$wpdb->posts} AS e ON e.ID = d.post_parent
+ WHERE d.post_type = %s
+ AND d.post_status = 'publish'
+ AND e.post_type = %s
+ AND e.post_status <> 'publish'
+ ";
+ $prepared = $wpdb->prepare( $sql, $date_pt, $event_pt );
+ $dates = array_map( 'intval', $wpdb->get_col( $prepared ) );
+
+ return $dates;
}Alternative PHP-only approach (may be heavier on large data sets):
function se_get_date_ids_for_non_published_events() {
static $dates = null;
if ( is_array( $dates ) ) {
return $dates;
}
// 1) Get all published date IDs.
$all_dates = get_posts([
'post_type' => SE_Event_Post_Type::$event_date_post_type,
'post_status' => 'publish',
'posts_per_page' => -1,
'fields' => 'ids',
]);
// 2) Filter out those whose parent event is still published.
$dates = array_filter( $all_dates, function( $date_id ) {
return get_post_status( wp_get_post_parent_id( $date_id ) ) !== 'publish';
});
return array_map( 'intval', $dates );
}Please update the code in src/template-functions.php (around lines 539–566) to one of these implementations to prevent private or draft parent events from leaking their published dates.
🤖 Prompt for AI Agents
In src/template-functions.php around lines 539–566, the current get_posts()
query can miss private parent events (due to capability checks) and leak their
published child dates; replace the two-step draft-events loop with a
capability-agnostic approach: fetch all published event_date posts
(posts_per_page => -1, fields => 'ids'), then filter those IDs by checking their
parent event status with get_post_status( wp_get_post_parent_id( $date_id ) )
!== 'publish' so only dates whose parent is non-published remain, cast IDs to
int, keep the static cache and return the resulting array; alternatively, if
performance is a concern for large sites, implement an equivalent direct SQL
query that joins post_parent to post_status to select published date IDs whose
parent is not 'publish'.
There was a problem hiding this comment.
This wont leak into pagination as we only get the parent posts, so its not an issue. This is unlikely to be an issue.
There was a problem hiding this comment.
@gin0115 Understood! You know your system's usage patterns best. If private parent events aren't a common scenario in your implementation and the current approach serves your needs effectively, then the existing logic is fine.
Thanks for clarifying - I'll keep this context in mind for future reviews of this codebase.
✏️ Learnings added
Learnt from: gin0115
PR: a8cteam51/simple-events#36
File: src/template-functions.php:543-570
Timestamp: 2025-08-13T13:19:14.673Z
Learning: In the simple-events codebase, the maintainer gin0115 has indicated that capability-based filtering concerns for private parent events in pagination are not a priority, as the current get_posts() approach for fetching non-published parent events is sufficient for their use case.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/template-functions.php (1)
539-566: Private parents can leak their published dates to anonymous users; make the helper capability-agnosticAs previously noted, relying on get_posts() for non-published parents is subject to capability filtering. For users without read_private_posts, private events won’t be returned, allowing their published date children to slip into pagination.
Replace the two-step get_posts() + loop with a direct SQL join that ignores capability checks and selects published date IDs whose parent is not publish.
Apply this diff:
function se_get_date_ids_for_non_published_events() { static $dates = null; if ( is_array( $dates ) ) { return $dates; } - // Get all events that not published (draft or pending or private). - $args = array( - 'post_type' => SE_Event_Post_Type::$post_type, - 'post_status' => array_diff( get_post_stati(), array( 'publish' ) ), - 'posts_per_page' => -1, - 'fields' => 'ids', - ); - $draft_dates = get_posts( $args ); - - $dates = array(); - - foreach ( $draft_dates as $draft_date ) { - // Get all dates for this event. - $event_dates = se_event_get_event_dates( $draft_date ); - if ( ! empty( $event_dates ) ) { - foreach ( $event_dates as $date ) { - $dates[] = $date['id']; - } - } - } - return $dates; + global $wpdb; + $event_pt = SE_Event_Post_Type::$post_type; + $date_pt = SE_Event_Post_Type::$event_date_post_type; + + // Fetch IDs of published date posts whose parent events are not published (capability-agnostic). + $sql = " + SELECT d.ID + FROM {$wpdb->posts} AS d + INNER JOIN {$wpdb->posts} AS e ON e.ID = d.post_parent + WHERE d.post_type = %s + AND d.post_status = 'publish' + AND e.post_type = %s + AND e.post_status <> 'publish' + "; + $prepared = $wpdb->prepare( $sql, $date_pt, $event_pt ); + $dates = array_map( 'intval', $wpdb->get_col( $prepared ) ); + + return $dates; }Suggested manual verification (especially for “Private” parent case):
- Create E1 (Private), add date(s) and ensure the date posts are Publish.
- Create E2 (Publish) dated after E1 and E3 (Publish) dated before E1.
- As a logged-out user, view E2/E3 and confirm E1’s dates never appear in next/previous links.
🧹 Nitpick comments (3)
src/template-functions.php (3)
436-444: Deduplicate post__not_in to avoid redundant IDsMerging the arrays can introduce duplicates; while harmless, it adds SQL overhead and noise. Deduplicate after merge.
Apply this diff:
- $args['post__not_in'] = array_merge( - $args['post__not_in'], - array_map( fn( array $date ): int => $date['id'], se_event_get_event_dates( $event_id ) ) - ); + $args['post__not_in'] = array_values( array_unique( array_merge( + $args['post__not_in'], + array_map( static fn( array $date ): int => $date['id'], se_event_get_event_dates( $event_id ) ) + ) ) );
506-513: Deduplicate post__not_in here as wellSame rationale as the “next” query: ensure the exclusion list is unique.
Apply this diff:
- $args['post__not_in'] = array_merge( - $args['post__not_in'], - array_map( fn( array $date ): int => $date['id'], se_event_get_event_dates( $event_id ) ) - ); + $args['post__not_in'] = array_values( array_unique( array_merge( + $args['post__not_in'], + array_map( static fn( array $date ): int => $date['id'], se_event_get_event_dates( $event_id ) ) + ) ) );
436-438: Optional: consider excluding by parent IDs to keep NOT IN lists smallerIf the site accumulates many unpublished events with many dates, enumerating all child date IDs can bloat the NOT IN clause. Excluding by parent can be leaner:
- Change helper to return non-published parent event IDs.
- Use post_parent__not_in instead of post__not_in.
This is optional and can be deferred; the current approach is correct.
Also applies to: 506-507
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/template-functions.php(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/template-functions.php (2)
src/event-functions.php (1)
se_event_get_event_dates(718-758)src/classes/class-se-event-post-type.php (1)
SE_Event_Post_Type(14-735)
1 participant
Changes proposed in this Pull Request
Testing instructions
When you are E2, NO next event, but the previous should be E3
When you are on E3, Next should be E2 and no pevious.
E1 should never show, unless made public
Mentions #35
Summary by CodeRabbit