Skip to:
Content

BuddyPress.org

Opened 4 years ago

Closed 4 years ago

#6720 closed defect (bug) (fixed)

The order of site wide activity items seems to be wrong when the scope is set to either groups or friends

Reported by: henry.wright Owned by: boonebgorges
Milestone: 2.5 Priority: normal
Severity: normal Version: 2.3.3
Component: Activity Keywords: has-patch has-unit-tests
Cc:

Description

The order of site wide activity items seems to be wrong when the scope is set to either groups or friends.

For example, I'd expect the most recent activity item (the item with the highest activity ID) to appear at the top. But this doesn't happen.

Note, when scope is All Members, the problem doesn't happen (activity items are ordered as expected, with most recent appearing first in the list).

Attachments (2)

6720.diff (2.1 KB) - added by Mamaduka 4 years ago.
6720-tests.diff (2.6 KB) - added by Mamaduka 4 years ago.

Download all attachments as: .zip

Change History (10)

#1 @imath
4 years ago

Hi @henry.wright

I don't think, there's a bug. First because i wasn't able to reproduce and second because the order is not linked to the activity ID but to the date_recorded field.

See https://buddypress.trac.wordpress.org/browser/trunk/src/bp-activity/classes/class-bp-activity-activity.php#L550

So ID 42 recorded at 2015/11/10 10:00 can be displayed after ID 41 if ID 41 was posted/edited at 2015/11/10 10:01

#2 @henry.wright
4 years ago

Hi @imath!

I think I understand the problem better now.

The problem exists if two activity items have the exact same recorded date. For example: 2015-11-08 01:13:25 and 2015-11-08 01:13:25.

In cases such as this, activity item with ID 20 can appear above activity item with ID 21 in the activity streams. During normal site usage, this will happen very rarely. I only came across the problem when bulk importing lots of new activity items into my site, of course each of which have the same recorded date (verified by looking in the db).

#3 @henry.wright
4 years ago

I'm thinking something like ...ORDER BY a.date_recorded {$sort}, a.id {$sort}... could solve it?

#4 @imath
4 years ago

As you said: it would really be an edge case. If you need to bulk import activity, you should use the date_recorded field else the time since would be wrong

I still don't think it's an issue. There's no reason to order by ID imho else it would be problematic for plugins editing the date recorded of their activities.

#5 @henry.wright
4 years ago

If you need to bulk import activity, you should use the date_recorded field...

Sounds reasonable to me. I could space each activity item 1 second apart.

Shall we close this?

Last edited 4 years ago by henry.wright (previous) (diff)

#6 @boonebgorges
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 2.5

There's no reason to order by ID imho else it would be problematic for plugins editing the date recorded of their activities.

I think you're misunderstanding what @henry.wright is suggesting. He's saying that we should use the id column as a *secondary* sort - a tie-breaker. I think it's a good idea, both here and elsewhere in BP where we're sorting by date. See https://core.trac.wordpress.org/ticket/30480, https://core.trac.wordpress.org/ticket/30478

@Mamaduka
4 years ago

@Mamaduka
4 years ago

#7 @Mamaduka
4 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

6720.diff adds secondary sort parameter like suggested in comments.

#8 @boonebgorges
4 years ago

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

In 10435:

In activity queries, use id column for secondary sort.

The default sort order for activity queries is by date_recorded. But this
order can be indeterminate when two items share the same date_recorded. To
prevent indeterminacy, we fall back on the always unique id column.

Fixes #6720.
Props henry.wright, Mamaduka.

Note: See TracTickets for help on using tickets.