Skip to content

Commit

Permalink
fix(chant edit): fix feast/folio dropdowns; specify chant pk on POST …
Browse files Browse the repository at this point in the history 
…request

Fix behaviour of feast and folio dropdown selectors on Chant Edit page to show chants on a specific folio or for a specific feast so that the selector remains on the selected option. Fix blank headers for filtered lists of chants on Chant Edit page.

Modify instructions on the Chant Edit page when no chant is selected (these has previously suggested that only full texts and volpiano was editable via that page; this has been corrected to note that all chant fields are editable).

Modify SourceEditChantsView `get_object` method so that the edited chant is always set by pk rather than recency of creation. Previously, this had been effectively true when changes were made through the form, as Django parsed the URL parameter for chant pk into the request.GET queryset; however, since URL parameters cannot currently be set with the Django test client, tests of this views were relying on the recency of chant feature. Now tests and the live form rely on the same logic (the presence of a `pk` option in the form data itself).

Reduce extraneous querysets in SourceEditChantsView.

Refs: #1707, #1708
  • Loading branch information
@dchiller
dchiller committed Dec 2, 2024
1 parent 3726063 commit 8d4d6e2
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 127 deletions.
39 changes: 19 additions & 20 deletions django/cantusdb_project/main_app/templates/chant_edit.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,13 @@
</div>
{% endif %}

{% if pk_specified %}
{% if user.is_authenticated %}
{% if chant %}
<p>
<a href="{% url 'chant-detail' chant.id %}">View</a> | Edit
</p>
{% endif %}
<form method="post" style="line-height: normal">{% csrf_token %}
<input type="hidden" name="referrer" value="{{ request.META.HTTP_REFERER }}">
<input type="hidden" name="pk" value="{{ chant.id }}">

