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.

Sign up for GitHub

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

Jump to bottom

Deaccessioned dataset file edit fix. #10901

Merged
merged 10 commits into from
Oct 3, 2024
Merged

Deaccessioned dataset file edit fix. #10901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
97a38ea
Fix Deaccessioned edition.
jp-tosca Oct 1, 2024
2aad633
Patch notes
jp-tosca Oct 1, 2024
db1a066
Added a signature of the method to receive includeDeaccessioned param
jp-tosca Oct 1, 2024
c2e1e15
Missing call to the new method
jp-tosca Oct 1, 2024
c46e0ce
Test added
jp-tosca Oct 2, 2024
2f4a84c
Typo
jp-tosca Oct 2, 2024
e57f834
Remove of clone from the API file deletion.
jp-tosca Oct 2, 2024
8accad7
Copy still needs to be created but not sent as a parameter to the com…
jp-tosca Oct 2, 2024
70a2651
File upload assertion
jp-tosca Oct 2, 2024
4e2bc1a
Update DatasetsIT.java
qqmyers Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/release-notes/10901deaccessioned file edit fix.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDatasetVersionCommand.java
View file
Open in desktop
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
View file
Open in desktop
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 test of deleting a file from a deaccessioned version.

// 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 diligence 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