Merge branch 'release/19.22.0'

Author: pattisdr

CHANGELOG

@@ -2,6 +2,11 @@
 
 We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO.
 
+19.22.0 (2019-08-14)
+===================
+- APIv2: Editable registrations
+- APIv2.16 treats subjects as relationships instead of attributes
+
 19.21.0 (2019-08-12)
 ===================
 - PageCounter optimization part I: split _id field into four columns, `action`,

api/base/exceptions.py

@@ -11,7 +11,7 @@ def get_resource_object_member(error_key, context):
     if field:
         return 'relationships' if isinstance(field, RelationshipField) else 'attributes'
     # If field cannot be found (where read/write operations have different serializers,
-    # assume error was in 'attributes' by default
+    # or fields serialized on __init__, assume error was in 'attributes' by default
     return 'attributes'
 
 def dict_error_formatting(errors, context, index=None):

api/base/filters.py

@@ -115,16 +115,21 @@ def _get_field_or_error(self, field_name):
 
         :raises InvalidFilterError: If the filter field is not valid
         """
-        serializer_class = self.serializer_class
-        if field_name not in serializer_class._declared_fields:
+        predeclared_fields = self.serializer_class._declared_fields
+        initialized_fields = self.get_serializer().fields if hasattr(self, 'get_serializer') else {}
+        serializer_fields = predeclared_fields.copy()
+        # Merges fields that were declared on serializer with fields that may have been dynamically added
+        serializer_fields.update(initialized_fields)
+
+        if field_name not in serializer_fields:
             raise InvalidFilterError(detail="'{0}' is not a valid field for this endpoint.".format(field_name))
-        if field_name not in getattr(serializer_class, 'filterable_fields', set()):
+        if field_name not in getattr(self.serializer_class, 'filterable_fields', set()):
             raise InvalidFilterFieldError(parameter='filter', value=field_name)
-        field = serializer_class._declared_fields[field_name]
+        field = serializer_fields[field_name]
         # You cannot filter on deprecated fields.
         if isinstance(field, ShowIfVersion) and utils.is_deprecated(self.request.version, field.min_version, field.max_version):
             raise InvalidFilterFieldError(parameter='filter', value=field_name)
-        return serializer_class._declared_fields[field_name]
+        return serializer_fields[field_name]
 
     def _validate_operator(self, field, field_name, op):
         """
@@ -311,7 +316,7 @@ def convert_value(self, value, field):
                     value=value,
                     field_type='date',
                 )
-        elif isinstance(field, (self.RELATIONSHIP_FIELDS, ser.SerializerMethodField)):
+        elif isinstance(field, (self.RELATIONSHIP_FIELDS, ser.SerializerMethodField, ser.ManyRelatedField)):
             if value == 'null':
                 value = None
             return value
@@ -439,11 +444,14 @@ def postprocess_query_param(self, key, field_name, operation):
             )
             operation['op'] = 'in'
         if field_name == 'subjects':
-            if Subject.objects.filter(_id=operation['value']).exists():
-                operation['source_field_name'] = 'subjects___id'
-            else:
-                operation['source_field_name'] = 'subjects__text'
-                operation['op'] = 'iexact'
+            self.postprocess_subject_query_param(operation)
+
+    def postprocess_subject_query_param(self, operation):
+        if Subject.objects.filter(_id=operation['value']).exists():
+            operation['source_field_name'] = 'subjects___id'
+        else:
+            operation['source_field_name'] = 'subjects__text'
+            operation['op'] = 'iexact'
 
     def get_filtered_queryset(self, field_name, params, default_queryset):
         """filters default queryset based on the serializer field type"""
@@ -511,12 +519,7 @@ def postprocess_query_param(self, key, field_name, operation):
             operation['source_field_name'] = 'guids___id'
 
         if field_name == 'subjects':
-            try:
-                Subject.objects.get(_id=operation['value'])
-                operation['source_field_name'] = 'subjects___id'
-            except Subject.DoesNotExist:
-                operation['source_field_name'] = 'subjects__text'
-                operation['op'] = 'iexact'
+            self.postprocess_subject_query_param(operation)
 
     def preprints_queryset(self, base_queryset, auth_user, allow_contribs=True, public_only=False):
         return Preprint.objects.can_view(

api/base/settings/defaults.py

@@ -166,6 +166,7 @@
         '2.13',
         '2.14',
         '2.15',
+        '2.16',
     ),
     'DEFAULT_FILTER_BACKENDS': ('api.base.filters.OSFOrderingFilter',),
     'DEFAULT_PAGINATION_CLASS': 'api.base.pagination.JSONAPIPagination',

api/base/urls.py

