Issue Details (XML | Word | Printable)

Key: SAK-17471
Type: Task Task
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: MAINT TEAM
Reporter: Lovemore Nalube
Votes: 0
Watchers: 6
Operations

If you were logged in you would be able to see more operations.
Sakai

Add group manipulation abilites to the Entity Broker

Created: 30-Nov-2009 01:15   Updated: 29-Jun-2010 16:56
Component/s: Entity Broker
Affects Version/s: 2.7.0
Fix Version/s: 2.7.0

Time Tracking:
Not Specified

File Attachments: 1. File SAK-17471.diff (22 kB)
Issue Links:
Relate

2.7.x Status: Closed
2.6.x Status: None
2.5.x Status: None
2.4.x Status: None
Maintenance Team Issue: Yes


 Description  « Hide
We have a need to manipulate site groups via EB vis: update groups with new ids, delete users from groups.

The currently supported method is a GET which is a custom action in the Memberships Provider. We would like to expand this to include more RESTful actions.

Proposed URL scheme for updating memberships:

GET /direct/membership/group/groupid - gets current membership for the given groupid (current behaviour)
POST /direct/membership/group/groupid - update the membership for the given groupid

Parameters for the POST: action=(update|add|remove) followed by list of userids. If update, replace the current membership with the provided list. If add, add the list to the existing membership, if remove, remove the list from the existing membership.

Proposed URL scheme for managing groups (add a new group in a site, delete a group, get and update group metadata):

GET /direct/site/siteid/groups - get a list of groups (group metadata, i.e. title, description) but not membership

GET /direct/site/siteid/group/groupid - get metadata for group but not membership
POST /direct/site/siteid/group/groupid - update metadata for group but not membership
PUT /direct/site/siteid/group/new - create a new group in the site (returns group id). Params include title, description, optionally initial list of members
DELETE /direct/site/siteid/group - delete an existing group in the site.


 All   Comments   Work Log   Change History   Subversion Commits   git Commits      Sort Order: Ascending order - Click to sort in descending order
Lovemore Nalube made changes - 30-Nov-2009 01:15
Field Original Value New Value
Summary qdd group manipulation abilites to the Entity Broker Add group manipulation abilites to the Entity Broker
[ Permlink | « Hide ]
Aaron Zeckoski added a comment - 30-Nov-2009 01:33
Removing via PUT is not restful, DELETE should be used to remove users from the group

[ Permlink | « Hide ]
Lovemore Nalube added a comment - 30-Nov-2009 01:41
Thanks for the correction Aaron, I agree. So do you think this is something that we can start plugging in?

[ Permlink | « Hide ]
Aaron Zeckoski added a comment - 30-Nov-2009 01:46
Sure. You can just add some stuff to the memberships provider to handle it. The easiest may just be to extend the current custom action to include added path params and support for multiple methods. That would probably result in the nicest URL path and least overall code even if it involves a bunch of if-then checks.

[ Permlink | « Hide ]
Stephen Marquard added a comment - 30-Nov-2009 03:48
I think we also need to be able to create a group that doesn't yet exist, and remove a group.

Lovemore Nalube made changes - 30-Nov-2009 05:49
Description We have a need to manipulate site groups via EB vis: update groups with new ids, delete users from groups.

The currently supported method is a GET which is a custom action in the Memberships Provider. We would like to expand this to include more RESTful actions.

These are the actions to introduce:
1. Add EB action to do a batch edit to user group memberships given userIds and groupId as params [PUT action],
2. To remove user from a group. In same action in (1) above, add param to indicate if adding or removing user(s) from the provided groupId [PUT action].
3. GET action for group names and groupIds where a siteId is specified.

We have a need to manipulate site groups via EB vis: update groups with new ids, delete users from groups.

The currently supported method is a GET which is a custom action in the Memberships Provider. We would like to expand this to include more RESTful actions.

These are the actions to introduce:
1. Add EB action to do a batch edit to user group memberships given userIds and groupId as params [PUT action],
2. To remove user from a group. In same action in (1) above, add param to indicate if adding or removing user(s) from the provided groupId [DELETE action].
3. GET action for group names and groupIds where a siteId is specified.

Actual custom actions in Membership provider:

1. group-users-add :: params: groupId, userIds :: returns: 200, 400 or 401
2. group-users-delete :: params: groupId, userIds :: returns: 200, 400 or 401


and those in Site provider:

