There were no tests for GetHiddenItems, and given that I have been working on tweaking this function (bug 10589, bug 10872), I thought some test modules might be in order. t/db_dependent/GetHiddenItems.t - Generic toggling of OpacHiddenItems system preference to test if NULL, blank, whitespace, and a rule all work. t/db_dependent/GetHiddenItems_PatronCategory.t - Setting of OpacHiddenItems, and then toggling a dummy patron's category code to see if visibility is correct.
Created attachment 21131 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied.
Created attachment 21132 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied.
Created attachment 21134 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied.
Without applying 10589 to master: $ git checkout -b bug_10895 origin/master $ git bz apply 10895 $ prove -v t/db_dependent/GetHiddenItems_PatronCategory.t [SNIP] Test Summary Report ------------------- t/db_dependent/GetHiddenItems_PatronCategory.t (Wstat: 3072 Tests: 22 Failed: 12) Failed tests: 7-11, 14-15, 18-22 Non-zero exit status: 12 Files=1, Tests=22, 1 wallclock secs ( 0.02 usr 0.01 sys + 0.38 cusr 0.02 csys = 0.43 CPU) Result: FAIL $ git reset --hard origin/master $ git bz apply 10589 $ git bz apply 10895 $ prove -v t/db_dependent/GetHiddenItems_PatronCategory.t [SNIP] All tests successful. Files=1, Tests=22, 1 wallclock secs ( 0.02 usr 0.00 sys + 0.36 cusr 0.05 csys = 0.43 CPU) Result: PASS I also checked it against koha-qa.pl, no worrisome output.
Side comment: tcohen helped me out by writing a version of GetHiddenItems.t which he placed in bug 10872, by creating a sub-test within t/db_dependent/Items.t for a couple more things than I had planned. So, there is only one test file attached, unlike the first comment suggests.
Created attachment 21153 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch.
Patch applied cleanly, go forth and signoff
perl t/db_dependent/GetHiddenItems_PatronCategory.t 1..22 ok 1 - use C4::Items; ok 2 - LIB1 Item linked to correct biblionumber. ok 3 - LIB1 Item linked to correct biblioitemnumber. ok 4 - LIB2 Item linked to correct biblionumber. ok 5 - LIB2 Item linked to correct biblioitemnumber. $VAR1 = { 'firstname' => 'Sample', 'userid' => 'Sample.Borrower', 'categorycode' => 'PT', 'phone' => '967-1111', 'zipcode' => 'H0H 0H0', 'surname' => 'Borrower', 'email' => 'santa@northpole.com', 'city' => 'Bogusville', 'address' => '123 Bogus Street', 'branchcode' => 'LIB1' }; # Sample Borrower added. CHECK: 54 xJZNdcnysc # 116 - LIB1 # 117 - LIB2 ok 6 - OpacHiddenItems hid everything correctly. # 116,117 # Borrower number: 54 # Sample Borrower environment variables set up. not ok 7 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 125. # got: 2 # expected: 0 # PatronSingleBranch: SPT # PatronEveryBranch: SPT # 116,117 not ok 8 - OpacHiddenItems hid LIB2 correctly. # Failed test 'OpacHiddenItems hid LIB2 correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 133. # got: 2 # expected: 1 # PatronSingleBranch: SPT # PatronEveryBranch: SS # 116,117 not ok 9 - OpacHiddenItems hid LIB2 correctly. # Failed test 'OpacHiddenItems hid LIB2 correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 141. # got: 2 # expected: 1 # PatronSingleBranch: SPT # PatronEveryBranch: '' # 116,117 not ok 10 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 149. # got: 2 # expected: 0 # PatronSingleBranch: SPT # PatronEveryBranch: SS|SPT # 116,117 not ok 11 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 157. # got: 2 # expected: 0 # PatronSingleBranch: SS # PatronEveryBranch: SPT # 116,117 ok 12 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SS # 116,117 ok 13 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: '' # 116,117 not ok 14 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 181. # got: 2 # expected: 0 # PatronSingleBranch: SS # PatronEveryBranch: SS|SPT # 116,117 not ok 15 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 189. # got: 2 # expected: 0 # PatronSingleBranch: '' # PatronEveryBranch: SPT # 116,117 ok 16 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SS # 116,117 ok 17 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: '' # 116,117 not ok 18 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 213. # got: 2 # expected: 0 # PatronSingleBranch: '' # PatronEveryBranch: SS|SPT # 116,117 not ok 19 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 221. # got: 2 # expected: 0 # PatronSingleBranch: SS|SPT # PatronEveryBranch: SPT # 116,117 not ok 20 - OpacHiddenItems hid LIB2 correctly. # Failed test 'OpacHiddenItems hid LIB2 correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 229. # got: 2 # expected: 1 # PatronSingleBranch: SS|SPT # PatronEveryBranch: SS # 116,117 not ok 21 - OpacHiddenItems hid LIB2 correctly. # Failed test 'OpacHiddenItems hid LIB2 correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 237. # got: 2 # expected: 1 # PatronSingleBranch: SS|SPT # PatronEveryBranch: '' # 116,117 not ok 22 - OpacHiddenItems unhid everything correctly. # Failed test 'OpacHiddenItems unhid everything correctly.' # at t/db_dependent/GetHiddenItems_PatronCategory.t line 245. # got: 2 # expected: 0 # PatronSingleBranch: SS|SPT # PatronEveryBranch: SS|SPT # 116,117 # Looks like you failed 12 tests of 22.
This is because you failed to apply bug 10900 first.
Here is how I tested this. mtompset@ubuntu:~/kohaclone$ git reset --hard origin/master HEAD is now at 5d7aa7e Bug 10725: Add filters on the pending orders table mtompset@ubuntu:~/kohaclone$ git bz apply 10900 Bug 10900 - Incorrect calling conventions accessing C4::Context 22585 - Bug 10900 - Incorrect calling conventions accessing C4::Context Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10900 - Incorrect calling conventions accessing C4::Context mtompset@ubuntu:~/kohaclone$ git bz apply 10589 Bug 10589 - Override OpacHiddenItems based on Patron Category 22587 - Bug 10589 - Override OpacHiddenItems based on Patron Category Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10589 - Override OpacHiddenItems based on Patron Category mtompset@ubuntu:~/kohaclone$ git bz apply 10895 Bug 10895 - Tests for C4::Items GetHiddenItems 21153 - Bug 10895 - Tests for C4::Items GetHiddenItems Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10895 - Tests for C4::Items GetHiddenItems mtompset@ubuntu:~/kohaclone$ prove -v t/db_dependent/GetHiddenItems_PatronCategory.t t/db_dependent/GetHiddenItems_PatronCategory.t .. 1..22 ok 1 - use C4::Items; ok 2 - LIB1 Item linked to correct biblionumber. ok 3 - LIB1 Item linked to correct biblioitemnumber. ok 4 - LIB2 Item linked to correct biblionumber. ok 5 - LIB2 Item linked to correct biblioitemnumber. $VAR1 = { 'firstname' => 'Sample', 'userid' => 'Sample.Borrower', 'categorycode' => 'PT', 'phone' => '967-1111', 'zipcode' => 'H0H 0H0', 'surname' => 'Borrower', 'email' => 'santa@northpole.com', 'city' => 'Bogusville', 'address' => '123 Bogus Street', 'branchcode' => 'LIB1' }; # Sample Borrower added. CHECK: 225 dtHtmrYqik # 79273 - LIB1 # 79274 - LIB2 ok 6 - OpacHiddenItems hid everything correctly. # 79273,79274 # Borrower number: 225 # Sample Borrower environment variables set up. ok 7 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SPT # PatronEveryBranch: SPT # ok 8 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SPT # PatronEveryBranch: SS # 79274 ok 9 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SPT # PatronEveryBranch: '' # 79274 ok 10 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SPT # PatronEveryBranch: SS|SPT # ok 11 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SPT # ok 12 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SS # 79273,79274 ok 13 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: '' # 79273,79274 ok 14 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SS|SPT # ok 15 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SPT # ok 16 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SS # 79273,79274 ok 17 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: '' # 79273,79274 ok 18 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SS|SPT # ok 19 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: SPT # ok 20 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: SS # 79274 ok 21 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: '' # 79274 ok 22 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: SS|SPT # ok All tests successful. Files=1, Tests=22, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.42 cusr 0.01 csy s = 0.45 CPU) Result: PASS
'branchcode' => 'LIB1' }; DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`kohatest`.`borrowers`, CONSTRAINT `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)) at /usr/share/koha/testclone/C4/SQLHelper.pm line 184. # Sample Borrower added. CHECK: 0 MflvoMfsof # 199954 - LIB1 # 199955 - LIB2 Mark: It seems that your tests depend on some branch codes that do not always exist. Could you take that into account? Perhaps add these branch codes in that case, and roll back at the end?
Created attachment 22655 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch.
The reason for failure was a lack of PT. I dropped my DB, did the web installer steps (only default items checked), and then tried to prove it. It failed, like M. de Rooy encountered. After adding checks in for SS, SPT, and PT patron categories into the test, all seems to work fine.
Created attachment 22660 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch.
Remember that 10589 and 10900 are dependencies for this test. This is how I tested it: mtompset@ubuntu:~/kohaclone$ git reset --hard origin/master HEAD is now at 7c521a8 Bug 8435: (follow-up) prevent t/00-load.t failing mtompset@ubuntu:~/kohaclone$ git bz apply 10900 Bug 10900 - Incorrect calling conventions accessing C4::Context 22585 - Bug 10900 - Incorrect calling conventions accessing C4::Context Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10900 - Incorrect calling conventions accessing C4::Context mtompset@ubuntu:~/kohaclone$ git bz apply 10589 Bug 10589 - Override OpacHiddenItems based on Patron Category 22629 - Bug 10589 - Override OpacHiddenItems based on Patron Category Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10589 - Override OpacHiddenItems based on Patron Category mtompset@ubuntu:~/kohaclone$ git bz apply 10895 Bug 10895 - Tests for C4::Items GetHiddenItems 22660 - Bug 10895 - Tests for C4::Items GetHiddenItems Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10895 - Tests for C4::Items GetHiddenItems mtompset@ubuntu:~/kohaclone$ prove -v t/db_dependent/GetHiddenItems_PatronCategory.t t/db_dependent/GetHiddenItems_PatronCategory.t .. 1..22 ok 1 - use C4::Items; ok 2 - LIB1 Item linked to correct biblionumber. ok 3 - LIB1 Item linked to correct biblioitemnumber. ok 4 - LIB2 Item linked to correct biblionumber. ok 5 - LIB2 Item linked to correct biblioitemnumber. $VAR1 = { 'firstname' => 'Sample', 'userid' => 'Sample.Borrower', 'categorycode' => 'PT', 'phone' => '967-1111', 'zipcode' => 'H0H 0H0', 'surname' => 'Borrower', 'email' => 'santa@northpole.com', 'city' => 'Bogusville', 'address' => '123 Bogus Street', 'branchcode' => 'LIB1' }; # Sample Borrower added. CHECK: 4 IInDuWYkdr # 7 - LIB1 # 8 - LIB2 ok 6 - OpacHiddenItems hid everything correctly. # 7,8 # Borrower number: 4 # Sample Borrower environment variables set up. ok 7 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SPT # PatronEveryBranch: SPT # ok 8 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SPT # PatronEveryBranch: SS # 8 ok 9 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SPT # PatronEveryBranch: '' # 8 ok 10 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SPT # PatronEveryBranch: SS|SPT # ok 11 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SPT # ok 12 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SS # 7,8 ok 13 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: '' # 7,8 ok 14 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS # PatronEveryBranch: SS|SPT # ok 15 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SPT # ok 16 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SS # 7,8 ok 17 - OpacHiddenItems hid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: '' # 7,8 ok 18 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: '' # PatronEveryBranch: SS|SPT # ok 19 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: SPT # ok 20 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: SS # 8 ok 21 - OpacHiddenItems hid LIB2 correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: '' # 8 ok 22 - OpacHiddenItems unhid everything correctly. # PatronSingleBranch: SS|SPT # PatronEveryBranch: SS|SPT # ok All tests successful. Files=1, Tests=22, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.40 cusr 0.02 csys = 0.45 CPU) Result: PASS mtompset@ubuntu:~/kohaclone$ ~/qa-test-tools/koha-qa.pl -v 2 -c 1 testing 1 commit(s) (applied to c9dadf4 'Bug 10589 - Override OpacHiddenItems ') OK t/db_dependent/GetHiddenItems_PatronCategory.t OK pod OK forbidden patterns OK valid OK critic mtompset@ubuntu:~/kohaclone$
Created attachment 22707 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Works as advertised. Impressive number of test cases. Needs the dependent reports. Note that these dependencies will make it harder to get everything in. I did not test the other one (10589) which might be more tricky.. Personally, I would not mind passing QA on this report, but more formally another QAer should do that ;)
(In reply to M. de Rooy from comment #17) > Note that these dependencies will make it harder to get everything in. I did > not test the other one (10589) which might be more tricky.. Personally, I > would not mind passing QA on this report, but more formally another QAer > should do that ;) 10589 should be 10900 here..
Created attachment 22811 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, unit tests pass as well
Punting to next release as it's tied to bug 10589.
This one is signed off and should be QA with 10900 and 10589.
Mark, In order to add more flexibility, don't you think it would worth to add this parameter at the patron category level? The granularity would be better and it does not seem too difficult (except that the UT should be rewrite). Could I have you opinion on this idea please?
(In reply to Jonathan Druart from comment #22) > Mark, > > In order to add more flexibility, don't you think it would worth to add this > parameter at the patron category level? > The granularity would be better and it does not seem too difficult (except > that the UT should be rewrite). > > Could I have you opinion on this idea please? In a way this is already at a patron category level, because you set the patron category code (S/PT/VOL/etc.etc.). You aren't going to have differing visibilities for every patron, but rather groups of patrons much like you have staff, volunteers, or general patrons. So though the finer granularity would be nice, I don't think the gain of finer granularity is worth a UI rework.
(In reply to Jonathan Druart from comment #21) > This one is signed off and should be QA with 10900 and 10589. http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10589#c34 Hopefully this will clarify that process.
On QAing the 3 bug reports: Please remove diag, print and the use of Dumper and YAML in t/db_dependent/GetHiddenItems_PatronCategory.t
Created attachment 31634 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, unit tests pass as well
Created attachment 31635 [details] [review] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, unit tests pass as well
Created attachment 31744 [details] [review] [PASSED QA] Bug 10895 - Tests for C4::Items GetHiddenItems Bug 10589 introduces PatronSingleBranch and PatronEveryBranch system preferences. This ensures that the visibility of two dummy records matches expectations. OpacHiddenItems is set to hide 2 records. The system preferences are then toggled between four cases ('SPT','SS','','SS|SPT') cross multipled for the two system preferences to verify the correct visibility. This will fail for a system with bug 10589 not applied, since this tests the functionality of that patch. This will fail for a system with bug 10900 not applied, since the calling convention for set_userenv was fixed in C4::Context in this patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Shouldn't 10589 be a dependency rather than a blocker?
(In reply to Kyle M Hall from comment #29) > Shouldn't 10589 be a dependency rather than a blocker? It's kind of both. These are the tests for 10589, but right now, 10589 and potentially this bug may need restructuring, because I'm going to take out the one offending line of code in 10589 and change the GetHiddenItems call to take a hash parameter. I'll move this to In Discussion, since 10589 and this are so related. If it ends up unchanged, I'll switch it back to PassedQA. If, however, it changes, I'll post the change, and change it to Needs Signoff.
(In reply to M. Tompsett from comment #30) > (In reply to Kyle M Hall from comment #29) > > Shouldn't 10589 be a dependency rather than a blocker? > > It's kind of both. These are the tests for 10589, but right now, 10589 and > potentially this bug may need restructuring, because I'm going to take out > the one offending line of code in 10589 and change the GetHiddenItems call > to take a hash parameter. > > I'll move this to In Discussion, since 10589 and this are so related. If it > ends up unchanged, I'll switch it back to PassedQA. If, however, it changes, > I'll post the change, and change it to Needs Signoff. ?
Haven't gotten back to this and bug 10589. Given the nature of the fix required for 10589, I may end up moving this to 10589, and closing this.
This may still be applicable, at least conceptually, to bug 14385.
This testing is quite outdated, and not even related to 14385 now. I will be moving any new testing to bug 10589. As such, marking this as moved.