Feature: Optionally make embargo reason required (#12067)

* require embargo reason option

* fix validation

* update api to also check the optional/not blank case

* release note

* doc updates

* fix test

* make content-type explicit - catch date parsing error

* comment re parse error handling

* refactor validation

* add validation tests

* fix blank test, update deprecated settings call

* api tests

* use properties per review

Author: qqmyers

doc/release-notes/12067-require-embargo-reason.md

@@ -0,0 +1,8 @@
+It is now possible to configure Dataverse to require an embargo reason when a user creates an embargo on one or more files.
+By default the embargo reason is optional.
+
+In addition, with this release, if an embargo reason is supplied, it must not be blank.
+
+New Feature Flag:
+
+dataverse.feature.require-embargo-reason - default false 

doc/sphinx-guides/source/api/native-api.rst

@@ -3697,14 +3697,14 @@ Set an Embargo on Files in a Dataset
 
 ``/api/datasets/$dataset-id/files/actions/:set-embargo`` can be used to set an embargo on one or more files in a dataset. Embargoes can be set on files that are only in a draft dataset version (and are not in any previously published version) by anyone who can edit the dataset. The same API call can be used by a superuser to add an embargo to files that have already been released as part of a previously published dataset version. 
 
-The API call requires a Json body that includes the embargo's end date (dateAvailable), a short reason (optional), and a list of the fileIds that the embargo should be set on. The dateAvailable must be after the current date and the duration (dateAvailable - today's date) must be less than the value specified by the :ref:`:MaxEmbargoDurationInMonths` setting. All files listed must be in the specified dataset. For example: 
+The API call requires a Json body that includes the embargo's end date (dateAvailable - YYYY-MM-DD format), a short reason (must not consist of whitespace only, optional unless Dataverse is configured to make it required), and a list of the fileIds that the embargo should be set on. The dateAvailable must be after the current date and the duration (dateAvailable - today's date) must be less than the value specified by the :ref:`:MaxEmbargoDurationInMonths` setting. All files listed must be in the specified dataset. For example: 
 
 .. code-block:: bash
 
   export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
   export SERVER_URL=https://demo.dataverse.org
   export PERSISTENT_IDENTIFIER=doi:10.5072/FK2/7U7YBV
-  export JSON='{"dateAvailable":"2021-10-20", "reason":"Standard project embargo", "fileIds":[300,301,302]}'
+  export JSON='{"dateAvailable":"2021-01-20", "reason":"Standard project embargo", "fileIds":[300,301,302]}'
 
   curl -H "X-Dataverse-key: $API_TOKEN" -H "Content-Type:application/json" "$SERVER_URL/api/datasets/:persistentId/files/actions/:set-embargo?persistentId=$PERSISTENT_IDENTIFIER" -d "$JSON"
 

doc/sphinx-guides/source/user/dataset-management.rst

@@ -750,7 +750,7 @@ Note that only one Preview URL (normal or with anonymized access) can be configu
 Embargoes
 =========
 
-A Dataverse instance may be configured to support file-level embargoes. Embargoes make file content inaccessible after a dataset version is published  - until the embargo end date.
+A Dataverse instance may be configured to support file-level embargoes. Embargoes make file content inaccessible after a dataset version is published  - until the embargo end date. A reason for the embargo may be supplied when creating the embargo. A reason may be required in some Dataverse instances.
 This means that file previews and the ability to download files will be blocked. The effect is similar to when a file is restricted except that the embargo will end at the specified date without further action and during the embargo, requests for file access cannot be made. 
 Embargoes of files in a version 1.0 dataset may also affect the date shown in the dataset and file citations. The recommended practice is for the citation to reflect the date on which all embargoes on files in version 1.0 end. (Since Dataverse creates one persistent identifier per dataset and doesn't create new ones for each version, the publication of later versions, with or without embargoed files, does not affect the citation date.)
 

src/main/java/edu/harvard/iq/dataverse/DatasetPage.java

@@ -157,6 +157,7 @@
 import edu.harvard.iq.dataverse.search.SearchFields;
 import edu.harvard.iq.dataverse.search.SearchUtil;
 import edu.harvard.iq.dataverse.search.SolrClientService;
+import edu.harvard.iq.dataverse.settings.FeatureFlags;
 import edu.harvard.iq.dataverse.settings.JvmSettings;
 import edu.harvard.iq.dataverse.util.SignpostingResources;
 import edu.harvard.iq.dataverse.util.FileMetadataUtil;
@@ -6887,4 +6888,7 @@ public void setRequestedCSL(String requestedCSL) {
         this.requestedCSL = requestedCSL;
     }
 
-}
+    public void validateEmbargoReason(FacesContext context, UIComponent component, Object value) {
+        FileUtil.validateEmbargoReason(context, component, value, removeEmbargo);
+    }
+}