1. groups :: params: siteId (retrieve site groups without the group users ie: get site group properties.)
2. group-new :: params: siteId, userIds (optional), groupTitle :: returns: 200, 400 or 401
3. group-delete :: params: groupId :: returns: 200, 400 or 401
[ Permlink | « Hide ]
Stephen Marquard added a comment - 01-Dec-2009 02:55
Proposed URL scheme for updating memberships:

GET /direct/membership/group/groupid - gets current membership for the given groupid (current behaviour)
POST /direct/membership/group/groupid - update the membership for the given groupid

Parameters for the POST: action=(update|add|remove) followed by list of userids. If update, replace the current membership with the provided list. If add, add the list to the existing membership, if remove, remove the list from the existing membership.

[ Permlink | « Hide ]
Aaron Zeckoski added a comment - 01-Dec-2009 03:01
We have http methods for a reason. Rather than providing actions we should use PUT, POST, DELETE. This is what they are designed for and this is what is restful.
PUT for add, POST for update, and DELETE for remove would make sense to me.

[ Permlink | « Hide ]
Lovemore Nalube added a comment - 01-Dec-2009 03:06
We can have DELETE for removing users like this:

DELETE /direct/membership/group/groupid - delete the membership for the given groupid based on a set of userIds

[ Permlink | « Hide ]
Stephen Marquard added a comment - 01-Dec-2009 03:09
Proposed URL scheme for managing groups (add a new group in a site, delete a group, get and update group metadata):

GET /direct/site/siteid/groups - get a list of groups (group metadata, i.e. title, description) but not membership

GET /direct/site/siteid/group/groupid - get metadata for group but not membership
POST /direct/site/siteid/group/groupid - update metadata for group but not membership
PUT /direct/site/siteid/group/new - create a new group in the site (returns group id). Params include title, description
DELETE /direct/site/siteid/group - delete an existing group in the site.

Stephen Marquard made changes - 01-Dec-2009 03:15
Description We have a need to manipulate site groups via EB vis: update groups with new ids, delete users from groups.

The currently supported method is a GET which is a custom action in the Memberships Provider. We would like to expand this to include more RESTful actions.

These are the actions to introduce:
1. Add EB action to do a batch edit to user group memberships given userIds and groupId as params [PUT action],
2. To remove user from a group. In same action in (1) above, add param to indicate if adding or removing user(s) from the provided groupId [DELETE action].
3. GET action for group names and groupIds where a siteId is specified.

Actual custom actions in Membership provider:

1. group-users-add :: params: groupId, userIds :: returns: 200, 400 or 401
2. group-users-delete :: params: groupId, userIds :: returns: 200, 400 or 401


and those in Site provider:

1. groups :: params: siteId (retrieve site groups without the group users ie: get site group properties.)
2. group-new :: params: siteId, userIds (optional), groupTitle :: returns: 200, 400 or 401
3. group-delete :: params: groupId :: returns: 200, 400 or 401
We have a need to manipulate site groups via EB vis: update groups with new ids, delete users from groups.

The currently supported method is a GET which is a custom action in the Memberships Provider. We would like to expand this to include more RESTful actions.

Proposed URL scheme for updating memberships:

GET /direct/membership/group/groupid - gets current membership for the given groupid (current behaviour)
POST /direct/membership/group/groupid - update the membership for the given groupid

Parameters for the POST: action=(update|add|remove) followed by list of userids. If update, replace the current membership with the provided list. If add, add the list to the existing membership, if remove, remove the list from the existing membership.

Proposed URL scheme for managing groups (add a new group in a site, delete a group, get and update group metadata):

GET /direct/site/siteid/groups - get a list of groups (group metadata, i.e. title, description) but not membership

GET /direct/site/siteid/group/groupid - get metadata for group but not membership
POST /direct/site/siteid/group/groupid - update metadata for group but not membership
PUT /direct/site/siteid/group/new - create a new group in the site (returns group id). Params include title, description, optionally initial list of members
DELETE /direct/site/siteid/group - delete an existing group in the site.
[ Permlink | « Hide ]
Stephen Marquard added a comment - 01-Dec-2009 03:16
Assigning to Lovemore to implement in UCT branch.

Stephen Marquard made changes - 01-Dec-2009 03:16
Assignee Aaron Zeckoski [ aaronz ] Lovemore Nalube [ lovenalube ]
[ Permlink | « Hide ]
Stephen Marquard added a comment - 01-Dec-2009 03:27
There are 2 types of groups in a site, viz. those managed by Site Info and those managed by Section Info.

The EB providers here will return info about Section Info-managed groups, but will not allow updates to them (as there are specific business rules that apply to these groups and they must be managed by the Section Manager rather than directly).

