Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 4 years ago

#5508 closed enhancement (wontfix)

improve filterability (sorting) of Activity loop

Reported by: mpa4hu Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.0
Component: Activity Keywords: has-patch
Cc: mpa4hu@…

Description

@boonebgorges Hello again,

Ill try to make a story out of this >.<
This continiues ticket https://buddypress.trac.wordpress.org/ticket/5480 created by me.

You were right this could make a good plugin, but since I'm not experienced in developing and totally newbie to wordpress itself, I'm just messing around scripts. Now I decided to build my first plugin out of this:

Plugin would add two new scopes (optionally) to the main stream.
First one is "Hot algorithm" which sorts activity by "favorite count" and time.
And second (since first one just makes plugin too small) is "Recent". when new comment is made to the activity, I'll save new meta "bp_last_timestap" with comments date and sort by that date. So this will look more like forums.

Then I thought what you sad and you are right, its really not smart to hook there and change whole query. I experimented with bp_has_activities() hooks and its just more simple to pass meta directly there (I was confused since documentation shows only how to pass strings)

TL;DR

If you could add a simple filter here (well at least I don't need anything passed there)

// Sorting
if ( $sort != 'ASC' && $sort != 'DESC' )
	$sort = 'DESC';

It will save me many troubles.
Please if you can make this for 2.0 I could support my plugin from this major version.

Attachments (1)

5508.patch (445 bytes) - added by slaFFik 4 years ago.

Download all attachments as: .zip

Change History (8)

#1 @boonebgorges
7 years ago

  • Component changed from Core to Activity
  • Milestone changed from Awaiting Review to 2.1

Let's look at a way to improve filterability for 2.1. I do not just want to place filters on SQL strings haphazardly. In the short run, while I know it's not ideal, you can do a preg_replace() on the entire SQL string as passed to bp_activity_paged_activities_sql.

#2 @DJPaul
7 years ago

Not going to have time to look at this for 2.1.

#3 @DJPaul
7 years ago

  • Milestone changed from 2.1 to 2.2

#4 @DJPaul
6 years ago

  • Milestone changed from 2.2 to Future Release
  • Summary changed from Filter for sorting to improve filterability (sorting) of Activity loop

#5 @slaFFik
4 years ago

  • Keywords has-patch added

We already have filters for SQL.
Adding only to $sort will question the need to add to other parts of the query separately as well. And imo it will be overkill.

@boonebgorges
I propose to do something like this:

// Sorting.
$sort = $r['sort'];
if ( empty( $sort ) ) {
	$sort = 'DESC';
}

Thus passing a param 'sort' will give ability to redefine this without a special filter.

@slaFFik
4 years ago

#6 @slaFFik
4 years ago

  • Keywords dev-feedback added

#7 @boonebgorges
4 years ago

  • Keywords dev-feedback removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

$sort is compared against a whitelist is for security. I don't think that BP allows you to pass sort or orderby in URL params by default, but if a plugin did so, you'd open yourself up for SQL injection. For this reason, as a general rule, it's not good practice to allow non-parameterized SQL values to be set via function parameter.

The situation described by @mpa4hu is quite specific, so I think it's OK to suggest that this is the "proper" way to do it:

function bp5508_filter_orderby( $sql, $r ) {
    if ( 'hottest' === $r['sort'] ) {
        $orderby = 'foo';
    } elseif ( 'recent' === $r['sort'] ) {
        $orderby = 'bar';
    }

    $sql = preg_replace( 'ORDER BY a\.date_recorded (ASC|DESC), a\.id (ASC|DESC)', 'ORDER BY ' . $orderby, $sql );

    return $sql;
}
add_filter( 'bp_activity_paged_activities_sql', 'bp5508_filter_orderby' );
Note: See TracTickets for help on using tickets.