Skip to content

Commit

Permalink
refactor(instrument list): reduce duplicate queries on instrument lis…
Browse files Browse the repository at this point in the history
…t view

- Uses existing total instrument count from paginator to get `instrument_num`
context variable
- Uses custom prefetch manager to filter instrument names by active language
on the back-end (simultaneously remove this check from the template rendering)
- preselect the related AVResource thumbnail object
- extract repeated code to determine "active language" into separate function

Also implements minor formatting and typing changes.

Refs: #141
  • Loading branch information
dchiller committed Aug 16, 2024
1 parent ea3a2f7 commit 1a6ff5e
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
<a href="#" class="text-decoration-none">
<h5 class="card-title notranslate">
{% for instrumentname in instrument.instrumentname_set.all %}
{% if instrumentname.language.en_label == active_language.en_label %}
{{ instrumentname.name|title }}
{% endif %}
{{ instrumentname.name|title }}
{% endfor %}
</h5>
</a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
<div class="card-body pb-0 pt-0">
<p class="card-title text-center notranslate ">
{% for instrumentname in instrument.instrumentname_set.all %}
{% if instrumentname.language.en_label == active_language.en_label %}
{{ instrumentname.name|title }}
{% endif %}
{{ instrumentname.name|title }}
{% endfor %}
</p>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
<div class="card-body pb-0 pt-0">
<p class="card-title text-center notranslate">
{% for instrumentname in instrument.instrumentname_set.all %}
{% if instrumentname.language.en_label == active_language.en_label %}
{{ instrumentname.name|title }}
{% endif %}
{{ instrumentname.name|title }}
{% endfor %}
</p>
</div>
Expand Down
56 changes: 41 additions & 15 deletions web-app/django/VIM/apps/instruments/views/instrument_list.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import Any
from django.db.models import Prefetch
from django.db.models.query import QuerySet
from django.views.generic import ListView
from VIM.apps.instruments.models import Instrument, Language
from VIM.apps.instruments.models import Instrument, Language, InstrumentName
import requests


Expand All @@ -15,7 +15,6 @@ class InstrumentList(ListView):

template_name = "instruments/index.html"
context_object_name = "instruments"
model = Instrument

def get_paginate_by(self, queryset) -> int:
pag_by_param: str = self.request.GET.get("paginate_by", "20")
Expand All @@ -25,24 +24,42 @@ def get_paginate_by(self, queryset) -> int:
paginate_by = 20
return paginate_by

def get_active_language_en_label(self) -> str:
"""
Returns the English label of the active language.
The active language is determined by the following order of precedence:
- by the `language` query parameter if present
- by the `active_language_en` session variable if present
- by the default language 'english'
Returns:
str: The English label of the active language
"""
language_en = self.request.GET.get("language")
if language_en:
return language_en
return self.request.session.get("active_language_en", "english")

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context["active_tab"] = "instruments"
context["instrument_num"] = Instrument.objects.count()
context["instrument_num"] = context["paginator"].count
context["languages"] = Language.objects.all()
active_language_en = self.request.session.get("active_language_en", None)
context["active_language"] = (
Language.objects.get(en_label=active_language_en)
if active_language_en
else Language.objects.get(en_label="english") # default in English
)
active_language_en = self.get_active_language_en_label()
context["active_language"] = Language.objects.get(en_label=active_language_en)
active_language_code = context["active_language"].wikidata_code

hbs_facet = self.request.GET.get("hbs_facet", None)
context["hbs_facet"] = hbs_facet

hbs_facets = requests.get(
f"http://solr:8983/solr/virtual-instrument-museum/select?facet.pivot=hbs_prim_cat_s,hbs_prim_cat_label_{active_language_code}_s&facet=true&indent=true&q=*:*&rows=0"
(
"http://solr:8983/solr/virtual-instrument-museum/select?"
f"facet.pivot=hbs_prim_cat_s,hbs_prim_cat_label_{active_language_code}_s"
"&facet=true&indent=true&q=*:*&rows=0"
),
timeout=10,
).json()["facet_counts"]["facet_pivot"][
f"hbs_prim_cat_s,hbs_prim_cat_label_{active_language_code}_s"
]
Expand All @@ -69,10 +86,19 @@ def get(self, request, *args, **kwargs):
request.session["active_language_en"] = language_en
return super().get(request, *args, **kwargs)

def get_queryset(self) -> QuerySet[Any]:
def get_queryset(self) -> QuerySet[Instrument]:
language_en = self.get_active_language_en_label()
instrumentname_prefetch_manager = Prefetch(
"instrumentname_set",
queryset=InstrumentName.objects.filter(language__en_label=language_en),
)
hbs_facet = self.request.GET.get("hbs_facet", None)
if hbs_facet:
return Instrument.objects.filter(
hornbostel_sachs_class__startswith=hbs_facet
return (
Instrument.objects.filter(hornbostel_sachs_class__startswith=hbs_facet)
.select_related("thumbnail")
.prefetch_related(instrumentname_prefetch_manager)
)
return super().get_queryset()
return Instrument.objects.select_related("thumbnail").prefetch_related(
instrumentname_prefetch_manager
)

0 comments on commit 1a6ff5e

Please sign in to comment.