Added ability to propose a Hornbostel-Sachs number

web-app/django/VIM/apps/instruments/admin.py

@@ -1,5 +1,11 @@
 from django.contrib import admin
-from VIM.apps.instruments.models import Instrument, InstrumentName, Language, AVResource
+from VIM.apps.instruments.models import (
+    Instrument,
+    InstrumentName,
+    Language,
+    AVResource,
+    HornbostelSachs,
+)
 
 admin.site.register(Instrument)
 admin.site.register(Language)
@@ -30,3 +36,23 @@ def get_readonly_fields(self, request, obj=None):
                 "on_wikidata",
             )
         return super().get_readonly_fields(request, obj)
+
+
+@admin.register(HornbostelSachs)
+class HornbostelSachsAdmin(admin.ModelAdmin):
+    list_filter = ("review_status",)
+    search_fields = (
+        "instrument__wikidata_id",
+        "hbs_class",
+    )
+
+    def get_readonly_fields(self, request, obj=None):
+        """
+        For users in the 'reviewer' group, allow only 'review_status', 'hbs_class', and 'is_main' to be editable.
+        """
+        if request.user.groups.filter(name="reviewer").exists():
+            return (
+                "instrument",
+                "contributor",
+            )
+        return super().get_readonly_fields(request, obj)

web-app/django/VIM/apps/instruments/management/commands/import_instruments.py

@@ -10,7 +10,13 @@
 from django.core.management.base import BaseCommand
 from django.core.exceptions import ValidationError
 from django.db import transaction
-from VIM.apps.instruments.models import Instrument, InstrumentName, Language, AVResource
+from VIM.apps.instruments.models import (
+    Instrument,
+    InstrumentName,
+    Language,
+    AVResource,
+    HornbostelSachs,
+)
 from VIM.apps.instruments.utils.validators import validate_image_extension
 
 
@@ -131,7 +137,6 @@ def create_database_objects(
         instrument, created = Instrument.objects.update_or_create(
             wikidata_id=instrument_attrs["wikidata_id"],
             defaults={
-                "hornbostel_sachs_class": instrument_attrs["hornbostel_sachs_class"],
                 "mimo_class": instrument_attrs["mimo_class"],
             },
         )
@@ -196,6 +201,21 @@ def create_database_objects(
                     },
                 )
 
+        # Set Hornbostel-Sachs classification if present
+        hbs_value = (
+            instrument_attrs["hornbostel_sachs_class"] or settings.EMPTY_HBS_CATEGORY
+        )
+        hbs_obj = None
+        if hbs_value and hbs_value != settings.EMPTY_HBS_CATEGORY:
+            hbs_obj = HornbostelSachs.objects.create(
+                instrument=instrument,
+                hbs_class=hbs_value,
+                is_main=True,
+                review_status="verified",
+                contributor=self.default_contributor,
+            )
+        instrument.hornbostel_sachs_class = hbs_obj
+
         # Create AVResource objects only when both image paths are available
         if original_img_path and thumbnail_img_path:
             # Validate extensions before creating AVResource objects
@@ -208,6 +228,7 @@ def create_database_objects(
                         f"Skipping images for {instrument.umil_id} (invalid format): {e}"
                     )
                 )
+                instrument.save()  # Save instrument even if images are skipped
                 return
 
             img_obj, _ = AVResource.objects.update_or_create(
@@ -230,7 +251,8 @@ def create_database_objects(
                 },
             )
             instrument.thumbnail = thumbnail_obj
-            instrument.save()
+
+        instrument.save()
 
     @staticmethod
     def find_image_file(directory, ins_id):
@@ -248,7 +270,7 @@ def find_image_file(directory, ins_id):
         # Return relative path (for AVResource.url storage)
         filename = os.path.basename(
             matches[0]
-        )  # each instrunment is guaranteed to have at most one image
+        )  # each instrument is guaranteed to have at most one image
         return os.path.join(directory, filename)
 
     def handle(self, *args, **options) -> None:

web-app/django/VIM/apps/instruments/management/commands/index_data.py

@@ -33,8 +33,8 @@ def handle(self, *args, **options):
                 sid=Concat(V("instrument-"), "id", output_field=CharField()),
                 umil_id_s=F("umil_id"),
                 wikidata_id_s=F("wikidata_id"),