src/main/java/edu/harvard/iq/dataverse/FilePage.java

@@ -35,6 +35,7 @@
 import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean;
 import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean.MakeDataCountEntry;
 import edu.harvard.iq.dataverse.privateurl.PrivateUrlServiceBean;
+import edu.harvard.iq.dataverse.settings.FeatureFlags;
 import edu.harvard.iq.dataverse.settings.JvmSettings;
 import edu.harvard.iq.dataverse.settings.SettingsServiceBean;
 import edu.harvard.iq.dataverse.util.BundleUtil;
@@ -60,7 +61,9 @@
 import jakarta.ejb.EJB;
 import jakarta.ejb.EJBException;
 import jakarta.faces.application.FacesMessage;
+import jakarta.faces.component.UIComponent;
 import jakarta.faces.context.FacesContext;
+import jakarta.faces.validator.ValidatorException;
 import jakarta.faces.view.ViewScoped;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
@@ -1489,4 +1492,8 @@ public String editFileMetadata(){
         return "";
     }
 
+    public void validateEmbargoReason(FacesContext context, UIComponent component, Object value) {
+        FileUtil.validateEmbargoReason(context, component, value, removeEmbargo);
+    }
+    
 }

src/main/java/edu/harvard/iq/dataverse/api/Datasets.java

