From e306886dbc17e937b70e136051c42c884dcb1d3d Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 21 Nov 2024 10:30:34 -0500 Subject: [PATCH 1/4] fix(browse chants): use short_heading in source filter dropdown Refs: #1715 --- django/cantusdb_project/main_app/templates/browse_chants.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/cantusdb_project/main_app/templates/browse_chants.html b/django/cantusdb_project/main_app/templates/browse_chants.html index 8ac640490..9116c5e31 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

From fb36a36737d9db27fb4a8add81e704b64396095c Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 21 Nov 2024 10:31:33 -0500 Subject: [PATCH 2/4] fix(browse chants): conform browse chants template styling to #1712 Follow #1712 and conform pagination styling of the browse chants templates to other filterable table templates. --- django/cantusdb_project/main_app/templates/browse_chants.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/django/cantusdb_project/main_app/templates/browse_chants.html b/django/cantusdb_project/main_app/templates/browse_chants.html index 9116c5e31..6de54bbab 100644 --- a/django/cantusdb_project/main_app/templates/browse_chants.html +++ b/django/cantusdb_project/main_app/templates/browse_chants.html @@ -147,7 +147,9 @@

Browse Chants

{% endfor %} - {% include "pagination.html" %} +
+ {% include "pagination.html" %} +
{% else %} No chants found. {% endif %} From 00c8088d2f41ad1d505e27f4ed3770d72ff542ba Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 21 Nov 2024 14:11:29 -0500 Subject: [PATCH 3/4] 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 From d69a1550ad61fdc048f5664b783cf797eeb57288 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 21 Nov 2024 14:13:31 -0500 Subject: [PATCH 4/4] fix(chant create): move folio-sequence uniqueness check to Form obj Moves validity checks on the Chant Create view to the Chant Create Form. Refs: #1713. --- django/cantusdb_project/main_app/forms.py | 29 +++++--- .../cantusdb_project/main_app/views/chant.py | 69 +++++++------------ 2 files changed, 42 insertions(+), 56 deletions(-) diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 1279c5247..253ab3d02 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -170,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 @@ -239,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): diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index 986c91452..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 - - 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.", - ) + kwargs = super().get_form_kwargs() + if "data" in kwargs: + kwargs["data"] = kwargs["data"].copy() + kwargs["data"]["source"] = self.source.id + return kwargs - 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):