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.

Sign up for GitHub

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

Temp/embargo and tests wip #835

Merged
merged 8 commits into from
Jan 8, 2025
Merged

Temp/embargo and tests wip #835

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
12e4991
wip: Fixing proposal tests
miggol Jan 7, 2025
c66e841
fix: use the test pdf for tests
EdoStorm96 Jan 7, 2025
c83f108
fix: bug concerning PreAss Wmo in tests
EdoStorm96 Jan 7, 2025
38947e0
fix: remove if True
EdoStorm96 Jan 7, 2025
c793bf9
style: black
EdoStorm96 Jan 7, 2025
31ac676
feat: comment about final_validation
EdoStorm96 Jan 8, 2025
54360ab
feat: explicitly add wmo to proposal in tests
EdoStorm96 Jan 8, 2025
790b5d0
style: black
EdoStorm96 Jan 8, 2025
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
121 changes: 69 additions & 52 deletions proposals/fixtures/testing/test_proposals.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,23 @@
"model": "proposals.proposal",
"pk": 1,
"fields": {
"reference_number": "23-003-01",
"reference_number": "24-009-01",
"reviewing_committee": 3,
"institution": 2,
"date_start": "2025-01-01",
"title": "Fixture proposal",
"summary": "asdf",
"institution": 4,
"date_start": "2024-12-27",
"title": "Normal test proposal",
"summary": "fsefsdfes",
"other_applicants": false,
"other_stakeholders": false,
"stakeholders": "",
"translated_forms": true,
"translated_forms_languages": "asdf",
"funding_details": "",
"funding_name": "",
"comments": "asdf",
"inform_local_staff": true,
"embargo": true,
"embargo_end_date": "2024-09-01",
"translated_forms": false,
"translated_forms_languages": null,
"funding_details": "fsefdsf",
"funding_name": "esfdegf",
"comments": "aaaaa",
"inform_local_staff": null,
"embargo": false,
"embargo_end_date": null,
"in_archive": false,
"is_pre_assessment": false,
"pre_assessment_pdf": "",
Expand All @@ -28,23 +28,28 @@
"pre_approval_pdf": "",
"in_course": false,
"is_exploration": false,
"pdf": "FETC-AK-23-003-01-Proposal.pdf",
"studies_similar": false,
"studies_number": 3,
"pdf": "",
"studies_similar": true,
"studies_number": 1,
"status": 1,
"status_review": null,
"dmp_file": "FETC-AK-23-003-01-DMP.pdf",
"privacy_officer": false,
"dmp_file": "",
"confirmation_comments": "",
"date_created": "2023-11-28T12:59:17.670Z",
"date_modified": "2023-11-28T13:36:55.297Z",
"date_created": "2024-12-19T14:52:13.624Z",
"date_modified": "2024-12-23T10:29:26.363Z",
"date_submitted_supervisor": null,
"date_reviewed_supervisor": null,
"date_submitted": "2023-11-28T13:36:55.296Z",
"date_submitted": null,
"date_reviewed": null,
"date_confirmed": null,
"has_minor_revision": false,
"minor_revision_description": null,
"self_assessment": "asdf",
"self_assessment": "gsdfges",
"knowledge_security": "N",
"knowledge_security_details": "",
"researcher_risk": "N",
"researcher_risk_details": "",
"relation": 5,
"student_program": "",
"student_context": null,
Expand All @@ -55,7 +60,7 @@
"parent": null,
"is_revision": false,
"funding": [
1
3
],
"applicants": [
5
Expand All @@ -66,23 +71,23 @@
"model": "proposals.proposal",
"pk": 2,
"fields": {
"reference_number": "23-004-01",
"reference_number": "24-010-01",
"reviewing_committee": 3,
"institution": 2,
"date_start": "2024-08-01",
"title": "Fixture Preassessment",
"institution": 4,
"date_start": "2042-12-25",
"title": "Preassessment test proposal",
"summary": "",
"other_applicants": false,
"other_stakeholders": true,
"stakeholders": "asdf",
"other_stakeholders": false,
"stakeholders": "",
"translated_forms": null,
"translated_forms_languages": null,
"funding_details": "",
"funding_name": "",
"comments": "asdf",
"funding_name": "aaaaa",
"comments": "asdfda",
"inform_local_staff": null,
"embargo": true,
"embargo_end_date": "2024-11-01",
"embargo": null,
"embargo_end_date": null,
"in_archive": false,
"is_pre_assessment": true,
"pre_assessment_pdf": "test_pdf.pdf",
Expand All @@ -91,23 +96,28 @@
"pre_approval_pdf": "",
"in_course": false,
"is_exploration": false,
"pdf": "FETC-AK-23-004-01-Proposal.pdf",
"pdf": "",
"studies_similar": null,
"studies_number": 1,
"status": 1,
"status_review": null,
"privacy_officer": null,
"dmp_file": "",
"confirmation_comments": "",
"date_created": "2023-11-28T14:10:12.708Z",
"date_modified": "2023-11-28T14:10:40.554Z",
"date_created": "2024-12-23T10:30:40.272Z",
"date_modified": "2024-12-23T10:35:22.000Z",
"date_submitted_supervisor": null,
"date_reviewed_supervisor": null,
"date_submitted": "2023-11-28T14:10:40.554Z",
"date_submitted": null,
"date_reviewed": null,
"date_confirmed": null,
"has_minor_revision": false,
"minor_revision_description": null,
"self_assessment": "asdf",
"knowledge_security": "",
"knowledge_security_details": "",
"researcher_risk": "",
"researcher_risk_details": "",
"relation": 5,
"student_program": "",
"student_context": null,
Expand All @@ -117,7 +127,9 @@
"supervisor": null,
"parent": null,
"is_revision": false,
"funding": [],
"funding": [
2
],
"applicants": [
5
]
Expand All @@ -127,48 +139,53 @@
"model": "proposals.proposal",
"pk": 3,
"fields": {
"reference_number": "23-005-01",
"reference_number": "24-011-01",
"reviewing_committee": 3,
"institution": 2,
"date_start": "2024-08-01",
"title": "Fixture Preapproval",
"summary": "asdf",
"institution": 4,
"date_start": "2046-12-21",
"title": "Preapproved test pdf",
"summary": "asd",
"other_applicants": false,
"other_stakeholders": false,
"stakeholders": "",
"translated_forms": null,
"translated_forms": false,
"translated_forms_languages": null,
"funding_details": "",
"funding_name": "",
"comments": "asdf",
"comments": "asd",
"inform_local_staff": null,
"embargo": true,
"embargo_end_date": "2025-01-10",
"embargo": false,
"embargo_end_date": null,
"in_archive": false,
"is_pre_assessment": false,
"pre_assessment_pdf": "",
"is_pre_approved": true,
"pre_approval_institute": "asdf",
"pre_approval_institute": "asd",
"pre_approval_pdf": "test_pdf.pdf",
"in_course": false,
"is_exploration": false,
"pdf": "FETC-AK-23-005-01-Proposal.pdf",
"pdf": "",
"studies_similar": null,
"studies_number": 1,
"status": 1,
"status_review": null,
"privacy_officer": true,
"dmp_file": "",
"confirmation_comments": "",
"date_created": "2023-11-28T14:28:19.045Z",
"date_modified": "2023-11-28T14:28:31.971Z",
"date_created": "2024-12-23T11:15:55.495Z",
"date_modified": "2024-12-23T11:17:02.358Z",
"date_submitted_supervisor": null,
"date_reviewed_supervisor": null,
"date_submitted": "2023-11-28T14:28:31.971Z",
"date_submitted": null,
"date_reviewed": null,
"date_confirmed": null,
"has_minor_revision": false,
"minor_revision_description": null,
"self_assessment": "asdf",
"self_assessment": "asd",
"knowledge_security": "",
"knowledge_security_details": "",
"researcher_risk": "",
"researcher_risk_details": "",
"relation": 5,
"student_program": "",
"student_context": null,
Expand Down
73 changes: 46 additions & 27 deletions proposals/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,8 @@ def __init__(self, *args, **kwargs):
self.request = kwargs.pop("request", None)

# Needed for validation
self.final_validation = kwargs.pop("final_validation", None)
final_validation = kwargs.pop("final_validation", None)
self.final_validation = None
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I intended to add a comment along the lines of

"we save this variable for later, but want it set for None for this part of the validation"

Because as it stands this is kind of confusing.


super(ProposalSubmitForm, self).__init__(*args, **kwargs)

Expand All @@ -847,6 +848,13 @@ def __init__(self, *args, **kwargs):
del self.fields["embargo"]
del self.fields["embargo_end_date"]

self.final_validation = final_validation

# If we have an existing instance and we're not POSTing,
# run an initial clean
if self.instance.pk and "data" not in kwargs:
self.initial_clean()

def clean(self):
"""
Check if the Proposal is complete:
Expand All @@ -857,53 +865,64 @@ def clean(self):

cleaned_data = super(ProposalSubmitForm, self).clean()

if (
check_local_facilities(self.proposal)
and cleaned_data["inform_local_staff"] is None
):
self.add_error("inform_local_staff", _("Dit veld is verplicht."))

if (
not self.instance.is_practice()
and not "js-redirect-submit" in self.request.POST
and not "save_back" in self.request.POST
):
if (
check_local_facilities(self.proposal)
and cleaned_data["inform_local_staff"] is None
):
self.add_error("inform_local_staff", _("Dit veld is verplicht."))

if cleaned_data["embargo"] is None:
self.add_error("embargo", _("Dit veld is verplicht."))
self.validate_embargo(cleaned_data)

embargo_end_date = cleaned_data["embargo_end_date"]
two_years_from_now = timezone.now().date() + timezone.timedelta(days=730)

if "embargo" in cleaned_data:
if cleaned_data["embargo"] is True and not embargo_end_date:
self.add_error(
"embargo_end_date",
_("Vul een datum in waarop de embargo afloopt."),
)

if embargo_end_date is not None and embargo_end_date > two_years_from_now:
self.add_error(
"embargo_end_date",
_(
"De embargo-periode kan maximaal 2 jaar zijn. Kies een datum binnen 2 jaar van vandaag."
),
)
# final_validation is a kwarg that should only be given by the
# actual submit view, not the stepper.
if not self.final_validation:
return
# For final validation, i.e. if a user is actually submitting,we
# For final validation, i.e. if a user is actually submitting, we
# instantiate a new stepper to reflect changes
# made to the current instance, which may not yet be saved.
from proposals.utils.stepper import Stepper

validator = Stepper(self.instance, request=self.request)
# And then we append stepper errors to show them to the user
# on the submit page
if validator.get_form_errors():
self.add_error(
None,
_("Aanvraag bevat nog foutmeldingen"),
)

def validate_embargo(self, cleaned_data):

# Only check embargo and end date if form was initialized with it.
if "embargo" not in self.fields:
return

self.mark_soft_required(cleaned_data, "embargo")

if "embargo" in cleaned_data:
self.check_dependency(
cleaned_data,
"embargo",
"embargo_end_date",
error_message=_("Vul een datum in waarop de embargo afloopt."),
)
embargo_end_date = cleaned_data.get("embargo_end_date", None)
if embargo_end_date:
two_years_from_now = timezone.now().date() + timezone.timedelta(days=730)
if embargo_end_date > two_years_from_now:
self.add_error(
"embargo_end_date",
_(
"De embargo-periode kan maximaal 2 jaar zijn. "
"Kies een datum binnen 2 jaar van vandaag."
),
)


class KnowledgeSecurityForm(SoftValidationMixin, ConditionalModelForm):

Expand Down
16 changes: 15 additions & 1 deletion proposals/tests/proposal_submission_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.test import TestCase

from main.tests import BaseViewTestCase
from proposals.models import Proposal, Relation
from proposals.models import Proposal, Relation, Wmo
from reviews.models import Review

from proposals.views.proposal_views import (
Expand Down Expand Up @@ -77,6 +77,12 @@ def setup_proposal(self):
Please note, this currently uses a mish-mash of both fixtures
and programmatically created users and objects.
"""
self.wmo = Wmo(
status=0,
metc="N",
is_medical="N",
)
self.wmo.save()
Comment on lines +80 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

This wmo must be implicitly getting used for the regular self.proposal.wmo without getting explicitly assigned. I don't know why (it's very confusing), but perhaps it would be better to add self.proposal.wmo = self.wmo down here:

    def setup_proposal(self):
        """
        Load our test proposals from a fixture, then add our user as
        an applicant to each of them.

        Please note, this currently uses a mish-mash of both fixtures
        and programmatically created users and objects.
        """
        self.wmo = Wmo(
            status=0,
            metc="N",
            is_medical="N",
        )
        self.wmo.save()
        self.proposal = Proposal.objects.get(pk=1)
        self.proposal.applicants.add(self.user)
        self.proposal.wmo = self.wmo
        self.proposal.save()

Right before the proposal gets saved. Just to make it explicit and less magic.

self.proposal = Proposal.objects.get(pk=1)
self.proposal.applicants.add(self.user)
self.proposal.save()
Expand Down Expand Up @@ -156,6 +162,7 @@ def test_submission_unsupervised(self):
self.get_post_data(),
user=self.user,
)

self.assertIn(page.status_code, [302, 200])
self.refresh()
# Post-submission tests
Expand Down Expand Up @@ -225,6 +232,13 @@ class PreassessmentSubmitTestCase(
def setUp(self):
super().setUp()
self.proposal = self.pre_assessment
wmo = Wmo(
status=0,
metc="N",
is_medical="N",
)
wmo.save()
self.proposal.wmo = wmo


class PreapprovedSubmitTestCase(
Expand Down
1 change: 1 addition & 0 deletions proposals/views/proposal_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,7 @@ def form_valid(self, form):
if (
"save_back" not in self.request.POST
and "js-redirect-submit" not in self.request.POST
and not form.instance.is_practice()
):
start_review(proposal)
return success_response
Expand Down
Loading