Subnav menu with 'user_has_access' set to false should redirect but doesn't

[r-a-y]

In #6534, we refactored the way we register menu items with the BP Navigation API.

Unfortunately, we broke functionality to redirect subnav menu items with user_has_access set to false.

Example code:

<?php
add_action( 'bp_setup_nav', function() {
        bp_core_new_nav_item( array(
                'name'                    => 'Secret Stash',
                'slug'                    => 'secret-stash',
                'screen_function'         => 'my_secret_stash_screen',
                'default_subnav_slug'     => 'my-stash',
                'show_for_displayed_user' => false,
        ) );

        bp_core_new_subnav_item( array(
                'name'            => 'My Stash',
                'slug'            => 'my-stash',
                'parent_url'      => trailingslashit( bp_displayed_user_domain() . 'secret-stash' ),
                'parent_slug'     => 'secret-stash',
                'user_has_access' => bp_is_my_profile(),
                'no_access_url'   => is_user_logged_in() ? bp_loggedin_user_domain() : '',
                'screen_function' => 'my_secret_stash_screen'
        ) );
} );

function my_secret_stash_screen() {
        add_action( 'bp_template_content', function() {
                echo "Ssh! It's a secret!";
        } ) ;
        bp_core_load_template( 'members/single/plugins' );
}

Expected behavior:
If you are logged in and you attempt to view another person's stash (example.com/members/ANOTHER-USER/secret-stash/), you should be redirected back to your profile page with an error message.

If you are logged out and you attempt to view the same URL, you should get redirected to login. On success, you can view your stash. If you login with another account, you get redirected back to your profile page with an error message.

Current behavior:
You get a 404 page when you view that URL.

---

The problem:

In #6534, we missed a portion of the refactor in bp_core_register_subnav_screen_function().

$parent_nav is an array that needs to use the reset() function so we grab the actual nav object.

See other areas in bp-core-buddybar.php that follows the same pattern:
​https://buddypress.trac.wordpress.org/browser/tags/2.9.2/src/bp-core/bp-core-buddybar.php?marks=308-315,506-514,683-686#L308

Attached patch fixes this up.

Patch also fixes the redirect message to show up on frontend pages (the mode portion of the patch) and alters the strings to better reflect what is happening (that instead of this).

[DJPaul]

Sure, you've done more work here than I have, so if this the way to go, let's do it.

[r-a-y]

#7552 was marked as a duplicate.

Going to commit this now.

[r-a-y]

In 11814:

Navigation: Restricted subnav menu pages should redirect instead of throwing a 404.

Back in BuddyPress 2.6.0, we refactored the way we register menu items with
the BP Navigation API (see #6534).

The problem is we missed an instance during the refactor. This caused
restricted subnav pages (those set with the 'user_has_access' parameter
to false in bp_core_new_subnav_item()) to 404 instead of being redirected
away.

Commit fixes this problem.

See #7659.

[r-a-y]

In 11815:

Navigation: Show redirect message on restricted subnav menu pages.

Commit also alters the message strings to better reflect what is happening
("that page" instead of "this page").

See #7659.

[boonebgorges]

[11814] broke compatibilty in cases where a plugin registered a subnav item *before* its corresponding parent. In most cases this doesn't make much sense, but in this specific case - where the plugin was using bp_core_new_subnav_item() to add a *group* subnav item - it's a little less silly. Anyway, the plugin was doing something like this:

add_action( 'bp_setup_nav', function() {
    if ( ! bp_is_group() )
        return;

    bp_core_new_subnav_item( array(
        'parent_slug' => $current_group->slug, // etc
    ) );
} );

and because of WP's load order rules, this plugin happened to load before the Groups component was fully set up.

The solution for this plugin is pretty simple: move to a later priority, after core components have had a chance to set up. So I'm not sure this is a bug worth worrying about, which is why I'm reporting it here rather than in a new ticket. But I wanted to record it for posterity.

[r-a-y]

[11814] broke compatibilty in cases where a plugin registered a subnav item *before* its corresponding parent.

Good call! This issue was also raised in #7931. There's a patch in that ticket.

Can you test your previous plugin with that patch to see if it will work, @boonebgorges ?