@@ -56,6 +56,7 @@
                 url(r'^requests/', include('api.requests.urls', namespace='requests')),
                 url(r'^scopes/', include('api.scopes.urls', namespace='scopes')),
                 url(r'^search/', include('api.search.urls', namespace='search')),
+                url(r'^subjects/', include('api.subjects.urls', namespace='subjects')),
                 url(r'^subscriptions/', include('api.subscriptions.urls', namespace='subscriptions')),
                 url(r'^taxonomies/', include('api.taxonomies.urls', namespace='taxonomies')),
                 url(r'^test/', include('api.test.urls', namespace='test')),

api/base/views.py

@@ -129,9 +129,6 @@ def partial(item):
                 with transaction.atomic():
                     ret = view.handle_exception(e).data
 
-            # Allow request to be gc'd
-            ser._context = None
-
             # Cache our final result
             cache[_cache_key] = ret
 

api/collections/permissions.py

@@ -4,7 +4,7 @@
 from rest_framework import permissions
 from rest_framework.exceptions import NotFound
 
-from api.base.utils import get_user_auth
+from api.base.utils import get_user_auth, assert_resource_type
 from osf.models import AbstractNode, Preprint, Collection, CollectionSubmission, CollectionProvider
 from osf.utils.permissions import WRITE, ADMIN
 
@@ -45,8 +45,14 @@ def has_object_permission(self, request, view, obj):
         return auth.user and (accepting_submissions or auth.user.has_perm('write_collection', obj))
 
 class CanUpdateDeleteCGMOrPublic(permissions.BasePermission):
+
+    acceptable_models = (CollectionSubmission, )
+
     def has_object_permission(self, request, view, obj):
-        assert isinstance(obj, CollectionSubmission), 'obj must be a CollectionSubmission, got {}'.format(obj)
+        if isinstance(obj, dict):
+            obj = obj.get('self', None)
+
+        assert_resource_type(obj, self.acceptable_models)
         collection = obj.collection
         auth = get_user_auth(request)
         if request.method in permissions.SAFE_METHODS:

api/collections/serializers.py

@@ -200,6 +200,22 @@ class Meta:
         related_view_kwargs={'guids': '<guid._id>'},
         always_embed=True,
     )
+
+    @property
+    def subjects_related_view(self):
+        # Overrides TaxonomizableSerializerMixin
+        return 'collections:collected-metadata-subjects'
+
+    @property
+    def subjects_self_view(self):
+        # Overrides TaxonomizableSerializerMixin
+        return 'collections:collected-metadata-relationships-subjects'
+
+    @property
+    def subjects_view_kwargs(self):
+        # Overrides TaxonomizableSerializerMixin
+        return {'collection_id': '<collection._id>', 'cgm_id': '<guid._id>'}
+
     collected_type = ser.CharField(required=False)
     status = ser.CharField(required=False)
     volume = ser.CharField(required=False)
@@ -220,12 +236,8 @@ def update(self, obj, validated_data):
         if validated_data and 'subjects' in validated_data:
             auth = get_user_auth(self.context['request'])
             subjects = validated_data.pop('subjects', None)
-            try:
-                obj.set_subjects(subjects, auth)
-            except PermissionsError as e:
-                raise exceptions.PermissionDenied(detail=str(e))
-            except (ValueError, NodeStateError) as e:
-                raise exceptions.ValidationError(detail=str(e))
+            self.update_subjects(obj, subjects, auth)
+
         if 'status' in validated_data:
             obj.status = validated_data.pop('status')
         if 'collected_type' in validated_data:

api/collections/urls.py

@@ -9,6 +9,8 @@
     url(r'^(?P<collection_id>\w+)/$', views.CollectionDetail.as_view(), name=views.CollectionDetail.view_name),
     url(r'^(?P<collection_id>\w+)/collected_metadata/$', views.CollectedMetaList.as_view(), name=views.CollectedMetaList.view_name),
     url(r'^(?P<collection_id>\w+)/collected_metadata/(?P<cgm_id>\w+)/$', views.CollectedMetaDetail.as_view(), name=views.CollectedMetaDetail.view_name),
+    url(r'^(?P<collection_id>\w+)/collected_metadata/(?P<cgm_id>\w+)/subjects/$', views.CollectedMetaSubjectsList.as_view(), name=views.CollectedMetaSubjectsList.view_name),
+    url(r'^(?P<collection_id>\w+)/collected_metadata/(?P<cgm_id>\w+)/relationships/subjects/$', views.CollectedMetaSubjectsRelationship.as_view(), name=views.CollectedMetaSubjectsRelationship.view_name),
     url(r'^(?P<collection_id>\w+)/linked_nodes/$', views.LinkedNodesList.as_view(), name=views.LinkedNodesList.view_name),
     url(r'^(?P<collection_id>\w+)/linked_preprints/$', views.LinkedPreprintsList.as_view(), name=views.LinkedPreprintsList.view_name),
     url(r'^(?P<collection_id>\w+)/linked_registrations/$', views.LinkedRegistrationsList.as_view(), name=views.LinkedRegistrationsList.view_name),

