Skip to:
Content

Opened 13 months ago

Closed 6 months ago

#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 7 months ago.
5533.2.patch (1.1 KB) - added by fahmiadib 7 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 @DJPaul12 months ago

  • Keywords good-first-bug added

comment:2 @DJPaul10 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 @DJPaul9 months ago

  • Milestone changed from 2.1 to 2.2

@fahmiadib7 months ago

comment:4 @fahmiadib7 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 7 months ago by fahmiadib (previous) (diff)

comment:5 @boonebgorges7 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 @fahmiadib7 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.

@fahmiadib7 months ago

comment:7 @boonebgorges6 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 @boonebgorges6 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.