Published datasets should contain files #10994
Conversation
stevenwinship
added
Size: 30
src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java
Outdated
Show resolved
Hide resolved
stevenwinship
force-pushed
the
10981-published-datasets-should-contain-files
branch
from
d5bb6c2 to
85af55f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } | ||
| } | ||
|
|
||
| private boolean requiresFilesToPublishDataset() { |
There was a problem hiding this comment.
I think in other cases we've used the patter getEffectiveRequireFilesToPublishDataset which might be useful for consistency. I also think this should probably be in Dataverse.java with the get/set - when there's a UI, we need to know the effective setting at other times than just publishing.
There was a problem hiding this comment.
renamed and added to Dataverse.Java
| @@ -292,6 +292,9 @@ public static JsonObjectBuilder json(Dataverse dv, Boolean hideEmail, Boolean re | |||
| if (dv.getFilePIDsEnabled() != null) { | |||
| bld.add("filePIDsEnabled", dv.getFilePIDsEnabled()); | |||
| } | |||
| if (dv.getRequireFilesToPublishDataset() != null) { | |||
| bld.add("requireFilesToPublishDataset", dv.getRequireFilesToPublishDataset()); | |||
| } | |||
There was a problem hiding this comment.
FWIW: My guess is that the SPA will want the effective settings for this and other things (file pids, storage driver, etc.). Not sure if that would go here or not. I guess it can wait until we address the whole set.
There was a problem hiding this comment.
swapped the requireFilesToPublishDataset for effectiveRequiresFilesToPublishDataset
|
|
||
| if (getDataset().getFiles().isEmpty() && requiresFilesToPublishDataset()) { | ||
| throw new IllegalCommandException(BundleUtil.getStringFromBundle("dataset.mayNotPublish.FilesRequired"), this); | ||
| } |
There was a problem hiding this comment.
This gives a warning in the current UI, right? It would probably be better to just disable the publish button, or have a persistent message that files must be added before this dataset can be published, but if just failing with a warning is OK with @sbarbosadataverse, et. al., the current code avoids changes to the current UI code.
There was a problem hiding this comment.
@stevenwinship - can you post a UI image for what this looks like?
There was a problem hiding this comment.
@stevenwinship images like this are so helpful. Can you please also add it under "Does this PR introduce a user interface change?" above?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I have one comment @stevenwinship can we say "published datasets should contain at least one 'data' file? To help users understand we want data and not just a pdf of am article? |
|
Changed the message
|
This comment has been minimized.
This comment has been minimized.
|
@stevenwinship Looks like the branch has conflicts which must be resolved. |
…://github.com/IQSS/dataverse into 10981-published-datasets-should-contain-files
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
|
I faced some issues in docker while trying to build and test this PR in local. Server log attached |
What this PR does / why we need it: The existing publishing model in HDV allows the publishing of datasets without files. This lends to the curation team contacting depositors about uploading files via support and often to deleting "empty" datasets.
To decrease the support contact and dataset deletions, change the publishing model to require all deposits contain at least one file before publishing can happen.
Which issue(s) this PR closes:#10981
Special notes for your reviewer:
Suggestions on how to test this: See the added IT test
Does this PR introduce a user interface change? If mockups are available, please link/include them here: No
Is there a release notes update needed for this change?: Included
Additional documentation: