As pointed by @andrew on bug 15448, NULL values are not handled correctly here, because NOT IN <LIST> considers NULL to be included in <LIST>: MariaDB [koha_kohadev]> SELECT itemnumber,copynumber FROM items WHERE itemnumber=1; +------------+------------+ | itemnumber | copynumber | +------------+------------+ | 1 | NULL | +------------+------------+ 1 row in set (0.00 sec) MariaDB [koha_kohadev]> SELECT itemnumber,copynumber FROM items WHERE itemnumber=1 AND copynumber NOT IN (2); Empty set (0.00 sec) MariaDB [koha_kohadev]> UPDATE items SET copynumber=1 WHERE itemnumber=1; Query OK, 1 row affected (0.00 sec) Rows matched: 1 Changed: 1 Warnings: 0 MariaDB [koha_kohadev]> SELECT itemnumber,copynumber FROM items WHERE itemnumber=1 AND copynumber NOT IN (2); +------------+------------+ | itemnumber | copynumber | +------------+------------+ | 1 | 1 | +------------+------------+ 1 row in set (0.00 sec)
Created attachment 116076 [details] [review] Bug 27580: Regression tests This test makes all the generated sample items have undef copynumber, and all rules are added copynumber => [ 2 ] on the tests as suggested by Andrew on his failing example. The tests are expected to pass. i.e. copynumber is clearly not equal to 2, so those items shouldn't be hidden. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => FAIL: Tests fail! undef is considered in [ 2 ] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 116077 [details] [review] Bug 27580: Handle NULL values correctly in Koha::Items->filter_by_visible_in_opac This patch fixes the internally used query so it doesn't consider NULL values as IN the set. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => FAIL: Tests fail :-/ 3. Apply this patch 4. Repeat 2 => SUCCESS: tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 116080 [details] [review] Bug 27580: Regression tests This test makes all the generated sample items have undef copynumber, and all rules are added copynumber => [ 2 ] on the tests as suggested by Andrew on his failing example. The tests are expected to pass. i.e. copynumber is clearly not equal to 2, so those items shouldn't be hidden. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => FAIL: Tests fail! undef is considered in [ 2 ] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 116081 [details] [review] Bug 27580: Handle NULL values correctly in Koha::Items->filter_by_visible_in_opac This patch fixes the internally used query so it doesn't consider NULL values as IN the set. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => FAIL: Tests fail :-/ 3. Apply this patch 4. Repeat 2 => SUCCESS: tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 116094 [details] [review] Bug 27580: Regression tests This test makes all the generated sample items have undef copynumber, and all rules are added copynumber => [ 2 ] on the tests as suggested by Andrew on his failing example. The tests are expected to pass. i.e. copynumber is clearly not equal to 2, so those items shouldn't be hidden. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => FAIL: Tests fail! undef is considered in [ 2 ] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 116095 [details] [review] Bug 27580: Handle NULL values correctly in Koha::Items->filter_by_visible_in_opac This patch fixes the internally used query so it doesn't consider NULL values as IN the set. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => FAIL: Tests fail :-/ 3. Apply this patch 4. Repeat 2 => SUCCESS: tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.03
Pushed to 20.05.x for 20.05.09
Backported: Pushed to 19.11.x branch for 19.11.15