Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update feast filter dropdown options on Source pages & general clean-up of source views #1711

Merged
merged 4 commits into from
Dec 3, 2024
Merged

Update feast filter dropdown options on Source pages & general clean-up of source views #1711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3726063
fix(feast selector options): show folio ranges in feast selector options
dchiller Nov 14, 2024
8d4d6e2
fix(chant edit): fix feast/folio dropdowns; specify chant pk on POST …
dchiller Nov 14, 2024
751e162
perf(SourceDetailView, SourceBrowseChantsView, permissions): reduce q…
dchiller Nov 14, 2024
cc9d2c5
Merge branch 'develop' into i1707-feast-folio-filter
dchiller Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 34 additions & 36 deletions django/cantusdb_project/main_app/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions django/cantusdb_project/main_app/templates/browse_chants.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ <h4><a href="{% url 'source-detail' source.id %}" title="{{ source.heading }}">{

<select name="feast" id="feastSelect" style="width: 200px;"> <!-- style attribute prevents select element from extending beyond left edge of div element -->
<option value="">Select a feast:</option>
{% for folio, feast_id, feast_name in feasts_with_folios %}
<option value="{{ feast_id }}">{{ folio }} - {{ feast_name }}</option>
{% for feast_id, feast_name, folio_range in feasts_with_folios %}
<option value="{{ feast_id }}">{{ feast_name }} ({{ folio_range }})</option>
{% endfor %}
</select>
<br>
Expand Down
43 changes: 21 additions & 22 deletions django/cantusdb_project/main_app/templates/chant_edit.html
View file
Open in desktop
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 All @@ -410,8 +409,8 @@ <h4><a href="{% url 'source-detail' source.id %}" title="{{ source.heading }}">{

<select id="feastSelect" style="width: 200px;"> <!-- style attribute prevents select element from extending beyond left edge of div element -->
<option value="">Select a feast:</option>
{% for folio, feast_id, feast_name in feasts_with_folios %}
<option value="{{ feast_id }}">{{ folio }} - {{ feast_name }}</option>
{% for feast_id, feast_name, folio_range in feast_selector_options %}
<option value="{{ feast_id }}">{{ feast_name }} ({{ folio_range }})</option>
{% endfor %}
</select>
<br>
Expand All @@ -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
4 changes: 2 additions & 2 deletions django/cantusdb_project/main_app/templates/source_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ <h4>{{ source.short_heading }}</h4>

<select id="feastSelect" onchange="jumpToFeast({{ source.id }})" style="width: 200px;"> <!-- style attribute prevents select element from extending beyond left edge of div element -->
<option value="">Select a feast:</option>
{% for folio, feast_id, feast_name in feasts_with_folios %}
<option value="{{ feast_id }}">{{ folio }} - {{ feast_name }}</option>
{% for feast_id, feast_name, folio_range in feasts_with_folios %}
<option value="{{ feast_id }}">{{ feast_name }} ({{ folio_range }})</option>
{% endfor %}
</select>

Expand Down
98 changes: 97 additions & 1 deletion django/cantusdb_project/main_app/tests/test_views/test_chant.py
View file
Open in desktop
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 @@ -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
Expand Down Expand Up @@ -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)
Loading
Loading