diff --git a/django/cantusdb_project/main_app/permissions.py b/django/cantusdb_project/main_app/permissions.py index bd36e909e..4bbd65d82 100644 --- a/django/cantusdb_project/main_app/permissions.py +++ b/django/cantusdb_project/main_app/permissions.py @@ -1,15 +1,18 @@ +from typing import Optional, Union from django.db.models import Q -from typing import Optional +from django.core.exceptions import PermissionDenied +from django.contrib.auth.models import AnonymousUser from main_app.models import ( Source, Chant, Sequence, ) from users.models import User -from django.core.exceptions import PermissionDenied -def user_can_edit_chants_in_source(user: User, source: Optional[Source]) -> bool: +def user_can_edit_chants_in_source( + user: Union[User, AnonymousUser], source: Optional[Source] +) -> bool: """ Checks if the user can edit Chants in a given Source. Used in ChantDetail, ChantList, ChantCreate, ChantDelete, ChantEdit, @@ -22,16 +25,17 @@ def user_can_edit_chants_in_source(user: User, source: Optional[Source]) -> bool return False source_id = source.id - user_is_assigned_to_source: bool = user.sources_user_can_edit.filter( # noqa + user_is_assigned_to_source = user.sources_user_can_edit.filter( # type: ignore[attr-defined] id=source_id ).exists() - user_is_project_manager: bool = user.groups.filter(name="project manager").exists() - user_is_editor: bool = user.groups.filter(name="editor").exists() - user_is_contributor: bool = user.groups.filter(name="contributor").exists() + user_groups = user.groups.all().values_list("name", flat=True) + user_is_pm = "project manager" in user_groups + user_is_editor = "editor" in user_groups + user_is_contributor = "contributor" in user_groups return ( - user_is_project_manager + user_is_pm or (user_is_editor and user_is_assigned_to_source) or (user_is_editor and source.created_by == user) or (user_is_contributor and user_is_assigned_to_source) @@ -50,19 +54,8 @@ def user_can_proofread_chant(user: User, chant: Chant) -> bool: if user.is_anonymous: return False - source_id = chant.source.id - user_can_proofread_src = user_can_proofread_source(user, chant.source) - - user_is_assigned_to_source: bool = user.sources_user_can_edit.filter( # noqa - id=source_id - ).exists() - - user_is_project_manager: bool = user.groups.filter(name="project manager").exists() - user_is_editor: bool = user.groups.filter(name="editor").exists() - - return user_can_proofread_src and ( - user_is_project_manager or (user_is_editor and user_is_assigned_to_source) - ) + source = chant.source + return user_can_proofread_source(user, source) def user_can_proofread_source(user: User, source: Source) -> bool: @@ -77,14 +70,15 @@ def user_can_proofread_source(user: User, source: Source) -> bool: return False source_id = source.id - user_is_assigned_to_source: bool = user.sources_user_can_edit.filter( + user_is_assigned_to_source: bool = user.sources_user_can_edit.filter( # type: ignore[attr-defined] id=source_id ).exists() - user_is_project_manager: bool = user.groups.filter(name="project manager").exists() - user_is_editor: bool = user.groups.filter(name="editor").exists() + user_groups = user.groups.all().values_list("name", flat=True) + user_is_pm: bool = "project manager" in user_groups + user_is_editor: bool = "editor" in user_groups - return user_is_project_manager or (user_is_editor and user_is_assigned_to_source) + return user_is_pm or (user_is_editor and user_is_assigned_to_source) def user_can_view_source(user: User, source: Source) -> bool: @@ -126,16 +120,17 @@ def user_can_edit_sequences(user: User, sequence: Sequence) -> bool: return False source_id = source.id - user_is_assigned_to_source: bool = user.sources_user_can_edit.filter( # noqa + user_is_assigned_to_source = user.sources_user_can_edit.filter( # type: ignore[attr-defined] id=source_id ).exists() - user_is_project_manager: bool = user.groups.filter(name="project manager").exists() - user_is_editor: bool = user.groups.filter(name="editor").exists() - user_is_contributor: bool = user.groups.filter(name="contributor").exists() + user_groups = user.groups.all().values_list("name", flat=True) + user_is_pm = "project manager" in user_groups + user_is_editor = "editor" in user_groups + user_is_contributor = "contributor" in user_groups return ( - user_is_project_manager + user_is_pm or (user_is_editor and user_is_assigned_to_source) or (user_is_editor and source.created_by == user) or (user_is_contributor and user_is_assigned_to_source) @@ -162,11 +157,14 @@ def user_can_edit_source(user: User, source: Source) -> bool: if user.is_anonymous: return False source_id = source.id - assigned_to_source = user.sources_user_can_edit.filter(id=source_id) # noqa + assigned_to_source = user.sources_user_can_edit.filter( # type: ignore[attr-defined] + id=source_id + ) - is_project_manager: bool = user.groups.filter(name="project manager").exists() - is_editor: bool = user.groups.filter(name="editor").exists() - is_contributor: bool = user.groups.filter(name="contributor").exists() + user_groups = user.groups.all().values_list("name", flat=True) + is_project_manager: bool = "project manager" in user_groups + is_editor: bool = "editor" in user_groups + is_contributor: bool = "contributor" in user_groups return ( is_project_manager @@ -178,8 +176,8 @@ def user_can_edit_source(user: User, source: Source) -> bool: def user_can_view_user_detail(viewing_user: User, user: User) -> bool: """ - Checks if the user can view the user detail pages of regular users in the database or just indexers. - Used in UserDetailView. + Checks if the user can view the user detail pages of regular users in + the database or just indexers. Used in UserDetailView. """ return viewing_user.is_authenticated or user.is_indexer diff --git a/django/cantusdb_project/main_app/templates/browse_chants.html b/django/cantusdb_project/main_app/templates/browse_chants.html index d8d63bb19..8ac640490 100644 --- a/django/cantusdb_project/main_app/templates/browse_chants.html +++ b/django/cantusdb_project/main_app/templates/browse_chants.html @@ -181,8 +181,8 @@

{
diff --git a/django/cantusdb_project/main_app/templates/chant_edit.html b/django/cantusdb_project/main_app/templates/chant_edit.html index 987e398e6..93b006111 100644 --- a/django/cantusdb_project/main_app/templates/chant_edit.html +++ b/django/cantusdb_project/main_app/templates/chant_edit.html @@ -43,14 +43,13 @@ {% endif %} - {% if pk_specified %} - {% if user.is_authenticated %} + {% if chant %}

View | Edit

- {% endif %}
{% csrf_token %} +
@@ -352,28 +351,28 @@
{% else %} -

Full text & Volpiano edit form

+

Chant Edit Form


-
-
1) Select a folio or a feast (in the right block)
-
2) Click "EDIT" next to any chant
-
3) The fulltext and Volpiano fields should appear in this area
-
4) Edit the fields according to the manuscript, following the fulltext guidelines created by Cantus
-
5) Click "SAVE"
-
-
+
    +
  1. Select a folio or a feast (in the right block).
    2. +
  2. Click "EDIT" next to any chant.
    3. +
  3. A form will appear in this area that allows you to edit the chant data.
    4. +
  4. Edit these fields according to the Cantus Database protocols.
    5. +
  5. Click "SAVE".
    6. +
+
{{ source.short_heading }}
-
+
- + Add new chant + + Add new chant
-
+
- + Add new source + + Add new source
{% endif %} @@ -385,13 +384,13 @@

Full text & Volpiano edit form

{{ source.short_heading }}

- {% if source.chant_set.exists %} + {% if source_has_chants %} - {% for folio, feast_id, feast_name in feasts_with_folios %} - + {% for feast_id, feast_name, folio_range in feast_selector_options %} + {% endfor %}
@@ -422,7 +421,7 @@

{ {% comment %} render if the user has selected a specific folio {% endcomment %} {% if feasts_current_folio %} {% for feast, chants in feasts_current_folio %} - Folio: {{ chant.folio }} - Feast: {{ feast.name }} + Folio: {{ folio_query }} - Feast: {{ feast.name }} {% for chant in chants %} @@ -463,7 +462,7 @@

{ {% comment %} render if the user has selected a specific feast {% endcomment %} {% elif folios_current_feast %} {% for folio, chants in folios_current_feast %} - Folio: {{ folio }} - Feast: {{ chant.feast }} + Folio: {{ folio }} - Feast: {{ feast.name }}

{% for chant in chants %} diff --git a/django/cantusdb_project/main_app/templates/source_detail.html b/django/cantusdb_project/main_app/templates/source_detail.html index 981bc290d..c0e939c3b 100644 --- a/django/cantusdb_project/main_app/templates/source_detail.html +++ b/django/cantusdb_project/main_app/templates/source_detail.html @@ -212,8 +212,8 @@

{{ source.short_heading }}

diff --git a/django/cantusdb_project/main_app/tests/test_views/test_chant.py b/django/cantusdb_project/main_app/tests/test_views/test_chant.py index 5cfeb3340..e7f2ebecb 100644 --- a/django/cantusdb_project/main_app/tests/test_views/test_chant.py +++ b/django/cantusdb_project/main_app/tests/test_views/test_chant.py @@ -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 @@ -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", @@ -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): @@ -321,6 +323,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 @@ -3136,3 +3139,96 @@ def test_non_existing_chant(self): chant = make_fake_chant() response = self.client.post(reverse("chant-delete", args=[chant.id + 100])) self.assertEqual(response.status_code, 404) + + +class ChantViewHelpersTest(TestCase): + """ + Tests for the helper functions defined in views.chant + """ + + @classmethod + def setUpTestData(cls): + cls.feasts = [make_fake_feast() for _ in range(4)] + + def test_get_feast_selector_options(self) -> None: + with self.subTest("r/v foliation"): + source = make_fake_source() + feasts = self.feasts + # Create chants for feasts[0] for range 001v, A001v + for folio in ["A001v", "001v"]: + make_fake_chant(source=source, folio=folio, feast=feasts[0]) + # Create chants for feasts[1] for range 001r, 002r + for folio in ["001r", "002r"]: + make_fake_chant(source=source, folio=folio, feast=feasts[1]) + # Create chants for feasts[2] for range 002r-002v, 003v + for folio in ["002r", "002v", "003v"]: + make_fake_chant(source=source, folio=folio, feast=feasts[2]) + # Create a chant on 003r with no feast that should show up in no ranges + make_fake_chant(source=source, folio="003r", feast=None) + feast_selector_options = get_feast_selector_options(source) + expected_result = [ + (feasts[1].id, feasts[1].name, "001r, 002r"), + (feasts[0].id, feasts[0].name, "001v, A001v"), + (feasts[2].id, feasts[2].name, "002r-002v, 003v"), + ] + self.assertEqual(feast_selector_options, expected_result) + with self.subTest("Foliation with numbers only"): + source = make_fake_source() + feasts = self.feasts + # Create chants for feasts[0] for range 002-004 + for folio in ["002", "003", "004"]: + make_fake_chant(source=source, folio=folio, feast=feasts[0]) + # Create chants for feasts[1] for range 001, 003 + for folio in ["001", "003"]: + make_fake_chant(source=source, folio=folio, feast=feasts[1]) + feast_selector_options = get_feast_selector_options(source) + expected_result = [ + (feasts[1].id, feasts[1].name, "001, 003"), + (feasts[0].id, feasts[0].name, "002-004"), + ] + self.assertEqual(feast_selector_options, expected_result) + with self.subTest("Unnumbered folios"): + source = make_fake_source() + feasts = self.feasts + # Create chants for feasts[0] for folios 003v-003w, 004v + for folio in ["003v", "003w", "004v"]: + make_fake_chant(source=source, folio=folio, feast=feasts[0]) + # Create chants for feasts[1] for folios 002r-002x + for folio in ["002r", "002x"]: + make_fake_chant(source=source, folio=folio, feast=feasts[1]) + # Create chants for feasts[2] for folios 003w-004r + for folio in ["003w", "004r"]: + make_fake_chant(source=source, folio=folio, feast=feasts[2]) + # Create chants for feasts[3] for folios 003w-003x + for folio in ["003x", "003w"]: + make_fake_chant(source=source, folio=folio, feast=feasts[3]) + feast_selector_options = get_feast_selector_options(source) + expected_result = [ + (feasts[1].id, feasts[1].name, "002r-002x"), + (feasts[0].id, feasts[0].name, "003v-003w, 004v"), + (feasts[3].id, feasts[3].name, "003w-003x"), + (feasts[2].id, feasts[2].name, "003w-004r"), + ] + self.assertEqual(feast_selector_options, expected_result) + with self.subTest("Unexpected folio numbers"): + # This subTest ensures that unexpected folio numbers (say, + # a something like "00q1r") are added correctly to ranges. + source = make_fake_source() + feasts = self.feasts + # Create chants for feasts[0] with a normal range (001r-002r) + for folio in ["001r", "001v", "002r"]: + make_fake_chant(source=source, folio=folio, feast=feasts[0]) + # Create chants for feasts[1] with an unexpected folio number (00q1r) + # and expected folio numbers + for folio in ["00q1r", "002v", "003r"]: + make_fake_chant(source=source, folio=folio, feast=feasts[1]) + # Create chants for feasts[2] with only unexpected folio numbers + for folio in ["00q2r", "00q3", "X00q3"]: + make_fake_chant(source=source, folio=folio, feast=feasts[2]) + feast_selector_options = get_feast_selector_options(source) + expected_result = [ + (feasts[0].id, feasts[0].name, "001r-002r"), + (feasts[1].id, feasts[1].name, "002v-003r, 00q1r"), + (feasts[2].id, feasts[2].name, "00q2r, 00q3, X00q3"), + ] + self.assertEqual(feast_selector_options, expected_result) diff --git a/django/cantusdb_project/main_app/tests/test_views/test_source.py b/django/cantusdb_project/main_app/tests/test_views/test_source.py index 41180e6ea..011d87af8 100644 --- a/django/cantusdb_project/main_app/tests/test_views/test_source.py +++ b/django/cantusdb_project/main_app/tests/test_views/test_source.py @@ -164,28 +164,6 @@ def test_context_sequence_folios(self): # the folios should be ordered by the "folio" field self.assertEqual(folios.query.order_by, ("folio",)) - def test_context_feasts_with_folios(self): - # create a source and several chants (associated with feasts) in it - source = make_fake_source() - feast_1 = make_fake_feast() - feast_2 = make_fake_feast() - make_fake_chant(source=source, folio="001r", feast=feast_1) - make_fake_chant(source=source, folio="001r", feast=feast_1) - make_fake_chant(source=source, folio="001v", feast=feast_2) - make_fake_chant(source=source, folio="001v", feast=None) - make_fake_chant(source=source, folio="001v", feast=feast_2) - make_fake_chant(source=source, folio="002r", feast=feast_1) - # request the page - response = self.client.get(reverse("source-detail", args=[source.id])) - # context "feasts_with_folios" is a list of tuples - # it records the folios where the feast changes - expected_result = [ - ("001r", feast_1.id, feast_1.name), - ("001v", feast_2.id, feast_2.name), - ("002r", feast_1.id, feast_1.name), - ] - self.assertEqual(response.context["feasts_with_folios"], expected_result) - def test_context_sequences(self): # create a sequence source and several sequences in it source = make_fake_source( @@ -698,27 +676,6 @@ def test_context_folios(self): folios = response.context["folios"] self.assertEqual(list(folios), ["001r", "001v", "002r", "002v"]) - def test_context_feasts_with_folios(self): - cantus_segment = make_fake_segment(id=4063) - source = make_fake_source(segment=cantus_segment) - feast_1 = make_fake_feast() - feast_2 = make_fake_feast() - make_fake_chant(source=source, folio="001r", feast=feast_1) - make_fake_chant(source=source, folio="001r", feast=feast_1) - make_fake_chant(source=source, folio="001v", feast=feast_2) - make_fake_chant(source=source, folio="001v", feast=None) - make_fake_chant(source=source, folio="001v", feast=feast_2) - make_fake_chant(source=source, folio="002r", feast=feast_1) - response = self.client.get(reverse("browse-chants", args=[source.id])) - # context "feasts_with_folios" is a list of tuples - # it records the folios where the feast changes - expected_result = [ - ("001r", feast_1.id, feast_1.name), - ("001v", feast_2.id, feast_2.name), - ("002r", feast_1.id, feast_1.name), - ] - self.assertEqual(response.context["feasts_with_folios"], expected_result) - def test_redirect_with_source_parameter(self): cantus_segment = make_fake_segment(id=4063) source = make_fake_source(segment=cantus_segment) diff --git a/django/cantusdb_project/main_app/views/chant.py b/django/cantusdb_project/main_app/views/chant.py index 7b6e6706e..4d7fb3de8 100644 --- a/django/cantusdb_project/main_app/views/chant.py +++ b/django/cantusdb_project/main_app/views/chant.py @@ -1,13 +1,14 @@ import urllib.parse from collections import Counter, defaultdict -from typing import Optional, Iterator, Any +from typing import Optional, Any, Iterator +import string from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.auth.mixins import UserPassesTestMixin +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 @@ -106,27 +107,122 @@ ) -def get_feast_selector_options(source: Source) -> list[tuple[str, int, str]]: - """Generate folio-feast pairs as options for the feast selector +def split_folio_name(folio: str) -> tuple[str, str, str]: + """ + Splits a folio name into its parts: prefix, number, and suffix. + + If + """ + prefix = folio[0] if folio[0].isalpha() else "" + number = folio.strip(string.ascii_letters) + suffix = folio[-1] if folio[-1].isalpha() else "" + return prefix, number, suffix - Going through all chants in the source, folio by folio, - a new entry (in the form of folio-feast) is added when the feast changes. + +def create_folio_ranges(folios: list[str]) -> str: + """ + Combines a list of folios (in ascending order) + into a single string with ranges. + + Example: + combine_folio_names(['001r', '001v', '002r', '003r', '003v','005v']) + returns '001r-002r, 003r-003v, 005v' + + Note: The resulting ranges may include folios that do not contain + chants with the given feast *if* the feast is on unnumbered folios or + adjacent to unnumbered folios. For example, if a feast appears on folios + 001v and 002r, but there is an unnumbered folio (per CantusDB convention, + called 001w and 001x) between them, the resulting feast range will appear + as '001v-002r'. Similarly, if the feast appears on 001v and 001x, the range + will appear as '001v-001x'. + """ + folio_ranges: list[dict[str, str]] = [{"start": folios[0]}] + most_recent_folio = folios[0] + most_recent_folio_split = split_folio_name(most_recent_folio) + for curr_folio in folios[1:]: + curr_folio_split = split_folio_name(curr_folio) + # Check if the folio number has incremented by one. + # We use this in the second conditional block below but + # check it here to catch if the folio number is not coercible + # to an integer. + try: + folio_num_inc_1 = ( + int(curr_folio_split[1]) == int(most_recent_folio_split[1]) + 1 + ) + except ValueError: + folio_num_inc_1 = False + # If the folio prefix has changed, start a new range + if curr_folio_split[0] != most_recent_folio_split[0]: + folio_ranges[-1]["end"] = most_recent_folio + folio_ranges.append({"start": curr_folio}) + # Next, we add to the current range if one of the following conditions + # is met: + # 1. The folio number does not change. + # 2. The folio number increases by one and either (a) there is no suffix + # or (b) the suffix of the current folio is "r" and the suffix of the previous + # folio was not "r". + elif (curr_folio_split[1] == most_recent_folio_split[1]) or ( + folio_num_inc_1 + and ( + curr_folio_split[2] == "" + or (curr_folio_split[2] == "r" and most_recent_folio_split[2] != "r") + ) + ): + folio_ranges[-1]["end"] = curr_folio + else: + folio_ranges[-1]["end"] = most_recent_folio + folio_ranges.append({"start": curr_folio}) + most_recent_folio = curr_folio + most_recent_folio_split = curr_folio_split + folio_ranges[-1]["end"] = most_recent_folio + # Create strings in the format "start-end" for each range. + # If a folio range only contains one folio, we don't need to display the end folio. + folio_range_strs = [ + ( + f"{folio_range['start']}-{folio_range['end']}" + if folio_range["start"] != folio_range["end"] + else f"{folio_range['start']}" + ) + for folio_range in folio_ranges + ] + return ", ".join(folio_range_strs) + + +def get_feast_selector_options(source: Source) -> list[tuple[int, str, str]]: + """ + Generate a list of feasts in the source to be used in the feast selector + dropdown. Returns a list of tuples in the following format + [ + (feast_id, feast_name, folios), + (feast_id, feast_name, folios), + ... + ] + where folios is a list of folio ranges associated with the feast. Args: - source (Source object): The source that the user is browsing in. + source (Source object): The source for which the dropdown is created. Returns: - list of tuples: A list of folios and Feast objects, to be unpacked in template. + list of tuples: A list of feasts and their associated folios. """ - folios_feasts_iter: Iterator[tuple[Optional[str], int, str]] = ( - source.chant_set.exclude(feast=None) - .select_related("feast", "genre", "service") - .values_list("folio", "feast_id", "feast__name") - .order_by("folio", "c_sequence") + chant_set_w_feasts: QuerySet[Chant, tuple[int, str]] = source.chant_set.exclude( + feast=None + ).values_list("feast_id", "feast__name") + feasts_agg_folios: Iterator[tuple[int, str, list[str]]] = ( + chant_set_w_feasts.annotate(folios=ArrayAgg("folio", distinct=True)) + .order_by("folios") .iterator() ) - deduped_folios_feasts_lists = list(dict.fromkeys(folios_feasts_iter)) - return deduped_folios_feasts_lists + feasts_with_folio_range = [] + for feast_with_folio in feasts_agg_folios: + feasts_with_folio_range.append( + ( + feast_with_folio[0], + feast_with_folio[1], + create_folio_ranges(feast_with_folio[2]), + ) + ) + return feasts_with_folio_range def get_chants_with_feasts( @@ -977,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: @@ -1112,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 @@ -1148,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 @@ -1164,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 @@ -1203,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): diff --git a/django/cantusdb_project/main_app/views/source.py b/django/cantusdb_project/main_app/views/source.py index ab26959e1..4414f9468 100644 --- a/django/cantusdb_project/main_app/views/source.py +++ b/django/cantusdb_project/main_app/views/source.py @@ -70,6 +70,7 @@ class SourceBrowseChantsView(ListView): context_object_name = "chants" template_name = "browse_chants.html" pk_url_kwarg = "source_id" + source: Source def get_queryset(self): """Gather the chants to be displayed. @@ -82,6 +83,7 @@ def get_queryset(self): """ source_id = self.kwargs.get(self.pk_url_kwarg) source = get_object_or_404(Source, id=source_id) + self.source = source display_unpublished = self.request.user.is_authenticated if (source.published is False) and (not display_unpublished): @@ -136,8 +138,7 @@ def get_queryset(self): def get_context_data(self, **kwargs): context: dict = super().get_context_data(**kwargs) - source_id: int = self.kwargs.get(self.pk_url_kwarg) - source: Source = get_object_or_404(Source, id=source_id) + source: Source = self.source if source.segment_id != CANTUS_SEGMENT_ID: # the chant list ("Browse Chants") page should only be visitable # for sources in the CANTUS Database segment, as sources in the Bower @@ -211,11 +212,11 @@ class SourceDetailView(DetailView): def get_queryset(self): return self.model.objects.select_related( - "holding_institution", "segment", "provenance" + "holding_institution", "segment", "provenance", "created_by" ).all() def get_context_data(self, **kwargs): - source = self.get_object() + source = self.object user = self.request.user if not user_can_view_source(user, source):