From f1af4adfcab87d9d2f1ae6ef59a4fa70ad9981ec Mon Sep 17 00:00:00 2001 From: John Tordoff Date: Tue, 17 Oct 2023 16:01:06 -0400 Subject: [PATCH] [ENG-4560] Remove 'Untitled' as the default entry from the Title entry field for the draft registration (#10454) --------- Co-authored-by: John Tordoff <> --- .../views/test_draft_node_detail.py | 1 + .../views/test_draft_registration_detail.py | 2 +- .../views/test_draft_registration_list.py | 2 +- .../views/test_registration_list.py | 6 ++++ osf/models/draft_node.py | 2 +- osf/models/mixins.py | 33 +++++++++++-------- osf/models/node.py | 2 +- osf/models/registrations.py | 11 +++++-- osf_tests/test_draft_node.py | 5 +-- osf_tests/test_draft_registration.py | 2 +- website/settings/defaults.py | 1 + 11 files changed, 45 insertions(+), 22 deletions(-) diff --git a/api_tests/draft_nodes/views/test_draft_node_detail.py b/api_tests/draft_nodes/views/test_draft_node_detail.py index 5bbbfa5b388a..11369073813d 100644 --- a/api_tests/draft_nodes/views/test_draft_node_detail.py +++ b/api_tests/draft_nodes/views/test_draft_node_detail.py @@ -55,6 +55,7 @@ def test_detail_response(self, app, user, user_two): assert res.status_code == 404 # cannot access draft node after it's been registered (it's now a node!) + draft_reg.title = 'test user generated title.' draft_reg.register(Auth(user)) url = '/{}draft_nodes/{}/'.format(API_BASE, draft_node._id) res = app.get(url, auth=user_two.auth, expect_errors=True) diff --git a/api_tests/draft_registrations/views/test_draft_registration_detail.py b/api_tests/draft_registrations/views/test_draft_registration_detail.py index d7e7626ac132..54ce2250932b 100644 --- a/api_tests/draft_registrations/views/test_draft_registration_detail.py +++ b/api_tests/draft_registrations/views/test_draft_registration_detail.py @@ -95,7 +95,7 @@ def test_detail_view_returns_editable_fields_no_specified_node(self, app, user): res = app.get(url, auth=user.auth, expect_errors=True) attributes = res.json['data']['attributes'] - assert attributes['title'] == 'Untitled' + assert attributes['title'] == '' assert attributes['description'] == '' assert attributes['category'] == '' assert attributes['node_license'] is None diff --git a/api_tests/draft_registrations/views/test_draft_registration_list.py b/api_tests/draft_registrations/views/test_draft_registration_list.py index 509e8daacc2a..c317d8836fcf 100644 --- a/api_tests/draft_registrations/views/test_draft_registration_list.py +++ b/api_tests/draft_registrations/views/test_draft_registration_list.py @@ -394,7 +394,7 @@ def test_draft_registration_attributes_not_copied_from_node(self, app, project_p res = app.post_json_api(url_draft_registrations, payload, auth=user.auth) assert res.status_code == 201 attributes = res.json['data']['attributes'] - assert attributes['title'] == 'Untitled' + assert attributes['title'] == '' assert attributes['description'] != project_public.description assert attributes['category'] != project_public.category assert set(attributes['tags']) != set([tag.name for tag in project_public.tags.all()]) diff --git a/api_tests/registrations/views/test_registration_list.py b/api_tests/registrations/views/test_registration_list.py index 601152b550bb..bac1cf78e2fd 100644 --- a/api_tests/registrations/views/test_registration_list.py +++ b/api_tests/registrations/views/test_registration_list.py @@ -1570,6 +1570,9 @@ def test_need_admin_perms_on_draft( payload_ver['data']['attributes']['draft_registration_id'] = draft_registration._id assert draft_registration.branched_from.is_admin_contributor(user) is False assert draft_registration.has_permission(user, permissions.ADMIN) is True + assert draft_registration.title is '' + draft_registration.title = 'test user generated title required' + draft_registration.save() res = app.post_json_api(url_registrations_ver, payload_ver, auth=user.auth) assert res.status_code == 201 @@ -1578,6 +1581,9 @@ def test_need_admin_perms_on_draft( assert draft_registration.branched_from.is_admin_contributor(user) is True assert draft_registration.has_permission(user, permissions.ADMIN) is True payload_ver['data']['attributes']['draft_registration_id'] = draft_registration._id + assert draft_registration.title is '' + draft_registration.title = 'test user generated title required' + draft_registration.save() res = app.post_json_api(url_registrations_ver, payload_ver, auth=user.auth) assert res.status_code == 201 diff --git a/osf/models/draft_node.py b/osf/models/draft_node.py index a64b216c040b..fec0bfc346e1 100644 --- a/osf/models/draft_node.py +++ b/osf/models/draft_node.py @@ -66,7 +66,7 @@ def register_node(self, schema, auth, draft_registration, parent=None, child_ids """ self.convert_draft_node_to_node(auth) # Copies editable fields from the DraftRegistration back to the Node - self.copy_editable_fields(draft_registration, auth=auth, save=True) + self.copy_editable_fields(draft_registration, save=True) # Calls super on Node, since self is no longer a DraftNode return super(Node, self).register_node(schema, auth, draft_registration, parent, child_ids, provider) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index daab45c8a531..b381b14913cb 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -2261,19 +2261,26 @@ 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): - """ - 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 - """ - 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) + def copy_editable_fields(self, resource, alternative_resource=None, include_contributors=True, save=True, excluded_attributes=None): + """ + This method copies 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. + + :param Object resource: Primary resource where you want to copy attributes + :param Object alternative_resource: Backup resource for copying attributes + :param Boolean include_contributors: represents whether to also copy the resource's contributors + :param Boolean save: represents whether to save the resources changes immediately + :param List: a list of strings representing attributes to exclude from copying + """ + if not excluded_attributes: + excluded_attributes = [] + + for attribute in ['title', 'description', 'category', 'node_license']: + if attribute not in excluded_attributes: + self.set_editable_attribute(attribute, resource, alternative_resource) if include_contributors: # Contributors will always come from "resource", as contributor constraints diff --git a/osf/models/node.py b/osf/models/node.py index 2266c20d06b5..0869e019ff22 100644 --- a/osf/models/node.py +++ b/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: diff --git a/osf/models/registrations.py b/osf/models/registrations.py index acf50e6d385d..ceff62c98a2b 100644 --- a/osf/models/registrations.py +++ b/osf/models/registrations.py @@ -1261,9 +1261,12 @@ def create_from_node(cls, user, schema, node=None, data=None, provider=None): else: provider.validate_schema(schema) + excluded_attributes = [] if not node: # If no node provided, a DraftNode is created for you - node = DraftNode.objects.create(creator=user, title='Untitled') + node = DraftNode.objects.create(creator=user, title=settings.DEFAULT_DRAFT_NODE_TITLE) + # Force the user to add their own title for no-project + excluded_attributes.append('title') if not (isinstance(node, Node) or isinstance(node, DraftNode)): raise DraftRegistrationStateError() @@ -1276,7 +1279,11 @@ def create_from_node(cls, user, schema, node=None, data=None, provider=None): provider=provider, ) draft.save() - draft.copy_editable_fields(node, Auth(user), save=True) + draft.copy_editable_fields( + node, + save=True, + excluded_attributes=excluded_attributes + ) draft.update(data, auth=Auth(user)) if node.type == 'osf.draftnode': diff --git a/osf_tests/test_draft_node.py b/osf_tests/test_draft_node.py index e8d2ef143244..288fb4c4dd98 100644 --- a/osf_tests/test_draft_node.py +++ b/osf_tests/test_draft_node.py @@ -22,6 +22,7 @@ get_default_metaschema, ) from website.project.signals import after_create_registration +from website import settings pytestmark = pytest.mark.django_db @@ -123,8 +124,8 @@ def test_create_draft_registration_without_node(self, user): schema=get_default_metaschema(), data=data, ) - assert draft.title == 'Untitled' - assert draft.branched_from.title == 'Untitled' + assert draft.title == '' + assert draft.branched_from.title == settings.DEFAULT_DRAFT_NODE_TITLE assert draft.branched_from.type == 'osf.draftnode' assert draft.branched_from.creator == user assert len(draft.logs.all()) == 0 diff --git a/osf_tests/test_draft_registration.py b/osf_tests/test_draft_registration.py index b6133c4e32ab..2527df493e76 100644 --- a/osf_tests/test_draft_registration.py +++ b/osf_tests/test_draft_registration.py @@ -269,7 +269,7 @@ def test_create_from_node_draft_node(self, user): schema=factories.get_default_metaschema(), ) - assert draft.title == 'Untitled' + assert draft.title == '' assert draft.description == '' assert draft.category == '' assert user in draft.contributors.all() diff --git a/website/settings/defaults.py b/website/settings/defaults.py index 1438e90e07bb..7c67bf55e901 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -2129,3 +2129,4 @@ def from_node_usage(cls, usage_bytes, private_limit=None, public_limit=None): PREPRINT_METRICS_START_DATE = datetime.datetime(2019, 1, 1) WAFFLE_VALUES_YAML = 'osf/features.yaml' +DEFAULT_DRAFT_NODE_TITLE = 'Untitled'