Merge branch 'develop' into 11447-mydata-retrieve-empty-result-set

Author: stevenwinship

doc/release-notes/8013-history-of-access-request-available-via-api.md

@@ -0,0 +1,8 @@
+### Feature: Extend List File Access Requests API ###
+
+Added ability to get access request history via the `/datafile/{id}/listRequests` API. The API returns a list of users/groups where the request for access is waiting for an accept or reject. Already accepted or rejected requests are not returned.
+
+By adding the flag 'includeHistory=true' all of the requests will be returned. Pagination is also implemented in this feature. Adding a start page parameter and max list size (`&start=0` and `&per_page=20`) can limit the amount of data being returned.
+
+See https://guides.dataverse.org/en/latest/api/dataaccess.html#list-file-access-requests
+

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

@@ -12,7 +12,7 @@ v6.11
 
 - The GET /api/mydata/retrieve, if the search returns no data, now includes the "data" block with 0 results. The message that was returned in "error_message" will be returned in "message" and the "success" will be `true`. All other errors will continue to reply with "success":false and the error message in "error_message".
 
-- The Croissant :ref:`metadata export format <metadata-export-formats>` has been updated from version 1.0 to 1.1, which is reflected in the ``conformsTo`` property. ``@vocab`` and ``sc`` properties now use "http" as `recommended <https://github.com/mlcommons/croissant/pull/929#pullrequestreview-3079137662>`_. The unused ``wd`` property has been dropped.
+- The Croissant :ref:`metadata export format <metadata-export-formats>` has been updated from version 1.0 to 1.1, which is reflected in the ``conformsTo`` property. The unused ``wd`` property has been dropped.
 
 v6.10
 -----

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

@@ -440,7 +440,22 @@ This method returns a list of Authenticated Users who have requested access to t
 
 A curl example using an ``id``::
 
-    curl -H "X-Dataverse-key:$API_TOKEN" -X GET http://$SERVER/api/access/datafile/{id}/listRequests
+    curl -H "X-Dataverse-key:$API_TOKEN" -X GET $SERVER/api/access/datafile/{id}/listRequests
+
+Query parameters have been added to retrieve the historical list of "created", "granted", and "rejected" requests:
+
+* `includeHistory` When `true` this will force the return of all requests and not just the "created" ones.
+* `start` For pagination, use this to request a specific page.
+* `per_page` For pagination, use this to limit the number of items in each paged list.
+
+.. note:: Pagination is only available when `includeHistory` is `true`
+
+If requesting a page beyond the last page this API will return a 404 "There are no access requests for this file:..."
+If requesting a page before page 1 or requesting the number of items to be 0 or less this API will ignore these parameters and return the entire list.
+
+A curl example using an ``id``::
+
+    curl -H "X-Dataverse-key:$API_TOKEN" -X GET "$SERVER/api/access/datafile/{id}/listRequests?includeHistory=true&start=1&per_page=20"
 
 User Has Requested Access to a File:
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -451,7 +466,7 @@ This method returns true or false depending on whether or not the calling user h
 
 A curl example using an ``id``::
 
-    curl -H "X-Dataverse-key:$API_TOKEN" -X GET "http://$SERVER/api/access/datafile/{id}/userFileAccessRequested"
+    curl -H "X-Dataverse-key:$API_TOKEN" -X GET "$SERVER/api/access/datafile/{id}/userFileAccessRequested"
 
 
 Get User Permissions on a File:

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

@@ -247,11 +247,31 @@ public String toUri() {
     inverseJoinColumns = @JoinColumn(name = "authenticated_user_id"))
     private List<AuthenticatedUser> fileAccessRequesters;
 
