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, the `shelfmark` field is allowed to be NULL and no constraint is added to sources to designate when no holding institution is necessary. After migration, these should be modified.

Refs: DDMAL#1631

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(

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}"

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."
     )

django/cantusdb_project/main_app/models/source.py

@@ -42,13 +42,17 @@ 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,
+        null=True,
+        help_text=(
+            "Primary Cantus Database identifier for the source "
+            "(e.g. library shelfmark, DACT ID, etc.)"
+        ),
     )
     name = models.CharField(
         max_length=255,
@@ -186,9 +190,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 +203,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)

django/cantusdb_project/main_app/templates/source_create.html

@@ -46,12 +46,9 @@ <h3>Create Source</h3>
                 <div class="form-row">
                     <div class="form-group m-1 col-lg-3">
                         <label for="{{ form.holding_institution.id_for_label }}" class="form-control-sm">
-                            Holding Institution:<span class="text-danger" title="This field is required">*</span>
+                            Holding Institution:
                         </label>
                         {{ form.holding_institution }}
-                        <p class=text-muted>
-                            <small>{{ form.holding_institution.help_text }}</small>
-                        </p>
                     </div>
                 </div>
                 <div class="form-row">
@@ -60,8 +57,10 @@ <h3>Create Source</h3>
                             Shelfmark:<span class="text-danger" title="This field is required">*</span>
                         </label>
                         {{ form.shelfmark }}
-                        <p class=text-muted>
-                            <small>{{ form.shelfmark.help_text }}</small>
+                        <p class="text-muted small">
+                            {{ form.shelfmark.help_text|safe }}
+                        </p>
+                    </div>
                     <div class="form-group m-1 col-lg-3">
                         <label for="{{ form.name.id_for_label }}" class="form-control-sm">
                             Name:

django/cantusdb_project/main_app/templates/source_edit.html

@@ -48,11 +48,11 @@ <h3>
         <div class="form-row">
             <div class="form-group m-1 col-lg-3">
                 <label for="{{ form.holding_institution.id_for_label }}" class="form-control-sm">
-                    Holding Institution:<span class="text-danger" title="This field is required">*</span>
+                    Holding Institution:
                 </label>
                 {{ form.holding_institution }}
-                <p class=text-muted>
-                    <small>{{ form.holding_institution.help_text }}</small>
+                <p class="text-muted small">
+                    {{ form.holding_institution.help_text }}
                 </p>
             </div>
         </div>
@@ -62,8 +62,10 @@ <h3>
                     Shelfmark:<span class="text-danger" title="This field is required">*</span>
                 </label>
                 {{ form.shelfmark }}
-                <p class=text-muted>
-                    <small>{{ form.shelfmark.help_text }}</small>
+                <p class="text-muted small">
+                    {{ form.shelfmark.help_text }}
+                </p>
+            </div>
             <div class="form-group m-1 col-lg-3">
                 <label for="{{ form.name.id_for_label }}" class="form-control-sm">
                     Name:

django/cantusdb_project/main_app/templates/source_list.html

@@ -103,9 +103,16 @@ <h3>Browse Sources</h3>
                             <b>{{ source.holding_institution.country }}</b>
                         </td>
                         <td class="text-wrap" style="text-align:center">
-                            <a href="{% url 'source-detail' source.id %}">
-                                <b>{{ source.holding_institution.city }}, {{ source.holding_institution.name }}</b>
-                            </a>
+                            {% if source.holding_institution %}
+                                <a href="{% url 'institution-detail' source.holding_institution.id %}">
+                                    <b>
+                                        {% if source.holding_institution.city %}
+                                            {{ source.holding_institution.city }},
+                                        {% endif %}
+                                        {{ source.holding_institution.name }}
+                                    </b>
+                                </a>
+                            {% endif %}
                         </td>
                         <td class="text-wrap" style="text-align:center">
                             <a href="{% url 'source-detail' source.id %}">

django/cantusdb_project/main_app/tests/make_fakes.py

@@ -362,15 +362,33 @@ def make_fake_institution(
     city: Optional[str] = None,
     region: Optional[str] = None,
     country: Optional[str] = None,
+    is_private_collector: Optional[bool] = None,
 ) -> Institution:
+    """
+    Note that the siglum and is_private_collector fields
+    are mutually exclusive. If both are specified, an exception
+    will be raised. If neither are specified, the function will
+    randomly determine whether the institution is a private collector or
+    will be given a fake siglum.
+    """
     name = name if name else faker.sentence()
-    siglum = siglum if siglum else faker.sentence(nb_words=1)
     city = city if city else faker.city()
     region = region if region else faker.country()
     country = country if country else faker.country()
 
+    if siglum and is_private_collector:
+        raise ValueError("Siglum and Private Collector cannot both be specified.")
+    is_private_collector = False if siglum else faker.boolean(chance_of_getting_true=20)
+    if not is_private_collector and not siglum:
+        siglum = faker.sentence(nb_words=1)
+
     inst = Institution.objects.create(
-        name=name, siglum=siglum, city=city, region=region, country=country
+        name=name,
+        siglum=siglum,
+        city=city,
+        region=region,
+        country=country,
+        is_private_collector=is_private_collector,
     )
     inst.save()
 
@@ -468,4 +486,4 @@ def get_random_search_term(target):
         slice_start = random.randint(0, len(target) - 2)
         slice_end = random.randint(slice_start + 2, len(target))
         search_term = target[slice_start:slice_end]
-    return search_term
+    return search_term

django/cantusdb_project/main_app/tests/test_views/test_source.py

@@ -54,17 +54,14 @@ def test_url_and_templates(self):
         self.assertTemplateUsed(response, "source_create.html")
 
     def test_create_source(self):
-        hinst = make_fake_institution(siglum="FA-Ke")
         response = self.client.post(
             reverse("source-create"),
             {
                 "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
             },
         )
-
         self.assertEqual(response.status_code, 302)
         created_source = Source.objects.get(shelfmark="test-shelfmark")
         self.assertRedirects(
@@ -108,13 +105,11 @@ def test_url_and_templates(self):
 
     def test_edit_source(self):
         source = make_fake_source()
-        hinst = make_fake_institution(siglum="FA-Ke")
 
         response = self.client.post(
             reverse("source-edit", args=[source.id]),
             {
                 "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
             },
@@ -985,18 +980,28 @@ def test_ordering(self) -> None:
         Order is currently available by country, city + institution name (parameter:
         "city_institution"), and siglum + shelfmark. Siglum + shelfmark is the default.
         """
-        # Create a bunch of sources
         sources = []
+        # Add a source from a private collector
+        private_collector = make_fake_institution(is_private_collector=True)
+        sources.append(make_fake_source(holding_institution=private_collector))
+        # Create a bunch of other sources
         for _ in range(10):
-            sources.append(make_fake_source())
+            inst = make_fake_institution(siglum=faker.word())
+            sources.append(make_fake_source(holding_institution=inst))
         # Default ordering is by siglum and shelfmark, ascending
         with self.subTest("Default ordering"):
             response = self.client.get(reverse("source-list"))
             response_sources = response.context["sources"]
             expected_source_order = sorted(
                 sources,
                 key=lambda source: (
-                    source.holding_institution.siglum,
+                    0 if source.holding_institution else 1,
+                    1 if source.holding_institution.is_private_collector else 0,
+                    (
+                        source.holding_institution.siglum
+                        if source.holding_institution.siglum
+                        else ""
+                    ),
                     source.shelfmark,
                 ),
             )