Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#5944 closed enhancement (fixed)

BP Members Hooks Documentation

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

Description

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

Relevant files:
bp-members-actions.php
bp-members-admin.php
bp-members-adminbar.php
bp-members-classes.php
bp-members-filters.php
bp-members-functions.php
bp-members-loader.php
bp-members-screens.php
bp-members-template.php
admin/bp-members-classes.php

Attachments (1)

bp-members-admin-5944.diff (7.4 KB) - added by tw2113 5 years ago.

Download all attachments as: .zip

Change History (26)

#1 @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).

#2 @tw2113
5 years ago

I am hoping someone familiar with the Members component could do a quick QA review of the attached diff file. I'm a little hesitant on my accuracy for what some of the hooks are for.

#3 @boonebgorges
5 years ago

A bit of feedback:

  • Let's capitalize CSS, JS, and URL in documentation. Also, please camel-case "JavaScript".
  • 'bp_members_admin_css' is not a dynamic filter, at least not in the sense that other filters in BP are dynamically generated. The value passed to the filter is of course dynamic, but the value passed to pretty much every filter is dynamic.
  • I think we shouldn't be putting line breaks in short descriptions. I'm pretty sure that messes up the parser. When in doubt, run through https://github.com/rmccue/WP-Parser.

#4 @tw2113
5 years ago

Everything else looks good though for what the hooks are meant to allow or when they're supposed to run?

#5 @boonebgorges
5 years ago

Yes, I think the descriptions are right.

#6 @tw2113
5 years ago

In 9233:

Add tests for bp_make_spam/ham_user and make_spam/ham_user filters.

Filters names were originally assigned to a passed in variable but
they were separated out for documentation purposes.

See #5944.

#7 follow-up: @tw2113
5 years ago

Conflicted regarding the bp_core_signup_blog filter.

return apply_filters( 'bp_core_signup_blog', wpmu_signup_blog( $blog_domain, $blog_path, $blog_title, $user_name, $user_email, $usermeta ) );

The conflict comes from the fact that wpmu_signup_blog doesn't actually return anything, so we're filtering nothing and also returning nothing.

Luckily, we only use the function in one location, and the variable the return value is assigned to is not used again.

#8 @DJPaul
5 years ago

The r9233 tests are failing when run in multisite. Trying to figure it out...

#9 @djpaul
5 years ago

In 9234:

Tests: after using our grant_super_admin helper, use restore_admins after the assertion to tidy up the $super_admins global.

This was otherwise breaking the seemingly unrelated tests added in r9233 when run in multisite. See #5944

#10 in reply to: ↑ 7 @boonebgorges
5 years ago

Replying to tw2113:

Conflicted regarding the bp_core_signup_blog filter.

return apply_filters( 'bp_core_signup_blog', wpmu_signup_blog( $blog_domain, $blog_path, $blog_title, $user_name, $user_email, $usermeta ) );

The conflict comes from the fact that wpmu_signup_blog doesn't actually return anything, so we're filtering nothing and also returning nothing.

Luckily, we only use the function in one location, and the variable the return value is assigned to is not used again.

Good catch, and groan. I'd say "rip it out" but it's possible that someone is actually using that filter to provide a return value in some cases. I'd suggest doing your normal documentation, and noting in the long description that the filter returns no value by default, and is left in place for backward compatibility.

#11 follow-up: @tw2113
5 years ago

Pretty well done with this component, and pending diff review before commit.

#12 in reply to: ↑ 11 @boonebgorges
5 years ago

Replying to tw2113:

Pretty well done with this component, and pending diff review before commit.

Did you mean to attach a diff for review?

#13 @tw2113
5 years ago

Nope, that's just me saying pre-commit review to check for periods, alignment, spelling, etc. General final QA on my part.

#14 @tw2113
5 years ago

In 9265:

Add hook documentation in admin/bp-memmbers-classes.php.

Props tw2113.
See #5944.

#15 @tw2113
5 years ago

In 9266:

Add hook documentation in bp-members-actions.php.

Props tw2113.
See #5944.

#16 @tw2113
5 years ago

In 9267:

Add hook documentation in bp-members-admin.php.

Props tw2113.
See #5944.

#17 @tw2113
5 years ago

In 9268:

Add hook documentation in bp-members-classes.php.

Props tw2113.
See #5944

#18 @tw2113
5 years ago

In 9269:

Add hook documentation in bp-members-filters.php.

Props tw2113.
See #5944.

#19 @tw2113
5 years ago

In 9270:

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

Props tw2113.
See #5944.

#20 @tw2113
5 years ago

In 9271:

Add hook documentation in bp-members-screens.php.

Props tw2113.
See #5944.

#21 @tw2113
5 years ago

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

In 9272:

Add hook documentation in bp-members-template.php.

Props tw2113.
Fixes #5944.

#22 @tw2113
5 years ago

  • Milestone changed from Future Release to 2.2

#23 @tw2113
5 years ago

In 9274:

Add hooks documentation to bp-members-activity.php.

See #5944

#24 @tw2113
5 years ago

In 9526:

Adds hooks documentation to three Member widgets.

See #5944.

#25 @DJPaul
3 years ago

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