To be able to tell these apart, the group metadata must include the group properties. Site Info managed groups have the property "group_prop_wsetup_created" = true.

Repository Revision Date User Message
Sakai Source Repository #69425 Wed Dec 02 07:44:41 PST 2009 lovemore.nalube@uct.ac.za SAK-17471 VULA-650. Add group manipulation abilites to the Entity Broker.
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/SiteEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/site.properties
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/membership.properties
ADD /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/model/EntityGroup.java

Repository Revision Date User Message
Sakai Source Repository #69440 Thu Dec 03 02:39:21 PST 2009 lovemore.nalube@uct.ac.za SAK-17471 VULA-650. Fix NPE when site has no groups
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/SiteEntityProvider.java

Repository Revision Date User Message
Sakai Source Repository #69443 Thu Dec 03 05:16:08 PST 2009 lovemore.nalube@uct.ac.za SAK-17471 VULA-650. Remove site ownser info from EB group, check permissions on all actions
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/SiteEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/model/EntityGroup.java

Repository Revision Date User Message
Sakai Source Repository #69461 Thu Dec 03 13:22:15 PST 2009 lovemore.nalube@uct.ac.za SAK-17471 VULA-650. return EntityData and not an ActionReturn. This makes new groups etc changes return same format as other current EPs
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/SiteEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/site.properties

[ Permlink | « Hide ]
Lovemore Nalube added a comment - 15-Jan-2010 00:55
This feature has been added to the UCT branch of Entity Broker. It's been tested and works as expected.

Can we introduce this into trunk?

Lovemore Nalube made changes - 15-Jan-2010 00:55
Assignee Lovemore Nalube [ lovenalube ] Aaron Zeckoski [ aaronz ]
Aaron Zeckoski made changes - 20-Jan-2010 08:34
Assignee Aaron Zeckoski [ aaronz ]
[ Permlink | « Hide ]
David Horwitz added a comment - 23-Jan-2010 01:52
Maintenance team review: If this issue is being worked on please take ownership of it by assigning it to yourself. Otherwise it is likely to be closed as an abandoned issue. If this is not being pursued please close the issue.

Anthony Whyte made changes - 25-Jan-2010 06:46
Affects Version/s 2.7.0-M2 [ 11673 ]
Affects Version/s 2.7.0-M1 [ 11672 ]
Repository Revision Date User Message
Sakai Source Repository #72645 Wed Jan 27 05:17:51 PST 2010 lovemore.nalube@uct.ac.za SAK-17471 Avoid NPE wrt a User's Role
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/site.properties
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/.classpath

Stephen Marquard made changes - 27-Jan-2010 23:55
Assignee Stephen Marquard [ smarquard ]
[ Permlink | « Hide ]
Stephen Marquard added a comment - 28-Jan-2010 00:45
role can return null here:

                         Role role = site.getUserRole(userId);

when the user is inactive in the site, as per kernel-impl/src/main/java/org/sakaiproject/authz/impl/BaseAuthzGroup.java:

 public Role getUserRole(String user)
        {
                if (m_lazy) baseAuthzGroupService.m_storage.completeGet(this);

                BaseMember grant = (BaseMember) m_userGrants.get(user);
                if ((grant != null) && (grant.active)) return grant.role;

                return null;
        }

The EP should not take account of a member's site status, i.e. it should still be possible to update a group, and add or remove an inactive member from a group.

The status in the group should be the status in the site on adding.

Please adjust to get the role via the Member record which doesn't check for active status, i.e.

                        Member m = site.getMember(userId);
                         Role role = m.getRole();

and avoid all use of getUserRole()

QA: inactive users can be added to and removed from groups, group update operations on groups with inactive members complete correctly.

Stephen Marquard made changes - 28-Jan-2010 00:58
Link This issue relates to SAK-17836 [ SAK-17836 ]
Repository Revision Date User Message
Sakai Source Repository #72937 Thu Jan 28 04:44:44 PST 2010 lovemore.nalube@uct.ac.za SAK-17471 Do action on user if inactive in site or not.
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java

Repository Revision Date User Message
Sakai Source Repository #72938 Thu Jan 28 05:24:29 PST 2010 lovemore.nalube@uct.ac.za SAK-17471 Do action on user if inactive in site or not. Stop using site.getUserRole(userId)
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java

Repository Revision Date User Message
Sakai Source Repository #72939 Thu Jan 28 05:30:18 PST 2010 lovemore.nalube@uct.ac.za SAK-17471 Fix getRole lines
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java

