Skip to content

Commit

Permalink
Remove constraint of one preprint per provider per node.
Browse files Browse the repository at this point in the history
- Nodes can hold supplemental material for multiple preprints.
- Remove migration which added a partial index on one preprint per provider per node, provided node existed.
- Project page now can display all the preprints for which the node holds supplemental material, provided the logged in user has permission to view those preprints.
- Removes a lot of now unused preprint-related node properties
- Email sent to contributors when a user is added to a node that is linked to a preprint lists all the published preprints the node holds supplemental information for.
- Remove preprint_url from relations_meta metadata tags since the node has been separated from the preprint.
- "preprint" property on the node serializer is true if the node is a supplemental node for a preprint the user can view.
  • Loading branch information
pattisdr committed Jul 28, 2018
1 parent d84be3c commit 446ec83
Show file tree
Hide file tree
Showing 20 changed files with 93 additions and 142 deletions.
7 changes: 6 additions & 1 deletion api/nodes/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class NodeSerializer(TaxonomizableSerializerMixin, JSONAPISerializer):
date_created = VersionedDateTimeField(source='created', read_only=True)
date_modified = VersionedDateTimeField(source='last_logged', read_only=True)
registration = ser.BooleanField(read_only=True, source='is_registration')
preprint = ser.BooleanField(read_only=True, source='has_published_preprint')
preprint = ser.SerializerMethodField()
fork = ser.BooleanField(read_only=True, source='is_fork')
collection = ser.BooleanField(read_only=True, source='is_collection')
tags = ValuesListField(attr_name='name', child=ser.CharField(), required=False)
Expand Down Expand Up @@ -400,6 +400,11 @@ def get_current_user_can_comment(self, obj):
auth = Auth(user if not user.is_anonymous else None)
return obj.can_comment(auth)

def get_preprint(self, obj):
# Whether the node has supplemental material for a preprint the user can view
user = self.context['request'].user if not self.context['request'].user.is_anonymous else None
return Preprint.objects.can_view(base_queryset=obj.preprints, user=user).exists()

class Meta:
type_ = 'nodes'

Expand Down
4 changes: 0 additions & 4 deletions api/preprints/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from api.taxonomies.serializers import TaxonomizableSerializerMixin
from framework.exceptions import PermissionsError
from website.exceptions import NodeStateError
from osf.exceptions import PreprintProviderError
from website.project import signals as project_signals
from osf.models import BaseFileNode, Preprint, PreprintProvider, Node, NodeLicense
from osf.utils import permissions as osf_permissions
Expand Down Expand Up @@ -290,9 +289,6 @@ def set_field(self, func, val, auth, save=False):
raise exceptions.PermissionDenied(detail=e.message)
except (ValueError, ValidationError, NodeStateError) as e:
raise exceptions.ValidationError(detail=e.message)
except PreprintProviderError as e:
if 'Only one preprint per provider can be submitted for a node' in e.message:
raise Conflict(e.message)


class PreprintCreateSerializer(PreprintSerializer):
Expand Down
16 changes: 15 additions & 1 deletion api_tests/nodes/views/test_node_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,17 +310,31 @@ def test_node_shows_wiki_relationship_based_on_disabled_status_and_version(self,
res = app.get(url, auth=user.auth)
assert 'wikis' in res.json['data']['relationships']

def test_preprint_field(self, app, user, project_public, url_public):
def test_preprint_field(self, app, user, user_two, project_public, url_public):
# Returns true if project holds supplemental material for a preprint a user can view
# Published preprint, admin_contrib
preprint = PreprintFactory(project=project_public, creator=user)
res = app.get(url_public, auth=user.auth)
assert res.status_code == 200
assert res.json['data']['attributes']['preprint'] is True

# Published preprint, non_contrib
res = app.get(url_public, auth=user_two.auth)
assert res.status_code == 200
assert res.json['data']['attributes']['preprint'] is True

# Unpublished preprint, admin contrib
preprint.is_published = False
preprint.save()
res = app.get(url_public, auth=user.auth)
assert res.status_code == 200
assert res.json['data']['attributes']['preprint'] is True

# Unpublished preprint, non_contrib
res = app.get(url_public, auth=user_two.auth)
assert res.status_code == 200
assert res.json['data']['attributes']['preprint'] is False

def test_node_shows_correct_templated_from_count(self, app, user, project_public, url_public):
url = url_public
res = app.get(url)
Expand Down
5 changes: 2 additions & 3 deletions api_tests/preprints/views/test_preprint_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,9 @@ def test_update_node_existing_preprint(self, app, user, preprint, url):
)

res = app.patch_json_api(url, update_node_payload, auth=user.auth, expect_errors=True)
assert res.status_code == 409
assert 'Only one preprint per provider can be submitted for a node' in res.json['errors'][0]['detail']
assert res.status_code == 200
preprint.reload()
assert preprint.node is None
assert preprint.node == node

