From b52bf609134c7e1d8bec1c43c8aedb8dfcf5c128 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 27 May 2026 20:21:18 +0200 Subject: [PATCH] fix(document): tie-break equal-date DATE sort by title asc, not createdAt Owner decision (#668): when two documents share a meta_date, order them by title ascending instead of createdAt ascending. title is @Column(nullable=false) so it is always present, giving a deterministic, human-meaningful total order. Only the DATE-sort fast path changes; the in-memory SENDER/RECEIVER/RELEVANCE comparators are untouched. ORDER BY meta_date NULLS LAST, title ASC Tests assert title-asc tiebreaking for same-date rows in BOTH directions, with a fixture whose title order is the OPPOSITE of insertion (createdAt) order so the test fails if the tiebreaker reverts to createdAt. The integration test drives the production resolveSort against real Postgres. Refs #668 --- .../document/DocumentService.java | 5 ++- .../document/DocumentServiceTest.java | 10 +++++ ...ndatedDocumentOrderingIntegrationTest.java | 45 +++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index b4762502..d25cf03c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -839,11 +839,12 @@ public class DocumentService { // Undated documents (null documentDate) must order last regardless of // direction — Postgres puts NULLs FIRST on ASC by default, which would // surface the undated pile at the top with no explanation (issue #668). - // The createdAt tiebreaker gives a stable total order when every row is + // The title tiebreaker gives a stable total order when every row is // null-dated (the "Nur undatierte" filter), so pagination is deterministic. + // title is @Column(nullable=false), so it is always present. return Sort.by( new Sort.Order(direction, "documentDate").nullsLast(), - Sort.Order.asc("createdAt")); + Sort.Order.asc("title")); } // SENDER and RECEIVER are sorted in-memory before this method is called return switch (sort) { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java index b0f16574..04b84fba 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java @@ -1463,6 +1463,11 @@ class DocumentServiceTest { assertThat(dateOrder).isNotNull(); assertThat(dateOrder.getDirection()).isEqualTo(Sort.Direction.DESC); assertThat(dateOrder.getNullHandling()).isEqualTo(Sort.NullHandling.NULLS_LAST); + // Owner-decided tiebreaker (#668): title ASC, not createdAt. + Sort.Order tiebreak = captor.getValue().getSort().getOrderFor("title"); + assertThat(tiebreak).isNotNull(); + assertThat(tiebreak.getDirection()).isEqualTo(Sort.Direction.ASC); + assertThat(captor.getValue().getSort().getOrderFor("createdAt")).isNull(); } @Test @@ -1481,6 +1486,11 @@ class DocumentServiceTest { assertThat(dateOrder).isNotNull(); assertThat(dateOrder.getDirection()).isEqualTo(Sort.Direction.ASC); assertThat(dateOrder.getNullHandling()).isEqualTo(Sort.NullHandling.NULLS_LAST); + // Owner-decided tiebreaker (#668): title ASC, not createdAt. + Sort.Order tiebreak = captor.getValue().getSort().getOrderFor("title"); + assertThat(tiebreak).isNotNull(); + assertThat(tiebreak.getDirection()).isEqualTo(Sort.Direction.ASC); + assertThat(captor.getValue().getSort().getOrderFor("createdAt")).isNull(); } @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/UndatedDocumentOrderingIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/UndatedDocumentOrderingIntegrationTest.java index e7e50d74..e1eeddc7 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/UndatedDocumentOrderingIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/UndatedDocumentOrderingIntegrationTest.java @@ -63,6 +63,51 @@ class UndatedDocumentOrderingIntegrationTest { assertThat(result.get(3).getDocumentDate()).isNull(); } + @Test + void sameDate_tiebreaksByTitleAsc_notCreatedAt_forBothDirections() throws Exception { + // Owner decision (#668): equal-date rows tie-break by title ASC, NOT + // createdAt. Insert two same-date docs so that createdAt order (insertion + // order) is the OPPOSITE of title order: the first-saved doc gets the later + // title ("zzz-first"), the second-saved doc gets the earlier title + // ("aaa-second"). If the tiebreaker were still createdAt-asc the first-saved + // row would lead; because it is title-asc the "aaa-second" row must lead — + // and it must lead in BOTH ASC and DESC date directions, since the date is + // equal so only the title tiebreaker decides. + // + // The Sort under test is built by the PRODUCTION resolveSort(DATE, dir) (via + // reflection — it is private), not hand-rolled here, so this test proves the + // real Postgres ordering that production emits, on real same-date rows. + documentRepository.deleteAll(); + LocalDate sameDate = LocalDate.of(1920, 3, 3); + save("zzz-first", sameDate); // saved first → earlier createdAt + save("aaa-second", sameDate); // saved second → later createdAt + + List asc = documentRepository.findAll(resolveProductionSort("ASC")); + assertThat(asc).extracting(Document::getTitle) + .containsExactly("aaa-second", "zzz-first"); + + List desc = documentRepository.findAll(resolveProductionSort("DESC")); + assertThat(desc).extracting(Document::getTitle) + .containsExactly("aaa-second", "zzz-first"); + } + + /** + * Invokes the production {@link DocumentService#resolveSort(DocumentSort, String)} + * for the DATE sort so the integration assertions exercise the real tiebreaker + * choice rather than a sort hand-built in the test. + */ + private Sort resolveProductionSort(String dir) throws Exception { + // resolveSort is a pure function of its arguments (uses no instance state), so a + // bean instance with null collaborators is sufficient to exercise it. + var ctor = DocumentService.class.getDeclaredConstructors()[0]; + ctor.setAccessible(true); + Object[] args = new Object[ctor.getParameterCount()]; + DocumentService service = (DocumentService) ctor.newInstance(args); + var m = DocumentService.class.getDeclaredMethod("resolveSort", DocumentSort.class, String.class); + m.setAccessible(true); + return (Sort) m.invoke(service, DocumentSort.DATE, dir); + } + @Test void undatedOnly_returnsExactlyTheNullDatedRows() { List result = documentRepository.findAll(undatedOnly(true));