Skip to:
Content

#5533 closed enhancement (fixed)

BP_Group_Extension::display() should be passed the group_id

Reported by: boonebgorges Owned by: boonebgorges
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Component - Groups Keywords: good-first-bug has-patch
Cc:

Description

This is for greater parity with settings_screen(), etc. See http://buddypress.org/support/topic/groups-extension-api-question-troubleshooting/

Attachments (2)

5533.1.patch (1.2 KB) - added by fahmiadib 10 months ago.
5533.2.patch (1.1 KB) - added by fahmiadib 10 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 @DJPaul15 months ago

  • Keywords good-first-bug added

comment:2 @DJPaul13 months ago

For anyone looking to work on this:

  • The first thing to do is update the function stub in the BP_Group_Extension class in bp-groups-classes.php with the new optional parameter.
  • Then check for/update any built-in implementations of this class (if there are any) that declare the function display( and add the optional parameter.
  • The display() method is called by the _display_hook method in this same class. Find out where this action is invoked from, and pass the Group ID to the action. And, clearly, you'll need to figure out the best way to get the Group ID, but finding where the method is called should help you figure out the best option.

comment:3 @DJPaul13 months ago

  • Milestone changed from 2.1 to 2.2

@fahmiadib10 months ago

comment:4 @fahmiadib10 months ago

  • Keywords has-patch added

I stumbled into this ticket when i was playing with Group Extension API. Am i in the right track here with the patch?

Patch:

  • Added new optional parameter $group_id in display() function stub in the BP_Group_Extension class
  • Modify _display_hook() to call new function call_display() which then call display() while passing $group_id
Last edited 10 months ago by fahmiadib (previous) (diff)

comment:5 @boonebgorges10 months ago

Yeah, this looks pretty good - thanks for the patch, fahmiadib! Any reason we can't just do $this->display( $this->group_id ) instead of call_user_func()?

comment:6 @fahmiadib10 months ago

After looking a little bit more, i agree with you. call_user_func() is used to call function from string. I will submit another patch.

@fahmiadib10 months ago

comment:7 @boonebgorges10 months ago

  • Owner set to boonebgorges
  • Status changed from new to accepted

Nice - thanks, fahmiadib! This is going to cause PHP notices for plugins currently registering BP_Group_Extension::display() methods when strict mode is enabled, but oh well, we have done this once before. Thanks for the patch!

comment:8 @boonebgorges10 months ago

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

In 9130:

Pass current group ID to BP_Group_Extension::display().

This creates greater parity with the other display methods in the class.

Props fahmiadib.
Fixes #5533.

Note: See TracTickets for help on using tickets.