IQSS/10137-2 Add flag to remove Return To Author Reason

doc/release-notes/10137-2-add-disable-reason-flag.md

@@ -0,0 +1,6 @@
+## Release Highlights
+
+### Feature flag to remove the required "reason" field in the "Return To Author" dialog 
+
+A reason field, that is required to not be empty, was added in v6.2. Installations that handle author communications through email or another system may prefer to not be required to use this new field. v6.2 includes a new 
+disable-return-to-author-reason feature flag that can be enabled to drop the reason field from the dialog and make sending a reason optional in the api/datasets/{id}/returnToAuthor call.  

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

@@ -2231,7 +2231,7 @@ The people who need to review the dataset (often curators or journal editors) ca
 Return a Dataset to Author
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-After the curators or journal editors have reviewed a dataset that has been submitted for review (see "Submit for Review", above) they can either choose to publish the dataset (see the ``:publish`` "action" above) or return the dataset to its authors. In the web interface there is a "Return to Author" button (see :doc:`/user/dataset-management`), but the interface does not provide a way to explain **why** the dataset is being returned. There is a way to do this outside of this interface, however. Instead of clicking the "Return to Author" button in the UI, a curator can write a "reason for return" into the database via API.
+After the curators or journal editors have reviewed a dataset that has been submitted for review (see "Submit for Review", above) they can either choose to publish the dataset (see the ``:publish`` "action" above) or return the dataset to its authors. In the web interface there is a "Return to Author" button (see :doc:`/user/dataset-management`).  The same operation can be done via this API call.
 
 Here's how curators can send a "reason for return" to the dataset authors. First, the curator creates a JSON file that contains the reason for return:
 
@@ -2254,7 +2254,7 @@ The fully expanded example above (without environment variables) looks like this
   curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST "https://demo.dataverse.org/api/datasets/:persistentId/returnToAuthor?persistentId=doi:10.5072/FK2/J8SJZB" -H "Content-type: application/json" -d @reason-for-return.json
 
 The review process can sometimes resemble a tennis match, with the authors submitting and resubmitting the dataset over and over until the curators are satisfied. Each time the curators send a "reason for return" via API, that reason is sent by email and is persisted into the database, stored at the dataset version level.
-The reason is required, please note that you can still type a creative and meaningful comment such as "The author would like to modify his dataset", "Files are missing", "Nothing to report" or "A curation report with comments and suggestions/instructions will follow in another email" that suits your situation.
+Note the reason is required, unless the `disable-return-to-author-reason` feature flag has been set (see :ref:`feature-flags`). Reason is a free text field and could be as simple as "The author would like to modify his dataset", "Files are missing", "Nothing to report" or "A curation report with comments and suggestions/instructions will follow in another email" that suits your situation.
 
 The :ref:`send-feedback` API call may be useful as a way to move the conversation to email. However, note that these emails go to contacts (versus authors) and there is no database record of the email contents. (:ref:`dataverse.mail.cc-support-on-contact-email` will send a copy of these emails to the support email address which would provide a record.)
 

doc/sphinx-guides/source/installation/config.rst

@@ -3274,6 +3274,9 @@ please find all known feature flags below. Any of these flags can be activated u
     * - add-publicobject-solr-field
       - Adds an extra boolean field `PublicObject_b:true` for public content (published Collections, Datasets and Files). Once reindexed with these fields, we can rely on it to remove a very expensive Solr join on all such documents in Solr queries, significantly improving overall performance (by enabling the feature flag above, `avoid-expensive-solr-join`). These two flags are separate so that an instance can reindex their holdings before enabling the optimization in searches, thus avoiding having their public objects temporarily disappear from search results while the reindexing is in progress. 
       - ``Off``
