Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/4 small issues #579

Merged
merged 6 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions proposals/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,17 +573,17 @@ def clean(self):
class ProposalDataManagementForm(SoftValidationMixin, forms.ModelForm):
class Meta:
model = Proposal
fields = ['avg_understood', 'dmp_file']
fields = ['privacy_officer', 'dmp_file']
widgets = {
'avg_understood': forms.RadioSelect(choices=YES_NO),
'privacy_officer': forms.RadioSelect(choices=YES_NO),
}

def clean(self):
cleaned_data = super(ProposalDataManagementForm, self).clean()

if cleaned_data['avg_understood'] is None:
if cleaned_data['privacy_officer'] is None:
self.add_error(
'avg_understood',
'privacy_officer',
_('Dit veld is verplicht om verder te gaan.')
)

Expand Down
8 changes: 6 additions & 2 deletions proposals/migrations/0034_auto_20211213_1503.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
import django.db.models.deletion
import main.validators
import proposals.utils.proposal_utils
import proposals.validators

def AVGUnderstoodValidator():
'''This was formerly a validator, imported from proposals.validators,
but it is currently no longer required, so it has been removed.
To prevent an error, it has been replaced with this stub.'''
pass

class Migration(migrations.Migration):

Expand All @@ -18,7 +22,7 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='proposal',
name='avg_understood',
field=models.BooleanField(default=False, validators=[proposals.validators.AVGUnderstoodValidator], verbose_name='Ik heb kennis genomen van het bovenstaande en begrijp mijn verantwoordelijkheden ten opzichte van de AVG.'),
field=models.BooleanField(default=False, validators=[AVGUnderstoodValidator], verbose_name='Ik heb kennis genomen van het bovenstaande en begrijp mijn verantwoordelijkheden ten opzichte van de AVG.'),
),
migrations.AlterField(
model_name='proposal',
Expand Down
19 changes: 0 additions & 19 deletions proposals/migrations/0050_alter_proposal_avg_understood.py

This file was deleted.

24 changes: 24 additions & 0 deletions proposals/migrations/0050_replace_avg_understood.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 3.2.20 on 2023-11-16 09:41

from django.db import migrations, models


class Migration(migrations.Migration):

replaces = [('proposals', '0050_alter_proposal_avg_understood'), ('proposals', '0051_auto_20231116_1030')]

dependencies = [
('proposals', '0049_alter_proposal_supervisor'),
]

operations = [
migrations.RemoveField(
model_name='proposal',
name='avg_understood',
),
migrations.AddField(
model_name='proposal',
name='privacy_officer',
field=models.BooleanField(blank=True, default=None, null=True, verbose_name='Ik heb mijn aanvraag en de documenten voor deelnemers besproken met de privacy officer.'),
),
]
4 changes: 1 addition & 3 deletions proposals/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

from main.models import YES, YES_NO_DOUBT
from main.validators import MaxWordsValidator, validate_pdf_or_doc
from .validators import AVGUnderstoodValidator
from .utils import available_urls, FilenameFactory, OverwriteStorage
from datetime import date, timedelta

Expand Down Expand Up @@ -399,12 +398,11 @@ class Proposal(models.Model):
blank=True,
)

avg_understood = models.BooleanField(
privacy_officer = models.BooleanField(
_('Ik heb mijn aanvraag en de documenten voor deelnemers besproken met de privacy officer.'),
default=None,
null=True,
blank=True,
validators=[AVGUnderstoodValidator],
)

dmp_file = models.FileField(
Expand Down
5 changes: 2 additions & 3 deletions proposals/templates/proposals/proposal_pdf.html
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,9 @@

{% block page_header %}
<div id="page-header">
{% blocktrans with title=proposal.title reference_number=proposal.reference_number submitter=proposal.created_by.get_full_name trimmed %}
FETC-GW - <em>{{ title }}</em> (referentienummer {{ reference_number }}, ingediend door {{ submitter }})
{% blocktrans with title=proposal.title reference_number=proposal.reference_number submitter=proposal.created_by.get_full_name reviewing_committee=proposal.reviewing_committee.name trimmed %}
FETC-GW - <em>{{ title }}</em> (referentienummer {{ reference_number }}-{{reviewing_committee}}, ingediend door {{ submitter }})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hate to be that guy... But conventionally we place the committee before the rest of the refnum.

Sorry, should've clarified that in my original comment

{% endblocktrans %}
- {% trans proposal.reviewing_committee.name %}
{% if proposal.is_revision %}
<br>
<strong>
Expand Down
2 changes: 1 addition & 1 deletion proposals/utils/pdf_diff_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ class DMPFileSection(PageBreakMixin, BaseSection):

section_title = _("Data Management Plan")

row_fields = ["dmp_file", "avg_understood"]
row_fields = ["dmp_file", "privacy_officer"]


class EmbargoSection(BaseSection):
Expand Down
11 changes: 0 additions & 11 deletions proposals/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,3 @@ def __call__(self, value):
if qs.exists():
raise forms.ValidationError(_('Er bestaat al een aanvraag met deze '
'titel.'), code='unique')


def AVGUnderstoodValidator(value):
# This does not seem to do anything, so I removed it from the model, however,
# if I try to remove this validator entirely I get an error when making
# migrations?
if value is None:
raise forms.ValidationError(
_('Je dient deze vraag in te vullen om jouw aanvraag in '
'te dienen'), code='avg'
)
1 change: 1 addition & 0 deletions reviews/migrations/0013_is_commission_review_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def update_is_commssion_review(apps, schema_editor):
SUPERVISOR_STAGE = 0
if review.stage == SUPERVISOR_STAGE:
review.is_commission_review = False
review.update_go()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory this should work... However, the production DB isn't as clean as we would like.

I'd feel better if the code was something like this:

if review.go:
    review.stage = CLOSED

It's a lot simpler, and should be safer.

review.save()

class Migration(migrations.Migration):
Expand Down
3 changes: 2 additions & 1 deletion reviews/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def update_go(self, last_decision=None):
self.save()

# For a supervisor review:
if self.stage == self.SUPERVISOR:
if self.is_commission_review == False:
# Update the status of the Proposal with the end date
self.proposal.date_reviewed_supervisor = self.date_end
self.proposal.save()
Expand All @@ -103,6 +103,7 @@ def update_go(self, last_decision=None):
# in an uWSGI environment, in which it errors.
from reviews.utils import start_assignment_phase
start_assignment_phase(self.proposal)
self.stage = self.CLOSED
# On NO-GO, reset the Proposal status
else:
# See comment above
Expand Down
2 changes: 1 addition & 1 deletion reviews/templates/reviews/review_detail_sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ <h3>
</em>
</li>
{% endif %}
{% if review.stage == review.SUPERVISOR %}
{% if review.is_commission_review == False %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's cleaner to write {% if not review.is_commission_review %}

<li>
<strong>
{% blocktrans trimmed %}
Expand Down
2 changes: 1 addition & 1 deletion reviews/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ def form_valid(self, form):

# Don't notify the secretary if this is a supervisor decision.
# If it was a GO they the secretary will be notified anyway
if not review.stage == review.SUPERVISOR:
if review.is_commission_review == True:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

== True is redundant

notify_secretary(form.instance)

return super(DecisionUpdateView, self).form_valid(form)
Loading