[legacy template pack]: Ajax pagination and filters for group members list shows all members

[jdgrimes]

Steps to reproduce:

• First be sure that you are using the Legacy template pack, and that JS is enabled in your browser.
• Go to the Members tab for a group. Note the number of members and number of pages.
• Then either click on page 2 (or another page), or change the selection in the order by dropdown.

Expected result:

The next page is displayed or the members list is reordered, respectively.

Actual result:

The Ajax refresh will result in all members being included in the list, not just the members of the group.

Tested on both 2.9.3 and trunk.

The cause:

The problem appears to be that bp_legacy_theme_object_template_loader() contains this:

<?php

        if ( ! empty( $_POST['template'] ) && 'groups/single/members' === $_POST['template'] && 'members' === $object ) {
                $template_part = 'groups/single/members.php';
        }

However, in buddypress.js, in bp_filter_request(), is this:

<?php

        if ( 'friends' === object || 'group_members' === object ) {
                object = 'members';
        }

So the value of object ends up being members instead of group_members, in the Ajax request. But it is expected to be group_members in the Ajax response handler, for the correct template to be used.

The fix:

Changing group_members to members in bp_legacy_theme_object_template_loader() fixed the issue for me.

#goodfirstbug

[dcavins]

It looks like this logic was changed in this revision: ​https://buddypress.trac.wordpress.org/changeset/11821

@DJPaul, removing the && 'group_members' === $object clause in bp_legacy_theme_object_template_loader() fixes this issue. Is there a situation in which 'groups/single/members' === $_POST['template'] would be set, and some other object would be set, or can we dispense with the $object check?

Thanks!

-David

[DJPaul]

That was part of the last security release. We'd have to check any changes here carefully.

cc @boonebgorges

[boonebgorges]

@jdgrimes Thanks very much for the clear ticket.

@dcavins Your fix looks correct. The change in bp-legacy in [11821] was my code, and the extra 'group_members' check was meant to limit the use of this 'template' feature, but it was ill-conceived. Taking it out does appear to fix the issue, and doesn't have any security implications.

No similar change is needed at the moment in bp-nouveau. See #7689, which rethinks the way AJAX templates are chosen, and which must be resolved separately before BP 3.0.

[dcavins]

In 11871:

Fix group members list AJAX pagination.

Correct broken AJAX pagination on group members list, introduced in r11821.

Props jdgrimes.

Fixes #7704.

[dcavins]

In 11872:

Fix group members list AJAX pagination.

Correct broken AJAX pagination on group members list, introduced in r11821.

Props jdgrimes.

Fixes #7704.

[DJPaul]

Milestone renamed