Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

#5409 closed defect (bug) (no action required)

Changes to BP_Activity_Activity::get_last_updated() to get last id depending on contexts

Reported by: imath Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Activity Keywords: has-patch
Cc:

Description

This patch suggests to modify BP_Activity_Activity::get_last_updated() in order to be able to get the last updated id for a context defined in arguments.
This is a reply to the 3rd point of DJPaul in his comment to #5328

PS : i haven't find a core function to help us build the query, there's the method BP_Activity_Activity::get_id() that is approaching but would need to be changed to edit the search argument and to add an exclude argument, as 'last_activity' is a type of activity the "load newest" feature needs to exclude.

Attachments (1)

5409.diff (6.5 KB) - added by imath 8 years ago.

Download all attachments as: .zip

Change History (6)

@imath
8 years ago

#1 @boonebgorges
8 years ago

In 7922:

Unit test for BP_Activity_Activity::get_last_updated()

See #5409

#2 follow-up: @boonebgorges
8 years ago

  • Keywords reporter-feedback added

imath - Thanks for putting this into a separate ticket.

I'm not convinced that extending get_last_updated() in this way is appropriate. If I understand https://buddypress.trac.wordpress.org/attachment/ticket/5328/5328.02.diff correctly, you could just use bp_activity_get() for your purposes. In bp_activity_heartbeat_last_recorded():

$activities = bp_activity_get( array(
    'search_terms'      => $data['bp_last_activity_update']['search'],
    'filter'            => $filter,
    'show_hidden'       => $show_hidden,
    'max'               => 1,
    'per_page'          => 1,
    'update_meta_cache' => false,
) );

if ( ! empty( $activities['activities'] ) ) {
    $response['last_id_recorded'] = $activities['activities'][0]->id;
}

It seems to me that we should move toward centralizing all of our component queries when possible, not reproducing query logic. This looks to me like a case where we can easily leverage the tools we already have. What do you think, imath?

#3 in reply to: ↑ 2 @imath
8 years ago

  • Keywords reporter-feedback removed

Replying to boonebgorges:

What do you think, imath?

I think i was too obsessed by only getting the id and no other data! I really need to explore the way you suggest and "rethink" the way i was loading newest activities. Thanks a lot boonebgorges :)
Excited! I'll check this asap!!

#4 @imath
8 years ago

I confirm, it's best to use bp_activity_get() :)
Actually as i explained in point a) of this comment on #5328 i'm using an activity loop.

As you've added unit test, i'm not sure i should close this ticket.

#5 @boonebgorges
8 years ago

  • Milestone 2.0 deleted
  • Resolution set to invalid
  • Status changed from new to closed

Cool, thanks!

I wrote the unit test knowing that we might close this ticket as invalid. It doesn't hurt to have it :)

Note: See TracTickets for help on using tickets.