-                hornbostel_sachs_class_s=F("hornbostel_sachs_class"),
-                hbs_prim_cat_s=Left(F("hornbostel_sachs_class"), 1),
+                hornbostel_sachs_class_s=F("hornbostel_sachs_class__hbs_class"),
+                hbs_prim_cat_s=Left(F("hornbostel_sachs_class__hbs_class"), 1),
                 mimo_class_s=F("mimo_class"),
                 type=V("instrument"),
                 thumbnail_url=Case(

web-app/django/VIM/apps/instruments/migrations/0014_hornbostelsachs_and_more.py

@@ -0,0 +1,110 @@
+from django.db import migrations, models
+import django.db.models.deletion
+from django.conf import settings
+
+
+def migrate_strings_to_objects(apps, schema_editor):
+    Instrument = apps.get_model("instruments", "Instrument")
+    HornbostelSachs = apps.get_model("instruments", "HornbostelSachs")
+
+    # We use a raw queryset or values to avoid potential model logic issues
+    for inst in Instrument.objects.exclude(hornbostel_sachs_class__isnull=True):
+        hbs_string = inst.hornbostel_sachs_class
+
+        # Create the new object to link to
+        hbs_obj = HornbostelSachs.objects.create(
+            hbs_class=hbs_string,
+            instrument=inst,
+            is_main=True,
+            review_status="unverified",
+        )
+
+        # Update the character column with the integer ID
+        # PostgreSQL will allow this temporarily before the type change
+        Instrument.objects.filter(pk=inst.pk).update(hornbostel_sachs_class=hbs_obj.id)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ("instruments", "0013_alter_avresource_format_and_more"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="HornbostelSachs",
+            fields=[
+                (
+                    "id",
+                    models.BigAutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                (
+                    "hbs_class",
+                    models.CharField(
+                        help_text="Hornbostel-Sachs classification",
+                        max_length=50,
+                        null=True,
+                    ),
+                ),
+                (
+                    "is_main",
+                    models.BooleanField(
+                        default=False,
+                        help_text="Is this the main HBS classification for this instrument?",
+                    ),
+                ),
+                (
+                    "review_status",
+                    models.CharField(
+                        choices=[
+                            ("verified", "Verified"),
+                            ("unverified", "Unverified"),
+                            ("under_review", "Under Review"),
+                            ("needs_additional_review", "Needs Additional Review"),
+                            ("rejected", "Rejected"),
+                        ],
+                        default="unverified",
+                        max_length=50,
+                    ),
+                ),
+                (
+                    "contributor",
+                    models.ForeignKey(
+                        blank=True,
+                        null=True,
+                        on_delete=django.db.models.deletion.SET_NULL,
+                        to=settings.AUTH_USER_MODEL,
+                    ),
+                ),
+                (
+                    "instrument",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="hbs_entries",
+                        to="instruments.instrument",
+                    ),
+                ),
+            ],
+        ),
+        migrations.RunPython(
+            migrate_strings_to_objects, reverse_code=migrations.RunPython.noop
+        ),
+        migrations.AlterField(
+            model_name="instrument",
+            name="hornbostel_sachs_class",
+            field=models.ForeignKey(
+                blank=True,
+                help_text="Currently selected Hornbostel–Sachs classification",
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="main_for",
+                to="instruments.hornbostelsachs",
+            ),
+        ),
+    ]

web-app/django/VIM/apps/instruments/models/__init__.py

@@ -2,3 +2,4 @@
 from VIM.apps.instruments.models.instrument_name import InstrumentName
 from VIM.apps.instruments.models.language import Language
 from VIM.apps.instruments.models.avresource import AVResource
+from VIM.apps.instruments.models.hornbostel_sachs import HornbostelSachs

web-app/django/VIM/apps/instruments/models/hornbostel_sachs.py

