Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

#5440 closed enhancement (fixed)

Add pagination to group membership requests admin page

Reported by: dcavins Owned by: boonebgorges
Milestone: 2.0 Priority: normal
Severity: normal Version: 1.9.2
Component: Groups Keywords: has-patch
Cc: david.cavins@…

Description

I've created a new patch that builds on the work in ticket #5423. This patch moves the group membership requests loop over to using BP_Group_Member_Query instead of BP_Groups_Group::get_membership_requests(). It also adds support for pagination by adding the necessary pieces to the template file (and moving some of the template file out to a new "refreshable" loop).

You must first apply the patch at #5423 to make the new arguments for BP_Group_Member_Query available.

Please let me know if you find anything that I need to clean up or if there are better ideas floating around out there.

Thanks!

Attachments (3)

5440.01.patch (16.5 KB) - added by dcavins 7 years ago.
5440.02.patch (8.9 KB) - added by boonebgorges 7 years ago.
5440.03.patch (11.0 KB) - added by dcavins 7 years ago.

Download all attachments as: .zip

Change History (9)

@dcavins
7 years ago

#1 @boonebgorges
7 years ago

In 8066:

Code quality improvements to BP_Group_Membership_Requests_Template

  • Refactor constructor to accept params as an associative array
  • Don't use extract() in constructor
  • Docblock

See #5440

Props dcavins

#2 @boonebgorges
7 years ago

In 8069:

Replace internals of BP_Group_Membership_Requests_Template with BP_Group_Member_Query

This brings us more in line with the Invites template functions, and is another
step toward centralizing all of our group queries.

The change also allows for true pagination. This changeset also adds the 'page'
parameter to the stack.

See #5440

Props dcavins

#3 @boonebgorges
7 years ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 2.0

Thanks, dcavins!

As you can see, I've begin the preliminary work on this. I've left the front-end implementation out for the moment. 5440.02 is the uncommitted parts of your patch.

It looks like you've tried to emulate the Invites loop by throwing around a $group_id global. Can you please revert these changes and keep things the way that they were? To be honest, neither of the techniques is very good, but I would like to change as few things as necessary here. We can worry about the inconsistency later.

I see that in your original patch you noticed that it was necessary to have the 'id' column from the bp_group_members table. However, your suggested fix - which involved changing the key to 'group_members_table_id' to avoid conflicts - isn't the best strategy, as it'll break plugins that are directly accessing the global value here. Could you make the necessary changes in the template functions as well?

Finally, can you please be certain (as you did in the case of invites) that these changes don't break existing themes (or at least bp-default)?

Thanks!

#4 @dcavins
7 years ago

  • Cc david.cavins@… added

Thanks very much for working on this. I believe I was able to make the changes you requested.

The $group_id global was included because the $groups_template global isn't reliable when used in an AJAX context. I was able to use buddypress()->groups->current_group where I needed access to the same bits, and it solves the problem I was having.

I switched the template functions (bp_get_group_request_reject_link() and bp_get_group_request_accept_link()) over to use your new table id membership_id (added in r8069) instead of the less descriptive id. It's not a necessary change, but it makes the code easier to follow.

I've tested this in my bp-compat theme, twenty twelve and Frisco. In a theme that doesn't include the structures for pagination, only the ten most recent requests are shown (but that's the current behavior anyway, which is part of why I wanted to make these changes).

Please let me know if you notice anything that I need to revisit or have other suggestions.

@dcavins
7 years ago

#5 @boonebgorges
7 years ago

  • Keywords needs-refresh removed

Looks excellent, dcavins. I'm going to make a few minor changes to function/file names for better consistency.

#6 @boonebgorges
7 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 8150:

Add AJAX-powered pagination for group membership requests admin pages

Fixes #5440

Props dcavins

Note: See TracTickets for help on using tickets.