Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata Author Archive Builder: Add support for the Co-Authors Plus plugin #1159

Conversation

acicovic
Copy link
Collaborator

@acicovic acicovic commented Nov 2, 2022

Description

We recently got a bug report about the plugin generating PHP 8 warnings, but from what I could tell this was probably due to our Author_Archive_Builder class not supporting the Co-Authors Plus plugin.

In this PR:

  • We're adding support for the Co-Authors Plus plugin in our Author_Archive_Builder class.
  • We make the class more resistant/flexible to things that could go wrong.
  • If everything else fails, we use the author's username in the metadata. Previously, we would have an empty string, resulting in Author - metadata which was not helpful.

Credit: Thanks to @GaryJones for conducting the initial debugging investigation.

Motivation and Context

  • Support the Co-Authors Plus plugin properly.
  • Improve problematic/incomplete metadata that can be an issue for our users.

How Has This Been Tested?

Manual local testing was conducted, including:

  • Testing with the Co-Authors Plus plugin disabled.
  • Testing with the Co-Authors Plus plugin enabled:
    • Multiple authors (WP Users)
    • Multiple authors (Guest Authors)
    • add_filter( 'coauthors_guest_authors_force', '__return_true' );
    • add_filter( 'coauthors_guest_authors_enabled', '__return_false' );

@acicovic acicovic added this to the 3.6.0 milestone Nov 2, 2022
@acicovic acicovic requested a review from a team as a code owner November 2, 2022 06:45
@acicovic acicovic self-assigned this Nov 2, 2022
Copy link
Contributor

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

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

I'd love to see some tests that set up the test environment like CAP creates, to properly check that this bug is actually fixed. This will then also serve when the CAP integration is refactored to its own integration class.

The changes currently present look sensible but I've not tested, and with no automated tests, I can't approve.

Copy link
Contributor

@chriszarate chriszarate left a comment

Choose a reason for hiding this comment

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

I'd love to see some tests that set up the test environment like CAP creates, to properly check that this bug is actually fixed. This will then also serve when the CAP integration is refactored to its own integration class.

I'd love those tests, too. @acicovic Can you please add an issue to record this as a task? Given our limited resources and the impact of this issue, let's get this merged for now. Thanks, all.

@acicovic acicovic mentioned this pull request Nov 7, 2022
8 tasks
@acicovic
Copy link
Collaborator Author

acicovic commented Nov 7, 2022

I totally agree about the tests. This is now being tracked as part of #1161.

@acicovic acicovic merged commit 49a5818 into develop Nov 7, 2022
@acicovic acicovic deleted the add/support-co-authors-plus-in-metadata-author-archive-builder branch November 7, 2022 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants