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

Changing sort order of coauthors broken in WordPress ^4.7.x #390

Closed
dsifford opened this Issue Dec 10, 2016 · 16 comments

Comments

Projects
None yet
8 participants
@dsifford

dsifford commented Dec 10, 2016

Hello there,

It appears that ever since the update to WordPress 4.7, changing sort order of coauthors no longer works properly.

I confirmed this on a clean install of WordPress with only co-authors-plus installed. It's most definitely an issue with this plugin.

Happy to help if needed. Thanks so much.

@blackwood

This comment has been minimized.

Show comment
Hide comment
@blackwood

blackwood Dec 12, 2016

Indeed, can replicate. However, changing line 17 in get_coauthors in the file template-tags.php from:

 $coauthor_terms = get_the_terms( $post_id, $coauthors_plus->coauthor_taxonomy );

to:

$coauthor_terms = wp_get_post_terms( $post_id, $coauthors_plus->coauthor_taxonomy, array( 
  'orderby' => 'term_order', 
) );

...fixes the issue for me.

The untested theory we have over here is that the default args provided by wp_get_object_terms could have changed in 4.7, which is how get_the_terms updates the cache if there's no cached terms available -- so whenever get_coauthors is called, it ignores the custom term_order, using name instead.

blackwood commented Dec 12, 2016

Indeed, can replicate. However, changing line 17 in get_coauthors in the file template-tags.php from:

 $coauthor_terms = get_the_terms( $post_id, $coauthors_plus->coauthor_taxonomy );

to:

$coauthor_terms = wp_get_post_terms( $post_id, $coauthors_plus->coauthor_taxonomy, array( 
  'orderby' => 'term_order', 
) );

...fixes the issue for me.

The untested theory we have over here is that the default args provided by wp_get_object_terms could have changed in 4.7, which is how get_the_terms updates the cache if there's no cached terms available -- so whenever get_coauthors is called, it ignores the custom term_order, using name instead.

@blackwood

This comment has been minimized.

Show comment
Hide comment
@blackwood

blackwood Dec 12, 2016

...but it could be something else entirely. It would definitely be great to get the plugin author's input here.

blackwood commented Dec 12, 2016

...but it could be something else entirely. It would definitely be great to get the plugin author's input here.

@blackwood

This comment has been minimized.

Show comment
Hide comment
@blackwood

blackwood Dec 12, 2016

#392 and #391 look like they address the same issue?

blackwood commented Dec 12, 2016

#392 and #391 look like they address the same issue?

@dsifford

This comment has been minimized.

Show comment
Hide comment
@dsifford

dsifford Dec 12, 2016

Thanks for the comments @blackwood -- I'll give your fix a whirl for the time being while waiting for the maintainers' input on this 😄

dsifford commented Dec 12, 2016

Thanks for the comments @blackwood -- I'll give your fix a whirl for the time being while waiting for the maintainers' input on this 😄

@dsifford

This comment has been minimized.

Show comment
Hide comment
@dsifford

dsifford Dec 12, 2016

@blackwood Your fix isn't doing the trick here for me. Hmm.. Looks like the two open PRs involve clearing object cache? Edit: It works.

Both PRs break the build so I'm curious what negative implications doing it @mslinnea's way has (if any?)

dsifford commented Dec 12, 2016

@blackwood Your fix isn't doing the trick here for me. Hmm.. Looks like the two open PRs involve clearing object cache? Edit: It works.

Both PRs break the build so I'm curious what negative implications doing it @mslinnea's way has (if any?)

@mslinnea

This comment has been minimized.

Show comment
Hide comment
@mslinnea

mslinnea Dec 12, 2016

Thanks for linking to my PRs. I don't know what's up with Travis, as the errors seem unrelated to my code changes.

I tried @blackwood's fix and it does work for me, but wp_get_post_terms is listed as an uncached function, which is why I steered clear of using it.

mslinnea commented Dec 12, 2016

Thanks for linking to my PRs. I don't know what's up with Travis, as the errors seem unrelated to my code changes.

I tried @blackwood's fix and it does work for me, but wp_get_post_terms is listed as an uncached function, which is why I steered clear of using it.

@dsifford

This comment has been minimized.

Show comment
Hide comment
@dsifford

dsifford Dec 12, 2016

@mslinnea Interesting. I wasn't aware of the cached/uncached functions. Thanks for the resource!

