Possible regression in bp_core_new_subnav_item()

[dontdream]

The attached code snippet adds a new tab to a members's Settings menu, and works fine on BP 2.9.4 and earlier versions.

On BP 3.0.0 and 3.1.0 it adds the new tab but, when I click the tab link, I get a 404 page instead of the tab content.

Reverting changeset 11814 in bp-core-buddybar.php makes the code work again.

Thanks for looking into this!

[dontdream]

demonstrates possible regression in bp_core_new_subnav_item()

[r-a-y]

Thanks for the report.

You might want to check out the note here:
​https://buddypress.trac.wordpress.org/ticket/7659#comment:7

The problem is hooking to 'bp_setup_nav'. Try hooking to 'bp_settings_setup_nav' or 'bp_setup_nav' at a later priority as you want to ensure that the Settings nav is setup before registering your subnav.

[r-a-y]

Actually, you are right, @dontdream !

Instead of returning if no parent nav is found, we can check if there is a parent nav, if so, we can use reset().

I tested the example code in #7659 and your test example and both test cases work.

[boonebgorges]

I think that 7931.01.patch​ will work for the issue described in ​https://buddypress.trac.wordpress.org/ticket/7659#comment:7. Can you think of any problems that might arise from this "loosening"? That is, we'll no longer be rejecting items registered against non-existent parents. I think that these items would just be ignored during normal rendering anyway, but it's worth thinking about.

[r-a-y]

I think we should be safe due to the conditional checking for the slug with bp_is_current_action() and with the parent nav screen function check:
​https://buddypress.trac.wordpress.org/browser/tags/3.1.0/src/bp-core/bp-core-buddybar.php?marks=617,618,625#L588

[dontdream]

I've tested 7931.01.patch and it works for me too. Thank you!

[r-a-y]

I've added a unit test that exhibits the problem.

@boonebgorges - Can you go over the naming of the unit test? I'm not usually good at naming these things.

[boonebgorges]

Test and name seems fine to me :)

[r-a-y]

In 12194:

Navigation: Ensure subnav menus can be registered before the parent nav is registered.

In #7659, a bug was fixed where restricted subnav menus should be
redirected instead of throwing a 404. The side-effect of the fix is this
broke how plugins can register subnav menus before the parent nav is
created.

This commit rectifies the problem by not bailing out if the parent nav
cannot be found during subnav menu registration. Includes a unit-test.

Props dontdream, boonebgorges, r-a-y.

Fixes #7931 (3.x branch).

[r-a-y]

In 12195:

Navigation: Ensure subnav menus can be registered before the parent nav is registered.

In #7659, a bug was fixed where restricted subnav menus should be
redirected instead of throwing a 404. The side-effect of the fix is this
broke how plugins can register subnav menus before the parent nav is
created.

This commit rectifies the problem by not bailing out if the parent nav
cannot be found during subnav menu registration. Includes a unit-test.

Props dontdream, boonebgorges, r-a-y.

See #7931 (trunk).