Skip to content

Commit

Permalink
Merge pull request #1666 from dchiller/i1665-source-completeness-fix
Browse files Browse the repository at this point in the history
Fix source_completeness field display and filtering behaviour
  • Loading branch information
dchiller authored Oct 18, 2024
2 parents 97a13c6 + 147c0f4 commit ff69093
Show file tree
Hide file tree
Showing 15 changed files with 87 additions and 54 deletions.
32 changes: 12 additions & 20 deletions django/cantusdb_project/main_app/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@
# ModelForm allows to build a form directly from a model
# see https://docs.djangoproject.com/en/3.0/topics/forms/modelforms/

# Define choices for the Source model's
# complete_inventory BooleanField
COMPLETE_INVENTORY_FORM_CHOICES = (
(True, "Full inventory"),
(False, "Partial inventory"),
)


class NameModelChoiceField(forms.ModelChoiceField):
"""
Expand Down Expand Up @@ -73,7 +80,6 @@ def label_from_instance(self, obj):
widget = CheckboxSelectMultiple()



class CantusDBLatinField(forms.CharField):
"""
A custom CharField for chant text fields. Validates that the text
Expand Down Expand Up @@ -107,6 +113,7 @@ def validate(self, value):
except ValueError as exc:
raise forms.ValidationError("Invalid characters in text.") from exc


