From 1a122a04294224b02a695eda9747b04971e5b6b3 Mon Sep 17 00:00:00 2001
From: George Katsikas <giorgakis.katsikas@gmail.com>
Date: Thu, 11 Jul 2024 14:39:15 +0300
Subject: [PATCH] refactor assignment of officers and supervisors fix removal
of stream work permission of supervisors
---
scipost_django/production/forms.py | 23 ++++++++++
scipost_django/production/models.py | 71 +++++++++++++++++++++++++++++
scipost_django/production/views.py | 51 ++++-----------------
3 files changed, 102 insertions(+), 43 deletions(-)
diff --git a/scipost_django/production/forms.py b/scipost_django/production/forms.py
index 32ba2b1f5..0a7d607d9 100644
--- a/scipost_django/production/forms.py
+++ b/scipost_django/production/forms.py
@@ -71,8 +71,15 @@ class AssignOfficerForm(forms.ModelForm):
model = ProductionStream
fields = ("officer",)
+ # Override the default clean method called via is_valid
+ # which overrides the instance and hides its previous value
+ def _post_clean(self):
+ return
+
def save(self, commit=True):
stream = super().save(False)
+ stream.set_officer(self.cleaned_data["officer"], self.production_user)
+
if commit:
if stream.status in [
constants.PRODUCTION_STREAM_INITIATED,
@@ -85,6 +92,7 @@ class AssignOfficerForm(forms.ModelForm):
return stream
def __init__(self, *args, **kwargs):
+ self.production_user = kwargs.pop("production_user", None)
super().__init__(*args, **kwargs)
self.fields["officer"].queryset = ProductionUser.objects.active().filter(
user__groups__name="Production Officers"
@@ -110,12 +118,27 @@ class AssignSupervisorForm(forms.ModelForm):
model = ProductionStream
fields = ("supervisor",)
+ # Override the default clean method called via is_valid
+ # which overrides the instance and hides its previous value
+ def _post_clean(self):
+ return
+
def __init__(self, *args, **kwargs):
+ self.production_user = kwargs.pop("production_user", None)
super().__init__(*args, **kwargs)
self.fields["supervisor"].queryset = ProductionUser.objects.active().filter(
user__groups__name="Production Supervisor"
)
+ def save(self, commit=True):
+ stream = self.instance
+ stream.set_supervisor(self.cleaned_data["supervisor"], self.production_user)
+
+ if commit:
+ stream.save()
+
+ return stream
+
class StreamStatusForm(forms.ModelForm):
class Meta:
diff --git a/scipost_django/production/models.py b/scipost_django/production/models.py
index e7e9445ab..b21bee18a 100644
--- a/scipost_django/production/models.py
+++ b/scipost_django/production/models.py
@@ -16,6 +16,8 @@ from django.db.models import Value
from django.db.models.functions import Concat
from django.conf import settings
+from guardian.shortcuts import assign_perm, remove_perm
+
from common.utils import latinise
from journals.models import Journal
from submissions.models.decision import EditorialDecision
@@ -127,6 +129,75 @@ class ProductionStream(models.Model):
def get_absolute_url(self):
return reverse("production:stream", args=(self.id,))
+ def set_supervisor(
+ self,
+ supervisor: ProductionUser | None,
+ performed_by: ProductionUser | None = None,
+ ):
+ # Remove permissions from previous supervisor
+ if prev_supervisor := self.supervisor:
+ remove_perm("can_perform_supervisory_actions", prev_supervisor.user, self)
+ # Remove work permission only if they have no other role
+ if prev_supervisor not in [self.officer, self.invitations_officer]:
+ remove_perm("can_work_for_stream", prev_supervisor.user, self)
+
+ self.supervisor = supervisor
+
+ # Add permissions to new supervisor
+ if supervisor:
+ assign_perm("can_perform_supervisory_actions", supervisor.user, self)
+ assign_perm("can_work_for_stream", supervisor.user, self)
+ comments = " assigned Production Supervisor:"
+ else:
+ comments = f" removed Production Supervisor: {prev_supervisor}"
+
+ self.add_event(
+ event="assignment", by=performed_by, to=supervisor, comments=comments
+ )
+
+ def set_officer(
+ self,
+ officer: ProductionUser | None,
+ performed_by: ProductionUser | None = None,
+ ):
+ # Remove permissions from previous officer
+ if (prev_officer := self.officer) and (
+ prev_officer not in [self.supervisor, self.invitations_officer]
+ ):
+ print("Removing permissions for", prev_officer.user)
+ remove_perm("can_work_for_stream", prev_officer.user, self)
+
+ self.officer = officer
+
+ # Add permissions to new officer
+ if officer:
+ assign_perm("can_work_for_stream", officer.user, self)
+ comments = " assigned Production Officer:"
+ else:
+ comments = f" removed Production Officer: {prev_officer}"
+
+ self.add_event(
+ event="assignment", by=performed_by, to=officer, comments=comments
+ )
+
+ def add_event(
+ self,
+ event,
+ by: ProductionUser | None = None,
+ comments=None,
+ duration=None,
+ to=None,
+ ):
+ by = by or ProductionUser.objects.filter(user__username="system").first()
+ ProductionEvent.objects.create(
+ stream=self,
+ event=event,
+ noted_by=by,
+ noted_to=to,
+ comments=comments,
+ duration=duration,
+ )
+
@cached_property
def total_duration(self):
totdur = self.work_logs.aggregate(models.Sum("duration"))
diff --git a/scipost_django/production/views.py b/scipost_django/production/views.py
index 37d91e392..01620a28e 100644
--- a/scipost_django/production/views.py
+++ b/scipost_django/production/views.py
@@ -610,35 +610,22 @@ def _hx_update_officer(request, stream_id):
officer_form = AssignOfficerForm(
request.POST or None,
instance=productionstream,
+ production_user=request.user.production_user,
auto_id=f"productionstream_{productionstream.id}_id_%s",
)
if officer_form.is_valid():
- officer_form.save()
- officer = officer_form.cleaned_data.get("officer")
+ productionstream = officer_form.save()
# Add officer to stream if they exist.
- if officer is not None:
- assign_perm("can_work_for_stream", officer.user, productionstream)
- messages.success(request, f"Officer {officer} has been assigned.")
-
- event = ProductionEvent(
- stream=productionstream,
- event="assignment",
- comments=" tasked Production Officer with proofs production:",
- noted_to=officer,
- noted_by=request.user.production_user,
- )
- event.save()
-
+ if officer := productionstream.officer:
# Temp fix.
# TODO: Implement proper email
ProductionUtils.load({"request": request, "stream": productionstream})
ProductionUtils.email_assigned_production_officer()
- # Remove old officer.
+ messages.success(request, f"Officer {officer} has been assigned.")
else:
- remove_perm("can_work_for_stream", prev_officer.user, productionstream)
messages.success(request, f"Officer {prev_officer} has been removed.")
else:
@@ -916,45 +903,23 @@ def _hx_update_supervisor(request, stream_id):
supervisor_form = AssignSupervisorForm(
request.POST or None,
instance=productionstream,
+ production_user=request.user.production_user,
auto_id=f"productionstream_{productionstream.id}_id_%s",
)
prev_supervisor = productionstream.supervisor
if supervisor_form.is_valid():
- supervisor_form.save()
- supervisor = supervisor_form.cleaned_data.get("supervisor")
+ productionstream = supervisor_form.save()
# Add supervisor to stream if they exist.
- if supervisor is not None:
- messages.success(request, f"Supervisor {supervisor} has been assigned.")
-
- assign_perm("can_work_for_stream", supervisor.user, productionstream)
- assign_perm(
- "can_perform_supervisory_actions", supervisor.user, productionstream
- )
-
- event = ProductionEvent(
- stream=productionstream,
- event="assignment",
- comments=" assigned Production Supervisor:",
- noted_to=supervisor,
- noted_by=request.user.production_user,
- )
- event.save()
-
+ if supervisor := productionstream.supervisor:
# Temp fix.
# TODO: Implement proper email
ProductionUtils.load({"request": request, "stream": productionstream})
ProductionUtils.email_assigned_supervisor()
- # Remove old supervisor.
+ messages.success(request, f"Supervisor {supervisor} has been assigned.")
else:
- remove_perm("can_work_for_stream", prev_supervisor.user, productionstream)
- remove_perm(
- "can_perform_supervisory_actions",
- prev_supervisor.user,
- productionstream,
- )
messages.success(request, f"Supervisor {prev_supervisor} has been removed.")
else:
--
GitLab