Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 7 years ago

#3886 closed defect (bug) (fixed)

Audit class methods and mark appropriately as static

Reported by: kayue Owned by:
Milestone: 1.9 Priority: normal
Severity: normal Version:
Component: Core Keywords: needs-patch
Cc:

Description (last modified by DJPaul)

Non-static method BP_Core_Notification::get_all_for_user() should not be called statically in /buddypress/bp-members/bp-members-notifications.php line 51

Change History (11)

#1 @kayue
9 years ago

To fix this, in bp-core-classes.php, should add a static keyword before declare the function.

/

  • Fetches all the notifications in the database for a specific user. *
  • @global object $bp Global BuddyPress settings object
  • @global wpdb $wpdb WordPress database object
  • @param integer $user_id User ID
  • @return array Associative array
  • @static */

static function get_all_for_user( $user_id ) {

global $bp, $wpdb;

return $wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE user_id = %d AND is_new = 1", $user_id ) );

}

#2 @DJPaul
9 years ago

  • Description modified (diff)
  • Keywords 1.7-early added
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Non-static method BP_Core_Notification::get_all_for_user() should not be called statically to Audit class methods and mark appropriately as static

We need to look at all of BuddyPress, and mark class methods as static where appropriate. There was a conversation on IRC (https://irclogs.wordpress.org/chanlog.php?channel=buddypress-dev&day=2011-12-21#m155663) between Spadeski and boonebgorges recently about the same thing.

#3 @DJPaul
9 years ago

  • Description modified (diff)

Marking this ticket for 1.7 so we can do all of these at the same time, and make sure we have plenty of time to check for any breakages.

#4 @boonebgorges
8 years ago

  • Keywords 1.7-early removed
  • Milestone changed from Future Release to 1.7
  • Priority changed from normal to low

Moving to 1.7 for further review. Note that we have to make sure that marking items static doesn't break plugins that are extending our core classes, as PHP will throw a fit if you override a method without declaring access levels, etc properly.

#5 @johnjamesjacoby
8 years ago

  • Milestone changed from 1.7 to 1.8

Going to move this to 1.8. There's no complaints or traction in 12 months, so we can iterate on this more in the future.

#6 @boonebgorges
7 years ago

  • Component changed from Notifications to Core
  • Milestone changed from 1.8 to Future Release
  • Priority changed from low to lowest
  • Severity changed from minor to trivial

#7 @boonebgorges
7 years ago

  • Milestone changed from Future Release to 1.9
  • Priority changed from lowest to normal
  • Severity changed from trivial to normal

Because of issues described in #5108, this has become much more of an annoying and pressing issue. Let's look at it in greater depth for 1.9.

#8 @boonebgorges
7 years ago

In 7316:

Explicit PHP5 visibility and static declarations for bp-groups-classes.php

See #5108, #3886

#9 @boonebgorges
7 years ago

In 7317:

Explicit visibility and static declarations in bp-blogs-classes.php

See #3886, #5108

#10 @r-a-y
7 years ago

See #5108. Marking as resolved to clear the milestone.

#11 @r-a-y
7 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.