[buddypress-trac] [BuddyPress Trac] #7218: Only load component action and screen code when we're on the component's page

[buddypress-trac]

#7218: Only load component action and screen code when we're on the component's
page
-------------------------+-----------------------
 Reporter:  r-a-y        |       Owner:
     Type:  enhancement  |      Status:  reopened
 Priority:  normal       |   Milestone:  3.0
Component:  Core         |     Version:
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |
-------------------------+-----------------------
Changes (by r-a-y):

 * keywords:  has-patch early => has-patch


Comment:

 `activity-original.patch` is my original patch updated for trunk.

 `activity-ray-ocd.patch` is basically my OCD version of my original idea.
 `bp-activity-actions.php` and `bp-activity-screens.php` have been split up
 into separate files in the new `bp-activity/actions/` and `bp-
 activity/screens/` directories and these files are loaded depending on the
 current page.  This is the patch I'm recommending.  Note: I haven't added
 PHPDoc file headers for each new file yet, but I'll do so once we're ready
 to commit.

 For unit tests, I ran into a bug with our mention unit tests (#7703).  The
 patch in that ticket should be committed before applying this patch and
 running PHPUnit.  Also, PHPUnit does not like conditional file loading
 when running multiple tests.  For an example, see
 https://laracasts.com/discuss/channels/testing/testing-single-method-
 works-but-multiple-methods-fail.  It appears that all files must be
 included at once before running our unit test suite.  Instead of
 cluttering up the `BP_Activity_Component` class with this PHPUnit logic,
 I'm including all action and screen code in our PHPUnit loader file.

 ----

 For the `late_includes()` method, as I mentioned in the ticket
 description, I've chosen to run this on the `'bp_setup_canonical_stack'`
 hook at priority `20` because that is the earliest when
 `bp_current_component()` and `bp_current_action()` is fully set up and
 should be safe to use conditional fucntions like
 `bp_is_current_component()`.  @boonebgorges chose `'bp_template_redirect'`
 at priority `0`, which runs later, but this might be too late for plugins
 that have chosen to remove hooks on `'bp_init'` at the default priority of
 `10`.

 Also in Boone's `7218.diff`, I'm not a fan of the existing
 `BP_Component::includes()` method as it does too many checks for
 superfluous files (view #BB3193 to see what I mean).  So I'm not a fan of
 re-using this logic for the `late_includes()` method.  My
 `late_includes()` method just includes files with the `require` line, no
 need to be fancy here.

 Let me know what you think.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7218#comment:58>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac