From 095a1dbc9edc6c56084fe25b395537bf9daa9aa0 Mon Sep 17 00:00:00 2001
From: Geert Kapteijns <ghkapteijns@gmail.com>
Date: Fri, 12 May 2017 13:08:57 +0200
Subject: [PATCH] Check if DOI belongs to existing resource. Move validation
logic to form.
---
commentaries/forms.py | 24 +++++++++++++++++++++---
commentaries/test_forms.py | 9 +++++++--
commentaries/test_models.py | 12 +++++++++++-
commentaries/test_views.py | 16 ++++++++++++++--
commentaries/views.py | 10 +++++++---
scipost/services.py | 12 +++++++++---
scipost/test_services.py | 6 ++++++
7 files changed, 75 insertions(+), 14 deletions(-)
diff --git a/commentaries/forms.py b/commentaries/forms.py
index 6364e224d..e1f29641d 100644
--- a/commentaries/forms.py
+++ b/commentaries/forms.py
@@ -6,6 +6,7 @@ from django.urls import reverse
from .models import Commentary
+from scipost.services import DOICaller
from scipost.models import Contributor
@@ -16,11 +17,23 @@ class DOIToQueryForm(forms.Form):
def clean_doi(self):
input_doi = self.cleaned_data['doi']
- if Commentary.objects.filter(pub_DOI=input_doi).exists():
+
+ if self.commentary_exists(input_doi):
error_message = 'There already exists a Commentary Page on this publication.'
raise forms.ValidationError(error_message)
+
+ caller = DOICaller(input_doi)
+ if caller.is_valid:
+ self.crossref_data = DOICaller(input_doi).data
+ else:
+ error_message = 'Could not find a resource for that DOI.'
+ raise forms.ValidationError(error_message)
+
return input_doi
+ def commentary_exists(self, input_doi):
+ return Commentary.objects.filter(pub_DOI=input_doi).exists()
+
class IdentifierToQueryForm(forms.Form):
identifier = forms.CharField(widget=forms.TextInput(
@@ -56,18 +69,23 @@ class RequestPublishedArticleForm(forms.ModelForm):
class Meta:
model = Commentary
fields = [
- 'type', 'discipline', 'domain', 'subject_area', 'pub_title', 'author_list', 'metadata', 'journal', 'volume',
+ 'discipline', 'domain', 'subject_area', 'pub_title', 'author_list', 'journal', 'volume',
'pages', 'pub_date', 'pub_DOI', 'pub_abstract'
]
widgets = {
'metadata': forms.HiddenInput(),
- 'type': forms.HiddenInput(),
}
placeholders = {
'pub_DOI': 'ex.: 10.21468/00.000.000000',
'pub_date': 'Format: YYYY-MM-DD',
}
+ def save(self, *args):
+ commentary = super().save(*args)
+ commentary.metadata = self.metadata
+ commentary.save()
+ return commentary
+
class RequestCommentaryForm(forms.ModelForm):
"""Create new valid Commetary by user request"""
diff --git a/commentaries/test_forms.py b/commentaries/test_forms.py
index 964eb0e80..b225d2180 100644
--- a/commentaries/test_forms.py
+++ b/commentaries/test_forms.py
@@ -8,9 +8,9 @@ from scipost.factories import UserFactory
from .factories import VettedCommentaryFactory, UnvettedCommentaryFactory
from .forms import RequestCommentaryForm, VetCommentaryForm, DOIToQueryForm
from .models import Commentary
-from common.helpers import model_form_data
from common.helpers.test import add_groups_and_permissions
+
class TestDOIToQueryForm(TestCase):
def setUp(self):
add_groups_and_permissions()
@@ -43,6 +43,11 @@ class TestDOIToQueryForm(TestCase):
form = DOIToQueryForm({'doi': old_doi})
self.assertTrue(form.is_valid())
+ def test_valid_but_nonexistent_doi_is_invalid(self):
+ doi = "10.21468/NonExistentJournal.2.2.010"
+ form = DOIToQueryForm({'doi': doi})
+ self.assertEqual(form.is_valid(), False)
+
class TestVetCommentaryForm(TestCase):
def setUp(self):
@@ -109,7 +114,7 @@ class TestVetCommentaryForm(TestCase):
class TestRequestCommentaryForm(TestCase):
def setUp(self):
add_groups_and_permissions()
- factory_instance = VettedCommentaryFactory.build()
+ factory_instance = UnvettedCommentaryFactory.build()
self.user = UserFactory()
self.valid_form_data = model_form_data(factory_instance, RequestCommentaryForm)
diff --git a/commentaries/test_models.py b/commentaries/test_models.py
index e4defabc8..5fb96f3ef 100644
--- a/commentaries/test_models.py
+++ b/commentaries/test_models.py
@@ -1 +1,11 @@
-# from django.test import TestCase
+from django.test import TestCase
+
+from common.helpers.test import add_groups_and_permissions
+
+from scipost.factories import ContributorFactory
+from .factories import UnvettedCommentaryFactory
+
+
+class TestCommentary(TestCase):
+ def setUp(self):
+ add_groups_and_permissions()
diff --git a/commentaries/test_views.py b/commentaries/test_views.py
index ca2a6c8e4..96220a2eb 100644
--- a/commentaries/test_views.py
+++ b/commentaries/test_views.py
@@ -5,10 +5,11 @@ from django.test import TestCase, Client, RequestFactory
from scipost.factories import ContributorFactory, UserFactory
from .factories import UnvettedCommentaryFactory, VettedCommentaryFactory, UnpublishedVettedCommentaryFactory
-from .forms import CommentarySearchForm
+from .forms import CommentarySearchForm, RequestPublishedArticleForm
from .models import Commentary
-from .views import RequestCommentary, prefill_using_DOI
+from .views import RequestPublishedArticle, prefill_using_DOI
from common.helpers.test import add_groups_and_permissions
+from common.helpers import model_form_data
class RequestCommentaryTest(TestCase):
@@ -34,6 +35,17 @@ class RequestCommentaryTest(TestCase):
"""Test different kind of invalid RequestCommentaryForm submits"""
raise NotImplementedError
+ def test_saved_commentary_has_a_type(self):
+ self.assertEqual(Commentary.objects.count(), 0)
+ commentary = UnvettedCommentaryFactory.build()
+ valid_post_data = model_form_data(commentary, RequestPublishedArticleForm)
+ print(valid_post_data)
+ request = RequestFactory().post(reverse('commentaries:request_published_article'), valid_post_data)
+ request.user = UserFactory()
+ response = RequestPublishedArticle.as_view()(request)
+
+ self.assertEqual(Commentary.objects.count(), 1)
+
class PrefillUsingDOITest(TestCase):
def setUp(self):
diff --git a/commentaries/views.py b/commentaries/views.py
index aae2fc598..197905b17 100644
--- a/commentaries/views.py
+++ b/commentaries/views.py
@@ -22,7 +22,7 @@ from .forms import RequestCommentaryForm, DOIToQueryForm, IdentifierToQueryForm,
from comments.models import Comment
from comments.forms import CommentForm
from scipost.models import Contributor
-from scipost.services import ArxivCaller, DOICaller
+from scipost.services import ArxivCaller
import strings
@@ -80,6 +80,11 @@ class RequestPublishedArticle(CreateView):
context['doi_query_form'] = DOIToQueryForm()
return context
+ def form_valid(self, form):
+ form.type = "published"
+ return super(RequestPublishedArticle, self).form_valid(form)
+
+
@permission_required('scipost.can_request_commentary_pages', raise_exception=True)
def prefill_using_DOI(request):
if request.method == "POST":
@@ -87,9 +92,8 @@ def prefill_using_DOI(request):
# The form checks if doi is valid and commentary doesn't already exist.
if doi_query_form.is_valid():
doi = doi_query_form.cleaned_data['doi']
- crossref_data = DOICaller(doi).data
additional_form_data = {'type': 'published', 'pub_DOI': doi}
- total_form_data = {**crossref_data, **additional_form_data}
+ total_form_data = {**doi_query_form.crossref_data, **additional_form_data}
form = RequestPublishedArticleForm(initial=total_form_data)
else:
form = RequestPublishedArticleForm()
diff --git a/scipost/services.py b/scipost/services.py
index 78c38e849..47efe58b9 100644
--- a/scipost/services.py
+++ b/scipost/services.py
@@ -14,14 +14,20 @@ class DOICaller:
def __init__(self, doi_string):
self.doi_string = doi_string
self._call_crosslink()
- self._format_data()
+ if self.is_valid:
+ self._format_data()
def _call_crosslink(self):
url = 'http://api.crossref.org/works/%s' % self.doi_string
- self.crossref_data = requests.get(url).json()['message']
+ request = requests.get(url)
+ if request.ok:
+ self.is_valid = True
+ self._crossref_data = requests.get(url).json()['message']
+ else:
+ self.is_valid = False
def _format_data(self):
- data = self.crossref_data
+ data = self._crossref_data
pub_title = data['title'][0]
authorlist = ['{} {}'.format(author['given'], author['family']) for author in data['author']]
journal = data['container-title'][0]
diff --git a/scipost/test_services.py b/scipost/test_services.py
index 120a160da..6794725eb 100644
--- a/scipost/test_services.py
+++ b/scipost/test_services.py
@@ -58,6 +58,7 @@ class DOICallerTest(TestCase):
'volume': '92',
'pub_title': 'Quasi-soliton scattering in quantum spin chains'
}
+ self.assertTrue(caller.is_valid)
self.assertEqual(caller.data, correct_data)
def test_works_for_scipost_doi(self):
@@ -72,4 +73,9 @@ class DOICallerTest(TestCase):
'volume': '2',
'authorlist': ['Yannis Brun', 'Jerome Dubail']
}
+ self.assertTrue(caller.is_valid)
self.assertEqual(caller.data, correct_data)
+
+ def test_valid_but_non_existent_doi(self):
+ caller = DOICaller('10.21468/NonExistentJournal.2.2.012')
+ self.assertEqual(caller.is_valid, False)
--
GitLab