Skip to content

feat(block-theme): move co-authors RSS feed code to the Newspack Plugin#4629

Merged
laurelfulford merged 6 commits intotrunkfrom
feat/move-coauthors-rss-function
Apr 16, 2026
Merged

feat(block-theme): move co-authors RSS feed code to the Newspack Plugin#4629
laurelfulford merged 6 commits intotrunkfrom
feat/move-coauthors-rss-function

Conversation

@laurelfulford
Copy link
Copy Markdown
Contributor

@laurelfulford laurelfulford commented Apr 4, 2026

All Submissions:

Changes proposed in this Pull Request:

This PR and Automattic/newspack-theme#2671 remove functionality that adds Co-Authors to the RSS feed from the classic theme, and adds it to the Newspack Plugin. This makes it available regardless of theme used.

Closes NPPD-1345

How to test the changes in this Pull Request:

  1. Apply this PR and feat(block-theme): move coauthors RSS function to the Newspack Plugin newspack-theme#2671
  2. Set up some posts with a few different options: multiple authors, or guest authors.
  3. View the site's RSS feed (/feed in Chrome to view; the same downloads it in Firefox, which also works!) and search for <dc:creator> in the posts you created.
  4. Confirm that your bylines are respected (guest authors, multiple authors).

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a Newspack-side integration for the Co-Authors Plus plugin so RSS/feed author output can include all co-authors, and wires that integration into the main plugin include list.

Changes:

  • Added a Newspack\RSS_Feed integration that filters the_author in feed contexts to output co-authors.
  • Included the new integration file in Newspack::includes() so it loads with other Co-Authors Plus integrations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
includes/plugins/co-authors-plus/class-rss-feed.php Introduces a feed-specific the_author filter to return co-author output in feeds.
includes/class-newspack.php Loads the new Co-Authors Plus RSS integration during plugin includes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread includes/plugins/co-authors-plus/class-rss-feed.php Outdated
Comment thread includes/plugins/co-authors-plus/class-co-authors-plus-rss-feed.php
@laurelfulford laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Apr 4, 2026
@laurelfulford laurelfulford marked this pull request as ready for review April 4, 2026 01:17
@laurelfulford laurelfulford requested a review from a team as a code owner April 4, 2026 01:17
@laurelfulford laurelfulford changed the title feat: move co-authors RSS feed code to the Newspack Plugin feat(block-theme): move co-authors RSS feed code to the Newspack Plugin Apr 4, 2026
Copy link
Copy Markdown
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with Co-Authors Plus active, posts with single, two, and three co-authors. RSS feed correctly shows all co-authors in <dc:creator>. Frontend author display is unaffected.

The migration is well done – proper namespacing, sanitization (wp_strip_all_tags + html_entity_decode), and solid tests. See inline comments for a minor note on separator behavior.

}
return implode( ', ', array_map( fn( $author ) => wp_strip_all_tags( html_entity_decode( $author->display_name ) ), $coauthors ) );
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): the old coauthors() template tag defaults to and before the last author, so 3 co-authors produced "Alice, Bob and Carol". With implode it's now "Alice, Bob, Carol". Per A8C/WPCOM style guidance, the ideal format here would be an oxford comma: "Alice, Bob, and Carol". Not blocking on this, but worth considering – especially since this changes existing behavior for sites with 3+ co-authors on a post.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that, @adekbadek 🤦‍♀️ the formatting's been fixed in 2b92d2b.

public function test_returns_combined_coauthor_names_in_feed() {
$post_id = $this->factory->post->create();
$GLOBALS['_test_cap_coauthors'] = [
$this->make_coauthor( 'Jane Smith' ),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: consider adding a test case for 3+ co-authors. The current tests cover 0, 1, and 2 authors, but the separator behavior only diverges from the old implementation at 3+.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! Tests have been updated in 2b92d2b!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Apr 16, 2026
@laurelfulford
Copy link
Copy Markdown
Contributor Author

Thanks for the testing, @adekbadek! Nits have been fixed, mergin'!

@laurelfulford laurelfulford merged commit 50a160c into trunk Apr 16, 2026
11 checks passed
@laurelfulford laurelfulford deleted the feat/move-coauthors-rss-function branch April 16, 2026 22:39
@github-actions
Copy link
Copy Markdown

Hey @laurelfulford, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-changelog [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants