From b47a06cf3eb33b3eda7f58915787720d4735dca4 Mon Sep 17 00:00:00 2001
From: Jacob Nesbitt <jjnesbitt2@gmail.com>
Date: Mon, 29 Jul 2024 15:36:54 -0400
Subject: [PATCH] Re-validate version metadata during unembargo

---
 dandiapi/api/services/embargo/__init__.py |  7 +++++++
 dandiapi/api/tests/test_unembargo.py      | 25 ++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/dandiapi/api/services/embargo/__init__.py b/dandiapi/api/services/embargo/__init__.py
index 77c31d048..6f3a173e8 100644
--- a/dandiapi/api/services/embargo/__init__.py
+++ b/dandiapi/api/services/embargo/__init__.py
@@ -13,6 +13,7 @@
 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.services.metadata import validate_version_metadata
 from dandiapi.api.storage import get_boto_client
 from dandiapi.api.tasks import unembargo_dandiset_task
 
@@ -93,9 +94,15 @@ def unembargo_dandiset(ds: Dandiset):
     # Fetch version to ensure changed embargo_status is included
     # Save version to update metadata through populate_metadata
     v = Version.objects.select_for_update().get(dandiset=ds, version='draft')
+    v.status = Version.Status.PENDING
     v.save()
     logger.info('Version metadata updated')
 
+    # Pre-emptively validate version metadata, so that old validation
+    # errors don't show up once un-embargo is finished
+    validate_version_metadata(version=v)
+    logger.info('Version metadata validated')
+
     # Notify owners of completed unembargo
     send_dandiset_unembargoed_message(ds)
     logger.info('Dandiset owners notified.')
diff --git a/dandiapi/api/tests/test_unembargo.py b/dandiapi/api/tests/test_unembargo.py
index 0d6eff3e8..e7a81e3d2 100644
--- a/dandiapi/api/tests/test_unembargo.py
+++ b/dandiapi/api/tests/test_unembargo.py
@@ -7,6 +7,7 @@
 import pytest
 
 from dandiapi.api.models.dandiset import Dandiset
+from dandiapi.api.models.version import Version
 from dandiapi.api.services.embargo import (
     AssetBlobEmbargoedError,
     _remove_dandiset_asset_blob_embargo_tags,
@@ -22,7 +23,6 @@
 
 if TYPE_CHECKING:
     from dandiapi.api.models.asset import AssetBlob
-    from dandiapi.api.models.version import Version
 
 
 @pytest.mark.django_db()
@@ -217,6 +217,29 @@ def test_unembargo_dandiset(
     assert owner_email_set == mailoutbox_to_email_set
 
 
+@pytest.mark.django_db()
+def test_unembargo_dandiset_validate_version_metadata(draft_version_factory, asset_factory, mocker):
+    from dandiapi.api.services import embargo as embargo_service
+
+    draft_version: Version = draft_version_factory(
+        dandiset__embargo_status=Dandiset.EmbargoStatus.UNEMBARGOING
+    )
+    ds: Dandiset = draft_version.dandiset
+    draft_version.validation_errors = ['error ajhh']
+    draft_version.status = Version.Status.INVALID
+    draft_version.save()
+    draft_version.assets.add(asset_factory())
+
+    # Spy on the imported function in the embargo service
+    validate_version_spy = mocker.spy(embargo_service, 'validate_version_metadata')
+
+    unembargo_dandiset(ds)
+
+    assert validate_version_spy.call_count == 1
+    draft_version.refresh_from_db()
+    assert not draft_version.validation_errors
+
+
 @pytest.mark.django_db()
 def test_unembargo_dandiset_task_failure(draft_version_factory, mailoutbox):
     # Intentionally set the status to embargoed so the task will fail