def test_update_deleted_node(self, app, user, preprint, url):
assert preprint.node is None
Expand Down
13 changes: 5 additions & 8 deletions api_tests/preprints/views/test_preprint_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ def test_file_is_not_in_node(self):
res.json['errors'][0]['detail'],
'This file is not a valid primary file for this preprint.')

def test_already_a_preprint_with_conflicting_provider(self):
def test_already_has_supplemental_node_on_another_preprint(self):
preprint = PreprintFactory(creator=self.user, project=self.public_project)
already_preprint_payload = build_preprint_create_payload(
preprint.node._id, preprint.provider._id)
Expand All @@ -494,13 +494,10 @@ def test_already_a_preprint_with_conflicting_provider(self):
already_preprint_payload,
auth=self.user.auth,
expect_errors=True)
# One preprint per provider per node constraint has been lifted
assert_equal(res.status_code, 201)

assert_equal(res.status_code, 409)
assert_in(
'Only one preprint per provider can be submitted for a node.',
res.json['errors'][0]['detail'])

def test_read_write_user_already_a_preprint_with_conflicting_provider(
def test_read_write_user_already_a_preprint_with_same_provider(
self):
assert_in(self.other_user, self.public_project.contributors)

Expand All @@ -513,7 +510,7 @@ def test_read_write_user_already_a_preprint_with_conflicting_provider(
auth=self.other_user.auth,
expect_errors=True)

assert_equal(res.status_code, 409)
assert_equal(res.status_code, 201)

def test_publish_preprint_fails_with_no_primary_file(self):
no_file_payload = build_preprint_create_payload(
Expand Down
26 changes: 0 additions & 26 deletions osf/migrations/0119_add_preprint_partial_index.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def divorce_preprints_from_nodes_sql(state, schema):
class Migration(migrations.Migration):

dependencies = [
('osf', '0119_add_preprint_partial_index'),
('osf', '0118_update_preprint_model_for_divorce'),
]

operations = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def migrate_preprint_service_permissions(state, schema):
class Migration(migrations.Migration):

dependencies = [
('osf', '0120_preprint_node_divorce'),
('osf', '0119_preprint_node_divorce'),
]

operations = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class Migration(migrations.Migration):

dependencies = [
('osf', '0121_transfer_preprint_service_permissions'),
('osf', '0120_transfer_preprint_service_permissions'),
]

operations = [
Expand Down
35 changes: 3 additions & 32 deletions osf/models/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
from osf.utils.fields import NonNaiveDateTimeField
from osf.utils.requests import DummyRequest, get_request_and_user_id
from osf.utils import sanitize
from osf.utils.workflows import DefaultStates
from website import language, settings
from website.citations.utils import datetime_to_csl
from website.exceptions import (InvalidTagError, NodeStateError,
Expand Down Expand Up @@ -448,38 +447,10 @@ def collecting_metadata_list(self):
return list(self.collecting_metadata_qs)

@property
def has_submitted_preprint(self):
# Attached to a submitted preprint
return self.preprints.filter(
deleted__isnull=True).filter(Q(date_withdrawn__isnull=True) | Q(ever_public=True)).exclude(machine_state=DefaultStates.INITIAL.value).exists()

@property
def has_published_preprint(self):
# Attached to a published preprint - anyone can view this preprint
return self.published_preprints_queryset.exists()

@property
def published_preprints_queryset(self):
def has_linked_published_preprints(self):
# Node holds supplemental material for published preprint(s)
Preprint = apps.get_model('osf.Preprint')
return self.preprints.filter(Preprint.objects.no_user_query)

@property
def preprint_url(self):
node_linked_preprint = self.linked_preprint
if node_linked_preprint:
return node_linked_preprint.url

@property
def linked_preprint(self):
try:
# if multiple preprints per project are supported on the front end this needs to change.
published_preprint = self.published_preprints_queryset.first()
if published_preprint:
return published_preprint
else:
return self.preprints.get_queryset()[0]
except IndexError:
pass
return self.preprints.filter(Preprint.objects.no_user_query).exists()

@property
def is_collection(self):
Expand Down
6 changes: 1 addition & 5 deletions osf/models/preprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

from framework.sentry import log_exception
from osf.exceptions import (
PreprintStateError, InvalidTagError, TagNotFoundError, PreprintProviderError
PreprintStateError, InvalidTagError, TagNotFoundError
)

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -695,10 +695,6 @@ def set_supplemental_node(self, node, auth, save=False):
if not node.has_permission(auth.user, 'write'):
raise PermissionsError('You must have write permissions on the supplemental node to attach.')

node_preprints = node.preprints.filter(provider=self.provider)
if node_preprints.exists():
raise PreprintProviderError('Only one preprint per provider can be submitted for a node. Check preprint `{}`.'.format(node_preprints.first()._id))

if node.is_deleted:
raise ValueError('Cannot attach a deleted project to a preprint.')

Expand Down
15 changes: 5 additions & 10 deletions osf_tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -4205,31 +4205,26 @@ def test_private_link(self, jane_doe, project, lvl1component):


class TestNodeProperties:
def test_has_published_preprint(self, project, preprint, user):
def test_has_linked_published_preprints(self, project, preprint, user):
# If no preprints, is False
assert project.has_published_preprint is False
assert project.has_linked_published_preprints is False

# A published preprint attached to a project is True
preprint.node = project
preprint.save()
assert project.has_published_preprint is True
assert project.has_linked_published_preprints is True

# Abandoned preprint is False
preprint.machine_state = DefaultStates.INITIAL.value
preprint.save()
assert project.has_published_preprint is False
assert project.has_linked_published_preprints is False

# Unpublished preprint is False
preprint.machine_state = DefaultStates.ACCEPTED.value
preprint.is_published = False
preprint.save()
assert project.has_published_preprint is False
assert project.has_linked_published_preprints is False

def test_preprint_url_does_not_return_unpublished_preprint_url(self):
node = ProjectFactory(is_public=True)
published = PreprintFactory(project=node, is_published=True, filename='file1.txt')
PreprintFactory(project=node, is_published=False, filename='file2.txt')
assert node.preprint_url == published.url

class TestCollectionProperties:

Expand Down
8 changes: 4 additions & 4 deletions tests/test_preprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -1387,10 +1387,10 @@ def test_set_supplemental_node_deleted(self):
self.preprint.set_supplemental_node(project, auth=self.auth, save=True)

def test_set_supplemental_node_already_has_a_preprint(self):
with assert_raises(PreprintProviderError):
project_two = ProjectFactory(creator=self.preprint.creator)
preprint = PreprintFactory(project=project_two, provider=self.preprint.provider)
self.preprint.set_supplemental_node(project_two, auth=self.auth, save=True)
project_two = ProjectFactory(creator=self.preprint.creator)
preprint = PreprintFactory(project=project_two, provider=self.preprint.provider)
self.preprint.set_supplemental_node(project_two, auth=self.auth, save=True)
assert project_two.preprints.count() == 2

def test_preprint_made_public(self):
# Testing for migrated preprints, that may have had is_public = False
Expand Down
4 changes: 3 additions & 1 deletion tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2029,7 +2029,9 @@ def test_email_sent_when_reg_user_is_added(self, send_mail):
branded_service=None,
can_change_preferences=False,
logo=settings.OSF_LOGO,
osf_contact_email=settings.OSF_CONTACT_EMAIL
osf_contact_email=settings.OSF_CONTACT_EMAIL,
published_preprints=[]

)
assert_almost_equal(contributor.contributor_added_email_records[project._id]['last_sent'], int(time.time()), delta=1)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_webtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,7 @@ def test_public_project_abandoned_preprint(self):

# Admin - preprint
res = self.app.get(url, auth=self.admin.auth)
assert_not_in('Has supplemental material for', res.body)
assert_in('Has supplemental material for', res.body)

# Write - preprint
res = self.app.get(url, auth=self.write_contrib.auth)
Expand Down
6 changes: 4 additions & 2 deletions website/project/views/contributor.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from website.profile import utils as profile_utils
from website.project.decorators import (must_have_permission, must_be_valid_project, must_not_be_registration,
must_be_contributor_or_public, must_be_contributor)
from website.project.views.node import serialize_preprints
from website.project.model import has_anonymous_link
from website.project.signals import unreg_contributor_added, contributor_added
from website.util import web_url_for, is_json_request
Expand Down Expand Up @@ -555,7 +556,7 @@ def notify_added_contributor(node, contributor, auth=None, throttle=None, email_
elif email_template == 'access_request':
mimetype = 'html'
email_template = getattr(mails, 'CONTRIBUTOR_ADDED_ACCESS_REQUEST'.format(email_template.upper()))
elif node.has_published_preprint:
elif node.has_linked_published_preprints:
# Project holds supplemental materials for a published preprint
email_template = getattr(mails, 'CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF'.format(email_template.upper()))
logo = settings.OSF_PREPRINTS_LOGO
Expand All @@ -582,7 +583,8 @@ def notify_added_contributor(node, contributor, auth=None, throttle=None, email_
branded_service=preprint_provider,
can_change_preferences=False,
logo=logo if logo else settings.OSF_LOGO,
osf_contact_email=settings.OSF_CONTACT_EMAIL
osf_contact_email=settings.OSF_CONTACT_EMAIL,
published_preprints=[] if isinstance(node, Preprint) else serialize_preprints(node, user=None)
)

contributor.contributor_added_email_records[node._id]['last_sent'] = get_timestamp()
Expand Down
Loading

0 comments on commit 446ec83

Please sign in to comment.