UI bug triage - Address bugs and revamp date range picker

.gitignore

@@ -164,6 +164,18 @@ apps/static/css/project.css
 apps/static/js/main.js
 apps/static/js/main.js.map
 
+# django-sass-processor compiles these from the adjacent .scss at request time
+# (or via `manage.py compilescss` for production). A committed .css here would
+# shadow that compilation, since FileSystemFinder is checked before CssFinder.
+apps/static/css/readux.css
+apps/static/css/components/collection.css
+apps/static/css/components/flatpage.css
+apps/static/css/components/login.css
+apps/static/css/components/menu-inverse.css
+apps/static/css/components/reader.css
+apps/static/css/components/search.css
+apps/static/css/components/user-form.css
+
 snippets/*
 
 

apps/iiif/manifests/documents.py

@@ -9,6 +9,9 @@
 from django_elasticsearch_dsl import Document, fields
 from django_elasticsearch_dsl.registries import registry
 
+from edtf import parse_edtf
+from edtf.convert import struct_time_to_jd
+from edtf.natlang import text_to_edtf
 from elasticsearch_dsl import MetaField, Keyword, analyzer
 from unidecode import unidecode
 
@@ -24,6 +27,32 @@
 )
 
 
+def _published_date_fallback_jd(instance, bound):
+    """date_earliest/date_latest are derived from published_date_edtf, a
+    separate field cataloguers are expected to fill in alongside the
+    display-only published_date. In practice a lot of records only ever get
+    published_date set, which silently excludes them from date-published
+    filtering/sorting and miscounts them as "no date" even though they show
+    a real date on screen. Fall back to parsing published_date itself so the
+    search filter reflects what's actually displayed.
+    """
+    if not instance.published_date:
+        return None
+    try:
+        edtf_string = text_to_edtf(instance.published_date)
+        if not edtf_string:
+            return None
+        edtf_obj = parse_edtf(edtf_string, fail_silently=True)
+        if not edtf_obj:
+            return None
+        struct_time = edtf_obj.lower_fuzzy() if bound == "lower" else edtf_obj.upper_fuzzy()
+        return struct_time_to_jd(struct_time)
+    except Exception:  # pylint: disable=broad-except
+        # published_date is free-text catalog data; malformed/unparseable
+        # values should degrade to "no fallback date", not break indexing.
+        return None
+
+
 @registry.register_document
 class ManifestDocument(Document):
     """Elasticsearch Document class for IIIF Manifest"""
@@ -124,6 +153,20 @@ def prepare_has_pdf(self, instance):
         """convert pdf field into boolean"""
         return bool(instance.pdf)
 
+    def prepare_date_earliest(self, instance):
+        """Fall back to parsing the display-only published_date when
+        date_earliest is unset (see _published_date_fallback_jd)"""
+        if instance.date_earliest is not None:
+            return instance.date_earliest
+        return _published_date_fallback_jd(instance, "lower")
+
+    def prepare_date_latest(self, instance):
+        """Fall back to parsing the display-only published_date when
+        date_latest is unset (see _published_date_fallback_jd)"""
+        if instance.date_latest is not None:
+            return instance.date_latest
+        return _published_date_fallback_jd(instance, "upper")
+
     def prepare_label_alphabetical(self, instance):
         """get the first 64 chars of a label, just for sorting purposes"""
         if instance.label:

apps/iiif/manifests/tests/test_documents.py

@@ -94,6 +94,42 @@ def test_prepare_summary(self):
         manifest.summary = "<p><strong>Has</strong> HTML tags</p>"
         assert self.doc.prepare_summary(instance=manifest) == "Has HTML tags"
 
+    def test_prepare_date_earliest_and_latest_use_model_value_when_set(self):
+        """Should use the model's own date_earliest/date_latest when present,
+        not the published_date fallback"""
+        manifest = ManifestFactory.create(published_date_edtf="2022-04-14")
+        manifest.refresh_from_db()
+        assert manifest.date_earliest is not None
+        assert self.doc.prepare_date_earliest(instance=manifest) == manifest.date_earliest
+        assert self.doc.prepare_date_latest(instance=manifest) == manifest.date_latest
+
+    def test_prepare_date_earliest_and_latest_fall_back_to_published_date(self):
+        """A manifest with only the display-only published_date set (no
+        published_date_edtf) should still get real date_earliest/date_latest
+        values for search, instead of being silently excluded/miscounted as
+        undated"""
+        manifest = ManifestFactory.create(published_date="1997-07-20")
+        manifest.refresh_from_db()
+        assert manifest.date_earliest is None
+        assert manifest.date_latest is None
+
+        earliest = self.doc.prepare_date_earliest(instance=manifest)
+        latest = self.doc.prepare_date_latest(instance=manifest)
+        assert earliest is not None
+        assert latest is not None
+        assert earliest == latest  # exact single date, no fuzziness
+
+    def test_prepare_date_earliest_and_latest_none_for_unparseable_or_missing(self):
+        """Should return None (not raise) when published_date is missing or
+        can't be parsed as a date at all"""
+        manifest = ManifestFactory.create(published_date=None)
+        assert self.doc.prepare_date_earliest(instance=manifest) is None
+        assert self.doc.prepare_date_latest(instance=manifest) is None
+
+        manifest.published_date = "S.l. : s.n."
+        assert self.doc.prepare_date_earliest(instance=manifest) is None
+        assert self.doc.prepare_date_latest(instance=manifest) is None
+
     def test_get_queryset(self):
         """Test prefetching"""
         manifest = ManifestFactory.create()

apps/readux/forms.py

@@ -169,6 +169,11 @@ class ManifestSearchForm(forms.Form):
             format="%Y-%m-%d",
         ),
     )
+    include_undated = forms.BooleanField(
+        label="Show volumes without a published date",
+        required=False,
+        widget=forms.CheckboxInput(attrs={"class": "uk-checkbox"}),
+    )
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)

apps/readux/tests/test_views.py

@@ -220,6 +220,67 @@ def test_get_queryset_filters(self):
         response = search_results.execute(ignore_cache=True)
         assert response.hits.total["value"] == 1
 
+    def test_get_queryset_date_range_excludes_undated_by_default(self):
+        """A date filter should exclude volumes with no published date unless
+        include_undated is set"""
+        undated = Manifest(
+            pid="uniquepid-undated",
+            label="undated volume",
+            summary="test",
+            author="Ben",
+        )
+        undated.save()
+        ManifestDocument().update(undated, True, "index")
+
+        volume_search_view = views.VolumeSearchView()
+        volume_search_view.request = Mock()
+
+        # date filter active, include_undated not set: undated volume excluded
+        volume_search_view.request.GET = {
+            "start_date": "2020-01-01",
+            "end_date": "2024-01-01",
+        }
+        search_results = volume_search_view.get_queryset()
+        response = search_results.execute(ignore_cache=True)
+        pids = {hit["pid"] for hit in response.hits}
+        assert response.hits.total["value"] == 2
+        assert undated.pid not in pids
+
+        # date filter active, include_undated set: undated volume included
+        # alongside anything actually within the date range
+        volume_search_view.request.GET = {
+            "start_date": "2020-01-01",
+            "end_date": "2024-01-01",
+            "include_undated": "on",
+        }
+        search_results = volume_search_view.get_queryset()
+        response = search_results.execute(ignore_cache=True)
+        pids = {hit["pid"] for hit in response.hits}
+        assert response.hits.total["value"] == 3
+        assert undated.pid in pids
+
+    def test_get_queryset_date_aggregation_unaffected_by_date_filter(self):
+        """The min/max date aggregation used to populate the year dropdowns
+        should reflect the full available range, not shrink to whatever date
+        filter is currently applied — otherwise a "reset to full range"
+        control could never recover the original bounds."""
+        volume_search_view = views.VolumeSearchView()
+        volume_search_view.request = Mock()
+
+        volume_search_view.request.GET = {
+            "start_date": "2022-01-01",
+            "end_date": "2022-12-31",
+        }
+        search_results = volume_search_view.get_queryset()
+        response = search_results.execute(ignore_cache=True)
+        # aggregation should span all three dated volumes (1900-2022), not
+        # just the ones matching the applied 2022 date filter
+        in_scope = response.aggregations.date_range_scope.in_scope
+        assert in_scope.dated_earliest.min_date.value is not None
+        assert in_scope.dated_latest.max_date.value is not None
+        span_days = in_scope.dated_latest.max_date.value - in_scope.dated_earliest.min_date.value
+        assert span_days > 365 * 100  # spans well over a century (1900-2022)
+
     def test_get_queryset_sorting(self):
         """Should sort according to default or chosen sort"""
         volume_search_view = views.VolumeSearchView()
@@ -317,10 +378,11 @@ def test_get_context_data(self, mock_set_date, mock_set_facets):
             # these are not nested facets, so delete "inner" attributes
             del response.aggregations.language.inner
             del response.aggregations.author.inner
+            response.aggregations.date_range_scope.in_scope.undated.doc_count = 5
 
             from datetime import date
             with patch("apps.readux.views.jd_to_date", side_effect=[date(1800, 1, 1), date(2022, 12, 31)]):
-                volume_search_view.get_context_data()
+                context_data = volume_search_view.get_context_data()
                 mock_set_facets.assert_called_with(
                     {
                         "language": response.aggregations.language.buckets,
@@ -330,3 +392,36 @@ def test_get_context_data(self, mock_set_date, mock_set_facets):
                     }
                 )
                 mock_set_date.assert_called_with("1800-01-01", "2022-12-31")
+                # min date resolved to a real (non-None) date, so no BCE clamping happened
+                assert context_data["date_range_has_bce"] is False
+                assert context_data["undated_volume_count"] == 5
+
+    @patch("apps.readux.forms.ManifestSearchForm.set_facets")
+    @patch("apps.readux.forms.ManifestSearchForm.set_date")
+    def test_get_context_data_flags_bce_clamping(self, mock_set_date, mock_set_facets):
+        """Should flag when the earliest date had to be clamped to year 1
+        because the true minimum is BCE (unrepresentable as a Python date)"""
+        volume_search_view = views.VolumeSearchView(kwargs={})
+        volume_search_view.request = Mock()
+        volume_search_view.request.GET = {}
+        volume_search_view.facets = [
+            ("language", Mock()),
+            ("author", Mock()),
+            ("collections", Mock()),
+        ]
+        with patch("apps.readux.views.VolumeSearchView.get_queryset") as mock_queryset:
+            volume_search_view.queryset = mock_queryset
+            volume_search_view.object_list = mock_queryset
+            response = Mock()
+            mock_queryset.return_value.__getitem__.return_value.execute.return_value = response
+            del response.aggregations.language.inner
+            del response.aggregations.author.inner
+            response.aggregations.date_range_scope.in_scope.undated.doc_count = 0
+
+            from datetime import date
+            # jd_to_date returns None for the min (BCE, unrepresentable), a real
+            # date for the max
+            with patch("apps.readux.views.jd_to_date", side_effect=[None, date(2022, 12, 31)]):
+                context_data = volume_search_view.get_context_data()
+                mock_set_date.assert_called_with("0001-01-01", "2022-12-31")
+                assert context_data["date_range_has_bce"] is True