diff --git a/config/alfresco/public-rest-context.xml b/config/alfresco/public-rest-context.xml
index db84cfbfa5..dc0247a135 100644
--- a/config/alfresco/public-rest-context.xml
+++ b/config/alfresco/public-rest-context.xml
@@ -407,8 +407,6 @@
-
-
diff --git a/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java b/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java
index e1dfac183f..1c46b8e797 100644
--- a/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java
+++ b/source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java
@@ -55,7 +55,6 @@ import org.springframework.extensions.webscripts.WebScriptRequest;
* @author Sergey Scherbovich (based on existing JavaScript webscript controller)
* @since 4.1.7.1
*/
-
public class CommentsPost extends AbstractCommentsWebScript
{
/**
@@ -146,7 +145,8 @@ public class CommentsPost extends AbstractCommentsWebScript
{
isUpdated = ((Date)modified).getTime() - ((Date)created).getTime() > 5000;
}
-
+
+ // TODO refactor v0 Comments API to use CommentService (see ACE-5437)
Serializable owner = this.nodeService.getProperty(commentNodeRef, ContentModel.PROP_OWNER);
String currentUser = this.serviceRegistry.getAuthenticationService().getCurrentUserName();
diff --git a/source/java/org/alfresco/rest/api/impl/CommentsImpl.java b/source/java/org/alfresco/rest/api/impl/CommentsImpl.java
index 0156d1cf69..a377b072e1 100644
--- a/source/java/org/alfresco/rest/api/impl/CommentsImpl.java
+++ b/source/java/org/alfresco/rest/api/impl/CommentsImpl.java
@@ -71,8 +71,6 @@ public class CommentsImpl implements Comments
private NodeService nodeService;
private CommentService commentService;
private ContentService contentService;
- private LockService lockService;
- private PermissionService permissionService;
private TypeConstraint typeConstraint;
public void setTypeConstraint(TypeConstraint typeConstraint)
@@ -84,17 +82,7 @@ public class CommentsImpl implements Comments
{
this.nodes = nodes;
}
-
- public void setLockService(LockService lockService)
- {
- this.lockService = lockService;
- }
-
- public void setPermissionService(PermissionService permissionService)
- {
- this.permissionService = permissionService;
- }
-
+
public void setNodeService(NodeService nodeService)
{
this.nodeService = nodeService;
@@ -122,72 +110,16 @@ public class CommentsImpl implements Comments
nodeProps.remove(ContentModel.PROP_CONTENT);
}
- boolean canEdit = false;
- boolean canDelete = false;
- if (! isWorkingCopyOrLocked(nodeRef))
- {
- canEdit = canEditPermission(commentNodeRef);
- canDelete = canDeletePermission(commentNodeRef);
- }
+ Map map = commentService.getCommentPermissions(nodeRef, commentNodeRef);
+ boolean canEdit = map.get(CommentService.CAN_EDIT);
+ boolean canDelete = map.get(CommentService.CAN_DELETE);
Comment comment = new Comment(commentNodeRef.getId(), nodeProps, canEdit, canDelete);
return comment;
}
-
- private boolean isWorkingCopyOrLocked(NodeRef nodeRef)
- {
- boolean isWorkingCopy = false;
- boolean isLocked = false;
-
- if (nodeRef != null)
- {
- Set aspects = nodeService.getAspects(nodeRef);
-
- isWorkingCopy = aspects.contains(ContentModel.ASPECT_WORKING_COPY);
- if(!isWorkingCopy)
- {
- if(aspects.contains(ContentModel.ASPECT_LOCKABLE))
- {
- LockStatus lockStatus = lockService.getLockStatus(nodeRef);
- if (lockStatus == LockStatus.LOCKED || lockStatus == LockStatus.LOCK_OWNER)
- {
- isLocked = true;
- }
- }
- }
- }
- return (isWorkingCopy || isLocked);
- }
-
- private boolean canEdit(NodeRef nodeRef, NodeRef commentNodeRef)
- {
- return ((! isWorkingCopyOrLocked(nodeRef) && canEditPermission(commentNodeRef)));
- }
-
- // TODO refactor (ACE-5437) - see also CommentsPost
- private boolean canEditPermission(NodeRef commentNodeRef)
- {
- String creator = (String)nodeService.getProperty(commentNodeRef, ContentModel.PROP_CREATOR);
- Serializable owner = nodeService.getProperty(commentNodeRef, ContentModel.PROP_OWNER);
- String currentUser = AuthenticationUtil.getFullyAuthenticatedUser();
-
- boolean isSiteManager = permissionService.hasPermission(commentNodeRef, SiteModel.SITE_MANAGER) == (AccessStatus.ALLOWED);
- boolean isCoordinator = permissionService.hasPermission(commentNodeRef, PermissionService.COORDINATOR) == (AccessStatus.ALLOWED);
- return (isSiteManager || isCoordinator || currentUser.equals(creator) || currentUser.equals(owner));
- }
-
- private boolean canDelete(NodeRef nodeRef, NodeRef commentNodeRef)
- {
- return ((! isWorkingCopyOrLocked(nodeRef) || canDeletePermission(commentNodeRef)));
- }
-
- private boolean canDeletePermission(NodeRef commentNodeRef)
- {
- return permissionService.hasPermission(commentNodeRef, PermissionService.DELETE) == AccessStatus.ALLOWED;
- }
-
+
public Comment createComment(String nodeId, Comment comment)
{
NodeRef nodeRef = nodes.validateNode(nodeId);
@@ -223,13 +155,7 @@ public class CommentsImpl implements Comments
{
throw new InvalidArgumentException();
}
-
- // MNT-16446 (pending future ACE-5437)
- if (! canEdit(nodeRef, commentNodeRef))
- {
- throw new PermissionDeniedException("Cannot edit comment");
- }
-
+
commentService.updateComment(commentNodeRef, title, content);
return toComment(nodeRef, commentNodeRef);
}
@@ -274,20 +200,18 @@ public class CommentsImpl implements Comments
}
@Override
- // TODO validate that it is a comment of the node
public void deleteComment(String nodeId, String commentNodeId)
{
try
{
NodeRef nodeRef = nodes.validateNode(nodeId);
NodeRef commentNodeRef = nodes.validateNode(commentNodeId);
-
- // MNT-16446 (pending future ACE-5437)
- if (! canDelete(nodeRef, commentNodeRef))
+
+ if (! nodeRef.equals(commentService.getDiscussableAncestor(commentNodeRef)))
{
- throw new PermissionDeniedException("Cannot delete comment");
+ throw new InvalidArgumentException("Unexpected "+nodeId+","+commentNodeId);
}
-
+
commentService.deleteComment(commentNodeRef);
}
catch(IllegalArgumentException e)
diff --git a/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java b/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java
index c6284e7068..d752b10f89 100644
--- a/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java
+++ b/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java
@@ -753,23 +753,9 @@ public class TestNodeComments extends EnterpriseTestApi
comment.setContent("my comment");
Comment createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
- {
- TenantUtil.runAsUserTenant(new TenantRunAsWork()
- {
- @Override
- public Void doWork() throws Exception
- {
- repoService.lockNode(nodeRef1);
- return null;
- }
- }, person11.getId(), network1.getId());
-
- publicApiClient.setRequestContext(new RequestContext(network1.getId(), person11.getId()));
-
- Comment updatedComment = new Comment();
- updatedComment.setContent(null);
- commentsProxy.updateNodeComment(nodeRef1.getId(), createdComment.getId(), updatedComment);
- }
+ Comment updatedComment = new Comment();
+ updatedComment.setContent(null);
+ commentsProxy.updateNodeComment(nodeRef1.getId(), createdComment.getId(), updatedComment);
fail();
}
@@ -778,9 +764,15 @@ public class TestNodeComments extends EnterpriseTestApi
assertEquals(HttpStatus.SC_BAD_REQUEST, e.getHttpResponse().getStatusCode());
}
- // locked node comments
+ // locked node - cannot add/edit/delete comments (MNT-14945, MNT-16446)
try
{
+ publicApiClient.setRequestContext(new RequestContext(network1.getId(), person11.getId()));
+
+ Comment comment = new Comment();
+ comment.setContent("my comment");
+ Comment createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
+
TenantUtil.runAsUserTenant(new TenantRunAsWork()
{
@Override
@@ -801,10 +793,18 @@ public class TestNodeComments extends EnterpriseTestApi
commentsProxy.getNodeComments(nodeRef1.getId(), createParams(paging, null));
// test POST for a locked node
+ try
+ {
+ comment = new Comment();
+ comment.setContent("my other comment");
+ createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
- Comment comment = new Comment();
- comment.setContent("my comment");
- Comment createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
+ fail("");
+ }
+ catch(PublicApiException e)
+ {
+ assertEquals(HttpStatus.SC_FORBIDDEN, e.getHttpResponse().getStatusCode());
+ }
// test PUT for a locked node
{