[wp-trac] [WordPress Trac] #21101: Allow get_comments() to query for explicit value of comment_approved

WordPress Trac wp-trac at lists.automattic.com
Tue Aug 21 15:11:04 UTC 2012


#21101: Allow get_comments() to query for explicit value of comment_approved
-------------------------+------------------
 Reporter:  nbachiyski   |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  3.5
Component:  Comments     |     Version:  3.4
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |
-------------------------+------------------

Comment (by nbachiyski):

 Replying to [comment:5 dd32]:
 > I'm curious as to why the notion of introducing an extra query var in
 place of status is really needed in this case?
 >
 > I don't see any discussion here as to why one was chosen over the other,
 and as more people start using custom comment types with improving API's,
 I think it's going to fast become confusing for some, especially for
 example, when the extra query variable offers no obvious benefit over
 something such as
 [http://core.trac.wordpress.org/attachment/ticket/21101/21101.diff
 21101.diff] to the untrained eye.

 We had the conversation in IRC earlier today, sorry for not posting it
 here.

 In my opinion using {{{status}}} will be more confusing.

 Currently, all the values for {{{status}}} are special strings, which map
 to a specific value of the {{{comment_approved}}} column. Developers know
 that if they use a value, which isn't one of the special ones, they will
 get the sensible default {{{comment_approved = '0' OR comment_approved =
 '1'}}}.

 Also, in general, when a variable name is different from the column name,
 it sets the expectations that it is handled in a special manner and
 doesn't set the column value directly.

 If we use the same variable for setting the column value directly we will
 both break those expectations, we may break some code, which relies on the
 old behaviour (for example setting it to random string like
 {{{default}}}), and we won't let developers use the values {{{hold}}} and
 {{{approved}}}.

 The last one is the worst, in my opinion. Imagine the documentation:
 "{{{status}}} sets the comment status via the {{{comment_approved}}} table
 column. Except the cases of the values {{{approve}}} and {{{hold}}} when
 the values in the database  are '0' and '1' respectively". When I read
 this I cannot help but think "whoa, there must be years of weird legacy
 behind this inconsistency". And I don't want people to think stuff like
 this.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/21101#comment:6>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list