#8045 closed defect (bug) (fixed)
AJAXify parts of group membership Dashboard UI using REST API
Reported by: | boonebgorges | Owned by: | imath |
---|---|---|---|
Milestone: | 5.0.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Groups | Keywords: | has-patch commit |
Cc: |
Description
Most actions on group admin (Dashboard) pages require page refreshes. This is an area where we may add AJAX functionality as a progressive enhancement, leveraging the REST API. A few specific examples:
- The group member add autocomplete/suggest. This already runs on AJAX, and I'm unsure whether we can easily convert to the REST API without discarding parts of our server-side "suggest" system, but it may be worth a try.
- The adding of a user to a group (current requires Save Changes)
- Promotion/banning/deleting etc of existing group members
- Group member pagination
It may turn out to be possible to write some of these enhancements so that they can also be ported to the front-end. Thinking here especially of the promote/demote/etc interface.
Attachments (7)
Change History (29)
#2
@
6 years ago
- Keywords dev-feedback added
In 8045.group-admin.patch I've tested a quick way to manage roles using the REST endpoint for the group members within the single Group Administration screen.
As you will see I haven’t changed the UI that much but using the BP REST API is introducing some regression when used into the single group admin screen:
- it’s not possible to demote an administrator to a moderator,
- it’s not possible to unban a user to an administrator or a moderator.
I had to play with css and adapt the markup so that if there’s no users into a specific role, then the paragraph informing there a no users is inside a table row to avoid overcomplicating the JavaScript.
Here's some thoughts about the BP REST API:
BP_REST_Group_Members_Endpoint
should probably best namedBP_REST_Group_Membership_Endpoint
.- the ‘join’ action should probably be into a
create_item()
method. - there should probably be some equivalent methods to
groups_{action}_membership_request()
where {action} can be ‘send’, ‘accept’, ‘reject’, ‘delete’… - there should be a way to demote an administrator to a moderator or unban a user to an administrator or a moderator.
- I can see high interests in being able to batch promote/demote/ban/unban from the endpoint.
This first patch is not very ambitious, but before suggesting something a bit more ambitious, I wanted to make sure I wasn’t going into a wrong direction :)
So here’s what i think it would be great to do:
- Use Backbone collection/model/views & underscore templates to replace the current UI and make it usable from the front end. (I thought about using ReastJS but I’m not sure loading this library into the front-end would be a good idea as most Nouveau JavaScript’s using Backbone)
- Use a new UI closer to the way WordPress displays the list of users into the Admin with tabs > All | Admins | Mods | Members and a unique selectbox to manage the role.
- Manage membership requests/invites into 2 other tabs > Requests | Invites,
- Add a search feature as it can be very challenging to find a user when he’s at the 10th page for example.
- Use the BP REST API to get/create/update/delete group memberships.
#3
@
6 years ago
@imath I created an issue, https://github.com/buddypress/BP-REST/issues/138, in the BP REST API repo to add and keep track of those suggestions.
A question:
there should probably be some equivalent methods to groups_{action}_membership_request() where {action} can be ‘send’, ‘accept’, ‘reject’, ‘delete’…
I don't get the need for a method for each action. The idea of keeping everything like it is was exactly not to overcomplex and reuse code. What benefit do you see in adding such methods?
I'd recommend to keeping using Backbone, as you already mentioned, it is the one that Nouveau is using, so some code reuse could be done in the back and front-end, not sure, but maybe.
I'm ok with the rest of the suggestions. Just bear in mind the 5.0 timeline. :)
#4
@
6 years ago
Thanks for your feedbacks @espellcaste, Backbone seems the fastest move I agree.
I don't get the need for a method for each action. The idea of keeping everything like it is was exactly not to overcomplex and reuse code. What benefit do you see in adding such methods?
Well maybe I haven't found how to handle private groups membership requests using the BP REST API. Maybe you can point me into the right direction.
About the timeline, we can always extend it, can't we ?
I'm asking because I've just added a PR to fix an issue for the Groups endpoint and I've just found the Members endpoint seems to be using the way WordPress is getting users, meaning it's not possible to list users when you don't have the capability to do it (unlogged users or regular members). Is there a reason I'm not aware of explaining why BP_User_Query
is not used to list members ?
Maybe working on some real use cases would help us to polish the API ?
#5
@
6 years ago
@imath I added your suggestions to the API, namely:
- Class name was changed into
BP_REST_Group_Membership_Endpoint
. - The
join
action was separated from the update endpoint and has its owncreate_item()
method. - Now it is possible to demote an administrator to a moderator.
- It's also possible to unban a user to an administrator or a moderator. (Technically you would be promoting a user.)
Related issue closed: https://github.com/buddypress/BP-REST/issues/138
Well maybe I haven't found how to handle private groups membership requests using the BP REST API. Maybe you can point me in the right direction.
Private group request is not implemeted at this time. This is something pending. Happy to get PR, though. :)
About the timeline, we can always extend it, can't we?
Yes! But personally, I'd like us to do something feasible for the current release timeline, and worry about doing something more complex or that would take more timer later if desired.
the Members endpoint seems to be using the way WordPress is getting users, meaning it's not possible to list users when you don't have the capability to do it (unlogged users or regular members). Is there a reason I'm not aware of explaining why BP_User_Query is not used to listing members?
At the time, that was the decision I made. But we can certainly change that and I did here in this commit.
Maybe working on some real use cases would help us to polish the API?
Indeed! That's exactly what I'm counting on. I tried my best to grasp how the API would be used but nothing is better than real use cases. So send your suggestions. Nothing is set on stone.
#6
@
6 years ago
@espellcaste Thanks a lot for your latest commits into the BP REST API. I'll update 8045.group-admin.patch asap :)
#7
@
6 years ago
8045.2.group-admin.patch is refreshed to take latest Groups Membership endpoint improvements in account + displays the error message to the user if something goes wrong.
#8
@
6 years ago
- Keywords has-patch added; needs-patch removed
Hi there, I personaly prefer 8045.1.patch. It's taking in charge the group members management :
- in the WordPress single Group Admin Screen,
- on the front end in the Group Manage Screen.
It's a good real use case for the BP_REST_Group_Membership_Endpoint
controler as it:
- lists group members / paginate / filter by role
- edit members group role
- ban/unban and remove members.
Here two demos of the applied patch :
I still need to improve some code formatting/indentation and I'd like to include a search feature. We can discuss about the big choice I've made: the UI is only available if Nouveau is active. But I guess we can also do something wider just like we did for avatars.
Happy to read feedbacks / improvements etc..
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
6 years ago
#10
@
6 years ago
8045.2.patch is adding a polyfill to wp.apiRequest in case BuddyPress 5.0 is used into WordPress < 4.9.
FYI @espellcaste I've tested the patch in WordPress 4.7 and found a notice error that is not thrown within latest version of WordPress
Could you look at it:
Notice: Undefined index: items in C:\pathtowordpress\wp-includes\rest-api.php on line 1032
This error is thrown when I'm using the filter to list members with a specific role.
This ticket was mentioned in Slack in #buddypress by boone. View the logs.
6 years ago
This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.
6 years ago
#13
@
6 years ago
@imath Based on travis, https://travis-ci.org/buddypress/BP-REST/builds/500147747, I don't see any problems with the unit tests with the version you are mentioning.
Also, I quite didn't understand where exactly you tested it as you said you were testing on WordPress 4.7 but the error was thrown on the latest WP version. :/ ???
Can you elaborate on that?
#14
@
6 years ago
As discussed during our latest dev chat, 8045.3.patch is improving previous version of the patch :
- There's a new
bp-api-request.js
file you can use as a dependency for any script wishing to make REST API Requests. This means it can be used for #8043 - If the BP REST API is not available, the regular screens are used to manage group members (admin && front)
- It's now easier for any template pack to include the feature.
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
5 years ago
#16
@
5 years ago
- Keywords dev-feedback removed
I think Version 4 of the patch is ready to be committed!
It adds a so *useful* search form to group membership management screen, the appearance of the UI is improved, and the way the availability of the REST API is checked has been improved.
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
5 years ago
#18
@
5 years ago
- Keywords commit added
Now it's ready!
.5.patch fixes a JavaScript error if the REST request Headers are not set, adds a feedback message when no members were found for the search terms or the requested role.
#20
@
5 years ago
Hi @espellcaste thanks a lot for your feedback, I will commit in a few minutes/hours 😊
#21
@
5 years ago
- Owner set to imath
- Resolution set to fixed
- Status changed from new to closed
In 12405:
Good point. I think it worths giving it a try, it should autocomplete faster ;)