publication date (issued) added #233
Conversation
| @@ -70,12 +72,13 @@ public void publishUnpublishUserSet_Success() throws Exception { | |||
|
|
|||
| // publish set by publisher | |||
| // expected change of ownership to editorial team | |||
| publishUserSet(userSet, getConfiguration().getEuropeanaPublisherNickname()); | |||
| String issued = "2018-10-31T01:30:00.000Z"; | |||
There was a problem hiding this comment.
must be used to verify the value in the response
| @@ -70,12 +72,13 @@ public void publishUnpublishUserSet_Success() throws Exception { | |||
|
|
|||
| // publish set by publisher | |||
| // expected change of ownership to editorial team | |||
| publishUserSet(userSet, getConfiguration().getEuropeanaPublisherNickname()); | |||
| String issued = "2018-10-31T01:30:00.000Z"; | |||
| publishUserSet(userSet, issued, getConfiguration().getEuropeanaPublisherNickname()); | |||
There was a problem hiding this comment.
response must be validated
There was a problem hiding this comment.
It is validated in the method publishUserSet.
| // unpublished set, the ownership is changed back to current user | ||
| assertFalse(containsKeyOrValue(result, getConfiguration().getEuropeanaPublisherNickname())); | ||
| assertTrue(containsKeyOrValue(result, USERNAME_PUBLISHER)); | ||
| assertEquals(HttpStatus.OK.value(), response.getStatus()); | ||
|
|
||
| publishUserSet(userSet, null, getConfiguration().getEuropeanaPublisherNickname()); |
There was a problem hiding this comment.
if this call is required, the response must be validated
| @@ -100,8 +107,7 @@ public void updatePublishedUserSet_Success() throws Exception { | |||
| WebUserSetImpl userSet = createTestUserSet(USER_SET_REGULAR, regularUserToken); | |||
|
|
|||
| // publish userset by other user, the ownership stays with the creator | |||
| publishUserSet(userSet, USERNAME_REGULAR); | |||
|
|
|||
| publishUserSet(userSet, null, USERNAME_REGULAR); | |||
There was a problem hiding this comment.
add check for the issued property
| @@ -135,7 +141,7 @@ public void updatePublishedUserSetWithVisibility_Success() throws Exception { | |||
| WebUserSetImpl userSet = createTestUserSet(USER_SET_REGULAR, regularUserToken); | |||
|
|
|||
| // publish userset by other user, the ownership stays with the creator | |||
| publishUserSet(userSet, USERNAME_REGULAR); | |||
| publishUserSet(userSet, null, USERNAME_REGULAR); | |||
There was a problem hiding this comment.
should verify the value of the issues field, the current date take before calling the method can be used for comparison as issues.after(requestTime)
| HttpServletRequest request) throws HttpException { | ||
| // check user credentials, if invalid respond with HTTP 401, | ||
| // or if unauthorized respond with HTTP 403 | ||
| Authentication authentication = verifyWriteAccess(SetOperations.PUBLISH, request); | ||
| return publishUnpublishUserSet(identifier, authentication, true, profileStr, request); | ||
| return publishUnpublishUserSet(identifier, authentication, true, profileStr, issued, request); |
There was a problem hiding this comment.
default value for issued must be set if missing in request
| @@ -868,6 +867,10 @@ PersistentUserSet updateUserSetForPublish(PersistentUserSet userSet, | |||
| userSet.setCreator(creator); | |||
| } | |||
| userSet.setVisibility(VisibilityTypes.PUBLISHED.getJsonValue()); | |||
| if(issued==null) { | |||
There was a problem hiding this comment.
here an exception should be thrown if the value is null, the controller is responsible for setting the default values for the optional fields
| @@ -895,6 +898,7 @@ PersistentUserSet updateUserSetForUnpublish(PersistentUserSet userSet, | |||
| } | |||
|
|
|||
| userSet.setVisibility(VisibilityTypes.PUBLIC.getJsonValue()); | |||
| userSet.setIssued(null); | |||
There was a problem hiding this comment.
this must be executed, on line 891 an exception must be thrown, so that the api returns 400
| if(issued==null) { | ||
| issued = new Date(); | ||
| } | ||
| userSet.setIssued(issued); |
There was a problem hiding this comment.
the specs were changed, this needs to be always executed, therefore the check from line 858 must be removed
|
|
||
| MockHttpServletResponse response; | ||
| String result; | ||
|
|
||
| // depublish set | ||
| // unpublish set |
There was a problem hiding this comment.
new tests for publishing published set and unpublishing not published set needs to be added
unpublish unpublished user sets
No description provided.