diff --git a/web-app/django/VIM/apps/instruments/admin.py b/web-app/django/VIM/apps/instruments/admin.py index fa511e3c..43bc3669 100644 --- a/web-app/django/VIM/apps/instruments/admin.py +++ b/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) diff --git a/web-app/django/VIM/apps/instruments/management/commands/import_instruments.py b/web-app/django/VIM/apps/instruments/management/commands/import_instruments.py index 7dc85eab..0cc45153 100644 --- a/web-app/django/VIM/apps/instruments/management/commands/import_instruments.py +++ b/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: diff --git a/web-app/django/VIM/apps/instruments/management/commands/index_data.py b/web-app/django/VIM/apps/instruments/management/commands/index_data.py index 8ca2b99a..a7efac22 100644 --- a/web-app/django/VIM/apps/instruments/management/commands/index_data.py +++ b/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( diff --git a/web-app/django/VIM/apps/instruments/migrations/0014_hornbostelsachs_and_more.py b/web-app/django/VIM/apps/instruments/migrations/0014_hornbostelsachs_and_more.py new file mode 100644 index 00000000..eae1bc5a --- /dev/null +++ b/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", + ), + ), + ] diff --git a/web-app/django/VIM/apps/instruments/models/__init__.py b/web-app/django/VIM/apps/instruments/models/__init__.py index 54813b80..3361ecdf 100644 --- a/web-app/django/VIM/apps/instruments/models/__init__.py +++ b/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 diff --git a/web-app/django/VIM/apps/instruments/models/hornbostel_sachs.py b/web-app/django/VIM/apps/instruments/models/hornbostel_sachs.py new file mode 100644 index 00000000..66b1e7fd --- /dev/null +++ b/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) diff --git a/web-app/django/VIM/apps/instruments/models/instrument.py b/web-app/django/VIM/apps/instruments/models/instrument.py index 7c8e432f..811e274f 100644 --- a/web-app/django/VIM/apps/instruments/models/instrument.py +++ b/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, diff --git a/web-app/django/VIM/apps/instruments/utils/validators.py b/web-app/django/VIM/apps/instruments/utils/validators.py index ec960f89..48dd7461 100644 --- a/web-app/django/VIM/apps/instruments/utils/validators.py +++ b/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]: diff --git a/web-app/django/VIM/apps/instruments/views/instrument_detail.py b/web-app/django/VIM/apps/instruments/views/instrument_detail.py index 18add74b..8881fcb7 100644 --- a/web-app/django/VIM/apps/instruments/views/instrument_detail.py +++ b/web-app/django/VIM/apps/instruments/views/instrument_detail.py @@ -3,7 +3,7 @@ from django.http import Http404 from django.views.generic import DetailView -from VIM.apps.instruments.models import Instrument, Language +from VIM.apps.instruments.models import Instrument, Language, HornbostelSachs class InstrumentDetail(DetailView): @@ -33,10 +33,11 @@ def get_object(self, queryset=None): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) + instrument = context["instrument"] # Query the instrument names in all languages - instrument_names = ( - context["instrument"].instrumentname_set.all().select_related("language") + instrument_names = instrument.instrumentname_set.all().select_related( + "language" ) if self.request.user.is_authenticated: # Show all names for authenticated users @@ -90,6 +91,7 @@ def get_context_data(self, **kwargs): context["active_tab"] = "instruments" + # Determine if the current user can delete the instrument context["can_delete_instrument"] = ( self.request.user.is_authenticated and context["instrument"].is_user_created @@ -98,5 +100,33 @@ def get_context_data(self, **kwargs): or context["instrument"].created_by == self.request.user ) ) + # Add user HBS to the context, if present + user_hbs = None + user = self.request.user + if user.is_authenticated: + user_hbs_qs = HornbostelSachs.objects.filter( + instrument=instrument, contributor=user + ).order_by( + "-is_main", "-id" + ) # prioritize main if more than one, fallback to latest + if user_hbs_qs.exists(): + user_hbs = user_hbs_qs.first() + context["user_hbs"] = user_hbs + + # Add HBS proposals for this instrument to the context, if instrument has no HBS + if ( + not instrument.hornbostel_sachs_class + or instrument.hornbostel_sachs_class.hbs_class == "0" + ): + hbs_proposals_qs = ( + HornbostelSachs.objects.filter(instrument=instrument, is_main=False) + .order_by("-id") + .values_list("hbs_class", flat=True) + ) + # Deduplicate and sort + hbs_proposals = sorted(set(hbs_proposals_qs)) + context["hbs_proposals"] = hbs_proposals + else: + context["hbs_proposals"] = None return context diff --git a/web-app/django/VIM/apps/instruments/views/update_umil_db.py b/web-app/django/VIM/apps/instruments/views/update_umil_db.py index 6817025d..6a96382f 100644 --- a/web-app/django/VIM/apps/instruments/views/update_umil_db.py +++ b/web-app/django/VIM/apps/instruments/views/update_umil_db.py @@ -7,7 +7,13 @@ from django.views.decorators.http import require_http_methods from django.http import HttpRequest, JsonResponse from django.core.exceptions import ValidationError -from VIM.apps.instruments.models import Instrument, Language, InstrumentName +from django.shortcuts import get_object_or_404 +from VIM.apps.instruments.models import ( + Instrument, + Language, + InstrumentName, + HornbostelSachs, +) from VIM.apps.instruments.utils.validators import ( validate_instrument_names, validate_umil_label_constraint, @@ -312,11 +318,102 @@ def schedule_indexing(): ) +def add_hbs(request, pk: int) -> JsonResponse: + """ + Add a Hornbostel-Sachs classification for an instrument. + + Expects JSON: + { + "wikidata_id": "Q12345", + "hornbostel_sachs_class": "111.242.12" + } + """ + + try: + data: Dict[str, Any] = json.loads(request.body) + except Exception as e: + return JsonResponse( + {"status": "error", "message": f"Invalid or missing JSON: {e}"}, + status=400, + ) + + wikidata_id = data.get("wikidata_id") + hbs_class = data.get("hornbostel_sachs_class") + + if not wikidata_id or not hbs_class: + return JsonResponse( + {"status": "error", "message": f"Missing required data"}, + status=400, + ) + + # Get instrument from pk (since path sends pk) + instrument = get_object_or_404(Instrument, wikidata_id=wikidata_id) + + # Check if the user has already provided an HBS classification for this instrument + existing_hbs = HornbostelSachs.objects.filter( + instrument=instrument, contributor=request.user + ) + + if existing_hbs.exists(): + # There is at least one existing HBS entry by this user for this instrument + for hbs in existing_hbs: + if hbs.hbs_class == hbs_class: + # If the submitted class is the same as an existing one, reject as duplicate + return JsonResponse( + { + "status": "error", + "message": f"You have already submitted this Hornbostel-Sachs class for this instrument.", + }, + status=400, + ) + # If the user already submitted an HBS for this instrument, update their previous entry with the new hbs_class + prev_hbs = existing_hbs.first() + prev_hbs.hbs_class = hbs_class + prev_hbs.review_status = "under_review" + prev_hbs.save() + return JsonResponse( + { + "status": "success", + "message": "Your Hornbostel-Sachs classification was updated to the new value.", + "hbs_id": prev_hbs.id, + }, + status=200, + ) + else: + # No previous HBS for this user/instrument + new_hbs = HornbostelSachs.objects.create( + instrument=instrument, + hbs_class=hbs_class, + contributor=request.user, + is_main=False, + review_status="under_review", + ) + return JsonResponse( + { + "status": "success", + "message": "Hornbostel-Sachs classification added successfully.", + "hbs_id": new_hbs.id, + }, + status=200, + ) + + @login_required @require_http_methods(["POST", "DELETE"]) def update_umil_db(request: HttpRequest, umil_id: str) -> JsonResponse: if request.method == "POST": - return add_name(request, umil_id) + try: + data = json.loads(request.body) + except Exception: + return JsonResponse( + {"status": "error", "message": "Malformed JSON."}, + status=400, + ) + # If the payload contains 'hornbostel_sachs_class', it is an add_class operation + if data.get("hornbostel_sachs_class") is not None: + return add_hbs(request, umil_id) + else: + return add_name(request, umil_id) elif request.method == "DELETE": return delete_name(request) diff --git a/web-app/django/VIM/settings.py b/web-app/django/VIM/settings.py index fdddcf27..2b80b704 100644 --- a/web-app/django/VIM/settings.py +++ b/web-app/django/VIM/settings.py @@ -247,7 +247,7 @@ SOLR_URL = "http://solr:8983/solr/virtual-instrument-museum" SOLR_TIMEOUT = 10 -EMPTY_HBS_CATEGORY = "0" +EMPTY_HBS_CATEGORY = None # DEFAULT PAGE SETTINGS DEFAULT_LANGUAGE = "English" diff --git a/web-app/django/VIM/templates/instruments/detail.html b/web-app/django/VIM/templates/instruments/detail.html index e2e77c03..0eb27407 100644 --- a/web-app/django/VIM/templates/instruments/detail.html +++ b/web-app/django/VIM/templates/instruments/detail.html @@ -11,6 +11,7 @@ {% vite_asset 'src/instruments/JumpToTop.ts' %} {% vite_asset 'src/instruments/AddName.ts' %} {% vite_asset 'src/instruments/DeleteName.ts' %} + {% vite_asset 'src/instruments/AddClass.ts' %} {% if can_delete_instrument %} {% vite_asset 'src/instruments/DeleteInstrument.ts' %} {% endif %} @@ -68,7 +69,39 @@