@@ -0,0 +1,57 @@
+from django.db import models
+
+
+class HornbostelSachs(models.Model):
+    instrument = models.ForeignKey(
+        "Instrument",
+        on_delete=models.CASCADE,
+        related_name="hbs_entries",
+    )
+
+    hbs_class = models.CharField(
+        max_length=50, null=True, help_text="Hornbostel-Sachs classification"
+    )
+
+    is_main = models.BooleanField(
+        default=False,
+        help_text="Is this the main HBS classification for this instrument?",
+    )
+
+    review_status = models.CharField(
+        max_length=50,
+        choices=[
+            ("verified", "Verified"),
+            ("unverified", "Unverified"),
+            ("under_review", "Under Review"),
+            ("needs_additional_review", "Needs Additional Review"),
+            ("rejected", "Rejected"),
+        ],
+        default="unverified",
+    )
+
+    contributor = models.ForeignKey(
+        "auth.User",
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
+    )
+
+    # TODO: add verified_by field to track who verified the name
+
+    def save(self, *args, **kwargs):
+        super().save(*args, **kwargs)
+        if self.is_main:
+            Instrument = self._meta.get_field("instrument").related_model
+            instrument = self.instrument
+            if instrument.hornbostel_sachs_class_id != self.id:
+                instrument.hornbostel_sachs_class = self
+                instrument.save(update_fields=["hornbostel_sachs_class"])
+
+            # If there is another HBS object set as main for this instrument, unset others
+            other_mains = (
+                type(self)
+                .objects.filter(instrument=self.instrument, is_main=True)
+                .exclude(pk=self.pk)
+            )
+            if other_mains.exists():
+                other_mains.update(is_main=False)

web-app/django/VIM/apps/instruments/models/instrument.py

@@ -28,8 +28,13 @@ class Instrument(models.Model):
         null=True,
         related_name="thumbnail_of",
     )
-    hornbostel_sachs_class = models.CharField(
-        max_length=50, blank=True, help_text="Hornbostel-Sachs classification"
+    hornbostel_sachs_class = models.ForeignKey(
+        "HornbostelSachs",
+        on_delete=models.SET_NULL,
+        null=True,
+        blank=True,
+        related_name="main_for",
+        help_text="Currently selected Hornbostel–Sachs classification",
     )
     mimo_class = models.CharField(
         max_length=50,

web-app/django/VIM/apps/instruments/utils/validators.py

@@ -135,9 +135,9 @@ def validate_hbs_classification(hbs_class: str) -> bool:
     Validate Hornbostel-Sachs classification format.
 
     Valid formats:
-    - Two digits minimum (e.g., "11", "21")
-    - With optional sub-classifications (e.g., "21.2", "311.121")
-    - First digit must be 1-5, second digit 0-9
+    - At least 1 character, only digits (1-9), dot, dash, and plus permitted
+    - First character must be 1-5
+    - If there is a second character, it must be 1-5
 
     Args:
         hbs_class: Hornbostel-Sachs classification string to validate
@@ -147,16 +147,25 @@ def validate_hbs_classification(hbs_class: str) -> bool:
 
     Example:
         >>> validate_hbs_classification("11")       # True
-        >>> validate_hbs_classification("21.2")     # True
-        >>> validate_hbs_classification("311.121")  # True
-        >>> validate_hbs_classification("6")        # False (needs 2 digits)
-        >>> validate_hbs_classification("11x")      # False (invalid format)
+        >>> validate_hbs_classification("21.2+2")   # True
+        >>> validate_hbs_classification("6")        # False (first char not 1-5)
+        >>> validate_hbs_classification("11x")      # False (invalid char)
     """
     if not hbs_class:
         return False
-    # Pattern: one digit (1-5), followed by another digit, optionally followed by more .digits
-    pattern = r"^[1-5][0-9](\.[0-9]+)*$"
-    return bool(re.match(pattern, hbs_class)) and len(hbs_class) >= 2
+    # Only digits (1-9), dot, dash, plus permitted
+    if not re.match(r"^[1-9.\-+]+$", hbs_class):
+        return False
+    # First character must be 1-5
+    first_char = hbs_class[0]
+    if not re.match(r"[1-5]", first_char):
+        return False
+    # If there is a second character, it must be 1-5
+    if len(hbs_class) > 1:
+        second_char = hbs_class[1]
+        if not re.match(r"[1-5]", second_char):
+            return False
+    return True
 
 
 def validate_image_file(image_file) -> Tuple[bool, str]: