fix draft registrations affiliated institution default value for no-p…

…roject registrations and clean-up code

api/nodes/serializers.py

@@ -1557,12 +1557,6 @@ class DraftRegistrationLegacySerializer(JSONAPISerializer):
         'html': 'get_absolute_url',
     })
 
-    affiliate_user_institutions = ser.BooleanField(
-        required=False,
-        default=True,
-        help_text='Specify whether user institution affiliations should be copied over to the draft registration.',
-    )
-
     def get_absolute_url(self, obj):
         return obj.absolute_url
 
@@ -1603,7 +1597,6 @@ def create(self, validated_data):
         registration_responses = validated_data.pop('registration_responses', None)
         schema = validated_data.pop('registration_schema')
         provider = validated_data.pop('provider', None)
-        affiliate_user_institutions = validated_data.pop('affiliate_user_institutions', True)
 
         self.enforce_metadata_or_registration_responses(metadata, registration_responses)
 
@@ -1618,9 +1611,6 @@ def create(self, validated_data):
         if registration_responses:
             self.update_registration_responses(draft, registration_responses)
 
-        if affiliate_user_institutions and draft.branched_from_type == DraftNode:
-            draft.affiliated_institutions.set(draft.creator.affiliated_institutions.all())
-
         return draft
 
     class Meta:

api_tests/draft_registrations/views/test_draft_registration_list.py

@@ -260,27 +260,65 @@ def test_create_project_based_draft_does_not_email_initiator(
 
         assert not mock_send_mail.called
 
-    def test_affiliated_institutions_are_copied_from_user(
-            self, app, user, url_draft_registrations, payload):
+    def test_affiliated_institutions_are_copied_from_node_no_institutions(self, app, user, url_draft_registrations, payload):
+        """
+        Draft registrations that are based on projects get those project's user institutional affiliation,
+        those "no-project" registrations inherit the user's institutional affiliation.
+
+        This tests a scenario where a user bases a registration on a node without affiliations, and so the
+        draft registration has no institutional affiliation from the user or the node.
+        """
         project = ProjectFactory(is_public=True, creator=user)
-        InstitutionFactory()
         payload['data']['relationships']['branched_from']['data']['id'] = project._id
+
         res = app.post_json_api(
-            url_draft_registrations, payload,
-            auth=user.auth, expect_errors=True)
+            url_draft_registrations,
+            payload,
+            auth=user.auth,
+        )
         assert res.status_code == 201
         draft_registration = DraftRegistration.load(res.json['data']['id'])
         assert not draft_registration.affiliated_institutions.exists()
 
+    def test_affiliated_institutions_are_copied_from_node(self, app, user, url_draft_registrations, payload):
+        """
+        Draft registrations that are based on projects get those project's user institutional affiliation,
+        those "no-project" registrations inherit the user's institutional affiliation.
+
+        This tests a scenario where a user bases their registration on a project that has a current institutional
+        affiliation which is copied over to the draft registrations.
+        """
+        institution = InstitutionFactory()
+
         project = ProjectFactory(is_public=True, creator=user)
+        project.affiliated_institutions.add(institution)
         payload['data']['relationships']['branched_from']['data']['id'] = project._id
-        user.add_multiple_institutions_non_sso(Institution.objects.filter(id__lt=3))
         res = app.post_json_api(
-            url_draft_registrations, payload,
-            auth=user.auth, expect_errors=True)
+            url_draft_registrations,
+            payload,
+            auth=user.auth,
+        )
+        assert res.status_code == 201
+        draft_registration = DraftRegistration.load(res.json['data']['id'])
+        assert list(draft_registration.affiliated_institutions.all()) == list(project.affiliated_institutions.all())
+
+    def test_affiliated_institutions_are_copied_from_user(self, app, user, url_draft_registrations, payload):
+        """
+        Draft registrations that are based on projects get those project's user institutional affiliation,
+        those "no-project" registrations inherit the user's institutional affiliation.
+        """
+        institution = InstitutionFactory()
+        user.add_or_update_affiliated_institution(institution)
+
+        del payload['data']['relationships']['branched_from']
+        res = app.post_json_api(
+            url_draft_registrations,
+            payload,
+            auth=user.auth,
+        )
         assert res.status_code == 201
         draft_registration = DraftRegistration.load(res.json['data']['id'])
-        assert not draft_registration.affiliated_institutions.all() == user.get_affiliated_institutions()
+        assert list(draft_registration.affiliated_institutions.all()) == list(user.get_affiliated_institutions())
 
 
 class TestDraftRegistrationCreateWithoutNode(TestDraftRegistrationCreate):

osf/models/mixins.py

@@ -2261,33 +2261,44 @@ def stage_m2m_values(self, fieldname, resource, alternative_resource=None):
         else:
             return []
 