api/collections/views.py

@@ -8,6 +8,8 @@
 from api.base import generic_bulk_views as bulk_views
 from api.base import permissions as base_permissions
 from api.base.filters import ListFilterMixin
+from api.base.parsers import JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON
+
 from api.base.views import JSONAPIBaseView
 from api.base.views import BaseLinkedList
 from api.base.views import LinkedNodesRelationship
@@ -34,6 +36,7 @@
 )
 from api.nodes.serializers import NodeSerializer
 from api.preprints.serializers import PreprintSerializer
+from api.subjects.views import SubjectRelationshipBaseView, BaseResourceSubjectsList
 from api.registrations.serializers import RegistrationSerializer
 from osf.models import (
     AbstractNode,
@@ -72,6 +75,18 @@ def collection_preprints(self, collection, user):
             ), user=user,
         )
 
+    def get_collection_submission(self, check_object_permissions=True):
+        collection_submission = get_object_or_error(
+            CollectionSubmission,
+            Q(collection=Collection.load(self.kwargs['collection_id']), guid___id=self.kwargs['cgm_id']),
+            self.request,
+            'submission',
+        )
+        # May raise a permission denied
+        if check_object_permissions:
+            self.check_object_permissions(self.request, collection_submission)
+        return collection_submission
+
 
 class CollectionList(JSONAPIBaseView, bulk_views.BulkUpdateJSONAPIView, bulk_views.BulkDestroyJSONAPIView, bulk_views.ListBulkCreateJSONAPIView, ListFilterMixin, CollectionMixin):
     """Organizer Collections organize projects and components. *Writeable*.
@@ -120,7 +135,7 @@ class CollectionList(JSONAPIBaseView, bulk_views.BulkUpdateJSONAPIView, bulk_vie
     New Organizer Collections are created by issuing a POST request to this endpoint.  The `title` field is
     mandatory. All other fields not listed above will be ignored.  If the Organizer Collection creation is successful
     the API will return a 201 response with the representation of the new node in the body.
-    For the new Collection's canonical URL, see the `/links/self` field of the response.
+    For the new Collection canonical URL, see the `/links/self` field of the response.
 
     ##Query Params
 
@@ -297,6 +312,7 @@ def perform_destroy(self, instance):
         collection = self.get_object()
         collection.delete()
 
+
 class CollectedMetaList(JSONAPIBaseView, generics.ListCreateAPIView, CollectionMixin, ListFilterMixin):
     permission_classes = (
         drf_permissions.IsAuthenticatedOrReadOnly,
@@ -308,7 +324,7 @@ class CollectedMetaList(JSONAPIBaseView, generics.ListCreateAPIView, CollectionM
 
     model_class = CollectionSubmission
     serializer_class = CollectionSubmissionSerializer
-    view_category = 'collected-metadata'
+    view_category = 'collections'
     view_name = 'collected-metadata-list'
 
     def get_serializer_class(self):
@@ -339,20 +355,14 @@ class CollectedMetaDetail(JSONAPIBaseView, generics.RetrieveUpdateDestroyAPIView
     required_write_scopes = [CoreScopes.COLLECTED_META_WRITE]
 
     serializer_class = CollectionSubmissionSerializer
-    view_category = 'collected-metadata'
+    view_category = 'collections'
     view_name = 'collected-metadata-detail'
 
+    parser_classes = (JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON,)
+
     # overrides RetrieveAPIView
     def get_object(self):
-        cgm = get_object_or_error(
-            CollectionSubmission,
-            Q(collection=Collection.load(self.kwargs['collection_id']), guid___id=self.kwargs['cgm_id']),
-            self.request,
-            'submission',
-        )
-        # May raise a permission denied
-        self.check_object_permissions(self.request, cgm)
-        return cgm
+        return self.get_collection_submission()
 
     def perform_destroy(self, instance):
         # Skip collection permission check -- perms class checks when getting CGM
@@ -363,6 +373,43 @@ def perform_update(self, serializer):
         serializer.save()
 
 
+class CollectedMetaSubjectsList(BaseResourceSubjectsList, CollectionMixin):
+    """The documentation for this endpoint can be found [here](https://developer.osf.io/#operation/collected_meta_subjects).
+    """
+    permission_classes = (
+        drf_permissions.IsAuthenticatedOrReadOnly,
+        CanUpdateDeleteCGMOrPublic,
+        base_permissions.TokenHasScope,
+    )
+
+    required_read_scopes = [CoreScopes.COLLECTED_META_READ]
+
+    view_category = 'collections'
+    view_name = 'collected-metadata-subjects'
+
+    def get_resource(self):
+        return self.get_collection_submission()
+
+
+class CollectedMetaSubjectsRelationship(SubjectRelationshipBaseView, CollectionMixin):
+    """The documentation for this endpoint can be found [here](https://developer.osf.io/#operation/collected_meta_subjects_relationship).
+    """
+    permission_classes = (
+        drf_permissions.IsAuthenticatedOrReadOnly,
+        CanUpdateDeleteCGMOrPublic,
+        base_permissions.TokenHasScope,
+    )
+
+    required_read_scopes = [CoreScopes.COLLECTED_META_READ]
+    required_write_scopes = [CoreScopes.COLLECTED_META_WRITE]
+
+    view_category = 'collections'
+    view_name = 'collected-metadata-relationships-subjects'
+
+    def get_resource(self, check_object_permissions=True):
+        return self.get_collection_submission(check_object_permissions)
+
+
 class LinkedNodesList(BaseLinkedList, CollectionMixin, NodeOptimizationMixin):
     """List of nodes linked to this node. *Read-only*.
 

api/guids/serializers.py

@@ -82,7 +82,7 @@ def to_representation(self, obj):
             ser = resolve(reverse(
                 get_related_view(obj),
                 kwargs={'node_id': obj._id, 'version': self.context['view'].kwargs.get('version', '2')},
-            )).func.cls.serializer_class()
+            )).func.cls.serializer_class(context=self.context)
             [ser.context.update({k: v}) for k, v in self.context.items()]
             return ser.to_representation(obj)
         return super(GuidSerializer, self).to_representation(obj)