@@ -1444,6 +1444,8 @@ public Response moveDataset(@Context ContainerRequestContext crc, @PathParam("id
     @POST
     @AuthRequired
     @Path("{id}/files/actions/:set-embargo")
+    @Consumes(MediaType.APPLICATION_JSON)
+    @Produces(MediaType.APPLICATION_JSON)
     public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathParam("id") String id, String jsonBody){
 
         // user is authenticated
@@ -1486,7 +1488,7 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar
         // check if embargoes are allowed(:MaxEmbargoDurationInMonths), gets the :MaxEmbargoDurationInMonths setting variable, if 0 or not set(null) return 400
         long maxEmbargoDurationInMonths = 0;
         try {
-            maxEmbargoDurationInMonths  = Long.parseLong(settingsService.get(SettingsServiceBean.Key.MaxEmbargoDurationInMonths.toString()));
+            maxEmbargoDurationInMonths  = Long.parseLong(settingsService.getValueForKey(SettingsServiceBean.Key.MaxEmbargoDurationInMonths));
         } catch (NumberFormatException nfe){
             if (nfe.getMessage().contains("null")) {
                 return error(Status.BAD_REQUEST, "No Embargoes allowed");
@@ -1496,13 +1498,19 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar
             return error(Status.BAD_REQUEST, "No Embargoes allowed");
         }
 
+        //Any parsing error should be handled via the JsonExceptionsHandler
         JsonObject json = JsonUtil.getJsonObject(jsonBody);
 
         Embargo embargo = new Embargo();
 
 
         LocalDate currentDateTime = LocalDate.now();
-        LocalDate dateAvailable = LocalDate.parse(json.getString("dateAvailable"));
+        LocalDate dateAvailable = null;
+        try {
+            dateAvailable = LocalDate.parse(json.getString("dateAvailable"));
+        } catch (DateTimeParseException e) {
+            return error(Status.BAD_REQUEST, "Unable to parse dateAvailable");
+        }
 
         // check :MaxEmbargoDurationInMonths if -1
         LocalDate maxEmbargoDateTime = maxEmbargoDurationInMonths != -1 ? LocalDate.now().plusMonths(maxEmbargoDurationInMonths) : null;
@@ -1519,8 +1527,16 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar
                 return error(Status.BAD_REQUEST, "Date available can not exceed MaxEmbargoDurationInMonths: "+maxEmbargoDurationInMonths);
             }
         }
-
-        embargo.setReason(json.getString("reason"));
+        String reason = null;
+        if(json.containsKey("reason")) {
+            reason = json.getString("reason");
+        }
+        if(reason == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) {
+            return error(Status.BAD_REQUEST, BundleUtil.getStringFromBundle("embargo.reason.required"));
+        } else if(reason != null && reason.isBlank()) {
+            return error(Status.BAD_REQUEST, BundleUtil.getStringFromBundle("embargo.reason.blank"));
+        }
+        embargo.setReason(reason);
 
         List<DataFile> datasetFiles = dataset.getFiles();
         List<DataFile> filesToEmbargo = new LinkedList<>();
@@ -1600,6 +1616,8 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar
     @POST
     @AuthRequired
     @Path("{id}/files/actions/:unset-embargo")
+    @Consumes(MediaType.APPLICATION_JSON)
+    @Produces(MediaType.APPLICATION_JSON)
     public Response removeFileEmbargo(@Context ContainerRequestContext crc, @PathParam("id") String id, String jsonBody){
 
         // user is authenticated

src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java

@@ -249,7 +249,11 @@ public enum FeatureFlags {
      * @since Dataverse 6.9
      */ 
     ONLY_UPDATE_DATACITE_WHEN_NEEDED("only-update-datacite-when-needed"),
-
+    
+    /** Require Embargo Reason. By default, adding a reason when embargoing is optional. This 
+     * flag makes a reason required, both in the UI and API.
+     */
+    REQUIRE_EMBARGO_REASON("require-embargo-reason"),
     ;
 
     final String flag;

src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java

@@ -37,6 +37,7 @@
 import edu.harvard.iq.dataverse.ingest.IngestableDataChecker;
 import edu.harvard.iq.dataverse.license.License;
 import edu.harvard.iq.dataverse.settings.ConfigCheckService;
+import edu.harvard.iq.dataverse.settings.FeatureFlags;
 import edu.harvard.iq.dataverse.settings.JvmSettings;
 import edu.harvard.iq.dataverse.util.file.BagItFileHandler;
 import edu.harvard.iq.dataverse.util.file.BagItFileHandlerFactory;
@@ -87,6 +88,10 @@
 import jakarta.activation.MimetypesFileTypeMap;
 import jakarta.ejb.EJBException;
 import jakarta.enterprise.inject.spi.CDI;
+import jakarta.faces.application.FacesMessage;
+import jakarta.faces.component.UIComponent;
+import jakarta.faces.context.FacesContext;
+import jakarta.faces.validator.ValidatorException;
 import jakarta.json.JsonArray;
 import jakarta.json.JsonObject;
 
@@ -1836,6 +1841,47 @@ public static boolean isActivelyEmbargoed(List<FileMetadata> fmdList) {
         }
         return false;
     }
+    
+    /**
+     * Validates that an embargo reason is not blank, and exists when required.
+     * This method is designed to be called from JSF validator methods.
+     * 
+     * @param context The FacesContext
+     * @param component The UIComponent being validated
+     * @param value The value to validate (embargo reason)
+     * @param removeEmbargo Whether the embargo is being removed (skips validation if true)
+     * @param saveButtonId The ID pattern of the save button that should trigger validation
+     * @throws ValidatorException if validation fails
+     */
+    public static void validateEmbargoReason(FacesContext context, UIComponent component, Object value,
+            boolean removeEmbargo) {
+        // Skip validation if removing embargo
+        if (removeEmbargo) {
+            return;
+        }
+        
+        // Get the source of the current request
+        String source = context.getExternalContext().getRequestParameterMap()
+            .get("jakarta.faces.source");
+        
+        // Only validate if the save button triggered this
+        if (source == null || !source.contains("fileEmbargoPopupSaveButton")) {
+            return;
+        }
+        
+        if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) {
+            throw new ValidatorException(
+                new FacesMessage(FacesMessage.SEVERITY_ERROR, 
+                    BundleUtil.getStringFromBundle("embargo.reason.required"), null)
+            );
+        }
+        if (value != null && value.toString().trim().isEmpty()) {
+            throw new ValidatorException(
+                new FacesMessage(FacesMessage.SEVERITY_ERROR, 
+                    BundleUtil.getStringFromBundle("embargo.reason.blank"), null)
+            );
+        }
+    }
 
     public static boolean isRetentionExpired(DataFile df) {
         Retention e = df.getRetention();

src/main/java/propertyFiles/Bundle.properties

@@ -41,6 +41,8 @@ embargoed.wasthrough=Was embargoed until
 embargoed.willbeuntil=Draft: will be embargoed until
 embargo.date.invalid=Date is outside the allowed range: ({0} to {1})
 embargo.date.required=An embargo date is required
+embargo.reason.required=An embargo reason is required
+embargo.reason.blank=An embargo reason cannot be blank
 retention.after=Was retained until
 retention.isfrom=Is retained until
 retention.willbeafter=Draft: will be retained until

src/main/webapp/file-edit-popup-fragment.xhtml

@@ -123,8 +123,9 @@
                                 maxdate="#{settingsWrapper.maxEmbargoDate}"
                                 disabled="#{bean.removeEmbargo}"
                                 validator="#{settingsWrapper.validateEmbargoDate}" >
-                                <p:ajax process="#{updateElements}"
+                                <p:ajax process="@this embargoCheckbox" 
                                     update="#{updateElements}"
+                                    partialSubmit="true"
                                     />
                                 </p:datePicker>
                             <div>
@@ -133,12 +134,23 @@
                             </div>
                         </div>
                         <div class="p-field p-col-12 p-md-4">
-                            <p class="help-block">#{bundle['file.editEmbargoDialog.reason.tip']}</p>
+                            <o:importConstants type="edu.harvard.iq.dataverse.settings.FeatureFlags" />
+                            <ui:param name="embargoReasonRequired" value="#{FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()}"/>
+                            <label class="help-block" for="datasetForm:fileEmbargoAddReason">
+                                #{bundle['file.editEmbargoDialog.reason.tip']} <span
+                                class="glyphicon glyphicon-asterisk text-danger"
+                                jsf:rendered="#{embargoReasonRequired and !bean.removeEmbargo}" />
+                            </label>
                             <p:inputText id="fileEmbargoAddReason" styleClass="form-control"
                                 disabled="#{bean.removeEmbargo}" type="text"
                                 value="#{bean.selectionEmbargo.reason}"
+                                validator="#{bean.validateEmbargoReason}"
                                 placeholder="#{bundle['file.editEmbargoDialog.newReason']}"
                                 onkeypress="if (event.keyCode == 13) { return false;}" />
+                            <div>
+                                <h:message for="fileEmbargoAddReason"
+                                    styleClass="bg-danger text-danger" />
+                            </div>
                         </div>
                     </div>
                 </div>
@@ -153,8 +165,9 @@
                         <p:selectBooleanCheckbox value="#{bean.removeEmbargo}" id="embargoCheckbox"
                             itemLabel="#{bundle['file.editEmbargoDialog.remove']}"
                             disabled="#{facesContext.validationFailed}">
-                            <p:ajax process="#{updateElements}"
+                            <p:ajax process="@this fileEmbargoDate" 
                                 update="#{updateElements}"
+                                partialSubmit="true"
                                 />
                         </p:selectBooleanCheckbox>
                     </div>