diff --git a/web-app/django/VIM/apps/instruments/admin.py b/web-app/django/VIM/apps/instruments/admin.py index fa511e3c..bb9dfbde 100644 --- a/web-app/django/VIM/apps/instruments/admin.py +++ b/web-app/django/VIM/apps/instruments/admin.py @@ -1,32 +1,83 @@ from django.contrib import admin -from VIM.apps.instruments.models import Instrument, InstrumentName, Language, AVResource +from VIM.apps.instruments.models import ( + Instrument, + InstrumentName, + InstrumentNameSource, + Source, + Language, + AVResource, +) admin.site.register(Instrument) +admin.site.register(InstrumentNameSource) admin.site.register(Language) admin.site.register(AVResource) +class InstrumentNameSourceInline(admin.TabularInline): + model = InstrumentNameSource + extra = 1 + fields = ( + "source", + "contributor", + ) + readonly_fields = () + + def get_readonly_fields(self, request, obj=None): + """ + Make fields read-only for users in the 'reviewer' group. + """ + if request.user.groups.filter(name="reviewer").exists(): + return ("source", "contributor") + return super().get_readonly_fields(request, obj) + + @admin.register(InstrumentName) class InstrumentNameAdmin(admin.ModelAdmin): - list_filter = ("verification_status", "on_wikidata") # Filter by status + list_filter = ("verification_status", "on_wikidata") search_fields = ( "name", - "source_name", + "sources__name", "instrument__wikidata_id", - ) # Search by name, source name, and instrument wikidata ID + ) + inlines = [InstrumentNameSourceInline] + list_editable = ("verification_status",) # Allow editing of verification_status + list_display = ( + "name", + "instrument", + "language", + "umil_label", + "on_wikidata", + "verification_status", + "all_sources", + ) # Show all sources in list view def get_readonly_fields(self, request, obj=None): """ - Make all fields except 'verification_status' read-only for users in the 'reviewer' group. + Make fields read-only for users in the 'reviewer' group. """ if request.user.groups.filter(name="reviewer").exists(): return ( "instrument", "language", "name", - "source_name", "umil_label", - "contributor", "on_wikidata", ) return super().get_readonly_fields(request, obj) + + def all_sources(self, obj): + """ + Display all associated source names for this instrument name + """ + return ", ".join(obj.sources.all.iterator()) + + all_sources.short_description = "Sources" + + +@admin.register(Source) +class SourceAdmin(admin.ModelAdmin): + list_display = ("name", "is_visible") + list_filter = ("is_visible",) + search_fields = ("name",) + list_editable = ("is_visible",) # Allow editing of is_visible 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 21076ce1..920d454f 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 @@ -8,7 +8,14 @@ from django.contrib.auth import get_user_model from django.core.management.base import BaseCommand from django.db import transaction -from VIM.apps.instruments.models import Instrument, InstrumentName, Language, AVResource +from VIM.apps.instruments.models import ( + Instrument, + InstrumentName, + InstrumentNameSource, + Source, + Language, + AVResource, +) class Command(BaseCommand): @@ -32,6 +39,12 @@ def __init__(self): f"Default contributor {settings.DDMAL_USERNAME} not found in the database." ) + # Set Wikidata as a source to associate with all instrument names imported from Wikidata dumps + self.wikidata_source, _ = Source.objects.get_or_create( + name="Wikidata", + defaults={"is_visible": True}, + ) + def parse_instrument_data( self, instrument_id: str, instrument_data: dict ) -> dict[str, str | dict[str, str]]: @@ -142,16 +155,22 @@ def create_database_objects( ) ) continue - InstrumentName.objects.update_or_create( + name_obj, _ = InstrumentName.objects.update_or_create( instrument=instrument, language=self.language_map[lang], umil_label=True, defaults={ "name": name, - "source_name": "Wikidata", - "contributor": self.default_contributor, - "verification_status": "verified", "on_wikidata": True, + "verification_status": "verified", + }, + ) + + InstrumentNameSource.objects.update_or_create( + instrument_name=name_obj, + source=self.wikidata_source, + defaults={ + "contributor": self.default_contributor, }, ) @@ -166,16 +185,22 @@ def create_database_objects( ) continue for alias in aliases: - InstrumentName.objects.update_or_create( + alias_obj, _ = InstrumentName.objects.update_or_create( instrument=instrument, language=self.language_map[lang], - name=alias, umil_label=False, defaults={ - "source_name": "Wikidata", - "contributor": self.default_contributor, - "verification_status": "verified", + "name": alias, "on_wikidata": True, + "verification_status": "verified", + }, + ) + + InstrumentNameSource.objects.update_or_create( + instrument_name=name_obj, + source=self.wikidata_source, + defaults={ + "contributor": self.default_contributor, }, ) diff --git a/web-app/django/VIM/apps/instruments/migrations/0012_source_remove_instrumentname_contributor_and_more.py b/web-app/django/VIM/apps/instruments/migrations/0012_source_remove_instrumentname_contributor_and_more.py new file mode 100644 index 00000000..f6d63d6c --- /dev/null +++ b/web-app/django/VIM/apps/instruments/migrations/0012_source_remove_instrumentname_contributor_and_more.py @@ -0,0 +1,99 @@ +# Generated by Django 4.2.5 on 2025-12-04 02:52 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("instruments", "0011_language_html_direction"), + ] + + operations = [ + migrations.CreateModel( + name="Source", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "name", + models.CharField( + help_text="Who or what called the instrument this?", + max_length=50, + ), + ), + ( + "is_visible", + models.BooleanField( + default=True, help_text="Should this source be visible?" + ), + ), + ], + ), + migrations.RemoveField( + model_name="instrumentname", + name="contributor", + ), + migrations.RemoveField( + model_name="instrumentname", + name="source_name", + ), + migrations.CreateModel( + name="InstrumentNameSource", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "contributor", + models.ForeignKey( + blank=True, + help_text="Users who contributed this source to this name", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="instrument_name_sources", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "instrument_name", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="source_links", + to="instruments.instrumentname", + ), + ), + ( + "source", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="instruments.source", + ), + ), + ], + ), + migrations.AddField( + model_name="instrumentname", + name="sources", + field=models.ManyToManyField( + related_name="instrument_names", + through="instruments.InstrumentNameSource", + to="instruments.source", + ), + ), + ] diff --git a/web-app/django/VIM/apps/instruments/models/__init__.py b/web-app/django/VIM/apps/instruments/models/__init__.py index 54813b80..36460607 100644 --- a/web-app/django/VIM/apps/instruments/models/__init__.py +++ b/web-app/django/VIM/apps/instruments/models/__init__.py @@ -1,4 +1,6 @@ from VIM.apps.instruments.models.instrument import Instrument from VIM.apps.instruments.models.instrument_name import InstrumentName +from VIM.apps.instruments.models.instrument_name_source import InstrumentNameSource +from VIM.apps.instruments.models.source import Source from VIM.apps.instruments.models.language import Language from VIM.apps.instruments.models.avresource import AVResource diff --git a/web-app/django/VIM/apps/instruments/models/instrument_name.py b/web-app/django/VIM/apps/instruments/models/instrument_name.py index b31af750..0751478a 100644 --- a/web-app/django/VIM/apps/instruments/models/instrument_name.py +++ b/web-app/django/VIM/apps/instruments/models/instrument_name.py @@ -5,9 +5,21 @@ class InstrumentName(models.Model): instrument = models.ForeignKey("Instrument", on_delete=models.CASCADE) language = models.ForeignKey("Language", on_delete=models.PROTECT) name = models.CharField(max_length=100, blank=False) - source_name = models.CharField( - max_length=50, blank=False, help_text="Who or what called the instrument this?" - ) # Stand-in for source data; format TBD + + # An instrument name may be supported by multiple sources, each with its own contributor + sources = models.ManyToManyField( + "Source", through="InstrumentNameSource", related_name="instrument_names" + ) + umil_label = models.BooleanField( + default=False, + help_text="Is this the label for the instrument? If true, it will be used as the main name.", + ) + + on_wikidata = models.BooleanField( + default=False, + help_text="Is this name already on Wikidata?", + ) + verification_status = models.CharField( max_length=50, choices=[ @@ -20,20 +32,6 @@ class InstrumentName(models.Model): default="unverified", help_text="Status of the name entry", ) - umil_label = models.BooleanField( - default=False, - help_text="Is this the label for the instrument? If true, it will be used as the main name.", - ) - contributor = models.ForeignKey( - "auth.User", - null=False, - on_delete=models.PROTECT, - help_text="User who contributed this name", - ) - on_wikidata = models.BooleanField( - default=False, - help_text="Is this name already on Wikidata?", - ) # Custom validation to ensure at most one UMIL label per instrument language class Meta: diff --git a/web-app/django/VIM/apps/instruments/models/instrument_name_source.py b/web-app/django/VIM/apps/instruments/models/instrument_name_source.py new file mode 100644 index 00000000..5f08eee5 --- /dev/null +++ b/web-app/django/VIM/apps/instruments/models/instrument_name_source.py @@ -0,0 +1,20 @@ +from django.db import models + + +class InstrumentNameSource(models.Model): + instrument_name = models.ForeignKey( + "InstrumentName", on_delete=models.CASCADE, related_name="source_links" + ) + source = models.ForeignKey("Source", on_delete=models.CASCADE) + + contributor = models.ForeignKey( + "auth.User", + on_delete=models.SET_NULL, + help_text="Users who contributed this source to this name", + related_name="instrument_name_sources", + blank=True, + null=True, + ) + + def __str__(self): + return f"{self.source.name} for {self.instrument_name.name}" diff --git a/web-app/django/VIM/apps/instruments/models/source.py b/web-app/django/VIM/apps/instruments/models/source.py new file mode 100644 index 00000000..7cda6887 --- /dev/null +++ b/web-app/django/VIM/apps/instruments/models/source.py @@ -0,0 +1,14 @@ +from django.db import models + + +class Source(models.Model): + name = models.CharField( + max_length=50, blank=False, help_text="Who or what called the instrument this?" + ) + + is_visible = models.BooleanField( + default=True, help_text="Should this source be visible?" + ) + + def __str__(self): + return f"{self.name}" 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 942b2429..f8510d35 100644 --- a/web-app/django/VIM/apps/instruments/views/instrument_detail.py +++ b/web-app/django/VIM/apps/instruments/views/instrument_detail.py @@ -16,7 +16,10 @@ def get_context_data(self, **kwargs): # Query the instrument names in all languages instrument_names = ( - context["instrument"].instrumentname_set.all().select_related("language") + context["instrument"] + .instrumentname_set.all() + .select_related("language") + .prefetch_related("sources") ) if self.request.user.is_authenticated: # Show all names for authenticated users 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 74a52731..1d1885fd 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 @@ -5,7 +5,13 @@ from django.views.decorators.http import require_http_methods from django.http import HttpRequest, JsonResponse from django.shortcuts import get_object_or_404 -from VIM.apps.instruments.models import Instrument, Language, InstrumentName +from VIM.apps.instruments.models import ( + Instrument, + InstrumentName, + InstrumentNameSource, + Source, + Language, +) from typing import Any, Dict, List @@ -55,16 +61,27 @@ def add_name(request: HttpRequest) -> JsonResponse: # been assigned to a previous entry entry_labels = {entry["language"]: False for entry in entries} - instrument_names_to_create = [] + # Deduplicate the entries with respect to language, name, and source: A user cannot suggest the same source for a label multiple times + unique_entries = { + (e["language"], e["name"].strip(), e["source"].strip()) for e in entries + } + + # Find which sources already exist in the database + all_source_names = {source for (_, _, source) in unique_entries} + existing_sources_qs = Source.objects.filter(name__in=all_source_names) + existing_sources = {s.name: s for s in existing_sources_qs} - # Keep track of the (name, language_code) enteties for fast detection of duplication in the request - unique_instruments = set() + # Bulk create missing sources + missing_source_names = all_source_names - set(existing_sources.keys()) + sources_to_create = [ + Source(name=src, is_visible=True) for src in missing_source_names + ] + if sources_to_create: + Source.objects.bulk_create(sources_to_create) - for entry in entries: - language_code: str = entry["language"] - name: str = entry["name"] - source: str = entry["source"] + instrument_name_sources_to_create = [] + for language_code, name, source in unique_entries: # Validate that entry info is provided if not name or not source or not language_code: return JsonResponse( @@ -78,23 +95,33 @@ def add_name(request: HttpRequest) -> JsonResponse: # Find language object from language code dictionary language_obj: Language = language.get(language_code) - # Skip the entry if duplicate in batch - entry_key = (name.lower(), language_code) - if entry_key in unique_instruments: - continue - unique_instruments.add(entry_key) - - # Check if the entry exists in UMIL_db - if InstrumentName.objects.filter( + # Check if the name exists in UMIL_db + existing_name = InstrumentName.objects.filter( instrument=instrument, language__wikidata_code=language_code, name__iexact=name, - ).exists(): + ).first() + source_obj, _ = Source.objects.get_or_create(name=source) + + if existing_name: + # Check if the InstrumentNameSource exists with this contributor + insource_exists = InstrumentNameSource.objects.filter( + instrument_name=existing_name, + source=source_obj, + contributor=request.user, + ).exists() + if not insource_exists: + instrument_name_sources_to_create.append( + InstrumentNameSource( + instrument_name=existing_name, + source=source_obj, + contributor=request.user, + ) + ) + # Skip further creation if InstrumentName already exists continue - # Within the entries, check if the language already has a name - # if it does, set umil_label to False - # otherwise, check against the UMILdb + # If name does not exist, create it, track umil_label if entry_labels[language_code]: umil_label = False else: @@ -105,20 +132,24 @@ def add_name(request: HttpRequest) -> JsonResponse: ) entry_labels[language_code] = True # Mark that this language now has a name - # Prepare the InstrumentName object - instrument_names_to_create.append( - InstrumentName( - instrument=instrument, - language=language_obj, - name=name, - source_name=source, - umil_label=umil_label, + instrument_name_obj = InstrumentName.objects.create( + instrument=instrument, + language=language_obj, + name=name, + umil_label=umil_label, + on_wikidata=False, + verification_status="unverified", + ) + instrument_name_sources_to_create.append( + InstrumentNameSource( + instrument_name=instrument_name_obj, + source=source_obj, contributor=request.user, ) ) - # Bulk create all InstrumentName objects - InstrumentName.objects.bulk_create(instrument_names_to_create) + # Bulk create all InstrumentNameSource objects + InstrumentNameSource.objects.bulk_create(instrument_name_sources_to_create) return JsonResponse( { @@ -135,6 +166,7 @@ def delete_name(request: HttpRequest) -> JsonResponse: # Parse the JSON request body data: Dict[str, Any] = json.loads(request.body) name_id: str = data.get("instrument_name_id") + source_name: str = data.get("source_name") # Check if name_id is provided, if not return 400 error if not name_id: @@ -147,10 +179,32 @@ def delete_name(request: HttpRequest) -> JsonResponse: ) instrument_name = get_object_or_404(InstrumentName, id=name_id) - - # If user is a superuser or created the name, allow deletion - if request.user.is_superuser or instrument_name.contributor == request.user: - instrument_name.delete() + source_obj = get_object_or_404(Source, name=source_name) + + # Todo: add logic for superuser deletion + + # If user is a contributor to any linked InstrumentNameSource, allow link deletion + if InstrumentNameSource.objects.filter( + instrument_name=instrument_name, contributor=request.user, source=source_obj + ).exists(): + InstrumentNameSource.objects.filter( + instrument_name=instrument_name, contributor=request.user, source=source_obj + ).delete() + # If no sources are left, delete the instrument name + if instrument_name.source_links.count() == 0: + umil_status = instrument_name.umil_label + instrument_name.delete() + # If the name being deleted is the UMIL label, + # restore UMIL label to another InstrumentName if one exists + if umil_status: + other_names = InstrumentName.objects.filter( + instrument=instrument_name.instrument, + language=instrument_name.language, + ) + next_label = other_names.first() + if next_label: + next_label.umil_label = True + next_label.save() return JsonResponse( { "status": "success", diff --git a/web-app/django/VIM/templates/instruments/detail.html b/web-app/django/VIM/templates/instruments/detail.html index a605595a..77c34ad2 100644 --- a/web-app/django/VIM/templates/instruments/detail.html +++ b/web-app/django/VIM/templates/instruments/detail.html @@ -115,7 +115,10 @@

