Skip to content

Commit

Permalink
Merge branch 'fix-untitled-projects' of https://github.com/johnetordo…
Browse files Browse the repository at this point in the history
…ff/osf.io into fix-default-affiliated-insitution

* 'fix-untitled-projects' of https://github.com/johnetordoff/osf.io:
  update docstrings for copying resource code
  fix excluded no project attributes for Draft Registration
  ensure registration don't are accidentally created with DEFAULT_DRAFT_NODE_TITLE titles
  • Loading branch information
John Tordoff committed Oct 13, 2023
2 parents eba70da + 8d2b451 commit 7626a45
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 22 deletions.
1 change: 1 addition & 0 deletions api_tests/draft_nodes/views/test_draft_node_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()])
Expand Down
6 changes: 6 additions & 0 deletions api_tests/registrations/views/test_registration_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion osf/models/draft_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
33 changes: 20 additions & 13 deletions osf/models/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion osf/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
11 changes: 9 additions & 2 deletions osf/models/registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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':
Expand Down
5 changes: 3 additions & 2 deletions osf_tests/test_draft_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
get_default_metaschema,
)
from website.project.signals import after_create_registration
from website import settings

pytestmark = pytest.mark.django_db

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion osf_tests/test_draft_registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions website/settings/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

0 comments on commit 7626a45

Please sign in to comment.