Also, perhaps @blackwood's fix will work for me... I SSH'ed into the server and made the change extremely quickly with vim so chances are I could have made a typo! I'll try it again in a few.

dsifford commented Dec 12, 2016

@mslinnea Interesting. I wasn't aware of the cached/uncached functions. Thanks for the resource!

Also, perhaps @blackwood's fix will work for me... I SSH'ed into the server and made the change extremely quickly with vim so chances are I could have made a typo! I'll try it again in a few.

@blackwood

This comment has been minimized.

Show comment
Hide comment
@blackwood

blackwood Dec 13, 2016

It'd be great if you could pass args along with get_the_terms so that it would bust the cache and reorder or set the cache with your required $args... but that'd be more of a core issue.

@mslinnea -- I really like the idea in #391, setting and clearing the cache manually on save/delete.

blackwood commented Dec 13, 2016

It'd be great if you could pass args along with get_the_terms so that it would bust the cache and reorder or set the cache with your required $args... but that'd be more of a core issue.

@mslinnea -- I really like the idea in #391, setting and clearing the cache manually on save/delete.

@dsifford

This comment has been minimized.

Show comment
Hide comment
@dsifford

dsifford Dec 13, 2016

Sorry for the delay. @blackwood's fix does work for me. Must have been a typo last time.

dsifford commented Dec 13, 2016

Sorry for the delay. @blackwood's fix does work for me. Must have been a typo last time.

@philipjohn

This comment has been minimized.

Show comment
Hide comment
@philipjohn

philipjohn Dec 14, 2016

Member

There were some changes to wp_get_object_terms() in 4.7 that look related to WP_Term_Query.

I've reproduced this myself and I'm just checking out the two PRs.

Member

philipjohn commented Dec 14, 2016

There were some changes to wp_get_object_terms() in 4.7 that look related to WP_Term_Query.

I've reproduced this myself and I'm just checking out the two PRs.

@philipjohn philipjohn modified the milestones: 3.3, 3.5 Jan 15, 2017

@aous77

This comment has been minimized.

Show comment
Hide comment
@aous77

aous77 Feb 14, 2017

Hi,
is there a fix for this issue?

aous77 commented Feb 14, 2017

Hi,
is there a fix for this issue?

@AustinBrister

This comment has been minimized.

Show comment
Hide comment
@AustinBrister

AustinBrister Feb 14, 2017

AustinBrister commented Feb 14, 2017

@aous77

This comment has been minimized.

Show comment
Hide comment
@aous77

aous77 Feb 14, 2017

this is great!
do we know when will it get pushed to wordpress repo?

aous77 commented Feb 14, 2017

this is great!
do we know when will it get pushed to wordpress repo?

@benlk

This comment has been minimized.

Show comment
Hide comment
@benlk

benlk Mar 13, 2017

Contributor

The fix appears to have been merged in #391, and since this issue is in the 3.5 milestone, https://github.com/Automattic/Co-Authors-Plus/milestone/20, it probably won't get released until August 31, 2017.

Contributor

benlk commented Mar 13, 2017

The fix appears to have been merged in #391, and since this issue is in the 3.5 milestone, https://github.com/Automattic/Co-Authors-Plus/milestone/20, it probably won't get released until August 31, 2017.

@ckeeney

This comment has been minimized.

Show comment
Hide comment
@ckeeney

ckeeney Jun 29, 2017

Is there any chance of getting this released before August 31? It seems a bit silly for a fix to such a widely discussed bug to sit in master unreleased for several months.

ckeeney commented Jun 29, 2017

Is there any chance of getting this released before August 31? It seems a bit silly for a fix to such a widely discussed bug to sit in master unreleased for several months.

@benlk

This comment has been minimized.

Show comment
Hide comment
@benlk

benlk Aug 29, 2017

Contributor

The 3.5 milestone was pushed to February 2018, but this issue is no longer in 3.5.

#406 was merged to master on March 20.

Version 3.2, released on April 3, contained the ordering fixes: https://github.com/Automattic/Co-Authors-Plus/releases/tag/3.2.2

Contributor

benlk commented Aug 29, 2017

The 3.5 milestone was pushed to February 2018, but this issue is no longer in 3.5.

#406 was merged to master on March 20.

Version 3.2, released on April 3, contained the ordering fixes: https://github.com/Automattic/Co-Authors-Plus/releases/tag/3.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment