-
Notifications
You must be signed in to change notification settings - Fork 2
feat: support multiple sources for a single instrument name #443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(source.name for source in obj.sources.all()) | ||
|
|
||
| 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 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", | ||
| ), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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: | ||
|
|
@@ -44,7 +42,3 @@ class Meta: | |
| name="unique_umil_label_per_instrument_language", | ||
| ) | ||
| ] | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not keep this? |
||
| # TODO: add verified_by field to track who verified the name | ||
| def __str__(self): | ||
| return f"{self.name} ({self.language.en_label}) - {self.instrument.wikidata_id}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 name", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this change? |
||
| related_name="instrument_name_sources", | ||
| blank=True, | ||
| null=True, | ||
| ) | ||
|
|
||
| def __str__(self): | ||
| return f"{self.source.name} for {self.instrument_name.name}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,9 +23,11 @@ def get_context_data(self, **kwargs): | |
| context["instrument_names"] = instrument_names.all() | ||
| else: | ||
| # Show only verified names for unauthenticated users | ||
| context["instrument_names"] = instrument_names.filter( | ||
| verification_status="verified" | ||
| ) | ||
| context["instrument_names"] = [ | ||
| name | ||
| for name in instrument_names | ||
| if name.verification_status == "verified" | ||
| ] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did this need to be changed? The QuerySet version is better. |
||
|
|
||
| # Initialize a dictionary to store label and aliases by language | ||
| label_aliases_dict = {} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.