diff --git a/api_tests/collections/test_views.py b/api_tests/collections/test_views.py index 7ae088ab5a7..aa2eeaea40c 100644 --- a/api_tests/collections/test_views.py +++ b/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(expect_none=True): + 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): diff --git a/osf/models/cedar_metadata.py b/osf/models/cedar_metadata.py index df8d4cda4d3..b14c27c4190 100644 --- a/osf/models/cedar_metadata.py +++ b/osf/models/cedar_metadata.py @@ -1,4 +1,6 @@ +from django.core.exceptions import ValidationError from django.db import models +from jsonschema import validate as jsonschema_validate, ValidationError as JsonSchemaValidationError from osf.models.base import BaseModel, ObjectIDMixin from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField @@ -47,6 +49,16 @@ def get_template_name(self): def get_template_version(self): return self.template.template_version + def clean(self): + if self.is_published: + try: + jsonschema_validate(self.metadata, self.template.template) + except JsonSchemaValidationError as e: + raise ValidationError( + f'CEDAR metadata does not validate against template "{self.template.schema_name}": {e.message}' + ) + def save(self, *args, **kwargs): + self.clean() self.guid.referent.update_search() return super().save(*args, **kwargs) diff --git a/osf/models/collection_submission.py b/osf/models/collection_submission.py index f2de5ba6610..86f52ebdce6 100644 --- a/osf/models/collection_submission.py +++ b/osf/models/collection_submission.py @@ -22,7 +22,6 @@ logger = logging.getLogger(__name__) - class CollectionSubmission(TaxonomizableMixin, BaseModel): primary_identifier_name = 'guid___id' diff --git a/osf/models/provider.py b/osf/models/provider.py index dd32a88ff59..1064f7e95c7 100644 --- a/osf/models/provider.py +++ b/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 @@ -208,6 +206,24 @@ def top_level_subjects(self): def readable_type(self): raise NotImplementedError + 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 get_asset_url(self, name): """ Helper that returns an associated ProviderAssetFile's url, or None @@ -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 = [ diff --git a/osf_tests/test_validate_required_metadata.py b/osf_tests/test_validate_required_metadata.py index 73d4b166815..4e49f2cf42a 100644 --- a/osf_tests/test_validate_required_metadata.py +++ b/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()