From 00c8088d2f41ad1d505e27f4ed3770d72ff542ba Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 21 Nov 2024 14:11:29 -0500 Subject: [PATCH] feat(edit chants): allow chant edit to save with no initial full text Allow edits to chants that did not have an initial full text to be saved even if no full text is added. Add check for uniqueness of folio-c_sequence to the chant edit page. This requires inclusion of the chant source in the chant edit form: the form, view, and test are edited to add the source to the form. Refs: #1696. --- django/cantusdb_project/main_app/forms.py | 44 +++++++++- .../main_app/tests/make_fakes.py | 20 +++-- .../main_app/tests/test_views/test_chant.py | 87 ++++++++++++++++--- .../cantusdb_project/main_app/views/chant.py | 30 ++++++- 4 files changed, 157 insertions(+), 24 deletions(-) diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 322a70870..1279c5247 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -1,3 +1,5 @@ +from typing import Optional, Any + from django import forms from django.contrib.auth.forms import ReadOnlyPasswordHashField from django.contrib.auth import get_user_model @@ -372,6 +374,7 @@ class Meta: "incipit_of_refrain", "later_addition", "rubrics", + "source", ] widgets = { # manuscript_full_text_std_spelling: defined below (required) & special field @@ -413,7 +416,7 @@ class Meta: widget=TextAreaWidget, help_text=Chant._meta.get_field("manuscript_full_text_std_spelling").help_text, label="Full text as in Source (standardized spelling)", - required=True, + required=False, ) manuscript_full_text = CantusDBLatinField( @@ -441,6 +444,45 @@ class Meta: required=False, ) + def clean_manuscript_full_text_std_spelling(self) -> Optional[str]: + """ + Provide a custom validation function for the + manuscript_full_text_std_spelling field to ensure that + if it initially contained text, it cannot be made blank. + """ + if ( + self["manuscript_full_text_std_spelling"].initial + and not self["manuscript_full_text_std_spelling"].data + ): + raise forms.ValidationError( + "This field cannot be blank for this chant.", + code="txt-req-prev-existing", + ) + entered_text: str = self["manuscript_full_text_std_spelling"].data + return entered_text + + def clean(self) -> dict[str, Any]: + """ + Custom validation to ensure that the edited chant does not duplicate + the folio and c_sequence of an already-existing chant. + """ + # Call super().clean() to ensure that the form's built-in validation + # is run before our custom validation. + super().clean() + folio = self.cleaned_data["folio"] + c_sequence = self.cleaned_data["c_sequence"] + source = self.cleaned_data["source"] + if ( + source.chant_set.exclude(id=self.instance.id) + .filter(folio=folio, c_sequence=c_sequence) + .exists() + ): + raise forms.ValidationError( + "A chant with this folio and sequence already exists.", + code="duplicate-folio-sequence", + ) + return self.cleaned_data + class SourceEditForm(forms.ModelForm): class Meta: diff --git a/django/cantusdb_project/main_app/tests/make_fakes.py b/django/cantusdb_project/main_app/tests/make_fakes.py index 97a789528..4d42940d7 100644 --- a/django/cantusdb_project/main_app/tests/make_fakes.py +++ b/django/cantusdb_project/main_app/tests/make_fakes.py @@ -5,6 +5,7 @@ from faker import Faker # type: ignore[import-untyped] from django.contrib.auth import get_user_model +from django.db.models import Max from main_app.models.century import Century from main_app.models.chant import Chant @@ -23,9 +24,6 @@ User = get_user_model() -# Max positive integer accepted by Django's positive integer field -MAX_SEQUENCE_NUMBER = 2147483647 - # Create a Faker instance with locale set to Latin faker = Faker("la") @@ -168,16 +166,23 @@ def make_fake_chant(**kwargs: Any) -> Chant: - manuscript_full_text, indexing_notes, manuscript_syllabized_full_text - manuscript_full_text_proofread, volpiano_proofread, manuscript_full_text_std_proofread (default to False) """ - # Handle `source`, `folio`, `c_sequence`, and `manuscript_full_text_std_spelling` fields, + # Handle `source`, `folio`, and `c_sequence` fields, # which cannot be set to None if kwargs.get("source") is None: kwargs["source"] = make_fake_source(segment_name="CANTUS Database") if kwargs.get("folio") is None: kwargs["folio"] = faker.bothify("##?") if kwargs.get("c_sequence") is None: - kwargs["c_sequence"] = random.randint(1, MAX_SEQUENCE_NUMBER) - if kwargs.get("manuscript_full_text_std_spelling") is None: - kwargs["manuscript_full_text_std_spelling"] = faker.sentence() + # When c_sequence is not specified, iterate + 1 on the maximum c_sequence + # of the chants with the same source and folio + current_max_c_sequence = ( + kwargs["source"] + .chant_set.filter(folio=kwargs["folio"]) + .aggregate(Max("c_sequence"))["c_sequence__max"] + ) + kwargs["c_sequence"] = ( + current_max_c_sequence + 1 if current_max_c_sequence else 1 + ) kwargs["marginalia"] = kwargs.get("marginalia", make_random_string(1)) kwargs["service"] = kwargs.get("service", make_fake_service()) @@ -212,6 +217,7 @@ def make_fake_chant(**kwargs: Any) -> Chant: # The following fields, when not specified, are generated with a random sentence for field in [ + "manuscript_full_text_std_spelling", "manuscript_full_text", "indexing_notes", "manuscript_syllabized_full_text", diff --git a/django/cantusdb_project/main_app/tests/test_views/test_chant.py b/django/cantusdb_project/main_app/tests/test_views/test_chant.py index e7f2ebecb..92a3608a3 100644 --- a/django/cantusdb_project/main_app/tests/test_views/test_chant.py +++ b/django/cantusdb_project/main_app/tests/test_views/test_chant.py @@ -181,10 +181,9 @@ def test_url_and_templates(self): response = self.client.get(reverse("source-edit-chants", args=[source1.id])) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "chant_edit.html") - with patch("requests.get", mock_requests_get): - response = self.client.get( - reverse("source-edit-chants", args=[source1.id + 100]) - ) + response = self.client.get( + reverse("source-edit-chants", args=[source1.id + 100]) + ) self.assertEqual(response.status_code, 404) self.assertTemplateUsed(response, "404.html") @@ -200,10 +199,9 @@ def test_update_chant(self): chant = make_fake_chant( source=source, manuscript_full_text_std_spelling="initial" ) - with patch("requests.get", mock_requests_get): - response = self.client.get( - reverse("source-edit-chants", args=[source.id]), {"pk": chant.id} - ) + response = self.client.get( + reverse("source-edit-chants", args=[source.id]), {"pk": chant.id} + ) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "chant_edit.html") @@ -270,10 +268,7 @@ def test_volpiano_signal(self): "pk": chant_2.id, }, ) - with patch("requests.get", mock_requests_get): - chant_2 = Chant.objects.get( - manuscript_full_text_std_spelling="resonare foobaz" - ) + chant_2 = Chant.objects.get(manuscript_full_text_std_spelling="resonare foobaz") self.assertEqual(chant_2.volpiano, expected_volpiano) self.assertEqual(chant_2.volpiano_intervals, expected_intervals) @@ -369,6 +364,64 @@ def test_invalid_text(self) -> None: "Word [ contains non-alphabetic characters.", ) + def test_full_text_requirement(self): + """ + When editing a chant, we generally require a full text to be provided + (this is also true when creating a chant); however, some chants were + created before this was required and so may not have full text. In + these cases, the user should be allowed to save edits to a chant (for + example, a correction to another field) without having to provide a + full text. + """ + source = make_fake_source() + with self.subTest("Chant with full text"): + chant_w_fulltext = make_fake_chant( + source=source, manuscript_full_text_std_spelling="Plena sum", mode=None + ) + response = self.client.post( + reverse("source-edit-chants", args=[source.id]), + { + "manuscript_full_text_std_spelling": "", + "folio": chant_w_fulltext.folio, + "c_sequence": chant_w_fulltext.c_sequence, + "pk": chant_w_fulltext.id, + "mode": "2", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertFormError( + response.context["form"], + "manuscript_full_text_std_spelling", + "This field cannot be blank for this chant.", + ) + self.assertEqual(response.context["form"]["mode"].data, "2") + self.assertEqual( + response.context["form"]["manuscript_full_text_std_spelling"].data, + "Plena sum", + ) + chant_w_fulltext.refresh_from_db() + self.assertEqual( + chant_w_fulltext.manuscript_full_text_std_spelling, "Plena sum" + ) + self.assertIsNone(chant_w_fulltext.mode) + with self.subTest("Chant with no full text"): + chant_wo_fulltext = make_fake_chant( + source=source, manuscript_full_text_std_spelling=None, mode="1" + ) + response = self.client.post( + reverse("source-edit-chants", args=[source.id]), + { + "folio": chant_wo_fulltext.folio, + "c_sequence": chant_wo_fulltext.c_sequence, + "pk": chant_wo_fulltext.id, + "mode": "2", + }, + ) + self.assertEqual(response.status_code, 302) + chant_wo_fulltext.refresh_from_db() + self.assertEqual(chant_wo_fulltext.mode, "2") + self.assertEqual(chant_wo_fulltext.manuscript_full_text, "") + class ChantEditSyllabificationViewTest(TestCase): @classmethod @@ -2838,7 +2891,15 @@ def test_empty_fulltext(self) -> None: """post with correct source and empty full-text""" source = self.source url = reverse("chant-create", args=[source.id]) - response = self.client.post(url, data={"manuscript_full_text_std_spelling": ""}) + response = self.client.post( + url, + data={ + "manuscript_full_text_std_spelling": "", + "folio": "010r", + "c_sequence": "1", + }, + ) + self.assertFormError( response.context["form"], "manuscript_full_text_std_spelling", diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index 4d7fb3de8..986c91452 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -1201,10 +1201,18 @@ def get_context_data(self, **kwargs): context["suggested_fulltext"] = suggested_fulltext return context - def form_valid(self, form): - if not form.is_valid(): - return super().form_invalid(form) + def get_form_kwargs(self): + """ + If the request has a data parameter (ie. it is a PUT or POST request), + we copy the data and add the source id to it. + """ + kwargs = super().get_form_kwargs() + if "data" in kwargs: + kwargs["data"] = kwargs["data"].copy() + kwargs["data"]["source"] = self.source.id + return kwargs + def form_valid(self, form): user: User = self.request.user chant: Chant = form.instance @@ -1239,6 +1247,22 @@ def form_valid(self, form): messages.success(self.request, "Chant updated successfully!") return return_response + def form_invalid(self, form): + """ + If the form is invalid with an error message on the + manuscript_full_text_std_spelling field ("Field cannot be blank + on this chant"), we display the edit form with all edited data + but the original manuscript_full_text_std_spelling field value. + """ + if form.has_error("manuscript_full_text_std_spelling", "txt-req-prev-existing"): + data = self.request.POST.copy() + data["manuscript_full_text_std_spelling"] = form[ + "manuscript_full_text_std_spelling" + ].initial + form.data = data + return super().form_invalid(form) + return super().form_invalid(form) + def get_success_url(self): # Take user back to the referring page # `ref` url parameter is used to indicate referring page