[ENG-9827] - save collection-submission custom metadata to CedarMetadataRecord on create/update

api/collections/serializers.py

@@ -429,15 +429,16 @@ def create(self, validated_data):
             raise exceptions.ValidationError('"creator" must be specified.')
         if not (creator.has_perm('write_collection', collection) or (hasattr(guid.referent, 'has_permission') and guid.referent.has_permission(creator, WRITE))):
             raise exceptions.PermissionDenied('Must have write permission on either collection or collected object to collect.')
-        if waffle.switch_is_active(features.COLLECTION_SUBMISSION_WITH_CEDAR) and collection.provider_id:
-            try:
-                collection.provider.validate_required_metadata(guid.referent)
-            except ValidationError as e:
-                raise InvalidModelValueError(e.message)
         try:
             obj = collection.collect_object(guid.referent, creator, **validated_data)
         except ValidationError as e:
             raise InvalidModelValueError(e.message)
+        if waffle.switch_is_active(features.COLLECTION_SUBMISSION_WITH_CEDAR):
+            if collection.provider_id:
+                try:
+                    collection.provider.validate_required_metadata(guid.referent)
+                except ValidationError as e:
+                    raise InvalidModelValueError(e.message)
         if subjects:
             auth = get_user_auth(self.context['request'])
             try:
@@ -470,15 +471,16 @@ def create(self, validated_data):
             raise exceptions.ValidationError('"creator" must be specified.')
         if not (creator.has_perm('write_collection', collection) or (hasattr(guid.referent, 'has_permission') and guid.referent.has_permission(creator, WRITE))):
             raise exceptions.PermissionDenied('Must have write permission on either collection or collected object to collect.')
-        if waffle.switch_is_active(features.COLLECTION_SUBMISSION_WITH_CEDAR) and collection.provider_id:
-            try:
-                collection.provider.validate_required_metadata(guid.referent)
-            except ValidationError as e:
-                raise InvalidModelValueError(e.message)
         try:
             obj = collection.collect_object(guid.referent, creator, **validated_data)
         except ValidationError as e:
             raise InvalidModelValueError(e.message)
+        if waffle.switch_is_active(features.COLLECTION_SUBMISSION_WITH_CEDAR):
+            if collection.provider_id:
+                try:
+                    collection.provider.validate_required_metadata(guid.referent)
+                except ValidationError as e:
+                    raise InvalidModelValueError(e.message)
         if subjects:
             auth = get_user_auth(self.context['request'])
             try:

api_tests/collections/test_views.py

@@ -10,6 +10,7 @@
 from api_tests.subjects.mixins import UpdateSubjectsMixin, SubjectsFilterMixin, SubjectsListMixin, \
     SubjectsRelationshipMixin
 from api_tests.utils import disconnected_from_listeners
+from tests.utils import capture_notifications
 from framework.auth.core import Auth
 from osf import features
 from osf.models import Collection, VersionedGuidMixin
@@ -4450,16 +4451,70 @@ def test_switch_active_no_provider_submission_succeeds(self, app, user_one, proj
                 )
         assert res.status_code == 201
 
-    def test_switch_active_missing_cedar_record_submission_fails(self, app, user_one, project, url, payload):
-        with override_switch(features.COLLECTION_SUBMISSION_WITH_CEDAR, active=True):
-            res = app.post_json_api(
-                url,
-                payload(guid=project._id),
-                auth=user_one.auth,
-                expect_errors=True,
-            )
+    def test_switch_active_submission_without_cedar_record_fails(
+            self, app, user_one, project, url, payload, cedar_template):
+        with capture_notifications():
+            with mock_update_share():
+                with override_switch(features.COLLECTION_SUBMISSION_WITH_CEDAR, active=True):
+                    res = app.post_json_api(
+                        url,
+                        payload(guid=project._id),
+                        auth=user_one.auth,
+                        expect_errors=True,
+                    )
         assert res.status_code == 400
