From b4a586b07a741c70cb928344476823395d7565b9 Mon Sep 17 00:00:00 2001
From: Jorran de Wit <jorrandewit@outlook.com>
Date: Sun, 15 Apr 2018 10:20:29 +0200
Subject: [PATCH] Code improvement
---
scipost/decorators.py | 13 +-
scipost/forms.py | 29 ++-
scipost/models.py | 30 ++-
.../templates/scipost/contributor_info.html | 2 +-
scipost/views.py | 182 +++++++-----------
submissions/views.py | 2 -
6 files changed, 112 insertions(+), 146 deletions(-)
diff --git a/scipost/decorators.py b/scipost/decorators.py
index cebaccc02..fea4dc361 100644
--- a/scipost/decorators.py
+++ b/scipost/decorators.py
@@ -2,13 +2,24 @@ __copyright__ = "Copyright 2016-2018, Stichting SciPost (SciPost Foundation)"
__license__ = "AGPL v3"
+from django.contrib.auth.decorators import user_passes_test
+
from .models import Contributor
def has_contributor(user):
- """Requires user to be related to any Contributor."""
+ """Require user to be related to any Contributor."""
try:
user.contributor
return True
except Contributor.DoesNotExist:
return False
+
+
+def is_contributor_user():
+ """Dceorator checking if user is related to any Contributor."""
+ def test(u):
+ if u.is_authenticated():
+ return has_contributor(u)
+ return False
+ return user_passes_test(test)
diff --git a/scipost/forms.py b/scipost/forms.py
index 3dcaaea21..7beb0185e 100644
--- a/scipost/forms.py
+++ b/scipost/forms.py
@@ -24,11 +24,11 @@ from ajax_select.fields import AutoCompleteSelectField
from haystack.forms import ModelSearchForm as HayStackSearchForm
from .behaviors import orcid_validator
-from .constants import (SCIPOST_DISCIPLINES, TITLE_CHOICES, SCIPOST_FROM_ADDRESSES,
- NO_SCIENTIST, DOUBLE_ACCOUNT, BARRED)
+from .constants import (
+ SCIPOST_DISCIPLINES, TITLE_CHOICES, SCIPOST_FROM_ADDRESSES, NO_SCIENTIST, DOUBLE_ACCOUNT,
+ BARRED)
from .decorators import has_contributor
-from .models import Contributor, DraftInvitation,\
- UnavailabilityPeriod, PrecookedEmail
+from .models import Contributor, DraftInvitation, UnavailabilityPeriod, PrecookedEmail
from affiliations.models import Affiliation, Institution
from common.forms import MonthYearWidget
@@ -81,10 +81,10 @@ class RegistrationForm(forms.Form):
last_name = forms.CharField(label='* Last name', max_length=100)
email = forms.EmailField(label='* Email address')
invitation_key = forms.CharField(max_length=40, widget=forms.HiddenInput(), required=False)
- orcid_id = forms.CharField(label="ORCID id", max_length=20, required=False,
- validators=[orcid_validator],
- widget=forms.TextInput(
- {'placeholder': 'Recommended. Get one at orcid.org'}))
+ orcid_id = forms.CharField(
+ label="ORCID id", max_length=20, required=False, validators=[orcid_validator],
+ widget=forms.TextInput({
+ 'placeholder': 'Recommended. Get one at orcid.org'}))
discipline = forms.ChoiceField(choices=SCIPOST_DISCIPLINES, label='* Main discipline')
country_of_employment = LazyTypedChoiceField(
choices=countries, label='* Country of employment', initial='NL',
@@ -94,12 +94,10 @@ class RegistrationForm(forms.Form):
affiliation = forms.CharField(label='* Affiliation', max_length=300)
address = forms.CharField(
label='Address', max_length=1000,
- widget=forms.TextInput({'placeholder': 'For postal correspondence'}),
- required=False)
+ widget=forms.TextInput({'placeholder': 'For postal correspondence'}), required=False)
personalwebpage = forms.URLField(
- label='Personal web page',
- widget=forms.TextInput({'placeholder': 'full URL, e.g. http://www.[yourpage].com'}),
- required=False)
+ label='Personal web page', required=False,
+ widget=forms.TextInput({'placeholder': 'full URL, e.g. http://www.[yourpage].com'}))
username = forms.CharField(label='* Username', max_length=100)
password = forms.CharField(label='* Password', widget=forms.PasswordInput())
password_verif = forms.CharField(label='* Verify password', widget=forms.PasswordInput(),
@@ -284,8 +282,9 @@ class AuthenticationForm(forms.Form):
next = forms.CharField(widget=forms.HiddenInput(), required=False)
def user_is_inactive(self):
- """
- Check if the User is active but only if the password is valid, to prevent any
+ """Check if the User is active only if the password is valid.
+
+ Only check to prevent any
possible clue (?) of the password.
"""
username = self.cleaned_data['username']
diff --git a/scipost/models.py b/scipost/models.py
index 83e5c3cb7..d5bc608c4 100644
--- a/scipost/models.py
+++ b/scipost/models.py
@@ -56,16 +56,12 @@ class Contributor(models.Model):
blank=True, null=True)
orcid_id = models.CharField(max_length=20, verbose_name="ORCID id",
blank=True, validators=[orcid_validator])
- address = models.CharField(max_length=1000, verbose_name="address",
- blank=True)
- personalwebpage = models.URLField(verbose_name='personal web page',
- blank=True)
+ address = models.CharField(max_length=1000, verbose_name="address", blank=True)
+ personalwebpage = models.URLField(verbose_name='personal web page', blank=True)
vetted_by = models.ForeignKey('self', on_delete=models.SET(get_sentinel_user),
- related_name="contrib_vetted_by",
- blank=True, null=True)
+ related_name="contrib_vetted_by", blank=True, null=True)
accepts_SciPost_emails = models.BooleanField(
- default=True,
- verbose_name="I accept to receive SciPost emails")
+ default=True, verbose_name="I accept to receive SciPost emails")
objects = ContributorQuerySet.as_manager()
@@ -73,48 +69,50 @@ class Contributor(models.Model):
return '%s, %s' % (self.user.last_name, self.user.first_name)
def save(self, *args, **kwargs):
+ """Generate new activitation key if not set."""
if not self.activation_key:
self.generate_key()
return super().save(*args, **kwargs)
def get_absolute_url(self):
+ """Return public information page url."""
return reverse('scipost:contributor_info', args=(self.id,))
- @property
- def get_formal_display(self):
- return '%s %s %s' % (self.get_title_display(), self.user.first_name, self.user.last_name)
-
@property
def is_currently_available(self):
+ """Check if Contributor is currently not marked as unavailable."""
return not self.unavailability_periods.today().exists()
def is_EdCol_Admin(self):
+ """Check if Contributor is an Editorial Administrator."""
return (self.user.groups.filter(name='Editorial Administrators').exists()
or self.user.is_superuser)
def is_SP_Admin(self):
+ """Check if Contributor is a SciPost Administrator."""
return (self.user.groups.filter(name='SciPost Administrators').exists()
or self.user.is_superuser)
def is_MEC(self):
+ """Check if Contributor is a member of the Editorial College."""
return self.fellowships.active().exists() or self.user.is_superuser
def is_VE(self):
+ """Check if Contributor is a Vetting Editor."""
return (self.user.groups.filter(name='Vetting Editors').exists()
or self.user.is_superuser)
def generate_key(self, feed=''):
- """
- Generate and save a new activation_key for the contributor, given a certain feed.
- """
+ """Generate a new activation_key for the contributor, given a certain feed."""
for i in range(5):
feed += random.choice(string.ascii_letters)
feed = feed.encode('utf8')
salt = self.user.username.encode('utf8')
- self.activation_key = hashlib.sha1(salt+salt).hexdigest()
+ self.activation_key = hashlib.sha1(salt + salt).hexdigest()
self.key_expires = datetime.datetime.now() + datetime.timedelta(days=2)
def expertises_as_string(self):
+ """Return joined expertises."""
if self.expertises:
return ', '.join([subject_areas_dict[exp].lower() for exp in self.expertises])
return ''
diff --git a/scipost/templates/scipost/contributor_info.html b/scipost/templates/scipost/contributor_info.html
index 26423a950..d65751ed7 100644
--- a/scipost/templates/scipost/contributor_info.html
+++ b/scipost/templates/scipost/contributor_info.html
@@ -4,7 +4,7 @@
{% block content %}
-<h1 class="highlight mb-4">Contributor info: {{ contributor.get_formal_display }}</h1>
+<h1 class="highlight mb-4">Contributor info: {{ contributor.get_title_display }} {{ contributor.user.first_name }} {{ contributor.user.last_name }}</h1>
{% include "scipost/_public_info_as_table.html" with contributor=contributor %}
diff --git a/scipost/views.py b/scipost/views.py
index 2565a46f8..ae5c1d9e2 100644
--- a/scipost/views.py
+++ b/scipost/views.py
@@ -9,7 +9,7 @@ from django.shortcuts import get_object_or_404, render
from django.conf import settings
from django.contrib import messages
from django.contrib.auth import login, logout, update_session_auth_hash
-from django.contrib.auth.decorators import login_required, user_passes_test
+from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import Group
from django.contrib.auth.views import password_reset, password_reset_confirm
from django.core import mail
@@ -29,16 +29,16 @@ from django.views.static import serve
from guardian.decorators import permission_required
from haystack.generic_views import SearchView
-from .constants import SCIPOST_SUBJECT_AREAS, subject_areas_raw_dict, SciPost_from_addresses_dict,\
- NORMAL_CONTRIBUTOR
-from .decorators import has_contributor
-from .models import Contributor, UnavailabilityPeriod,\
- AuthorshipClaim, EditorialCollege, EditorialCollegeFellowship
-from .forms import AuthenticationForm, UnavailabilityPeriodForm,\
- RegistrationForm, AuthorshipClaimForm,\
- SearchForm, VetRegistrationForm, reg_ref_dict,\
- UpdatePersonalDataForm, UpdateUserDataForm, PasswordChangeForm,\
- EmailGroupMembersForm, EmailParticularForm, SendPrecookedEmailForm
+from .constants import (
+ SCIPOST_SUBJECT_AREAS, subject_areas_raw_dict, SciPost_from_addresses_dict, NORMAL_CONTRIBUTOR)
+from .decorators import has_contributor, is_contributor_user
+from .models import (
+ Contributor, UnavailabilityPeriod, AuthorshipClaim, EditorialCollege,
+ EditorialCollegeFellowship)
+from .forms import (
+ AuthenticationForm, UnavailabilityPeriodForm, RegistrationForm, AuthorshipClaimForm,
+ SearchForm, VetRegistrationForm, reg_ref_dict, UpdatePersonalDataForm, UpdateUserDataForm,
+ PasswordChangeForm, EmailGroupMembersForm, EmailParticularForm, SendPrecookedEmailForm)
from .utils import Utils, EMAIL_FOOTER, SCIPOST_SUMMARY_FOOTER, SCIPOST_SUMMARY_FOOTER_HTML
from affiliations.forms import AffiliationsFormset
@@ -49,8 +49,7 @@ from invitations.constants import STATUS_REGISTERED
from invitations.models import RegistrationInvitation
from journals.models import Publication, Journal, PublicationAuthorsTable
from news.models import NewsItem
-from submissions.models import Submission, RefereeInvitation,\
- Report, EICRecommendation
+from submissions.models import Submission, RefereeInvitation, Report, EICRecommendation
from partners.models import MembershipAgreement
from theses.models import ThesisLink
@@ -60,18 +59,18 @@ from theses.models import ThesisLink
##############
def is_registered(user):
- """
- This method checks if user is activated assuming an validated user
- has at least one permission group (`Registered Contributor` or `Partner Accounts`).
- """
+ """Check if user is a validated user; has at least one permission group."""
return user.groups.exists()
class SearchView(SearchView):
+ """Search CBV inherited from Haystack."""
+
template_name = 'search/search.html'
form_class = SearchForm
def get_context_data(self, *args, **kwargs):
+ """Update context with some additional information."""
ctx = super().get_context_data(*args, **kwargs)
ctx['search_query'] = self.request.GET.get('q')
ctx['results_count'] = kwargs['object_list'].count()
@@ -83,7 +82,7 @@ class SearchView(SearchView):
#############
def index(request):
- '''Main page.'''
+ """Homepage view of SciPost."""
context = {
'latest_newsitem': NewsItem.objects.filter(on_homepage=True).order_by('-date').first(),
'submissions': Submission.objects.public().order_by('-submission_date')[:3],
@@ -96,7 +95,8 @@ def index(request):
def protected_serve(request, path, show_indexes=False):
- """
+ """Serve media files from outside the public MEDIA_ROOT folder.
+
Serve files that are saved outside the default MEDIA_ROOT folder for superusers only!
This will be usefull eg. in the admin pages.
"""
@@ -112,6 +112,7 @@ def protected_serve(request, path, show_indexes=False):
###############
def feeds(request):
+ """Information page for RSS and Atom feeds."""
context = {'subject_areas_physics': SCIPOST_SUBJECT_AREAS[0][1]}
return render(request, 'scipost/feeds.html', context)
@@ -121,7 +122,8 @@ def feeds(request):
################
def register(request):
- """
+ """Contributor registration form page.
+
This public registration view shows and processes the form
that will create new user account requests. After registration
the Contributor will need to activate its account via the mail
@@ -155,7 +157,8 @@ def register(request):
def invitation(request, key):
- """
+ """Registration Invitation reception page.
+
If a scientist has recieved an invitation (RegistrationInvitation)
he/she will finish it's invitation via still view which will prefill
the default registration form.
@@ -244,9 +247,8 @@ def unsubscribe(request, contributor_id, key):
@permission_required('scipost.can_vet_registration_requests', return_403=True)
def vet_registration_requests(request):
- contributors_to_vet = (Contributor.objects
- .awaiting_vetting()
- .order_by('key_expires'))
+ """List of new Registration requests to vet."""
+ contributors_to_vet = Contributor.objects.awaiting_vetting().order_by('key_expires')
form = VetRegistrationForm()
context = {'contributors_to_vet': contributors_to_vet, 'form': form}
return render(request, 'scipost/vet_registration_requests.html', context)
@@ -254,7 +256,7 @@ def vet_registration_requests(request):
@permission_required('scipost.can_vet_registration_requests', return_403=True)
def vet_registration_request_ack(request, contributor_id):
- # process the form
+ """Form view to vet new Registration requests."""
form = VetRegistrationForm(request.POST or None)
contributor = Contributor.objects.get(pk=contributor_id)
if form.is_valid():
@@ -274,11 +276,10 @@ def vet_registration_request_ack(request, contributor_id):
except RefereeInvitation.DoesNotExist:
pending_ref_inv_exists = False
- email_text = ('Dear ' + contributor.get_title_display() + ' '
- + contributor.user.last_name +
- ', \n\nYour registration to the SciPost publication portal '
- 'has been accepted. '
- 'You can now login at https://scipost.org and contribute. \n\n')
+ email_text = (
+ 'Dear ' + contributor.get_title_display() + ' ' + contributor.user.last_name +
+ ', \n\nYour registration to the SciPost publication portal has been accepted. '
+ 'You can now login at https://scipost.org and contribute. \n\n')
if pending_ref_inv_exists:
email_text += (
'Note that you have pending refereeing invitations; please navigate to '
@@ -293,17 +294,15 @@ def vet_registration_request_ack(request, contributor_id):
emailmessage.send(fail_silently=False)
else:
ref_reason = form.cleaned_data['refusal_reason']
- email_text = ('Dear ' + contributor.get_title_display() + ' '
- + contributor.user.last_name +
- ', \n\nYour registration to the SciPost publication portal '
- 'has been turned down, the reason being: '
- + reg_ref_dict[ref_reason] + '. You can however still view '
- 'all SciPost contents, just not submit papers, '
- 'comments or votes. We nonetheless thank you for your interest.'
- '\n\nThe SciPost Team.')
+ email_text = (
+ 'Dear ' + contributor.get_title_display() + ' ' + contributor.user.last_name +
+ ', \n\nYour registration to the SciPost publication portal has been turned down,'
+ ' the reason being: ' + reg_ref_dict[ref_reason] + '. You can however still view '
+ 'all SciPost contents, just not submit papers, comments or votes. We nonetheless '
+ 'thank you for your interest.\n\nThe SciPost Team.')
if form.cleaned_data['email_response_field']:
- email_text += ('\n\nFurther explanations: '
- + form.cleaned_data['email_response_field'])
+ email_text += (
+ '\n\nFurther explanations: ' + form.cleaned_data['email_response_field'])
emailmessage = EmailMessage('SciPost registration: unsuccessful',
email_text,
'SciPost registration <registration@scipost.org>',
@@ -337,9 +336,7 @@ def registration_requests(request):
@require_POST
@permission_required('scipost.can_resend_registration_requests', return_403=True)
def registration_requests_reset(request, contributor_id):
- '''
- Reset specific activation_key for Contributor and resend activation mail.
- '''
+ """Reset specific activation_key for Contributor and resend activation mail."""
contributor = get_object_or_404(Contributor.objects.awaiting_validation(), id=contributor_id)
contributor.generate_key()
contributor.save()
@@ -351,15 +348,7 @@ def registration_requests_reset(request, contributor_id):
def login_view(request):
- """
- This view shows and processes a user's login session.
-
- The function based method login() is deprecated from
- Django 1.11 and replaced by Class Based Views.
-
- See:
- https://docs.djangoproject.com/en/1.11/releases/1.11/#django-contrib-auth
- """
+ """Login form page."""
form = AuthenticationForm(request.POST or None, initial=request.GET)
if form.is_valid():
user = form.authenticate()
@@ -381,13 +370,7 @@ def login_view(request):
def logout_view(request):
- """
- The function based method logout() is deprecated from
- Django 1.11 and replaced by Class Based Views.
-
- See:
- https://docs.djangoproject.com/en/1.11/releases/1.11/#django-contrib-auth
- """
+ """Logout form page."""
logout(request)
messages.success(request, ('<h3>Keep contributing!</h3>'
'You are now logged out of SciPost.'))
@@ -395,11 +378,9 @@ def logout_view(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def mark_unavailable_period(request):
- '''
- Mark period unavailable for Contributor using this view.
- '''
+ """Form view to mark period unavailable for Contributor."""
unav_form = UnavailabilityPeriodForm(request.POST or None)
if unav_form.is_valid():
unav = unav_form.save(commit=False)
@@ -415,11 +396,9 @@ def mark_unavailable_period(request):
@require_POST
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def delete_unavailable_period(request, period_id):
- '''
- Delete period unavailable registered.
- '''
+ """Delete period unavailable registered."""
unav = get_object_or_404(UnavailabilityPeriod,
contributor=request.user.contributor, id=int(period_id))
unav.delete()
@@ -428,11 +407,9 @@ def delete_unavailable_period(request, period_id):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_editorial_account(request):
- """
- The Personal Page tab: Account
- """
+ """Personal Page tab: Account."""
contributor = request.user.contributor
context = {
'contributor': contributor,
@@ -442,11 +419,9 @@ def _personal_page_editorial_account(request):
return render(request, 'partials/scipost/personal_page/account.html', context)
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_editorial_actions(request):
- """
- The Personal Page tab: Editorial Actions
- """
+ """Personal Page tab: Editorial Actions."""
permission = request.user.groups.filter(name__in=[
'Ambassadors',
'Advisory Board',
@@ -491,11 +466,9 @@ def _personal_page_editorial_actions(request):
@permission_required('scipost.can_referee', return_403=True)
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_refereeing(request):
- """
- The Personal Page tab: Refereeing
- """
+ """Personal Page tab: Refereeing."""
context = {
'contributor': request.user.contributor
}
@@ -503,11 +476,9 @@ def _personal_page_refereeing(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_publications(request):
- """
- The Personal Page tab: Publications
- """
+ """Personal Page tab: Publications."""
contributor = request.user.contributor
context = {
'contributor': contributor,
@@ -522,11 +493,9 @@ def _personal_page_publications(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_submissions(request):
- """
- The Personal Page tab: Submissions
- """
+ """Personal Page tab: Submissions."""
contributor = request.user.contributor
context = {'contributor': contributor}
@@ -541,11 +510,9 @@ def _personal_page_submissions(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_commentaries(request):
- """
- The Personal Page tab: Commentaries
- """
+ """Personal Page tab: Commentaries."""
contributor = request.user.contributor
context = {'contributor': contributor}
@@ -559,11 +526,9 @@ def _personal_page_commentaries(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_theses(request):
- """
- The Personal Page tab: Theses
- """
+ """Personal Page tab: Theses."""
contributor = request.user.contributor
context = {'contributor': contributor}
@@ -577,11 +542,9 @@ def _personal_page_theses(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_comments(request):
- """
- The Personal Page tab: Comments
- """
+ """Personal Page tab: Comments."""
contributor = request.user.contributor
context = {
'contributor': contributor,
@@ -592,11 +555,9 @@ def _personal_page_comments(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def _personal_page_author_replies(request):
- """
- The Personal Page tab: Author Replies
- """
+ """Personal Page tab: Author Replies."""
contributor = request.user.contributor
context = {
'contributor': contributor,
@@ -608,9 +569,7 @@ def _personal_page_author_replies(request):
@login_required
def personal_page(request, tab='account'):
- """
- The Personal Page is the main view for accessing user functions.
- """
+ """Personal Page is the main view for accessing user functions."""
if request.is_ajax():
if tab == 'account':
return _personal_page_editorial_account(request)
@@ -659,6 +618,7 @@ def personal_page(request, tab='account'):
@login_required
def change_password(request):
+ """Change password form view."""
form = PasswordChangeForm(request.POST or None, current_user=request.user)
if form.is_valid():
form.save_new_password()
@@ -729,7 +689,7 @@ def update_personal_data(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def claim_authorships(request):
"""
The system auto-detects potential authorships (of submissions,
@@ -778,7 +738,7 @@ def claim_authorships(request):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def claim_pub_authorship(request, publication_id, claim):
if request.method == 'POST':
contributor = Contributor.objects.get(user=request.user)
@@ -794,7 +754,7 @@ def claim_pub_authorship(request, publication_id, claim):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def claim_sub_authorship(request, submission_id, claim):
if request.method == 'POST':
contributor = Contributor.objects.get(user=request.user)
@@ -810,7 +770,7 @@ def claim_sub_authorship(request, submission_id, claim):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def claim_com_authorship(request, commentary_id, claim):
if request.method == 'POST':
contributor = Contributor.objects.get(user=request.user)
@@ -826,7 +786,7 @@ def claim_com_authorship(request, commentary_id, claim):
@login_required
-@user_passes_test(has_contributor)
+@is_contributor_user
def claim_thesis_authorship(request, thesis_id, claim):
if request.method == 'POST':
contributor = Contributor.objects.get(user=request.user)
diff --git a/submissions/views.py b/submissions/views.py
index 124890e11..bac948e24 100644
--- a/submissions/views.py
+++ b/submissions/views.py
@@ -43,8 +43,6 @@ from .utils import SubmissionUtils
from colleges.permissions import fellowship_required, fellowship_or_admin_required
from comments.forms import CommentForm
-from invitations.constants import INVITATION_REFEREEING
-from invitations.models import RegistrationInvitation
from journals.models import Journal
from mails.views import MailEditingSubView
from production.forms import ProofsDecisionForm
--
GitLab