-    
-    public List<FileAccessRequest> getFileAccessRequests(){
-        return fileAccessRequests;
+    public List<FileAccessRequest> getFileAccessRequests() {
+        return getFileAccessRequests(0, 0);
     }
-    
+
+    /**
+     * Get Requests with pagination option
+     * @param numResultsPerPageRequested
+     * @param paginationStart starts at 1
+     * @return
+     */
+    public List<FileAccessRequest> getFileAccessRequests(int numResultsPerPageRequested, int paginationStart) {
+        if (numResultsPerPageRequested < 1 || paginationStart < 1) {
+            return fileAccessRequests;
+        } else {
+            int startIndex = (paginationStart - 1) * numResultsPerPageRequested;
+            int endIndex = startIndex + numResultsPerPageRequested;
+            if (startIndex >= fileAccessRequests.size()) {
+                return List.of();
+            } else if (endIndex > fileAccessRequests.size()) {
+                endIndex = fileAccessRequests.size();
+            }
+            return fileAccessRequests.subList(startIndex, endIndex);
+        }
+    }
+
     public List<FileAccessRequest> getFileAccessRequests(FileAccessRequest.RequestState state){
         return fileAccessRequests.stream().filter(far -> far.getState() == state).collect(Collectors.toList());
     }
@@ -849,6 +869,15 @@ public void addFileAccessRequest(FileAccessRequest request) {
         this.fileAccessRequests.add(request);
     }
 
+    public List<FileAccessRequest> getAccessRequestsForAssignee(RoleAssignee roleAssignee) {
+        if (this.fileAccessRequests == null) {
+            return null;
+        }
+
+        return this.fileAccessRequests.stream()
+                .filter(fileAccessRequest -> fileAccessRequest.getRequester().equals(roleAssignee)).toList();
+    }
+
     public FileAccessRequest getAccessRequestForAssignee(RoleAssignee roleAssignee) {
         if (this.fileAccessRequests == null) {
             return null;

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

@@ -59,9 +59,25 @@ public boolean isValid(DatasetField value, ConstraintValidatorContext context) {
         }
 
         // if value is not primitive or not empty
-        if (!dsfType.isPrimitive() || !StringUtils.isBlank(value.getValue())) {
+        // For controlled vocabulary fields, check that actual CV values are selected,
+        // not just that datasetFieldValues contains something (which might be an invalid N/A placeholder)
+        // See https://github.com/IQSS/dataverse/issues/11900
+        if (!dsfType.isPrimitive()) {
             return true;
         }
+        
+        if (dsfType.isControlledVocabulary()) {
+            // For CV fields, check if there are actual controlled vocabulary values selected
+            if (value.getControlledVocabularyValues() != null && !value.getControlledVocabularyValues().isEmpty()) {
+                return true;
+            }
+            // If no CV values, fall through to required field check below
+        } else {
+            // For non-CV primitive fields, check if value is not blank
+            if (!StringUtils.isBlank(value.getValue())) {
+                return true;
+            }
+        }
 
         if (value.isRequired()) { 
             String errorMessage = null;

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

@@ -4039,8 +4039,8 @@ public String save() {
             dataset.setOwner(ownerId != null ? dataverseService.find(ownerId) : null);
         }
         // Validate
-        Set<ConstraintViolation> constraintViolations = workingVersion.validate();
-        if (!constraintViolations.isEmpty()) {
+        workingVersion.validate(); // add validation messages to dataset fields
+        if (!workingVersion.isValid()) {
             FacesContext.getCurrentInstance().validationFailed();
             return "";
         }

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

@@ -4,6 +4,7 @@
 import java.util.Date;
 
 import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
+import edu.harvard.iq.dataverse.util.BundleUtil;
 import jakarta.persistence.Column;
 import jakarta.persistence.Entity;
 import jakarta.persistence.EnumType;
@@ -143,6 +144,20 @@ public String getStateLabel() {
         }
         return null; 
     }
+
+    // For use by UI to allow for internationalization
+    public String getStateLabelNationalized() {
+        if(isStateCreated()) {
+            return BundleUtil.getStringFromBundle("permission.fileAccess.created");
+        }
+        if(isStateGranted()) {
+            return BundleUtil.getStringFromBundle("permission.fileAccess.granted");
+        }
+        if(isStateRejected()) {
+            return BundleUtil.getStringFromBundle("permission.fileAccess.rejected");
+        }
+        return null;
+    }
 
     public void setStateCreated() {
         this.requestState = RequestState.CREATED;
@@ -197,4 +212,4 @@ public boolean equals(Object object) {
     }
 
 
-}
+}

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

@@ -116,6 +116,15 @@ public boolean isShowDeleted() {
     public void setShowDeleted(boolean showDeleted) {
         this.showDeleted = showDeleted;
     }
+    private boolean showHistory = false;
+
+    public boolean isShowHistory() {
+        return showHistory;
+    }
+
+    public void setShowHistory(boolean showHistory) {
+        this.showHistory = showHistory;
+    }
 
     public Dataset getDataset() {
         return dataset;
@@ -143,6 +152,13 @@ public void showDeletedCheckboxChange() {
         }
 
     }
+    private boolean backingShowHistory = false;
+    public void showHistoryCheckboxChange() {
+        if (backingShowHistory != showHistory) {
+            initMaps();
+            backingShowHistory = showHistory;
+        }
+    }
 
     public String init() {
         if (dataset.getId() != null) {
@@ -199,7 +215,7 @@ private void initMaps() {
                 fileMap.put(file, raList);
 
                 // populate the file access requests map
-                for (FileAccessRequest fileAccessRequest : file.getFileAccessRequests(FileAccessRequest.RequestState.CREATED)) {
+                for (FileAccessRequest fileAccessRequest : !showHistory ? file.getFileAccessRequests(FileAccessRequest.RequestState.CREATED) : file.getFileAccessRequests()) {
                     List<FileAccessRequest> fileAccessRequestList = fileAccessRequestMap.get(fileAccessRequest.getRequester());
                     if (fileAccessRequestList == null) {
                         fileAccessRequestList = new ArrayList<>();
@@ -250,6 +266,21 @@ public String formatAccessRequestTimestamp(List<FileAccessRequest> fileAccessReq
         return Util.getDateTimeFormat().format(date);
     }
 
+    public String getAccessRequestStates(List<FileAccessRequest> fileAccessRequests) {
+        String result = "";
+        if (fileAccessRequests != null) {
+            Map<String, Long> items = fileAccessRequests.stream()
+                    .sorted(Comparator.comparing(FileAccessRequest::getState))
+                    .collect(Collectors.groupingBy(
+                            FileAccessRequest::getStateLabelNationalized,
+                            Collectors.counting()));
+
+            result = items.entrySet().stream().map(entry -> entry.getKey() + ":" + entry.getValue())
+                    .collect(Collectors.joining(", ", "[ ", " ]"));
+        }
+        return result;
+    }
+
     private void addFileToRoleAssignee(RoleAssignment assignment, boolean fileDeleted) {
         RoleAssignee ra = roleAssigneeService.getRoleAssignee(assignment.getAssigneeIdentifier());
         List<RoleAssignmentRow> assignments = roleAssigneeMap.get(ra);

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

@@ -28,6 +28,7 @@
 import edu.harvard.iq.dataverse.export.DDIExportServiceBean;
 import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean;
 import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean.MakeDataCountEntry;
+import edu.harvard.iq.dataverse.mydata.Pager;
 import edu.harvard.iq.dataverse.settings.JvmSettings;
 import edu.harvard.iq.dataverse.settings.SettingsServiceBean;
 import edu.harvard.iq.dataverse.util.*;
@@ -1695,8 +1696,11 @@ public Response requestFileAccess(@Context ContainerRequestContext crc
     @GET
     @AuthRequired
     @Path("/datafile/{id}/listRequests")
-    public Response listFileAccessRequests(@Context ContainerRequestContext crc, @PathParam("id") String fileToRequestAccessId, @Context HttpHeaders headers) {
-
+    public Response listFileAccessRequests(@Context ContainerRequestContext crc, @PathParam("id") String fileToRequestAccessId,
+                                           @QueryParam("includeHistory") boolean includeHistory,
+                                           @QueryParam("per_page") final int numResultsPerPageRequested,
+                                           @QueryParam("start") final int paginationStart,
+                                           @Context HttpHeaders headers) {
         DataverseRequest dataverseRequest;
 
         DataFile dataFile;
@@ -1717,7 +1721,8 @@ public Response listFileAccessRequests(@Context ContainerRequestContext crc, @Pa
             return error(FORBIDDEN, BundleUtil.getStringFromBundle("access.api.rejectAccess.failure.noPermissions"));
         }
 
-        List<FileAccessRequest> requests = dataFile.getFileAccessRequests(FileAccessRequest.RequestState.CREATED);
+        List<FileAccessRequest> requests = !includeHistory ? dataFile.getFileAccessRequests(FileAccessRequest.RequestState.CREATED) :
+                dataFile.getFileAccessRequests(numResultsPerPageRequested, paginationStart);
 
         if (requests == null || requests.isEmpty()) {
             List<String> args = Arrays.asList(dataFile.getDisplayName());
@@ -1727,7 +1732,21 @@ public Response listFileAccessRequests(@Context ContainerRequestContext crc, @Pa
         JsonArrayBuilder userArray = Json.createArrayBuilder();
 
         for (FileAccessRequest fileAccessRequest : requests) {
-            userArray.add(json(fileAccessRequest.getRequester()));
+            userArray.add(json(fileAccessRequest));
+        }
+
+        // Check for pagination request
+        if (includeHistory && numResultsPerPageRequested > 0 && paginationStart > 0) {
+            JsonObjectBuilder builder = Json.createObjectBuilder()
+                    .add("status", ApiConstants.STATUS_OK)
+                    .add("data", userArray);
+
+            Pager pager = new Pager(dataFile.getFileAccessRequests().size(), numResultsPerPageRequested, paginationStart);
+            builder.add("pagination", pager.asJsonObjectBuilder());
+
+            return Response.ok( builder.build() )
+                    .type(MediaType.APPLICATION_JSON)
+                    .build();
         }
 
         return ok(userArray);

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

@@ -110,10 +110,14 @@ protected void validateOrDie(DatasetVersion dsv, Boolean lenient) throws Command
         Set<ConstraintViolation> constraintViolations = dsv.validate();
         if (!constraintViolations.isEmpty()) {
             if (lenient) {
-                // populate invalid fields with N/A
+                // populate invalid primitive fields with N/A
+                // Note: controlled vocabulary fields should NOT get N/A values in datasetfieldvalue,
+                // as this creates an inconsistent state where the CV field appears valid but is empty.
+                // See https://github.com/IQSS/dataverse/issues/11900
                 constraintViolations.stream()
                     .filter(cv -> cv.getRootBean() instanceof DatasetField)
                     .map(cv -> ((DatasetField) cv.getRootBean()))
+                    .filter(f -> !f.getDatasetFieldType().isControlledVocabulary())
                     .forEach(f -> f.setSingleValue(DatasetField.NA_VALUE));
 
             } else {