From b76f07aa12bbe98c4bbd05054079c5391ec2b434 Mon Sep 17 00:00:00 2001 From: Edo Storm Date: Tue, 24 Oct 2023 16:27:01 +0200 Subject: [PATCH 1/6] added error as solution. Not ideal, because self is still selectable remove supervisors from reviewers (issue 545) --- proposals/forms.py | 13 ++++++++++--- reviews/views.py | 7 +++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/proposals/forms.py b/proposals/forms.py index fee52a503..1bef24859 100644 --- a/proposals/forms.py +++ b/proposals/forms.py @@ -65,7 +65,6 @@ class Meta: } _soft_validation_fields = ['relation', - 'supervisor', 'other_applicants', 'other_stakeholders', 'stakeholders', @@ -183,8 +182,17 @@ def clean(self): self.mark_soft_required(cleaned_data, 'date_start') relation = cleaned_data.get('relation') + supervisor = cleaned_data.get('supervisor') + + if relation and relation.needs_supervisor and \ + supervisor == self.user: + error = forms.ValidationError( + _('De eindverantwoordelijke onderzoeker moet iemand anders zijn dan jijzelf.') + ) + self.add_error('supervisor', error) + if relation and relation.needs_supervisor and \ - not cleaned_data.get('supervisor'): + not supervisor: error = forms.ValidationError( _('Je dient een eindverantwoordelijke op te geven.'), code='required') @@ -196,7 +204,6 @@ def clean(self): other_applicants = cleaned_data.get('other_applicants') applicants = cleaned_data.get('applicants') - supervisor = cleaned_data.get('supervisor') # Always make sure the applicant is actually in the applicants list if self.user not in applicants and self.user != supervisor: diff --git a/reviews/views.py b/reviews/views.py index 9a08ce0c1..cc721731f 100644 --- a/reviews/views.py +++ b/reviews/views.py @@ -156,13 +156,16 @@ def get_review_counts_last_year(self): decisions = self.get_committee_decisions() - reviewers = get_user_model().objects.filter(decision__in=decisions) + reviewers = set([d.reviewer.id for d in decisions if not d.reviewer == d.review.proposal.supervisor]) + + reviewers_no_supervisor = get_user_model().objects.filter(id__in=reviewers) + base_filter = Q( decision__review__date_start__gt=self.start_date, decision__review__date_start__lt=self.end_date, decision__review__stage__gt=Review.SUPERVISOR, ) - return reviewers.annotate( + return reviewers_no_supervisor.annotate( total=Count("decision", filter=base_filter), num_short_route=Count( "decision", From cfdd749931cfd4cca4cbc3446320fa1cbe2d2b79 Mon Sep 17 00:00:00 2001 From: Edo Storm Date: Thu, 26 Oct 2023 11:44:20 +0200 Subject: [PATCH 2/6] Desiree's suggested rewording for supervisor minor change remove issue 545 from this branch --- proposals/forms.py | 15 ++++++------- .../0049_alter_proposal_supervisor.py | 21 +++++++++++++++++++ proposals/models.py | 20 ++++++++++-------- .../vue_templates/proposal_archive_list.html | 2 +- .../vue_templates/proposal_list.html | 2 +- reviews/views.py | 6 ++---- 6 files changed, 44 insertions(+), 22 deletions(-) create mode 100644 proposals/migrations/0049_alter_proposal_supervisor.py diff --git a/proposals/forms.py b/proposals/forms.py index 1bef24859..cc011526e 100644 --- a/proposals/forms.py +++ b/proposals/forms.py @@ -165,6 +165,7 @@ def clean(self): """ Check for conditional requirements: - If relation needs supervisor, make sure supervisor is set + - If relation needs supervisor, make sure supervisor is a different person - If other_applicants is checked, make sure applicants are set - If other_stakeholders is checked, make sure stakeholders is not empty - Maximum number of words for summary @@ -183,13 +184,6 @@ def clean(self): relation = cleaned_data.get('relation') supervisor = cleaned_data.get('supervisor') - - if relation and relation.needs_supervisor and \ - supervisor == self.user: - error = forms.ValidationError( - _('De eindverantwoordelijke onderzoeker moet iemand anders zijn dan jijzelf.') - ) - self.add_error('supervisor', error) if relation and relation.needs_supervisor and \ not supervisor: @@ -198,6 +192,13 @@ def clean(self): code='required') self.add_error('supervisor', error) + if relation and relation.needs_supervisor and \ + supervisor == self.user: + error = forms.ValidationError( + _('Je kunt niet jezelf als eindverantwoordelijke opgeven.') + ) + self.add_error('supervisor', error) + if relation.check_in_course: self.mark_soft_required(cleaned_data, 'student_context') self.mark_soft_required(cleaned_data, 'student_justification') diff --git a/proposals/migrations/0049_alter_proposal_supervisor.py b/proposals/migrations/0049_alter_proposal_supervisor.py new file mode 100644 index 000000000..3ea070c38 --- /dev/null +++ b/proposals/migrations/0049_alter_proposal_supervisor.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.20 on 2023-10-26 09:38 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('proposals', '0048_auto_20231016_1202'), + ] + + operations = [ + migrations.AlterField( + model_name='proposal', + name='supervisor', + field=models.ForeignKey(blank=True, help_text='Je aanvraag moet, als je alles hebt ingevuld, via de portal \n naar je promotor of begeleider gestuurd worden. Deze persoon \n is de eindverantwoordelijk onderzoeker, en zal de aanvraag \n vervolgens waar nodig kunnen aanpassen en indienen bij de FETC-GW.\n

Belangrijk: als je je promotor of \n begeleider niet kunt vinden met dit veld, dan moeten zij \n waarschijnlijk eerst één keer inloggen in deze portal. \n Je kunt nog wel verder met de aanvraag, maar vergeet dit veld \n niet in te vullen voor je de aanvraag indient. Je aanvraag \n zal dan namelijk niet in behandeling kunnen worden genomen.', null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL, verbose_name='Promotor/Begeleider'), + ), + ] diff --git a/proposals/models.py b/proposals/models.py index 864266d5e..df5d9492f 100644 --- a/proposals/models.py +++ b/proposals/models.py @@ -514,17 +514,19 @@ class Proposal(models.Model): supervisor = models.ForeignKey( settings.AUTH_USER_MODEL, - verbose_name=_('Eindverantwoordelijke onderzoeker'), + verbose_name=_('Promotor/Begeleider'), blank=True, null=True, - help_text=_('''Aan het einde van de procedure kan je deze aanvraag ter - verificatie naar je eindverantwoordelijke sturen. De - eindverantwoordelijke zal de aanvraag vervolgens kunnen aanpassen en - indienen bij de FETC-GW.

NB: als je je - eindverantwoordelijke niet kunt vinden met dit veld, moeten zij - waarschijnlijk eerst één keer inloggen in deze portal. Je kunt nog wel - verder met de aanvraag, maar vergeet dit veld niet in te vullen voor je de - aanvraag indient.'''), + help_text=_('''Je aanvraag moet, als je alles hebt ingevuld, via de portal + naar je promotor of begeleider gestuurd worden. Deze persoon + is de eindverantwoordelijk onderzoeker, en zal de aanvraag + vervolgens waar nodig kunnen aanpassen en indienen bij de FETC-GW. +

Belangrijk: als je je promotor of + begeleider niet kunt vinden met dit veld, dan moeten zij + waarschijnlijk eerst één keer inloggen in deze portal. + Je kunt nog wel verder met de aanvraag, maar vergeet dit veld + niet in te vullen voor je de aanvraag indient. Je aanvraag + zal dan namelijk niet in behandeling kunnen worden genomen.'''), on_delete=models.CASCADE, ) diff --git a/proposals/templates/proposals/vue_templates/proposal_archive_list.html b/proposals/templates/proposals/vue_templates/proposal_archive_list.html index d5e8473ef..a2bb72f80 100644 --- a/proposals/templates/proposals/vue_templates/proposal_archive_list.html +++ b/proposals/templates/proposals/vue_templates/proposal_archive_list.html @@ -112,7 +112,7 @@

- {% trans "Eindverantwoordelijke onderzoeker" %}: + {% trans "Promotor/Begeleider" %}: diff --git a/proposals/templates/proposals/vue_templates/proposal_list.html b/proposals/templates/proposals/vue_templates/proposal_list.html index bd774a319..53edb90be 100644 --- a/proposals/templates/proposals/vue_templates/proposal_list.html +++ b/proposals/templates/proposals/vue_templates/proposal_list.html @@ -163,7 +163,7 @@

- {% trans "Eindverantwoordelijke onderzoeker" %}: + {% trans "Promotor/Begeleider" %}: diff --git a/reviews/views.py b/reviews/views.py index cc721731f..bb1bb383c 100644 --- a/reviews/views.py +++ b/reviews/views.py @@ -156,16 +156,14 @@ def get_review_counts_last_year(self): decisions = self.get_committee_decisions() - reviewers = set([d.reviewer.id for d in decisions if not d.reviewer == d.review.proposal.supervisor]) - - reviewers_no_supervisor = get_user_model().objects.filter(id__in=reviewers) + reviewers = get_user_model().objects.filter(decision__in=decisions) base_filter = Q( decision__review__date_start__gt=self.start_date, decision__review__date_start__lt=self.end_date, decision__review__stage__gt=Review.SUPERVISOR, ) - return reviewers_no_supervisor.annotate( + return reviewers.annotate( total=Count("decision", filter=base_filter), num_short_route=Count( "decision", From 5b70cb0d6d53b5fb5ef0d6c9ec1a8d4f78e74e38 Mon Sep 17 00:00:00 2001 From: Edo Storm Date: Wed, 1 Nov 2023 13:51:42 +0100 Subject: [PATCH 3/6] Ensured user cannot select themself as supervisor --- proposals/forms.py | 3 ++- .../templates/proposals/proposal_form.html | 20 ++----------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/proposals/forms.py b/proposals/forms.py index cc011526e..86787b78d 100644 --- a/proposals/forms.py +++ b/proposals/forms.py @@ -56,7 +56,7 @@ class Meta: }), 'funding': forms.CheckboxSelectMultiple(), 'applicants': SelectMultipleUser(), - 'supervisor': SelectUser(), + 'supervisor': forms.Select(), } error_messages = { 'title': { @@ -131,6 +131,7 @@ def __init__(self, *args, **kwargs): self.fields['supervisor'].choices = [(None, _( 'Selecteer...'))] + get_users_as_list(supervisors) + self.fields['applicants'].choices = get_users_as_list(applicants) if in_course: diff --git a/proposals/templates/proposals/proposal_form.html b/proposals/templates/proposals/proposal_form.html index d37706bdc..24f7cd99d 100644 --- a/proposals/templates/proposals/proposal_form.html +++ b/proposals/templates/proposals/proposal_form.html @@ -11,7 +11,7 @@