Opened 13 years ago
Closed 13 years ago
#4483 closed defect (bug) (fixed)
Allow ASC and DESC sort in bp_has_groups()
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 1.8 | Priority: | low |
| Severity: | normal | Version: | |
| Component: | Groups | Keywords: | |
| Cc: | jkudish |
Description
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)
Change History (7)
#3
@
13 years ago
- Component changed from Core to Groups
- Milestone changed from 1.7 to 1.8
Punting to 1.8.
#4
@
13 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
typeparameter intoorderandorderby. When it exists,typealways supercedesorderandorderby- this is primarily for backpat, and it also makestypeuseful as a sort of shorthand for certain useful combinations oforderandorderby. - I've removed the logic in
bp_has_groups()that sets a defaulttypeof 'active'. This is necessary so that it's possible to actually useorderandorderbywithout also manually settingtypeto false. Backward compatibility is maintained, because I made the default values oforderandorderbyfunctionally equivalent totype = 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.
The
orderargument is currently used in place of howorder_byusually would be used. This means back-compat will be a pain