class StyledChoiceField(forms.ChoiceField):
"""
A custom ChoiceField that uses the custom SelectWidget defined in widgets.py
Expand All @@ -116,7 +123,6 @@ class StyledChoiceField(forms.ChoiceField):
widget = SelectWidget()



class ChantCreateForm(forms.ModelForm):
class Meta:
model = Chant
Expand Down Expand Up @@ -314,10 +320,8 @@ class Meta:
required=False,
)

TRUE_FALSE_CHOICES_INVEN = ((True, "Complete"), (False, "Incomplete"))

complete_inventory = StyledChoiceField(
choices=TRUE_FALSE_CHOICES_INVEN, required=False
choices=COMPLETE_INVENTORY_FORM_CHOICES, required=False
)


Expand Down Expand Up @@ -509,11 +513,9 @@ class Meta:
required=False,
)

CHOICES_COMPLETE_INV = (
(True, "complete inventory"),
(False, "partial inventory"),
complete_inventory = StyledChoiceField(
choices=COMPLETE_INVENTORY_FORM_CHOICES, required=False
)
complete_inventory = StyledChoiceField(choices=CHOICES_COMPLETE_INV, required=False)


class SequenceEditForm(forms.ModelForm):
Expand Down Expand Up @@ -763,15 +765,13 @@ class Meta:
# help_text="RISM-style siglum + Shelf-mark (e.g. GB-Ob 202).",
# )


shelfmark = forms.CharField(
required=True,
widget=TextInputWidget,
)

name = forms.CharField(required=False, widget=TextInputWidget)


holding_institution = forms.ModelChoiceField(
queryset=Institution.objects.all().order_by("city", "name"),
required=False,
Expand All @@ -781,12 +781,6 @@ class Meta:
queryset=Provenance.objects.all().order_by("name"),
required=False,
)
TRUE_FALSE_CHOICES_SOURCE = (
(True, "Full source"),
(False, "Fragment or Fragmented"),
)

full_source = forms.ChoiceField(choices=TRUE_FALSE_CHOICES_SOURCE, required=False)

century = forms.ModelMultipleChoiceField(
queryset=Century.objects.all().order_by("name"),
Expand Down Expand Up @@ -841,10 +835,8 @@ class Meta:
widget=FilteredSelectMultiple(verbose_name="other editors", is_stacked=False),
)

TRUE_FALSE_CHOICES_INVEN = ((True, "Complete"), (False, "Incomplete"))

complete_inventory = forms.ChoiceField(
choices=TRUE_FALSE_CHOICES_INVEN, required=False
choices=COMPLETE_INVENTORY_FORM_CHOICES, required=False
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"HR": "Croatia",
"I": "Italy",
"IRL": "Ireland",
"N": "Norway",
"NL": "Netherlands",
"NZ": "New Zealand",
"P": "Portugal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Command(BaseCommand):
def handle(self, *args, **options):
sources = Source.objects.all()
for source in sources:
if source.full_source:
if source.full_source or source.full_source is None:
source.source_completeness = (
source.SourceCompletenessChoices.FULL_SOURCE
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Generated by Django 4.2.14 on 2024-10-15 14:37

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("main_app", "0031_source_name_source_production_method_and_more"),
]

operations = [
migrations.AlterField(
model_name="source",
name="source_completeness",
field=models.IntegerField(
choices=[
(1, "Complete source"),
(2, "Fragment"),
(3, "Reconstruction"),
],
default=1,
verbose_name="Complete Source/Fragment",
),
),
]
6 changes: 3 additions & 3 deletions django/cantusdb_project/main_app/models/source.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ class Source(BaseModel):
)

class SourceCompletenessChoices(models.IntegerChoices):
FULL_SOURCE = 1, "Full source"
FRAGMENT = 2, "Fragment/Fragmented"
FULL_SOURCE = 1, "Complete source"
FRAGMENT = 2, "Fragment"
RECONSTRUCTION = 3, "Reconstruction"

source_completeness = models.IntegerField(
choices=SourceCompletenessChoices.choices,
default=SourceCompletenessChoices.FULL_SOURCE,
verbose_name="Full Source/Fragment",
verbose_name="Complete Source/Fragment",
)

full_source = models.BooleanField(blank=True, null=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{% include "global_search_bar.html" %}
</object>
<h3>{{ institution.name }} {% if institution.siglum %}({{ institution.siglum }}){% endif %}</h3>
<h4>{{ institution.city }}, {{ institution.country }}</h4>
<h4>{% if institution.city %}{{ institution.city }}{% else %}[No City]{% endif %}, {{ institution.country }}</h4>
{% if institution_authorities %}
<hr />
<div class="row">
Expand All @@ -29,14 +29,14 @@ <h5>Cantus Database</h5>
<table class="table table-bordered table-sm small">
<thead>
<tr>
<th>Shelfmark</th>
<th>Sources</th>
</tr>
</thead>
<tbody>
{% for source in cantus_sources %}
<tr>
<td>
<a href="{% url "source-detail" source.id %}"><b>{{ source.shelfmark }}</b></a>
<a href="{% url "source-detail" source.id %}"><b>{{ source.shelfmark }}{% if source.name %} ("{{ source.name }}"){% endif %}</b></a>
</td>
</tr>
{% endfor %}
Expand All @@ -47,18 +47,18 @@ <h5>Cantus Database</h5>

{% if num_bower_sources > 0 %}
<div class="col">
<h5>Bower Sequence Database</h5>
<h5>Clavis Sequentiarum (Sequence Database by Calvin Bower)</h5>
<table class="table table-bordered table-sm small">
<thead>
<tr>
<th>Shelfmark</th>
<th>Sources</th>
</tr>
</thead>
<tbody>
{% for source in bower_sources %}
<tr>
<td>
<a href="{% url "source-detail" source.id %}"><b>{{ source.shelfmark }}</b></a>
<a href="{% url "source-detail" source.id %}"><b>{{ source.shelfmark }}{% if source.name %} ("{{ source.name }}"){% endif %}</b></a>
</td>
</tr>
{% endfor %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ <h3>Institutions</h3>
{{ institution.country }}
</td>
<td>
{{ institution.city }}
{% if institution.city %}
{{ institution.city }}
{% else %}
[No City]
{% endif %}
</td>
<td class="text-wrap">
<a href="{% url "institution-detail" institution.id %}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ <h3>Create Source</h3>
<div class="form-row">
<div class="form-group m-1 col-lg-2">
<label for="{{ form.complete_inventory.id_for_label }}">
<small>Complete / partial inventory:</small>
<small>Full / partial inventory:</small>
</label>
{{ form.complete_inventory }}
</div>
Expand Down
6 changes: 3 additions & 3 deletions django/cantusdb_project/main_app/templates/source_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ <h3>{{ source.heading }}</h3>
{% endif %}

{% if source.complete_inventory is not None %}
<dt>Complete/Partial Inventory</dt>
<dd>{{ source.complete_inventory|yesno:"Complete Inventory,Partial Inventory" }}</dd>
<dt>Full/Partial Inventory</dt>
<dd>{{ source.complete_inventory|yesno:"Full Inventory,Partial Inventory" }}</dd>
{% endif %}

<dt>Full Source/Fragment</dt>
<dt>Complete Source/Fragment</dt>
<dd>{{ source.get_source_completeness_display }}</dd>

{% if user.is_authenticated %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ <h3>
<div class="form-row mb-3">
<div class="form-group m-1 col-lg-5">
<label for="{{ form.complete_inventory.id_for_label }}">
Complete / partial inventory:
Full / partial inventory:
</label>
{{ form.complete_inventory }}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ <h3>Browse Sources</h3>
</select>
</div>
<div class="form-group m-1 col-lg">
<label for="fullSourceFilter"><small>Full source/Fragment</small></label>
<label for="fullSourceFilter"><small>Complete Source/Fragment</small></label>
<select id="fullSourceFilter" name="fullSource" class="form-control custom-select custom-select-sm">
<option value="">- Any -</option>
<option value="true">Complete source</option>
Expand Down
4 changes: 2 additions & 2 deletions django/cantusdb_project/main_app/tests/make_fakes.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ def make_fake_source(
summary: Optional[str] = None,
provenance: Optional[Provenance] = None,
century: Optional[Century] = None,
full_source: Optional[bool] = True,
source_completeness: int = Source.SourceCompletenessChoices.FULL_SOURCE,
indexing_notes: Optional[str] = None,
) -> Source:
"""Generates a fake Source object."""
Expand Down Expand Up @@ -447,7 +447,7 @@ def make_fake_source(
summary=summary,
provenance=provenance,
# century: ManyToManyField, must be set below
full_source=full_source,
source_completeness=source_completeness,
indexing_notes=indexing_notes,
provenance_notes=faker.sentence(),
date=faker.sentence(nb_words=3),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,24 @@ class TestPopulateSourceCompleteness(TestCase):
def test_populate_source_completeness(self):
# make a few "Full Source" sources
for _ in range(5):
make_fake_source(full_source=True)
source = make_fake_source()
source.full_source = True
source.save()
# make a few "Fragment" sources
for _ in range(3):
make_fake_source(full_source=False)
source = make_fake_source()
source.full_source = False
source.save()
# make a few "Full Source" sources with full_source=None
for _ in range(2):
source = make_fake_source()
source.full_source = None
source.save()
# run the command
call_command("populate_source_completeness")
sources = Source.objects.all()
for source in sources:
if source.full_source:
if source.full_source or source.full_source is None:
self.assertEqual(
source.source_completeness,
source.SourceCompletenessChoices.FULL_SOURCE,
Expand Down
16 changes: 7 additions & 9 deletions django/cantusdb_project/main_app/tests/test_views/test_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -846,44 +846,42 @@ def test_filter_by_century(self):

def test_filter_by_full_source(self):
full_source = make_fake_source(
full_source=True,
source_completeness=Source.SourceCompletenessChoices.FULL_SOURCE,
published=True,
shelfmark="full source",
)
fragment = make_fake_source(
full_source=False,
source_completeness=Source.SourceCompletenessChoices.FRAGMENT,
published=True,
shelfmark="fragment",
)
unknown = make_fake_source(
full_source=None,
reconstruction = make_fake_source(
source_completeness=Source.SourceCompletenessChoices.RECONSTRUCTION,
published=True,
shelfmark="full_source field is empty",
)

# display full sources
response = self.client.get(reverse("source-list"), {"fullSource": "true"})
sources = response.context["sources"]
# full_source and unknown_source should be in the list, fragment should not
self.assertIn(full_source, sources)
self.assertNotIn(fragment, sources)
self.assertIn(unknown, sources)
self.assertNotIn(reconstruction, sources)

# display fragments
response = self.client.get(reverse("source-list"), {"fullSource": "false"})
sources = response.context["sources"]
# fragment should be in the list, full_source and unknown_source should not
self.assertNotIn(full_source, sources)
self.assertIn(fragment, sources)
self.assertNotIn(unknown, sources)
self.assertNotIn(reconstruction, sources)

# display all sources
response = self.client.get(reverse("source-list"))
sources = response.context["sources"]
# all three should be in the list
self.assertIn(full_source, sources)
self.assertIn(fragment, sources)
self.assertIn(unknown, sources)
self.assertIn(reconstruction, sources)

def test_search_by_title(self):
"""The "general search" field searches in `title`, `shelfmark`, `description`, and `summary`"""
Expand Down
9 changes: 6 additions & 3 deletions django/cantusdb_project/main_app/views/source.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,13 @@ def get_queryset(self) -> QuerySet[Source]:
q_obj_filter &= Q(segment__id=int(segment_id))
if (full_source_str := self.request.GET.get("fullSource")) in ["true", "false"]:
if full_source_str == "true":
full_source_q = Q(full_source=True) | Q(full_source=None)
q_obj_filter &= full_source_q
q_obj_filter &= Q(
source_completeness=Source.SourceCompletenessChoices.FULL_SOURCE
)
else:
q_obj_filter &= Q(full_source=False)
q_obj_filter &= Q(
source_completeness=Source.SourceCompletenessChoices.FRAGMENT
)

if general_str := self.request.GET.get("general"):
# Strip spaces at the beginning and end. Then make list of terms split on spaces
Expand Down

0 comments on commit ff69093

Please sign in to comment.