Bug 41602 - Cannot combine filters with holds status filters
Summary: Cannot combine filters with holds status filters
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Baptiste Wojtkowski (bwoj)
URL:
Keywords:
Depends on: 37334 41254
Blocks:
  Show dependency treegraph
 
Reported: 2026-01-13 14:49 UTC by Baptiste Wojtkowski (bwoj)
Modified: 2026-02-04 06:17 UTC (History)
3 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Fixes a '500 error' when using multiple filters for the holdings table for a record in the staff interface.
Version(s) released in:
Circulation function:


Attachments
Bug 41602: Add a test (1.33 KB, patch)
2026-01-22 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 41602: Allow _status to be in nested structure (2.03 KB, patch)
2026-01-22 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 41602: Add a test (1.38 KB, patch)
2026-01-22 19:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 41602: Allow _status to be in nested structure (2.08 KB, patch)
2026-01-22 19:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 41602: Test proposal (2.03 KB, patch)
2026-01-23 10:52 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Baptiste Wojtkowski (bwoj) 2026-01-13 14:49:44 UTC
Test plan:
1 - Go on http://your_ktd:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=360
2 - Filter by Status: Available
3 - Check any other filter combined with the Status, you get a 500 error
Comment 1 Jonathan Druart 2026-01-13 15:28:51 UTC
Should be done on top of bug 41254.
Comment 2 Jonathan Druart 2026-01-22 14:36:38 UTC
Created attachment 191862 [details] [review]
Bug 41602: Add a test

Patch from commit 5b21d47
Comment 3 Jonathan Druart 2026-01-22 14:36:39 UTC
Created attachment 191863 [details] [review]
Bug 41602: Allow _status to be in nested structure

This patch allows _status to be in a nested structure.

Cannot be combined in all the possible ways, but at least it should fix
the UI.

Test plan:
Use the filters on the holdings table and confirm that you can combine
the status with other filters.
Comment 4 David Nind 2026-01-22 19:20:36 UTC
Created attachment 191883 [details] [review]
Bug 41602: Add a test

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2026-01-22 19:20:39 UTC
Created attachment 191884 [details] [review]
Bug 41602: Allow _status to be in nested structure

This patch allows _status to be in a nested structure.

Cannot be combined in all the possible ways, but at least it should fix
the UI.

Test plan:
Use the filters on the holdings table and confirm that you can combine
the status with other filters.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2026-01-22 19:22:45 UTC
Testing notes (using KTD):
1. Tests pass: prove t/db_dependent/Koha/Items.t
Comment 7 Baptiste Wojtkowski (bwoj) 2026-01-23 10:46:10 UTC
I am not sure I understand the code perfectly, but as far as I understand the _status key is totally removed from the search and studied before the real search. Status is hence always considered as an "And" relation, which makes sense in case of filters, but if a developer uses the "search" function of items with "-or", won't it be broken ?
Comment 8 Baptiste Wojtkowski (bwoj) 2026-01-23 10:52:50 UTC
Created attachment 191916 [details] [review]
Bug 41602: Test proposal
Comment 9 Baptiste Wojtkowski (bwoj) 2026-01-23 10:53:20 UTC
I added a test in a separate commit to illustrate my point
Comment 10 Baptiste Wojtkowski (bwoj) 2026-01-23 10:53:54 UTC
(Patch tested and it works for the studied case nonetheless)
Comment 11 Jonathan Druart 2026-01-23 11:02:44 UTC
(In reply to Baptiste Wojtkowski (bwoj) from comment #7)
> I am not sure I understand the code perfectly, but as far as I understand
> the _status key is totally removed from the search and studied before the
> real search. Status is hence always considered as an "And" relation, which
> makes sense in case of filters, but if a developer uses the "search"
> function of items with "-or", won't it be broken ?

Yes, it's what I wanted to explain with "Cannot be combined in all the possible ways, but at least it should fix the UI." I could have been a bit more verbose.

We only support what the UI proposes. More would be tricky.
Comment 12 Baptiste Wojtkowski (bwoj) 2026-01-23 13:44:58 UTC
Shouldn't we throw an error if there is something like an or clause and a status ? Or at least document it in the pod ?
Comment 13 Jonathan Druart 2026-01-23 14:27:33 UTC
(In reply to Baptiste Wojtkowski (bwoj) from comment #12)
> Shouldn't we throw an error if there is something like an or clause and a
> status ? Or at least document it in the pod ?

Yes, maybe, good idea.
Comment 14 Jonathan Druart 2026-01-26 12:30:57 UTC
(In reply to Jonathan Druart from comment #13)
> (In reply to Baptiste Wojtkowski (bwoj) from comment #12)
> > Shouldn't we throw an error if there is something like an or clause and a
> > status ? Or at least document it in the pod ?
> 
> Yes, maybe, good idea.

I don't know how to implement that in a reliable way...
Comment 15 Baptiste Wojtkowski (bwoj) 2026-01-26 14:03:43 UTC
Can you determine if there is a -and at the "root" of the parameters tree and _status right under it ? I think we do not guarantee to the user anything concerning _status except this case and empty parameters tree.
We'll lose some cases where it would have worked
Comment 16 Baptiste Wojtkowski (bwoj) 2026-01-26 14:04:02 UTC
But we should not need it otherwise