Opened 8 years ago
Last modified 8 years ago
#40047 new enhancement
Add term_meta to WP_Term class getter method
Reported by: | barryceelen | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 4.8 |
Component: | Taxonomy | Keywords: | has-patch 2nd-opinion |
Focuses: | Cc: |
Description
The WP_Post
and WP_User
classes allow getting a meta value by calling its getter function, eg. $post->my_meta_key
. This functionality was omitted when introducing the WP_Term
class.
Attachments (1)
Change History (6)
#3
@
8 years ago
- Keywords 2nd-opinion added
Thanks for the ticket and the patch, @barryceelen.
I intentionally left this feature out of the initial implementation of WP_Term
because I find it odd. It has the potential to cause arbitrary backward compatibility problems, in the case where core wants to introduce a class property that matches a meta key in use by a plugin. It looks like @nacin independently thought something similar about WP_Comment
(where there's also no meta support of this type): https://core.trac.wordpress.org/ticket/32619#comment:6
Is there a strong argument in favor of including it here and on comments? Or is it primarily for the (very) minor convenience, as well as consistency with user and post objects?
cc @flixos90, who mentioned this issue during WP_Term
development, but whose comment I carefully ignored at the time :) https://core.trac.wordpress.org/ticket/14162#comment:21 I vaguely recall having a conversation with him about this, which may be somewhere in the Slack archives.
#4
@
8 years ago
@boonebgorges I think we need to discuss this in a broader scope as you mentioned comments do not support this either. I personally lean towards introducing this functionality, but I do not have a strong opinion for one side or the other here.
One should always be aware that any class property will take precedence. So I don't think the BC concerns are very critical here, especially since core rarely introduces new properties (since we never change the database schema).
In my own projects I often use wrapper classes for WordPress objects that make interaction with them easier, and if a type supports meta, I include this functionality as I consider it valuable. So it would not benefit me personally, but I think it's useful to anyone who uses the WP_Post
/WP_User
/WP_Comment
/WP_Term
objects as is.
On the other hand, multisite's WP_Site
and WP_Network
could have something like this too, for site and network options respectively (due to recent discussions between what conceptually differs between options and metadata). If these options became available in the same way as we're discussing meta, changes would be high that people would unexpectedly do many unnecessary switch_to_blog()
calls. So the possibly high complexity / cost of retrieving such a value gets kind of obscured when one can assume these values are like class properties. Not a strong argument against it, but something to consider.
Maybe introducing a method get_meta()
on all of these classes would be a better solution, for explicit and clear access, and they would still make it a bit easier to retrieve metadata compared to calling get_*_meta()
with the object's ID.
#5
@
8 years ago
So I don't think the BC concerns are very critical here, especially since core rarely introduces new properties (since we never change the database schema).
Perhaps not, but there's some precedent for adding properties that are not part of the schema. See eg $post->ancestors
.
I'm not against having a helper method for fetching meta - get_meta()
would be fine, or even using meta as the fallback in a general get()
method. It's just the __get()
implementation that feel inappropriate to me.
In any case, I agree that we need a general agreement about this across our object classes.
Not sure whether the
break;
after the first switch statement is explicitly needed as it is preceded by a return.