Currently one is not able to sort the holdings table by status. IN catalogue_detail.inc I see the following: 451 data: "me.lost_status", 452 className: "status", 453 searchable: false, // FIXME We are losing the ability to search on the status 454 orderable: false, 455 render: function (data, type, row, meta) { Setting orderable to true brings back the icons, but I am assuming since this wasn't fixed initially that it is not trivial.
We do use this feature regularly. Thanks for fixing it.
On bug 33568 I clearly stated that we were loosing the ability to search on the status. """ commit 385048f27ccb5eaeeac2e90130d621305a537960 Bug 33568: Status See the FIXME in the code, we are losing the ability to search on the status, but this is really impossible to implement if we are not storing the status (DB or search engine). """ Actually the search is not impossible, tricky but feasible. However the sort is clearly not possible if we don't store the status (ie. if we keep calculating it on the fly). I think the use cases and the needs are answered with the filtering restored. I am renaming the bug to highlight that, but feel free to open a separate one if it is still problematic. But keep in mind that we need to rethink how/when/where we store and calculate the status, and it's not a trivial change.
Created attachment 172300 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items
Created attachment 172301 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters"
Created attachment 172302 [details] [review] Bug 37334: Restore filtering holdings per status
I am not sure this is the best we can do, but it seems to work. I would like, before continuing (or, better, getting help to get it done), to know if Lucas and Dani are happy with this change. Does it answer your needs? There are more TODO: 1. Tests 2. POD 3. Discuss, rename "_status" (?) This is ready for testing, not to be pushed however.
I am waiting for feedback.
Thanks for your work here Jonathan, I really appreciate it. When I apply this patch and attempt to sort the column I get this 500 error: ==> /var/log/koha/kohadev/plack-api-error.log <== [2024/10/07 16:28:03] [ERROR] GET /api/v1/biblios/9/items: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'me' in 'order clause' at /kohadevbox/koha/Koha/Objects.pm line 403>>
(In reply to Lucas Gass from comment #8) > Thanks for your work here Jonathan, I really appreciate it. > > When I apply this patch and attempt to sort the column I get this 500 error: > > > ==> /var/log/koha/kohadev/plack-api-error.log <== > [2024/10/07 16:28:03] [ERROR] GET /api/v1/biblios/9/items: unhandled > exception > (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI > Exception: DBD::mysql::st execute failed: Unknown column 'me' in 'order > clause' at /kohadevbox/koha/Koha/Objects.pm line 403>> Which column? Status is not sortable, and sort on other columns works for me.
(In reply to Jonathan Druart from comment #9) > (In reply to Lucas Gass from comment #8) > > Thanks for your work here Jonathan, I really appreciate it. > > > > When I apply this patch and attempt to sort the column I get this 500 error: > > > > > > ==> /var/log/koha/kohadev/plack-api-error.log <== > > [2024/10/07 16:28:03] [ERROR] GET /api/v1/biblios/9/items: unhandled > > exception > > (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI > > Exception: DBD::mysql::st execute failed: Unknown column 'me' in 'order > > clause' at /kohadevbox/koha/Koha/Objects.pm line 403>> > > Which column? Status is not sortable, and sort on other columns works for me. Sorry, this is some misunderstanding on my part. I testing the filtering of the Status column and it works great for me. I tested with all the listed statuses. I did file this additional bug for SORTING of the status column: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38122
ping, Lucas?
I took a peek at the code, and this will
(In reply to David Cook from comment #12) > I took a peek at the code, and this will Slow down things? Yes maybe, but only if you filter on status.
Everything works great here except for when filtering by not_for_loan status. In that case I seem to get a 500 error: ==> /var/log/koha/kohadev/plack-api-error.log <== [2024/10/25 22:31:33] [ERROR] GET /api/v1/biblios/32/items: unhandled exception (Koha::Exceptions::Object::MethodNotCoveredByTests)<<The method Koha::ItemTypes->column is not covered by tests!
Created attachment 173499 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items
Created attachment 173500 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters"
Created attachment 173501 [details] [review] Bug 37334: Restore filtering holdings per status
(In reply to Lucas Gass (lukeg) from comment #14) > Everything works great here except for when filtering by not_for_loan > status. In that case I seem to get a 500 error: > > ==> /var/log/koha/kohadev/plack-api-error.log <== > [2024/10/25 22:31:33] [ERROR] GET /api/v1/biblios/32/items: unhandled > exception (Koha::Exceptions::Object::MethodNotCoveredByTests)<<The method > Koha::ItemTypes->column is not covered by tests! Fixed!
(In reply to Jonathan Druart from comment #18) > (In reply to Lucas Gass (lukeg) from comment #14) > > Everything works great here except for when filtering by not_for_loan > > status. In that case I seem to get a 500 error: > > > > ==> /var/log/koha/kohadev/plack-api-error.log <== > > [2024/10/25 22:31:33] [ERROR] GET /api/v1/biblios/32/items: unhandled > > exception (Koha::Exceptions::Object::MethodNotCoveredByTests)<<The method > > Koha::ItemTypes->column is not covered by tests! > > Fixed! Yes, everything seems to work great now. Can we move this out of 'In discussion', happy to add my sign off at this point.
Created attachment 173949 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 173950 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 173951 [details] [review] Bug 37334: Restore filtering holdings per status Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Things are working, I am signing off to keep this moving.
Tested this and while filtering seems to be working with every other status, available status returns either all items or items with other statuses. E.g. I have an item A with damaged value 2 and item B with all statuses (notforloan, damaged, withdrawn, itemlost, restricted and itype is also available for loan) as 0. Only item A is returned when I filter holdings table with "Available" status.
Created attachment 173982 [details] [review] Bug 37334: Fix logic for not available Several major changes here: * the push @not_availab_itemnumbers for lost, damaged, etc. was totally wrong * not for loan is only for positive values * restricted can be NULL
(In reply to Emmi Takkinen from comment #24) > Tested this and while filtering seems to be working with every other status, > available status returns either all items or items with other statuses. > > E.g. I have an item A with damaged value 2 and item B with all statuses > (notforloan, damaged, withdrawn, itemlost, restricted and itype is also > available for loan) as 0. Only item A is returned when I filter holdings > table with "Available" status. Indeed, lot of wrong logics in the previous version, thanks!
I won't be able to work more on this soon. Feel free to test, but it's not ready for inclusion (hence the previous "In discussion" status). It's missing a lot of tests. Please take over if you are willing to make it move forward. The tests can be interesting to write but I won't have the time during the next weeks.
*** Bug 38122 has been marked as a duplicate of this bug. ***
(In reply to Emmi Takkinen from comment #24) > Tested this and while filtering seems to be working with every other status, > available status returns either all items or items with other statuses. > > E.g. I have an item A with damaged value 2 and item B with all statuses > (notforloan, damaged, withdrawn, itemlost, restricted and itype is also > available for loan) as 0. Only item A is returned when I filter holdings > table with "Available" status. I tried to recreate this exact scenario and it seems to work for me. However I did find flaws with the restricted and notforloan filters.
Created attachment 174310 [details] [review] Bug 37334: Fix restricted and notforloan filter logic
I am going to put this back to into discussion. Emmi, if you get a chance can you test again with my follow-up patches. Like Jonathan said, we'll still need to write a bunch of tests here.
First patch needed a minor rebase. Filtering with at least "Available", "Checked out", "In transit", "Damaged" and "Lost" status works, but "Not for loan" status doesn't return results if its value starts with -. E.g. I have authorized values 7 "Added to bundle" and -7 "Auton varastossa". Filtering first one works, second doesn't.
(In reply to Emmi Takkinen from comment #32) > First patch needed a minor rebase. Filtering with at least "Available", > "Checked out", "In transit", "Damaged" and "Lost" status works, but "Not for > loan" status doesn't return results if its value starts with -. E.g. I have > authorized values 7 "Added to bundle" and -7 "Auton varastossa". Filtering > first one works, second doesn't. Yes, I think it's the correct behaviour. C4/Reserves.pm:1322 $item->notforloan > 0 || # item with negative or zero notforloan value is holdable
If I filter on Not for Loan, I would expect to see ALL values, positive or negative. Libraries using negative NFL values would want to see copies that may be on order or in processing, in addition to any that are not holdable. The filtering needs to account for any value where NFL != 0
Created attachment 174798 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 174799 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 174800 [details] [review] Bug 37334: Restore filtering holdings per status Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 174801 [details] [review] Bug 37334: Fix logic for not available Several major changes here: * the push @not_availab_itemnumbers for lost, damaged, etc. was totally wrong * not for loan is only for positive values * restricted can be NULL
Created attachment 174802 [details] [review] Bug 37334: Fix restricted and notforloan filter logic
Created attachment 174803 [details] [review] Bug 37334: Return all non zero notforloan statuses
(In reply to Catrina Berka from comment #34) > If I filter on Not for Loan, I would expect to see ALL values, positive or > negative. Libraries using negative NFL values would want to see copies that > may be on order or in processing, in addition to any that are not holdable. > The filtering needs to account for any value where NFL != 0 Rebased and added a patch, based on Catrina's comment, so the notforloan filter will return ALL nfl statuses, positive or negative.
I am noticing that filtering by available is returning items with a negative notforloan status when it should not.
Created attachment 174804 [details] [review] Bug 37334: filter_by_available should not return notforloan statues other than 0
Created attachment 174813 [details] [review] Bug 37334: Add _status tests
There is a few more tests to write here for the 'filter_by*' routines.
Created attachment 174814 [details] [review] Bug 37334: Add filter_by_checked_out tests
Created attachment 174901 [details] [review] Bug 37334: Add POD
Jonathan had an empty routine, I removed it. I don't think we need this? sub filter_by_for_loan { } We can already filter by not_for_loan, available, and checked_out.
Created attachment 174902 [details] [review] Bug 37334: Add filter_by_in_transit tests
Created attachment 174903 [details] [review] Bug 37334: Add filter_by_has_holds tests
Trying to write tests for filter_by_has_recalls made me realize filtering by recalls is working quite right.
(In reply to Lucas Gass (lukeg) from comment #51) > Trying to write tests for filter_by_has_recalls made me realize filtering by > recalls is working quite right. It is NOT working quite right, fix incoming.
(In reply to Lucas Gass (lukeg) from comment #52) > (In reply to Lucas Gass (lukeg) from comment #51) > > Trying to write tests for filter_by_has_recalls made me realize filtering by > > recalls is working quite right. > > It is NOT working quite right, fix incoming. filter_by_has_recalls needs to filter by item_level recalls only, anything else should be considered available.
Created attachment 174955 [details] [review] Bug 37334: Fix filter_by_has_recalls
Is this ready for testing?
(In reply to Emmi Takkinen from comment #55) > Is this ready for testing? It is blocked by Bug 38512. I think we need to make sure the display of recalls is correct before we can continue here, please test that one first!
This can be unblocked because Bug 38512 is PQA. It needs rebased on top of that one now, setting to patch doesn't apply and will rebase asap.
Created attachment 175859 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 175860 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 175861 [details] [review] Bug 37334: Restore filtering holdings per status Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 175862 [details] [review] Bug 37334: Fix logic for not available Several major changes here: * the push @not_availab_itemnumbers for lost, damaged, etc. was totally wrong * not for loan is only for positive values * restricted can be NULL
Created attachment 175863 [details] [review] Bug 37334: Fix restricted and notforloan filter logic
Created attachment 175864 [details] [review] Bug 37334: Return all non zero notforloan statuses
Created attachment 175865 [details] [review] Bug 37334: filter_by_available should not return notforloan statues other than 0
Created attachment 175866 [details] [review] Bug 37334: Add _status tests
Created attachment 175867 [details] [review] Bug 37334: Add filter_by_checked_out tests
Created attachment 175868 [details] [review] Bug 37334: Add POD
Created attachment 175869 [details] [review] Bug 37334: Add filter_by_in_transit tests
Created attachment 175870 [details] [review] Bug 37334: Add filter_by_has_holds tests
Created attachment 175871 [details] [review] Bug 37334: Fix filter_by_has_recalls
Patches no longer apply. Applying: Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc). error: could not build fake ancestor Patch failed at 0001 Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-37334-Allow-embed-status-for-GET-bibliobiblioi-34QjuN.patch
Created attachment 175999 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176000 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176001 [details] [review] Bug 37334: Restore filtering holdings per status Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176002 [details] [review] Bug 37334: Fix logic for not available Several major changes here: * the push @not_availab_itemnumbers for lost, damaged, etc. was totally wrong * not for loan is only for positive values * restricted can be NULL
Created attachment 176003 [details] [review] Bug 37334: Fix restricted and notforloan filter logic
Created attachment 176004 [details] [review] Bug 37334: Return all non zero notforloan statuses
Created attachment 176005 [details] [review] Bug 37334: filter_by_available should not return notforloan statues other than 0
Created attachment 176006 [details] [review] Bug 37334: Add _status tests
Created attachment 176007 [details] [review] Bug 37334: Add filter_by_checked_out tests
Created attachment 176008 [details] [review] Bug 37334: Add POD
Created attachment 176009 [details] [review] Bug 37334: Add filter_by_in_transit tests
Created attachment 176010 [details] [review] Bug 37334: Add filter_by_has_holds tests
Created attachment 176011 [details] [review] Bug 37334: Fix filter_by_has_recalls
Created attachment 176023 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176024 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176025 [details] [review] Bug 37334: Restore filtering holdings per status Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176026 [details] [review] Bug 37334: Fix logic for not available Several major changes here: * the push @not_availab_itemnumbers for lost, damaged, etc. was totally wrong * not for loan is only for positive values * restricted can be NULL Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176027 [details] [review] Bug 37334: Fix restricted and notforloan filter logic Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176028 [details] [review] Bug 37334: Return all non zero notforloan statuses Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176029 [details] [review] Bug 37334: filter_by_available should not return notforloan statues other than 0 Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176030 [details] [review] Bug 37334: Add _status tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176031 [details] [review] Bug 37334: Add filter_by_checked_out tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176032 [details] [review] Bug 37334: Add POD Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176033 [details] [review] Bug 37334: Add filter_by_in_transit tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176034 [details] [review] Bug 37334: Add filter_by_has_holds tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 176035 [details] [review] Bug 37334: Fix filter_by_has_recalls Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Tested and now filtering with status works as intended. Also tests pass.
Comment on attachment 176023 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Review of attachment 176023 [details] [review]: ----------------------------------------------------------------- ::: Koha/Item.pm @@ +1469,5 @@ > + if ( $self->damaged ) { > + push @statuses, 'damaged'; > + } > + if ( $self->notforloan || $self->item_type->notforloan ) { > + # TODO on a big Koha::Items loop we are going to join with item_type too often, use a cache Is the concern big enough to at least have a ticket opened to measure this? And eventually implement a cache. @@ +1487,5 @@ > + if ( $self->restricted ) { > + push @statuses, 'restricted'; > + } > + > + # TODO in_bundle? Was that meant to be temporary or does it have a long term use? Does it warrant at least opening a follow-up ticket or it's too minor? Does it warrant some manual testing related to bundles?
(In reply to Victor Grousset/tuxayo from comment #99) > Comment on attachment 176023 [details] [review] [review] > Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items > > Review of attachment 176023 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: Koha/Item.pm > @@ +1469,5 @@ > > + if ( $self->damaged ) { > > + push @statuses, 'damaged'; > > + } > > + if ( $self->notforloan || $self->item_type->notforloan ) { > > + # TODO on a big Koha::Items loop we are going to join with item_type too often, use a cache > > Is the concern big enough to at least have a ticket opened to measure this? > And eventually implement a cache. Yes, we should open a new bug report for this. > @@ +1487,5 @@ > > + if ( $self->restricted ) { > > + push @statuses, 'restricted'; > > + } > > + > > + # TODO in_bundle? > > Was that meant to be temporary or does it have a long term use? > Does it warrant at least opening a follow-up ticket or it's too minor? > Does it warrant some manual testing related to bundles? in_bundle is a status, so that needs to be included in this bug, I think. I will add ASAP.
To pass the tests, only these two commits are needed: Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items Bug 37334: Restore filtering holdings per status Looking the diffs of the other commits, there are substantial changes that would have been expected to be necessary for tests to pass. After that smoke test, let's look at proper coverage: https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL17:_Unit_tests_are_required These are not covered by tests: sub filter_by_has_recalls sub filter_by_available search() have very little coverage. At least having the most complex case (not_for_loan) covered would be safer. The "recalled" and "available" cases would also cover the above paragraph. Or maybe they should be tested directly and not via search(). No opinion on this.
> Yes, we should open a new bug report for this. ok, I'm experimenting with hijacking the release notes field to keep track of this. ------ Thanks for the existing tests. coverage of _status is perfect and so is all the other filter_by_* subs.
Created attachment 176475 [details] [review] Bug 37334: Add ability to filter by in_bundle
Created attachment 176477 [details] [review] Bug 37334: filter_by_in_bundle tests
Created attachment 176480 [details] [review] Bug 37334: Add filter_by_recall test
Created attachment 176481 [details] [review] Bug 37334: Add filter_by_available tests
I have added the ability to filter by bundles w/ tests. I have also included tests for filter_by_has_recalls and filter_by_available Resetting to 'Signed off'
*** Bug 38686 has been marked as a duplicate of this bug. ***
(In reply to Victor Grousset/tuxayo from comment #101) > These are not covered by tests: > sub filter_by_has_recalls > sub filter_by_available All good, thanks :) > search() have very little coverage. > At least having the most complex case (not_for_loan) covered would be safer. > The "recalled" and "available" cases would also cover the above paragraph. > Or maybe they should be tested directly and not via search(). No opinion on > this. Still the same issue with search() unfortunately.
Created attachment 177321 [details] [review] Bug 37334: Add search tests
There aren't any test plans that I can see on here... I tried applying the patches and using the search box at the top of the holdings table to search for statuses and it doesn't find anything. Is that what this is trying to solve? To recreate: 1. Go to a record details page (e.g. http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=285) 2. Add a couple of items with different statuses (or change the status of a couple of items) 2.1. Click New > New item 2.2. Change the Not for loan status to Staff collection (or another not for loan status) 2.3. Click Add item 2.4. Repeat 2.2 and 2.3 for various not for loan statuses 2.5. Click Normal to go back to the record details page 3. Try to search for the status in the holdings 3.1. Using the small search box right above the holdings table, search for one of the statuses (e.g. "Staff") --> No results
Created attachment 177415 [details] [review] Bug 37334: Allow embed _status for GET /biblio/{biblio_id}/items To test; 0. APPLY PATCH, restart_all 1. Go to a record details page (e.g. http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=285) 2. Click the 'Show filters' button above the table 3. Now, at the top of the Status column you should see a dropdown to filter by status 4. You'll have to create several item status to test everything. 5. Add some new items/or change some item status to: DAMAGED, WITHDRAWN, NOTFORLOAN, RESTRICTED, and LOST 6. Test the status filter for each of these statuses, ensuring the work correctly. 7. Create more new items and have some that are checked out, ensure that filter works 8. Put some items into transit and make sure the 'In transit' filter works 9. Put some items on hold, both item and bib level. Make sure the 'On hold' filter works. 10. Turn on the USERecalls system preference and make some recalls, both item and bib level. Make sure the 'Recalled' filter works 11. Create some bundles, make sure the 'In bundle' status works. 12. Also make sure that the available filter is working. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177416 [details] [review] Bug 37334: Allow a column filter to be shown even searchable is off We do not want to use the usual filtering method here, we are building the query from "additional filters" Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177417 [details] [review] Bug 37334: Restore filtering holdings per status Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177418 [details] [review] Bug 37334: Fix logic for not available Several major changes here: * the push @not_availab_itemnumbers for lost, damaged, etc. was totally wrong * not for loan is only for positive values * restricted can be NULL Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177419 [details] [review] Bug 37334: Fix restricted and notforloan filter logic Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177420 [details] [review] Bug 37334: Return all non zero notforloan statuses Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177421 [details] [review] Bug 37334: filter_by_available should not return notforloan statues other than 0 Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177422 [details] [review] Bug 37334: Add _status tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177423 [details] [review] Bug 37334: Add filter_by_checked_out tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177424 [details] [review] Bug 37334: Add POD Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177425 [details] [review] Bug 37334: Add filter_by_in_transit tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177426 [details] [review] Bug 37334: Add filter_by_has_holds tests Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177427 [details] [review] Bug 37334: Fix filter_by_has_recalls Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Created attachment 177428 [details] [review] Bug 37334: Add ability to filter by in_bundle
Created attachment 177429 [details] [review] Bug 37334: filter_by_in_bundle tests
Created attachment 177430 [details] [review] Bug 37334: Add filter_by_recall test
Created attachment 177431 [details] [review] Bug 37334: Add filter_by_available tests
Created attachment 177432 [details] [review] Bug 37334: Add search tests
(In reply to Caroline Cyr La Rose from comment #111) > There aren't any test plans that I can see on here... I tried applying the > patches and using the search box at the top of the holdings table to search > for statuses and it doesn't find anything. Is that what this is trying to > solve? I attached a test plan to the first commit.
For testing, one needs to run `yarn api:bundle` after applying the patch and before restarting the services. Otherwise there is an error 400 when loading the record page.