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

Allow edits to chants with no existing full text; fix browse source dropdown #1716

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
73 changes: 61 additions & 12 deletions django/cantusdb_project/main_app/forms.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -372,6 +381,7 @@ class Meta:
"incipit_of_refrain",
"later_addition",
"rubrics",
"source",
]
widgets = {
# manuscript_full_text_std_spelling: defined below (required) & special field
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions django/cantusdb_project/main_app/templates/browse_chants.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ <h3>Browse Chants</h3>
<select id="sourceFilter" name="source" class="form-control custom-select custom-select-sm">
{% comment %} <option value="">- Any -</option> {% endcomment %}
{% for source_obj in sources %}
<option value="{{ source_obj.id }}"{% if source_obj.id == source.id %} selected {% endif %}>{{ source_obj.siglum }}</option>
<option value="{{ source_obj.id }}"{% if source_obj.id == source.id %} selected {% endif %}>{{ source_obj.short_heading }}</option>
{% endfor %}
</select>
</div>
Expand Down Expand Up @@ -147,7 +147,9 @@ <h3>Browse Chants</h3>
{% endfor %}
</tbody>
</table>
{% include "pagination.html" %}
<div class="small">
{% include "pagination.html" %}
</div>
{% else %}
No chants found.
{% endif %}
Expand Down
20 changes: 13 additions & 7 deletions django/cantusdb_project/main_app/tests/make_fakes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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",
Expand Down
87 changes: 74 additions & 13 deletions django/cantusdb_project/main_app/tests/test_views/test_chant.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
Loading
Loading