Merge branch 'develop' of https://github.com/centerforopenscience/osf.io into feature/notification-refactor-p2-s

# Conflicts:
#	tests/test_adding_contributor_views.py

Author: Johnetordoff

admin/users/views.py

@@ -512,7 +512,7 @@ def get_claim_links(self, user):
 
         for guid, value in user.unclaimed_records.items():
             obj = Guid.load(guid)
-            url = f'{DOMAIN}user/{user._id}/{guid}/claim/?token={value["token"]}'
+            url = f'{DOMAIN}legacy/user/{user._id}/{guid}/claim/?token={value["token"]}'
             links.append(f'Claim URL for {obj.content_type.model} {obj._id}: {url}')
 
         return links or ['User currently has no active unclaimed records for any nodes.']

api/nodes/serializers.py

@@ -1308,6 +1308,12 @@ def update(self, instance, validated_data):
             validated_data,
         )
 
+class NodeContributorsUpdateSerializer(ser.Serializer):
+    def update(self, instance, validated_data):
+        if project := instance.root:
+            instance.copy_contributors_from(project)
+        return instance
+
 
 class NodeLinksSerializer(JSONAPISerializer):
 

api/nodes/views.py

@@ -109,6 +109,7 @@
     DraftRegistrationDetailLegacySerializer,
     NodeContributorsSerializer,
     NodeContributorDetailSerializer,
+    NodeContributorsUpdateSerializer,
     NodeInstitutionsRelationshipSerializer,
     NodeContributorsCreateSerializer,
     NodeViewOnlyLinkSerializer,
