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

Author, Author Bio, Author Name: Add a fallback for Author Archive Template #55451

Merged
merged 5 commits into from
Feb 20, 2024

Conversation

t-hamano
Copy link
Contributor

Fixes #55410
Similar to #40648

Props to @them-es for reporting the issue with this comment.

What?

This PR fixes an issue where the block is not displayed in the Author Archive template or a warning error occurs when the author has no posts in the following three blocks.

  • Avatar
  • Author Name
  • Author Biography

Why?

These three blocks assume that a post ID context ($block->context['postId']) exists. But my understanding is that if that author hasn't posted any articles yet, the post ID context doesn't exist, so the block won't show up or you'll get an error.

Since the Author Archive page is accessible, I think they should be rendered correctly even in cases where they don't have any posts.

How?

Use author query parameter as ID when no post context exists.

Testing Instructions

  • First, create a user with an avatar. For example, if you register as a user using the email address below, the avatar will be displayed.
  • Enter the user's biography (Also, please note the author ID).
  • Create the Author Archive template in the Site Editor.
  • Enter the following code in the code editor and save it.
    <!-- wp:post-author-name /-->
    <!-- wp:post-author-biography /-->
    <!-- wp:avatar /-->
    
  • Access http://localhost:8888/?author={user_id}.
  • Although this author does not have any posts, their avatar, name, and biography should all be displayed correctly.

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Block] Avatar Affects the Avatar Block [Block] Author Biography Affects the Author Biography [Block] Author Name Affects the Author Name Block labels Oct 18, 2023
@t-hamano t-hamano self-assigned this Oct 18, 2023
@t-hamano t-hamano marked this pull request as ready for review October 18, 2023 13:36
@t-hamano t-hamano requested a review from ajitbohra as a code owner October 18, 2023 13:36
@github-actions
Copy link

github-actions bot commented Oct 18, 2023

Flaky tests detected in 8151386.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7953908460
📝 Reported issues:

@t-hamano t-hamano force-pushed the fix/author-template-fallback branch from 536351c to fa2bad5 Compare December 28, 2023 12:30
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

This tests as advertised for me. Nice work @t-hamano 👍

✅ Could replicate issue on trunk
✅ Avatar, Post Author Biography, and Post Author Name blocks render correctly in author archive
✅ Updated blocks continue to work as intended in other contexts where query string arg isn't provided
✅ Couldn't spot any other blocks or locations needing the same tweak

I left one tiny nit/suggestion. Whether you adopt it or not, I'll leave up to you.

Comment on lines 17 to 21
if ( ! isset( $block->context['postId'] ) ) {
return '';
$author_id = get_query_var( 'author' );
} else {
$author_id = get_post_field( 'post_author', $block->context['postId'] );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Now this conditional is more than a simple guard clause, perhaps it should test for the positive scenario? Sort of like the code style guidelines recommend for ternary operators.

I don't feel strongly about this but it might be good to have all three locations logic match at least 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review!

I rewrote the two places by 8151386. If I rewrite this part using the ternary operator, nested conditional statements will occur, so I think it's fine as is, but what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies if I wasn't clear with my suggestion. I didn't mean that you needed to change these if/else statements to ternaries, just that we should check the positive case first. Like what is suggested for when you do use ternary operators.

This is such a minor detail, don't feel you need to keep changing the code over it.

Below is a quick diff to illustrate what I was originally suggesting:

Example diff
diff --git a/packages/block-library/src/post-author-biography/index.php b/packages/block-library/src/post-author-biography/index.php
index 6c4cdc301e..23f5f16cbc 100644
--- a/packages/block-library/src/post-author-biography/index.php
+++ b/packages/block-library/src/post-author-biography/index.php
@@ -14,7 +14,11 @@
  * @return string Returns the rendered post author biography block.
  */
 function render_block_core_post_author_biography( $attributes, $content, $block ) {
-	$author_id = isset( $block->context['postId'] ) ? get_post_field( 'post_author', $block->context['postId'] ) : get_query_var( 'author' );
+	if ( isset( $block->context['postId'] ) ) {
+		$author_id = get_post_field( 'post_author', $block->context['postId'] );
+	} else {
+		$author_id = get_query_var( 'author' );
+	}
 
 	if ( empty( $author_id ) ) {
 		return '';
diff --git a/packages/block-library/src/post-author-name/index.php b/packages/block-library/src/post-author-name/index.php
index 92d37f70f9..a5fadfcbf3 100644
--- a/packages/block-library/src/post-author-name/index.php
+++ b/packages/block-library/src/post-author-name/index.php
@@ -14,7 +14,11 @@
  * @return string Returns the rendered post author name block.
  */
 function render_block_core_post_author_name( $attributes, $content, $block ) {
-	$author_id = isset( $block->context['postId'] ) ? get_post_field( 'post_author', $block->context['postId'] ) : get_query_var( 'author' );
+	if ( isset( $block->context['postId'] ) ) {
+		$author_id = get_post_field( 'post_author', $block->context['postId'] );
+	} else {
+		$author_id = get_query_var( 'author' );
+	}
 
 	if ( empty( $author_id ) ) {
 		return '';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I mistakenly thought that I should use ternary operators itself. I understand what you mean 😅

Copy link

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

Thanks for your patience here @t-hamano 👍

I've given this another quick smoke test and it LGTM!

🚢

@t-hamano t-hamano merged commit 189e23b into trunk Feb 20, 2024
57 checks passed
@t-hamano t-hamano deleted the fix/author-template-fallback branch February 20, 2024 05:03
@github-actions github-actions bot added this to the Gutenberg 17.8 milestone Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Author Biography Affects the Author Biography [Block] Author Name Affects the Author Name Block [Block] Avatar Affects the Avatar Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Author Name / Author Bio / Avatar: Not displayed in author archive template
2 participants