901/improve feeds by jmarx · Pull Request #1146 · GatherPress/gatherpress

jmarx · 2025-08-02T17:29:53Z

Description of the Change

In this pr we are fixing the events feed so it returns events appropriately

Closes #901

How to test the Change

Visit /events/feed and ensure you are getting an rss/xml feed of events and not anything else(Comments or RSVPs).

Changelog Entry

Fixed - Bug

Checklist:

I agree to follow this project's Code of Conduct.
I have updated the documentation accordingly.
I have added tests to cover my change.
All new and existing tests pass.

what-the-diff · 2025-08-02T17:30:16Z

PR Summary

Introduction of Feed_Improvements Instance
A new instance for a feature named Feed_Improvements has been added in one of the primary setup files. This paves way for enhancing the overall feed features provided by the application.
Establishment of New Test File
In order to ensure the newly added Feed_Improvements functionality works as expected, a test file class-test-feed-improvements.php has been introduced. This will help us maintain the integrity of this feature by allowing regular checks via these tests.
Introduction of Test Methods for Feed_Improvements
Multiple testing methods have been implemented for checking various aspects of Feed_Improvements:
- Verifying if Feed_Improvements successfully retains only a single active instance at any given time.
- Check on force_events_feed_query method to ensure both event feeds and non-event feeds are handled as intended.
- Assessment of functionality to customize event and non-event posts via customize_event_excerpt and customize_event_content methods.
- Test to check the operation of get_event_datetime_info across a variety of scenarios including instances with no date and time information.

mauteri

@carstingaxion mind reviewing and adding some feedback on this too? Thx!

mauteri · 2025-08-02T19:20:37Z

includes/core/classes/class-feed-improvements.php

+ *
+ * @since 1.0.0
+ */
+class Feed_Improvements {


Change name to Event_Feed and class-event-feed.php and make additional updates that match this change.

@mauteri Done.

mauteri · 2025-08-02T19:27:31Z

includes/core/classes/class-feed-improvements.php

+		if ( ! get_option( 'gatherpress_events_feed_rewrite_flushed' ) ) {
+			flush_rewrite_rules();
+			update_option( 'gatherpress_events_feed_rewrite_flushed', true );
+		}


Let's get rid of this and handle updates to rewrite rules that @carstingaxion put in here #880. We'll make this ticket part of 0.33

@mauteri Done

mauteri · 2025-08-02T19:36:50Z

includes/core/classes/class-feed-improvements.php

+	}
+
+
+


Remove extra line breaks, should just be one.

@mauteri Done

mauteri · 2025-08-02T19:37:52Z

includes/core/classes/class-feed-improvements.php

