DT-2957: Update the API call for getting DAR details for a dataset ID by rushtong · Pull Request #2822 · DataBiosphere/consent

rushtong · 2026-02-24T15:06:03Z

Addresses

https://broadworkbench.atlassian.net/browse/DT-2957

Summary

Significant refactor of the current GET /metrics/dataset/{datasetId} API. We're deprecating that unauthenticated API in favor of an authenticated one (GET /api/metrics/dar-summaries/{datasetId}) that will return the same essential data in addition to an expired state so the UI can distinguish them in the Dataset Statistics page.

This PR is required for the front end changes here: DataBiosphere/duos-ui#3343

There will be a follow-on PR that removes deprecated code right after these are merged.

Have you read CONTRIBUTING.md lately? If not, do that first.

Label PR with a Jira ticket number and include a link to the ticket
Label PR with a security risk modifier [no, low, medium, high]
PR describes scope of changes
Get a minimum of one thumbs worth of review, preferably two if enough team members are available
Get PO sign-off for all non-trivial UI or workflow changes
Verify all tests go green
Test this change deployed correctly and works on dev environment after deployment

rushtong · 2026-02-24T18:39:10Z

src/main/java/org/broadinstitute/consent/http/service/MetricsService.java

-    this.electionDAO = electionDAO;
-  }
-
-  public static class DarMetricsSummary {


Extracted to the models package.

rushtong · 2026-02-24T18:40:19Z

src/main/java/org/broadinstitute/consent/http/resources/MetricsResource.java

+   * @param datasetId the id of the dataset for which to generate metrics
+   * @return Response containing DatasetMetrics for the given datasetId
+   */
+  @Deprecated(forRemoval = true, since = "2026-02-23")


This will be removed when the front end is updated to use the GET /api/metrics/dar-summaries/{datasetId} API

src/main/resources/assets/schemas/DatasetMetrics.yaml

rushtong · 2026-02-24T20:01:28Z

src/main/java/org/broadinstitute/consent/http/db/DataAccessRequestDAO.java

  void deleteByReferenceId(@Bind("referenceId") String referenceId);

-  @SqlUpdate("DELETE FROM data_access_request WHERE reference_id IN (<referenceIds>)")
-  void deleteByReferenceIds(


Drive-by fix: unused

rushtong · 2026-02-24T20:01:35Z

src/main/java/org/broadinstitute/consent/http/db/DataAccessRequestDAO.java

-   * @param referenceIds List<String>
-   */
-  @SqlUpdate("DELETE FROM dar_dataset WHERE reference_id in (<referenceIds>)")
-  void deleteDARDatasetRelationByReferenceIds(


Drive-by fix: unused

src/main/java/org/broadinstitute/consent/http/db/DataAccessRequestDAO.java

src/main/java/org/broadinstitute/consent/http/models/DarMetricsSummary.java

Copilot

Pull request overview

This PR updates the dataset metrics backend to support a new authenticated endpoint for DAR summary metrics (including an “expired” flag), while deprecating the prior unauthenticated dataset metrics route.

Changes:

Refactors MetricsService to return DAR summaries via a new DAO query that includes expired DARs.
Adds GET /api/metrics/dar-summaries/{datasetId} and deprecates GET /metrics/dataset/{datasetId}.
Introduces a DarMetricsSummary model + tests, and updates OpenAPI docs/schemas accordingly.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/main/java/org/broadinstitute/consent/http/service/MetricsService.java	Reworks metrics generation to return DAR summaries (includes expired)
src/main/java/org/broadinstitute/consent/http/resources/MetricsResource.java	Adds new `/api/metrics/dar-summaries/{datasetId}` endpoint; deprecates old one
src/main/java/org/broadinstitute/consent/http/db/DataAccessRequestDAO.java	Adds summary-metrics query that includes expired DARs and returns `dar_code`
src/main/java/org/broadinstitute/consent/http/models/DarMetricsSummary.java	New API model for DAR summary including `expired`
src/main/java/org/broadinstitute/consent/http/models/DatasetMetrics.java	Simplifies metrics payload to only include DAR summaries
src/main/resources/assets/api-docs.yaml	Registers new metrics path and factors deprecated path into a separate file
src/main/resources/assets/paths/darSummariesByDatasetId.yaml	New OpenAPI path for DAR summaries
src/main/resources/assets/paths/metricsDatasetById.yaml	New OpenAPI path for deprecated dataset metrics route
src/main/resources/assets/schemas/DarMetric.yaml	Adds `expired` to DAR metric schema
src/main/resources/assets/schemas/DatasetMetrics.yaml	Removes `dataset` and `elections` from the schema
src/main/java/org/broadinstitute/consent/http/db/ElectionDAO.java	Removes unused `findLastElectionsByReferenceIdsAndType`
src/main/java/org/broadinstitute/consent/http/ConsentModule.java	Updates DI wiring for simplified `MetricsService` constructor
src/test/java/org/broadinstitute/consent/http/service/MetricsServiceTest.java	Updates tests for refactored service / new DAO method
src/test/java/org/broadinstitute/consent/http/resources/MetricsResourceTest.java	Adds coverage for new DAR summaries resource method
src/test/java/org/broadinstitute/consent/http/models/DarMetricsSummaryTest.java	New unit tests for `DarMetricsSummary` behavior
src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java	Adds test coverage for new “includes expired” DAO query
src/test/java/org/broadinstitute/consent/http/db/ElectionDAOTest.java	Minor modernizations (`getFirst`) and removes obsolete test

Comments suppressed due to low confidence (2)

src/main/resources/assets/paths/darSummariesByDatasetId.yaml:16

Response 200 description says "Dataset Metrics" but this endpoint returns a list of DAR summaries, not dataset metrics. Update the response description to something like "DAR Summary Metrics" / "DAR summaries" to match the endpoint semantics.

    200:
      description: Dataset Metrics
      content:

src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java:887

This call creates an extra OPEN DataAccess election for expiredReferenceId but never uses its ID and it has no votes associated, so it doesn't affect the query and is confusing in the test setup. Consider removing it (or add a comment explaining why a second election is needed).

    createDataAccessElection(expiredReferenceId, dataset.getDatasetId());

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T15:28:28Z

src/main/java/org/broadinstitute/consent/http/models/DarMetricsSummary.java

+    // If the DAR or its submission date is null, we consider it expired for metrics purposes
+    if (dar == null || dar.getSubmissionDate() == null) {
+      return true;
+    }
+    LocalDateTime oneYearAgo = LocalDateTime.now().minusYears(1);
+    ZonedDateTime zonedDateTime = oneYearAgo.atZone(ZoneId.systemDefault());
+    Timestamp lastYear = Timestamp.from(zonedDateTime.toInstant());
+    return dar.getSubmissionDate().before(lastYear);


computeExpired uses a different expiration definition than DataAccessRequest (which sets expired using EXPIRATION_DURATION_MILLIS / 365 days when mapping submission_date). Using LocalDateTime.now().minusYears(1) can diverge around leap years and makes the summary potentially inconsistent with dar.getExpired(). Consider deriving this field from dar.getExpired() (and only special-case null DARs if needed) to keep expiration logic consistent across the API.

Suggested change

// If the DAR or its submission date is null, we consider it expired for metrics purposes

if (dar == null || dar.getSubmissionDate() == null) {

return true;

}

LocalDateTime oneYearAgo = LocalDateTime.now().minusYears(1);

ZonedDateTime zonedDateTime = oneYearAgo.atZone(ZoneId.systemDefault());

Timestamp lastYear = Timestamp.from(zonedDateTime.toInstant());

return dar.getSubmissionDate().before(lastYear);

// If the DAR is null, we consider it expired for metrics purposes.

// Otherwise, rely on the expiration logic defined in DataAccessRequest.

if (dar == null) {

return true;

}

return dar.getExpired();

Why not this?

In short, we are doing this but with one additional check to ensure that submission date isn't also null.

src/main/resources/assets/schemas/DarMetric.yaml

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

sonarqubecloud · 2026-02-25T18:02:12Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

otchet-broad

Still not sure why we're not just using what Copilot suggested. I think that matches what I was intending with my comment in the same area of code.

otchet-broad · 2026-02-26T15:12:41Z

src/main/java/org/broadinstitute/consent/http/models/DarMetricsSummary.java

+    // If the DAR or its submission date is null, we consider it expired for metrics purposes
+    if (dar == null || dar.getSubmissionDate() == null) {
+      return true;
+    }
+    LocalDateTime oneYearAgo = LocalDateTime.now().minusYears(1);
+    ZonedDateTime zonedDateTime = oneYearAgo.atZone(ZoneId.systemDefault());
+    Timestamp lastYear = Timestamp.from(zonedDateTime.toInstant());
+    return dar.getSubmissionDate().before(lastYear);


Why not this?

otchet-broad

👍

rushtong added 4 commits February 23, 2026 14:21

feat: refactoring to remove unused objects (dataset/elections)

2c40a8c

feat: deprecate the old; replace with new API

c425a4f

feat: refactoring, expired state, new query

309976c

Merge branch 'develop' into gr-DT-2957-dataset-dar-details

8067534

rushtong changed the title ~~Gr dt 2957 dataset dar details~~ DT-2957: Update the API call for getting DAR details for a dataset ID Feb 24, 2026

rushtong added 7 commits February 24, 2026 10:51

fixes

61bc0ee

tests and some logic fixes

c9b4531

drive-by cleanup

1687f4b

deprecation note in swagger doc

ac7bd25

sonar warning

5e04126

rm commented code

bde9e91

sonar warning

eeb65b4

rushtong commented Feb 24, 2026

View reviewed changes

src/main/resources/assets/schemas/DatasetMetrics.yaml Show resolved Hide resolved

rushtong added 6 commits February 24, 2026 13:49

test simplification

f94eee2

test simplification

50aff79

suppress unused - we always need a user for @Auth-ed methods

ced1193

address sonar warnings

9d72fed

dar metric swagger update

b35ebc9

test updates

efa62d0

rushtong commented Feb 24, 2026

View reviewed changes

add operation ids

3e6e83f

rushtong mentioned this pull request Feb 24, 2026

DT-2957: Update Dataset Statistics page to use new API and clearly show expiration DataBiosphere/duos-ui#3343

Merged

rushtong marked this pull request as ready for review February 24, 2026 22:26

rushtong requested a review from a team as a code owner February 24, 2026 22:26

rushtong requested review from Copilot and removed request for a team and Copilot February 24, 2026 22:26

rushtong requested review from kevinmarete and otchet-broad February 24, 2026 22:26

otchet-broad reviewed Feb 25, 2026

View reviewed changes

src/main/java/org/broadinstitute/consent/http/db/DataAccessRequestDAO.java Outdated Show resolved Hide resolved

otchet-broad reviewed Feb 25, 2026

View reviewed changes

src/main/java/org/broadinstitute/consent/http/db/DataAccessRequestDAO.java Outdated Show resolved Hide resolved

otchet-broad reviewed Feb 25, 2026

View reviewed changes

src/main/java/org/broadinstitute/consent/http/models/DarMetricsSummary.java Outdated Show resolved Hide resolved

PR feedback

3e394f6

Copilot AI review requested due to automatic review settings February 25, 2026 15:21

Copilot started reviewing on behalf of rushtong February 25, 2026 15:21 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

rushtong and others added 4 commits February 25, 2026 10:33

PR feedback

5b2b966

Update src/main/resources/assets/schemas/DarMetric.yaml

a87c6f1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Include closeouts in results.

a6ccb67

spotless

a5b174b

rushtong requested a review from otchet-broad February 25, 2026 19:36

kevinmarete approved these changes Feb 26, 2026

View reviewed changes

otchet-broad reviewed Feb 26, 2026

View reviewed changes

otchet-broad approved these changes Feb 26, 2026

View reviewed changes

rushtong merged commit 35a03bd into develop Feb 26, 2026
14 checks passed

rushtong deleted the gr-DT-2957-dataset-dar-details branch February 26, 2026 18:05

rushtong mentioned this pull request Feb 26, 2026

DT-2957: Removed unused API #2824

Merged

Conversation

rushtong commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Addresses

Summary

Uh oh!

rushtong Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

rushtong Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rushtong Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

rushtong Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

otchet-broad Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

rushtong Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 25, 2026

Quality Gate passed

Uh oh!

otchet-broad left a comment

Choose a reason for hiding this comment

Uh oh!

otchet-broad Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

otchet-broad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rushtong commented Feb 24, 2026 •

edited

Loading