diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 322a70870..253ab3d02 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 @@ -168,6 +170,7 @@ class Meta: "incipit_of_refrain", "later_addition", "rubrics", + "source", ] # the widgets dictionary is ignored for a model field with a non-empty # choices attribute. In this case, you must override the form field to @@ -237,17 +240,23 @@ class Meta: help_text="Select the project (if any) that the chant belongs to.", ) - # automatically computed fields - # source and incipit are mandatory fields in model, - # but have to be optional in the form, otherwise the field validation won't pass - source = forms.ModelChoiceField( - queryset=Source.objects.all().order_by("title"), - required=False, - error_messages={ - "invalid_choice": "This source does not exist, please switch to a different source." - }, - ) - incipit = forms.CharField(required=False) + def clean(self) -> dict[str, Any]: + """ + Provide custom clean method that ensures the created 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.filter(folio=folio, c_sequence=c_sequence): + raise forms.ValidationError( + "Chant with the same sequence and folio already exists in this source.", + code="duplicate-folio-sequence", + ) + return self.cleaned_data class SourceCreateForm(forms.ModelForm): @@ -372,6 +381,7 @@ class Meta: "incipit_of_refrain", "later_addition", "rubrics", + "source", ] widgets = { # manuscript_full_text_std_spelling: defined below (required) & special field @@ -413,7 +423,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 +451,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/templates/browse_chants.html b/django/cantusdb_project/main_app/templates/browse_chants.html index 8ac640490..6de54bbab 100644 --- a/django/cantusdb_project/main_app/templates/browse_chants.html +++ b/django/cantusdb_project/main_app/templates/browse_chants.html @@ -24,7 +24,7 @@

Browse Chants

@@ -147,7 +147,9 @@

Browse Chants

{% endfor %} - {% include "pagination.html" %} +
+ {% include "pagination.html" %} +
{% else %} No chants found. {% endif %} 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..f0f373301 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -880,8 +880,16 @@ def test_func(self): return user_can_edit_chants_in_source(user, self.source) - # if success_url and get_success_url not specified, will direct to chant detail page def get_success_url(self): + """ + Get the incipit of the created chant (generated by a signal) + and display a success message. + """ + self.object.refresh_from_db() + messages.success( + self.request, + "Chant '" + self.object.incipit + "' created successfully!", + ) return reverse("chant-create", args=[self.source.id]) def get_initial(self): @@ -960,53 +968,24 @@ def get_context_data(self, **kwargs: Any) -> dict[Any, Any]: context["suggested_chants"] = suggested_chants return context - def form_valid(self, form): + def get_form_kwargs(self): """ - Validates the new chant. - - Custom validation steps are: - - Check if a chant with the same sequence and folio already exists in the source. - - Compute the chant incipit. - - Adds the "created_by" and "updated_by" fields to the chant. + In the case of a submitted form (there is data in the request), + we copy the data dictionary and add the source id to it. """ - # compute source - form.instance.source = self.source - - # compute incipit, within 30 charactors, keep words complete - words = form.instance.manuscript_full_text_std_spelling.split(" ") - incipit = "" - for word in words: - new_incipit = incipit + word + " " - if len(new_incipit) >= 30: - break - incipit = new_incipit + kwargs = super().get_form_kwargs() + if "data" in kwargs: + kwargs["data"] = kwargs["data"].copy() + kwargs["data"]["source"] = self.source.id + return kwargs - form.instance.incipit = incipit.strip(" ") - - # if a chant with the same sequence and folio already exists in the source - if ( - Chant.objects.all() - .filter( - source=self.source, - folio=form.instance.folio, - c_sequence=form.instance.c_sequence, - ) - .exists() - ): - form.add_error( - None, - "Chant with the same sequence and folio already exists in this source.", - ) - - if form.is_valid(): - form.instance.created_by = self.request.user - form.instance.last_updated_by = self.request.user - messages.success( - self.request, - "Chant '" + form.instance.incipit + "' created successfully!", - ) - return super().form_valid(form) - return super().form_invalid(form) + def form_valid(self, form): + """ + Adds the "created_by" and "updated_by" fields to the chant. + """ + form.instance.created_by = self.request.user + form.instance.last_updated_by = self.request.user + return super().form_valid(form) class ChantDeleteView(LoginRequiredMixin, UserPassesTestMixin, DeleteView): @@ -1201,10 +1180,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 +1226,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