Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 10 years ago

#5533 closed enhancement (fixed)

BP_Group_Extension::display() should be passed the group_id

Reported by: boonebgorges's profile boonebgorges Owned by: boonebgorges's profile boonebgorges
Milestone: 2.2 Priority: normal
Severity: normal Version:
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 years ago.
5533.2.patch (1.1 KB) - added by fahmiadib 10 years ago.

Download all attachments as: .zip

Change History (10)

#1 @DJPaul
10 years ago

  • Keywords good-first-bug added

#2 @DJPaul
10 years 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.

#3 @DJPaul
10 years ago

  • Milestone changed from 2.1 to 2.2

@fahmiadib
10 years ago

#4 @fahmiadib
10 years 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 years ago by fahmiadib (previous) (diff)

#5 @boonebgorges
10 years 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()?

#6 @fahmiadib
10 years 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.

@fahmiadib
10 years ago

#7 @boonebgorges
10 years 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!

#8 @boonebgorges
10 years 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.