From 329d8f4cd5312296140646c294e243e8ec37bc9a Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Fri, 13 Sep 2024 16:25:07 +0200 Subject: [PATCH 01/24] Fixed: Add name field to source models Allows for the description of colloquial names for sources --- .../cantusdb_project/main_app/admin/source.py | 1 + django/cantusdb_project/main_app/forms.py | 27 ++++++++++-------- ...0032_source_name_alter_source_shelfmark.py | 28 +++++++++++++++++++ .../main_app/models/source.py | 6 ++++ 4 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 django/cantusdb_project/main_app/migrations/0032_source_name_alter_source_shelfmark.py diff --git a/django/cantusdb_project/main_app/admin/source.py b/django/cantusdb_project/main_app/admin/source.py index 0bc8d6c4f..952cd54f9 100644 --- a/django/cantusdb_project/main_app/admin/source.py +++ b/django/cantusdb_project/main_app/admin/source.py @@ -28,6 +28,7 @@ class SourceAdmin(BaseModelAdmin): "holding_institution__migrated_identifier", "id", "provenance_notes", + "name", ) readonly_fields = ( ("title", "siglum") diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 81dce3c75..4f65fa883 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -720,22 +720,27 @@ class Meta: model = Source fields = "__all__" - title = forms.CharField( - required=True, - widget=TextInputWidget, - help_text="Full Source Identification (City, Archive, Shelf-mark)", - ) - title.widget.attrs.update({"style": "width: 610px;"}) + # title = forms.CharField( + # required=True, + # widget=TextInputWidget, + # help_text="Full Source Identification (City, Archive, Shelf-mark)", + # ) + # title.widget.attrs.update({"style": "width: 610px;"}) + # + # siglum = forms.CharField( + # required=True, + # widget=TextInputWidget, + # help_text="RISM-style siglum + Shelf-mark (e.g. GB-Ob 202).", + # ) - siglum = forms.CharField( + shelfmark = forms.CharField( required=True, widget=TextInputWidget, - help_text="RISM-style siglum + Shelf-mark (e.g. GB-Ob 202).", ) - shelfmark = forms.CharField( - required=True, - widget=TextInputWidget, + name = forms.CharField( + required=False, + widget=TextInputWidget ) holding_institution = forms.ModelChoiceField( diff --git a/django/cantusdb_project/main_app/migrations/0032_source_name_alter_source_shelfmark.py b/django/cantusdb_project/main_app/migrations/0032_source_name_alter_source_shelfmark.py new file mode 100644 index 000000000..12a3e2327 --- /dev/null +++ b/django/cantusdb_project/main_app/migrations/0032_source_name_alter_source_shelfmark.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.14 on 2024-09-13 14:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("main_app", "0031_alter_source_holding_institution_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="source", + name="name", + field=models.CharField( + blank=True, + help_text="A colloquial or commonly-used name for the source", + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="source", + name="shelfmark", + field=models.CharField(max_length=255), + ), + ] diff --git a/django/cantusdb_project/main_app/models/source.py b/django/cantusdb_project/main_app/models/source.py index 919fbac20..297c3f087 100644 --- a/django/cantusdb_project/main_app/models/source.py +++ b/django/cantusdb_project/main_app/models/source.py @@ -50,6 +50,12 @@ class Source(BaseModel): blank=False, null=False, ) + name = models.CharField( + max_length=255, + blank=True, + null=True, + help_text="A colloquial or commonly-used name for the source" + ) provenance = models.ForeignKey( "Provenance", on_delete=models.PROTECT, From 61a2d31c868a286a7cc00c57659188ac7d73260d Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Fri, 13 Sep 2024 16:41:07 +0200 Subject: [PATCH 02/24] New: Add Source Identifier field Allows for the capture of other identifiers for a source. --- .../cantusdb_project/main_app/admin/source.py | 11 +++- .../migrations/0033_sourceidentifier.py | 53 +++++++++++++++++++ .../main_app/models/__init__.py | 1 + .../main_app/models/source_identifier.py | 33 ++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 django/cantusdb_project/main_app/migrations/0033_sourceidentifier.py create mode 100644 django/cantusdb_project/main_app/models/source_identifier.py diff --git a/django/cantusdb_project/main_app/admin/source.py b/django/cantusdb_project/main_app/admin/source.py index 952cd54f9..ebcfba82c 100644 --- a/django/cantusdb_project/main_app/admin/source.py +++ b/django/cantusdb_project/main_app/admin/source.py @@ -3,7 +3,7 @@ from main_app.admin.base_admin import BaseModelAdmin, EXCLUDE, READ_ONLY from main_app.admin.filters import InputFilter from main_app.forms import AdminSourceForm -from main_app.models import Source +from main_app.models import Source, SourceIdentifier class SourceKeyFilter(InputFilter): @@ -15,10 +15,19 @@ def queryset(self, request, queryset): return queryset.filter(holding_institution__siglum__icontains=self.value()) +class IdentifiersInline(admin.TabularInline): + model = SourceIdentifier + extra = 0 + + def get_queryset(self, request): + return super().get_queryset(request).select_related("source__holding_institution") + + @admin.register(Source) class SourceAdmin(BaseModelAdmin): exclude = EXCLUDE + ("source_status",) raw_id_fields = ("holding_institution",) + inlines = (IdentifiersInline,) # These search fields are also available on the user-source inline relationship in the user admin page search_fields = ( diff --git a/django/cantusdb_project/main_app/migrations/0033_sourceidentifier.py b/django/cantusdb_project/main_app/migrations/0033_sourceidentifier.py new file mode 100644 index 000000000..d17e23cb0 --- /dev/null +++ b/django/cantusdb_project/main_app/migrations/0033_sourceidentifier.py @@ -0,0 +1,53 @@ +# Generated by Django 4.2.14 on 2024-09-13 14:34 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("main_app", "0032_source_name_alter_source_shelfmark"), + ] + + operations = [ + migrations.CreateModel( + name="SourceIdentifier", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("identifier", models.CharField(max_length=255)), + ( + "type", + models.IntegerField( + choices=[ + (1, "Other catalogues"), + (2, "olim (Former shelfmark)"), + (3, "Alternative names"), + (4, "RISM Online"), + ] + ), + ), + ("note", models.TextField(blank=True, null=True)), + ( + "source", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="identifiers", + to="main_app.source", + ), + ), + ], + options={ + "verbose_name": "Source Identifier", + "ordering": ("type",), + }, + ), + ] diff --git a/django/cantusdb_project/main_app/models/__init__.py b/django/cantusdb_project/main_app/models/__init__.py index ea1149eca..5ed2a4227 100644 --- a/django/cantusdb_project/main_app/models/__init__.py +++ b/django/cantusdb_project/main_app/models/__init__.py @@ -10,6 +10,7 @@ from main_app.models.segment import Segment from main_app.models.sequence import Sequence from main_app.models.source import Source +from main_app.models.source_identifier import SourceIdentifier from main_app.models.institution import Institution from main_app.models.institution_identifier import InstitutionIdentifier from main_app.models.project import Project diff --git a/django/cantusdb_project/main_app/models/source_identifier.py b/django/cantusdb_project/main_app/models/source_identifier.py new file mode 100644 index 000000000..85ccb5f71 --- /dev/null +++ b/django/cantusdb_project/main_app/models/source_identifier.py @@ -0,0 +1,33 @@ +from django.db import models + +class SourceIdentifier(models.Model): + class Meta: + verbose_name = "Source Identifier" + ordering = ('type',) + + OTHER = 1 + OLIM = 2 + ALTN = 3 + RISM_ONLINE = 4 + + IDENTIFIER_TYPES = ( + (OTHER, 'Other catalogues'), + (OLIM, 'olim (Former shelfmark)'), + (ALTN, 'Alternative names'), + (RISM_ONLINE, "RISM Online") + ) + + identifier = models.CharField(max_length=255) + type = models.IntegerField(choices=IDENTIFIER_TYPES) + note = models.TextField(blank=True, null=True) + source = models.ForeignKey("Source", + related_name="identifiers", + on_delete=models.CASCADE) + + def __str__(self): + return f"{self.identifier}" + + @property + def identifier_type(self): + d = dict(self.IDENTIFIER_TYPES) + return d[self.type] From fe72a2f164b2675dc9e69d68936d8920f2cd61f7 Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Fri, 13 Sep 2024 16:41:27 +0200 Subject: [PATCH 03/24] Fixed: Change "Private" to "Cantus" --- django/cantusdb_project/main_app/models/source.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/cantusdb_project/main_app/models/source.py b/django/cantusdb_project/main_app/models/source.py index 297c3f087..a8f2cd19a 100644 --- a/django/cantusdb_project/main_app/models/source.py +++ b/django/cantusdb_project/main_app/models/source.py @@ -177,7 +177,7 @@ def short_heading(self) -> str: if holdinst.siglum and holdinst.siglum != "XX-NN": title.append(f"{holdinst.siglum}") elif holdinst.is_private_collector: - title.append("Private") + title.append("Cantus") tt = self.shelfmark if self.shelfmark else self.title title.append(tt) From d0f7bf5b232e4d56621107dbf19885785cb670f1 Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Fri, 13 Sep 2024 16:43:42 +0200 Subject: [PATCH 04/24] Fixed: Ensure the source identifiers are searchable in the admin UI --- django/cantusdb_project/main_app/admin/source.py | 1 + 1 file changed, 1 insertion(+) diff --git a/django/cantusdb_project/main_app/admin/source.py b/django/cantusdb_project/main_app/admin/source.py index ebcfba82c..79818d5d2 100644 --- a/django/cantusdb_project/main_app/admin/source.py +++ b/django/cantusdb_project/main_app/admin/source.py @@ -38,6 +38,7 @@ class SourceAdmin(BaseModelAdmin): "id", "provenance_notes", "name", + "identifiers__identifier" ) readonly_fields = ( ("title", "siglum") From b407110118d5a7ac73c409454c182477820717bc Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Fri, 13 Sep 2024 17:18:41 +0200 Subject: [PATCH 05/24] Fixed: Adjust display fields --- django/cantusdb_project/main_app/models/source.py | 4 ++++ django/cantusdb_project/main_app/templates/source_detail.html | 2 +- django/cantusdb_project/main_app/templates/source_list.html | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/django/cantusdb_project/main_app/models/source.py b/django/cantusdb_project/main_app/models/source.py index a8f2cd19a..98caddc06 100644 --- a/django/cantusdb_project/main_app/models/source.py +++ b/django/cantusdb_project/main_app/models/source.py @@ -181,4 +181,8 @@ def short_heading(self) -> str: tt = self.shelfmark if self.shelfmark else self.title title.append(tt) + + if not self.full_source: + title.append("(fragment)") + return " ".join(title) diff --git a/django/cantusdb_project/main_app/templates/source_detail.html b/django/cantusdb_project/main_app/templates/source_detail.html index 357f5dc46..a7eab6478 100644 --- a/django/cantusdb_project/main_app/templates/source_detail.html +++ b/django/cantusdb_project/main_app/templates/source_detail.html @@ -35,7 +35,7 @@

{{ source.heading }}

{% if source.holding_institution %} -
Siglum
+
Cantus Siglum
{{ source.short_heading }}
Holding Institution
diff --git a/django/cantusdb_project/main_app/templates/source_list.html b/django/cantusdb_project/main_app/templates/source_list.html index a65800af0..66bb2e78f 100644 --- a/django/cantusdb_project/main_app/templates/source_list.html +++ b/django/cantusdb_project/main_app/templates/source_list.html @@ -88,8 +88,8 @@

Browse Sources

{% sortable_header request "country" %} - {% sortable_header request "city_institution" "City + Institution" %} - {% sortable_header request "source" "Source" %} + {% sortable_header request "city_institution" "City + Holding Institution" %} + {% sortable_header request "source" "Cantus Siglum" %} Summary Date/Origin Image Link From bf5c984342f6b98e206862f8ced801229fb1f7e4 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Tue, 1 Oct 2024 13:46:17 -0400 Subject: [PATCH 06/24] fix: maintain compatibility in external/export APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The addition of the institution model and the change from office to service introduced changes in external APIs (the JSON CID export, 
the JSON melody export, and the cached concordances command). This commit rolls back those changes to maintain compatibility. 
The modifications may be reintroduced at a later date if notice is given 
to external users (in particular, Cantus Index). Also simplifies the logic and adjusts the typing of functions related to the JSON melody export and the AJAX melody list views to address #1638. Makes relevant changes to associated tests. Fixes additional type errors in views/api.py. Refs #1641. --- .../commands/update_cached_concordances.py | 5 +- .../main_app/tests/test_functions.py | 4 +- .../main_app/tests/test_views/test_api.py | 11 +- django/cantusdb_project/main_app/views/api.py | 209 +++++++----------- 4 files changed, 89 insertions(+), 140 deletions(-) diff --git a/django/cantusdb_project/main_app/management/commands/update_cached_concordances.py b/django/cantusdb_project/main_app/management/commands/update_cached_concordances.py index 7c0fb2ebc..820e02d2a 100644 --- a/django/cantusdb_project/main_app/management/commands/update_cached_concordances.py +++ b/django/cantusdb_project/main_app/management/commands/update_cached_concordances.py @@ -112,7 +112,10 @@ def make_chant_dict(chant: dict) -> dict: "incipit": chant["incipit"], "feast": chant["feast__name"], "genre": chant["genre__name"], - "service": chant["service__name"], + "office": chant[ + "service__name" + ], # We keep the office key for backwards compatibility + # with external applications (e.g. Cantus Index) using this export "position": chant["position"], "cantus_id": chant["cantus_id"], "image": chant["image_link"], diff --git a/django/cantusdb_project/main_app/tests/test_functions.py b/django/cantusdb_project/main_app/tests/test_functions.py index a250d6cd6..0f89fb8d2 100644 --- a/django/cantusdb_project/main_app/tests/test_functions.py +++ b/django/cantusdb_project/main_app/tests/test_functions.py @@ -208,7 +208,7 @@ def test_concordances_structure(self): "incipit", "feast", "genre", - "service", + "office", "position", "cantus_id", "image", @@ -259,7 +259,7 @@ def test_concordances_values(self): ("incipit", chant.incipit), ("feast", chant.feast.name), ("genre", chant.genre.name), - ("service", chant.service.name), + ("office", chant.service.name), ("position", chant.position), ("cantus_id", chant.cantus_id), ("image", chant.image_link), diff --git a/django/cantusdb_project/main_app/tests/test_views/test_api.py b/django/cantusdb_project/main_app/tests/test_views/test_api.py index 8991f3f44..05985e8f8 100644 --- a/django/cantusdb_project/main_app/tests/test_views/test_api.py +++ b/django/cantusdb_project/main_app/tests/test_views/test_api.py @@ -270,8 +270,7 @@ def test_json_melody_fields(self): "mid", "nid", "cid", - "holding_institution", - "shelfmark", + "siglum", "srcnid", "folio", "incipit", @@ -279,7 +278,7 @@ def test_json_melody_fields(self): "volpiano", "mode", "feast", - "service", + "office", "genre", "position", "chantlink", @@ -765,7 +764,7 @@ def test_structure(self): "incipit": "some string" "feast": "some string" "genre": "some string" - "service": "some string" + "office": "some string" "position": "some string" "cantus_id": "some string" "image": "some string" @@ -811,7 +810,7 @@ def test_structure(self): "incipit", "feast", "genre", - "service", + "office", "position", "cantus_id", "image", @@ -834,7 +833,7 @@ def test_values(self): "incipit": chant.incipit, "feast": chant.feast.name, "genre": chant.genre.name, - "service": chant.service.name, + "office": chant.service.name, "position": chant.position, "mode": chant.mode, "image": chant.image_link, diff --git a/django/cantusdb_project/main_app/views/api.py b/django/cantusdb_project/main_app/views/api.py index 48f4da16e..c3599cfeb 100644 --- a/django/cantusdb_project/main_app/views/api.py +++ b/django/cantusdb_project/main_app/views/api.py @@ -1,14 +1,13 @@ import csv -from typing import List -from typing import Optional +from typing import Optional, Union, Any from django.contrib.auth import get_user_model from django.contrib.flatpages.models import FlatPage from django.core.exceptions import PermissionDenied from django.db.models.query import QuerySet -from django.db.models import Model from django.http.response import JsonResponse -from django.http import HttpResponse, HttpResponseNotFound +from django.http import HttpResponse, HttpResponseNotFound, Http404, HttpRequest from django.urls.base import reverse +from django.shortcuts import get_object_or_404 from articles.models import Article from main_app.models import ( Chant, @@ -19,16 +18,9 @@ Source, ) from next_chants import next_chants -from django.http import Http404 -from django.core.exceptions import PermissionDenied -from django.urls import reverse -from django.contrib.auth import get_user_model -from typing import List -from django.contrib.flatpages.models import FlatPage -from django.shortcuts import get_object_or_404 -def ajax_melody_list(request, cantus_id) -> JsonResponse: +def ajax_melody_list(request: HttpRequest, cantus_id: str) -> JsonResponse: """ Function-based view responding to the AJAX call for melody list on the chant detail page, accessed with ``chants/``, click on "Display melodies connected with this chant" @@ -41,7 +33,9 @@ def ajax_melody_list(request, cantus_id) -> JsonResponse: """ chants: QuerySet[Chant] = ( Chant.objects.filter(cantus_id=cantus_id) - .select_related("source__holding_institution", "feast", "genre", "service") + .select_related( + "source", "source__holding_institution", "feast", "genre", "service" + ) .exclude(volpiano=None) .order_by("id") ) @@ -50,38 +44,29 @@ def ajax_melody_list(request, cantus_id) -> JsonResponse: if not display_unpublished: chants = chants.filter(source__published=True) - concordance_values: QuerySet[dict] = chants.values( - "source__holding_institution__siglum", - "source__shelfmark", - "folio", - "service__name", - "genre__name", - "position", - "feast__name", - "cantus_id", - "volpiano", - "mode", - # OldCantus seems to use whichever is present: ms spelling, std spelling, incipit (in that order) - "manuscript_full_text_std_spelling", - ) - - concordances: list[dict] = list(concordance_values) - for i, concordance in enumerate(concordances): - # we need to use each chant's _source_'s siglum, and not the - # legacy sigla that were attached to chants in OldCantus - holding_inst_sig = concordance.pop("source__holding_institution__siglum") - source_shelfmark = concordance.pop("source__shelfmark") - - concordance["siglum"] = f"{holding_inst_sig} {source_shelfmark}" - # for chants that do not have a source, do not attempt - # to return a source link - if chants[i].source: - concordance["source_link"] = chants[i].source.get_absolute_url() - concordance["ci_link"] = chants[i].get_ci_url() - concordance["chant_link"] = chants[i].get_absolute_url() - concordance["db"] = "CD" - - concordance_count: int = len(concordances) + concordances: list[dict[str, str]] = [] + for chant in chants: + concordance: dict[str, str] = { + "siglum": chant.source.short_heading, + "folio": chant.folio or "", + "service__name": chant.service.name if chant.service else "", + "genre__name": chant.genre.name if chant.genre else "", + "position": chant.position or "", + "feast__name": chant.feast.name if chant.feast else "", + "cantus_id": chant.cantus_id or "", + # Query above filters out chants with volpiano=None + "volpiano": chant.volpiano, # type: ignore[dict-item] + "mode": chant.mode or "", + "manuscript_full_text_std_spelling": chant.manuscript_full_text_std_spelling + or "", + "ci_link": chant.get_ci_url(), + "chant_link": chant.get_absolute_url(), + "source_link": chant.source.get_absolute_url(), + "db": "CD", + } + concordances.append(concordance) + + concordance_count = len(concordances) return JsonResponse( {"concordances": concordances, "concordance_count": concordance_count}, safe=True, @@ -154,7 +139,6 @@ def csv_export(request, source_id): feast = entry.feast.name if entry.feast else "" service = entry.service.name if entry.service else "" genre = entry.genre.name if entry.genre else "" - diff_db = entry.diff_db.id if entry.diff_db else "" writer.writerow( [ @@ -357,88 +341,47 @@ def ajax_search_bar(request, search_term): return JsonResponse({"chants": returned_values}, safe=True) -def json_melody_export(request, cantus_id: str) -> JsonResponse: - chants = Chant.objects.filter( - cantus_id=cantus_id, volpiano__isnull=False, source__published=True - ) - - db_keys = [ - "melody_id", - "id", - "cantus_id", - "source__holding_institution", - "source__shelfmark", - "source__id", # don't fetch the entire Source object, just the id of - # the source. __id is removed in standardize_for_api below - "folio", - "incipit", - "manuscript_full_text", - "volpiano", - "mode", - "feast__id", - "service__id", - "genre__id", - "position", - ] - - chants_values = list(chants.values(*db_keys)) # a list of dictionaries. Each - # dictionary represents metadata on one chant - - standardized_chants_values = [ - standardize_dict_for_json_melody_export(cv, request) for cv in chants_values - ] - - return JsonResponse(standardized_chants_values, safe=False) - - -def standardize_dict_for_json_melody_export( - chant_values: List[dict], request -) -> List[dict]: - """Take a list of dictionaries, and in each dictionary, change several - of the keys to match their values in OldCantus - - Args: - chant_values (List[dict]): a list of dictionaries, each containing - information on a single chant in the database - request: passed when this is called in json_melody_export. Used to get the domain - while building the chant links - - Returns: - List[dict]: a list of dictionaries, with updated keys +def json_melody_export(request: HttpRequest, cantus_id: str) -> JsonResponse: """ - keymap = { # map attribute names from Chant model (i.e. db_keys - # in json_melody_export) to corresponding attribute names - # in old API, and remove artifacts of query process (i.e. __id suffixes) - "melody_id": "mid", # <- - "id": "nid", # <- - "cantus_id": "cid", # <- - "source__shelfmark": "shelfmark", - "source__holding_institution": "holding_institution", - "source__id": "srcnid", # <- - "folio": "folio", - "incipit": "incipit", - "manuscript_full_text": "fulltext", # <- - "volpiano": "volpiano", - "mode": "mode", - "feast__id": "feast", # <- - "service__id": "service", # <- - "genre__id": "genre", # <- - "position": "position", - } - - standardized_chant_values = {keymap[key]: chant_values[key] for key in chant_values} + Similar to the ajax_melody_list view, but designed for external use (for instance, + it returns absolute URLs for the chant and source detail pages), only returns + chants in published sources, and contains slightly different chant text fields. + """ + chants: QuerySet[Chant] = Chant.objects.filter( + cantus_id=cantus_id, volpiano__isnull=False, source__published=True + ).select_related("source") + + chants_export: list[dict[str, Optional[Union[str, int]]]] = [] + for chant in chants: + chant_values = { + "mid": chant.melody_id, + "nid": chant.id, + "cid": chant.cantus_id, + "siglum": chant.source.short_heading, + "srcnid": chant.source.id, + "folio": chant.folio, + "incipit": chant.incipit, + "fulltext": chant.manuscript_full_text_std_spelling, + "volpiano": chant.volpiano, + "mode": chant.mode, + "feast": chant.feast_id, + "office": chant.service_id, # We keep the office key for backwards compatibility + # with external applications + "genre": chant.genre_id, + "position": chant.position, + } + chant_uri = request.build_absolute_uri( + reverse("chant-detail", args=[chant_values["nid"]]) + ) + chant_values["chantlink"] = chant_uri + src_uri = request.build_absolute_uri( + reverse("source-detail", args=[chant_values["srcnid"]]) + ) + chant_values["srclink"] = src_uri - # manually build a couple of last fields that aren't represented in Chant object - chant_uri = request.build_absolute_uri( - reverse("chant-detail", args=[chant_values["id"]]) - ) - standardized_chant_values["chantlink"] = chant_uri - src_uri = request.build_absolute_uri( - reverse("source-detail", args=[chant_values["source__id"]]) - ) - standardized_chant_values["srclink"] = src_uri + chants_export.append(chant_values) - return standardized_chant_values + return JsonResponse(chants_export, safe=False) def json_sources_export(request) -> JsonResponse: @@ -532,7 +475,9 @@ def build_json_cid_dictionary(chant, request) -> dict: "incipit": chant.incipit if chant.incipit else "", "feast": chant.feast.name if chant.feast else "", "genre": chant.genre.name if chant.genre else "", - "service": chant.service.name if chant.service else "", + "office": ( + chant.service.name if chant.service else "" + ), # We keep the office key for backwards compatibility with external applications "position": chant.position if chant.position else "", "cantus_id": chant.cantus_id if chant.cantus_id else "", "image": chant.image_link if chant.image_link else "", @@ -548,7 +493,7 @@ def build_json_cid_dictionary(chant, request) -> dict: return dictionary -def record_exists(rec_type: type[Model], pk: int) -> bool: +def record_exists(rec_type: Union[Chant, Source, Sequence, Article], pk: int) -> bool: """Determines whether record of specific type (chant, source, sequence, article) exists for a given pk Args: @@ -586,7 +531,9 @@ def get_user_id_from_old_indexer_id(pk: int) -> Optional[int]: return None -NODE_TYPES_AND_VIEWS = [ +NODE_TYPES_AND_VIEWS: list[ + tuple[Union[type[Chant], type[Source], type[Sequence], type[Article]], str] +] = [ (Chant, "chant-detail"), (Source, "source-detail"), (Sequence, "sequence-detail"), @@ -600,7 +547,7 @@ def get_user_id_from_old_indexer_id(pk: int) -> Optional[int]: NODE_ID_CUTOFF = 1_000_000 -def json_node_export(request, id: int) -> HttpResponse: +def json_node_export(request: HttpRequest, id: int) -> HttpResponse: """ returns all fields of the chant/sequence/source/indexer with the specified `id` """ @@ -612,12 +559,12 @@ def json_node_export(request, id: int) -> HttpResponse: raise Http404() user_id = get_user_id_from_old_indexer_id(id) - if get_user_id_from_old_indexer_id(id) is not None: + if user_id is not None: User = get_user_model() user = User.objects.filter(id=user_id) # in order to easily unpack the object's properties in `vals` below, `user` needs to be # a queryset rather than an individual object. - vals = dict(*user.values()) + vals: dict[str, Any] = dict(*user.values()) return JsonResponse(vals) # This seems to return the first object for which the node id matches. From 3265dbdf0601deabec3cc597becc8700d0d01b9f Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 16:14:12 -0400 Subject: [PATCH 07/24] fix(ms073): temporarily turn off volpiano display With this commit, volpiano can be added to chants in this source but no volpiano will display in any user-facing way. --- .../main_app/templates/browse_chants.html | 3 ++- .../main_app/templates/chant_detail.html | 8 +++--- .../main_app/templates/chant_search.html | 6 ++++- .../main_app/templates/source_list.html | 3 ++- django/cantusdb_project/main_app/views/api.py | 26 +++++++++++-------- .../cantusdb_project/main_app/views/chant.py | 9 +++++++ 6 files changed, 38 insertions(+), 17 deletions(-) diff --git a/django/cantusdb_project/main_app/templates/browse_chants.html b/django/cantusdb_project/main_app/templates/browse_chants.html index 8b9245520..771aceac0 100644 --- a/django/cantusdb_project/main_app/templates/browse_chants.html +++ b/django/cantusdb_project/main_app/templates/browse_chants.html @@ -88,7 +88,8 @@

Browse Chants

{{ chant.incipit|default:"" }}

{{ chant.manuscript_full_text_std_spelling|default:"" }}
- {% if chant.volpiano %} + + {% if chant.volpiano and chant.source.id != 680970 %} {{ chant.volpiano|default:"" }} {% endif %}

diff --git a/django/cantusdb_project/main_app/templates/chant_detail.html b/django/cantusdb_project/main_app/templates/chant_detail.html index 022f48101..a46a18156 100644 --- a/django/cantusdb_project/main_app/templates/chant_detail.html +++ b/django/cantusdb_project/main_app/templates/chant_detail.html @@ -240,8 +240,9 @@

{{ chant.incipit }}

Full text as in Source (source spelling)
{{ chant.manuscript_full_text }}
{% endif %} - - {% if chant.volpiano %} + + + {% if chant.volpiano and chant.source.id != 680970 %}
Volpiano

{{ chant.volpiano }}

@@ -253,7 +254,8 @@

{{ chant.incipit }}

{{ chant.indexing_notes }}
{% endif %} - {% if chant.volpiano %} + + {% if chant.volpiano and chant.source.id != 680970 %}
Melody with text
diff --git a/django/cantusdb_project/main_app/templates/chant_search.html b/django/cantusdb_project/main_app/templates/chant_search.html index 601bd380a..5ade5b0e1 100644 --- a/django/cantusdb_project/main_app/templates/chant_search.html +++ b/django/cantusdb_project/main_app/templates/chant_search.html @@ -240,6 +240,8 @@

Search Chants

{% for chant in chants %} + + {% if chant.source.id != 680970 or melodies != "true" %} {% if not source %} @@ -297,7 +299,8 @@

Search Chants

{% endif %} - {% if chant.volpiano %} + + {% if chant.volpiano and chant.source.id != 680970 %} {% endif %} @@ -307,6 +310,7 @@

Search Chants

{% endif %} + {% endif %} {% endfor %} diff --git a/django/cantusdb_project/main_app/templates/source_list.html b/django/cantusdb_project/main_app/templates/source_list.html index 66bb2e78f..c3cff7bfd 100644 --- a/django/cantusdb_project/main_app/templates/source_list.html +++ b/django/cantusdb_project/main_app/templates/source_list.html @@ -130,7 +130,8 @@

Browse Sources

{{ source.number_of_chants|default_if_none:"0" }} - {% if source.number_of_melodies %} + + {% if source.number_of_melodies and source.id != 680970 %}
/ {{ source.number_of_melodies }} {% endif %} diff --git a/django/cantusdb_project/main_app/views/api.py b/django/cantusdb_project/main_app/views/api.py index 48f4da16e..831c533d4 100644 --- a/django/cantusdb_project/main_app/views/api.py +++ b/django/cantusdb_project/main_app/views/api.py @@ -281,17 +281,21 @@ def ajax_melody_search(request): chants = chants.filter(feast__name__icontains=feast_name) if mode: chants = chants.filter(mode__icontains=mode) - - result_values = chants.order_by("id").values( - "id", - "source__holding_institution__siglum", - "source__shelfmark", - "folio", - "incipit", - "genre__name", - "feast__name", - "mode", - "volpiano", + # See #1635 re the following source exclusion. Temporarily disable volpiano display for this source. + result_values = ( + chants.exclude(source__id=680970) + .order_by("id") + .values( + "id", + "source__holding_institution__siglum", + "source__shelfmark", + "folio", + "incipit", + "genre__name", + "feast__name", + "mode", + "volpiano", + ) ) # convert queryset to a list of dicts because QuerySet is not JSON serializable # the above constructed queryset will be evaluated here diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index 4bb38dc16..002d447c9 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -367,6 +367,9 @@ def get_context_data(self, **kwargs) -> dict: if search_position: search_parameters.append(f"position={search_position}") search_melodies: Optional[str] = self.request.GET.get("melodies") + # This was added to context so that we could implement #1635 and can be + # removed once that is undone. + context["melodies"] = search_melodies if search_melodies: search_parameters.append(f"melodies={search_melodies}") search_bar: Optional[str] = self.request.GET.get("search_bar") @@ -653,6 +656,12 @@ def get_queryset(self) -> QuerySet: # If the "apply" button hasn't been clicked, return empty queryset if not self.request.GET: return Chant.objects.none() + # See #1635 re the following source exclusion. Temporarily disable volpiano display for this source. + if ( + self.request.GET.get("melodies") == "true" + and self.kwargs["source_pk"] == 680970 + ): + return Chant.objects.none() # Create a Q object to filter the QuerySet of Chants q_obj_filter = Q() From 19a0828d70fb1af0721d368c9cab4a266b9be1b6 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 20:00:28 -0400 Subject: [PATCH 08/24] feat(source ids): add command to fix dact_id and fragmentarium_id formats --- .../commands/reformat_source_ids.py | 49 ++++++++++++++++ .../tests/test_reformat_source_ids.py | 56 +++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 django/cantusdb_project/main_app/management/commands/reformat_source_ids.py create mode 100644 django/cantusdb_project/main_app/tests/test_reformat_source_ids.py diff --git a/django/cantusdb_project/main_app/management/commands/reformat_source_ids.py b/django/cantusdb_project/main_app/management/commands/reformat_source_ids.py new file mode 100644 index 000000000..b11f3941a --- /dev/null +++ b/django/cantusdb_project/main_app/management/commands/reformat_source_ids.py @@ -0,0 +1,49 @@ +""" +A command designed to do a one-time reformatting of DACT IDs and Fragment +IDs in the database. + +Fragment IDs should be of the form "F-XXXX" where XXXX is some alphanumeric. +Fragment IDs are currently assumed to be in the form "F-XXXX" or "XXXX". +DACT IDs should be of the form "D:0XXXX" where XXXX is the Fragment ID alphanumeric. +DACT IDs are currently assumed to be in the form "0XXXX" or "D-0XXXX". + +This command simply adds the prefix "F-" to all Fragment IDs and "D:" to all +DACT IDs where they are missing. +""" + +from django.core.management.base import BaseCommand + +from main_app.models import Source + + +class Command(BaseCommand): + help = "Reformat DACT IDs and Fragment IDs in the database." + + def handle(self, *args, **options): + sources = Source.objects.all() + for source in sources: + if source.dact_id: + if len(source.dact_id) == 5 and source.dact_id.startswith("0"): + source.dact_id = f"D:{source.dact_id}" + elif len(source.dact_id) == 7 and source.dact_id.startswith("D-0"): + source.dact_id = f"D:{source.dact_id[2:]}" + else: + self.stdout.write( + self.style.WARNING( + f"{source.id} | DACT ID {source.dact_id} is not in the correct format." + ) + ) + if source.fragmentarium_id: + if len(source.fragmentarium_id) == 4: + source.fragmentarium_id = f"F-{source.fragmentarium_id}" + elif len( + source.fragmentarium_id + ) == 6 and source.fragmentarium_id.startswith("F-"): + pass + else: + self.stdout.write( + self.style.WARNING( + f"{source.id} | Fragment ID {source.fragmentarium_id} is not in the correct format." + ) + ) + source.save() diff --git a/django/cantusdb_project/main_app/tests/test_reformat_source_ids.py b/django/cantusdb_project/main_app/tests/test_reformat_source_ids.py new file mode 100644 index 000000000..7148d786e --- /dev/null +++ b/django/cantusdb_project/main_app/tests/test_reformat_source_ids.py @@ -0,0 +1,56 @@ +from django.test import TestCase +from django.core.management import call_command + +from main_app.models import Source +from main_app.tests.make_fakes import make_fake_institution, make_fake_segment + + +class TestReformatSourceIDs(TestCase): + def test_command(self): + segment = make_fake_segment() + fake_inst = make_fake_institution() + correct_source_1 = Source.objects.create( + segment=segment, + shelfmark="Correct Source 1", + holding_institution=fake_inst, + dact_id="0a1b3", + fragmentarium_id="a1b3", + ) + correct_source_2 = Source.objects.create( + segment=segment, + shelfmark="Correct Source 2", + holding_institution=fake_inst, + dact_id="D-0a1b3", + fragmentarium_id="F-a1b3", + ) + source_with_no_ids = Source.objects.create( + segment=segment, + shelfmark="Source with no IDs", + holding_institution=fake_inst, + ) + source_with_incorrect_ids = Source.objects.create( + segment=segment, + shelfmark="Source with incorrect IDs", + holding_institution=fake_inst, + dact_id="a1b3", + fragmentarium_id="F-1b3", + ) + + call_command("reformat_source_ids") + self.assertEqual(Source.objects.get(pk=correct_source_1.pk).dact_id, "D:0a1b3") + self.assertEqual( + Source.objects.get(pk=correct_source_1.pk).fragmentarium_id, "F-a1b3" + ) + self.assertEqual(Source.objects.get(pk=correct_source_2.pk).dact_id, "D:0a1b3") + self.assertEqual( + Source.objects.get(pk=correct_source_2.pk).fragmentarium_id, "F-a1b3" + ) + self.assertIsNone(Source.objects.get(pk=source_with_no_ids.pk).dact_id) + self.assertIsNone(Source.objects.get(pk=source_with_no_ids.pk).fragmentarium_id) + self.assertEqual( + Source.objects.get(pk=source_with_incorrect_ids.pk).dact_id, "a1b3" + ) + self.assertEqual( + Source.objects.get(pk=source_with_incorrect_ids.pk).fragmentarium_id, + "F-1b3", + ) From d10152ea72fceb65454df0626fa9eb24088cba54 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 3 Oct 2024 12:54:02 -0400 Subject: [PATCH 09/24] refactor(tests): refactor test_views.test_chant.ChantCreateViewTest Fixes type annotations. Fixes deprecated call to assertFormError (see https://docs.djangoproject.com/en/4.2/topics/testing/tools/#django.test.SimpleTestCase.assertFormError). Mocks requests.get for entire class. --- .../main_app/tests/test_views/test_chant.py | 190 +++++++++--------- 1 file changed, 95 insertions(+), 95 deletions(-) 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 706cfaba0..4a2a569f6 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 @@ -4,6 +4,7 @@ from unittest.mock import patch import random +from typing import ClassVar from django.test import TestCase, Client from django.urls import reverse @@ -2699,37 +2700,62 @@ def test_image_link_column(self): self.assertIn(f'Image', html) +@patch("requests.get", mock_requests_get) class ChantCreateViewTest(TestCase): + source: ClassVar[Source] + @classmethod def setUpTestData(cls): - Group.objects.create(name="project manager") + # Create project manager and contributor users + prod_manager_group = Group.objects.create(name="project manager") + contributor_group = Group.objects.create(name="contributor") + user_model = get_user_model() + pm_usr = user_model.objects.create_user(email="pm@test.com", password="pass") + pm_usr.groups.set([prod_manager_group]) + pm_usr.save() + contributor_usr = user_model.objects.create_user( + email="contrib@test.com", password="pass" + ) + contributor_usr.groups.set([contributor_group]) + contributor_usr.save() + # Create a fake source that the contributor user can edit + cls.source = make_fake_source() + cls.source.current_editors.add(contributor_usr) + cls.source.save() def setUp(self): - self.user = get_user_model().objects.create(email="test@test.com") - self.user.set_password("pass") - self.user.save() - self.client = Client() - project_manager = Group.objects.get(name="project manager") - project_manager.user_set.add(self.user) - self.client.login(email="test@test.com", password="pass") - - def test_url_and_templates(self): - source = make_fake_source() + # Log in as a contributor before each test + self.client.login(email="contrib@test.com", password="pass") + + def test_permissions(self) -> None: + # The client starts logged in as a contributor + # with access to the source. Test that the client + # can access the ChantCreate view. + with self.subTest("Contributor can access ChantCreate view"): + response = self.client.get(reverse("chant-create", args=[self.source.id])) + self.assertEqual(response.status_code, 200) + with self.subTest("Project manager can access ChantCreate view"): + # Log in as a project manager + self.client.logout() + self.client.login(email="pm@test.com", password="pass") + response = self.client.get(reverse("chant-create", args=[self.source.id])) + self.assertEqual(response.status_code, 200) + with self.subTest("Unauthenticated user cannot access ChantCreate view"): + # Log out + self.client.logout() + response = self.client.get(reverse("chant-create", args=[self.source.id])) + self.assertEqual(response.status_code, 302) - with patch("requests.get", mock_requests_get): - response_1 = self.client.get(reverse("chant-create", args=[source.id])) - response_2 = self.client.get( - reverse("chant-create", args=[source.id + 100]) - ) + def test_url_and_templates(self) -> None: + source = self.source + response_1 = self.client.get(reverse("chant-create", args=[source.id])) self.assertEqual(response_1.status_code, 200) self.assertTemplateUsed(response_1, "chant_create.html") + self.assertTemplateUsed(response_1, "base.html") - self.assertEqual(response_2.status_code, 404) - self.assertTemplateUsed(response_2, "404.html") - - def test_create_chant(self): - source = make_fake_source() + def test_create_chant(self) -> None: + source = self.source response = self.client.post( reverse("chant-create", args=[source.id]), { @@ -2740,40 +2766,37 @@ def test_create_chant(self): ) self.assertEqual(response.status_code, 302) self.assertRedirects(response, reverse("chant-create", args=[source.id])) - chant = Chant.objects.first() + chant = Chant.objects.get(source=source) self.assertEqual(chant.manuscript_full_text_std_spelling, "initial") - def test_view_url_path(self): - source = make_fake_source() - with patch("requests.get", mock_requests_get): - response = self.client.get(f"/chant-create/{source.id}") + def test_view_url_path(self) -> None: + source = self.source + response = self.client.get(f"/chant-create/{source.id}") self.assertEqual(response.status_code, 200) - def test_context(self): - """some context variable passed to templates""" - source = make_fake_source() + def test_context(self) -> None: + """Test that correct source is in context""" + source = self.source url = reverse("chant-create", args=[source.id]) - with patch("requests.get", mock_requests_get): - response = self.client.get(url) - self.assertEqual(response.context["source"].title, source.title) + response = self.client.get(url) + self.assertEqual(response.context["source"].id, source.id) - def test_post_error(self): + def test_empty_fulltext(self) -> None: """post with correct source and empty full-text""" - source = make_fake_source() + source = self.source url = reverse("chant-create", args=[source.id]) response = self.client.post(url, data={"manuscript_full_text_std_spelling": ""}) self.assertFormError( - response, - "form", + response.context["form"], "manuscript_full_text_std_spelling", "This field is required.", ) - def test_nonexistent_source(self): + def test_nonexistent_source(self) -> None: """ users should not be able to access Chant Create page for a source that doesn't exist """ - nonexistent_source_id: str = faker.numerify( + nonexistent_source_id = faker.numerify( "#####" ) # there's not supposed to be 5-digits source id with patch("requests.get", mock_requests_get): @@ -2782,16 +2805,16 @@ def test_nonexistent_source(self): ) self.assertEqual(response.status_code, 404) - def test_repeated_seq(self): + def test_repeated_seq(self) -> None: """post with a folio and seq that already exists in the source""" - TEST_FOLIO = "001r" + test_folio = "001r" # create some chants in the test source - source = make_fake_source() + source = self.source for i in range(1, 5): Chant.objects.create( source=source, manuscript_full_text=faker.text(10), - folio=TEST_FOLIO, + folio=test_folio, c_sequence=i, ) # post a chant with the same folio and seq @@ -2801,36 +2824,19 @@ def test_repeated_seq(self): url, data={ "manuscript_full_text_std_spelling": fake_text, - "folio": TEST_FOLIO, + "folio": test_folio, "c_sequence": random.randint(1, 4), }, follow=True, ) self.assertFormError( - response, - "form", + response.context["form"], None, errors="Chant with the same sequence and folio already exists in this source.", ) - def test_view_url_reverse_name(self): - fake_sources = [make_fake_source() for _ in range(10)] - for source in fake_sources: - with patch("requests.get", mock_requests_get): - response = self.client.get(reverse("chant-create", args=[source.id])) - self.assertEqual(response.status_code, 200) - - def test_template_used(self): - fake_sources = [make_fake_source() for _ in range(10)] - for source in fake_sources: - with patch("requests.get", mock_requests_get): - response = self.client.get(reverse("chant-create", args=[source.id])) - self.assertEqual(response.status_code, 200) - self.assertTemplateUsed(response, "base.html") - self.assertTemplateUsed(response, "chant_create.html") - - def test_volpiano_signal(self): - source = make_fake_source() + def test_volpiano_signal(self) -> None: + source = self.source self.client.post( reverse("chant-create", args=[source.id]), { @@ -2844,10 +2850,9 @@ def test_volpiano_signal(self): # clefs, accidentals, etc., to be deleted }, ) - with patch("requests.get", mock_requests_get): - chant_1 = Chant.objects.get( - manuscript_full_text_std_spelling="ut queant lactose" - ) + chant_1 = Chant.objects.get( + manuscript_full_text_std_spelling="ut queant lactose" + ) self.assertEqual(chant_1.volpiano, "9abcdefg)A-B1C2D3E4F5G67?. yiz") self.assertEqual(chant_1.volpiano_notes, "9abcdefg9abcdefg") self.client.post( @@ -2859,16 +2864,13 @@ def test_volpiano_signal(self): "volpiano": "abacadaeafagahaja", }, ) - 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, "abacadaeafagahaja") self.assertEqual(chant_2.volpiano_intervals, "1-12-23-34-45-56-67-78-8") - def test_initial_values(self): + def test_initial_values(self) -> None: # create a chant with a known folio, feast, service, c_sequence and image_link - source: Source = make_fake_source() + source: Source = self.source folio: str = "001r" sequence: int = 1 feast: Feast = make_fake_feast() @@ -2885,12 +2887,11 @@ def test_initial_values(self): "image_link": image_link, }, ) - with patch("requests.get", mock_requests_get): - # when we request the Chant Create page, the same folio, feast, service and image_link should - # be preselected, and c_sequence should be incremented by 1. - response = self.client.get( - reverse("chant-create", args=[source.id]), - ) + # when we request the Chant Create page, the same folio, feast, service and image_link should + # be preselected, and c_sequence should be incremented by 1. + response = self.client.get( + reverse("chant-create", args=[source.id]), + ) observed_initial_folio: int = response.context["form"].initial["folio"] with self.subTest(subtest="test initial value of folio field"): @@ -2912,12 +2913,11 @@ def test_initial_values(self): with self.subTest(subtest="test initial value of image_link field"): self.assertEqual(observed_initial_image, image_link) - def test_suggested_chant_buttons(self): - source: Source = make_fake_source() - with patch("requests.get", mock_requests_get): - response_empty_source = self.client.get( - reverse("chant-create", args=[source.id]), - ) + def test_suggested_chant_buttons(self) -> None: + source: Source = self.source + response_empty_source = self.client.get( + reverse("chant-create", args=[source.id]), + ) with self.subTest( test="Ensure no suggestions displayed when there is no previous chant" ): @@ -2925,11 +2925,11 @@ def test_suggested_chant_buttons(self): response_empty_source, "Suggestions based on previous chant:" ) - previous_chant: Chant = make_fake_chant(cantus_id="001010", source=source) - with patch("requests.get", mock_requests_get): - response_after_previous_chant = self.client.get( - reverse("chant-create", args=[source.id]), - ) + # Make a chant to serve as the previous chant + make_fake_chant(cantus_id="001010", source=source) + response_after_previous_chant = self.client.get( + reverse("chant-create", args=[source.id]), + ) suggested_chants = response_after_previous_chant.context["suggested_chants"] with self.subTest( test="Ensure suggested chant suggestions present when previous chant exists" @@ -2940,11 +2940,11 @@ def test_suggested_chant_buttons(self): self.assertIsNotNone(suggested_chants) self.assertEqual(len(suggested_chants), 5) - rare_chant: Chant = make_fake_chant(cantus_id="a07763", source=source) - with patch("requests.get", mock_requests_get): - response_after_rare_chant = self.client.get( - reverse("chant-create", args=[source.id]), - ) + # Make a chant with a rare cantus_id to serve as the previous chant + make_fake_chant(cantus_id="a07763", source=source) + response_after_rare_chant = self.client.get( + reverse("chant-create", args=[source.id]), + ) with self.subTest( test="When previous chant has no suggested chants, ensure no suggestions are displayed" ): From 34c130ff49773d4c7e470b2eb8ed7196caa95f1b Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 3 Oct 2024 12:54:26 -0400 Subject: [PATCH 10/24] refactor(tests): fix lint errors in test_chant.py --- .../main_app/tests/test_views/test_chant.py | 62 ++++++++++--------- 1 file changed, 33 insertions(+), 29 deletions(-) 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 4a2a569f6..0d8374a47 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 @@ -113,12 +113,11 @@ def test_chant_edit_link(self): ) # have to create project manager user - "View | Edit" toggle only visible for those with edit access for a chant's source - self.user = get_user_model().objects.create(email="test@test.com") - self.user.set_password("pass") - self.user.save() - self.client = Client() + pm_user = get_user_model().objects.create(email="test@test.com") + pm_user.set_password("pass") + pm_user.save() project_manager = Group.objects.get(name="project manager") - project_manager.user_set.add(self.user) + project_manager.user_set.add(pm_user) self.client.login(email="test@test.com", password="pass") response = self.client.get(reverse("chant-detail", args=[chant.id])) @@ -502,7 +501,8 @@ def test_filter_by_melody(self): source=source, volpiano=make_fake_volpiano(), ) - chant_without_melody = Chant.objects.create(source=source) + # Create a chant without a melody + Chant.objects.create(source=source) response = self.client.get(reverse("chant-search"), {"melodies": "true"}) # only chants with melodies should be in the result self.assertEqual(len(response.context["chants"]), 1) @@ -554,9 +554,11 @@ def test_search_bar_search(self): manuscript_full_text="Full text contains, but does not start with 'the'", cantus_id="123456", ) - chant_starting_with_a_number = make_fake_chant( + # Create a chant starting with a number that won't be found by either + # search term + make_fake_chant( manuscript_full_text=( - "1 is a number. " "How unusual, to find an arabic numeral in a chant!" + "1 is a number. How unusual, to find an arabic numeral in a chant!" ), cantus_id="234567", ) @@ -1371,7 +1373,7 @@ def test_column_header_links(self): # additional properties for which there are search fields feast = make_fake_feast() position = make_random_string(1) - chant = make_fake_chant( + make_fake_chant( manuscript_full_text_std_spelling=fulltext, service=service, genre=genre, @@ -1528,7 +1530,7 @@ def test_feast_column(self): url = feast.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling=fulltext, feast=feast, @@ -1552,7 +1554,7 @@ def test_service_column(self): url = service.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling=fulltext, service=service, @@ -1576,7 +1578,7 @@ def test_genre_column(self): url = genre.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling=fulltext, genre=genre, @@ -1819,7 +1821,8 @@ def test_filter_by_melody(self): source=source, volpiano=make_fake_volpiano, ) - chant_without_melody = Chant.objects.create(source=source) + # Create a chant without melody that won't be in the result + Chant.objects.create(source=source) response = self.client.get( reverse("chant-search-ms", args=[source.id]), {"melodies": "true"} ) @@ -1837,11 +1840,11 @@ def test_keyword_search_starts_with(self): source=source, manuscript_full_text_std_spelling="quick brown fox jumps over the lazy dog", ) - chant_2 = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling="brown fox jumps over the lazy dog", ) - chant_3 = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling="lazy brown fox jumps quick over the dog", ) @@ -1860,7 +1863,8 @@ def test_keyword_search_contains(self): source=source, manuscript_full_text_std_spelling="Quick brown fox jumps over the lazy dog", ) - chant_2 = make_fake_chant( + # Make a chant that won't be returned by the search term + make_fake_chant( source=source, manuscript_full_text_std_spelling="brown fox jumps over the lazy dog", ) @@ -1886,11 +1890,11 @@ def test_indexing_notes_search_starts_with(self): source=source, indexing_notes="quick brown fox jumps over the lazy dog", ) - chant_2 = make_fake_chant( + make_fake_chant( source=source, indexing_notes="brown fox jumps over the lazy dog", ) - chant_3 = make_fake_chant( + make_fake_chant( source=source, indexing_notes="lazy brown fox jumps quick over the dog", ) @@ -1909,7 +1913,8 @@ def test_indexing_notes_search_contains(self): source=source, indexing_notes="Quick brown fox jumps over the lazy dog", ) - chant_2 = make_fake_chant( + # Make a chant that won't be returned by the search term + make_fake_chant( source=source, indexing_notes="brown fox jumps over the lazy dog", ) @@ -1932,13 +1937,13 @@ def test_keyword_search_searching_all_fields(self): doesnt_include_search_term = "longevity is the soul of wit" source = make_fake_source() - chant_ms_spelling = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text=includes_search_term, # <== includes_search_term manuscript_full_text_std_spelling=doesnt_include_search_term, ) - chant_std_spelling = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text=doesnt_include_search_term, manuscript_full_text_std_spelling=includes_search_term, # <== @@ -1955,7 +1960,8 @@ def test_keyword_search_searching_all_fields(self): manuscript_full_text_std_spelling=None, ) - chant_without_search_term = make_fake_chant( + # This chant contains no search terms + make_fake_chant( source=source, manuscript_full_text=doesnt_include_search_term, manuscript_full_text_std_spelling=doesnt_include_search_term, @@ -2344,7 +2350,7 @@ def test_column_header_links(self): # additional properties for which there are search fields feast = make_fake_feast() position = make_random_string(1) - chant = make_fake_chant( + make_fake_chant( service=service, genre=genre, cantus_id=cantus_id, @@ -2450,9 +2456,7 @@ def test_source_link_column(self): url = source.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( - source=source, manuscript_full_text_std_spelling=fulltext - ) + make_fake_chant(source=source, manuscript_full_text_std_spelling=fulltext) response = self.client.get( reverse("chant-search-ms", args=[source.id]), {"keyword": search_term, "op": "contains"}, @@ -2488,7 +2492,7 @@ def test_feast_column(self): url = feast.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling=fulltext, feast=feast, @@ -2513,7 +2517,7 @@ def test_service_column(self): url = service.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling=fulltext, service=service, @@ -2538,7 +2542,7 @@ def test_genre_column(self): url = genre.get_absolute_url() fulltext = "manuscript full text" search_term = "full" - chant = make_fake_chant( + make_fake_chant( source=source, manuscript_full_text_std_spelling=fulltext, genre=genre, From 3907d4201df9fafb3c721f172ba21e58c4590646 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Thu, 3 Oct 2024 14:05:42 -0400 Subject: [PATCH 11/24] refactor(chant create view): reduce duplicate queries and tests Reduces duplicate queries in the source create view. Removes a function that contained a duplicate test for the existence of a source. --- .../cantusdb_project/main_app/views/chant.py | 40 +++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index 4bb38dc16..b0bbdfbc2 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -760,13 +760,15 @@ class ChantCreateView(LoginRequiredMixin, UserPassesTestMixin, CreateView): template_name = "chant_create.html" form_class = ChantCreateForm pk_url_kwarg = "source_pk" + source: Source + latest_chant: Optional[Chant] def test_func(self): user = self.request.user source_id = self.kwargs.get(self.pk_url_kwarg) - source = get_object_or_404(Source, id=source_id) + self.source = get_object_or_404(Source, id=source_id) - return user_can_edit_chants_in_source(user, source) + 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): @@ -784,8 +786,10 @@ def get_initial(self): """ try: latest_chant = self.source.chant_set.latest("date_updated") + self.latest_chant = latest_chant except Chant.DoesNotExist: # if there is no chant in source, start with folio 001r, and c_sequence 1 + self.latest_chant = None return { "folio": "001r", "feast": "", @@ -807,22 +811,12 @@ def get_initial(self): "image_link": latest_image, } - def dispatch(self, request, *args, **kwargs): - """Make sure the source specified in url exists before we display the form""" - self.source = get_object_or_404(Source, pk=kwargs["source_pk"]) - return super().dispatch(request, *args, **kwargs) - - def get_suggested_feasts(self): + def get_suggested_feasts(self, latest_chant: Chant) -> dict[Feast, int]: """based on the feast of the most recently edited chant, provide a list of suggested feasts that might follow the feast of that chant. Returns: a dictionary, with feast objects as keys and counts as values """ - try: - latest_chant = self.source.chant_set.latest("date_updated") - except Chant.DoesNotExist: - return None - current_feast = latest_chant.feast chants_that_end_current_feast = Chant.objects.filter( is_last_chant_in_feast=True, feast=current_feast @@ -843,22 +837,18 @@ def get_suggested_feasts(self): def get_context_data(self, **kwargs: Any) -> dict[Any, Any]: context = super().get_context_data(**kwargs) context["source"] = self.source - previous_chant: Optional[Chant] = None - try: - previous_chant = self.source.chant_set.latest("date_updated") - except Chant.DoesNotExist: - pass + previous_chant = self.latest_chant context["previous_chant"] = previous_chant - context["suggested_feasts"] = self.get_suggested_feasts() - - previous_cantus_id: Optional[str] = None + suggested_feasts = None + suggested_chants = None if previous_chant: + suggested_feasts = self.get_suggested_feasts(previous_chant) previous_cantus_id = previous_chant.cantus_id - - suggested_chants = None - if previous_cantus_id: - suggested_chants = get_suggested_chants(previous_cantus_id) + if previous_cantus_id: + suggested_chants = get_suggested_chants(previous_cantus_id) + context["suggested_feasts"] = suggested_feasts context["suggested_chants"] = suggested_chants + return context def form_valid(self, form): From 6308058c3675992605f03f4a7af2c86b54d5cdb6 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 16:16:35 -0400 Subject: [PATCH 12/24] fix(source melody search): remove error from "source" queryset --- django/cantusdb_project/main_app/views/chant.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index 4bb38dc16..b68435911 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -553,9 +553,9 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) # if searching in a specific source, pass the source into context if self.request.GET.get("source"): - context["source"] = Source.objects.get( - id=self.request.GET.get("source") - ).select_related("holding_institution", "feast", "service", "genre") + context["source"] = Source.objects.select_related( + "holding_institution" + ).get(id=self.request.GET.get("source")) return context From 192a1640cfa61bf21edcb880024fcdcc566edf3b Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Fri, 4 Oct 2024 11:59:42 -0400 Subject: [PATCH 13/24] feat(chant views): catch invalid text errors Modifies chant create, edit, and detail views to prevent and catch text syllabification errors. Modifies edit syllabification view for the same. Invalidates chant text fields if they error on syllabification. Catches errors for texts with error rather than propagating to a server error. --- django/cantusdb_project/main_app/forms.py | 107 +++++++++++++----- .../main_app/templates/chant_create.html | 5 +- .../main_app/templates/chant_edit.html | 35 +++--- .../templates/chant_syllabification_edit.html | 9 +- .../main_app/tests/test_views/test_chant.py | 87 +++++++++++++- .../cantusdb_project/main_app/views/chant.py | 68 +++++++---- 6 files changed, 233 insertions(+), 78 deletions(-) diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 4f65fa883..997bc561b 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -1,5 +1,14 @@ from django import forms from django.contrib.auth.forms import ReadOnlyPasswordHashField +from django.contrib.auth import get_user_model +from django.db.models import Q +from django.contrib.admin.widgets import ( + FilteredSelectMultiple, +) +from django.forms.widgets import CheckboxSelectMultiple +from dal import autocomplete +from volpiano_display_utilities.cantus_text_syllabification import syllabify_text +from volpiano_display_utilities.latin_word_syllabification import LatinError from .models import ( Chant, Service, @@ -22,13 +31,6 @@ SelectWidget, CheckboxWidget, ) -from django.contrib.auth import get_user_model -from django.db.models import Q -from django.contrib.admin.widgets import ( - FilteredSelectMultiple, -) -from django.forms.widgets import CheckboxSelectMultiple -from dal import autocomplete # ModelForm allows to build a form directly from a model # see https://docs.djangoproject.com/en/3.0/topics/forms/modelforms/ @@ -71,6 +73,40 @@ def label_from_instance(self, obj): widget = CheckboxSelectMultiple() +class CantusDBLatinField(forms.CharField): + """ + A custom CharField for chant text fields. Validates that the text + can be syllabified (essentially, that it does not have any improper + characters). + """ + + def validate(self, value): + super().validate(value) + if value: + try: + syllabify_text(value) + except LatinError as err: + raise forms.ValidationError(str(err)) + except ValueError as exc: + raise forms.ValidationError("Invalid characters in text.") from exc + + +class CantusDBSyllabifiedLatinField(forms.CharField): + """ + A custom CharField for chant syllabified text fields. Validates that the text + can be syllabified (essentially, that it does not have any improper + characters). + """ + + def validate(self, value): + super().validate(value) + if value: + try: + syllabify_text(value, text_presyllabified=True) + except ValueError as exc: + raise forms.ValidationError("Invalid characters in text.") from exc + + class ChantCreateForm(forms.ModelForm): class Meta: model = Chant @@ -125,8 +161,8 @@ class Meta: "finalis": TextInputWidget(), "extra": TextInputWidget(), "chant_range": VolpianoInputWidget(), - # manuscript_full_text_std_spelling: defined below (required) - "manuscript_full_text": TextAreaWidget(), + # manuscript_full_text_std_spelling: defined below (required & special field) + # "manuscript_full_text": defined below (special field) "volpiano": VolpianoAreaWidget(), "image_link": TextInputWidget(), "melody_id": TextInputWidget(), @@ -153,14 +189,18 @@ class Meta: help_text="Each folio starts with '1'.", ) - manuscript_full_text_std_spelling = forms.CharField( + manuscript_full_text_std_spelling = CantusDBLatinField( + 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, + ) + + manuscript_full_text = CantusDBLatinField( widget=TextAreaWidget, - help_text="Manuscript full text with standardized spelling. Enter the words " - "according to the manuscript but normalize their spellings following " - "Classical Latin forms. Use upper-case letters for proper nouns, " - 'the first word of each chant, and the first word after "Alleluia" for ' - "Mass Alleluias. Punctuation is omitted.", + label="Full text as in Source (source spelling)", + help_text=Chant._meta.get_field("manuscript_full_text").help_text, + required=False, ) project = SelectWidgetNameModelChoiceField( @@ -319,8 +359,8 @@ class Meta: "rubrics", ] widgets = { - # manuscript_full_text_std_spelling: defined below (required) - "manuscript_full_text": TextAreaWidget(), + # manuscript_full_text_std_spelling: defined below (required) & special field + # manuscript_full_text: defined below (special field) "volpiano": VolpianoAreaWidget(), "marginalia": TextInputWidget(), # folio: defined below (required) @@ -354,14 +394,18 @@ class Meta: "rubrics": TextInputWidget(), } - manuscript_full_text_std_spelling = forms.CharField( + manuscript_full_text_std_spelling = CantusDBLatinField( + 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, + ) + + manuscript_full_text = CantusDBLatinField( widget=TextAreaWidget, - help_text="Manuscript full text with standardized spelling. Enter the words " - "according to the manuscript but normalize their spellings following " - "Classical Latin forms. Use upper-case letters for proper nouns, " - 'the first word of each chant, and the first word after "Alleluia" for ' - "Mass Alleluias. Punctuation is omitted.", + label="Full text as in Source (source spelling)", + help_text=Chant._meta.get_field("manuscript_full_text").help_text, + required=False, ) folio = forms.CharField( @@ -550,10 +594,14 @@ class Meta: "manuscript_full_text", "manuscript_syllabized_full_text", ] - widgets = { - "manuscript_full_text": TextAreaWidget(), - "manuscript_syllabized_full_text": TextAreaWidget(), - } + + manuscript_full_text = CantusDBLatinField( + widget=TextAreaWidget, label="Full text as in Source (source spelling)" + ) + + manuscript_syllabized_full_text = CantusDBSyllabifiedLatinField( + widget=TextAreaWidget, label="Syllabized full text" + ) class AdminCenturyForm(forms.ModelForm): @@ -738,10 +786,7 @@ class Meta: widget=TextInputWidget, ) - name = forms.CharField( - required=False, - widget=TextInputWidget - ) + name = forms.CharField(required=False, widget=TextInputWidget) holding_institution = forms.ModelChoiceField( queryset=Institution.objects.all().order_by("name"), diff --git a/django/cantusdb_project/main_app/templates/chant_create.html b/django/cantusdb_project/main_app/templates/chant_create.html index 5aec5de03..4b29bb0bb 100644 --- a/django/cantusdb_project/main_app/templates/chant_create.html +++ b/django/cantusdb_project/main_app/templates/chant_create.html @@ -223,7 +223,7 @@

Create Chant

- + {{ form.manuscript_full_text_std_spelling }}

{{ form.manuscript_full_text_std_spelling.help_text }} @@ -241,8 +241,7 @@

Create Chant

- + {{ form.manuscript_full_text }}

{{ form.manuscript_full_text.help_text }} diff --git a/django/cantusdb_project/main_app/templates/chant_edit.html b/django/cantusdb_project/main_app/templates/chant_edit.html index 038eef616..47860c7d5 100644 --- a/django/cantusdb_project/main_app/templates/chant_edit.html +++ b/django/cantusdb_project/main_app/templates/chant_edit.html @@ -21,6 +21,7 @@ {% for message in messages %}

{% endfor %} @@ -283,25 +284,33 @@

Syllabification is based on saved syllabized text.

{% endif %}
- {% for syl_text, syl_mel in syllabized_text_with_melody %} - -
{{ syl_mel }}
- -
{{ syl_text }}
-
- {% endfor %} + {% if syllabized_text_with_melody %} + {% for syl_text, syl_mel in syllabized_text_with_melody %} + +
{{ syl_mel }}
+ +
{{ syl_text }}
+
+ {% endfor %} + {% else %} +

Error aligning text and melody. Please check text for invalid characters.

+ {% endif %}
{% endif %} -
-
- - Edit syllabification (new window) - + + {% if syllabized_text_with_melody %} + -
+ {% endif %}
diff --git a/django/cantusdb_project/main_app/templates/chant_syllabification_edit.html b/django/cantusdb_project/main_app/templates/chant_syllabification_edit.html index 446afd73a..65b49f09f 100644 --- a/django/cantusdb_project/main_app/templates/chant_syllabification_edit.html +++ b/django/cantusdb_project/main_app/templates/chant_syllabification_edit.html @@ -12,6 +12,7 @@ {% for message in messages %} {% endfor %} @@ -37,8 +38,8 @@

Edit Syllabification

-
@@ -46,8 +47,8 @@

Edit Syllabification

-
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 0d8374a47..d91ef212c 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 @@ -164,7 +164,6 @@ def setUp(self): self.user = get_user_model().objects.create(email="test@test.com") self.user.set_password("pass") self.user.save() - self.client = Client() project_manager = Group.objects.get(name="project manager") project_manager.user_set.add(self.user) self.client.login(email="test@test.com", password="pass") @@ -323,6 +322,44 @@ def test_proofread_chant(self): chant.refresh_from_db() self.assertIs(chant.manuscript_full_text_std_proofread, True) + def test_invalid_text(self) -> None: + """ + The user should not be able to create a chant with invalid text + (either invalid characters or unmatched brackets). + Instead, the user should be shown an error message. + """ + source = make_fake_source() + with self.subTest("Chant with invalid characters"): + response = self.client.post( + reverse("source-edit-chants", args=[source.id]), + { + "manuscript_full_text_std_spelling": "this is a ch@nt t%xt with inv&lid ch!ra+ers", + "folio": "001r", + "c_sequence": "1", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertFormError( + response.context["form"], + "manuscript_full_text_std_spelling", + "Invalid characters in text.", + ) + with self.subTest("Chant with unmatched brackets"): + response = self.client.post( + reverse("source-edit-chants", args=[source.id]), + { + "manuscript_full_text_std_spelling": "this is a chant with [ unmatched brackets", + "folio": "001r", + "c_sequence": "1", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertFormError( + response.context["form"], + "manuscript_full_text_std_spelling", + "Word [ contains non-alphabetic characters.", + ) + class ChantEditSyllabificationViewTest(TestCase): @classmethod @@ -363,7 +400,10 @@ def test_edit_syllabification(self): self.assertEqual(chant.manuscript_syllabized_full_text, "lorem ipsum") response = self.client.post( f"/edit-syllabification/{chant.id}", - {"manuscript_syllabized_full_text": "lore-m i-psum"}, + { + "manuscript_full_text": "lorem ipsum", + "manuscript_syllabized_full_text": "lore-m i-psum", + }, ) self.assertEqual(response.status_code, 302) # 302 Found chant.refresh_from_db() @@ -2817,13 +2857,13 @@ def test_repeated_seq(self) -> None: for i in range(1, 5): Chant.objects.create( source=source, - manuscript_full_text=faker.text(10), + manuscript_full_text=" ".join(faker.words(faker.random_int(3, 10))), folio=test_folio, c_sequence=i, ) # post a chant with the same folio and seq url = reverse("chant-create", args=[source.id]) - fake_text = faker.text(10) + fake_text = "this is also a fake but valid text" response = self.client.post( url, data={ @@ -2960,6 +3000,45 @@ def test_suggested_chant_buttons(self) -> None: ) self.assertIsNone(response_after_rare_chant.context["suggested_chants"]) + def test_invalid_text(self) -> None: + """ + The user should not be able to create a chant with invalid text + (either invalid characters or unmatched brackets). + Instead, the user should be shown an error message. + """ + with self.subTest("Chant with invalid characters"): + source = self.source + response = self.client.post( + reverse("chant-create", args=[source.id]), + { + "manuscript_full_text_std_spelling": "this is a ch@nt t%xt with inv&lid ch!ra+ers", + "folio": "001r", + "c_sequence": "1", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertFormError( + response.context["form"], + "manuscript_full_text_std_spelling", + "Invalid characters in text.", + ) + with self.subTest("Chant with unmatched brackets"): + source = self.source + response = self.client.post( + reverse("chant-create", args=[source.id]), + { + "manuscript_full_text_std_spelling": "this is a chant with [ unmatched brackets", + "folio": "001r", + "c_sequence": "1", + }, + ) + self.assertEqual(response.status_code, 200) + self.assertFormError( + response.context["form"], + "manuscript_full_text_std_spelling", + "Word [ contains non-alphabetic characters.", + ) + class CISearchViewTest(TestCase): diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index b0bbdfbc2..654fdf71f 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -7,6 +7,7 @@ from django.contrib.auth.mixins import UserPassesTestMixin from django.core.exceptions import PermissionDenied from django.db.models import Q, QuerySet +from django.forms import BaseModelForm from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404 from django.urls import reverse @@ -18,6 +19,7 @@ TemplateView, UpdateView, ) +from volpiano_display_utilities.latin_word_syllabification import LatinError from volpiano_display_utilities.cantus_text_syllabification import ( syllabify_text, flatten_syllabified_text, @@ -205,11 +207,14 @@ def get_context_data(self, **kwargs): # syllabification section if chant.volpiano: has_syl_text = bool(chant.manuscript_syllabized_full_text) - text_and_mel, _ = align_text_and_volpiano( - chant.get_best_text_for_syllabizing(), - chant.volpiano, - text_presyllabified=has_syl_text, - ) + try: + text_and_mel, _ = align_text_and_volpiano( + chant.get_best_text_for_syllabizing(), + chant.volpiano, + text_presyllabified=has_syl_text, + ) + except LatinError: + text_and_mel = None context["syllabized_text_with_melody"] = text_and_mel if project := chant.project: @@ -848,16 +853,19 @@ def get_context_data(self, **kwargs: Any) -> dict[Any, Any]: suggested_chants = get_suggested_chants(previous_cantus_id) context["suggested_feasts"] = suggested_feasts context["suggested_chants"] = suggested_chants - return context def form_valid(self, form): - """compute source, incipit; folio/sequence (if left empty) - validate the form: add success/error message + """ + 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. """ # compute source - form.instance.source = self.source # same effect as the next line - # form.instance.source = get_object_or_404(Source, pk=self.kwargs['source_pk']) + form.instance.source = self.source # compute incipit, within 30 charactors, keep words complete words = form.instance.manuscript_full_text_std_spelling.split(" ") @@ -893,8 +901,7 @@ def form_valid(self, form): "Chant '" + form.instance.incipit + "' created successfully!", ) return super().form_valid(form) - else: - return super().form_invalid(form) + return super().form_invalid(form) class ChantDeleteView(LoginRequiredMixin, UserPassesTestMixin, DeleteView): @@ -1113,11 +1120,18 @@ def get_context_data(self, **kwargs): has_syl_text = bool(chant.manuscript_syllabized_full_text) # Note: the second value returned is a flag indicating whether the alignment process # encountered errors. In future, this could be used to display a message to the user. - text_and_mel, _ = align_text_and_volpiano( - chant.get_best_text_for_syllabizing(), - chant.volpiano, - text_presyllabified=has_syl_text, - ) + try: + text_and_mel, _ = align_text_and_volpiano( + chant.get_best_text_for_syllabizing(), + chant.volpiano, + text_presyllabified=has_syl_text, + ) + except LatinError as err: + messages.error( + self.request, + "Error in aligning text and melody: " + str(err), + ) + text_and_mel = None context["syllabized_text_with_melody"] = text_and_mel user = self.request.user @@ -1224,12 +1238,20 @@ def get_initial(self): initial = super().get_initial() chant = self.get_object() has_syl_text = bool(chant.manuscript_syllabized_full_text) - syls_text, _ = syllabify_text( - text=chant.get_best_text_for_syllabizing(), - clean_text=True, - text_presyllabified=has_syl_text, - ) - self.flattened_syls_text = flatten_syllabified_text(syls_text) + try: + syls_text, _ = syllabify_text( + text=chant.get_best_text_for_syllabizing(), + clean_text=True, + text_presyllabified=has_syl_text, + ) + self.flattened_syls_text = flatten_syllabified_text(syls_text) + except LatinError as err: + messages.error( + self.request, + "Error in syllabifying text: " + str(err), + ) + syls_text = None + self.flattened_syls_text = "" initial["manuscript_syllabized_full_text"] = self.flattened_syls_text return initial From ff924651b5c54e78526f41df9796bdd8e0e0b60e Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 11:22:24 -0400 Subject: [PATCH 14/24] feat(source model): add source_completeness and production_method fields Updates source create, source edit, and source detail pages. Adds command to populate the `source_completeness` field based on the current contents of the `full_source` field (which can be deleted after this field is populated). --- django/cantusdb_project/main_app/forms.py | 26 +++++------------ .../commands/populate_source_completeness.py | 22 ++++++++++++++ .../main_app/models/source.py | 24 ++++++++++++++- .../main_app/templates/source_create.html | 13 +++++---- .../main_app/templates/source_detail.html | 11 ++++--- .../main_app/templates/source_edit.html | 12 ++++---- .../test_populate_source_completeness.py | 29 +++++++++++++++++++ 7 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 django/cantusdb_project/main_app/management/commands/populate_source_completeness.py create mode 100644 django/cantusdb_project/main_app/tests/test_populate_source_completeness.py diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 4f65fa883..98ef482d3 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -212,6 +212,8 @@ class Meta: "fragmentarium_id", "dact_id", "indexing_notes", + "production_method", + "source_completeness", ] widgets = { # "title": TextInputWidget(), @@ -246,6 +248,8 @@ class Meta: "other_editors": autocomplete.ModelSelect2Multiple( url="all-users-autocomplete" ), + "production_method": SelectWidget(), + "source_completeness": SelectWidget(), } field_classes = { "segment_m2m": CheckboxNameModelMultipleChoiceField, @@ -262,15 +266,6 @@ class Meta: widget=TextInputWidget, ) - TRUE_FALSE_CHOICES_SOURCE = ( - (True, "Full source"), - (False, "Fragment or Fragmented"), - ) - - full_source = forms.ChoiceField(choices=TRUE_FALSE_CHOICES_SOURCE, required=False) - full_source.widget.attrs.update( - {"class": "form-control custom-select custom-select-sm"} - ) TRUE_FALSE_CHOICES_INVEN = ((True, "Complete"), (False, "Incomplete")) complete_inventory = forms.ChoiceField( @@ -413,6 +408,8 @@ class Meta: "full_text_entered_by", "proofreaders", "other_editors", + "production_method", + "source_completeness", ] widgets = { "segment_m2m": CheckboxSelectMultiple(), @@ -446,6 +443,8 @@ class Meta: "other_editors": autocomplete.ModelSelect2Multiple( url="all-users-autocomplete" ), + "production_method": SelectWidget(), + "source_completeness": SelectWidget(), } field_classes = { "segment_m2m": CheckboxNameModelMultipleChoiceField, @@ -462,15 +461,6 @@ class Meta: widget=autocomplete.ModelSelect2(url="holding-autocomplete"), ) - CHOICES_FULL_SOURCE = ( - (None, "None"), - (True, "Full source"), - (False, "Fragment or Fragmented"), - ) - full_source = forms.ChoiceField(choices=CHOICES_FULL_SOURCE, required=False) - full_source.widget.attrs.update( - {"class": "form-control custom-select custom-select-sm"} - ) CHOICES_CURSUS = ( (None, "None"), diff --git a/django/cantusdb_project/main_app/management/commands/populate_source_completeness.py b/django/cantusdb_project/main_app/management/commands/populate_source_completeness.py new file mode 100644 index 000000000..fe7d9e279 --- /dev/null +++ b/django/cantusdb_project/main_app/management/commands/populate_source_completeness.py @@ -0,0 +1,22 @@ +""" +A temporary command to populate the source_completeness field in the Source model, +based on the full_source field. This command will be removed once the source_completeness +is initially populated. +""" + +from django.core.management.base import BaseCommand +from main_app.models import Source + + +class Command(BaseCommand): + def handle(self, *args, **options): + sources = Source.objects.all() + for source in sources: + if source.full_source: + source.source_completeness = ( + source.SourceCompletenessChoices.FULL_SOURCE + ) + else: + source.source_completeness = source.SourceCompletenessChoices.FRAGMENT + source.save() + self.stdout.write(self.style.SUCCESS("Source completeness populated")) diff --git a/django/cantusdb_project/main_app/models/source.py b/django/cantusdb_project/main_app/models/source.py index 98caddc06..5b08f2082 100644 --- a/django/cantusdb_project/main_app/models/source.py +++ b/django/cantusdb_project/main_app/models/source.py @@ -71,6 +71,18 @@ class Source(BaseModel): null=True, help_text="More exact indication of the provenance (if necessary)", ) + + class SourceCompletenessChoices(models.IntegerChoices): + FULL_SOURCE = 1, "Full source" + FRAGMENT = 2, "Fragment/Fragmented" + RECONSTRUCTION = 3, "Reconstruction" + + source_completeness = models.IntegerField( + choices=SourceCompletenessChoices.choices, + default=SourceCompletenessChoices.FULL_SOURCE, + verbose_name="Full Source/Fragment", + ) + full_source = models.BooleanField(blank=True, null=True) date = models.CharField( blank=True, @@ -140,6 +152,16 @@ class Source(BaseModel): blank=False, null=False, default=False ) + class ProductionMethodChoices(models.IntegerChoices): + MANUSCRIPT = 1, "Manuscript" + PRINTED = 2, "Printed" + + production_method = models.IntegerField( + default=ProductionMethodChoices.MANUSCRIPT, + choices=ProductionMethodChoices.choices, + verbose_name="Manuscript/Printed", + ) + # number_of_chants and number_of_melodies are used for rendering the source-list page (perhaps among other places) # they are automatically recalculated in main_app.signals.update_source_chant_count and # main_app.signals.update_source_melody_count every time a chant or sequence is saved or deleted @@ -182,7 +204,7 @@ def short_heading(self) -> str: tt = self.shelfmark if self.shelfmark else self.title title.append(tt) - if not self.full_source: + if self.source_completeness == self.SourceCompletenessChoices.FRAGMENT: title.append("(fragment)") return " ".join(title) diff --git a/django/cantusdb_project/main_app/templates/source_create.html b/django/cantusdb_project/main_app/templates/source_create.html index 62a8d50a3..d15d5f81f 100644 --- a/django/cantusdb_project/main_app/templates/source_create.html +++ b/django/cantusdb_project/main_app/templates/source_create.html @@ -89,19 +89,20 @@

Create Source

- - {{ form.full_source }} -

{{ form.full_source.help_text }}

+ {{ form.source_completeness.label_tag }} + {{ form.source_completeness }}
-
+
{{ form.century.label_tag }} {{ form.century }}
+
+ {{ form.production_method.label_tag }} + {{ form.production_method }} +
diff --git a/django/cantusdb_project/main_app/templates/source_detail.html b/django/cantusdb_project/main_app/templates/source_detail.html index a7eab6478..f22603e44 100644 --- a/django/cantusdb_project/main_app/templates/source_detail.html +++ b/django/cantusdb_project/main_app/templates/source_detail.html @@ -43,6 +43,11 @@

{{ source.heading }}

{% endif %} + {% if source.production_method %} +
Manuscript/Printed
+
{{ source.get_production_method_display }}
+ {% endif %} + {% if source.summary %}
Summary
{{ source.summary }}
@@ -112,10 +117,8 @@

{{ source.heading }}

{{ source.complete_inventory|yesno:"Complete Inventory,Partial Inventory" }}
{% endif %} - {% if source.full_source is not None %} -
Full Source/Fragment
-
{{ source.full_source|yesno:"Full Source,Fragment or Fragmented" }}
- {% endif %} +
Full Source/Fragment
+
{{ source.get_source_completeness_display }}
{% if user.is_authenticated %}
Source Status
diff --git a/django/cantusdb_project/main_app/templates/source_edit.html b/django/cantusdb_project/main_app/templates/source_edit.html index 89d876b07..48a112d99 100644 --- a/django/cantusdb_project/main_app/templates/source_edit.html +++ b/django/cantusdb_project/main_app/templates/source_edit.html @@ -90,18 +90,20 @@

More exact indication of the provenance (if necessary)

- - {{ form.full_source }} + {{ form.source_completeness.label_tag }} + {{ form.source_completeness }}
-
+
{{ form.century.label_tag }} {{ form.century }}
+
+ {{ form.production_method.label_tag }} + {{ form.production_method }} +
diff --git a/django/cantusdb_project/main_app/tests/test_populate_source_completeness.py b/django/cantusdb_project/main_app/tests/test_populate_source_completeness.py new file mode 100644 index 000000000..065bbb2ea --- /dev/null +++ b/django/cantusdb_project/main_app/tests/test_populate_source_completeness.py @@ -0,0 +1,29 @@ +from django.test import TestCase +from django.core.management import call_command + +from main_app.models import Source +from main_app.tests.make_fakes import make_fake_source + + +class TestPopulateSourceCompleteness(TestCase): + def test_populate_source_completeness(self): + # make a few "Full Source" sources + for _ in range(5): + make_fake_source(full_source=True) + # make a few "Fragment" sources + for _ in range(3): + make_fake_source(full_source=False) + # run the command + call_command("populate_source_completeness") + sources = Source.objects.all() + for source in sources: + if source.full_source: + self.assertEqual( + source.source_completeness, + source.SourceCompletenessChoices.FULL_SOURCE, + ) + else: + self.assertEqual( + source.source_completeness, + source.SourceCompletenessChoices.FRAGMENT, + ) From 2a272c87883bdd310ee82e91ed9a8b80f87a7326 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 11:30:59 -0400 Subject: [PATCH 15/24] refactor(forms): create StyledChoiceField on forms Removes the need to duplicate process of using default SelectWidget on ChoiceFields and then manually changing attributes. --- django/cantusdb_project/main_app/forms.py | 29 ++++++++++------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 98ef482d3..1bd0760aa 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -71,6 +71,15 @@ def label_from_instance(self, obj): widget = CheckboxSelectMultiple() +class StyledChoiceField(forms.ChoiceField): + """ + A custom ChoiceField that uses the custom SelectWidget defined in widgets.py + as its widget (for styling). + """ + + widget = SelectWidget() + + class ChantCreateForm(forms.ModelForm): class Meta: model = Chant @@ -268,12 +277,9 @@ class Meta: TRUE_FALSE_CHOICES_INVEN = ((True, "Complete"), (False, "Incomplete")) - complete_inventory = forms.ChoiceField( + complete_inventory = StyledChoiceField( choices=TRUE_FALSE_CHOICES_INVEN, required=False ) - complete_inventory.widget.attrs.update( - {"class": "form-control custom-select custom-select-sm"} - ) class ChantEditForm(forms.ModelForm): @@ -416,6 +422,7 @@ class Meta: "provenance": autocomplete.ModelSelect2(url="provenance-autocomplete"), "provenance_notes": TextInputWidget(), "date": TextInputWidget(), + "cursus": SelectWidget(), "summary": TextAreaWidget(), "liturgical_occasions": TextAreaWidget(), "description": TextAreaWidget(), @@ -461,23 +468,11 @@ class Meta: widget=autocomplete.ModelSelect2(url="holding-autocomplete"), ) - - CHOICES_CURSUS = ( - (None, "None"), - ("Monastic", "Monastic"), - ("Secular", "Secular"), - ) - cursus = forms.ChoiceField(choices=CHOICES_CURSUS, required=False) - cursus.widget.attrs.update({"class": "form-control custom-select custom-select-sm"}) - CHOICES_COMPLETE_INV = ( (True, "complete inventory"), (False, "partial inventory"), ) - complete_inventory = forms.ChoiceField(choices=CHOICES_COMPLETE_INV, required=False) - complete_inventory.widget.attrs.update( - {"class": "form-control custom-select custom-select-sm"} - ) + complete_inventory = StyledChoiceField(choices=CHOICES_COMPLETE_INV, required=False) class SequenceEditForm(forms.ModelForm): From f646e77ed95f4e02070d12a18f475e0ac56a7c45 Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 11:29:16 -0400 Subject: [PATCH 16/24] feat(source name): add source name to create, edit, and detail pages --- django/cantusdb_project/main_app/forms.py | 9 ++++----- django/cantusdb_project/main_app/models/source.py | 2 +- .../main_app/templates/source_create.html | 7 +++++++ .../main_app/templates/source_detail.html | 4 ++-- .../cantusdb_project/main_app/templates/source_edit.html | 7 +++++++ .../main_app/tests/test_views/test_source.py | 4 ++++ 6 files changed, 25 insertions(+), 8 deletions(-) diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 1bd0760aa..5ae87697b 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -200,6 +200,7 @@ class Meta: # "siglum", "holding_institution", "shelfmark", + "name", "segment_m2m", "provenance", "provenance_notes", @@ -228,6 +229,7 @@ class Meta: # "title": TextInputWidget(), # "siglum": TextInputWidget(), "provenance": autocomplete.ModelSelect2(url="provenance-autocomplete"), + "name": TextInputWidget(), "provenance_notes": TextInputWidget(), "date": TextInputWidget(), "cursus": SelectWidget(), @@ -392,6 +394,7 @@ class Meta: # "siglum", "holding_institution", "shelfmark", + "name", "segment_m2m", "provenance", "provenance_notes", @@ -419,6 +422,7 @@ class Meta: ] widgets = { "segment_m2m": CheckboxSelectMultiple(), + "name": TextInputWidget(), "provenance": autocomplete.ModelSelect2(url="provenance-autocomplete"), "provenance_notes": TextInputWidget(), "date": TextInputWidget(), @@ -723,11 +727,6 @@ class Meta: widget=TextInputWidget, ) - name = forms.CharField( - required=False, - widget=TextInputWidget - ) - holding_institution = forms.ModelChoiceField( queryset=Institution.objects.all().order_by("name"), required=True, diff --git a/django/cantusdb_project/main_app/models/source.py b/django/cantusdb_project/main_app/models/source.py index 5b08f2082..18577238f 100644 --- a/django/cantusdb_project/main_app/models/source.py +++ b/django/cantusdb_project/main_app/models/source.py @@ -54,7 +54,7 @@ class Source(BaseModel): max_length=255, blank=True, null=True, - help_text="A colloquial or commonly-used name for the source" + help_text="A colloquial or commonly-used name for the source", ) provenance = models.ForeignKey( "Provenance", diff --git a/django/cantusdb_project/main_app/templates/source_create.html b/django/cantusdb_project/main_app/templates/source_create.html index d15d5f81f..57ce751e9 100644 --- a/django/cantusdb_project/main_app/templates/source_create.html +++ b/django/cantusdb_project/main_app/templates/source_create.html @@ -62,6 +62,13 @@

Create Source

{{ form.shelfmark }}

{{ form.shelfmark.help_text }} +

+ + {{ form.name }} +

+ {{ form.name.help_text|safe }}

diff --git a/django/cantusdb_project/main_app/templates/source_detail.html b/django/cantusdb_project/main_app/templates/source_detail.html index f22603e44..5e4f8e2e3 100644 --- a/django/cantusdb_project/main_app/templates/source_detail.html +++ b/django/cantusdb_project/main_app/templates/source_detail.html @@ -34,9 +34,9 @@

{{ source.heading }}

{% endif %}
+
Cantus Siglum
+
{{ source.short_heading }}{% if source.name %} "{{ source.name }}"{% endif %}
{% if source.holding_institution %} -
Cantus Siglum
-
{{ source.short_heading }}
Holding Institution
{{ source.holding_institution }} diff --git a/django/cantusdb_project/main_app/templates/source_edit.html b/django/cantusdb_project/main_app/templates/source_edit.html index 48a112d99..1cde438b5 100644 --- a/django/cantusdb_project/main_app/templates/source_edit.html +++ b/django/cantusdb_project/main_app/templates/source_edit.html @@ -64,6 +64,13 @@

{{ form.shelfmark }}

{{ form.shelfmark.help_text }} +

+ + {{ form.name }} +

+ {{ form.name.help_text|safe }}

diff --git a/django/cantusdb_project/main_app/tests/test_views/test_source.py b/django/cantusdb_project/main_app/tests/test_views/test_source.py index e7102d4ec..8022998dc 100644 --- a/django/cantusdb_project/main_app/tests/test_views/test_source.py +++ b/django/cantusdb_project/main_app/tests/test_views/test_source.py @@ -60,6 +60,8 @@ def test_create_source(self): { "shelfmark": "test-shelfmark", # shelfmark is a required field "holding_institution": hinst.id, # holding institution is a required field + "source_completeness": "1", # required field + "production_method": "1", # required field }, ) @@ -113,6 +115,8 @@ def test_edit_source(self): { "shelfmark": "test-shelfmark", # shelfmark is a required field, "holding_institution": hinst.id, # holding institution is a required field + "source_completeness": "1", # required field + "production_method": "1", # required field }, ) From d97563c2a9b0a6421781004a69a128da7573ab2f Mon Sep 17 00:00:00 2001 From: Dylan Hillerbrand Date: Wed, 2 Oct 2024 12:17:18 -0400 Subject: [PATCH 17/24] fix(source): make holding_institution optional for sources This commit removes the requirement for a source to have a holding_institution. Tests and forms are updated to account for this. The `migrate_records` command is updated so that sources with un-parseable sigla that are not otherwise accounted for (by virtue of being private collections or prints) do not have institution records made. Note: For the purposes of data migration, no constraint is added to sources to designate when no holding institution is necessary. After migration, these should be modified. Refs: #1631 --- django/cantusdb_project/main_app/forms.py | 25 +++++-------------- .../management/commands/migrate_records.py | 6 +++-- .../main_app/models/institution.py | 4 +-- .../main_app/models/source.py | 18 ++++++++----- .../main_app/templates/source_create.html | 11 ++++---- .../main_app/templates/source_edit.html | 12 +++++---- .../main_app/templates/source_list.html | 13 +++++++--- .../main_app/tests/make_fakes.py | 24 +++++++++++++++--- .../main_app/tests/test_views/test_source.py | 21 ++++++++++------ 9 files changed, 80 insertions(+), 54 deletions(-) diff --git a/django/cantusdb_project/main_app/forms.py b/django/cantusdb_project/main_app/forms.py index 5ae87697b..282622240 100644 --- a/django/cantusdb_project/main_app/forms.py +++ b/django/cantusdb_project/main_app/forms.py @@ -228,6 +228,7 @@ class Meta: widgets = { # "title": TextInputWidget(), # "siglum": TextInputWidget(), + "shelfmark": TextInputWidget(), "provenance": autocomplete.ModelSelect2(url="provenance-autocomplete"), "name": TextInputWidget(), "provenance_notes": TextInputWidget(), @@ -268,13 +269,8 @@ class Meta: holding_institution = forms.ModelChoiceField( queryset=Institution.objects.all(), - required=True, widget=autocomplete.ModelSelect2(url="holding-autocomplete"), - ) - - shelfmark = forms.CharField( - required=True, - widget=TextInputWidget, + required=False, ) TRUE_FALSE_CHOICES_INVEN = ((True, "Complete"), (False, "Incomplete")) @@ -421,6 +417,7 @@ class Meta: "source_completeness", ] widgets = { + "shelfmark": TextInputWidget(), "segment_m2m": CheckboxSelectMultiple(), "name": TextInputWidget(), "provenance": autocomplete.ModelSelect2(url="provenance-autocomplete"), @@ -461,15 +458,10 @@ class Meta: "segment_m2m": CheckboxNameModelMultipleChoiceField, } - shelfmark = forms.CharField( - required=True, - widget=TextInputWidget, - ) - holding_institution = forms.ModelChoiceField( queryset=Institution.objects.all(), - required=True, widget=autocomplete.ModelSelect2(url="holding-autocomplete"), + required=False, ) CHOICES_COMPLETE_INV = ( @@ -722,14 +714,9 @@ class Meta: # help_text="RISM-style siglum + Shelf-mark (e.g. GB-Ob 202).", # ) - shelfmark = forms.CharField( - required=True, - widget=TextInputWidget, - ) - holding_institution = forms.ModelChoiceField( - queryset=Institution.objects.all().order_by("name"), - required=True, + queryset=Institution.objects.all().order_by("siglum"), + required=False, ) provenance = forms.ModelChoiceField( diff --git a/django/cantusdb_project/main_app/management/commands/migrate_records.py b/django/cantusdb_project/main_app/management/commands/migrate_records.py index bfda4e0a6..bc3f26325 100644 --- a/django/cantusdb_project/main_app/management/commands/migrate_records.py +++ b/django/cantusdb_project/main_app/management/commands/migrate_records.py @@ -163,7 +163,7 @@ def handle(self, *args, **options): ) ) institution = print_inst - elif siglum in created_institutions: + elif siglum in created_institutions and source.id not in bad_siglum: print( self.style.SUCCESS( f"Re-using the pre-created institution for {siglum}" @@ -185,7 +185,7 @@ def handle(self, *args, **options): institution.alternate_names = "\n".join(list(deduped_names)) institution.save() - elif siglum not in created_institutions: + elif siglum not in created_institutions and source.id not in bad_siglum: print(self.style.SUCCESS(f"Creating institution record for {siglum}")) iobj = { @@ -229,6 +229,8 @@ def handle(self, *args, **options): created_institutions[siglum] = institution else: + source.shelfmark = shelfmark.strip() + source.save() print( self.style.ERROR( f"Could not determine the holding institution for {source}" diff --git a/django/cantusdb_project/main_app/models/institution.py b/django/cantusdb_project/main_app/models/institution.py index e38450d90..a5d966f1b 100644 --- a/django/cantusdb_project/main_app/models/institution.py +++ b/django/cantusdb_project/main_app/models/institution.py @@ -28,7 +28,7 @@ class Meta: ), ] - name = models.CharField(max_length=255, default="s.n.") + name = models.CharField(max_length=255, default="[No Name]") siglum = models.CharField( verbose_name="RISM Siglum", max_length=32, @@ -50,7 +50,7 @@ class Meta: region = models.CharField( max_length=64, blank=True, null=True, help_text=region_help_text ) - country = models.CharField(max_length=64, default="s.l.") + country = models.CharField(max_length=64, default="[No Country]") alternate_names = models.TextField( blank=True, null=True, help_text="Enter alternate names on separate lines." ) diff --git a/django/cantusdb_project/main_app/models/source.py b/django/cantusdb_project/main_app/models/source.py index 18577238f..037e16769 100644 --- a/django/cantusdb_project/main_app/models/source.py +++ b/django/cantusdb_project/main_app/models/source.py @@ -42,13 +42,18 @@ class Source(BaseModel): holding_institution = models.ForeignKey( "Institution", on_delete=models.PROTECT, - null=False, - blank=False, + null=True, + blank=True, ) shelfmark = models.CharField( max_length=255, blank=False, null=False, + help_text=( + "Primary Cantus Database identifier for the source " + "(e.g. library shelfmark, DACT ID, etc.)" + ), + default="[No Shelfmark]", ) name = models.CharField( max_length=255, @@ -186,9 +191,10 @@ def heading(self) -> str: title.append(city) title.append(f"{holdinst.name},") - tt = self.shelfmark if self.shelfmark else self.title + title.append(self.shelfmark) - title.append(tt) + if self.name: + title.append(f"({self.name})") return " ".join(title) @@ -198,8 +204,8 @@ def short_heading(self) -> str: if holdinst := self.holding_institution: if holdinst.siglum and holdinst.siglum != "XX-NN": title.append(f"{holdinst.siglum}") - elif holdinst.is_private_collector: - title.append("Cantus") + else: + title.append("Cantus") tt = self.shelfmark if self.shelfmark else self.title title.append(tt) diff --git a/django/cantusdb_project/main_app/templates/source_create.html b/django/cantusdb_project/main_app/templates/source_create.html index 57ce751e9..395a373fa 100644 --- a/django/cantusdb_project/main_app/templates/source_create.html +++ b/django/cantusdb_project/main_app/templates/source_create.html @@ -46,12 +46,9 @@

Create Source

{{ form.holding_institution }} -

- {{ form.holding_institution.help_text }} -

@@ -60,8 +57,10 @@

Create Source

Shelfmark:* {{ form.shelfmark }} -

- {{ form.shelfmark.help_text }} +

+ {{ form.shelfmark.help_text|safe }} +

+