Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#5938 closed enhancement (fixed)

BP Activity Hooks Documentation

Reported by: tw2113 Owned by:
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc:

Description

Changing this process up. Going to create one ticket per component, instead of one ticket per file.

This ticket will be where all BP Activity patches go for the Hooks Documentation.

Attachments (11)

bp-activity-admin-5938.diff (8.4 KB) - added by tw2113 5 years ago.
Initial pass at bp-activity-admin.php
bp-activity-akismet-5938.diff (4.2 KB) - added by tw2113 5 years ago.
Diff file for the akismet file hooks
bp-activity-cssjs-5938.diff (890 bytes) - added by tw2113 5 years ago.
bp-activity-cssjs.php hook documentation
bp-activity-classes-5938.diff (12.1 KB) - added by tw2113 5 years ago.
Diff file for the activity classes file hooks
bp-activity-filters-5938.diff (5.9 KB) - added by tw2113 5 years ago.
Diff file for the activity filters file hooks
bp-activity-loader-5938.diff (641 bytes) - added by tw2113 5 years ago.
bp-activity-screens-5938.diff (11.1 KB) - added by tw2113 5 years ago.
bp-activity-notifications-5938.diff (11.0 KB) - added by tw2113 5 years ago.
bp-activity-functions-5938.diff (20.5 KB) - added by tw2113 5 years ago.
bp-activity-template-5938.diff (74.2 KB) - added by tw2113 5 years ago.
bp-activity-template-5938-1.diff (74.2 KB) - added by tw2113 5 years ago.

Download all attachments as: .zip

Change History (41)

@tw2113
5 years ago

Initial pass at bp-activity-admin.php

#1 @boonebgorges
5 years ago

In 9078:

Add hook documentation to bp-activity-admin.php.

Props tw2113.
See #5938.

#2 @tw2113
5 years ago

Relevant Files:
bp-activity-actions.php
bp-activity-admin.php
bp-activity-akismet.php
bp-activity-cache.php
bp-activity-classes.php
bp-activity-cssjs.php
bp-activity-filters.php
bp-activity-functions.php
bp-activity-loader.php
bp-activity-notifications.php
bp-activity-screens.php
bp-activity-template.php

Started, pending done: bp-activity-actions.php (https://buddypress.trac.wordpress.org/ticket/5936)
Done: bp-activity-admin.php

#3 @DJPaul
5 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Type changed from enhancement to task

Moving into Future Release; we can keep these here and move them into particular releases' milestones as we commit things (or work on them).

@tw2113
5 years ago

Diff file for the akismet file hooks

@tw2113
5 years ago

bp-activity-cssjs.php hook documentation

#4 @tw2113
5 years ago

With bp-activity-classes.php, how do we want to handle the save method? I'm counting 14 filters right at the start that are actually nicely grouped together. Right below it is a do_action_ref_array() with the comment of

Use this, not the filters above.

#5 @tw2113
5 years ago

Also curious how we want to handle the bp_activity_get_user_join_filter as it's used in two places in bp-activity-classes.php but conditionally has some extra parameters passed through.

#6 @boonebgorges
5 years ago

In 9086:

Hook documentation for bp-activity-akismet.php and bp-activity-cssjs.php.

Props tw2113.
See #5938.

#7 @boonebgorges
5 years ago

With bp-activity-classes.php, how do we want to handle the save method? I'm counting 14 filters right at the start

Let's not bother documenting the filters that we're encouraging should not be used. Just do the one with do_action_ref_array().

Also curious how we want to handle the bp_activity_get_user_join_filter as it's used in two places in bp-activity-classes.php but conditionally has some extra parameters passed through.

This is going to pose problems for the parser. Let's do the following: create full documentation for the hook with the most params; mark the other one with the /* This filter is documented in... */ notation; then open a ticket for that specific hook so we can add the additional params to the hook that doesn't have them. (All hooks should have the same arguments passed to them.)

#8 @boonebgorges
5 years ago

In 9087:

Pass $pag_sql variable to 'bp_activity_get_user_join_filter'.

There are two instances of this filter, and the first one had this $pag_sql
variable passed to it, while it was missing in the second case (where there is
no pagination). For greater consistency in documentation and plugin
development, we match the format of the variables passed to each instance of
the filter.

See #5938.

#9 @boonebgorges
5 years ago

I've gone ahead and made the change for 'bp_activity_get_user_join_filter', but please do let us know when/if this comes up in the future - I consider it a bug. Thanks so much for your continued work on this!

#10 @tw2113
5 years ago

Noted on the save method, sounds like best path. Will do on duplicate filters with different params.

@tw2113
5 years ago

Diff file for the activity classes file hooks

@tw2113
5 years ago

Diff file for the activity filters file hooks

#11 @tw2113
5 years ago

From what I can tell, all that's left for the Activity component is bp-activity-functions, bp-activity-notifications, bp-activity-screens, and bp-activity-template. I have most of notifications done, though I have some spots that I'm not sure how to handle quite yet. The other 3 should not be touched quite yet.

Inside the bp_activity_format_notifications function, there are two places where the filter name is assigned to a variable and the variable is passed into the apply_filters() first param. How would you like me to handle that one? Put the docbloc above the apply_filters() line still and make note of the two possible filter names?

I'm also not sure what is getting passed in as the $params array for bp_activity_new_comment_notification, so some help with that one would be appreciated to determine what type of data it holds.

This ticket was mentioned in Slack in #buddypress by tw2113. View the logs.


5 years ago

#13 @boonebgorges
5 years ago

In 9110:

Hook documentation for bp-activity-classes.php.

Props tw2113.
See #5938.

#14 @boonebgorges
5 years ago

In 9111:

Hook documentation for bp-activity-filters.php.

Props tw2113.
See #5938.

#15 @boonebgorges
5 years ago

In 9112:

Hook documentation for bp-activity-loader.php and bp-activity-screens.php.

Props tw2113.
See #5938.

#16 @tw2113
5 years ago

Naturally, the last hook I have, is a bit odd. :)


