Skip to:

Opened 12 years ago

Closed 11 years ago

#4483 closed defect (bug) (fixed)

Allow ASC and DESC sort in bp_has_groups()

Reported by: boonebgorges's profile boonebgorges Owned by: boonebgorges's profile boonebgorges
Milestone: 1.8 Priority: low
Severity: normal Version:
Component: Groups Keywords:
Cc: jkudish


bp_has_groups() allows sorting by several different types: 'newest', 'popular', 'active', etc. But these sort orders only go in one direction (generally, DESC). This is a pretty arbitrary limitation, and it causes problems with the sortable group list in the Groups Admin UI (#4414).

My suggestion: Introduce 'order' and 'orderby' parameters to the bp_has_groups() stack (which has the added benefit of better matching the way that queries are handled elsewhere in BP/WP). Map 'type' parameters onto 'order' and 'orderby' inside of the function (eg, 'active' is equivalent to 'orderby' = 'last_activity' and 'order' = 'DESC'). Allow 'order'/'orderby', when present, to override the 'type' parameter.

Marking this low priority. I'm putting it in the 1.7 milestone, but I don't think it's crucial to the release, so it's puntable.

Attachments (1)

4483.patch (19.6 KB) - added by boonebgorges 11 years ago.

Download all attachments as: .zip

Change History (7)

#1 @jkudish
12 years ago

The order argument is currently used in place of how order_by usually would be used. This means back-compat will be a pain

#2 @jkudish
12 years ago

  • Cc jkudish added

#3 @johnjamesjacoby
11 years ago

  • Component changed from Core to Groups
  • Milestone changed from 1.7 to 1.8

Punting to 1.8.

#4 @boonebgorges
11 years ago

jkudish - order does not appear to be an argument, but a piece of legacy code that does nothing. I'll have to do more research into it, but in any case it's something that appears to have been broken for a long time.

I've attached a first pass at order and orderby support for bp_has_groups() and the associated stack. Most of it is pretty straightforward, but there are a few items to note:

  • In the database class, I've added a few utility methods that convert the existing type parameter into order and orderby. When it exists, type always supercedes order and orderby - this is primarily for backpat, and it also makes type useful as a sort of shorthand for certain useful combinations of order and orderby.
  • I've removed the logic in bp_has_groups() that sets a default type of 'active'. This is necessary so that it's possible to actually use order and orderby without also manually setting type to false. Backward compatibility is maintained, because I made the default values of order and orderby functionally equivalent to type = active.

I've written a number of tests to ensure the new functionality, and to make sure that the old behaviors related to the default value of type continue to work as expected.

Since this passes all the tests, I'd like to get it into trunk soon (so I can start using it, eg, to make columns sortable by ASC and DESC in the group admin), but I welcome feedback on the patch first.

11 years ago

#5 @boonebgorges
11 years ago

In 7087:

Introduces 'order' and 'orderby' parameters for the bp_has_groups() stack

'order' and 'orderby' are more specific and flexible versions of the existing
'type' parameter ('popular', 'newest', etc). Backward compatibility is
maintained by always obeying 'type' when it is passed, and by internally
converting the legacy 'type' values into the corresponding 'order' and
'orderby' values.

Adds unit tests to ensure that the adjusted default values for functions in
the stack are backward compatibile (in particular, the fact that the 'type'
parameter is now empty by default, replaced with the corresponding default
values for 'order' and 'orderby'). Unit tests are also added for the new
params, as well as some internal utility methods.

See #4483

#6 @boonebgorges
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

In 7088:

Add sortable column support for the Groups Administration Dashboard panel

The columns have been marked sortable since they were originall introduced, but
the sorting did not work because of limitations in how bp_has_groups() worked.
As of r7087, bp_has_groups() supports 'order' and 'orderby' params, which
allows us to fix this broken feature.

Fixes #4483

Note: See TracTickets for help on using tickets.