From 947d8aeb6c2e4ccb06c182bb4648bec549722612 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 10:57:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(search):=20respect=20DATE=20sort=20when=20t?= =?UTF-8?q?ext=20is=20present=20=E2=80=94=20do=20not=20override=20with=20r?= =?UTF-8?q?elevance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user explicitly selects DATE sort with a text query active, the previous code treated it identically to RELEVANCE, silently discarding the user's sort choice. Remove DATE from the useRankOrder condition so that explicit DATE sort always goes through the standard JPA sort path. Co-Authored-By: Claude Sonnet 4.6 --- .../service/DocumentService.java | 4 +- .../service/DocumentServiceSortTest.java | 100 ++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index caf2e280..c0747cc4 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -319,8 +319,8 @@ public class DocumentService { return sortBySender(results, dir); } - // RELEVANCE: default when text present and no explicit non-relevance sort requested - boolean useRankOrder = hasText && (sort == null || sort == DocumentSort.RELEVANCE || sort == DocumentSort.DATE); + // RELEVANCE: default when text present and no explicit sort given + boolean useRankOrder = hasText && (sort == null || sort == DocumentSort.RELEVANCE); if (useRankOrder) { List results = documentRepository.findAll(spec); final List ids = rankedIds; diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java new file mode 100644 index 00000000..44a7a51e --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java @@ -0,0 +1,100 @@ +package org.raddatz.familienarchiv.service; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.dto.DocumentSort; +import org.raddatz.familienarchiv.model.Document; +import org.raddatz.familienarchiv.model.DocumentStatus; +import org.raddatz.familienarchiv.repository.DocumentRepository; +import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.domain.Specification; + +import java.time.LocalDate; +import java.util.List; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DocumentServiceSortTest { + + @Mock DocumentRepository documentRepository; + @Mock PersonService personService; + @Mock FileService fileService; + @Mock TagService tagService; + @Mock DocumentVersionService documentVersionService; + @Mock AnnotationService annotationService; + @InjectMocks DocumentService documentService; + + // ─── searchDocuments — DATE sort ────────────────────────────────────────── + + @Test + void searchDocuments_with_DATE_sort_and_text_sorts_chronologically_not_by_relevance() { + UUID id1 = UUID.randomUUID(); // rank position 0 (higher relevance, older doc) + UUID id2 = UUID.randomUUID(); // rank position 1 (lower relevance, newer doc) + + Document older = Document.builder().id(id1) + .title("Brief").status(DocumentStatus.UPLOADED) + .documentDate(LocalDate.of(1940, 1, 1)).build(); + Document newer = Document.builder().id(id2) + .title("Brief").status(DocumentStatus.UPLOADED) + .documentDate(LocalDate.of(1960, 1, 1)).build(); + + // FTS returns id1 first (higher rank), id2 second + when(documentRepository.findRankedIdsByFts("Brief")).thenReturn(List.of(id1, id2)); + // findAll(spec, sort) — the correct date path — returns date-DESC order + when(documentRepository.findAll(any(Specification.class), any(Sort.class))) + .thenReturn(List.of(newer, older)); + + List result = documentService.searchDocuments( + "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC"); + + // Expect: date order (newer 1960 first), NOT rank order (older 1940 first) + assertThat(result).hasSize(2); + assertThat(result.get(0).getId()).isEqualTo(id2); // newer doc first + } + + // ─── searchDocuments — RELEVANCE sort ───────────────────────────────────── + + @Test + void searchDocuments_with_RELEVANCE_sort_and_text_preserves_fts_rank_order() { + UUID id1 = UUID.randomUUID(); // rank position 0 + UUID id2 = UUID.randomUUID(); // rank position 1 + + Document doc1 = Document.builder().id(id1).title("Brief").status(DocumentStatus.UPLOADED).build(); + Document doc2 = Document.builder().id(id2).title("Brief").status(DocumentStatus.UPLOADED).build(); + + when(documentRepository.findRankedIdsByFts("Brief")).thenReturn(List.of(id1, id2)); + when(documentRepository.findAll(any(Specification.class))) + .thenReturn(List.of(doc2, doc1)); // unordered from DB + + List result = documentService.searchDocuments( + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null); + + // Expect: rank order restored (id1 first) + assertThat(result.get(0).getId()).isEqualTo(id1); + } + + @Test + void searchDocuments_with_null_sort_and_text_defaults_to_fts_rank_order() { + UUID id1 = UUID.randomUUID(); + UUID id2 = UUID.randomUUID(); + + Document doc1 = Document.builder().id(id1).title("Brief").status(DocumentStatus.UPLOADED).build(); + Document doc2 = Document.builder().id(id2).title("Brief").status(DocumentStatus.UPLOADED).build(); + + when(documentRepository.findRankedIdsByFts("Brief")).thenReturn(List.of(id1, id2)); + when(documentRepository.findAll(any(Specification.class))) + .thenReturn(List.of(doc2, doc1)); + + List result = documentService.searchDocuments( + "Brief", null, null, null, null, null, null, null, null, null); + + assertThat(result.get(0).getId()).isEqualTo(id1); + } +}