Skip to:
Content

Opened 5 years ago

Closed 3 years ago

Last modified 2 years ago

#5138 closed enhancement (fixed)

Audit & replace $bp global touches with wrapper functions & methods

Reported by: johnjamesjacoby Owned by: johnjamesjacoby
Milestone: 2.3 Priority: normal
Severity: normal Version: 1.0
Component: Route Parser Keywords:
Cc:

Description (last modified by johnjamesjacoby)

BuddyPress touches the $bp global directly countless times. In the past, this was fine enough, as it gave us an easy and reliable way to get global data relative to BuddyPress's current state. As BuddyPress becomes more robust, this is becoming more fragile, more difficult to extend, and a bottleneck for future enhancements to the software.

One specific case is adding rewrite rules, a feature slated for 1.8, bumped to 1.9.

Because we directly reference $bp->loggedin_user->domain inside templates, functions, methods, and classes, translating that reference into an unpretty permalink becomes problematic, as existing plugins have come to expect that it is always a pretty URL.

Another case is the $bp->pages array. As we introduce rewrite rules, our dependance on this array will disappear, though because we reference it directly in 60+ places, we're forced to touch these references rather than using a function and replacing its guts.

There are numerous other examples that require ongoing clean-up, both related and unrelated to adding rewrite rules in 1.9 and beyond. I'm creating this ticket as a general task to patch and commit changes to $bp touches towards.

Change History (21)

#1 @johnjamesjacoby
5 years ago

  • Description modified (diff)

#2 @johnjamesjacoby
5 years ago

$bp->loggedin_user->domain ended up being a bad example, as I cleaned most of that up during the epic 1.5 release.

#3 @boonebgorges
5 years ago

  • Milestone changed from 1.9 to 2.0

#4 @johnjamesjacoby
4 years ago

In r7749:

Replace $bp global references in slug functions with buddypress() calls. See #5138.

#5 @johnjamesjacoby
4 years ago

In 7751:

Replace $bp global references in slug functions with buddypress() calls. See #5138. (trunk)

#6 @johnjamesjacoby
4 years ago

In 8181:

Remove $bp global from core messages functions. See #5138.

#7 @johnjamesjacoby
4 years ago

In 8182:

Replace $bp global touches in core component functions with buddypress() calls. See #5138.

#8 @boonebgorges
4 years ago

  • Milestone changed from 2.0 to 2.1

#9 @DJPaul
4 years ago

  • Milestone changed from 2.1 to Future Release

#10 @johnjamesjacoby
3 years ago

In 9373:

Coding standards improvements to BP_Activity_Activity::save(). See #5138, #5891.

#11 @johnjamesjacoby
3 years ago

In 9374:

Add brackets and remove $bp global touch in BP_Activity_Activity::get_id(). See #5138, #5891.

#12 @johnjamesjacoby
3 years ago

In 9377:

Harden BP_Activity_Activity::rebuild_activity_comment_tree() by using intval() on recursively returned values, and use wp_list_pluck() to avoid touching object id directly. Also remove $bp global touch. See #5138.

#13 @johnjamesjacoby
3 years ago

In 9378:

Remove $bp global touches in bp-activity-classes.php. See #5138.

#14 @DJPaul
3 years ago

I just ran Scrutiniser to evaluate these changes (specifically r9377) and it thinks Activity::rebuild_activity_comment_tree() got "worse" though it is not clear to me why it thinks that: https://scrutinizer-ci.com/g/buddypress/BuddyPress/inspections/979b3a27-156d-4e63-9758-7fd68adf831c

#15 @johnjamesjacoby
3 years ago

Looks like in the case of the save() method, it's due to the added length of the method thanks to the extra brackets. Either way, what we have now conforms and is easier on the eyes.

#16 @johnjamesjacoby
3 years ago

In 9470:

Beautify /deprecated/2.1.php:

  • Add brackets
  • Remove $bp global touches (See #5138)
  • Use bp_is_root_blog() rather than direct comparisons to bp_get_root_blog()
  • Easier to navigate when testing & debugging deprecated functionality

#17 @johnjamesjacoby
3 years ago

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

In 9471:

Replace all remaining $bp global touches with buddypress().

All existing tests continue to pass as normal. I will further manually scrutinize each replacement to ensure correctness.

Fixes #5138. Any stragglers or updates will reference this ticket.

#18 @DJPaul
3 years ago

  • Keywords needs-patch removed
  • Milestone changed from Future Release to 2.3

#19 @johnjamesjacoby
3 years ago

In 9536:

Activity: Clean up bp_has_activities():

  • Remove $bp global usage. See #5138.
  • Remove extract() usage. See #5698.
  • Remove one-time-use variables.
  • Consolidate arrays for improved readability.

#20 @johnjamesjacoby
3 years ago

In 9649:

Activity: Remove unused $bp references from bp-activity-functions.php. See #5138.

#21 @DJPaul
2 years ago

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