bp_get_activity_show_filters

We currently have the following, with a different argument list passed for both cases.

/**
 * Filters the options available in the activity filter dropdown.
 *
 * @since BuddyPress (2.1.0)
 *
 * @param array $filters Array of filter options for the given context, in the following format: $option_value => $option_name
 * @param string $context Context for the filter. 'activity', 'member', 'member_groups', 'group'.
 */
$filters = apply_filters( 'bp_get_activity_show_filters', $filters, $context );

// Build the options output
$output = '';

if ( ! empty( $filters ) ) {
	foreach ( $filters as $value => $filter ) {
		$output .= '<option value="' . esc_attr( $value ) . '">' . esc_html( $filter ) . '</option>' . "\n";
	}
}

return apply_filters( 'bp_get_activity_show_filters', $output, $filters, $context );

Not sure if you, once again, want to try and normalize the parameters, or have an idea for a different solution.

#17 @boonebgorges
5 years ago

Shoot, this is a bug. The first one filters the options, the second filters the markup. As it stands, it's actually pretty much impossible for this filter to work - if you expect markup, you'll mess up the value passed to the first filter, while if you expect an array, you'll mess up the value passed to the second one. That said, naming conventions suggest that we should change the name of the first one, to keep the second one consistent with the function name. Let me fix that.

#18 @boonebgorges
5 years ago

In 9133:

Remove duplicate 'bp_get_activity_show_filters' filter.

'bp_get_activity_show_filters_options' now filters the options used to build
the options markup, while 'bp_get_activity_show_filters' filters the markup.

See #5938.

#19 @tw2113
5 years ago

  • Milestone changed from Future Release to 2.2

Just uploaded the final file I had for the Activity component. So once bp-activity-notifications-5938.diff, bp-activity-functions-5938.diff, and bp-activity-template-5938.diff are committed, the Activity component will have all relative hooks done, excluding deprecated/non-encouraged ones.

#20 @boonebgorges
5 years ago

In 9134:

Add hook documentation to bp-activity-notifications.php.

See #5938.
Props tw2113.

#21 @boonebgorges
5 years ago

In 9135:

Add hook documentation in bp-activity-functions.php.

Props tw2113.
See #5938.

#22 @tw2113
5 years ago

Should have better coverage using $value in place of parameters not passed in via variable, in bp-activity-template-5938-1.diff

#23 @boonebgorges
5 years ago

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

Typo in [9136].

#24 @tw2113
5 years ago

In 9259:

Add missing @since tag for activity_loop_start.

See #5938.

#25 @tw2113
5 years ago

In 9385:

Fix missed capitalization on the P in BuddyPress in phpdoc.

See #5938.

#26 @tw2113
4 years ago

In 9540:

Updates filter docs from changes made in #9536.

See #5938.

#27 @tw2113
4 years ago

In 9642:

Fixes double digit version numbers missed from hooks docs work.

See #5938.

#28 @tw2113
4 years ago

In 9644:

Fixes double digit version numbers missed from hooks docs work.

See #5938.

#29 @tw2113
4 years ago

In 9645:

Fixes double digit version numbers missed from hooks docs work.

See #5938.

#30 @DJPaul
3 years ago

  • Type changed from task to enhancement
Note: See TracTickets for help on using tickets.