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
Should be done on top of bug 41254.
Created attachment 191862 [details] [review] Bug 41602: Add a test Patch from commit 5b21d47
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.
Created attachment 191883 [details] [review] Bug 41602: Add a test Signed-off-by: David Nind <david@davidnind.com>
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>
Testing notes (using KTD): 1. Tests pass: prove t/db_dependent/Koha/Items.t
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 ?
Created attachment 191916 [details] [review] Bug 41602: Test proposal
I added a test in a separate commit to illustrate my point
(Patch tested and it works for the studied case nonetheless)
(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.
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 ?
(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.
(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...
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
But we should not need it otherwise