Skip to:

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#7423 closed defect (bug) (fixed)

Group Types Default Pagination Broken

Reported by: reubenlillie Owned by: r-a-y
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.7
Component: Groups Keywords: has-patch commit


Expected: Pagination links appearing for group type listings with over "n" results (n = 20 by default) would navigate to their respective pages.
Actual: Links appear active (e.g., cursor changes on focus/hover, href exists), but both next-page and numbered links fail to load. That is, it is currently impossible for users with a default BP installation to navigate to groups listed beyond the "nth" result when viewing by group type.
Query issue?
Template issue?

NB: Functionality works as expected when viewing "All Groups" listing but not when viewing by "group type" (2.6+).

Attachments (3)

7423.01.patch (2.5 KB) - added by r-a-y 4 years ago.
7423.tests.patch (1.7 KB) - added by dcavins 4 years ago.
Tests for group directory by type
7423.02.patch (4.2 KB) - added by r-a-y 4 years ago.

Download all attachments as: .zip

Change History (14)

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.

4 years ago

#2 @DJPaul
4 years ago

  • Milestone changed from Awaiting Review to Future Release

#3 @r-a-y
4 years ago

  • Keywords has-patch added; ui-feedback removed
  • Milestone changed from Future Release to 2.8
  • Owner set to r-a-y
  • Priority changed from high to normal
  • Status changed from new to assigned
  • Version changed from 2.7.2 to 2.7

This is a legit bug that only occurs if there is more than one page of groups on a group type directory.

There are two issues:

  1. In the BP_Groups_Component::setup_globals() method, we throw a 404 for group pages that is not a single group page or the group creation page. This meant that the AJAX call was throwing a 404.
  2. Currently, group type directory detection occurs on frontend screens only and not during AJAX. We need to move the group type directory detection into the BP_Groups_Component class so this can be picked up by AJAX.

Patch fixes this both up. Was an oversight on my part!

Would love to squeeze this in for BP 2.8, since this bug was introduced in 2.7.

Last edited 4 years ago by r-a-y (previous) (diff)

4 years ago

4 years ago

Tests for group directory by type

#4 @dcavins
4 years ago

@r-a-y I wanted to add some tests for the correct behavior. I've no idea why the last two new tests are failing. (bp_current_action() is returning type, even though I can verify in an actual install that the behavior is correct. Maybe it's a quirk with go_to? Any ideas?)

Also, the member directory tests are in routing/members.php, rather than members/types.php, so there's some doubt that I've added them in the right spot.

Last edited 4 years ago by dcavins (previous) (diff)

#5 @r-a-y
4 years ago

Maybe it's a quirk with go_to? Any ideas?)

This. We only run 'bp_init' on the go_to() method because we can't really test for redirections using wp_redirect(). If we do try to run the 'bp_template_redirect' hook in the go_to() method, PHPUnit will crap out.

Instead of redirecting, we could change invalid group type requests to 404 instead. That way, we would be in line with what we are currently doing with invalid member type requests.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.

4 years ago

4 years ago

#7 @r-a-y
4 years ago

02.patch switches out the invalid group type redirection and just 404s invalid group types.

I moved the unit tests written by @dcavins to /routing/groups.php and added some 404 tests as well.

#8 @DJPaul
4 years ago

Latest patch looks good to me

#9 @boonebgorges
4 years ago

  • Keywords commit added

Approach looks good to me too.

#10 @r-a-y
4 years ago

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

In 11405:

Groups: Fix AJAX pagination when on a group type directory page.

This commit moves group type directory detection from the
'template_redirect' hook to our groups component class to ensure
detection is available for AJAX purposes.

Commit also changes invalid group type directory requests to 404 instead
of redirecting back to the main Groups directory.

Props r-a-y, dcavins.

Fixes #7423.

#11 @boonebgorges
4 years ago

In 11408:

After [11405], be sure member types are reset between group routing tests.

See #7423.

Note: See TracTickets for help on using tickets.