-
Notifications
You must be signed in to change notification settings - Fork 33
feature: allow primary term selected by yoast #3898
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
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a Yoast primary-term lookup to the metadata builder: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.33)Invalid configuration: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
#3036 Mention for explicitly setting for enabling/disabling but I feel it should be the default behaviour when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Metadata/class-metadata-builder.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{html,php}
⚙️ CodeRabbit configuration file
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
Files:
src/Metadata/class-metadata-builder.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E against WordPress latest
- GitHub Check: build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Metadata/class-metadata-builder.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{html,php}
⚙️ CodeRabbit configuration file
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
Files:
src/Metadata/class-metadata-builder.php
🧠 Learnings (4)
📓 Common learnings
Learnt from: up1512001
Repo: Parsely/wp-parsely PR: 3898
File: src/Metadata/class-metadata-builder.php:700-715
Timestamp: 2025-12-20T18:31:49.903Z
Learning: The Yoast SEO function `yoast_get_primary_term( $taxonomy = 'category', $post = null )` accepts `int|WP_Post|null` for the `$post` parameter, so passing a WP_Post object directly is valid and does not require passing `$post->ID`.
📚 Learning: 2025-12-20T18:31:49.903Z
Learnt from: up1512001
Repo: Parsely/wp-parsely PR: 3898
File: src/Metadata/class-metadata-builder.php:700-715
Timestamp: 2025-12-20T18:31:49.903Z
Learning: When calling yoast_get_primary_term in PHP, the $post parameter accepts int|WP_Post|null. You can pass a WP_Post object directly (instead of $post->ID) and it will be treated correctly. Ensure usage matches the expected types in PHPDoc/type hints and avoid extracting $post->ID unless necessary.
Applied to files:
src/Metadata/class-metadata-builder.php
📚 Learning: 2024-07-26T21:07:21.167Z
Learnt from: acicovic
Repo: Parsely/wp-parsely PR: 0
File: :0-0
Timestamp: 2024-07-26T21:07:21.167Z
Learning: User: acicovic
URL: https://github.com/Parsely/wp-parsely/pull/2355
Timestamp: 2024-04-03T08:04:35.576Z
Learning: In the context of the `wp-parsely` project's documentation, bullet points are consistently capitalized. This standard should be respected in reviews and suggestions regarding document formatting.
Applied to files:
src/Metadata/class-metadata-builder.php
📚 Learning: 2024-07-26T21:07:21.167Z
Learnt from: vaurdan
Repo: Parsely/wp-parsely PR: 2544
File: src/Endpoints/content-helper/class-smart-linking-endpoint.php:92-103
Timestamp: 2024-07-26T21:07:21.167Z
Learning: The `private_api_request_validate_post_id` function in `class-smart-linking-endpoint.php` checks if the post exists to validate the `post_id`, but does not explicitly check for the `post_id` being a positive integer.
Applied to files:
src/Metadata/class-metadata-builder.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: E2E against WordPress latest
| /** | ||
| * Attempt to get primary term if it's set by Yoast SEO plugin. | ||
| */ | ||
| $primary_category = self::get_primary_term_from_yoast( $post_obj, $parsely_options['custom_taxonomy_section'] ); | ||
|
|
||
| if ( '' !== $primary_category ) { | ||
| $category_name = $primary_category; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && git clone --depth 1 https://github.com/Parsely/wp-parsely.git 2>/dev/null && head -n 415 wp-parsely/src/Metadata/class-metadata-builder.php | tail -n 35Repository: Parsely/wp-parsely
Length of output: 1218
🏁 Script executed:
sed -n '384,391p' wp-parsely/src/Metadata/class-metadata-builder.php && echo "---" && sed -n '700,715p' wp-parsely/src/Metadata/class-metadata-builder.phpRepository: Parsely/wp-parsely
Length of output: 155
🏁 Script executed:
grep -n "Attempt to get primary term" wp-parsely/src/Metadata/class-metadata-builder.phpRepository: Parsely/wp-parsely
Length of output: 145
🏁 Script executed:
find . -name "class-metadata-builder.php" -type f 2>/dev/null | head -5Repository: Parsely/wp-parsely
Length of output: 103
🏁 Script executed:
sed -n '384,391p' ./src/Metadata/class-metadata-builder.php && echo "---LINE-700-715---" && sed -n '700,715p' ./src/Metadata/class-metadata-builder.phpRepository: Parsely/wp-parsely
Length of output: 903
Fix grammar error and placeholder version in docblock.
Line 701: "if its set" should be "if it's set."
Line 703: Replace the placeholder "@SInCE 3.21.3 -- supposed to be change when merged" with the actual version number where this method was introduced.
The type validation concern in the original review is unnecessary—the function return type is explicitly defined as string on line 711, and it falls back to an empty string if Yoast functions are unavailable, guaranteeing a string return in all cases.
🤖 Prompt for AI Agents
In src/Metadata/class-metadata-builder.php around lines 384-391, update the
docblock: fix the grammar "if its set" to "if it's set", replace the placeholder
"@since 3.21.3 -- supposed to be change when merged" with the actual release
version this change is introduced in (use the correct project release number),
and remove the redundant note about type validation since the function is
already declared to return string and falls back to an empty string when Yoast
functions are unavailable.
| /** | ||
| * Attempt to get primary term of given taxonomy if its set by Yoast SEO plugin. | ||
| * | ||
| * @since 3.21.3 -- supposed to be change when merged. | ||
| * | ||
| * @param WP_Post $post The post object. | ||
| * @param string $taxonomy The taxonomy name. | ||
| * | ||
| * @return string Primary category name or empty string. | ||
| */ | ||
| private static function get_primary_term_from_yoast( WP_Post $post, string $taxonomy = 'category' ): string { | ||
| if ( ! function_exists( 'yoast_get_primary_term' ) ) { | ||
| return ''; | ||
| } | ||
| return yoast_get_primary_term( $taxonomy, $post ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update placeholder version and validate return type.
Line 703 contains a placeholder version "3.21.3 -- supposed to be change when merged" that must be updated to the actual release version before merging.
Additionally:
- Line 701: "if its set" should be "if it's set".
- Line 714 directly returns the result from
yoast_get_primary_term()without validation. If the Yoast function returnsfalse,null, or other non-string types, this violates the method'sstringreturn type declaration. Cast the result to ensure type safety.
🔎 Proposed fixes
/**
- * Attempt to get primary term of given taxonomy if its set by Yoast SEO plugin.
+ * Attempt to get primary term of given taxonomy if it's set by Yoast SEO plugin.
*
- * @since 3.21.3 -- supposed to be change when merged.
+ * @since 3.22.0
*
* @param WP_Post $post The post object.
* @param string $taxonomy The taxonomy name.
*
* @return string Primary category name or empty string.
*/
private static function get_primary_term_from_yoast( WP_Post $post, string $taxonomy = 'category' ): string {
if ( ! function_exists( 'yoast_get_primary_term' ) ) {
return '';
}
- return yoast_get_primary_term( $taxonomy, $post );
+ $primary_term = yoast_get_primary_term( $taxonomy, $post );
+ return is_string( $primary_term ) ? $primary_term : '';
}Based on learnings, yoast_get_primary_term() accepts int|WP_Post|null for the $post parameter, so passing the WP_Post object directly is correct.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Attempt to get primary term of given taxonomy if its set by Yoast SEO plugin. | |
| * | |
| * @since 3.21.3 -- supposed to be change when merged. | |
| * | |
| * @param WP_Post $post The post object. | |
| * @param string $taxonomy The taxonomy name. | |
| * | |
| * @return string Primary category name or empty string. | |
| */ | |
| private static function get_primary_term_from_yoast( WP_Post $post, string $taxonomy = 'category' ): string { | |
| if ( ! function_exists( 'yoast_get_primary_term' ) ) { | |
| return ''; | |
| } | |
| return yoast_get_primary_term( $taxonomy, $post ); | |
| } | |
| /** | |
| * Attempt to get primary term of given taxonomy if it's set by Yoast SEO plugin. | |
| * | |
| * @since 3.22.0 | |
| * | |
| * @param WP_Post $post The post object. | |
| * @param string $taxonomy The taxonomy name. | |
| * | |
| * @return string Primary category name or empty string. | |
| */ | |
| private static function get_primary_term_from_yoast( WP_Post $post, string $taxonomy = 'category' ): string { | |
| if ( ! function_exists( 'yoast_get_primary_term' ) ) { | |
| return ''; | |
| } | |
| $primary_term = yoast_get_primary_term( $taxonomy, $post ); | |
| return is_string( $primary_term ) ? $primary_term : ''; | |
| } |
🤖 Prompt for AI Agents
In src/Metadata/class-metadata-builder.php around lines 700 to 715, update the
placeholder doc version at line 703 from "3.21.3 -- supposed to be change when
merged" to the actual release version, fix the grammar in the doc comment ("if
its set" → "if it's set"), and ensure the method returns a string by
validating/casting the Yoast result before returning (call
yoast_get_primary_term(...), check for false/null or non-string and return an
empty string or cast to string as appropriate to satisfy the declared string
return type); note that passing the WP_Post object directly to
yoast_get_primary_term is correct.
Description
As
Yoast Primaryterm is quite for some time its better to use it when Yoast plugin is activated and multiple categories are selected.Motivation and context
Closes #3036
How has this been tested?
parsely-sectionas primary term for any selected taxonomy if changed from settingsScreenshots (if appropriate)
Editor category selection
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.