Add support for TimberPost->terms( array ) & more#737
Conversation
This is backwards compatible as if a TermClass is passed to the method it will take precedence over the property
update docblocks. also removes usage of _get_terms property, as this could return wrong results depending on arguments passed to the method
|
@aaemnnosttv this is great work. Thanks much for your work on this (and with tests!). I had a question on the |
|
Thanks Jared! I've really been digging Timber, so I'm happy to contribute. Regarding the named constructor, I actually use this in the tests. In addition to just being maybe a bit more expressive than using // some function/method..
// get all non-empty terms for categories and tags
$terms = get_terms(['post_tag','category']);
return array_map('TimberTerm::from', $terms);Whereas without that, you would need to use a closure or something to map over that in order to Tests are another reason (and probably the reason I thought to add it) where you can see above, we're able to convert all of those array items to TimberTerms without using a It's just a little helper which can be used to help with readability, but also with reducing the amount of code needed to do certain things. :) |
|
Awesome, something I hadn't thought of before. Would it make sense to apply the same method to posts and users as well? |
Add support for TimberPost->terms( array ) & more
|
Yeah I think the same use case would apply to all of them. I definitely thought of extending that to those as well, but as I said, I wanted to keep this PR limited ;) I could put together another one if you're interested? |
|
@aaemnnosttv yes please! Thanks for making it a separate PR though, one of the toughest things can be wading into code that touches too many pieces at-once. |
2 participants
This PR addresses a few things (sorry about that) but I tried to limit the reach of the code changed to only that which is relevant. I think the commit messages give a pretty good summary, but here's the gist of it:
terms/get_termsmethods on aTimberPostand passing an array of taxonomies to fetch terms for, rather than all of them.termsas well as tags and categories to be cleaner and more readableTimberTerm, which takes the same arguments as the constructor method. Usage likeTimberTerm::from('My Term Name'). This is also totally compatible with extended classes.MyExtendedTimberTerm::from('My Term Name')_get_termsproperty and deprecate it onTimberPost. This cache is not only unnecessary, but could actually return the wrong results as it only caches based on the taxonomy argument, and did not take the other method arguments into consideration.