Connect unembargo endpoint and task

dandiapi/api/mail.py

@@ -182,31 +182,6 @@ def send_pending_users_message(users: Iterable[User]):
         connection.send_messages(messages)
 
 
-def build_dandisets_to_unembargo_message(dandisets: Iterable[Dandiset]):
-    dandiset_context = [
-        {
-            'identifier': ds.identifier,
-            'owners': [user.username for user in ds.owners],
-            'asset_count': ds.draft_version.asset_count,
-            'size': ds.draft_version.size,
-        }
-        for ds in dandisets
-    ]
-    render_context = {**BASE_RENDER_CONTEXT, 'dandisets': dandiset_context}
-    return build_message(
-        subject='DANDI: New Dandisets to unembargo',
-        message=render_to_string('api/mail/dandisets_to_unembargo.txt', render_context),
-        to=[settings.DANDI_DEV_EMAIL],
-    )
-
-
-def send_dandisets_to_unembargo_message(dandisets: Iterable[Dandiset]):
-    logger.info('Sending dandisets to unembargo message to devs at %s', settings.DANDI_DEV_EMAIL)
-    messages = [build_dandisets_to_unembargo_message(dandisets)]
-    with mail.get_connection() as connection:
-        connection.send_messages(messages)
-
-
 def build_dandiset_unembargoed_message(dandiset: Dandiset):
     dandiset_context = {
         'identifier': dandiset.identifier,

dandiapi/api/models/dandiset.py

@@ -62,6 +62,10 @@ def identifier(self) -> str:
     def embargoed(self) -> bool:
         return self.embargo_status == self.EmbargoStatus.EMBARGOED
 
+    @property
+    def unembargo_in_progress(self) -> bool:
+        return self.embargo_status == self.EmbargoStatus.UNEMBARGOING
+
     @property
     def most_recent_published_version(self):
         return self.versions.exclude(version='draft').order_by('modified').last()

dandiapi/api/services/dandiset/__init__.py

@@ -56,7 +56,7 @@ def delete_dandiset(*, user, dandiset: Dandiset) -> None:
         raise NotAllowedError('Cannot delete dandisets with published versions.')
     if dandiset.versions.filter(status=Version.Status.PUBLISHING).exists():
         raise NotAllowedError('Cannot delete dandisets that are currently being published.')
-    if dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+    if dandiset.unembargo_in_progress:
         raise DandisetUnembargoInProgressError
 
     # Delete all versions first, so that AssetPath deletion is cascaded

dandiapi/api/services/embargo/__init__.py

@@ -8,11 +8,12 @@
 from django.conf import settings
 from django.db import transaction
 
-from dandiapi.api.mail import send_dandiset_unembargoed_message, send_dandisets_to_unembargo_message
+from dandiapi.api.mail import send_dandiset_unembargoed_message
 from dandiapi.api.models import AssetBlob, Dandiset, Version
 from dandiapi.api.services.asset.exceptions import DandisetOwnerRequiredError
 from dandiapi.api.services.exceptions import DandiError
 from dandiapi.api.storage import get_boto_client
+from dandiapi.api.tasks import unembargo_dandiset_task
 
 from .exceptions import (
     AssetBlobEmbargoedError,
@@ -119,10 +120,8 @@ def kickoff_dandiset_unembargo(*, user: User, dandiset: Dandiset):
     if dandiset.uploads.count():
         raise DandisetActiveUploadsError
 
-    # A scheduled task will pick up any new dandisets with this status
-    Dandiset.objects.filter(pk=dandiset.pk).update(
-        embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
-    )
-
-    # Send initial email to ensure it's seen in a timely manner
-    send_dandisets_to_unembargo_message(dandisets=[dandiset])
+    with transaction.atomic():
+        Dandiset.objects.filter(pk=dandiset.pk).update(
+            embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
+        )
+        transaction.on_commit(lambda: unembargo_dandiset_task.delay(dandiset.pk))

dandiapi/api/tasks/__init__.py

@@ -12,6 +12,7 @@
     write_dandiset_yaml,
 )
 from dandiapi.api.models import Asset, AssetBlob, Version
+from dandiapi.api.models.dandiset import Dandiset
 
 logger = get_task_logger(__name__)
 
@@ -74,3 +75,11 @@ def publish_dandiset_task(dandiset_id: int):
     from dandiapi.api.services.publish import _publish_dandiset
 
     _publish_dandiset(dandiset_id=dandiset_id)
+
+
+@shared_task
+def unembargo_dandiset_task(dandiset_id: int):
+    from dandiapi.api.services.embargo import unembargo_dandiset
+
+    ds = Dandiset.objects.get(pk=dandiset_id)
+    unembargo_dandiset(ds)

dandiapi/api/tasks/scheduled.py

@@ -20,10 +20,9 @@
 from django.db.models.query_utils import Q
 
 from dandiapi.analytics.tasks import collect_s3_log_records_task
-from dandiapi.api.mail import send_dandisets_to_unembargo_message, send_pending_users_message
+from dandiapi.api.mail import send_pending_users_message
 from dandiapi.api.models import UserMetadata, Version
 from dandiapi.api.models.asset import Asset
-from dandiapi.api.models.dandiset import Dandiset
 from dandiapi.api.services.metadata import version_aggregate_assets_summary
 from dandiapi.api.services.metadata.exceptions import VersionMetadataConcurrentlyModifiedError
 from dandiapi.api.tasks import (
@@ -112,14 +111,6 @@ def send_pending_users_email() -> None:
         send_pending_users_message(pending_users)
 
 
-@shared_task(soft_time_limit=20)
-def send_dandisets_to_unembargo_email() -> None:
-    """Send an email to admins listing dandisets that have requested unembargo."""
-    dandisets = Dandiset.objects.filter(embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING)
-    if dandisets.exists():
-        send_dandisets_to_unembargo_message(dandisets)
-
-
 @shared_task(soft_time_limit=60)
 def refresh_materialized_view_search() -> None:
     """
@@ -148,9 +139,6 @@ def register_scheduled_tasks(sender: Celery, **kwargs):
     # Send daily email to admins containing a list of users awaiting approval
     sender.add_periodic_task(crontab(hour=0, minute=0), send_pending_users_email.s())
 
-    # Send daily email to admins containing a list of dandisets to unembargo
-    sender.add_periodic_task(crontab(hour=0, minute=0), send_dandisets_to_unembargo_email.s())
-
     # Refresh the materialized view used by asset search every 10 mins.
     sender.add_periodic_task(timedelta(minutes=10), refresh_materialized_view_search.s())
 

dandiapi/api/tests/test_asset.py

@@ -637,7 +637,7 @@ def test_asset_create_embargo(
 
 
 @pytest.mark.django_db()
-def test_asset_create_unembargoing(
+def test_asset_create_unembargo_in_progress(
     api_client, user, draft_version_factory, dandiset_factory, embargoed_asset_blob
 ):
     dandiset = dandiset_factory(embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING)
@@ -1122,7 +1122,7 @@ def test_asset_rest_update_embargo(api_client, user, draft_version, asset, embar
 
 
 @pytest.mark.django_db()
-def test_asset_rest_update_unembargoing(
+def test_asset_rest_update_unembargo_in_progress(
     api_client, user, draft_version_factory, asset, embargoed_asset_blob
 ):
     draft_version = draft_version_factory(
@@ -1325,7 +1325,7 @@ def test_asset_rest_delete(api_client, user, draft_version, asset):
 
 
 @pytest.mark.django_db()
-def test_asset_rest_delete_unembargoing(api_client, user, draft_version_factory, asset):
+def test_asset_rest_delete_unembargo_in_progress(api_client, user, draft_version_factory, asset):
     draft_version = draft_version_factory(
         dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
     )

dandiapi/api/tests/test_dandiset.py

@@ -735,30 +735,28 @@ def test_dandiset_rest_create_with_invalid_identifier(api_client, admin_user):
 
 
 @pytest.mark.django_db()
-def test_dandiset_rest_delete(api_client, draft_version_factory, user):
+@pytest.mark.parametrize(
+    ('embargo_status', 'success'),
+    [
+        (Dandiset.EmbargoStatus.OPEN, True),
+        (Dandiset.EmbargoStatus.EMBARGOED, True),
+        (Dandiset.EmbargoStatus.UNEMBARGOING, False),
+    ],
+)
+def test_dandiset_rest_delete(api_client, draft_version_factory, user, embargo_status, success):
     api_client.force_authenticate(user=user)
 
     # Ensure that open or embargoed dandisets can be deleted
-    draft_version = draft_version_factory(dandiset__embargo_status=Dandiset.EmbargoStatus.OPEN)
-    assign_perm('owner', user, draft_version.dandiset)
-    response = api_client.delete(f'/api/dandisets/{draft_version.dandiset.identifier}/')
-    assert response.status_code == 204
-    assert not Dandiset.objects.all()
-
-    draft_version = draft_version_factory(dandiset__embargo_status=Dandiset.EmbargoStatus.EMBARGOED)
+    draft_version = draft_version_factory(dandiset__embargo_status=embargo_status)
     assign_perm('owner', user, draft_version.dandiset)
     response = api_client.delete(f'/api/dandisets/{draft_version.dandiset.identifier}/')
-    assert response.status_code == 204
-    assert not Dandiset.objects.all()
 
-    # Ensure that currently unembargoing dandisets can't be deleted
-    draft_version = draft_version_factory(
-        dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
-    )
-    assign_perm('owner', user, draft_version.dandiset)
-    response = api_client.delete(f'/api/dandisets/{draft_version.dandiset.identifier}/')
-    assert response.status_code == 400
-    assert Dandiset.objects.count() == 1
+    if success:
+        assert response.status_code == 204
+        assert not Dandiset.objects.all()
+    else:
+        assert response.status_code >= 400
+        assert Dandiset.objects.count() == 1
 
 
 @pytest.mark.django_db()
@@ -885,13 +883,13 @@ def test_dandiset_rest_change_owner(
 
 
 @pytest.mark.django_db()
-def test_dandiset_rest_change_owners_unembargoing(
+def test_dandiset_rest_change_owners_unembargo_in_progress(
     api_client,
     draft_version_factory,
     user_factory,
     social_account_factory,
 ):
-    """Test that unembargoing a dandiset prevents user modification."""
+    """Test that a dandiset undergoing unembargo prevents user modification."""
     draft_version = draft_version_factory(
         dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
     )

dandiapi/api/tests/test_unembargo.py

@@ -69,23 +69,27 @@ def test_kickoff_dandiset_unembargo_active_uploads(
     assert resp.status_code == 400
 
 
-@pytest.mark.django_db()
-def test_kickoff_dandiset_unembargo(api_client, user, draft_version_factory, mailoutbox):
+# transaction=True required due to how `kickoff_dandiset_unembargo` calls `unembargo_dandiset_task`
+@pytest.mark.django_db(transaction=True)
+def test_kickoff_dandiset_unembargo(api_client, user, draft_version_factory, mailoutbox, mocker):
     draft_version = draft_version_factory(dandiset__embargo_status=Dandiset.EmbargoStatus.EMBARGOED)
     ds: Dandiset = draft_version.dandiset
 
     assign_perm('owner', user, ds)
     api_client.force_authenticate(user=user)
 
+    # mock this task to check if called
+    patched_task = mocker.patch('dandiapi.api.services.embargo.unembargo_dandiset_task')
+
     resp = api_client.post(f'/api/dandisets/{ds.identifier}/unembargo/')
     assert resp.status_code == 200
 
     ds.refresh_from_db()
     assert ds.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING
-    assert mailoutbox
-    assert 'unembargo' in mailoutbox[0].subject
-    assert ds.identifier in mailoutbox[0].message().get_payload()
-    assert user.username in mailoutbox[0].message().get_payload()
+
+    # Check that unembargo dandiset task was delayed
+    assert len(patched_task.mock_calls) == 1
+    assert str(patched_task.mock_calls[0]) == f'call.delay({ds.pk})'
 
 
 @pytest.mark.django_db()

dandiapi/api/tests/test_upload.py

@@ -110,7 +110,7 @@ def test_upload_initialize(api_client, user, dandiset_factory, embargoed):
 
 
 @pytest.mark.django_db()
-def test_upload_initialize_unembargoing(api_client, user, dandiset_factory):
+def test_upload_initialize_unembargo_in_progress(api_client, user, dandiset_factory):
     dandiset = dandiset_factory(embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING)
     api_client.force_authenticate(user=user)
     assign_perm('owner', user, dandiset)

dandiapi/api/tests/test_version.py

@@ -606,7 +606,7 @@ def test_version_rest_update(api_client, user, draft_version):
 
 
 @pytest.mark.django_db()
-def test_version_rest_update_unembargoing(api_client, user, draft_version_factory):
+def test_version_rest_update_unembargo_in_progress(api_client, user, draft_version_factory):
     draft_version = draft_version_factory(
         dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
     )
@@ -802,7 +802,7 @@ def test_version_rest_publish_embargo(api_client: APIClient, user: User, draft_v
 
 
 @pytest.mark.django_db()
-def test_version_rest_publish_unembargoing(
+def test_version_rest_publish_unembargo_in_progress(
     api_client: APIClient, user: User, draft_version_factory
 ):
     draft_version = draft_version_factory(

dandiapi/api/views/asset.py

@@ -324,7 +324,7 @@ def create(self, request, versions__dandiset__pk, versions__version):
             version=versions__version,
         )
 
-        if version.dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+        if version.dandiset.unembargo_in_progress:
             raise DandisetUnembargoInProgressError
 
         serializer = AssetRequestSerializer(data=self.request.data)
@@ -361,7 +361,7 @@ def update(self, request, versions__dandiset__pk, versions__version, **kwargs):
         )
         if version.version != 'draft':
             raise DraftDandisetNotModifiableError
-        if version.dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+        if version.dandiset.unembargo_in_progress:
             raise DandisetUnembargoInProgressError
 
         serializer = AssetRequestSerializer(data=self.request.data)
@@ -399,7 +399,7 @@ def destroy(self, request, versions__dandiset__pk, versions__version, **kwargs):
             dandiset__pk=versions__dandiset__pk,
             version=versions__version,
         )
-        if version.dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+        if version.dandiset.unembargo_in_progress:
             raise DandisetUnembargoInProgressError
 
         # Lock asset for delete

dandiapi/api/views/dandiset.py

@@ -377,7 +377,7 @@ def unembargo(self, request, dandiset__pk):
     def users(self, request, dandiset__pk):  # noqa: C901
         dandiset: Dandiset = self.get_object()
         if request.method == 'PUT':
-            if dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+            if dandiset.unembargo_in_progress:
                 raise DandisetUnembargoInProgressError
 
             # Verify that the user is currently an owner

dandiapi/api/views/upload.py

@@ -137,7 +137,7 @@ def upload_initialize_view(request: Request) -> HttpResponseBase:
         return response
 
     # Ensure dandiset not in the process of unembargo
-    if dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+    if dandiset.unembargo_in_progress:
         raise DandisetUnembargoInProgressError
 
     logging.info(

dandiapi/api/views/version.py

@@ -95,7 +95,7 @@ def update(self, request, **kwargs):
                 'Only draft versions can be modified.',
                 status=status.HTTP_405_METHOD_NOT_ALLOWED,
             )
-        if version.dandiset.embargo_status == Dandiset.EmbargoStatus.UNEMBARGOING:
+        if version.dandiset.unembargo_in_progress:
             raise DandisetUnembargoInProgressError
 
         serializer = VersionMetadataSerializer(data=request.data)

web/src/views/DandisetLandingView/DandisetActions.vue

@@ -72,7 +72,7 @@
           id="view-data"
           outlined
           block
-          :disabled="currentDandiset.dandiset.embargo_status === 'UNEMBARGOING'"
+          :disabled="unembargo_in_progress"
           :to="fileBrowserLink"
           exact
         >
@@ -156,6 +156,7 @@ const store = useDandisetStore();
 
 const currentDandiset = computed(() => store.dandiset);
 const currentVersion = computed(() => store.version);
+const unembargo_in_progress = computed(() => currentDandiset.value && currentDandiset.value.dandiset.embargo_status === 'UNEMBARGOING')
 
 const fileBrowserLink: ComputedRef<Location|null> = computed(() => {
   if (!currentDandiset.value) {