api/nodes/serializers.py

@@ -526,6 +526,21 @@ class NodeSerializer(TaxonomizableSerializerMixin, JSONAPISerializer):
         related_view_kwargs={'node_id': '<_id>'},
     ))
 
+    @property
+    def subjects_related_view(self):
+        # Overrides TaxonomizableSerializerMixin
+        return 'nodes:node-subjects'
+
+    @property
+    def subjects_view_kwargs(self):
+        # Overrides TaxonomizableSerializerMixin
+        return {'node_id': '<_id>'}
+
+    @property
+    def subjects_self_view(self):
+        # Overrides TaxonomizableSerializerMixin
+        return 'nodes:node-relationships-subjects'
+
     def get_current_user_permissions(self, obj):
         """
         Returns the logged-in user's permissions to the
@@ -827,14 +842,7 @@ def update(self, node, validated_data):
                 node.save()
             if 'subjects' in validated_data:
                 subjects = validated_data.pop('subjects', None)
-                try:
-                    node.set_subjects(subjects, auth)
-                except PermissionsError as e:
-                    raise exceptions.PermissionDenied(detail=str(e))
-                except ValueError as e:
-                    raise exceptions.ValidationError(detail=str(e))
-                except NodeStateError as e:
-                    raise exceptions.ValidationError(detail=str(e))
+                self.update_subjects(node, subjects, auth)
 
             try:
                 node.update(validated_data, auth=auth)
@@ -978,14 +986,14 @@ def get_folder_info(self, data, addon_name):
         return set_folder, folder_info
 
     def get_account_or_error(self, addon_name, external_account_id, auth):
-            external_account = ExternalAccount.load(external_account_id)
-            if not external_account:
-                raise exceptions.NotFound('Unable to find requested account.')
-            if not auth.user.external_accounts.filter(id=external_account.id).exists():
-                raise exceptions.PermissionDenied('Requested action requires account ownership.')
-            if external_account.provider != addon_name:
-                raise Conflict('Cannot authorize the {} addon with an account for {}'.format(addon_name, external_account.provider))
-            return external_account
+        external_account = ExternalAccount.load(external_account_id)
+        if not external_account:
+            raise exceptions.NotFound('Unable to find requested account.')
+        if not auth.user.external_accounts.filter(id=external_account.id).exists():
+            raise exceptions.PermissionDenied('Requested action requires account ownership.')
+        if external_account.provider != addon_name:
+            raise Conflict('Cannot authorize the {} addon with an account for {}'.format(addon_name, external_account.provider))
+        return external_account
 
     def should_call_set_folder(self, folder_info, instance, auth, node_settings):
         if (folder_info and not (   # If we have folder information to set