-        assert 'CEDAR metadata record' in res.json['errors'][0]['detail']
+
+    def test_switch_active_submission_with_cedar_record_succeeds(
+            self, app, user_one, project, url, payload, cedar_template):
+        from osf.models import CedarMetadataRecord
+        CedarMetadataRecord.objects.create(
+            guid=project.guids.first(),
+            template=cedar_template,
+            metadata={'title': 'Test'},
+            is_published=True,
+        )
+        with capture_notifications():
+            with mock_update_share():
+                with override_switch(features.COLLECTION_SUBMISSION_WITH_CEDAR, active=True):
+                    res = app.post_json_api(
+                        url,
+                        payload(guid=project._id),
+                        auth=user_one.auth,
+                    )
+        assert res.status_code == 201
+
+    def test_switch_inactive_submission_without_cedar_record_succeeds(
+            self, app, user_one, project, url, payload, cedar_template):
+        with capture_notifications():
+            with mock_update_share():
+                with override_switch(features.COLLECTION_SUBMISSION_WITH_CEDAR, active=False):
+                    res = app.post_json_api(url, payload(guid=project._id), auth=user_one.auth)
+        assert res.status_code == 201
+
+    def test_switch_active_update_does_not_alter_cedar_record(
+            self, app, user_one, project, url, payload, cedar_template, collection):
+        from osf.models import CedarMetadataRecord
+        original_metadata = {'title': 'Original'}
+        CedarMetadataRecord.objects.create(
+            guid=project.guids.first(),
+            template=cedar_template,
+            metadata=original_metadata,
+            is_published=True,
+        )
+        collection.status_choices = ['pending', 'approved']
+        collection.save()
+        with capture_notifications():
+            with mock_update_share():
+                with override_switch(features.COLLECTION_SUBMISSION_WITH_CEDAR, active=True):
+                    res = app.post_json_api(url, payload(guid=project._id, status='pending'), auth=user_one.auth)
+        assert res.status_code == 201
+
+        detail_url = f'/{API_BASE}collections/{collection._id}/collected_metadata/{project._id}/'
+        with override_switch(features.COLLECTION_SUBMISSION_WITH_CEDAR, active=True):
+            app.patch_json_api(detail_url, payload(status='approved'), auth=user_one.auth)
+
+        record = CedarMetadataRecord.objects.get(guid__in=project.guids.all(), template=cedar_template)
+        assert record.metadata == original_metadata
 
 
 class TestCollectedMetaSubjectFiltering(SubjectsFilterMixin):

osf/models/collection_submission.py

@@ -22,7 +22,6 @@
 
 logger = logging.getLogger(__name__)
 
-
 class CollectionSubmission(TaxonomizableMixin, BaseModel):
     primary_identifier_name = 'guid___id'
 

osf/models/provider.py

@@ -1,6 +1,5 @@
 import json
 import requests
-from jsonschema import validate as jsonschema_validate, ValidationError as JsonSchemaValidationError
 
 from django.apps import apps
 from django.contrib.postgres import fields
@@ -21,7 +20,6 @@
 from .brand import Brand
 from .citation import CitationStyle
 from .licenses import NodeLicense
-from .cedar_metadata import CedarMetadataRecord
 from .storage import ProviderAssetFile
 from .subject import Subject
 from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField
@@ -167,6 +165,24 @@ def update_or_create_from_json(cls, provider_data, user):
         related_name='required_by_providers',
     )
 
+    def validate_required_metadata(self, obj):
+        """
+        Raises ValidationError if obj does not have a published CedarMetadataRecord for
+        this provider's required_metadata_template.
+        Does nothing when required_metadata_template is not set.
+        """
+        if not self.required_metadata_template_id:
+            return
+        guid = obj.guids.first()
+        if guid is None or not guid.cedar_metadata_records.filter(
+            template_id=self.required_metadata_template_id,
+            is_published=True,
+        ).exists():
+            raise ValidationError(
+                f'Submitted object must have a published CEDAR metadata record for template '
+                f'"{self.required_metadata_template.schema_name}" to be submitted to this collection.'
+            )
+
     def __repr__(self):
         return ('(name={self.name!r}, default_license={self.default_license!r}, '
                 'allow_submissions={self.allow_submissions!r}) with id {self.id!r}').format(self=self)
@@ -259,27 +275,6 @@ def setup_share_source(self, provider_home_page):
 
         self.save()
 
-    def validate_required_metadata(self, osf_obj):
-        if not self.required_metadata_template:
-            return
-
-        record = CedarMetadataRecord.objects.filter(
-            guid__in=osf_obj.guids.all(),
-            template=self.required_metadata_template,
-            is_published=True,
-        ).first()
-
-        if record is None:
-            raise ValidationError(
-                f'Object must have a published CEDAR metadata record for the required template '
-                f'"{self.required_metadata_template.schema_name}".'
-            )
-
-        try:
-            jsonschema_validate(record.metadata, self.required_metadata_template.template)
-        except JsonSchemaValidationError as e:
-            raise ValidationError(e.message)
-
 
 class CollectionProvider(AbstractProvider):
     DEFAULT_SUBSCRIPTIONS = [

osf_tests/test_validate_required_metadata.py

@@ -84,20 +84,6 @@ def test_published_valid_record_passes(self, provider, cedar_template, preprint)
 
         provider.validate_required_metadata(preprint)
 
-    def test_published_invalid_record_raises(self, provider, cedar_template, preprint):
-        provider.required_metadata_template = cedar_template
-        provider.save()
-
-        CedarMetadataRecord.objects.create(
-            guid=preprint.guids.first(),
-            template=cedar_template,
-            metadata={'title': 123},
-            is_published=True,
-        )
-
-        with pytest.raises(ValidationError):
-            provider.validate_required_metadata(preprint)
-
     def test_record_for_wrong_template_raises(self, provider, cedar_template, preprint):
         provider.required_metadata_template = cedar_template
         provider.save()