M18.12 - Fix tribe_get_linked_posts_by_post_type() ordering #2138
Conversation
it's a few lines above already
- Added `$return_all_if_none` parameter to `get_linked_post_info()` - So that `$my_linked_posts` from `saved_linked_post_dropdown()` can use that method (getting all if none) and the `tribe_get_linked_posts_by_post_type()` function returns none if none are linked, as expected. - Open to alternative solutions...
cliffordp
added
code review
|
Cliff could you continue these changes in branch and then we can merge after it gets a QA pass? |
|
@jesseeproductions Continue what changes? It's ready for code review as-is. Alexis said @borkweb would do it on Monday when he's back. If you're referring to the |
|
The double revert made it seem like you were reverting not adding back coding, my bad. |
There was a problem hiding this comment.
@cliffordp - this is really great. Nice work on this one! There's just one ultra-minor tweak that I'd like to see happen and once that's done I'd say this is ready for QA-ing in the branch! (we'll merge after QA has completed)
💯
src/Tribe/Linked_Posts.php
Outdated
|
|
||
| $linked_ids_order_key = $this->get_order_meta_key( $post_type ); | ||
|
|
||
| if ( $linked_ids_order_key ) { |
There was a problem hiding this comment.
Let's invert this to avoid nesting desired logic:
if ( ! $linked_ids_order_key ) {
return;
}
update_post_meta( $target_post_id, $linked_ids_order_key, $current_order );
src/Tribe/Linked_Posts.php
Outdated
| * | ||
| * @return array | ||
| */ | ||
| public function get_linked_post_info( $linked_post_type, $args = array(), $linked_post_ids = null ) { | ||
| public function get_linked_post_info( $linked_post_type, $args = array(), $linked_post_ids = null, $return_all_if_none = false ) { |
There was a problem hiding this comment.
If we want to avoid using an extra parameter here, you could feasibly just add a filter instead:
/**
* blah blah
*/
$return_all_if_none = apply_filters( 'tribe_events_return_all_linked_posts_if_none', false, $linked_post_type, $args, $linked_post_ids );Then when you call this method for that one case where you want that to be true, just do:
add_filter( 'tribe_events_return_all_linked_posts_if_none', '__return_true' );
// do the call
remove_filter( 'tribe_events_return_all_linked_posts_if_none', '__return_true' );per Matt's code review
- remove the parameter added to `get_linked_post_info()` - implement as a filter instead - fix typo in docblock for the existing `$linked_post_ids` param - tweak readme to mention new filter and clarify other things
|
|
@borkweb please re-review |
borkweb
added
merge
|
Let's QA this in the branch before merging |
|
QA passed so merging now :) |
cliffordp
added
merge
https://central.tri.be/issues/105116
Start by reverting the reverts from #2105
Then will fix from there.