Update DOIs with Latest CMR Metadata

admin_ui/tests/factories.py

@@ -1,7 +1,6 @@
 import factory
 from django.contrib.auth import get_user_model
 from django.contrib.auth.hashers import make_password
-from django.contrib.contenttypes.models import ContentType
 
 from admg_webapp.users.models import User
 from api_app.models import Change
@@ -11,53 +10,6 @@ class ChangeFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = Change
 
-    @staticmethod
-    def make_create_change_object(factory, custom_fields={}):
-        """make a Change.Actions.CREATE change object to use during testing"""
-        content_type = ContentType.objects.get_for_model(factory._meta.model)
-
-        # _meta.fields does not contain many to many
-        model_field_names = {
-            field.name
-            for field in factory._meta.get_model_class()._meta._forward_fields_map.values()
-        }
-        overrides = {
-            field: value for field, value in custom_fields.items() if field in model_field_names
-        }
-
-        return Change.objects.create(
-            content_type=content_type,
-            status=Change.Statuses.CREATED,
-            action=Change.Actions.CREATE,
-            update={**factory.as_change_dict(), **overrides},
-        )
-
-    @staticmethod
-    def make_update_change_object(factory, create_draft, fields_to_keep=[]):
-        """make a Change.Actions.CREATE change object to use during testing"""
-        content_type = ContentType.objects.get_for_model(factory._meta.model)
-
-        # we want the ability to keep the original's values, say short_name or concept_id
-        # models can't take any field though, so this checks that the fields are real
-        # TODO: move this to an error check?
-        model_field_names = {
-            field.name
-            for field in factory._meta.get_model_class()._meta._forward_fields_map.values()
-        }
-        overrides = {
-            field: create_draft.update[field]
-            for field in fields_to_keep
-            if field in model_field_names
-        }
-
-        return Change.objects.create(
-            content_type=content_type,
-            status=Change.Statuses.CREATED,
-            action=Change.Actions.UPDATE,
-            model_instance_uuid=create_draft.uuid,
-            update={**factory.as_change_dict(), **overrides},
-        )
-
 
 class UserFactory(factory.django.DjangoModelFactory):
     username = factory.Faker('user_name')

admin_ui/tests/test_views/test_change.py

@@ -5,15 +5,16 @@
 from admg_webapp.users.models import User
 from admin_ui.views.change import CampaignDetailView
 from api_app.models import Change
+from api_app.tests.test_change import TestChange
 from data_models.models import Campaign, Season
 
-from admin_ui.tests import factories
+from .. import factories
 from data_models.tests.factories import CampaignFactory
 
 
 class TestChangeUpdateView(TestCase):
     def setUp(self):
-        self.change = factories.ChangeFactory.make_create_change_object(CampaignFactory)
+        self.change = TestChange.make_create_change_object(CampaignFactory)
         self.user = factories.UserFactory.create()
         self.url = reverse("change-update", args=(self.change.uuid,))
 
@@ -79,7 +80,7 @@ class TestCampaignDetailView(TestCase):
     def setUp(self):
         # admin is needed to force publish without the intervening steps
         self.user = factories.UserFactory.create(role=User.Roles.ADMIN)
-        self.create_change = factories.ChangeFactory.make_create_change_object(CampaignFactory)
+        self.create_change = TestChange.make_create_change_object(CampaignFactory)
         self.create_change.publish(self.user)
 
         self.update_changes = []
