69409: Implement community feedback

This commit is contained in:
Yana De Pauw
2020-03-10 18:13:09 +01:00
parent 542a40ac1c
commit 7f5a178036
6 changed files with 225 additions and 38 deletions

View File

@@ -15,8 +15,6 @@ import static org.dspace.app.rest.utils.RegexUtils.REGEX_UUID;
import static org.dspace.app.util.AuthorizeUtil.authorizeManageAdminGroup;
import static org.dspace.app.util.AuthorizeUtil.authorizeManageSubmittersGroup;
import static org.dspace.app.util.AuthorizeUtil.authorizeManageWorkflowsGroup;
import static org.dspace.app.util.GroupUtil.getCollection;
import static org.dspace.app.util.GroupUtil.getCommunity;
import static org.springframework.web.bind.annotation.RequestMethod.DELETE;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
@@ -28,12 +26,12 @@ import java.util.Optional;
import java.util.UUID;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.dspace.app.rest.exception.UnprocessableEntityException;
import org.dspace.app.rest.model.GroupRest;
import org.dspace.app.rest.utils.GroupUtil;
import org.dspace.app.rest.utils.Utils;
import org.dspace.authorize.AuthorizeException;
import org.dspace.authorize.service.AuthorizeService;
@@ -44,6 +42,8 @@ import org.dspace.eperson.EPerson;
import org.dspace.eperson.Group;
import org.dspace.eperson.service.EPersonService;
import org.dspace.eperson.service.GroupService;
import org.dspace.xmlworkflow.storedcomponents.CollectionRole;
import org.dspace.xmlworkflow.storedcomponents.service.CollectionRoleService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.rest.webmvc.ResourceNotFoundException;
import org.springframework.security.access.prepost.PreAuthorize;
@@ -67,16 +67,25 @@ public class GroupRestController {
@Autowired
private AuthorizeService authorizeService;
@Autowired
private CollectionRoleService collectionRoleService;
@Autowired
Utils utils;
@Autowired
GroupUtil groupUtil;
/**
* Method to add one or more subgroups to a group.
* The subgroups to be added should be provided in the request body as a uri-list.
* @param uuid the uuid of the group to add the subgroups to
* Note that only the 'AUTHENTICATED' state will be checked in PreAuthorize, a more detailed check will be done by
* using the 'checkAuthorization' method.
*
* @param uuid the uuid of the group to add the subgroups to
*/
@PreAuthorize("hasAuthority('AUTHENTICATED')")
@RequestMapping( method = POST, path = "/{uuid}/subgroups", consumes = {"text/uri-list"})
@RequestMapping(method = POST, path = "/{uuid}/subgroups", consumes = {"text/uri-list"})
public void addChildGroups(@PathVariable UUID uuid, HttpServletResponse response, HttpServletRequest request)
throws SQLException, AuthorizeException {
@@ -104,7 +113,7 @@ public class GroupRestController {
groupService.addMember(context, parentGroup, childGroup);
}
context.commit();
context.complete();
response.setStatus(SC_NO_CONTENT);
}
@@ -130,10 +139,13 @@ public class GroupRestController {
/**
* Method to add one or more members to a group.
* The members to be added should be provided in the request body as a uri-list.
* @param uuid the uuid of the group to add the members to
* Note that only the 'AUTHENTICATED' state will be checked in PreAuthorize, a more detailed check will be done by
* using the 'checkAuthorization' method.
*
* @param uuid the uuid of the group to add the members to
*/
@PreAuthorize("hasAuthority('AUTHENTICATED')")
@RequestMapping( method = POST, path = "/{uuid}/epersons", consumes = {"text/uri-list"})
@RequestMapping(method = POST, path = "/{uuid}/epersons", consumes = {"text/uri-list"})
public void addMembers(@PathVariable UUID uuid, HttpServletResponse response, HttpServletRequest request)
throws SQLException, AuthorizeException {
@@ -161,7 +173,7 @@ public class GroupRestController {
groupService.addMember(context, parentGroup, member);
}
context.commit();
context.complete();
response.setStatus(SC_NO_CONTENT);
}
@@ -181,13 +193,16 @@ public class GroupRestController {
/**
* Method to remove a subgroup from a group.
* @param parentUUID the uuid of the parent group
* @param childUUID the uuid of the subgroup which has to be removed
* Note that only the 'AUTHENTICATED' state will be checked in PreAuthorize, a more detailed check will be done by
* using the 'checkAuthorization' method.
*
* @param parentUUID the uuid of the parent group
* @param childUUID the uuid of the subgroup which has to be removed
*/
@PreAuthorize("hasAuthority('AUTHENTICATED')")
@RequestMapping( method = DELETE, path = "/{parentUUID}/subgroups/{childUUID}")
@RequestMapping(method = DELETE, path = "/{parentUUID}/subgroups/{childUUID}")
public void removeChildGroup(@PathVariable UUID parentUUID, @PathVariable UUID childUUID,
HttpServletResponse response, HttpServletRequest request)
HttpServletResponse response, HttpServletRequest request)
throws IOException, SQLException, AuthorizeException {
Context context = obtainContext(request);
@@ -206,18 +221,21 @@ public class GroupRestController {
groupService.removeMember(context, parentGroup, childGroup);
context.commit();
context.complete();
response.setStatus(SC_NO_CONTENT);
}
/**
* Method to remove a member from a group.
* @param parentUUID the uuid of the parent group
* @param memberUUID the uuid of the member which has to be removed
* Note that only the 'AUTHENTICATED' state will be checked in PreAuthorize, a more detailed check will be done by
* using the 'checkAuthorization' method.
*
* @param parentUUID the uuid of the parent group
* @param memberUUID the uuid of the member which has to be removed
*/
@PreAuthorize("hasAuthority('AUTHENTICATED')")
@RequestMapping( method = DELETE, path = "/{parentUUID}/epersons/{memberUUID}")
@RequestMapping(method = DELETE, path = "/{parentUUID}/epersons/{memberUUID}")
public void removeMember(@PathVariable UUID parentUUID, @PathVariable UUID memberUUID,
HttpServletResponse response, HttpServletRequest request)
throws IOException, SQLException, AuthorizeException {
@@ -238,18 +256,28 @@ public class GroupRestController {
groupService.removeMember(context, parentGroup, childGroup);
context.commit();
context.complete();
response.setStatus(SC_NO_CONTENT);
}
/**
* This method checks whether the current user has sufficient rights to modify the group.
* Depending on the kind of group and due to delegated administration, separate checks need to be done to verify
* whether the user is allowed to modify the group.
*
* @param context the context of which the user will be checked
* @param group the group to be checked
* @throws SQLException
* @throws AuthorizeException
*/
private void checkAuthorization(Context context, Group group) throws SQLException, AuthorizeException {
if (authorizeService.isAdmin(context)) {
return;
}
Collection collection = getCollection(context, group);
Collection collection = groupUtil.getCollection(context, group);
if (collection != null) {
if (group.equals(collection.getSubmitters())) {
@@ -257,11 +285,13 @@ public class GroupRestController {
return;
}
if (group.equals(collection.getWorkflowStep1(context))
|| group.equals(collection.getWorkflowStep2(context))
|| group.equals(collection.getWorkflowStep3(context))) {
authorizeManageWorkflowsGroup(context, collection);
return;
List<CollectionRole> collectionRoles = collectionRoleService.findByCollection(context, collection);
for (CollectionRole role : collectionRoles) {
if (group.equals(role.getGroup())) {
authorizeManageWorkflowsGroup(context, collection);
return;
}
}
if (group.equals(collection.getAdministrators())) {
@@ -270,7 +300,7 @@ public class GroupRestController {
}
}
Community community = getCommunity(context, group);
Community community = groupUtil.getCommunity(context, group);
if (community != null) {
authorizeManageAdminGroup(context, community);
return;

View File

@@ -26,7 +26,7 @@ import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.stereotype.Component;
/**
* Link repository for "groups" subresource of an individual group.
* Link repository for "epersons" subresource of an individual group.
*/
@Component(GroupRest.CATEGORY + "." + GroupRest.NAME + "." + GroupRest.EPERSONS)
public class GroupEPersonLinkRepository extends AbstractDSpaceRestRepository

View File

@@ -5,7 +5,7 @@
*
* http://www.dspace.org/license/
*/
package org.dspace.app.util;
package org.dspace.app.rest.utils;
import java.sql.SQLException;
import java.util.regex.Matcher;
@@ -13,15 +13,17 @@ import java.util.regex.Pattern;
import org.dspace.content.Collection;
import org.dspace.content.Community;
import org.dspace.content.factory.ContentServiceFactory;
import org.dspace.content.service.CollectionService;
import org.dspace.content.service.CommunityService;
import org.dspace.core.Context;
import org.dspace.eperson.Group;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
/**
* A class which provides utility methods for Groups
*/
@Component
public class GroupUtil {
private GroupUtil() {
@@ -30,7 +32,7 @@ public class GroupUtil {
/**
* UUID regex used in the collection regex
*/
private static final String UUID_REGEX = "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0" +
private static final String UUID_REGEX = "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}\\-[0-9a-fA-F]{4}\\-[0" +
"-9a-fA-F]{12}";
/**
* Collection regex used to extract the ID
@@ -52,15 +54,16 @@ public class GroupUtil {
*/
private static final String[] COMMUNITY_SUFFIXES = {"_ADMIN"};
protected static final CollectionService collectionService = ContentServiceFactory.getInstance()
.getCollectionService();
protected static final CommunityService communityService = ContentServiceFactory.getInstance()
.getCommunityService();
@Autowired
protected CollectionService collectionService;
@Autowired
protected CommunityService communityService;
/**
* Get the collection a given group is related to, or null if it is not related to a collection.
*/
public static Collection getCollection(Context context, Group group) throws SQLException {
public Collection getCollection(Context context, Group group) throws SQLException {
String groupName = group.getName();
@@ -84,7 +87,7 @@ public class GroupUtil {
/**
* Get the community a given group is related to, or null if it is not related to a community.
*/
public static Community getCommunity(Context context, Group group) throws SQLException {
public Community getCommunity(Context context, Group group) throws SQLException {
String groupName = group.getName();

View File

@@ -60,7 +60,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
@Test
@Ignore
// Ignored until an endpoint is added to return all groups
// Ignored until an endpoint is added to return all groups. Anonymous is not considered a direct group.
public void testStatusAuthenticated() throws Exception {
String token = getAuthToken(eperson.getEmail(), password);

View File

@@ -368,7 +368,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
}
@Test
public void findByMetadata() throws Exception {
public void findByMetadataUsingLastName() throws Exception {
context.turnOffAuthorisationSystem();
EPerson ePerson = EPersonBuilder.createEPerson(context)
.withNameInMetadata("John", "Doe")
@@ -422,6 +422,160 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
.andExpect(jsonPath("$.page.totalElements", is(4)));
}
@Test
public void findByMetadataUsingFirstName() throws Exception {
context.turnOffAuthorisationSystem();
EPerson ePerson = EPersonBuilder.createEPerson(context)
.withNameInMetadata("John", "Doe")
.withEmail("Johndoe@fake-email.com")
.build();
EPerson ePerson2 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Jane", "Smith")
.withEmail("janesmith@fake-email.com")
.build();
EPerson ePerson3 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("John", "Smith")
.withEmail("tomdoe@fake-email.com")
.build();
EPerson ePerson4 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("John-Postfix", "Smath")
.withEmail("dirkdoepostfix@fake-email.com")
.build();
EPerson ePerson5 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Prefix-John", "Smoth")
.withEmail("harrydoeprefix@fake-email.com")
.build();
String authToken = getAuthToken(admin.getEmail(), password);
getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")
.param("query", ePerson.getFirstName()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder(
EPersonMatcher.matchEPersonEntry(ePerson),
EPersonMatcher.matchEPersonEntry(ePerson3),
EPersonMatcher.matchEPersonEntry(ePerson4),
EPersonMatcher.matchEPersonEntry(ePerson5)
)))
.andExpect(jsonPath("$.page.totalElements", is(4)));
// it must be case insensitive
getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")
.param("query", ePerson.getFirstName().toLowerCase()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder(
EPersonMatcher.matchEPersonEntry(ePerson),
EPersonMatcher.matchEPersonEntry(ePerson3),
EPersonMatcher.matchEPersonEntry(ePerson4),
EPersonMatcher.matchEPersonEntry(ePerson5)
)))
.andExpect(jsonPath("$.page.totalElements", is(4)));
}
@Test
public void findByMetadataUsingEmail() throws Exception {
context.turnOffAuthorisationSystem();
EPerson ePerson = EPersonBuilder.createEPerson(context)
.withNameInMetadata("John", "Doe")
.withEmail("Johndoe@fake-email.com")
.build();
EPerson ePerson2 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Jane", "Smith")
.withEmail("janesmith@fake-email.com")
.build();
EPerson ePerson3 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Tom", "Doe")
.withEmail("tomdoe@fake-email.com")
.build();
EPerson ePerson4 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Dirk", "Doe-Postfix")
.withEmail("dirkdoepostfix@fake-email.com")
.build();
EPerson ePerson5 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Harry", "Prefix-Doe")
.withEmail("harrydoeprefix@fake-email.com")
.build();
String authToken = getAuthToken(admin.getEmail(), password);
getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")
.param("query", ePerson.getEmail()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.epersons", Matchers.contains(
EPersonMatcher.matchEPersonEntry(ePerson)
)))
.andExpect(jsonPath("$.page.totalElements", is(1)));
// it must be case insensitive
getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")
.param("query", ePerson.getEmail().toLowerCase()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.epersons", Matchers.contains(
EPersonMatcher.matchEPersonEntry(ePerson)
)))
.andExpect(jsonPath("$.page.totalElements", is(1)));
}
@Test
public void findByMetadataUsingUuid() throws Exception {
context.turnOffAuthorisationSystem();
EPerson ePerson = EPersonBuilder.createEPerson(context)
.withNameInMetadata("John", "Doe")
.withEmail("Johndoe@fake-email.com")
.build();
EPerson ePerson2 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Jane", "Smith")
.withEmail("janesmith@fake-email.com")
.build();
EPerson ePerson3 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Tom", "Doe")
.withEmail("tomdoe@fake-email.com")
.build();
EPerson ePerson4 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Dirk", "Doe-Postfix")
.withEmail("dirkdoepostfix@fake-email.com")
.build();
EPerson ePerson5 = EPersonBuilder.createEPerson(context)
.withNameInMetadata("Harry", "Prefix-Doe")
.withEmail("harrydoeprefix@fake-email.com")
.build();
String authToken = getAuthToken(admin.getEmail(), password);
getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")
.param("query", String.valueOf(ePerson.getID())))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.epersons", Matchers.contains(
EPersonMatcher.matchEPersonEntry(ePerson)
)))
.andExpect(jsonPath("$.page.totalElements", is(1)));
// it must be case insensitive
getClient(authToken).perform(get("/api/eperson/epersons/search/byMetadata")
.param("query", String.valueOf(ePerson.getID()).toLowerCase()))
.andExpect(status().isOk())
.andExpect(content().contentType(contentType))
.andExpect(jsonPath("$._embedded.epersons", Matchers.contains(
EPersonMatcher.matchEPersonEntry(ePerson)
)))
.andExpect(jsonPath("$.page.totalElements", is(1)));
}
@Test
public void findByMetadataUnauthorized() throws Exception {
getClient().perform(get("/api/eperson/epersons/search/byMetadata")

View File

@@ -119,7 +119,7 @@ public class EPersonBuilder extends AbstractDSpaceObjectBuilder<EPerson> {
ePersonService.delete(c, ePerson);
} catch (AuthorizeException e) {
// cannot occur, just wrap it to make the compiler happy
throw new RuntimeException(e.getMessage(), e);
throw new RuntimeException(e);
}
}
c.complete();