-
Notifications
You must be signed in to change notification settings - Fork 42
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
See #1719: Added Co-Author JSON-LD support. #1720
See #1719: Added Co-Author JSON-LD support. #1720
Conversation
@@ -114,7 +114,7 @@ public function after_get_jsonld( $jsonld, $post_id, $context ) { | |||
* for Person and Organization, so check before we add it to graph. | |||
* reference : https://schema.org/author | |||
*/ | |||
if ( $author_jsonld && ! $this->is_author_entity_present_in_graph( $jsonld, $article_jsonld['author']['@id'] ) ) { | |||
if ( $author_jsonld && ! $this->is_author_entity_present_in_graph( $jsonld, $this->get_primary_author($article_jsonld['author'])['@id'] ) ) { |
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.
We need to be careful about this syntax as I think older version of PHP do not support it.
Let's change it to:
$primary_author = $this->get_primary_author($article_jsonld['author']);
if ( $author_jsonld && ! $this->is_author_entity_present_in_graph( $jsonld, $primary_author['@id'] ) ) {
private function get_primary_author( $author ) { | ||
|
||
// Nested array of co-authors. Return the primary author. | ||
if ( ! isset( $author['@id'] ) ) { |
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.
I think we need to add a couple more checks here, like if ( is_array( $author) && !empty( $author ) && ! isset( $author['@id'] ) ) {
... otherwise you risk trying to access index 0
of something which is not an array or has no index 0
.
'wl_jsonld_author', | ||
array( | ||
'author' => $this->get_author( $post->post_author, $references ), | ||
'references' => $references |
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.
I don't think $references
would work as an output in this case.
Check src/includes/class-wordlift-entity-post-to-jsonld-converter.php:L123 to see how we do it in a similar case:
/**
* Call the `wl_post_jsonld_array` filter. This filter allows 3rd parties to also modify the references.
*
* @param array $value {
*
* @type array $jsonld The JSON-LD structure.
* @type int[] $references An array of post IDs.
* }
* @since 3.25.0
*
* @see https://www.geeklab.info/2010/04/wordpress-pass-variables-by-reference-with-apply_filter/
*
* @api
*/
$ret_val = apply_filters(
'wl_entity_jsonld_array',
array(
'jsonld' => $jsonld,
'references' => $references,
),
$post_id
);
$jsonld = $ret_val['jsonld'];
$references = $ret_val['references'];
$coauthor_plugin_path = 'co-authors-plus/co-authors-plus.php'; | ||
|
||
// If the co-authors plugin is active. | ||
if ( is_plugin_active( $coauthor_plugin_path ) ) { |
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.
To be on the safe side also add && function_exists( 'get_coauthors' )
, we don't want to find ourselves breaking a website because this function changed or is renamed.
} | ||
} | ||
|
||
return $author; |
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.
Here we should send back both author and references, see the example filter I linked before.
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 17 New issues |
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.
@mauanga did you test it by any chance?
wl_jsonld_author
which checks if theco-author-plus
plugin is active and conditionally filters the author JSON-LD array passed to it if co-authors exist on a post.Jsonld_Article_Wrapper
class to ensure we are checking the primary author.