Skip to content

Commit

Permalink
#3943 - Remove reasons for return from get Notifications API
Browse files Browse the repository at this point in the history 
Also, completely remove Workflow Action.
  • Loading branch information
@sekmiller
sekmiller committed Jul 18, 2017
1 parent eb5d93e commit 2107d88
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 124 deletions.
2 changes: 0 additions & 2 deletions src/main/java/edu/harvard/iq/dataverse/api/Datasets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@
import edu.harvard.iq.dataverse.util.SystemConfig;
import edu.harvard.iq.dataverse.util.json.JsonParseException;
import static edu.harvard.iq.dataverse.util.json.JsonPrinter.*;
import edu.harvard.iq.dataverse.workflows.WorkflowAction;
import edu.harvard.iq.dataverse.workflows.WorkflowComment;
import java.io.InputStream;
import java.io.StringReader;
import java.util.Collections;
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/api/Notifications.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,16 @@ public Response getAllNotificationsForUser() {
Type type = notification.getType();
notificationObjectBuilder.add("id", notification.getId());
notificationObjectBuilder.add("type", type.toString());
/* FIXME - Re-add reasons for return if/when they are added to the notifications page.
if (Type.RETURNEDDS.equals(type) || Type.SUBMITTEDDS.equals(type)) {
JsonArrayBuilder reasons = getReasonsForReturn(notification);
for (JsonValue reason : reasons.build()) {
reasonsForReturn.add(reason);
}
notificationObjectBuilder.add("reasonsForReturn", reasonsForReturn);
}
*/
jsonArrayBuilder.add(notificationObjectBuilder);
}
JsonObjectBuilder result = Json.createObjectBuilder().add("notifications", jsonArrayBuilder);
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ReturnDatasetToAuthorCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException;
import edu.harvard.iq.dataverse.util.BundleUtil;
import edu.harvard.iq.dataverse.workflows.WorkflowAction;
import static edu.harvard.iq.dataverse.workflows.WorkflowAction.Type.RETURN_TO_AUTHOR;
import edu.harvard.iq.dataverse.workflows.WorkflowComment;
import java.sql.Timestamp;
import java.util.Date;
Expand Down Expand Up @@ -62,7 +60,7 @@ public Dataset save(CommandContext ctxt) throws CommandException {

DatasetVersionUser ddu = ctxt.datasets().getDatasetVersionUser(theDataset.getLatestVersion(), this.getUser());

WorkflowComment workflowComment = new WorkflowComment(new WorkflowAction(theDataset.getEditVersion(), RETURN_TO_AUTHOR), comment, (AuthenticatedUser) this.getUser());
WorkflowComment workflowComment = new WorkflowComment(theDataset.getEditVersion(), WorkflowComment.Type.RETURN_TO_AUTHOR, comment, (AuthenticatedUser) this.getUser());
ctxt.datasets().addWorkflowComment(workflowComment);

if (ddu != null) {
Expand Down
90 changes: 0 additions & 90 deletions src/main/java/edu/harvard/iq/dataverse/workflows/WorkflowAction.java
View file

This file was deleted.

21 changes: 14 additions & 7 deletions src/main/java/edu/harvard/iq/dataverse/workflows/WorkflowComment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
@Entity
public class WorkflowComment implements Serializable {

/*
This release only supports Return to Author as a comment type
More may be added in future releases,
*/
public enum Type {
RETURN_TO_AUTHOR //, SUBMIT_FOR_REVIEW not available in this release but may be added in the future
};

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
Expand All @@ -40,7 +48,7 @@ public class WorkflowComment implements Serializable {
*/
@Column(nullable = false)
@Enumerated(EnumType.STRING)
private WorkflowAction.Type type;
private Type type;

// This is nullable because when you submit for review, especially the first time, you don't have to enter a message.
@Column(nullable = true, columnDefinition = "TEXT")
Expand All @@ -59,11 +67,10 @@ public class WorkflowComment implements Serializable {
// TODO: How should we best associate these entries to notifications, which can go to multiple authors and curators?
// @Transient
// private List<UserNotification> notifications;
public WorkflowComment(WorkflowAction workflowAction, String message, AuthenticatedUser authenticatedUser) {
this.type = workflowAction.getType();
if (this.type.equals(WorkflowAction.Type.RETURN_TO_AUTHOR)) {
DatasetVersion datasetVersionFromAction = (DatasetVersion) workflowAction.getTarget();
this.datasetVersion = datasetVersionFromAction;
public WorkflowComment(DatasetVersion version, WorkflowComment.Type type, String message, AuthenticatedUser authenticatedUser) {
this.type = type;
if (this.type.equals(WorkflowComment.Type.RETURN_TO_AUTHOR)) {
this.datasetVersion = version;
}
this.message = message;
this.authenticatedUser = authenticatedUser;
Expand Down Expand Up @@ -92,7 +99,7 @@ public DatasetVersion getDatasetVersion() {
return datasetVersion;
}

public WorkflowAction.Type getType() {
public Type getType() {
return type;
}

Expand Down
45 changes: 23 additions & 22 deletions src/test/java/edu/harvard/iq/dataverse/api/InReviewWorkflowIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,16 @@ public void testCuratorSendsCommentsToAuthor() {
returnToAuthorAlreadyReturned.then().assertThat()
.body("message", equalTo("This dataset cannot be return to the author(s) because the latest version is not In Review. The author(s) needs to click Submit for Review first."))
.statusCode(FORBIDDEN.getStatusCode());

//FIXME when/if reasons for return are returned to notifications page and the API is
// updated appropriately, these tests will have to be updated.
Response authorChecksForCommentsAgain = UtilIT.getNotifications(authorApiToken);
authorChecksForCommentsAgain.prettyPrint();
authorChecksForCommentsAgain.then().assertThat()
.body("data.notifications[0].type", equalTo("RETURNEDDS"))
// The author thinks, "This why we have curators!"
.body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
//.body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[1].type", equalTo("CREATEACC"))
.body("data.notifications[1].reasonsForReturn", equalTo(null))
//.body("data.notifications[1].reasonsForReturn", equalTo(null))
.statusCode(OK.getStatusCode());

// The author upload the file she forgot.
Expand All @@ -206,12 +207,12 @@ public void testCuratorSendsCommentsToAuthor() {
curatorChecksNotifications.then().assertThat()
// TODO: Test this issue from the UI as well: https://github.com/IQSS/dataverse/issues/2526
.body("data.notifications[0].type", equalTo("SUBMITTEDDS"))
.body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
//.body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[1].type", equalTo("SUBMITTEDDS"))
// Yes, it's a little weird that the first "SUBMITTEDDS" notification now shows the return reason when it showed nothing before. For now we are simply always showing all the reasons for return. They start to stack up. That way you can see the history.
.body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
//.body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[2].type", equalTo("CREATEACC"))
.body("data.notifications[2].reasonsForReturn", equalTo(null))
//.body("data.notifications[2].reasonsForReturn", equalTo(null))
.statusCode(OK.getStatusCode());

String reasonForReturn2 = "A README is required.";
Expand All @@ -226,14 +227,14 @@ public void testCuratorSendsCommentsToAuthor() {
authorChecksForComments3.prettyPrint();
authorChecksForComments3.then().assertThat()
.body("data.notifications[0].type", equalTo("RETURNEDDS"))
.body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[0].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[1].type", equalTo("RETURNEDDS"))
// Yes, it's a little weird that the reason for return on the first "RETURNEDDS" changed. We're showing the history.
.body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[1].reasonsForReturn[1].message", equalTo("A README is required."))
// .body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[1].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[2].type", equalTo("CREATEACC"))
.body("data.notifications[2].reasonsForReturn", equalTo(null))
// .body("data.notifications[2].reasonsForReturn", equalTo(null))
.statusCode(OK.getStatusCode());

String pathToReadme = "README.md";
Expand All @@ -257,17 +258,17 @@ public void testCuratorSendsCommentsToAuthor() {
curatorHopesTheReadmeIsThereNow.then().assertThat()
// TODO: Test this issue from the UI as well: https://github.com/IQSS/dataverse/issues/2526
.body("data.notifications[0].type", equalTo("SUBMITTEDDS"))
.body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[0].reasonsForReturn[1].message", equalTo("A README is required."))
// .body("data.notifications[0].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[0].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[1].type", equalTo("SUBMITTEDDS"))
.body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[1].reasonsForReturn[1].message", equalTo("A README is required."))
// .body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[1].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[2].type", equalTo("SUBMITTEDDS"))
// Yes, it's a little weird that the first "SUBMITTEDDS" notification now shows the return reason when it showed nothing before. We're showing the history.
.body("data.notifications[2].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[2].reasonsForReturn[1].message", equalTo("A README is required."))
// .body("data.notifications[2].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[2].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[3].type", equalTo("CREATEACC"))
.body("data.notifications[3].reasonsForReturn", equalTo(null))
// .body("data.notifications[3].reasonsForReturn", equalTo(null))
.statusCode(OK.getStatusCode());

// The curator publishes the dataverse.
Expand All @@ -288,14 +289,14 @@ public void testCuratorSendsCommentsToAuthor() {
// FIXME: Why is this ASSIGNROLE and not "your dataset has been published"?
.body("data.notifications[0].type", equalTo("ASSIGNROLE"))
.body("data.notifications[1].type", equalTo("RETURNEDDS"))
.body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[1].reasonsForReturn[1].message", equalTo("A README is required."))
// .body("data.notifications[1].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[1].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[2].type", equalTo("RETURNEDDS"))
// Yes, it's a little weird that the reason for return on the first "RETURNEDDS" changed. For now we are always showing the most recent reason for return.
.body("data.notifications[2].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
// .body("data.notifications[2].reasonsForReturn[0].message", equalTo("You forgot to upload any files."))
.body("data.notifications[2].reasonsForReturn[1].message", equalTo("A README is required."))
.body("data.notifications[3].type", equalTo("CREATEACC"))
.body("data.notifications[3].reasonsForReturn", equalTo(null))
// .body("data.notifications[3].type", equalTo("CREATEACC"))
// .body("data.notifications[3].reasonsForReturn", equalTo(null))
.statusCode(OK.getStatusCode());

// These println's are here in case you want to log into the GUI to see what notifications look like.
Expand Down

0 comments on commit 2107d88

Please sign in to comment.