fix: authors and guest authors in homepage posts

[dkoo]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Fixes four bugs related to author querying:

1. A SQL syntax error when filtering Homepage Posts blocks by authors and including at least one Co-Authors Plus guest author
2. When filtering Homepage Posts blocks by CAP guest authors, posts by linked user accounts are not included in the results
3. An unhandled include query param when fetching saved authors to display in the block sidebar controls
4. When filtering by a WP user, posts that were created by that user but are attributed to an unrelated CAP guest author are also fetched

How to test the changes in this Pull Request:

SQL syntax error and linked accounts

1. On master, add several Homepage Posts blocks to a post and filter them by a regular WP user, a guest author with no linked account, and a guest author with a linked account. For ease of testing, use the same authors in all blocks and add separators between block instances so you can easily see where one block ends and the next begins.
2. Observe that the posts shown in the editor vs. the front-end are different, and that the front-end blocks in particular don't show all of the posts you would expect:

• Likely only the first block instance will show any posts at all.
• For the guest author with linked account, posts that are attributed to the linked account are not shown—only posts directly attributed to the guest author.
• In the debug log, you'll probably see a SQL syntax error similar to this:

WordPress database error Not unique table/alias: 'wp_term_relationships' for query SELECT SQL_CALC_FOUND_ROWS  wp_posts.ID FROM wp_posts  LEFT JOIN wp_term_relationships ON (wp_posts.ID = wp_term_relationships.object_id)

3. Check out this branch and refresh both editor and front-end. Confirm that both environments show all the posts you would expect and that the posts match in editor vs. front-end. Confirm that the issues outlined in step 4 no longer apply.

Saved authors in block sidebar controls

1. Switch back to master and refresh the editor with your test blocks.
2. Select one of the blocks and expand the "Display Settings" sidebar. Observe that in the Authors field, not all of the authors you had saved before are shown here.
3. Check out this branch and refresh. Confirm that you now see all WP users and guest authors in the Authors field.

Unrelated CAP guest author posts

1. Log in as a WP user and publish a brand new post, then assign an unrelated/unlinked CAP guest author to the post (removing the WP user from the authors UI).
2. Switch back to master. In one of the test Homepage Posts blocks, filter by the WP user in step 1. Observe that in the editor and on the front-end, the block displays post(s) created by the WP user but assigned to unrelated guest authors.
3. Check out this branch, refresh, confirm that the block only shows posts that are directly attributed to the WP user.

Posts Carousel block

No changes were made to the Posts Carousel block, but it uses some of the same query functions and endpoints, so we should test it in the editor and on the front-end to ensure it still behaves as expected.

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?

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1201922961350702
  • https://app.asana.com/0/0/1201752178188396

[miguelpeixe]

I'm no longer getting the query error but am still getting unexpected results:

I have a WP user linked to a guest author, let's name them Linked 1 and Guest 1. Guest 1 is the author of 2 posts and Linked 1 does not author any post.

I also have another guest author, Guest 2, not linked to a WP user, which is the author of 1 post.

With 2 blocks, one querying Linked 1 and the other Guest 2, I get the expected posts on the editor but the frontend only shows the posts from the Guest 2 block.

If I change the Linked 1 block to query from Guest 1, I get the same expected result on the editor but while the Guest 1 block shows the expected posts on the frontend, the Guest 2 block stops showing its posts.

[dkoo]

@miguelpeixe you're right, there's still something weird going on here with the linked accounts. Investigating...

[dkoo]

@miguelpeixe This needed a lot more work to get functioning with multiple different guest author queries. I found that the posts_clauses filter modifications were persisting on subsequent WP queries, even though we're removing the "closure" after running each query. For that reason, 1226c7d encloses the appended parts inside comments and does a preg_replace instead of simply appending the clause if a previous conflicting part was already appended. This seems to work for me when including multiple Homepage Posts blocks that include different CAP guest authors on the same page.

Because this posts_clauses filtering seems pretty fragile and complex, I also changed the logic to only use that logic if the Homepage Posts block contains both WP users and guest authors—otherwise, if the block is only filtering by one or the other, we can more cleanly use authors__in or a tax_query instead of modifying the SQL directly.

Lastly, the discrepancies between editor and front-end query results were tripping me up, so I refactored the editor-based query functions to use the same build_articles_query function as the front-end. This should hopefully ensure that the results are identical in both editor and front-end.

cc @kariae because I believe you added the posts_clauses filter in the first place—I'd love to get your review on this as well since it makes some pretty significant changes to Homepage Posts queries.

Edit to add: b9fd0e4 fixes Specific Posts mode in the editor, which was broken by refactoring the posts endpoint params to more closely match the block attributes.

[dkoo]

I realize the changeset of this PR is getting a bit big for a hotfix, so let me know if you think we should rebase this on master and treat it as a regular feature update.

[kariae]

Thanks @dkoo, @miguelpeixe for digging into this.

Because this posts_clauses filtering seems pretty fragile and complex, I also changed the logic to only use that logic if the Homepage Posts block contains both WP users and guest authors—otherwise, if the block is only filtering by one or the other, we can more cleanly use authors__in or a tax_query instead of modifying the SQL directly.

I like the split fix you did here Derrick, and it did fix the above issues for me. IMHO, this is still a hotfix as we're using a workaround that fixed the discussed issues. But I think we need to stop it here, I dug a bit about how CAP do their queries and it seems a bit complex but I think we need to follow the same logic (so that we don't have to create smaller logic blocks depending on if we have co-authors or not or a mix of co-authors and WP users).

The filter used by CAP to filter an author's posts , I think we can use the same logic in case we need to do some refactoring here.

[miguelpeixe]

The blocks now work with all different combinations of deduplication and author/co-authors filters I've tried 🙌

There's also a good improvement in readability for this complex logic 🙇

I'm a bit hesitant to have this merged as a hotfix. Although not visible, it is a big change in a very core feature. I'll let you decide, @dkoo!

[dkoo]

The filter used by CAP to filter an author's posts , I think we can use the same logic in case we need to do some refactoring here.

This is really helpful—thanks for digging this up! I'll do a bit of testing to see if I can refactor our clauses filtering to use the same or similar logic.

I'm a bit hesitant to have this merged as a hotfix. Although not visible, it is a big change in a very core feature. I'll let you decide, @dkoo!

This was my fear, too—this change might directly affect the homepages of every Newspack publisher. I think the safe way to merge this feature would be to rebase the changes on master and build a new alpha release with them. Then we can run the alpha on a couple of publisher sites and watch for unexpected behavior before rolling it out to all sites.

[dkoo]

@miguelpeixe and @kariae, based on the discussion above I'm closing this PR in favor of #1083. Please review the additional commit and testing instructions in that PR.