<div class="form-row">
<div class="form-group m-1 col-lg-2">
Expand Down Expand Up @@ -352,28 +351,28 @@
</div>
</form>
{% else %}
<h3>Full text &amp; Volpiano edit form</h3>
<h3>Chant Edit Form</h3>
<br>
<dl>
<dd><small>1) Select a <b>folio</b> or a <b>feast</b> (in the right block)</small></dd>
<dd><small>2) Click <b>"EDIT"</b> next to any chant</small></dd>
<dd><small>3) The <b>fulltext</b> and <b>Volpiano</b> fields should appear in this area</small></dd>
<dd><small>4) Edit the fields <b>according to the manuscript, following the fulltext guidelines created by Cantus</b></small></dd>
<dd><small>5) Click <b>"SAVE"</b></small></dd>
</dl>
<div style="margin-top:5px;">
<ol>
<li class="small">Select a <b>folio</b> or a <b>feast</b> (in the right block).</li>
<li class="small">Click <b>"EDIT"</b> next to any chant.</li>
<li class="small">A form will appear in this area that allows you to edit the chant data.</li>
<li class="small">Edit these fields according to the Cantus Database protocols.</li>
<li class="small">Click <b>"SAVE"</b>.</li>
</ol>
<div class="mt-1">
<a href="{% url 'source-detail' source.id %}" title="{{ source.heading }}">
{{ source.short_heading }}
</a>
</div>
<div style="margin-top:5px;">
<div class="mt-1">
<a href="{% url "chant-create" source.pk %}">
<small><b>&plus; Add new chant</b></small>
<small><b>+ Add new chant</b></small>
</a>
</div>
<div style="margin-top:5px;">
<div class="mt-1">
<a href="{% url "source-create" %}">
<small><b>&plus; Add new source</b></small>
<small><b>+ Add new source</b></small>
</a>
</div>
{% endif %}
Expand All @@ -385,13 +384,13 @@ <h3>Full text &amp; Volpiano edit form</h3>
<h4><a href="{% url 'source-detail' source.id %}" title="{{ source.heading }}">{{ source.short_heading }}</a></h4>
</div>
<div class="card-body">
{% if source.chant_set.exists %}
{% if source_has_chants %}
<small>
<!--a small selector of all folios of this source-->
<select id="folioSelect" class="w-30">
<option value="">Select a folio:</option>
{% for folio in folios %}
{% if folio == initial_GET_folio %}
{% if folio == folio_query %}
<option value="{{ folio }}" selected>{{ folio }}</option>
{% else %}
<option value="{{ folio }}">{{ folio }}</option>
Expand Down Expand Up @@ -422,7 +421,7 @@ <h4><a href="{% url 'source-detail' source.id %}" title="{{ source.heading }}">{
{% comment %} render if the user has selected a specific folio {% endcomment %}
{% if feasts_current_folio %}
{% for feast, chants in feasts_current_folio %}
<small>Folio: <b>{{ chant.folio }}</b> - Feast: <b title="{{ chant.feast.description }}">{{ feast.name }}</b></small>
<small>Folio: <b>{{ folio_query }}</b> - Feast: <b title="{{ chant.feast.description }}">{{ feast.name }}</b></small>
<table class="table table-sm small table-bordered">
{% for chant in chants %}
<tr>
Expand Down Expand Up @@ -463,7 +462,7 @@ <h4><a href="{% url 'source-detail' source.id %}" title="{{ source.heading }}">{
{% comment %} render if the user has selected a specific feast {% endcomment %}
{% elif folios_current_feast %}
{% for folio, chants in folios_current_feast %}
<small>Folio: <b>{{ folio }}</b> - Feast: <b title="{{ chant.feast.description }}">{{ chant.feast }}</b></small>
<small>Folio: <b>{{ folio }}</b> - Feast: <b title="{{ feast.description }}">{{ feast.name }}</b></small>
<table class="table table-sm small table-bordered">
{% for chant in chants %}
<tr>
Expand Down
5 changes: 4 additions & 1 deletion django/cantusdb_project/main_app/tests/test_views/test_chant.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ def test_volpiano_signal(self):
reverse("source-edit-chants", args=[source.id]),
{
"manuscript_full_text_std_spelling": "ut queant lactose",
"pk": chant_1.id,
"folio": "001r",
"c_sequence": "1",
# liquescents, to be converted to lowercase
Expand All @@ -251,7 +252,7 @@ def test_volpiano_signal(self):
self.assertEqual(chant_1.volpiano, "9abcdefg)A-B1C2D3E4F5G67?. yiz")
self.assertEqual(chant_1.volpiano_notes, "9abcdefg9abcdefg")

make_fake_chant(
chant_2 = make_fake_chant(
manuscript_full_text_std_spelling="resonare foobaz",
source=source,
folio="001r",
Expand All @@ -266,6 +267,7 @@ def test_volpiano_signal(self):
"folio": "001r",
"c_sequence": "2",
"volpiano": "abacadaeafagahaja",
"pk": chant_2.id,
},
)
with patch("requests.get", mock_requests_get):
Expand Down Expand Up @@ -319,6 +321,7 @@ def test_proofread_chant(self):
"folio": folio,
"c_sequence": c_sequence,
"manuscript_full_text_std_spelling": ms_std,
"pk": chant.id,
},
)
self.assertEqual(response.status_code, 302) # 302 Found
Expand Down
156 changes: 50 additions & 106 deletions django/cantusdb_project/main_app/views/chant.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from django.contrib.postgres.aggregates import ArrayAgg
from django.core.exceptions import PermissionDenied
from django.db.models import Q, QuerySet
from django.forms import BaseModelForm
from django.http import Http404, HttpResponse
from django.shortcuts import get_object_or_404
from django.urls import reverse
Expand Down Expand Up @@ -1074,129 +1073,85 @@ class SourceEditChantsView(LoginRequiredMixin, UserPassesTestMixin, UpdateView):
model = Chant
form_class = ChantEditForm
pk_url_kwarg = "source_id"
source: Source
source_has_chants: bool

def test_func(self):
def test_func(self) -> bool:
user = self.request.user
source_id = self.kwargs.get(self.pk_url_kwarg)
source = get_object_or_404(Source, id=source_id)
self.source = get_object_or_404(Source, id=source_id)

return user_can_edit_chants_in_source(user, source)
return user_can_edit_chants_in_source(user, self.source)

def get_queryset(self):
def get_queryset(self) -> QuerySet[Chant]:
"""
When a user visits the edit-chant page for a certain Source,
there are 2 dropdowns on the right side of the page: one for folio, and the other for feast.
When either a folio or a feast is selected, a list of Chants in the selected folio/feast will
be rendered.
Returns:
a QuerySet of Chants in the Source, filtered by the optional search parameters.
Note: the first folio is selected by default.
a QuerySet of Chants in the Source, with associated feast,
genre, and service objects selected.
"""

# when arriving at this page, the url must have a source specified
source_id = self.kwargs.get(self.pk_url_kwarg)
source = Source.objects.get(id=source_id)

# optional search params
feast_id = self.request.GET.get("feast")
folio = self.request.GET.get("folio")
source = self.source

# get all chants in the specified source
chants = source.chant_set.select_related(
"feast", "service", "genre", "source__holding_institution"
)
if not source.chant_set.exists():
# return empty queryset
return chants.all()
# filter the chants with optional search params
if feast_id:
chants = chants.filter(feast__id=feast_id)
elif folio:
chants = chants.filter(folio=folio)
# if none of the optional search params are specified, the first folio in the
# source is selected by default
else:
folios = chants.values_list("folio", flat=True).distinct().order_by("folio")
if not folios:
# if the source has no chants (conceivable), or if it has chants but
# none of them have folios specified (we don't really expect this to happen)
raise Http404
initial_folio = folios[0]
chants = chants.filter(folio=initial_folio)
chants = source.chant_set.select_related("feast", "service", "genre")
self.queryset = chants
return self.queryset

def get_object(self, **kwargs):
def get_object(self, queryset=None) -> Optional[Chant]:
"""
If the Source has no Chant, an Http404 is raised.
This is because there would be no Chant for the UpdateView to handle.
Returns:
the Chant that we wish to edit (specified by the Chant's pk)
the Chant that we wish to edit (specified by the Chant's pk).
If no pk is specified or if no chants exist in the source,
None is returned.
"""
queryset = self.get_queryset()
if queryset.count() == 0:
return None

pk = self.request.GET.get("pk")
# if a pk is not specified, this means that the user has not yet selected a Chant to edit
# thus, we will not render the update form
# instead, we will render the instructions page
if self.request.method == "GET":
pk = self.request.GET.get("pk")
elif self.request.method == "POST":
pk = self.request.POST.get("pk")
else:
pk = None

self.source_has_chants = queryset.exists()
if not pk:
pk = queryset.latest("date_created").pk
queryset = queryset.filter(pk=pk)
return queryset.get()
return None
if not self.source_has_chants:
return None
return queryset.get(pk=pk)

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
source_id = self.kwargs.get(self.pk_url_kwarg)
source = Source.objects.get(id=source_id)
context["source"] = source

chants_in_source = source.chant_set.select_related(
"feast", "genre", "service", "source__holding_institution"
)
context["source"] = self.source

# the following code block is sort of obsolete because if there is no Chant
# in the Source, a 404 will be raised
if not chants_in_source.exists():
# these are needed in the selectors and hyperlinks on the right side of the page
# if there's no chant in the source, there should be no options in those selectors
context["folios"] = None
context["feasts_with_folios"] = None
context["previous_folio"] = None
context["next_folio"] = None
chants_in_source = self.queryset
context["source_has_chants"] = self.source_has_chants
if not context["source_has_chants"]:
return context

# generate options for the folio selector on the right side of the page
# generate options for the selectors on the right side of the page
folios = (
chants_in_source.values_list("folio", flat=True)
.distinct()
.order_by("folio")
)
context["folios"] = folios
# the options for the feast selector on the right, same as the source detail page
context["feasts_with_folios"] = get_feast_selector_options(source)
context["feast_selector_options"] = get_feast_selector_options(self.source)

if self.request.GET.get("feast"):
# generate the list of chants to display in the lower right sidebar card
# this card displays chants in the source filtered by the folio or feast selector
# if no feast or folio is selected, defaults to the chants on the first folio
if feast_param := self.request.GET.get("feast"):
# if there is a "feast" query parameter, it means the user has chosen a specific feast
# need to render a list of chants, grouped and ordered by folio and within each group,
# ordered by c_sequence
context["folios_current_feast"] = get_chants_with_folios(self.queryset)
# ordered by c_sequence. We get a Feast object in order to display some additional
# feast information in that list of chants.
context["feast"] = Feast.objects.get(id=feast_param)
context["folios_current_feast"] = get_chants_with_folios(
self.queryset.filter(feast_id=feast_param)
)
else:
# the user has selected a folio, or,
# they have just navigated to the edit-chant page (where the first folio gets
# selected by default)
if self.request.GET.get("folio"):
# if browsing chants on a specific folio
folio = self.request.GET.get("folio")
else:
folio = folios[0]
# will be used in the template to pre-select the first folio in the drop-down
context["initial_GET_folio"] = folio
folio = self.request.GET.get("folio") or folios[0]
context["folio_query"] = folio
try:
index = list(folios).index(folio)
except ValueError:
Expand All @@ -1209,19 +1164,12 @@ def get_context_data(self, **kwargs):
# if there is a "folio" query parameter, it means the user has chosen a specific folio
# need to render a list of chants, ordered by c_sequence and grouped by feast
context["feasts_current_folio"] = get_chants_with_feasts(
self.queryset.select_related(
"feast", "genre", "service", "source__holding_institution"
).order_by("c_sequence")
self.queryset.filter(folio=folio).order_by("c_sequence")
)

# this boolean lets us decide whether to show the user the instructions or the editing form
# if the pk hasn't been specified, a user hasn't selected a specific chant they want to edit
# if so, we should display the instructions
pk = self.request.GET.get("pk")
pk_specified = bool(pk)
context["pk_specified"] = pk_specified

chant = self.get_object()
chant = self.object
if not chant:
return context
if chant.volpiano:
has_syl_text = bool(chant.manuscript_syllabized_full_text)
# Note: the second value returned is a flag indicating whether the alignment process
Expand All @@ -1245,8 +1193,6 @@ def get_context_data(self, **kwargs):
# in case the chant has no manuscript_full_text_std_spelling, we check Cantus Index
# for the expected text for chants with the same Cantus ID, and pass it to the context
# to suggest it to the user
if not chant:
return context
cantus_id = chant.cantus_id
if not cantus_id:
return context
Expand All @@ -1261,7 +1207,6 @@ def form_valid(self, form):

user: User = self.request.user
chant: Chant = form.instance
proofreaders = []

if not user_can_proofread_chant(user, chant):
# Preserve the original values for proofreader-specific fields
Expand Down Expand Up @@ -1300,9 +1245,8 @@ def get_success_url(self):
next_url = self.request.GET.get("ref")
if next_url:
return self.request.POST.get("referrer")
else:
# ref not found, stay on the same page after save
return self.request.get_full_path()
# ref not found, stay on the same page after save
return self.request.get_full_path()


class ChantEditSyllabificationView(LoginRequiredMixin, UserPassesTestMixin, UpdateView):
Expand Down

0 comments on commit 8d4d6e2

Please sign in to comment.