@@ -436,11 +437,16 @@ class NodeContributorsList(BaseContributorList, bulk_views.BulkUpdateJSONAPIView
     def get_resource(self):
         return self.get_node()
 
+    def get_object(self):
+        return self.get_node()
+
     # overrides ListBulkCreateJSONAPIView, BulkUpdateJSONAPIView, BulkDeleteJSONAPIView
     def get_serializer_class(self):
         """
         Use NodeContributorDetailSerializer which requires 'id'
         """
+        if self.request.method == 'PATCH' and self.request.query_params.get('copy_contributors_from_parent_project'):
+            return NodeContributorsUpdateSerializer
         if self.request.method == 'PUT' or self.request.method == 'PATCH' or self.request.method == 'DELETE':
             return NodeContributorDetailSerializer
         elif self.request.method == 'POST':
@@ -501,6 +507,18 @@ def get_serializer_context(self):
         context['default_email'] = 'default'
         return context
 
+    def patch(self, request, *args, **kwargs):
+        """
+        Override the default patch behavior to handle the special case
+        of updating contributors by copying contributors from the parent project.
+        """
+        if request.query_params.get('copy_contributors_from_parent_project'):
+            instance = self.get_object()
+            serializer = self.get_serializer_class()(instance, data=request.data)
+            serializer.is_valid(raise_exception=True)
+            serializer.save()
+            return Response(serializer.data, status=200)
+        return super().patch(request, *args, **kwargs)
 
 class NodeContributorDetail(BaseContributorDetail, generics.RetrieveUpdateDestroyAPIView, NodeMixin, UserMixin):
     """The documentation for this endpoint can be found [here](https://developer.osf.io/#operation/nodes_contributors_read).

api_tests/base/test_utils.py

@@ -5,7 +5,6 @@
 from django.conf import settings as django_conf_settings
 
 from rest_framework import fields
-from rest_framework.exceptions import ValidationError
 
 from api.base import utils as api_utils
 from osf.models.base import coerce_guid, Guid, GuidMixin, OptionalGuidMixin, VersionedGuidMixin, InvalidGuid
@@ -63,47 +62,6 @@ def test_push_status_message_no_response(self):
             except BaseException:
                 assert False, f'Exception from push_status_message via API v2 with type "{status}".'
 
-    def test_push_status_message_expected_error(self):
-        status_message = 'This is a message'
-        try:
-            push_status_message(status_message, kind='error')
-            assert False, 'push_status_message() should have generated a ValidationError exception.'
-
-        except ValidationError as e:
-            assert (
-                e.detail[0] == status_message
-            ), 'push_status_message() should have passed along the message with the Exception.'
-
-        except RuntimeError:
-            assert False, 'push_status_message() should have caught the runtime error and replaced it.'
-
-        except BaseException:
-            assert False, 'Exception from push_status_message when called from the v2 API with type "error"'
-
-    @mock.patch('framework.status.get_session')
-    def test_push_status_message_unexpected_error(self, mock_get_session):
-        status_message = 'This is a message'
-        exception_message = 'this is some very unexpected problem'
-        mock_session = mock.Mock()
-        mock_session.attach_mock(mock.Mock(side_effect=RuntimeError(exception_message)), 'get')
-        mock_get_session.return_value = mock_session
-        try:
-            push_status_message(status_message, kind='error')
-            assert False, 'push_status_message() should have generated a RuntimeError exception.'
-        except ValidationError:
-            assert False, 'push_status_message() should have re-raised the RuntimeError not gotten ValidationError.'
-        except RuntimeError as e:
-            assert str(e) == exception_message, (
-                'push_status_message() should have re-raised the '
-                'original RuntimeError with the original message.'
-            )
-
-        except BaseException:
-            assert False, (
-                'Unexpected Exception from push_status_message when called '
-                'from the v2 API with type "error"'
-            )
-
 
 @pytest.mark.django_db
 class TestCoerceGuid:

api_tests/files/views/test_file_detail.py

@@ -121,10 +121,8 @@ def test_file_with_wrong_guid(self, app, user):
     def test_file_guid_not_created_with_basic_auth(
             self, mock_allow, app, user, file_url):
         res = app.get(f'{file_url}?create_guid=1', auth=user.auth)
-        guid = res.json['data']['attributes'].get('guid', None)
         assert res.status_code == 200
         assert mock_allow.call_count == 1
-        assert guid is None
 
     @mock.patch('api.base.throttling.CreateGuidThrottle.allow_request')
     def test_file_guid_created_with_cookie(

api_tests/metadata_records/test_custom_item_metadata.py

@@ -351,6 +351,7 @@ def test_with_write_permission(self, app, public_osfguid, private_osfguid, anybo
                     resource_type_general='book-chapter',
                 ),
                 auth=anybody_with_write_permission.auth,
+                expect_errors=True,
             )
             assert res.status_code == 400
             expected.assert_expectations(db_record=db_record, api_record=None)  # db unchanged

api_tests/nodes/views/test_node_contributors_detail_update.py

@@ -32,6 +32,21 @@ def project(self, user, contrib):
         )
         return project
 
+    @pytest.fixture()
+    def component_project_and_parent_project(self, user, contrib):
+        # both need to be in same method to keep root same as parent
+        parent_project = ProjectFactory(creator=user)
+        parent_project.add_contributor(
+            contrib,
+            permissions=permissions.WRITE,
+            visible=True,
+            save=True
+        )
+        component_project = ProjectFactory(creator=user)
+        component_project.root = parent_project
+        component_project.save()
+        return component_project, parent_project
+
     @pytest.fixture()
     def url_creator(self, user, project):
         return f'/{API_BASE}nodes/{project._id}/contributors/{user._id}/'
@@ -414,3 +429,22 @@ def test_change_admin_self_with_other_admin(self, app, user, contrib, project, u
             attributes = res.json['data']['attributes']
             assert attributes['permission'] == permissions.WRITE
             assert project.get_permissions(user) == [permissions.READ, permissions.WRITE]
+
+    def test_update_component_project_with_parent_contributors(self, app, user, component_project_and_parent_project):
+        def exists_unique_parent_contributors(parent_project, component_project):
+            return parent_project.contributors.exclude(
+                id__in=component_project.contributors.values_list('id', flat=True)
+            ).exists()
+        component_project, parent_project = component_project_and_parent_project
+        assert exists_unique_parent_contributors(parent_project, component_project)
+        res = app.patch_json_api(
+            f'/{API_BASE}nodes/{component_project._id}/contributors/?copy_contributors_from_parent_project=true',
+            {
+                'data': {
+                    'type': 3
+                }
+            },
+            auth=user.auth
+        )
+        assert res.status_code == 200
+        assert not exists_unique_parent_contributors(parent_project, component_project)

osf/models/user.py

@@ -1693,7 +1693,7 @@ def get_claim_url(self, project_id, external=False):
         base_url = website_settings.DOMAIN if external else '/'
         unclaimed_record = self.get_unclaimed_record(project_id)
         token = unclaimed_record['token']
-        return f'{base_url}user/{uid}/{project_id}/claim/?token={token}'
+        return f'{base_url}legacy/user/{uid}/{project_id}/claim/?token={token}'
 
     def is_affiliated_with_institution(self, institution):
         """Return if this user is affiliated with the given ``institution``."""

osf_tests/test_user.py

@@ -1165,7 +1165,7 @@ def test_get_claim_url(self, unreg_user, unreg_moderator, project, provider):
         domain = settings.DOMAIN
         assert (
             unreg_user.get_claim_url(pid, external=True) ==
-            f'{domain}user/{uid}/{pid}/claim/?token={token}'
+            f'{domain}legacy/user/{uid}/{pid}/claim/?token={token}'
         )
 
         # test_unreg_moderator
@@ -1175,7 +1175,7 @@ def test_get_claim_url(self, unreg_user, unreg_moderator, project, provider):
         domain = settings.DOMAIN
         assert (
             unreg_moderator.get_claim_url(pid, external=True) ==
-            f'{domain}user/{uid}/{pid}/claim/?token={token}'
+            f'{domain}legacy/user/{uid}/{pid}/claim/?token={token}'
         )
 
     def test_get_claim_url_raises_value_error_if_not_valid_pid(self, unreg_user, unreg_moderator):

tasks/__init__.py

@@ -378,7 +378,6 @@ def test_module(ctx, module=None, numprocesses=None, nocapture=False, params=Non
     'api_tests/guids',
     'api_tests/meetings',
     'api_tests/metadata_records',
-    'api_tests/osf_groups',
     'api_tests/reviews',
     'api_tests/regions',
     'api_tests/search',