-    def copy_editable_fields(self, resource, auth=None, alternative_resource=None, include_contributors=True, save=True):
+    def copy_editable_fields(
+            self, resource, alternative_resource=None, include_contributors=True, save=True, excluded_attributes=None
+    ):
         """
-        Copy various editable fields from the 'resource' object to the current object.
-        Includes, title, description, category, contributors, node_license, tags, subjects, and affiliated_institutions
-        The field on the resource will always supersede the field on the alternative_resource. For example,
-        copying fields from the draft_registration to the registration.  resource will be a DraftRegistration object,
-        but the alternative_resource will be a Node.  DraftRegistration fields will trump Node fields.
-        TODO, add optional logging parameter
+        Copy editable fields from 'resource' to the current object.
+
+        :param Object resource: Primary resource to copy attributes from
+        :param Object alternative_resource: Backup resource for copying attributes
+        :param bool include_contributors: Whether to also copy contributors
+        :param bool save: Whether to save the changes immediately
+        :param List excluded_attributes: List of attributes to exclude from copying
         """
-        self.set_editable_attribute('title', resource, alternative_resource)
-        self.set_editable_attribute('description', resource, alternative_resource)
-        self.set_editable_attribute('category', resource, alternative_resource)
-        self.set_editable_attribute('node_license', resource, alternative_resource)
+        if not excluded_attributes:
+            excluded_attributes = []
+
+        self._copy_basic_attributes(resource, alternative_resource, excluded_attributes)
 
         if include_contributors:
-            # Contributors will always come from "resource", as contributor constraints
-            # will require contributors to be present on the resource
             self.copy_contributors_from(resource)
-            # Copy unclaimed records for unregistered users
             self.copy_unclaimed_records(resource)
 
-        self.tags.add(*self.stage_m2m_values('all_tags', resource, alternative_resource))
-        self.subjects.add(*self.stage_m2m_values('subjects', resource, alternative_resource))
-        self.affiliated_institutions.add(*self.stage_m2m_values('affiliated_institutions', resource, alternative_resource))
+        self._copy_m2m_fields(resource, alternative_resource, excluded_attributes)
 
         if save:
             self.save()
 
+    def _copy_basic_attributes(self, resource, alternative_resource, excluded_attributes):
+        # Copy basic attributes such as title, description, category, and node_license
+        for attribute in ['title', 'description', 'category', 'node_license']:
+            if attribute not in excluded_attributes:
+                self.set_editable_attribute(attribute, resource, alternative_resource)
+
+    def _copy_m2m_fields(self, resource, alternative_resource, excluded_attributes):
+        # Copy m2m fields like tags, subjects, and affiliated_institutions
+        m2m_fields = ['tags', 'subjects', 'affiliated_institutions']
+        for field in m2m_fields:
+            if field not in excluded_attributes:
+                getattr(self, field).add(*self.stage_m2m_values(field, resource, alternative_resource))
+
     class Meta:
         abstract = True

osf/models/node.py

@@ -1488,7 +1488,7 @@ def register_node(self, schema, auth, draft_registration, parent=None, child_ids
             resource = self
             alternative_resource = None
 
-        registered.copy_editable_fields(resource, auth=auth, alternative_resource=alternative_resource)
+        registered.copy_editable_fields(resource, alternative_resource=alternative_resource)
         registered.copy_registration_responses_into_schema_response(draft_registration)
 
         if settings.ENABLE_ARCHIVER:

osf/models/registrations.py

@@ -1261,25 +1261,35 @@ def create_from_node(cls, user, schema, node=None, data=None, provider=None):
         else:
             provider.validate_schema(schema)
 
-        if not node:
-            # If no node provided, a DraftNode is created for you
-            node = DraftNode.objects.create(creator=user, title='Untitled')
+        excluded_attributes = []
+
+        if node:
+            branched_from = node
+        else:
+            branched_from = DraftNode.objects.create(creator=user, title='Untitled')
+            excluded_attributes = ['affiliated_institutions']
 
-        if not (isinstance(node, Node) or isinstance(node, DraftNode)):
+        if not isinstance(branched_from, (Node, DraftNode)):
             raise DraftRegistrationStateError()
 
         draft = cls(
             initiator=user,
-            branched_from=node,
+            branched_from=branched_from,
             registration_schema=schema,
             registration_metadata=data or {},
             provider=provider,
         )
         draft.save()
-        draft.copy_editable_fields(node, Auth(user), save=True)
         draft.update(data, auth=Auth(user))
+        draft.copy_editable_fields(
+            branched_from,
+            Auth(user),
+            save=True,
+            excluded_attributes=excluded_attributes
+        )
 
-        if node.type == 'osf.draftnode':
+        if not node:
+            draft.affiliated_institutions.add(*draft.creator.get_affiliated_institutions())
             initiator_permissions = draft.contributor_set.get(user=user).permission
             signals.contributor_added.send(
                 draft,