From 496bb35eea6ce94c75b4777359e1520cb09c66a1 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 20 Sep 2021 16:37:43 +0200 Subject: [PATCH] Implement community feedbacks --- .../versioning/VersionHistoryServiceImpl.java | 26 ++++ .../service/VersionHistoryService.java | 2 + .../impl/CanCreateVersionFeature.java | 4 +- .../impl/CanDeleteVersionFeature.java | 4 +- .../impl/CanEditVersionFeature.java | 4 +- .../impl/CanManageVersionsFeature.java | 4 +- .../converter/VersionHistoryConverter.java | 41 ++---- .../exception/DSpaceForbiddenException.java | 28 ----- ...sionHistoryDraftVersionLinkRepository.java | 4 +- .../repository/VersionRestRepository.java | 15 ++- ...orOfAInprogressSubmissionInformations.java | 3 + .../rest/VersionHistoryRestRepositoryIT.java | 118 +++++++++++++++++- .../app/rest/VersionRestRepositoryIT.java | 41 ++++++ .../CanCreateVersionFeatureIT.java | 3 +- 14 files changed, 211 insertions(+), 86 deletions(-) delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceForbiddenException.java diff --git a/dspace-api/src/main/java/org/dspace/versioning/VersionHistoryServiceImpl.java b/dspace-api/src/main/java/org/dspace/versioning/VersionHistoryServiceImpl.java index c7a2f9044c..96c39ac3a8 100644 --- a/dspace-api/src/main/java/org/dspace/versioning/VersionHistoryServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/versioning/VersionHistoryServiceImpl.java @@ -11,11 +11,15 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; import org.apache.commons.collections4.CollectionUtils; import org.dspace.authorize.AuthorizeException; +import org.dspace.authorize.service.AuthorizeService; import org.dspace.content.Item; import org.dspace.core.Context; +import org.dspace.eperson.EPerson; +import org.dspace.services.ConfigurationService; import org.dspace.versioning.dao.VersionHistoryDAO; import org.dspace.versioning.service.VersionHistoryService; import org.dspace.versioning.service.VersioningService; @@ -34,6 +38,12 @@ public class VersionHistoryServiceImpl implements VersionHistoryService { @Autowired(required = true) private VersioningService versioningService; + @Autowired + private AuthorizeService authorizeService; + + @Autowired + private ConfigurationService configurationService; + protected VersionHistoryServiceImpl() { } @@ -210,4 +220,20 @@ public class VersionHistoryServiceImpl implements VersionHistoryService { return versionHistoryDAO.findByItem(context, item); } + @Override + public boolean canSeeDraftVersion(Context context, VersionHistory versionHistory) throws SQLException { + Version version = this.getLatestVersion(context, versionHistory); + if (Objects.nonNull(version)) { + EPerson submitter = version.getItem().getSubmitter(); + boolean isAdmin = authorizeService.isAdmin(context); + boolean canCreateVersion = configurationService + .getBooleanProperty("versioning.submitterCanCreateNewVersion"); + if (!isAdmin && !(canCreateVersion && Objects.equals(submitter, context.getCurrentUser()))) { + return false; + } + return true; + } + return false; + } + } diff --git a/dspace-api/src/main/java/org/dspace/versioning/service/VersionHistoryService.java b/dspace-api/src/main/java/org/dspace/versioning/service/VersionHistoryService.java index e7c88879a5..cb900a0492 100644 --- a/dspace-api/src/main/java/org/dspace/versioning/service/VersionHistoryService.java +++ b/dspace-api/src/main/java/org/dspace/versioning/service/VersionHistoryService.java @@ -67,4 +67,6 @@ public interface VersionHistoryService extends DSpaceCRUDService public void remove(VersionHistory versionHistory, Version version); + public boolean canSeeDraftVersion(Context context, VersionHistory versionHistory) throws SQLException; + } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanCreateVersionFeature.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanCreateVersionFeature.java index 7c5645afa6..4dbc3cdeeb 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanCreateVersionFeature.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanCreateVersionFeature.java @@ -55,9 +55,7 @@ public class CanCreateVersionFeature implements AuthorizationFeature { } Item item = itemService.find(context, UUID.fromString(((ItemRest) object).getUuid())); if (Objects.nonNull(item)) { - String stringBlockEntity = configurationService.getProperty("versioning.block.entity"); - boolean isBlockEntity = StringUtils.isNotBlank(stringBlockEntity) ? - Boolean.valueOf(stringBlockEntity) : true; + boolean isBlockEntity = configurationService.getBooleanProperty("versioning.block.entity", true); boolean hasEntityType = StringUtils.isNotBlank(itemService. getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)); if (isBlockEntity && hasEntityType) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanDeleteVersionFeature.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanDeleteVersionFeature.java index c9aed2ebd6..7b47cfe444 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanDeleteVersionFeature.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanDeleteVersionFeature.java @@ -54,9 +54,7 @@ public class CanDeleteVersionFeature extends DeleteFeature { Version version = versioningService.getVersion(context, ((VersionRest)object).getId()); if (Objects.nonNull(version) && Objects.nonNull(version.getItem())) { ItemRest itemRest = itemConverter.convert(version.getItem(), DefaultProjection.DEFAULT); - String stringBlockEntity = configurationService.getProperty("versioning.block.entity"); - boolean isBlockEntity = StringUtils.isNotBlank(stringBlockEntity) ? - Boolean.valueOf(stringBlockEntity) : true; + boolean isBlockEntity = configurationService.getBooleanProperty("versioning.block.entity", true); boolean hasEntityType = StringUtils.isNotBlank( itemService.getMetadataFirstValue(version.getItem(), "dspace", "entity", "type", Item.ANY)); if (isBlockEntity && hasEntityType) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanEditVersionFeature.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanEditVersionFeature.java index 389e03b9bd..ed2d852272 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanEditVersionFeature.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanEditVersionFeature.java @@ -57,9 +57,7 @@ public class CanEditVersionFeature implements AuthorizationFeature { } Version version = versioningService.getVersion(context, (((VersionRest) object).getId())); if (Objects.nonNull(version) && Objects.nonNull(version.getItem())) { - String stringBlockEntity = configurationService.getProperty("versioning.block.entity"); - boolean isBlockEntity = StringUtils.isNotBlank(stringBlockEntity) ? - Boolean.valueOf(stringBlockEntity) : true; + boolean isBlockEntity = configurationService.getBooleanProperty("versioning.block.entity", true); boolean hasEntityType = StringUtils.isNotBlank( itemService.getMetadataFirstValue(version.getItem(), "dspace", "entity", "type", Item.ANY)); if (isBlockEntity && hasEntityType) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanManageVersionsFeature.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanManageVersionsFeature.java index c0b47f24a1..59452accee 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanManageVersionsFeature.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanManageVersionsFeature.java @@ -53,9 +53,7 @@ public class CanManageVersionsFeature implements AuthorizationFeature { } Item item = itemService.find(context, UUID.fromString(((ItemRest) object).getUuid())); if (Objects.nonNull(item)) { - String stringBlockEntity = configurationService.getProperty("versioning.block.entity"); - boolean isBlockEntity = StringUtils.isNotBlank(stringBlockEntity) ? - Boolean.valueOf(stringBlockEntity) : true; + boolean isBlockEntity = configurationService.getBooleanProperty("versioning.block.entity", true); boolean hasEntityType = StringUtils.isNotBlank(itemService. getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)); if (isBlockEntity && hasEntityType) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/VersionHistoryConverter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/VersionHistoryConverter.java index bd6b027063..97c1be7e26 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/VersionHistoryConverter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/VersionHistoryConverter.java @@ -15,13 +15,9 @@ import org.apache.logging.log4j.Logger; import org.dspace.app.rest.model.VersionHistoryRest; import org.dspace.app.rest.projection.Projection; import org.dspace.app.rest.utils.ContextUtil; -import org.dspace.authorize.service.AuthorizeService; import org.dspace.core.Context; -import org.dspace.eperson.EPerson; -import org.dspace.services.ConfigurationService; import org.dspace.services.RequestService; import org.dspace.services.model.Request; -import org.dspace.versioning.Version; import org.dspace.versioning.VersionHistory; import org.dspace.versioning.service.VersionHistoryService; import org.springframework.beans.factory.annotation.Autowired; @@ -38,12 +34,6 @@ public class VersionHistoryConverter implements DSpaceConverter idRef = new AtomicReference(); + String tokenAdmin = getAuthToken(admin.getEmail(), password); + + // retrieve the workspace item + getClient(tokenAdmin).perform(get("/api/submission/workspaceitems/search/item") + .param("uuid", String.valueOf(version.getItem().getID()))) + .andExpect(status().isOk()) + .andDo(result -> idRef.set(read(result.getResponse().getContentAsString(), "$.id"))); + + // submit the workspaceitem to complete the deposit + getClient(tokenAdmin).perform(post(BASE_REST_SERVER_URL + "/api/workflow/workflowitems") + .content("/api/submission/workspaceitems/" + idRef.get()) + .contentType(textUriContentType)) + .andExpect(status().isCreated()); + + getClient(tokenAdmin).perform(get("/api/versioning/versionhistories/" + vh.getID() + "/draftVersion")) + .andExpect(status().isNoContent()); + } + + @Test + public void findWorkflowItemOfDraftVersionAdminTest() throws Exception { + context.turnOffAuthorisationSystem(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + + Collection col = CollectionBuilder.createCollection(context, parentCommunity) + .withName("Collection 1") + .withWorkflowGroup(1, admin) + .build(); + + Item item = ItemBuilder.createItem(context, col) + .withTitle("Public test item") + .withIssueDate("2021-04-27") + .withAuthor("Doe, John") + .withSubject("ExtraEntry") + .build(); + + XmlWorkflowItem witem = WorkflowItemBuilder.createWorkflowItem(context, col) + .withTitle("Workflow Item 1") + .withIssueDate("2017-10-17") + .withAuthor("Doe, John") + .withSubject("ExtraEntry") + .build(); + + Version version = VersionBuilder.createVersion(context, item, "test").build(); + version.setItem(witem.getItem()); + VersionHistory vh = versionHistoryService.findByItem(context, version.getItem()); + context.turnOffAuthorisationSystem(); + + String tokenAdmin = getAuthToken(admin.getEmail(), password); + + getClient(tokenAdmin).perform(get("/api/versioning/versionhistories/" + vh.getID() + "/draftVersion")) + .andExpect(jsonPath("$", Matchers.is( + WorkflowItemMatcher.matchItemWithTitleAndDateIssuedAndSubject(witem, + "Workflow Item 1", "2017-10-17", "ExtraEntry")))); + } + @Test public void findWorkspaceItemOfDraftVersionLoggedUserTest() throws Exception { context.turnOffAuthorisationSystem(); @@ -412,7 +528,7 @@ public class VersionHistoryRestRepositoryIT extends AbstractControllerIntegratio } @Test - public void findVersionsOfVersionHistoryCheckPaginationAfterDelitingOfVersionTest() throws Exception { + public void findVersionsOfVersionHistoryCheckPaginationAfterDeletingOfVersionTest() throws Exception { //disable file upload mandatory configurationService.setProperty("webui.submit.upload.required", false); context.turnOffAuthorisationSystem(); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/VersionRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/VersionRestRepositoryIT.java index 397562f368..916aca0198 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/VersionRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/VersionRestRepositoryIT.java @@ -8,6 +8,7 @@ package org.dspace.app.rest; import static com.jayway.jsonpath.JsonPath.read; import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath; +import static org.hamcrest.Matchers.emptyOrNullString; import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.is; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -293,6 +294,46 @@ public class VersionRestRepositoryIT extends AbstractControllerIntegrationTest { } } + @Test + public void createFirstVersionWithoutSummaryTest() throws Exception { + context.turnOffAuthorisationSystem(); + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + + Collection col = CollectionBuilder.createCollection(context, parentCommunity) + .withName("Collection test") + .build(); + + Item item = ItemBuilder.createItem(context, col) + .withTitle("Public test item") + .withIssueDate("2021-04-27") + .withAuthor("Doe, John") + .withSubject("ExtraEntry") + .build(); + + context.restoreAuthSystemState(); + + AtomicReference idRef = new AtomicReference(); + String adminToken = getAuthToken(admin.getEmail(), password); + + try { + getClient(adminToken).perform(post("/api/versioning/versions") + .contentType(MediaType.parseMediaType(RestMediaTypes.TEXT_URI_LIST_VALUE)) + .content("/api/core/items/" + item.getID())) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$", Matchers.allOf( + hasJsonPath("$.version", is(2)), + hasJsonPath("$.summary", emptyOrNullString()), + hasJsonPath("$.submitterName", is("first (admin) last (admin)")), + hasJsonPath("$.type", is("version")) + ))) + .andDo(result -> idRef.set(read(result.getResponse().getContentAsString(), "$.id"))); + } finally { + VersionBuilder.delete(idRef.get()); + } + } + @Test public void createFirstVersionItemBadRequestTest() throws Exception { String adminToken = getAuthToken(admin.getEmail(), password); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanCreateVersionFeatureIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanCreateVersionFeatureIT.java index 2ee953d332..8a6d836466 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanCreateVersionFeatureIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanCreateVersionFeatureIT.java @@ -409,7 +409,7 @@ public class CanCreateVersionFeatureIT extends AbstractControllerIntegrationTest context.turnOffAuthorisationSystem(); configurationService.setProperty("versioning.submitterCanCreateNewVersion", true); - configurationService.setProperty("versioning.block.entity", ""); + configurationService.setProperty("versioning.block.entity", null); Community rootCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") @@ -417,7 +417,6 @@ public class CanCreateVersionFeatureIT extends AbstractControllerIntegrationTest Collection col = CollectionBuilder.createCollection(context, rootCommunity) .withName("Collection 1") - .withAdminGroup(eperson) .withSubmitterGroup(eperson) .build();