From 7f96bb0c028aa2a696f6f8145c6b40474ef1d9be Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 12 Mar 2026 17:29:01 -0400 Subject: [PATCH] Set embargoedUntil to actual unembargo timestamp upon status transition Fixes #1286 When a dandiset transitions from EmbargoedAccess to OpenAccess, record the actual unembargo timestamp in the embargoedUntil field, replacing the original embargo end date. The logic is in Version._populate_access_metadata() which detects the status transition and sets the timestamp at that point. See https://github.com/dandi/dandi-schema/pull/143 for field semantics. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 --- dandiapi/api/models/version.py | 20 +++++++++++++++++--- dandiapi/api/tests/test_unembargo.py | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/dandiapi/api/models/version.py b/dandiapi/api/models/version.py index 9363b8aa3..2fd5b1cbb 100644 --- a/dandiapi/api/models/version.py +++ b/dandiapi/api/models/version.py @@ -11,6 +11,7 @@ from django.core.validators import RegexValidator from django.db import models from django.db.models.query_utils import Q +from django.utils import timezone from django_extensions.db.models import TimeStampedModel from dandiapi.api.asset_paths import get_root_paths @@ -216,13 +217,26 @@ def _populate_access_metadata(self): # Ensure that every item in access is a dict access = [x for x in access if isinstance(x, dict)] or default_access + new_status = ( + AccessType.EmbargoedAccess.value + if self.dandiset.embargoed + else AccessType.OpenAccess.value + ) + + # When transitioning from embargoed to open access, record the actual + # unembargo timestamp in embargoedUntil. + # See https://github.com/dandi/dandi-schema/pull/143 for the field semantics. + if ( + access[0].get('status') == AccessType.EmbargoedAccess.value + and new_status == AccessType.OpenAccess.value + ): + access[0]['embargoedUntil'] = timezone.now().isoformat() + # Set first access item access[0] = { **access[0], 'schemaKey': 'AccessRequirements', - 'status': AccessType.EmbargoedAccess.value - if self.dandiset.embargoed - else AccessType.OpenAccess.value, + 'status': new_status, } return access diff --git a/dandiapi/api/tests/test_unembargo.py b/dandiapi/api/tests/test_unembargo.py index f49cb084a..0606e962b 100644 --- a/dandiapi/api/tests/test_unembargo.py +++ b/dandiapi/api/tests/test_unembargo.py @@ -1,9 +1,11 @@ from __future__ import annotations +import datetime from typing import TYPE_CHECKING import dandischema from django.core.files.storage import default_storage +from django.utils import timezone import pytest from dandiapi.api.manifests import all_manifest_filepaths @@ -255,10 +257,22 @@ def test_unembargo_dandiset( write_manifest_files(draft_version.id) + # Seed access metadata to match what would exist in reality: the version was + # created while EMBARGOED (with EmbargoedAccess status and a future embargoedUntil), + # then kickoff changed the dandiset to UNEMBARGOING without saving the version. + original_embargoed_until = '2099-01-01T00:00:00+00:00' + draft_version.metadata['access'][0]['status'] = ( + dandischema.models.AccessType.EmbargoedAccess.value + ) + draft_version.metadata['access'][0]['embargoedUntil'] = original_embargoed_until + draft_version.save() + assert all(asset.is_embargoed for asset in draft_version.assets.all()) assert all(asset.status == Asset.Status.VALID for asset in draft_version.assets.all()) + before_unembargo = timezone.now() unembargo_dandiset(dandiset, owners[0]) + after_unembargo = timezone.now() for zarr_file in zarr_files: zarr_file_s3_path = zarr_archive.s3_path(str(zarr_file.path)) @@ -277,6 +291,12 @@ def test_unembargo_dandiset( == dandischema.models.AccessType.OpenAccess.value ) + # Verify embargoedUntil was replaced with the actual unembargo timestamp + embargoed_until = draft_version.metadata['access'][0]['embargoedUntil'] + assert embargoed_until != original_embargoed_until + embargoed_until_dt = datetime.datetime.fromisoformat(embargoed_until) + assert before_unembargo <= embargoed_until_dt <= after_unembargo + # Check that a correct email exists assert mailoutbox assert 'has been unembargoed' in mailoutbox[0].subject