Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deaccessioned dataset file edit fix. #10901

Merged
merged 10 commits into from
Oct 3, 2024
Merged
1 change: 1 addition & 0 deletions doc/release-notes/10901deaccessioned file edit fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When a dataset was deaccessioned and was the only previous version it will cause an error when trying to update the files.
9 changes: 7 additions & 2 deletions src/main/java/edu/harvard/iq/dataverse/Dataset.java
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,20 @@ public DatasetVersion getLatestVersion() {
return getVersions().get(0);
}

public DatasetVersion getLatestVersionForCopy() {
public DatasetVersion getLatestVersionForCopy(boolean includeDeaccessioned) {
for (DatasetVersion testDsv : getVersions()) {
if (testDsv.isReleased() || testDsv.isArchived()) {
if (testDsv.isReleased() || testDsv.isArchived()
|| (testDsv.isDeaccessioned() && includeDeaccessioned)) {
return testDsv;
}
}
return getVersions().get(0);
}

public DatasetVersion getLatestVersionForCopy(){
return getLatestVersionForCopy(false);
}

public List<DatasetVersion> getVersions() {
return versions;
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/edu/harvard/iq/dataverse/api/Files.java
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,10 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP
DataFile dataFile = findDataFileOrDie(fileIdOrPersistentId);
FileMetadata fileToDelete = dataFile.getLatestFileMetadata();
Dataset dataset = dataFile.getOwner();
DatasetVersion v = dataset.getOrCreateEditVersion();
dataset.getOrCreateEditVersion();
deletePhysicalFile = !dataFile.isReleased();

UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, Arrays.asList(fileToDelete), v);
UpdateDatasetVersionCommand update_cmd = new UpdateDatasetVersionCommand(dataset, dvRequest, Arrays.asList(fileToDelete));
update_cmd.setValidateLenient(true);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public Dataset execute(CommandContext ctxt) throws CommandException {
*/
if(persistedVersion==null) {
Long id = getDataset().getLatestVersion().getId();
persistedVersion = ctxt.datasetVersion().find(id!=null ? id: getDataset().getLatestVersionForCopy().getId());
persistedVersion = ctxt.datasetVersion().find(id!=null ? id : getDataset().getLatestVersionForCopy(true).getId());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the true parameter so that only this call is affected resolved my earlier comment. Thinking further on this particular use - we're getting the persistedVersion so we can see if there are changes to a system metadatablock (line 122) in which case you need a key to make changes. It's an interesting question whether that comparison should be versus the last public/published version if there is one or against a later deaccessioned one (if it exists). Probably rare enough that we should just ignore my comment and accept the fix as is.

}

//Will throw an IllegalCommandException if a system metadatablock is changed and the appropriate key is not supplied.
Expand Down
54 changes: 54 additions & 0 deletions src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,60 @@ public void testCreatePublishDestroyDataset() {
deleteDatasetResponse.prettyPrint();
assertEquals(200, deleteDatasetResponse.getStatusCode());

// Start of deaccession test.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow - the testCreatePublishDestroyDataset method is doing quite a bit more than testing destroy! +1 for splitting it up some day.


// Create Dataset for deaccession test.
Response deaccessionTestDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken);
deaccessionTestDataset.prettyPrint();
deaccessionTestDataset.then().assertThat().statusCode(CREATED.getStatusCode());
Integer deaccessionTestDatasetId = UtilIT.getDatasetIdFromResponse(deaccessionTestDataset);

// File upload for deaccession test.
String pathToFile = "src/main/webapp/resources/images/dataverseproject.png";
Response uploadResponse = UtilIT.uploadFileViaNative(deaccessionTestDatasetId.toString(), pathToFile, apiToken);
uploadResponse.prettyPrint();
uploadResponse.then().assertThat().statusCode(OK.getStatusCode());
Integer deaccessionTestFileId = JsonPath.from(uploadResponse.body().asString()).getInt("data.files[0].dataFile.id");

// Publish Dataset for deaccession test.
Response deaccessionTestPublishResponse = UtilIT.publishDatasetViaNativeApi(deaccessionTestDatasetId, "major", apiToken);
deaccessionTestPublishResponse.prettyPrint();

// Deaccession Dataset for deaccession test.
Response deaccessionTestDatasetResponse = UtilIT.deaccessionDataset(deaccessionTestDatasetId, DS_VERSION_LATEST_PUBLISHED, "Test deaccession reason.", null, apiToken);
deaccessionTestDatasetResponse.prettyPrint();
deaccessionTestDatasetResponse.then().assertThat().statusCode(OK.getStatusCode());

// Version check for deaccession test - Deaccessioned.
Response deaccessionTestVersions = UtilIT.getDatasetVersions(deaccessionTestDatasetId.toString(), apiToken);
deaccessionTestVersions.prettyPrint();
deaccessionTestVersions.then().assertThat()
.body("data[0].latestVersionPublishingState", equalTo("DEACCESSIONED"))
.statusCode(OK.getStatusCode());

// File deletion / Draft creation due deltigion check for deaccession test.
Response deaccessionTestDeleteFile = UtilIT.deleteFileInDataset(deaccessionTestFileId, apiToken);
deaccessionTestDeleteFile.prettyPrint();
deaccessionTestDeleteFile
.then().assertThat()
.statusCode(OK.getStatusCode());

// Version check for deaccession test - Draft.
deaccessionTestVersions = UtilIT.getDatasetVersions(deaccessionTestDatasetId.toString(), apiToken);
deaccessionTestVersions.prettyPrint();
deaccessionTestVersions.then().assertThat()
.body("data[0].latestVersionPublishingState", equalTo("DRAFT"))
.statusCode(OK.getStatusCode());

// Deleting Dataset for deaccession test.
Response deaccessionTestDelete = UtilIT.destroyDataset(deaccessionTestDatasetId, apiToken);
deaccessionTestDelete.prettyPrint();
deaccessionTestDelete.then()
.assertThat()
.statusCode(OK.getStatusCode());

// End of deaccession test.

Response deleteDataverseResponse = UtilIT.deleteDataverse(dataverseAlias, apiToken);
deleteDataverseResponse.prettyPrint();
assertEquals(200, deleteDataverseResponse.getStatusCode());
Expand Down
Loading