+	 * @param \WP $wp WordPress environment instance.
+	 * @return void
+	 */
+	public function intercept_events_feed( \WP $wp ): void {


add use WP and only have WP here.

@mauteri Removed this method so it doesn't matter.

mauteri · 2025-08-02T19:38:25Z

includes/core/classes/class-feed-improvements.php

+				// Also try to force the main query to be an events query.
+				add_action(
+					'wp',
+					function () {


static function (): void {

@mauteri Removed this method so oit doesn't matter.

mauteri · 2025-08-02T19:58:47Z

includes/core/classes/class-feed-improvements.php

+	 * @param Event $event The event object.
+	 * @return array Array of event information strings.
+	 */
+	private function get_event_datetime_info( Event $event ): array {


This method is duplicating functionality you can get from the event you are passing to it. Look at class-event.php and you'll find methods like is_same_date, get_display_datetime, get_formatted_datetime and others you can use rather than calculating all this below.

@mauteri Done. Using the existing logic now

mauteri · 2025-08-02T19:59:25Z

includes/core/classes/class-feed-improvements.php

+	}
+
+
+


remove extra line breaks, just 1

@mauteri Done

mauteri · 2025-08-02T20:07:22Z

includes/core/classes/class-feed-improvements.php

+	 *
+	 * @return void
+	 */
+	public function force_events_feed(): void {


Look at prepare_event_query_before_execution method in class-event-query.php. You might be able to have that method handle for feeds too so we don't have to pre_get_posts hooks that are ordering events and could change independently causing issues and bugs.

@mauteri Done

mauteri · 2025-08-02T20:09:35Z

includes/core/classes/class-feed-improvements.php

+
+				// Remove any comment-related query vars.
+				$query->set( 'comments', false );
+				$query->set( 'comment_feed', false );


Why removing comments?

@mauteri I had a hard time with this one. Let's discuss. The comments feed kept overwriting the events feed. And that url would be nothing but comments. Maybe we handle this separately. Or maybe you have ideas.

@mauteri added comments to the feed here:
https://github.com/GatherPress/gatherpress/pull/1146/files#diff-318af0ae6b17ab115e37e8dfd22e0de9a705de64bdd372fbec5892d474a51e6eR224

I would look at how comments are applied to feeds for regular posts and follow that. There's a callback hook on pre_get_comments in GatherPress that will prevent including RSVPs in with it, so you don't need to worry about filtering those out.

@mauteri yeah I think that filter is applied to get_comments (which I am using there) I put in some RSVPs and don't see them in the comment feed. Is that what you're concerned about?

mauteri · 2025-08-02T20:11:28Z

includes/core/classes/class-feed-improvements.php

+	 * @param \WP_Query $query The WP_Query instance.
+	 * @return void
+	 */
+	public function force_events_feed_query( \WP_Query $query ): void {


Why are there 2 pre_get_posts callbacks? I would leverage what is in class-event-query.php to handle feeds as well and make sure they are upcoming.

@mauteri Done

mauteri · 2025-08-02T21:23:23Z

includes/core/classes/class-feed-improvements.php

+	public function add_events_feed_rewrite_rules(): void {
+		// Add a rewrite rule for /events/feed/ to be treated as an events feed.
+		add_rewrite_rule(
+			'^events/feed/?$',


I feel like the slug should be a setting (at least eventually)

@mauteri Let's handle separately

@mauteri I handled this as well. I didn't realize that setting was already available.

jmarx · 2025-08-02T23:57:37Z

@mauteri I handled most of your review comments. But we still have another couple open issues. Can you please take another look and confirm the ones I handled were adequate?

mauteri · 2025-08-03T15:09:19Z

@mauteri I handled most of your review comments. But we still have another couple open issues. Can you please take another look and confirm the ones I handled were adequate?

@jmarx I'd like @carstingaxion to look at this next and provide additional feedback. Thx!

carstingaxion · 2025-08-03T15:32:12Z

@mauteri I handled most of your review comments. But we still have another couple open issues. Can you please take another look and confirm the ones I handled were adequate?

@jmarx I'd like @carstingaxion to look at this next and provide additional feedback. Thx!

I’ll be available in 27 minutes and will start with this. Thanks for your patience.

carstingaxion · 2025-08-03T17:30:52Z

includes/core/classes/class-event-feed.php

+		return $custom_content;
+	}
+
+	/**


What is the purpose of that method?

It seems (to me) to replicate, what register_post_type() is already doing?

If there is a real reason for having it, it might be better to build the URL based on the gatherpress_general['urls']['events'] setting and not with the hardcoded 'events' string.

@carstingaxion I am now using settings for building the url. The purpose of the method is to customize output of the event content for the purposes of RSS feed.

Hi @jmarx,

I'm sorry that I need to ask again: Why are you creating that new rewrite_rule?
I don't get it.

Before:

After:

@carstingaxion
Sorry, I completely misunderstood your comment/question. I get what you're saying now. I removed the redundant rewrite rule.

carstingaxion · 2025-08-03T17:32:06Z

includes/core/classes/class-event-feed.php

+			// Check if this is /events/feed/.
+			if ( strpos( $request_uri, '/events/feed' ) !== false ) {


It might be better to build the URL based on the gatherpress_general['urls']['events'] setting and not with the hardcoded 'events' string.

@carstingaxion I am using the setting now.

carstingaxion · 2025-08-03T17:50:16Z

includes/core/classes/class-event-feed.php

+		// Customize RSS excerpts for events.
+		add_filter( 'the_excerpt_rss', array( $this, 'customize_event_excerpt' ) );
+		add_filter( 'the_content_feed', array( $this, 'customize_event_content' ) );


I'm not really happy with that hard replacements at all and would like to keep some more flexibility for the editor and or theme designer.

I haven't tested this, but I'd like to try hooking into get_template_part_slug or later and provide a default template from within GatherPress. This template could be overwrittren by a theme or within the block-editor or both.

Maybe even better would be using register_block_template() which allows to hook into the default template hierachy, just by setting the $template_name correctly to match a default template.

In our case this should be "gatherpress/feed-gatherpress_event" I think.

Using this, GatherPress could provide a default different from wp core and allow theme designers to adjust the template (even inside the block editor).

@carstingaxion OK, I implemented some filters to make sure it can be overwritten if need be:
0608028

carstingaxion · 2025-08-03T18:31:06Z

Hello @jmarx,

thanks for starting the journey on this ticket!

I left some review comments and hope that you can understand.

In general, it would help me to understand, if you could provide some basic PR description and what is changing;
including some Testing instructions on what to try and what to assert.

… to overwrite if need be

jmarx · 2025-08-10T18:27:05Z

@mauteri Removed all the unneeded code. The feed should be using the standard wordpress format now.

mauteri · 2025-08-10T18:30:00Z

includes/core/classes/class-event-feed.php

+		$event_info       = array();
+		$display_datetime = $event->get_display_datetime();
+
+		if ( ! empty( $display_datetime ) && __( '—', 'gatherpress' ) !== $display_datetime ) {


You missed this.

if ( ! empty( $display_datetime ) && '—' !== $display_datetime ) {

mauteri · 2025-08-10T18:31:26Z

@mauteri Removed all the unneeded code. The feed should be using the standard wordpress format now.

@jmarx Okay thanks. You missed one thing, so please make that change. Also, I'd like @carstingaxion to have another look at this and approve if he thinks it's good now since he put in the original ticket.

carstingaxion · 2025-08-10T19:04:49Z

includes/core/classes/class-event-feed.php

+			$request_uri = sanitize_text_field( wp_unslash( $_SERVER['REQUEST_URI'] ) );
+
+			// Check if this is the events feed URL.
+			if ( str_contains( $request_uri, '/' . $this->rewrite_slug . '/feed' ) ) {


The word "feed" can also be changed, so make sure to use feed_base from the global $wp_rewrite.

https://developer.wordpress.org/reference/classes/wp_rewrite/#properties

carstingaxion · 2025-08-10T19:07:15Z

includes/core/classes/class-event-feed.php

+		}
+
+		// Allow themes and editors to customize the excerpt.
+		return apply_filters( 'gatherpress_event_feed_excerpt', $excerpt );


Shouldn't phpcs complain about the missing doc-block here?
I suggest adding some more info here.

carstingaxion · 2025-08-10T19:07:45Z

includes/core/classes/class-event-feed.php

+		}
+
+		// Allow themes and editors to customize the content.
+		return apply_filters( 'gatherpress_event_feed_content', $content );


Please add some docs about using this filter, too.

carstingaxion · 2025-08-10T19:10:18Z

includes/core/classes/class-event-feed.php

+	 * @return string The customized content.
+	 */
+	public function apply_event_content( string $content ): string {
+		global $post;


This global can be omitted, because get_post_type() will use the global as its default, if nothing else is given.

https://developer.wordpress.org/reference/functions/get_post_type/

jmarx · 2025-08-11T00:19:23Z

@mauteri @carstingaxion I think we should be good now?

carstingaxion

In addition to what you already prepared @jmarx , I’d like to suggest to improve all feed endpoints where events are queried. Even this was not part of the initial issue, I mentioned this in a comment on the same issue: #901 (comment)

gatherpress_event post type archive feed
gatherpress_topic taxonomy archive feed
_gatherpress_venue taxonomy archive feed

jmarx · 2025-08-11T13:49:43Z

In addition to what you already prepared @jmarx , I’d like to suggest to improve all feed endpoints where events are queried. Even this was not part of the initial issue, I mentioned this in a comment on the same issue: #901 (comment)

gatherpress_event post type archive feed

gatherpress_topic taxonomy archive feed

_gatherpress_venue taxonomy archive feed

@carstingaxion Agreed. But should we not do that in a follow up ticket? To keep these prs/chunks of work, tight?
I'll leave that up to you and @mauteri

jmarx · 2025-08-11T13:51:00Z

@carstingaxion I suggest we have Andrew Short create follow up tickets for those feeds. And we can use this as model.
cc @mauteri

carstingaxion · 2025-08-14T14:34:01Z

includes/core/classes/class-event-feed.php

+	 * @return string The customized excerpt.
+	 */
+	public function get_default_event_excerpt( string $excerpt ): string {
+		global $post;


Suggested change

global $post;

if ( Event::POST_TYPE !== get_post_type() ) {

return $excerpt;

}

I already mentioned this before, but I guess you may have missed this @jmarx

I would remove the global $post and rely on get_post_type() falling back to the global internaly.

@carstingaxion Fixed. I swear I tried this and it was not working before. But it's good now.

@jmarx you still have the global $post and have $post passing as an argument.

carstingaxion · 2025-08-14T14:34:42Z

includes/core/classes/class-event-feed.php

+	 * @return string The customized content.
+	 */
+	public function get_default_event_content( string $content ): string {
+		global $post;


Same as above: I would remove the global $post and rely on get_post_type() falling back to the global internaly.

@carstingaxion Fixed

carstingaxion · 2025-08-14T14:35:13Z

includes/core/classes/class-event-feed.php

+	 * @return string The customized excerpt.
+	 */
+	public function apply_event_excerpt( string $excerpt ): string {
+		global $post;


Same as above: I would remove the global $post and rely on get_post_type() falling back to the global internaly.

@carstingaxion Fixed

carstingaxion · 2025-08-14T14:36:04Z

includes/core/classes/class-event-feed.php

+	public function apply_event_content( string $content ): string {
+
+		// Only apply to events.
+		if ( Event::POST_TYPE !== get_post_type( get_the_ID() ) ) {


Almost the same as above: I would remove the get_the_ID() and rely on get_post_type() falling back to the global internaly.

@carstingaxion fixed

mauteri · 2025-08-14T23:37:13Z

includes/core/classes/class-event-feed.php

+	 * @return string The customized excerpt.
+	 */
+	public function get_default_event_excerpt( string $excerpt ): string {
+		global $post;


@jmarx you still have the global $post and have $post passing as an argument.

mauteri · 2025-08-14T23:41:35Z

includes/core/classes/class-event-feed.php

+	protected function __construct() {
+		// Initialize the rewrite slug from settings.
+		$settings           = Settings::get_instance();
+		$this->rewrite_slug = $settings->get_value( 'general', 'urls', 'events' );


@jmarx this is your text domain issue. You're only using the rewrite_slug once so no need to make it a property of the class. Just add this code where you use it and just make it a regular variable.

mauteri · 2025-08-14T23:42:54Z

includes/core/classes/class-setup.php

+	 *
+	 * @return void
+	 */
+	public function instantiate_event_feed(): void {


@jmarx you don't need to do this (see my comment above). You can put it back where singletons initialize. You need to move that code out of the constructor and into an init hook.

mauteri · 2025-08-14T23:43:37Z

test/unit/php/includes/tests/core/classes/class-test-event-feed.php

+ * @coversDefaultClass \GatherPress\Core\Event_Feed
+ */
+class Test_Event_Feed extends Base {
+


Remove line break.

jmarx · 2025-08-15T12:18:13Z

@mauteri @carstingaxion Sorry for the sloppiness there. Can you check again?

mauteri · 2025-08-15T12:15:47Z

includes/core/classes/class-event-feed.php

-	 * @var string
-	 */
-	private $rewrite_slug;
+


Remove extra line breaks

mauteri · 2025-08-15T12:16:18Z

includes/core/classes/class-event-feed.php

 	 */
 	public function get_default_event_excerpt( string $excerpt ): string {
-		global $post;



Remove extra line break

mauteri · 2025-08-15T12:16:35Z

includes/core/classes/class-setup.php

-	 * @return void
-	 */
-	public function instantiate_event_feed(): void {
 		Event_Feed::get_instance();


Put in alphabetical order

@mauteri Done

mauteri · 2025-08-15T12:18:36Z

includes/core/classes/class-event-feed.php

+	 * @return string The customized content.
+	 */
+	public function apply_event_content( string $content ): string {
+


Remove extra line break

@mauteri Done

github-actions · 2025-08-29T19:03:10Z

Preview changes with Playground

You can preview the recent changes for PR#1146 with the following PHP versions:

PHP Version 8.3

PHP Version 7.4

Download .zip with build changes

Made with 💙 from GatherPress & a little bit of WordPress Playground. Changes will not persist between sessions.

jmarx added 3 commits August 2, 2025 12:35

WIP: Improve feeds

37fcb9d

Update test

a7a3c64

Update test file again

a555d38

jmarx added 4 commits August 2, 2025 13:36

Satisfy linter errors

3a28df0

More linter issues and removal of unused code

02d88a5

More linter errors

34be562

Another one

7310a2c

jmarx marked this pull request as ready for review August 2, 2025 18:27

jmarx requested a review from mauteri August 2, 2025 18:27

mauteri requested a review from carstingaxion August 2, 2025 19:28

mauteri requested changes Aug 2, 2025

View reviewed changes

mauteri reviewed Aug 2, 2025

View reviewed changes

jmarx added 4 commits August 2, 2025 19:23

Code review changes

4b43089

more linter errors

f962f89

Alignment issue

11d3b9f

No longer need this

7bb491a

jmarx requested a review from mauteri August 2, 2025 23:56

jmarx added 4 commits August 2, 2025 20:00

Remove mroe spaces

e53a450

Remove more spaces

eb3f6e9

Add comments to feed

a889914

Fix linter

a4b2b09

carstingaxion suggested changes Aug 3, 2025

View reviewed changes

jmarx added 3 commits August 3, 2025 14:33

Use settings for event feed urls

5b8a048

Linter issue

06b53ec

Use filters to apply feed changes to maintain flexibility for editors…

0608028

… to overwrite if need be

Linter issues

ffc3eaa

jmarx requested a review from mauteri August 10, 2025 18:25

mauteri requested changes Aug 10, 2025

View reviewed changes

carstingaxion suggested changes Aug 10, 2025

View reviewed changes

CR changes

455fe75

jmarx requested review from carstingaxion and mauteri August 11, 2025 00:19

carstingaxion reviewed Aug 11, 2025

View reviewed changes

jmarx requested a review from carstingaxion August 11, 2025 13:51

carstingaxion suggested changes Aug 14, 2025

View reviewed changes

Remove global reference

8fe9813

mauteri requested changes Aug 14, 2025

View reviewed changes

CR changes

fe24ab0

jmarx requested a review from mauteri August 15, 2025 12:18

mauteri requested changes Aug 15, 2025

View reviewed changes

jmarx added 2 commits August 15, 2025 08:24

More things

e5fb89a

Line break

58181fc

jmarx requested a review from mauteri August 15, 2025 12:34

jmarx added 2 commits August 23, 2025 14:49

Remove line break

fac862a

Rename Class to Feed

0a96694

mauteri approved these changes Sep 10, 2025

View reviewed changes

mauteri merged commit 06d6ff8 into develop Sep 10, 2025
16 checks passed

mauteri deleted the 901/improve-feeds branch September 10, 2025 14:27

		// Check if this is /events/feed/.
		if ( strpos( $request_uri, '/events/feed' ) !== false ) {

-		global $post;
+		if ( Event::POST_TYPE !== get_post_type() ) {
+			return $excerpt;
+		}

Conversation

jmarx commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the Change

How to test the Change

Changelog Entry

Checklist:

Uh oh!

what-the-diff bot commented Aug 2, 2025

PR Summary

Uh oh!

mauteri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mauteri Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarx commented Aug 2, 2025

Uh oh!

mauteri commented Aug 3, 2025

Uh oh!

carstingaxion commented Aug 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarx commented Aug 2, 2025 •

edited

Loading

mauteri Aug 2, 2025 •

edited

Loading

carstingaxion Aug 3, 2025 •

edited

Loading