Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5423 closed enhancement (fixed)

Add pagination to group invitations loop

Reported by: dcavins Owned by: boonebgorges
Milestone: 2.0 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch
Cc:

Description

The group invitations loop bp_group_has_invites() doesn't handle pagination. I'd like to include an invitation management pane on the WordPress admin group edit page and that will require pagination.

This patch adds the necessary arguments to modify the query for pagination and adds the basic js that returns the correct part of the invitations list.

One piece that I don't have working is that the jQuery that adds and removes members from the list works only on the initial page load and will not work once the list has been refreshed by loading a different page of results or adding or removing a member. If someone can point me in the right direction on that problem, I'd appreciate it.

Thanks for looking at this patch.

Attachments (8)

invite-pagination.patch (16.3 KB) - added by dcavins 8 years ago.
Adds pagination pieces to invitation code
5423.02.patch (24.9 KB) - added by boonebgorges 8 years ago.
5423.03.patch (37.4 KB) - added by dcavins 8 years ago.
5423.04.patch (38.6 KB) - added by dcavins 8 years ago.
5423.05.patch (40.4 KB) - added by dcavins 8 years ago.
Re-adds the global $group_id, which was "optimized" out of patch #4.
5423.05.2.patch (38.7 KB) - added by dcavins 8 years ago.
5423.06.patch (9.9 KB) - added by boonebgorges 8 years ago.
5423.07.patch (1.2 KB) - added by dcavins 8 years ago.

Download all attachments as: .zip

Change History (20)

@dcavins
8 years ago

Adds pagination pieces to invitation code

#1 @boonebgorges
8 years ago

  • Component changed from Core to Groups

Thanks for the patch, dcavins. I really like the new technique of loading the entire invites list via AJAX on every change - IMO, this simplifies everything greatly, and provides for a much more consistent UI. The fact that we are able to get pagination in this way is a bonus.

As I think I mentioned in a previous ticket, our has_invites() queries are very lame, and your extension inherited some of that lameness :) It's really much more appropriate as a BP_Group_Member_Query query. 5423.02.patch is an update of your patch, which makes the necessary mods to BP_Group_Member_Query and the bp_group_has_members() stack. (These mods will also help with some of your other group-invite-related tickets.)

Could I ask you to take a look at the new patch? First, let me know whether you think it's a viable way forward in terms of the queries. Second, I ran into a couple issue that may or may not be my fault, but I was hoping you could look more closely at.

  • Removing an invitation by unchecking the relevant checkbox doesn't seem to work (page refreshes but with the invite still in page)
  • Removing an invitation by clicking Remove Invite seems to break sometimes. I got a nonce notice a couple times - maybe after the page is reloaded? Not totally sure.

Thanks for your work on this! Looking like a great improvement (and a good excuse to enhance BP_Group_Member_Query).

#2 @boonebgorges
8 years ago

Another issue just dawned on me. The changes to the javascript file will break backward compatibility for users who are using bp-legacy but have overridden send-invites.php in their theme. Can you have a look to see if the JS can be rewritten in such a way as to continue to support this case?

#3 @dcavins
8 years ago

Hi boonebgorges-

I really like your extension of the BP_Group_Member_Query for this use. It'll help me on several other invitation-related tickets, too. :)

Here's a new patch that combines your BP_Group_Member_Query work with some other pagination and template updates.

I have some ideas about supporting the previous send-invites.php template javascript. I'll look into it in the next day or so.

Notes: bp_group_has_invites() now accepts pagination parameters and uses BP_Group_Member_Query to build the list.

Solved "javascript fails on AJAX-produced pages" problem from my earlier patch by moving a hidden input storing group_id out of the piece refreshed via an AJAX call. (The code created in wp_ajax couldn't produce the group ID, so it was getting lost, and it's needed for the invite/uninvite action.)

In buddypress.js, changed the target in the .on() function. It wasn't working when monitoring a div that was being recreated via AJAX. So it's now monitoring a parent div that remains in the DOM all the time. (I'm sure "monitoring" is the wrong word. Event bubbling wasn't being picked up when the container div was replaced via AJAX.)

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

@dcavins
8 years ago

#4 @dcavins
8 years ago

I was able to add backwards compatibility with the old template style by checking for the new pagination target div. This new patch (5423.04) maintains the old invite/remove invite javascript for themes using the old-style template.

We can't actually provide pagination in the old template (pagination requires some new structural elements), but this patch prevents the old-style templates from being totally broken.

@dcavins
8 years ago

@dcavins
8 years ago

Re-adds the global $group_id, which was "optimized" out of patch #4.

@dcavins
8 years ago

#5 @boonebgorges
8 years ago

In 8045:

Swap bp_group_has_invites() internals for BP_Group_Member_Query, and introduce pagination arguments

See #5423

Props boonebgorges, dcavins

#6 @boonebgorges
8 years ago

  • Milestone changed from Awaiting Review to 2.0

Thanks very much, dcavins.

I've committed the backend stuff (r8044, r8055). I've got to find a few more minutes to review your most recent changes to the JS, but at a glance, it looks good. I'm posting 5423.06.patch, which is 5423.05.2 minus the already committed bits.

#7 @boonebgorges
8 years ago

dcavins - I had another look at this, and I think it's almost ready to go. The one remaining issue is that the invitation pagination counts (Viewing 10-12 of 12) are not updating dynamically when adding/subtracting users from the list. Yet when I refresh the page, the counts are updated. Maybe something funky is happening with caching or something? Would you mind taking a look?

#8 @dcavins
8 years ago

Oh, how embarrassing. Using the wrong selector often gives the wrong result. I'm attaching the fix.

Thanks very much for working on this new set of features. It's not sexy, but it is useful.

@dcavins
8 years ago

#9 @boonebgorges
8 years ago

Great, thanks, dcavins.

Going to make a few small changes:

  • I have to switch the default per_page param in bp_group_has_invites() to false. Otherwise, themes like bp-default won't show anything beyond the first 10 invitations. This means that, to support pagination in bp-legacy, we have to manually pass a per_page argument. Not the most elegant thing in the world, but I've put it right in the template. It works.
  • I moved the 'Select people to invite from your invite list' so that it only shows up when you haven't yet selected any members. The notice is really unnecessary when you've got invites, and also its persistence was causing some funniness with fading out and then reappearing after the AJAX response.

#10 follow-up: @boonebgorges
8 years ago

In 8062:

Change default per_page for bp_group_has_invites() to false

This change is necessary for backward compatibility, as the old internals
did not have pagination parameters. See eg bp-default.

See #5423

#11 @boonebgorges
8 years ago

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

In 8063:

Introduce pagination for Send Invites page in single groups

For better support for up-to-date pagination, the AJAX for group invitations
now refreshes the entire invitation section, rather than simply adding single
items to/removing single items from the DOM. This change requires a new
template, groups/single/invite-loop.php, which is the item loaded by the AJAX
request. Backward compatibility with bp-default-style themes is maintained in
the JS.

Fixes #5423

Props dcavins

#12 in reply to: ↑ 10 @dcavins
8 years ago

Replying to boonebgorges:

In 8062:

Change default per_page for bp_group_has_invites() to false

This change is necessary for backward compatibility, as the old internals
did not have pagination parameters. See eg bp-default.

See #5423

Ha, that's actually currently the case with member requests. (If there are more than 10, they don't show.) I think that your change is totally reasonable, since the new underlying structure allows for easy pagination if desired.

Thanks again for putting in the time on this ticket.

Note: See TracTickets for help on using tickets.