+    * - disable-return-to-author-reason
+      - Removes the reason field in the `Publish/Return To Author` dialog that was added as a required field in v6.2 and makes the reason an optional parameter in the :ref:`return-a-dataset` API call. 
+      - ``Off``
 
 **Note:** Feature flags can be set via any `supported MicroProfile Config API source`_, e.g. the environment variable
 ``DATAVERSE_FEATURE_XXX`` (e.g. ``DATAVERSE_FEATURE_API_SESSION_AUTH=1``). These environment variables can be set in your shell before starting Payara. If you are using :doc:`Docker for development </container/dev-usage>`, you can set them in the `docker compose <https://docs.docker.com/compose/environment-variables/set-environment-variables/>`_ file.

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

@@ -45,6 +45,7 @@
 import edu.harvard.iq.dataverse.privateurl.PrivateUrl;
 import edu.harvard.iq.dataverse.privateurl.PrivateUrlServiceBean;
 import edu.harvard.iq.dataverse.search.IndexServiceBean;
+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.storageuse.UploadSessionQuotaLimit;
@@ -2478,7 +2479,8 @@ public Response returnToAuthor(@Context ContainerRequestContext crc, @PathParam(
             Dataset dataset = findDatasetOrDie(idSupplied);
             String reasonForReturn = null;
             reasonForReturn = json.getString("reasonForReturn");
-            if (reasonForReturn == null || reasonForReturn.isEmpty()) {
+            if ((reasonForReturn == null || reasonForReturn.isEmpty())
+                    && !FeatureFlags.DISABLE_RETURN_TO_AUTHOR_REASON.enabled()) {
                 return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("dataset.reject.datasetNotInReview"));
             }
             AuthenticatedUser authenticatedUser = getRequestAuthenticatedUserOrDie(crc);

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ReturnDatasetToAuthorCommand.java

@@ -11,6 +11,7 @@
 import edu.harvard.iq.dataverse.engine.command.RequiredPermissions;
 import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
 import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException;
+import edu.harvard.iq.dataverse.settings.FeatureFlags;
 import edu.harvard.iq.dataverse.util.BundleUtil;
 import edu.harvard.iq.dataverse.workflows.WorkflowComment;
 import java.io.IOException;
@@ -26,7 +27,7 @@ public class ReturnDatasetToAuthorCommand extends AbstractDatasetCommand<Dataset
     public ReturnDatasetToAuthorCommand(DataverseRequest aRequest, Dataset anAffectedDvObject, String comment) {
         super(aRequest, anAffectedDvObject);
 
-        if (comment == null || comment.isEmpty()) {
+        if ((comment == null || comment.isEmpty()) && !FeatureFlags.DISABLE_RETURN_TO_AUTHOR_REASON.enabled()) {
             throw new IllegalArgumentException(BundleUtil.getStringFromBundle("dataset.reject.commentNull"));
         }
 

src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java

@@ -312,7 +312,7 @@ public Future<String> indexDataverse(Dataverse dataverse, boolean processPaths)
         String status;
         try {
             if (dataverse.getId() != null) {
-                solrClientService.getSolrClient().add(docs, COMMIT_WITHIN);
+                solrClientService.getSolrClient().add(docs);
             } else {
                 logger.info("WARNING: indexing of a dataverse with no id attempted");
             }
@@ -345,7 +345,6 @@ public void indexDatasetInNewTransaction(Long datasetId) { //Dataset dataset) {
     private static final Map<Long, Boolean> INDEXING_NOW = new ConcurrentHashMap<>();
     // semaphore for async indexing
     private static final Semaphore ASYNC_INDEX_SEMAPHORE = new Semaphore(JvmSettings.MAX_ASYNC_INDEXES.lookupOptional(Integer.class).orElse(4), true);
-    static final int COMMIT_WITHIN = 30000;  //Same as current autoHardIndex time
 
     @Inject
     @Metric(name = "index_permit_wait_time", absolute = true, unit = MetricUnits.NANOSECONDS,
@@ -1562,7 +1561,7 @@ private String addOrUpdateDataset(IndexableDataset indexableDataset, Set<Long> d
         final SolrInputDocuments docs = toSolrDocs(indexableDataset, datafilesInDraftVersion);
 
         try {
-            solrClientService.getSolrClient().add(docs.getDocuments(), COMMIT_WITHIN);
+            solrClientService.getSolrClient().add(docs.getDocuments());
         } catch (SolrServerException | IOException ex) {
             if (ex.getCause() instanceof SolrServerException) {
                 throw new SolrServerException(ex);
@@ -1814,7 +1813,7 @@ private void updatePathForExistingSolrDocs(DvObject object) throws SolrServerExc
 
             sid.removeField(SearchFields.SUBTREE);
             sid.addField(SearchFields.SUBTREE, paths);
-            UpdateResponse addResponse = solrClientService.getSolrClient().add(sid, COMMIT_WITHIN);
+            UpdateResponse addResponse = solrClientService.getSolrClient().add(sid);
             if (object.isInstanceofDataset()) {
                 for (DataFile df : dataset.getFiles()) {
                     solrQuery.setQuery(SearchUtil.constructQuery(SearchFields.ENTITY_ID, df.getId().toString()));
@@ -1827,7 +1826,7 @@ private void updatePathForExistingSolrDocs(DvObject object) throws SolrServerExc
                         }
                         sid.removeField(SearchFields.SUBTREE);
                         sid.addField(SearchFields.SUBTREE, paths);
-                        addResponse = solrClientService.getSolrClient().add(sid, COMMIT_WITHIN);
+                        addResponse = solrClientService.getSolrClient().add(sid);
                     }
                 }
             }
@@ -1869,7 +1868,7 @@ public String delete(Dataverse doomed) {
         logger.fine("deleting Solr document for dataverse " + doomed.getId());
         UpdateResponse updateResponse;
         try {
-            updateResponse = solrClientService.getSolrClient().deleteById(solrDocIdentifierDataverse + doomed.getId(), COMMIT_WITHIN);
+            updateResponse = solrClientService.getSolrClient().deleteById(solrDocIdentifierDataverse + doomed.getId());
         } catch (SolrServerException | IOException ex) {
             return ex.toString();
         }
@@ -1889,7 +1888,7 @@ public String removeSolrDocFromIndex(String doomed) {
         logger.fine("deleting Solr document: " + doomed);
         UpdateResponse updateResponse;
         try {
-            updateResponse = solrClientService.getSolrClient().deleteById(doomed, COMMIT_WITHIN);
+            updateResponse = solrClientService.getSolrClient().deleteById(doomed);
         } catch (SolrServerException | IOException ex) {
             return ex.toString();
         }

src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java

@@ -356,7 +356,7 @@ private void persistToSolr(Collection<SolrInputDocument> docs) throws SolrServer
         /**
          * @todo Do something with these responses from Solr.
          */
-        UpdateResponse addResponse = solrClientService.getSolrClient().add(docs, IndexServiceBean.COMMIT_WITHIN);
+        UpdateResponse addResponse = solrClientService.getSolrClient().add(docs);
     }
 
     public IndexResponse indexPermissionsOnSelfAndChildren(long definitionPointId) {
@@ -496,7 +496,7 @@ public IndexResponse deleteMultipleSolrIds(List<String> solrIdsToDelete) {
             return new IndexResponse("nothing to delete");
         }
         try {
-            solrClientService.getSolrClient().deleteById(solrIdsToDelete, IndexServiceBean.COMMIT_WITHIN);
+            solrClientService.getSolrClient().deleteById(solrIdsToDelete);
         } catch (SolrServerException | IOException ex) {
             /**
              * @todo mark these for re-deletion
@@ -509,7 +509,7 @@ public IndexResponse deleteMultipleSolrIds(List<String> solrIdsToDelete) {
     public JsonObjectBuilder deleteAllFromSolrAndResetIndexTimes() throws SolrServerException, IOException {
         JsonObjectBuilder response = Json.createObjectBuilder();
         logger.info("attempting to delete all Solr documents before a complete re-index");
-        solrClientService.getSolrClient().deleteByQuery("*:*", IndexServiceBean.COMMIT_WITHIN);
+        solrClientService.getSolrClient().deleteByQuery("*:*");
         int numRowsAffected = dvObjectService.clearAllIndexTimes();
         response.add(numRowsClearedByClearAllIndexTimes, numRowsAffected);
         response.add(messageString, "Solr index and database index timestamps cleared.");

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

@@ -58,6 +58,17 @@ public enum FeatureFlags {
      * @since Dataverse 6.3
      */
     ADD_PUBLICOBJECT_SOLR_FIELD("add-publicobject-solr-field"),
+    /**
+     * With this flag enabled, the Return To Author pop-up will not have a required
+     * "Reason" field, and a reason will not be required in the 
+     * /api/datasets/{id}/returnToAuthor api call.
+     * 
+     * @apiNote Raise flag by setting
+     * "dataverse.feature.disable-return-to-author-reason"
+     * @since Dataverse 6.3
+     */
+    DISABLE_RETURN_TO_AUTHOR_REASON("disable-return-to-author-reason"),
+
     ;
 
     final String flag;

src/main/java/propertyFiles/Bundle.properties

@@ -1486,7 +1486,8 @@ dataset.submit.failure.inReview=You cannot submit this dataset for review becaus
 dataset.status.failure.notallowed=Status update failed - label not allowed
 dataset.status.failure.disabled=Status labeling disabled for this dataset
 dataset.status.failure.isReleased=Latest version of dataset is already released. Status can only be set on draft versions
-dataset.rejectMessage=Return this dataset to contributor for modification. The reason for return entered below will be sent by email to the author.
+dataset.rejectMessage=Return this dataset to contributor for modification.
+dataset.rejectMessageReason=The reason for return entered below will be sent by email to the author.
 dataset.rejectMessage.label=Return to Author Reason
 dataset.rejectWatermark=Please enter a reason for returning this dataset to its author(s).
 dataset.reject.enterReason.error=Reason for return to author is required.

src/main/webapp/dataset.xhtml

@@ -1862,19 +1862,23 @@
                         </div>
                     </p:dialog>
                     <p:dialog id="sendBackToContributor" header="#{bundle['dataset.rejectBtn']}" widgetVar="sendBackToContributor" modal="true">
+                    <o:importConstants type="edu.harvard.iq.dataverse.settings.FeatureFlags" />
+                    <ui:param name="disableReasonField" value="#{FeatureFlags.DISABLE_RETURN_TO_AUTHOR_REASON.enabled()}"/>
                         <p class="text-warning">
-                            <span class="glyphicon glyphicon-warning-sign"/> #{bundle['dataset.rejectMessage']}
+                            <span class="glyphicon glyphicon-warning-sign"/> #{bundle['dataset.rejectMessage']} #{disableReasonField ? '':bundle['dataset.rejectMessageReason']}
                         </p>
-
-                        <p:inputTextarea id="returnReason" rows="4" value="#{DatasetPage.returnReason}" title="#{bundle['dataset.rejectMessage.label']}" maxlength="2000" widgetVar="returnReason"
-                                         cols="70" counter="display" counterTemplate="{0} characters remaining." autoResize="false"
-                                         required="#{param['DO_RETURN_TO_AUTHOR_VALIDATION']}"
-                                         requiredMessage="#{bundle['dataset.reject.enterReason.error']}"/>
-                        <p>
-                            <h:outputText id="display"/>
-                        </p>
-                        <p:watermark for="returnReason" value="#{bundle['dataset.rejectWatermark']}" id="returnReasonWatermark"/>
-                        <h:message for="returnReason" styleClass="bg-danger text-danger"/>
+
+                        <ui:fragment  rendered="#{!disableReasonField}">
+                            <p:inputTextarea id="returnReason" rows="4" value="#{DatasetPage.returnReason}" title="#{bundle['dataset.rejectMessage.label']}" maxlength="2000" widgetVar="returnReason"
+                                             cols="70" counter="display" counterTemplate="{0} characters remaining." autoResize="false"
+                                             required="#{param['DO_RETURN_TO_AUTHOR_VALIDATION']}"
+                                             requiredMessage="#{bundle['dataset.reject.enterReason.error']}"/>
+                            <p>
+                                <h:outputText id="display"/>
+                            </p>
+                            <p:watermark for="returnReason" value="#{bundle['dataset.rejectWatermark']}" id="returnReasonWatermark"/>
+                            <h:message for="returnReason" styleClass="bg-danger text-danger"/>
+                        </ui:fragment>
 
                         <div class="button-block">
                             <p:commandButton styleClass="btn btn-default" value="#{bundle.continue}"

src/test/java/edu/harvard/iq/dataverse/engine/command/impl/ReturnDatasetToAuthorCommandTest.java

@@ -21,6 +21,9 @@
 import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
 import edu.harvard.iq.dataverse.mocks.MocksFactory;
 import edu.harvard.iq.dataverse.search.IndexServiceBean;
+import edu.harvard.iq.dataverse.settings.JvmSettings;
+import edu.harvard.iq.dataverse.util.testing.JvmSetting;
+import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings;
 import edu.harvard.iq.dataverse.workflows.WorkflowComment;
 import java.util.Collections;
 import java.util.List;
@@ -29,10 +32,10 @@
 import jakarta.servlet.http.HttpServletRequest;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.*;
-import static org.junit.jupiter.api.Assertions.*;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+@LocalJvmSettings
 public class ReturnDatasetToAuthorCommandTest {
 
     private Dataset dataset;
@@ -142,6 +145,7 @@ public List<AuthenticatedUser> getUsersWithPermissionOn(Permission permission, D
         }
      */
     @Test
+    @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "false", varArgs = "disable-return-to-author-reason")
     void testDatasetNull() {
         assertThrows(IllegalArgumentException.class,
             () -> new ReturnDatasetToAuthorCommand(dataverseRequest, null, ""));
@@ -179,6 +183,7 @@ public void testNotInReviewDataset() {
     }
 
     @Test
+    @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "false", varArgs = "disable-return-to-author-reason")
     public void testEmptyOrNullComment(){
         dataset.getLatestVersion().setVersionState(DatasetVersion.VersionState.DRAFT);
         Dataset updatedDataset = null;
@@ -198,6 +203,27 @@ public void testEmptyOrNullComment(){
         }
         assertEquals(expected, actual);
     }
+
+    /** Test the disable reason flag
+     * @throws Exception when the test is in error.
+     */
+    @Test
+    @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "disable-return-to-author-reason")
+    public void testEmptyOrNullCommentWhenDisabled() throws Exception {
+        dataset.getLatestVersion().setVersionState(DatasetVersion.VersionState.DRAFT);
+        Dataset updatedDataset = null;
+
+        testEngine.submit(new AddLockCommand(dataverseRequest, dataset,
+                new DatasetLock(DatasetLock.Reason.InReview, dataverseRequest.getAuthenticatedUser())));
+
+        updatedDataset = testEngine.submit(new ReturnDatasetToAuthorCommand(dataverseRequest, dataset, null));
+        assertNotNull(updatedDataset);
+        testEngine.submit(new AddLockCommand(dataverseRequest, dataset,
+                new DatasetLock(DatasetLock.Reason.InReview, dataverseRequest.getAuthenticatedUser())));
+        updatedDataset = testEngine.submit(new ReturnDatasetToAuthorCommand(dataverseRequest, dataset, ""));
+        assertNotNull(updatedDataset);
+    }
+
 
    @Test
     public void testAllGood() {