@@ -109,7 +110,7 @@ def test_filter_latest_changes_with_multiple_models_returns_latest_change(self):
         method should return the latest Change object for each Campaign.
         """
 
-        create_change = factories.ChangeFactory.make_create_change_object(CampaignFactory)
+        create_change = TestChange.make_create_change_object(CampaignFactory)
         create_change.publish(self.user)
 
         update_changes = []

admin_ui/tests/test_views/test_doi.py

@@ -1,38 +1 @@
-from datetime import datetime, timezone
 
-from django.test import TestCase
-from django.urls import reverse
-
-from admin_ui.views.doi import update_dois
-
-# from api_app.tests.test_change import TestChange
-
-from admin_ui.tests import factories
-from data_models.tests.factories import DOIFactory
-
-from freezegun import freeze_time
-
-frozen_time = datetime(2023, 4, 1, 0, 0, 0, tzinfo=timezone.utc)
-
-
-class TestDoiApprovalView(TestCase):
-    def setUp(self):
-        self.change = factories.ChangeFactory.make_create_change_object(DOIFactory)
-        self.user = factories.UserFactory.create()
-
-    def test_requires_auth(self):
-        url = reverse("doi-approval", args=(self.change.uuid,))
-        response = self.client.get(url)
-        self.assertEqual(302, response.status_code)
-        self.assertEqual(f"{reverse('account_login')}?next={url}", response.url)
-
-    @freeze_time(frozen_time)
-    def test_update_dois_sets_updated_at(self):
-        old_updated_at = self.change.updated_at
-        assert old_updated_at != frozen_time
-
-        doi_form_value = {"uuid": self.change.uuid, "keep": True}
-
-        update_dois(dois=[doi_form_value], user=self.user)
-        self.change.refresh_from_db()
-        assert self.change.updated_at == frozen_time

admin_ui/views/doi.py

@@ -1,15 +1,11 @@
-from typing import Sequence
-from uuid import UUID
 from api_app.views.generic_views import NotificationSidebar
 from api_app.models import ApprovalLog, Change
 from cmr import tasks
 from data_models.models import DOI, Campaign
 from django.contrib import messages
-from django.contrib.auth import get_user_model
 from django.contrib.auth.decorators import login_required
 from django.http import Http404, HttpResponseRedirect
 from django.urls import reverse
-from django.utils import timezone
 from django.utils.decorators import method_decorator
 from django.views import View
 from django.views.generic.detail import SingleObjectMixin
@@ -20,73 +16,6 @@
 from .. import forms
 
 
-def trash_dois(doi_uuids: Sequence[UUID], user: get_user_model()):
-    for doi in Change.objects.filter(uuid__in=doi_uuids):
-        doi.trash(user=user, doi=True)
-        doi.save()
-
-
-def update_dois(dois: Sequence[dict], user: get_user_model()):
-    ignored_updates = []
-    change_status_to_edit = []
-    change_status_to_review = []
-    stored_dois = Change.objects.in_bulk([doi["uuid"] for doi in dois])
-    for doi in dois:
-        stored_doi = stored_dois[doi["uuid"]]
-
-        if stored_doi.status == Change.Statuses.PUBLISHED:
-            ignored_updates.append(doi)
-            continue
-
-        # Persist DOI updates
-        for field, value in doi.items():
-            if field in ["uuid", "keep"]:
-                continue
-            stored_doi.update[field] = value
-        # never been previously edited and checkmark and trash haven't been selected
-        if stored_doi.status == Change.Statuses.CREATED and doi["keep"] is None:
-            stored_doi.status = Change.Statuses.IN_PROGRESS
-            stored_doi.updated_at = timezone.now()
-            change_status_to_edit.append(stored_doi)
-        # checkmark was selected
-        elif doi["keep"] is True:
-            if stored_doi.status == Change.Statuses.IN_TRASH:
-                stored_doi.untrash(user=user, doi=True)
-            stored_doi.status = Change.Statuses.AWAITING_REVIEW
-            stored_doi.updated_at = timezone.now()
-            change_status_to_review.append(stored_doi)
-
-    Change.objects.bulk_update(
-        stored_dois.values(), ["update", "updated_at", "status"], batch_size=100
-    )
-
-    ApprovalLog.objects.bulk_create(
-        [
-            ApprovalLog(
-                change=doi,
-                user=user,
-                action=ApprovalLog.Actions.EDIT,
-                notes="Transitioned via the DOI Approval form",
-            )
-            for doi in change_status_to_edit
-        ]
-    )
-
-    ApprovalLog.objects.bulk_create(
-        [
-            ApprovalLog(
-                change=doi,
-                user=user,
-                action=ApprovalLog.Actions.SUBMIT,
-                notes="Transitioned via the DOI Approval form",
-            )
-            for doi in change_status_to_review
-        ]
-    )
-
-    return change_status_to_edit + change_status_to_review, ignored_updates
-
-
 @method_decorator(login_required, name="dispatch")
 class DoiFetchView(NotificationSidebar, View):
     queryset = Change.objects.of_type(Campaign)
@@ -176,13 +105,12 @@ def get_initial(self):
             for v in paginated_queryset.only("uuid", "update", "status")
         ]
 
-    def form_valid(self, formset: forms.DoiFormSet):
-        changed_dois: Sequence[forms.DoiForm] = [
-            form.cleaned_data for form in formset.forms if form.has_changed()
-        ]
+    def form_valid(self, formset):
+        changed_dois = [form.cleaned_data for form in formset.forms if form.has_changed()]
 
-        to_update: list[dict] = []
-        to_trash: list[dict] = []
+        to_update = []
+        to_trash = []
+        ignored_updates = []
 
         for doi in changed_dois:
             if doi["keep"] is False:
@@ -191,19 +119,70 @@ def form_valid(self, formset: forms.DoiFormSet):
                 to_update.append(doi)
 
         if to_trash:
-            trash_dois(doi_uuids=[doi["uuid"] for doi in to_trash], user=self.request.user)
+            for doi in Change.objects.filter(uuid__in=[doi["uuid"] for doi in to_trash]):
+                doi.trash(user=self.request.user, doi=True)
+                doi.save()
 
         if to_update:
-            updated_dois, ignored_dois = update_dois(dois=to_update)
+            change_status_to_edit = []
+            change_status_to_review = []
+            stored_dois = Change.objects.in_bulk([doi["uuid"] for doi in to_update])
+            for doi in to_update:
+                stored_doi = stored_dois[doi["uuid"]]
+
+                if stored_doi.status == Change.Statuses.PUBLISHED:
+                    ignored_updates.append(doi)
+                    continue
+
+                # Persist DOI updates
+                for field, value in doi.items():
+                    if field in ["uuid", "keep"]:
+                        continue
+                    stored_doi.update[field] = value
+                # never been previously edited and checkmark and trash haven't been selected
+                if stored_doi.status == Change.Statuses.CREATED and doi["keep"] is None:
+                    stored_doi.status = Change.Statuses.IN_PROGRESS
+                    change_status_to_edit.append(stored_doi)
+                # checkmark was selected
+                elif doi["keep"] is True:
+                    if stored_doi.status == Change.Statuses.IN_TRASH:
+                        stored_doi.untrash(user=self.request.user, doi=True)
+                    stored_doi.status = Change.Statuses.AWAITING_REVIEW
+                    change_status_to_review.append(stored_doi)
+
+            Change.objects.bulk_update(stored_dois.values(), ["update", "status"], batch_size=100)
+
+            ApprovalLog.objects.bulk_create(
+                [
+                    ApprovalLog(
+                        change=doi,
+                        user=self.request.user,
+                        action=ApprovalLog.Actions.EDIT,
+                        notes="Transitioned via the DOI Approval form",
+                    )
+                    for doi in change_status_to_edit
+                ]
+            )
+
+            ApprovalLog.objects.bulk_create(
+                [
+                    ApprovalLog(
+                        change=doi,
+                        user=self.request.user,
+                        action=ApprovalLog.Actions.SUBMIT,
+                        notes="Transitioned via the DOI Approval form",
+                    )
+                    for doi in change_status_to_review
+                ]
+            )
 
         messages.info(
             self.request,
-            f"Updated {len(updated_dois)} and removed {len(to_trash)} DOIs.",
+            f"Updated {len(to_update) - len(ignored_updates)} and removed {len(to_trash)} DOIs.",
         )
-
-        if ignored_dois:
+        if ignored_updates:
             messages.warning(
-                self.request, f"Ignored changes to published {len(ignored_dois)} DOIs."
+                self.request, f"Ignored changes to published {len(ignored_updates)} DOIs."
             )
         return super().form_valid(formset)
 

api_app/models.py

@@ -14,7 +14,6 @@
 from rest_framework.serializers import ValidationError
 
 from admg_webapp.users.models import User
-from api_app.signals import temp_disconnect_signal
 from data_models import serializers
 
 
@@ -233,8 +232,7 @@ class Statuses(models.IntegerChoices):
         IN_PROGRESS = 1, "In Progress"
         # The change has been added to the review pile, but hasn't been claimed
         AWAITING_REVIEW = 2, "Awaiting Review"
-        # The change as been claimed, and can now be can now be reviewed or rejected.
-        # Rejection sends it back to the in_progress state
+        # The change as been claimed, and can now be can now be reviewed or rejected. Rejection sends it back to the in_progress state
         IN_REVIEW = 3, "In Review"
         # The change has been added to the admin review pile, but hasn't been claimed
         AWAITING_ADMIN_REVIEW = 4, "Awaiting Admin Review"
@@ -845,13 +843,10 @@ def set_change_updated_at(sender, instance, **kwargs):
     Set `updated_at` on the related Change object to the value of
     the ApprovalLog's `date` field.
     """
-    with temp_disconnect_signal(post_save, create_approval_log_dispatcher, Change, "save"):
-        try:
-            if instance.change:  # Check if the 'change' field exists
-                instance.change.updated_at = instance.date
-                instance.change.save()
-        except Change.DoesNotExist:
-            pass
+    try:
+        Change.objects.filter(pk=instance.change.pk).update(updated_at=instance.date)
+    except Change.DoesNotExist:
+        pass
 
 
 class Recommendation(models.Model):

api_app/signals.py

@@ -1,18 +0,0 @@
-class temp_disconnect_signal:
-    """Context manager for temporarily disconnecting a model from a signal"""
-
-    def __init__(self, signal, receiver, sender, dispatch_uid=None):
-        self.signal = signal
-        self.receiver = receiver
-        self.sender = sender
-        self.dispatch_uid = dispatch_uid
-
-    def __enter__(self):
-        self.signal.disconnect(
-            receiver=self.receiver, sender=self.sender, dispatch_uid=self.dispatch_uid
-        )
-
-    def __exit__(self, type, value, traceback):
-        self.signal.connect(
-            receiver=self.receiver, sender=self.sender, dispatch_uid=self.dispatch_uid
-        )