Lovemore Nalube made changes - 29-Jan-2010 01:01
Assignee Stephen Marquard [ smarquard ] David Horwitz [ dhorwitz ]
Stephen Marquard made changes - 29-Jan-2010 02:06
Link This issue is related to SAK-16014 [ SAK-16014 ]
[ Permlink | « Hide ]
David Horwitz added a comment - 29-Jan-2010 02:08
This doesn't merge as its incompatible with SAK-16014.

We need to:
1) Test that SAK-16014 doesn't do the same thing
2) if not reworked so that its compatible

David Horwitz made changes - 29-Jan-2010 02:08
Assignee David Horwitz [ dhorwitz ] Lovemore Nalube [ lovenalube ]
Repository Revision Date User Message
Sakai Source Repository #72979 Fri Jan 29 04:41:27 PST 2010 lovemore.nalube@uct.ac.za SAK-17471 Revert classpath file.
Files Changed
MODIFY /msub/uct.ac.za/vula/trunk/entitybroker/core-providers/.classpath

[ Permlink | « Hide ]
Lovemore Nalube added a comment - 01-Feb-2010 23:56
This patch introduces these features to trunk.

Lovemore Nalube made changes - 01-Feb-2010 23:56
Attachment SAK-17471.diff [ 19543 ]
Anthony Whyte made changes - 07-Feb-2010 17:47
Affects Version/s 2.7.0-b01 [ 11681 ]
Affects Version/s 2.7.0-M2 [ 11673 ]
[ Permlink | « Hide ]
Lovemore Nalube added a comment - 16-Feb-2010 04:31
Please audit changes in the attached patch for patching trunk.

Lovemore Nalube made changes - 16-Feb-2010 04:31
Assignee Lovemore Nalube [ lovenalube ] Aaron Zeckoski [ aaronz ]
[ Permlink | « Hide ]
Aaron Zeckoski added a comment - 12-Mar-2010 09:31
Note that I accidently committed the changed for this issue under a different JIRA: SAK-18131

commit -m "http://bugs.sakaiproject.org/jira/browse/SAK-18131..." /opt/sakai/entitybroker/core-providers/src/java/membership.properties /opt/sakai/entitybroker/core-providers/src/java/site.properties /opt/sakai/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java /opt/sakai/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/ServerConfigEntityProvider.java /opt/sakai/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/SiteEntityProvider.java
    Sending /opt/sakai/entitybroker/core-providers/src/java/membership.properties
    Sending /opt/sakai/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/MembershipEntityProvider.java
    Sending /opt/sakai/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/ServerConfigEntityProvider.java
    Sending /opt/sakai/entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/SiteEntityProvider.java
    Sending /opt/sakai/entitybroker/core-providers/src/java/site.properties
    Transmitting file data ...
    Committed revision 74580.

Aaron Zeckoski made changes - 12-Mar-2010 09:31
Status Open [ 1 ] Resolved [ 5 ]
Assignee Aaron Zeckoski [ aaronz ] MAINT TEAM [ maintenanceteam ]
Fix Version/s 2.7.0-b05 [ 11810 ]
Resolution Fixed [ 1 ]
[ Permlink | « Hide ]
Anthony Whyte added a comment - 23-Mar-2010 12:58
In entitybroker-1.3.x and released as part of entitybroker-1.3.9.

Anthony Whyte made changes - 23-Mar-2010 12:58
2.7.x Status None Closed
Anthony Whyte made changes - 23-Mar-2010 12:58
Status Resolved [ 5 ] Closed [ 6 ]
Anthony Whyte made changes - 24-Mar-2010 10:17
Fix Version/s 2.7.0-b06 [ 11825 ]
Fix Version/s 2.7.0-b05 [ 11810 ]
Steve Swinsburg made changes - 08-Apr-2010 04:36
Maintenance Team Issue [Yes]
Anthony Whyte made changes - 29-Jun-2010 16:35
Affects Version/s 2.7.0 [ 11276 ]
Affects Version/s 2.7.0-b01 [ 11681 ]
Anthony Whyte made changes - 29-Jun-2010 16:56
Fix Version/s 2.7.0 [ 11276 ]
Fix Version/s 2.7.0-b06 [ 11825 ]

Powered by a free Atlassian JIRA open source license for Sakai Foundation. Try JIRA - bug tracking software for your team.
Atlassian JIRA the Professional Issue Tracker. (Enterprise Edition, Version: 3.13.2-#335) - Bug/feature request - Atlassian news - Contact Administrators