From de48551357ef79f5a0e3dd7d27bb5bc7e165fb25 Mon Sep 17 00:00:00 2001
From: Jorran de Wit <jorrandewit@outlook.com>
Date: Tue, 6 Jun 2017 17:27:36 +0200
Subject: [PATCH] General improvements
Include the removal of redundant HTML, fix security fix (view permission) and modelform use.
---
production/admin.py | 4 ++
production/constants.py | 7 +--
production/forms.py | 7 ++-
production/managers.py | 11 +++++
production/models.py | 13 +++--
.../production/_production_event_li.html | 12 -----
.../production/_production_events.html | 18 +++++++
.../production/_production_stream_card.html | 35 +++++++-------
.../templates/production/completed.html | 26 +++++++---
.../templates/production/production.html | 22 ++++++---
production/views.py | 48 +++++++------------
11 files changed, 121 insertions(+), 82 deletions(-)
create mode 100644 production/managers.py
delete mode 100644 production/templates/production/_production_event_li.html
create mode 100644 production/templates/production/_production_events.html
diff --git a/production/admin.py b/production/admin.py
index 64b634398..16774de5f 100644
--- a/production/admin.py
+++ b/production/admin.py
@@ -15,11 +15,13 @@ class ProductionStreamAdminForm(forms.ModelForm):
model = ProductionStream
fields = '__all__'
+
class ProductionStreamAdmin(admin.ModelAdmin):
search_fields = ['submission']
list_display = ['submission', 'opened', 'status']
form = ProductionStreamAdminForm
+
admin.site.register(ProductionStream, ProductionStreamAdmin)
@@ -31,9 +33,11 @@ class ProductionEventAdminForm(forms.ModelForm):
model = ProductionEvent
fields = '__all__'
+
class ProductionEventAdmin(admin.ModelAdmin):
search_field = ['stream', 'event', 'comment', 'noted_by']
list_display = ['stream', 'event', 'noted_on', 'noted_by']
form = ProductionEventAdminForm
+
admin.site.register(ProductionEvent, ProductionEventAdmin)
diff --git a/production/constants.py b/production/constants.py
index ce9caee48..265902708 100644
--- a/production/constants.py
+++ b/production/constants.py
@@ -1,9 +1,10 @@
+PRODUCTION_STREAM_ONGOING = 'ongoing'
+PRODUCTION_STREAM_COMPLETED = 'completed'
PRODUCTION_STREAM_STATUS = (
- ('ongoing', 'Ongoing'),
- ('completed', 'Completed'),
+ (PRODUCTION_STREAM_ONGOING, 'Ongoing'),
+ (PRODUCTION_STREAM_COMPLETED, 'Completed'),
)
-
PRODUCTION_EVENTS = (
('assigned_to_supervisor', 'Assigned by EdAdmin to Supervisor'),
('message_edadmin_to_supervisor', 'Message from EdAdmin to Supervisor'),
diff --git a/production/forms.py b/production/forms.py
index 9f47ebe67..4db8b8dec 100644
--- a/production/forms.py
+++ b/production/forms.py
@@ -6,8 +6,13 @@ from .models import ProductionEvent
class ProductionEventForm(forms.ModelForm):
class Meta:
model = ProductionEvent
- exclude = ['stream', 'noted_on', 'noted_by']
+ fields = (
+ 'event',
+ 'comments',
+ 'duration'
+ )
widgets = {
+ 'stream': forms.HiddenInput(),
'comments': forms.Textarea(attrs={'rows': 4}),
'duration': forms.TextInput(attrs={'placeholder': 'HH:MM:SS'})
}
diff --git a/production/managers.py b/production/managers.py
new file mode 100644
index 000000000..4379df507
--- /dev/null
+++ b/production/managers.py
@@ -0,0 +1,11 @@
+from django.db import models
+
+from .constants import PRODUCTION_STREAM_COMPLETED, PRODUCTION_STREAM_ONGOING
+
+
+class ProductionStreamManager(models.Manager):
+ def completed(self):
+ return self.filter(status=PRODUCTION_STREAM_COMPLETED)
+
+ def ongoing(self):
+ return self.filter(status=PRODUCTION_STREAM_ONGOING)
diff --git a/production/models.py b/production/models.py
index 8cd82781c..f2ec3b93c 100644
--- a/production/models.py
+++ b/production/models.py
@@ -1,7 +1,8 @@
from django.db import models
from django.utils import timezone
-from .constants import PRODUCTION_STREAM_STATUS, PRODUCTION_EVENTS
+from .constants import PRODUCTION_STREAM_STATUS, PRODUCTION_STREAM_ONGOING, PRODUCTION_EVENTS
+from .managers import ProductionStreamManager
from scipost.models import Contributor
@@ -12,16 +13,18 @@ from scipost.models import Contributor
class ProductionStream(models.Model):
submission = models.OneToOneField('submissions.Submission', on_delete=models.CASCADE)
- opened = models.DateTimeField()
+ opened = models.DateTimeField(auto_now_add=True)
closed = models.DateTimeField(default=timezone.now)
status = models.CharField(max_length=32,
- choices=PRODUCTION_STREAM_STATUS, default='ongoing')
+ choices=PRODUCTION_STREAM_STATUS, default=PRODUCTION_STREAM_ONGOING)
+
+ objects = ProductionStreamManager()
def __str__(self):
return str(self.submission)
def total_duration(self):
- totdur = self.productionevent_set.all().aggregate(models.Sum('duration'))
+ totdur = self.productionevent_set.aggregate(models.Sum('duration'))
return totdur['duration__sum']
@@ -29,7 +32,7 @@ class ProductionEvent(models.Model):
stream = models.ForeignKey(ProductionStream, on_delete=models.CASCADE)
event = models.CharField(max_length=64, choices=PRODUCTION_EVENTS)
comments = models.TextField(blank=True, null=True)
- noted_on = models.DateTimeField(default=timezone.now)
+ noted_on = models.DateTimeField(auto_now_add=True)
noted_by = models.ForeignKey(Contributor, on_delete=models.CASCADE)
duration = models.DurationField(blank=True, null=True)
diff --git a/production/templates/production/_production_event_li.html b/production/templates/production/_production_event_li.html
deleted file mode 100644
index 117a80234..000000000
--- a/production/templates/production/_production_event_li.html
+++ /dev/null
@@ -1,12 +0,0 @@
-{% load scipost_extras %}
-
-<li id="{{ event.id }}">
- <div class="font-weight-bold">{{ event.get_event_display }} <small class="text-muted">noted {{ event.noted_on }} by {{ event.noted_by }}</small>
- </div>
- {% if event.duration %}
- <div><small>Duration: {{ event.duration|duration }}</small></div>
- {% endif %}
- {% if event.comments %}
- <div>{{ event.comments|linebreaks }}</div>
- {% endif %}
-</li>
diff --git a/production/templates/production/_production_events.html b/production/templates/production/_production_events.html
new file mode 100644
index 000000000..5903e4768
--- /dev/null
+++ b/production/templates/production/_production_events.html
@@ -0,0 +1,18 @@
+{% load scipost_extras %}
+
+<ul>
+ {% for event in events %}
+ <li id="{{ event.id }}">
+ <div class="font-weight-bold">{{ event.get_event_display }} <small class="text-muted">noted {{ event.noted_on }} by {{ event.noted_by }}</small>
+ </div>
+ {% if event.duration %}
+ <div><small>Duration: {{ event.duration|duration }}</small></div>
+ {% endif %}
+ {% if event.comments %}
+ <div>{{ event.comments|linebreaks }}</div>
+ {% endif %}
+ </li>
+ {% empty %}
+ <li>No events were found.</li>
+ {% endfor %}
+</ul>
diff --git a/production/templates/production/_production_stream_card.html b/production/templates/production/_production_stream_card.html
index c39436c5c..138be8b93 100644
--- a/production/templates/production/_production_stream_card.html
+++ b/production/templates/production/_production_stream_card.html
@@ -1,35 +1,32 @@
{% load bootstrap %}
{% load scipost_extras %}
+<div class="w-100">
+ {% include 'submissions/_submission_card_content_sparse.html' with submission=stream.submission %}
+</div>
<div class="card-block">
- {% include 'submissions/_submission_card_content_sparse.html' with submission=stream.submission %}
<div class="row">
- <div class="col-7">
+ <div class="{% if form %}col-lg-7{% else %}col-12{% endif %}">
<h3>Events</h3>
- <ul>
- {% for event in stream.productionevent_set.all %}
- {% include 'production/_production_event_li.html' with event=event %}
- {% empty %}
- <li>No events were found.</li>
- {% endfor %}
- </ul>
+ {% include 'production/_production_events.html' with events=stream.productionevent_set.all %}
<br/>
+
{% if stream.total_duration %}
- <h3>Total duration for this stream: {{ stream.total_duration|duration }}</h3>
+ <h3>Total duration for this stream: {{ stream.total_duration|duration }}</h3>
{% endif %}
{% if perms.scipost.can_publish_accepted_submission %}
- <h3><a href="{% url 'production:mark_as_completed' stream_id=stream.id %}">Mark this stream as completed</a></h3>
+ <h3><a href="{% url 'production:mark_as_completed' stream_id=stream.id %}">Mark this stream as completed</a></h3>
{% endif %}
</div>
{% if form %}
- <div class="col-5">
- <h3>Add an event to this production stream:</h3>
- <form action="{% url 'production:add_event' stream_id=stream.id %}" method="post">
- {% csrf_token %}
- {{ form|bootstrap }}
- <input type="submit" name="submit" value="Submit">
- </form>
- </div>
+ <div class="col-lg-5 mt-4 mt-lg-0">
+ <h3>Add an event to this production stream:</h3>
+ <form action="{% url 'production:add_event' stream_id=stream.id %}" method="post">
+ {% csrf_token %}
+ {{ form|bootstrap }}
+ <input type="submit" class="btn btn-secondary" name="submit" value="Submit">
+ </form>
+ </div>
{% endif %}
</div>
</div>
diff --git a/production/templates/production/completed.html b/production/templates/production/completed.html
index 1cd7059cd..0bb839176 100644
--- a/production/templates/production/completed.html
+++ b/production/templates/production/completed.html
@@ -1,18 +1,32 @@
-{% extends 'scipost/base.html' %}
+{% extends 'scipost/_personal_page_base.html' %}
+
+{% block breadcrumb_items %}
+ {{block.super}}
+ <span class="breadcrumb-item">Completed production streams</span>
+{% endblock %}
{% load bootstrap %}
{% block content %}
+<div class="row">
+ <div class="col-12">
+ <h1 class="highlight">Completed production streams</h1>
+ </div>
+</div>
<div class="row">
<div class="col-12">
- <h3>Completed production streams</h3>
<ul class="list-group list-group-flush">
{% for stream in streams %}
- <li class="list-group-item">
- {% include 'production/_production_stream_card.html' with stream=stream %}
- </li>
- <hr/>
+ <li class="list-group-item">
+ <div class="w-100">{% include 'submissions/_submission_card_content_sparse.html' with submission=stream.submission %}</div>
+ <div class="card-block">
+ <h3>Events</h3>
+ {% include 'production/_production_events.html' with events=stream.productionevent_set.all %}
+ </div>
+ </li>
+ {% empty %}
+ <li class="list-group-item">No completed production streams found.</li>
{% endfor %}
</ul>
</div>
diff --git a/production/templates/production/production.html b/production/templates/production/production.html
index 36b7dcf39..21f992911 100644
--- a/production/templates/production/production.html
+++ b/production/templates/production/production.html
@@ -1,18 +1,28 @@
-{% extends 'scipost/base.html' %}
+{% extends 'scipost/_personal_page_base.html' %}
+
+{% block breadcrumb_items %}
+ {{block.super}}
+ <span class="breadcrumb-item">Production page</span>
+{% endblock %}
{% load bootstrap %}
{% block content %}
+<div class="row">
+ <div class="col-12">
+ <h1 class="highlight">Manuscripts under production</h1>
+ </div>
+</div>
<div class="row">
<div class="col-12">
- <h3>Manuscripts under production</h3>
<ul class="list-group list-group-flush">
{% for stream in streams %}
- <li class="list-group-item">
- {% include 'production/_production_stream_card.html' with stream=stream form=prodevent_form %}
- </li>
- <hr/>
+ <li class="list-group-item{% if not forloop.first %} pt-3{% endif %}">
+ {% include 'production/_production_stream_card.html' with stream=stream form=prodevent_form %}
+ </li>
+ {% empty %}
+ <li class="list-group-item">No production streams found.</li>
{% endfor %}
</ul>
</div>
diff --git a/production/views.py b/production/views.py
index 52363947a..abbbe4470 100644
--- a/production/views.py
+++ b/production/views.py
@@ -1,15 +1,14 @@
+from django.contrib import messages
from django.core.urlresolvers import reverse
-from django.db import transaction
from django.shortcuts import get_object_or_404, render, redirect
from django.utils import timezone
from guardian.decorators import permission_required
-from .models import ProductionStream, ProductionEvent
+from .constants import PRODUCTION_STREAM_COMPLETED
+from .models import ProductionStream
from .forms import ProductionEventForm
-from submissions.models import Submission
-
######################
# Production process #
@@ -21,7 +20,7 @@ def production(request):
Overview page for the production process.
All papers with accepted but not yet published status are included here.
"""
- streams = ProductionStream.objects.filter(status='ongoing').order_by('opened')
+ streams = ProductionStream.objects.ongoing().order_by('opened')
prodevent_form = ProductionEventForm()
context = {
'streams': streams,
@@ -35,40 +34,29 @@ def completed(request):
"""
Overview page for closed production streams.
"""
- streams = ProductionStream.objects.filter(status='completed').order_by('-opened')
- context = {'streams': streams,}
+ streams = ProductionStream.objects.completed().order_by('-opened')
+ context = {'streams': streams}
return render(request, 'production/completed.html', context)
@permission_required('scipost.can_view_production', return_403=True)
-@transaction.atomic
def add_event(request, stream_id):
- stream = get_object_or_404(ProductionStream, pk=stream_id)
- if request.method == 'POST':
- prodevent_form = ProductionEventForm(request.POST)
- if prodevent_form.is_valid():
- prodevent = ProductionEvent(
- stream=stream,
- event=prodevent_form.cleaned_data['event'],
- comments=prodevent_form.cleaned_data['comments'],
- noted_on=timezone.now(),
- noted_by=request.user.contributor,
- duration=prodevent_form.cleaned_data['duration'],)
- prodevent.save()
- return redirect(reverse('production:production'))
- else:
- errormessage = 'The form was invalidly filled.'
- return render(request, 'scipost/error.html', {'errormessage': errormessage})
+ stream = get_object_or_404(ProductionStream.objects.ongoing(), pk=stream_id)
+ prodevent_form = ProductionEventForm(request.POST or None)
+ if prodevent_form.is_valid():
+ prodevent = prodevent_form.save(commit=False)
+ prodevent.stream = stream
+ prodevent.noted_by = request.user.contributor
+ prodevent.save()
else:
- errormessage = 'This view can only be posted to.'
- return render(request, 'scipost/error.html', {'errormessage': errormessage})
+ messages.warning(request, 'The form was invalidly filled.')
+ return redirect(reverse('production:production'))
-@permission_required('scipost.can_view_production', return_403=True)
-@transaction.atomic
+@permission_required('scipost.can_publish_accepted_submission', return_403=True)
def mark_as_completed(request, stream_id):
- stream = get_object_or_404(ProductionStream, pk=stream_id)
- stream.status = 'completed'
+ stream = get_object_or_404(ProductionStream.objects.ongoing(), pk=stream_id)
+ stream.status = PRODUCTION_STREAM_COMPLETED
stream.closed = timezone.now()
stream.save()
return redirect(reverse('production:production'))
--
GitLab