{{ active_instrument_label.name|title }}

- {{ names.label.source_name }} + + {{ names.label.sources.all.0.name }} + {# TODO: show all visible sources in a separate page #} +
{% if user.is_authenticated %} @@ -141,7 +144,7 @@

{{ active_instrument_label.name|title }}

data-bs-target="#deleteNameModal" data-instrument-language="{{ names.label.language.en_label }}" data-instrument-name="{{ names.label.name }}" - data-instrument-source="{{ names.label.source_name }}" + data-instrument-source="{{ names.label.sources.all.0.name }}" data-instrument-id="{{ names.label.id }}" data-instrument-pk="{{ instrument.pk }}"> Delete @@ -182,7 +185,10 @@

{{ active_instrument_label.name|title }}

Source
- {{ names.label.source_name }} + + {{ names.label.sources.all.0.name }} + {# TODO: show all visible sources in a separate page #} +
{% if user.is_authenticated %} @@ -216,7 +222,7 @@

{{ active_instrument_label.name|title }}

data-bs-target="#deleteNameModal" data-instrument-language="{{ names.label.language.en_label }}" data-instrument-name="{{ names.label.name }}" - data-instrument-source="{{ names.label.source_name }}" + data-instrument-source="{{ names.label.sources.all.0.name }}" data-instrument-id="{{ names.label.id }}" data-instrument-pk="{{ instrument.pk }}"> Delete diff --git a/web-app/frontend/src/instruments/helpers/DeleteNameManager.ts b/web-app/frontend/src/instruments/helpers/DeleteNameManager.ts index 4030f7ab..2b9fa3f3 100644 --- a/web-app/frontend/src/instruments/helpers/DeleteNameManager.ts +++ b/web-app/frontend/src/instruments/helpers/DeleteNameManager.ts @@ -120,6 +120,9 @@ export class DeleteNameManager { }, body: JSON.stringify({ instrument_name_id: this.nameId, + source_name: this.deleteNameModal.querySelector( + '#instrumentSourceInModal', + )?.textContent, }), });