Bug 14385 - Extend OpacHiddenItems to allow specifying exempt borrower categories
Summary: Extend OpacHiddenItems to allow specifying exempt borrower categories
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: M. Tompsett
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 19040
Blocks: 10589 20664
  Show dependency treegraph
 
Reported: 2015-06-15 04:54 UTC by Aleisha Amohia
Modified: 2018-09-18 00:22 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (21.83 KB, patch)
2015-06-16 05:25 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (21.98 KB, patch)
2016-07-19 03:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (708 bytes, patch)
2016-07-19 15:51 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (762 bytes, patch)
2016-07-20 08:10 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.03 KB, patch)
2016-07-20 09:01 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (762 bytes, patch)
2016-07-20 09:01 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (10.01 KB, patch)
2016-09-16 02:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.04 KB, patch)
2017-06-28 02:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (774 bytes, patch)
2017-06-28 02:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.42 KB, patch)
2017-06-28 02:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Added missing GetHiddenItems parameter change case (1007 bytes, patch)
2017-06-28 02:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Always use mocks, not set_preference (1.57 KB, patch)
2017-06-28 02:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes (2.03 KB, patch)
2017-06-28 02:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes (1.50 KB, patch)
2017-07-30 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak borrower category code (3.35 KB, patch)
2017-07-30 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Search was returning URLS for wrong interface (1.02 KB, patch)
2017-07-30 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Corrected issues with category code. (2.70 KB, patch)
2017-07-30 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Properly trigger all changes in C4/Search.pm (2.56 KB, patch)
2017-08-01 01:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix QA Test tool failures (1.64 KB, patch)
2017-08-01 01:40 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.09 KB, patch)
2017-08-01 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (829 bytes, patch)
2017-08-01 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.47 KB, patch)
2017-08-01 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Added missing GetHiddenItems parameter change case (1.04 KB, patch)
2017-08-01 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Always use mocks, not set_preference (1.62 KB, patch)
2017-08-01 01:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes (2.08 KB, patch)
2017-08-01 01:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes (1.55 KB, patch)
2017-08-01 01:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak borrower category code (3.40 KB, patch)
2017-08-01 01:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Search was returning URLS for wrong interface (1.08 KB, patch)
2017-08-01 01:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Corrected issues with category code. (2.75 KB, patch)
2017-08-01 01:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Properly trigger all changes in C4/Search.pm (2.61 KB, patch)
2017-08-01 01:44 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix QA Test tool failures (1.69 KB, patch)
2017-08-01 01:45 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add some commenting to make sense of logic (1.28 KB, patch)
2017-08-01 23:16 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref (7.96 KB, patch)
2017-08-01 23:16 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (1.17 KB, patch)
2017-08-01 23:16 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.11 KB, patch)
2017-08-01 23:16 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.09 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (829 bytes, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.47 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Added missing GetHiddenItems parameter change case (1.04 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Always use mocks, not set_preference (1.62 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes (2.08 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes (1.55 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak borrower category code (3.40 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Search was returning URLS for wrong interface (1.08 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Corrected issues with category code. (2.75 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Properly trigger all changes in C4/Search.pm (2.61 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix QA Test tool failures (1.69 KB, patch)
2017-08-16 15:10 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add some commenting to make sense of logic (1.28 KB, patch)
2017-08-16 15:11 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref (1.88 KB, patch)
2017-08-16 15:11 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (3.29 KB, patch)
2017-08-16 15:11 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.11 KB, patch)
2017-08-16 15:11 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref (8.12 KB, patch)
2017-08-16 15:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (1.59 KB, patch)
2017-08-16 15:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.11 KB, patch)
2017-08-16 15:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref (8.12 KB, patch)
2017-08-16 15:51 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (1.17 KB, patch)
2017-08-16 15:51 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.11 KB, patch)
2017-08-16 15:51 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (1.22 KB, patch)
2017-08-16 16:09 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.11 KB, patch)
2017-08-16 16:09 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.09 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (829 bytes, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.47 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Added missing GetHiddenItems parameter change case (1.04 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Always use mocks, not set_preference (1.62 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes (2.08 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes (1.55 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Tweak borrower category code (3.40 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Search was returning URLS for wrong interface (1.08 KB, patch)
2017-08-18 21:56 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Corrected issues with category code. (2.75 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Properly trigger all changes in C4/Search.pm (2.61 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix QA Test tool failures (1.69 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add some commenting to make sense of logic (1.28 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref (8.16 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (1.22 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.11 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Add borrower category overrides (14.63 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Modify opac-ISBDdetail and opac-MARCdetail (10.15 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix hiding in opac-tags (5.43 KB, patch)
2017-08-18 21:57 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Trigger the C4::ILSDI::Services changes (1.82 KB, patch)
2017-08-21 23:14 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Added missing 'my' (800 bytes, patch)
2017-08-22 00:29 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Test C4/Labels/Label.pm changes (5.11 KB, patch)
2017-08-22 14:50 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Improve C4::Record::marcrecord2csv test cases (1.44 KB, patch)
2017-08-22 16:47 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Corrected opac-details searchResult call (1.77 KB, patch)
2017-08-22 22:39 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix breaking issues constraint in ITerator test (967 bytes, patch)
2017-08-24 01:21 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Fix ILSDI_Services test when clubs with branch exist (962 bytes, patch)
2017-08-24 01:35 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.16 KB, patch)
2017-08-27 23:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385 Followup: C4::Borrowers not found (901 bytes, patch)
2017-08-27 23:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.54 KB, patch)
2017-08-27 23:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Added missing GetHiddenItems parameter change case (1.11 KB, patch)
2017-08-27 23:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Always use mocks, not set_preference (1.69 KB, patch)
2017-08-27 23:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Tweaks so t/db_dependent/00-strict.t passes (2.15 KB, patch)
2017-08-27 23:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Tweak t/db_dependent/Items.t to fully test changes (1.62 KB, patch)
2017-08-27 23:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Tweak borrower category code (3.47 KB, patch)
2017-08-27 23:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Search was returning URLS for wrong interface (1.15 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Corrected issues with category code. (2.82 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Properly trigger all changes in C4/Search.pm (2.68 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Fix QA Test tool failures (1.76 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Add some commenting to make sense of logic (1.35 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref (8.23 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. (1.29 KB, patch)
2017-08-27 23:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change (3.18 KB, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Add borrower category overrides (14.70 KB, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Modify opac-ISBDdetail and opac-MARCdetail (10.22 KB, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Fix hiding in opac-tags (5.50 KB, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Trigger the C4::ILSDI::Services changes (1.89 KB, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Added missing 'my' (871 bytes, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Test C4/Labels/Label.pm changes (5.18 KB, patch)
2017-08-27 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Improve C4::Record::marcrecord2csv test cases (1.51 KB, patch)
2017-08-27 23:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Corrected opac-details searchResult call (1.84 KB, patch)
2017-08-27 23:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Fix breaking issues constraint in ITerator test (1.01 KB, patch)
2017-08-27 23:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14385: Fix ILSDI_Services test when clubs with branch exist (1.01 KB, patch)
2017-08-27 23:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.15 KB, patch)
2018-01-04 01:03 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (888 bytes, patch)
2018-01-04 01:03 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.53 KB, patch)
2018-01-04 01:03 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: A lot of patches (58.46 KB, patch)
2018-01-04 01:03 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.15 KB, patch)
2018-03-05 20:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (878 bytes, patch)
2018-03-05 20:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.53 KB, patch)
2018-03-05 20:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (53.14 KB, patch)
2018-03-05 20:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (60.82 KB, patch)
2018-03-06 01:52 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (60.86 KB, patch)
2018-03-06 01:55 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (60.86 KB, patch)
2018-03-06 01:58 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (60.81 KB, patch)
2018-03-06 02:01 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (60.81 KB, patch)
2018-03-06 02:17 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (60.82 KB, patch)
2018-03-06 02:19 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (61.02 KB, patch)
2018-04-11 13:09 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.48 KB, patch)
2018-09-14 13:19 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (885 bytes, patch)
2018-09-14 13:19 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.54 KB, patch)
2018-09-14 13:19 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (61.29 KB, patch)
2018-09-14 13:19 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories (22.41 KB, patch)
2018-09-14 22:51 UTC, Joy Nelson
Details | Diff | Splinter Review
Bug 14385 Followup: C4::Borrowers not found (878 bytes, patch)
2018-09-14 22:51 UTC, Joy Nelson
Details | Diff | Splinter Review
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate (2.53 KB, patch)
2018-09-14 22:51 UTC, Joy Nelson
Details | Diff | Splinter Review
Bug 14385: Squash of a lot of patches rebased (61.14 KB, patch)
2018-09-14 22:51 UTC, Joy Nelson
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2015-06-15 04:54:26 UTC
Want to be able to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

Todo:
• C4::Items->GetHiddenItemnumbers:
** It must take an optional borrowercategory
** It must check the borrowercategory against the exception list, and if there's a match, then it doesn't mark that item as hidden.
** Update the test cases to check this.
• C4::Search->searchResults:
** $search_context should be modified to include a reference to the borrower details
** the places using $search_context will need to be updated to handle the new structure
** the borrower category will be passed on to GetHiddenItemnumbers
** update the test cases to account for this
• opac/opac-search.pl, opac/opac-detail.pl:
** Call Members::GetMember to get info on the current user, pass the category into searchResults as the search context.
• opac-MARCdetail.pl, opac/opac-detail.pl, opac/opac-ISBDdetail.pl:
** Call Members::GetMember to get info on the current user, pass the category into GetHiddenItemnumbers
*** Account for no one being logged in (i.e. don't pass the category.)
** Note: opac-MARCdetail.pl will need a bit of rearrangement to where it figures out the current user.
Comment 1 Aleisha Amohia 2015-06-16 05:25:19 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2016-01-04 07:32:50 UTC
Another one I can't sign off on, could someone please test this
Comment 3 M. Tompsett 2016-04-22 01:02:03 UTC
This reminds me of bug 10589.
Comment 4 Claire Gravely 2016-06-04 09:42:48 UTC
CONFLICT (content): Merge conflict in C4/Search.pm
Comment 5 M. Tompsett 2016-06-06 19:14:37 UTC
Also consider the refactor I'm doing on bug 16335. I just put up my filter work.
Comment 6 Aleisha Amohia 2016-07-19 03:43:48 UTC Comment hidden (obsolete)
Comment 7 Claire Gravely 2016-07-19 09:57:40 UTC
When testing I get the following Software error on the OPAC:

Can't locate C4/Borrowers.pm in @INC (you may need to install the C4::Borrowers module) (@INC contains: /home/claire/koha /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .) at /home/claire/koha/opac/opac-search.pl line 56.
BEGIN failed--compilation aborted at /home/claire/koha/opac/opac-search.pl line 56.

Claire
Comment 8 Mirko Tietgen 2016-07-19 15:51:09 UTC Comment hidden (obsolete)
Comment 9 Mirko Tietgen 2016-07-19 15:53:49 UTC
I have not tested the functionality, but the followup makes the error go away to allow testing.
Comment 10 Claire Gravely 2016-07-20 08:10:31 UTC Comment hidden (obsolete)
Comment 11 Claire Gravely 2016-07-20 09:01:08 UTC
Created attachment 53506 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Comment 12 Claire Gravely 2016-07-20 09:01:11 UTC
Created attachment 53507 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Comment 13 Jonathan Druart 2016-08-01 11:33:10 UTC
Aleisha, as Mark told you in comment 3 and comment 5, your patch conflict with bug 10589 and bug 16335.

Moreover you forgot the changes to sysprefs.sql and an atomic update file.

Mark, bug 10589 is failed QA, do you plan to work on it?
Comment 14 M. Tompsett 2016-08-01 11:43:51 UTC
*** Bug 10589 has been marked as a duplicate of this bug. ***
Comment 15 Aleisha Amohia 2016-08-02 03:30:48 UTC
(In reply to Jonathan Druart from comment #13)
> Aleisha, as Mark told you in comment 3 and comment 5, your patch conflict
> with bug 10589 and bug 16335.

I see 10589 has been closed will this one goes ahead. Mark, should we wait for you to finish your work with 16335 before finishing this one?
Comment 16 M. Tompsett 2016-08-02 08:23:45 UTC
(In reply to Aleisha Amohia from comment #15)
> (In reply to Jonathan Druart from comment #13)
> > Aleisha, as Mark told you in comment 3 and comment 5, your patch conflict
> > with bug 10589 and bug 16335.
> 
> I see 10589 has been closed will this one goes ahead. Mark, should we wait
> for you to finish your work with 16335 before finishing this one?

Go ahead with this. 16335 is going to take a long time before I get it finished.
Comment 17 M. Tompsett 2016-09-09 14:54:54 UTC
(In reply to Aleisha Amohia from comment #15)
> I see 10589 has been closed will this one goes ahead. Mark, should we wait
> for you to finish your work with 16335 before finishing this one?

Just fix the problems raised by JD in comment #13:
> Moreover you forgot the changes to sysprefs.sql and an atomic update file.

Awaiting this so I can sign off.
Comment 18 Aleisha Amohia 2016-09-16 02:43:33 UTC
Created attachment 55593 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT
Comment 19 Christopher Brannon 2017-04-08 21:02:29 UTC
vagrant@kohadevbox:kohaclone(bug_14385)$ git bz apply 14385
Bug 14385 - Extend OpacHiddenItems to allow specifying exempt borrower categories

53506 - Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories
53507 - Bug 14385 Followup: C4::Borrowers not found
55593 - Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories
Using index info to reconstruct a base tree...
M       C4/Items.pm
M       C4/Search.pm
M       catalogue/search.pl
M       cataloguing/addbooks.pl
M       koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
M       opac/opac-ISBDdetail.pl
M       opac/opac-MARCdetail.pl
M       opac/opac-detail.pl
M       opac/opac-search.pl
M       t/db_dependent/Items.t
M       t/db_dependent/Search.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Search.t
Auto-merging t/db_dependent/Items.t
Auto-merging opac/opac-search.pl
Auto-merging opac/opac-detail.pl
Auto-merging opac/opac-MARCdetail.pl
Auto-merging opac/opac-ISBDdetail.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging cataloguing/addbooks.pl
Auto-merging catalogue/search.pl
Auto-merging C4/Search.pm
Auto-merging C4/Items.pm
Applying: Bug 14385 Followup: C4::Borrowers not found
Using index info to reconstruct a base tree...
M       opac/opac-search.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-search.pl
Applying: Bug 14385: Followup: changes to sysprefs.sql and atomicupdate
Using index info to reconstruct a base tree...
M       installer/data/mysql/sysprefs.sql
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/sysprefs.sql
Comment 20 Christopher Brannon 2017-04-08 21:21:31 UTC
Not sure if the above means that the apply failed, but I did see changed files.  Tried this out, but the exception is not working.  Item is still hidden.
Comment 21 M. Tompsett 2017-05-31 14:14:14 UTC
Is anyone going to try to resurrect this? If not, I'll take a stab at it, because this is the LAST set of patches, I need to free myself from the shackles of customized, unsupported code.
Comment 22 M. Tompsett 2017-06-27 16:29:06 UTC
Found a missing C4/Biblio.pm modification... Still looking further.
Comment 23 M. Tompsett 2017-06-28 00:03:01 UTC
Taking based on conversation with Aleisha on IRC. I think this is a cleaner start to the filtering that 10589 was trying to do. I'll be reopening that one after this.
Comment 24 M. Tompsett 2017-06-28 02:56:50 UTC
Created attachment 64688 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Comment 25 M. Tompsett 2017-06-28 02:56:59 UTC
Created attachment 64689 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Comment 26 M. Tompsett 2017-06-28 02:57:05 UTC
Created attachment 64690 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT
Comment 27 M. Tompsett 2017-06-28 02:57:14 UTC
Created attachment 64691 [details] [review]
Bug 14385: Added missing GetHiddenItems parameter change case

Without this prove t had a failure.
Comment 28 M. Tompsett 2017-06-28 02:57:21 UTC
Created attachment 64692 [details] [review]
Bug 14385: Always use mocks, not set_preference
Comment 29 M. Tompsett 2017-06-28 02:57:31 UTC
Created attachment 64693 [details] [review]
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes

There was a typo botcat vs borcat and borrowernumber was never
defined. Grabbing from userenv, like other code does.
Comment 30 M. Tompsett 2017-07-14 02:09:40 UTC
Bug 17829 means the C4::Member::GetMember logic here doesn't work. Tweaking.
Comment 31 M. Tompsett 2017-07-29 03:15:58 UTC
Perhaps when I have a more solid block of time, I'll be able to debug why the opac search results give links to the staff client. DOH! Still things to fix.
Comment 32 M. Tompsett 2017-07-30 01:38:47 UTC
Good progress. Just need to finalize testing the C4/Search.pm changes.
Here is the test plan thus far.

prove -v t/db_dependent/Items.t
C4::Biblio changes (EmbedItemsInMarcBiblio) tested
C4::Items changes (GetHiddentItemnumbers) tested
the Items.t is tested itself too.

perldoc C4::Items
/PERLDOC
-- finds changes

installer/data/mysql/atomicupdate/bug_14385-add_OpacHiddenItemsExceptions_syspref.sql
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
-- run an upgrade
-- log in to staff client
-- home -> koha administration -> global system preferences -> opac
-- should be an added OpacHiddenItemsExceptions under OpacHiddenItems
   in the Policy section.
-- should be able to save various values.

installer/data/mysql/sysprefs.sql
-- drop the db
-- create the db
-- fresh install
-- log in to staff client
-- home -> koha administration -> global system preferences -> opac
-- should be an added OpacHiddenItemsExceptions under OpacHiddenItems
   in the Policy section.
-- should be able to save various values.

opac/opac-search.pl
-- set the OpacHiddenItems to "itype: [BK]"
-- set the OpacHiddenItemsExceptions to "S"
-- make sure to catalogue at least two books, since you dropped everything.
-- Adding from Z39.50 is easiest.
-- do a full reindex
-- in the OPAC, search for "itype:BK"
-- there should be nothing found.
-- log in as the superlibrarian (which you probably set to S for staff)
-- in the OPAC, search for "itype:BK"
-- there should be two entries
-- change OpacHiddenItemsExceptions to "PT"
   (because your superlibrarian is probably S and not PT)
-- in the OPAC, search for "itype:BK"
-- there should be nothing found.

opac/opac-detail.pl
-- As you know there are two books, try a direct URL:
   {OPACBaseURL}/opac-detail.pl?biblionumber=1
-- it should return a 404 page
-- change OpacHiddenItemsExceptions back to "S"
-- try the direct URL again.
-- it should show up.

opac/opac-MARCdetail.pl
-- change the OpacHiddenItemsExceptions back to "PT"
-- try a direct URL:
   {OPACBaseURL}/opac-MARCdetail.pl?biblionumber=1
-- it should return a 404 page
-- change OpacHiddenItemsExceptions back to "S"
-- try the direct URL again.
-- it should show up.

opac/opac-ISBDdetail.pl
-- change the OpacHiddenItemsExceptions back to "PT"
-- try a direct URL:
   {OPACBaseURL}/opac-ISBDdetail.pl?biblionumber=1
-- it should return a 404 page
-- change OpacHiddenItemsExceptions back to "S"
-- try the direct URL again.
-- it should show up.

prove -v t/db_dependent/Search.t
the Search.t itself is proved.

catalogue/search.pl
-- Log in to the staff client
-- search for "itype:BK"
-- two results
-- change OpacHiddenItemsExceptions and OpacHiddenItems as desired
-- two results all the time, because this is the staff client!

cataloguing/addbooks.pl
-- Make sure you have records in the cataloguing reservoir
   This is done by staging at least one MARC record.
-- In the staff client, home -> Cataloguing -> Cataloguing search
-- Search for something that will find some records in the reservoir
-- Make sure results are expected regardless of hidden item
   system preferences.
Comment 33 M. Tompsett 2017-07-30 01:43:11 UTC
Created attachment 65322 [details] [review]
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes

This will test all the if structures fully in GetHiddenItemnumbers.
prove t/db_dependent/Items.t
Comment 34 M. Tompsett 2017-07-30 01:43:15 UTC
Created attachment 65323 [details] [review]
Bug 14385: Tweak borrower category code

$borrower->{categorycode} on a Koha::Patron is not the
same as $borrower->categorycode. Fixed error.
Comment 35 M. Tompsett 2017-07-30 01:43:21 UTC
Created attachment 65324 [details] [review]
Bug 14385: Search was returning URLS for wrong interface

There was one search context place wrong. Changed it to $is_opac
as the logic for setting $is_opac was modified correctly.
Comment 36 M. Tompsett 2017-07-30 01:43:25 UTC
Created attachment 65325 [details] [review]
Bug 14385: Corrected issues with category code.

When a user isn't logged in, $borrower is undef and causes error
when determining category code. Added conditional check.
Comment 37 M. Tompsett 2017-08-01 01:36:35 UTC
Created attachment 65376 [details] [review]
Bug 14385: Properly trigger all changes in C4/Search.pm

prove t/db_dependent/Search.t
Comment 38 M. Tompsett 2017-08-01 01:40:10 UTC
Created attachment 65377 [details] [review]
Bug 14385: Fix QA Test tool failures

C4/Search.pm had some tabs.
Comment 39 M. Tompsett 2017-08-01 01:43:20 UTC
Created attachment 65378 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 40 M. Tompsett 2017-08-01 01:43:31 UTC
Created attachment 65379 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 41 M. Tompsett 2017-08-01 01:43:40 UTC
Created attachment 65380 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 42 M. Tompsett 2017-08-01 01:43:50 UTC
Created attachment 65381 [details] [review]
Bug 14385: Added missing GetHiddenItems parameter change case

Without this prove t had a failure.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 43 M. Tompsett 2017-08-01 01:43:59 UTC
Created attachment 65382 [details] [review]
Bug 14385: Always use mocks, not set_preference

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 44 M. Tompsett 2017-08-01 01:44:07 UTC
Created attachment 65383 [details] [review]
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes

There was a typo botcat vs borcat and borrowernumber was never
defined. Grabbing from userenv, like other code does.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 45 M. Tompsett 2017-08-01 01:44:15 UTC
Created attachment 65384 [details] [review]
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes

This will test all the if structures fully in GetHiddenItemnumbers.
prove t/db_dependent/Items.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 46 M. Tompsett 2017-08-01 01:44:23 UTC
Created attachment 65385 [details] [review]
Bug 14385: Tweak borrower category code

$borrower->{categorycode} on a Koha::Patron is not the
same as $borrower->categorycode. Fixed error.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 47 M. Tompsett 2017-08-01 01:44:38 UTC
Created attachment 65386 [details] [review]
Bug 14385: Search was returning URLS for wrong interface

There was one search context place wrong. Changed it to $is_opac
as the logic for setting $is_opac was modified correctly.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 48 M. Tompsett 2017-08-01 01:44:46 UTC
Created attachment 65387 [details] [review]
Bug 14385: Corrected issues with category code.

When a user isn't logged in, $borrower is undef and causes error
when determining category code. Added conditional check.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 49 M. Tompsett 2017-08-01 01:44:53 UTC
Created attachment 65388 [details] [review]
Bug 14385: Properly trigger all changes in C4/Search.pm

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 50 M. Tompsett 2017-08-01 01:45:04 UTC
Created attachment 65389 [details] [review]
Bug 14385: Fix QA Test tool failures

C4/Search.pm had some tabs.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 51 M. Tompsett 2017-08-01 01:50:09 UTC
Follow the test plan in comment #32 and add a prove t/db_dependent/Search.t to complete it. :)

Run QA Test tools, and all should be good.

Read the code, make sure all the searchResults() and GetHiddenItemnumbers() calls were tweaked with the new parameters.

Thanks to Aleisha Amohia for dealing with the searchResult() issues that I had failed to handle in my previous attempt on bug 10589, which will now become dependent on this patch, as I seek to add PatronEveryBranch (which this is), vs. PatronSingleBranch (which will be added).

I have signed off all the code (including Catalyst's), and so if anyone else could sign off all the code, I would consider this signed off.
Comment 52 M. Tompsett 2017-08-01 17:33:55 UTC
*sigh* found an issue. EmbedItemsInMarcBiblio.
If $opac, we should be passing borcat.
Comment 53 M. Tompsett 2017-08-01 23:16:20 UTC
Created attachment 65408 [details] [review]
Bug 14385: Add some commenting to make sense of logic
Comment 54 M. Tompsett 2017-08-01 23:16:26 UTC
Created attachment 65409 [details] [review]
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref
Comment 55 M. Tompsett 2017-08-01 23:16:32 UTC
Created attachment 65410 [details] [review]
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t
Comment 56 M. Tompsett 2017-08-01 23:16:39 UTC
Created attachment 65411 [details] [review]
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change
Comment 57 M. Tompsett 2017-08-02 00:42:25 UTC
The problem with this, as it stands is the GetMarcBiblio call does not receive the patron category code, so it can't pass it to EmbedItemsInMarcBiblio which can't then pass it to GetHiddenItemnumbers. And while I'm tempted to hack in a user category code and branch lookup, that would defeat the niceness of the refactored parameter passing thus far.
Comment 58 M. Tompsett 2017-08-04 02:15:42 UTC
To test all the EmbedItemsInMarcBiblio calls:
 prove t/db_dependent/Biblio.t
 prove t/db_dependent/Record/marcrecord2csv.t
 prove t/db_dependent/Koha/BiblioUtils/Iterator.t
 prove t/db_dependent/Exporter/Record.t
 ./misc/migration_tools/build_oai_sets.pl -i
 And catalog any new item in the staff client. I did a new from Z39.50.
Comment 59 M. Tompsett 2017-08-11 01:44:35 UTC
(In reply to M. Tompsett from comment #57)
> The problem with this, as it stands is the GetMarcBiblio call does not
> receive the patron category code, so it can't pass it to
> EmbedItemsInMarcBiblio which can't then pass it to GetHiddenItemnumbers. And
> while I'm tempted to hack in a user category code and branch lookup, that
> would defeat the niceness of the refactored parameter passing thus far.

And my attempt to start the refactor on bug 19040 is proving painful for having a full trigger all the changes test plan.

Options:
1) Push 19040 seeing it eyeballs perfectly well, and I continue on fixing here.
2) Don't push 19040, but push this knowing this one flaw.
   -- it is basically the search results look weird, because they are semi-hidden.
3) Hack the lookup, which could really slow code down.
4) Give up.

I'd appreciate feedback.
Comment 60 M. Tompsett 2017-08-16 15:10:10 UTC
Created attachment 66103 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 61 M. Tompsett 2017-08-16 15:10:15 UTC
Created attachment 66104 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 62 M. Tompsett 2017-08-16 15:10:19 UTC
Created attachment 66105 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 63 M. Tompsett 2017-08-16 15:10:24 UTC
Created attachment 66106 [details] [review]
Bug 14385: Added missing GetHiddenItems parameter change case

Without this prove t had a failure.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 64 M. Tompsett 2017-08-16 15:10:28 UTC
Created attachment 66107 [details] [review]
Bug 14385: Always use mocks, not set_preference

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 65 M. Tompsett 2017-08-16 15:10:32 UTC
Created attachment 66108 [details] [review]
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes

There was a typo botcat vs borcat and borrowernumber was never
defined. Grabbing from userenv, like other code does.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 66 M. Tompsett 2017-08-16 15:10:37 UTC
Created attachment 66109 [details] [review]
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes

This will test all the if structures fully in GetHiddenItemnumbers.
prove t/db_dependent/Items.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 67 M. Tompsett 2017-08-16 15:10:41 UTC
Created attachment 66110 [details] [review]
Bug 14385: Tweak borrower category code

$borrower->{categorycode} on a Koha::Patron is not the
same as $borrower->categorycode. Fixed error.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 68 M. Tompsett 2017-08-16 15:10:45 UTC
Created attachment 66111 [details] [review]
Bug 14385: Search was returning URLS for wrong interface

There was one search context place wrong. Changed it to $is_opac
as the logic for setting $is_opac was modified correctly.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 69 M. Tompsett 2017-08-16 15:10:50 UTC
Created attachment 66112 [details] [review]
Bug 14385: Corrected issues with category code.

When a user isn't logged in, $borrower is undef and causes error
when determining category code. Added conditional check.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 70 M. Tompsett 2017-08-16 15:10:54 UTC
Created attachment 66113 [details] [review]
Bug 14385: Properly trigger all changes in C4/Search.pm

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 71 M. Tompsett 2017-08-16 15:10:59 UTC
Created attachment 66114 [details] [review]
Bug 14385: Fix QA Test tool failures

C4/Search.pm had some tabs.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 72 M. Tompsett 2017-08-16 15:11:03 UTC
Created attachment 66115 [details] [review]
Bug 14385: Add some commenting to make sense of logic
Comment 73 M. Tompsett 2017-08-16 15:11:07 UTC
Created attachment 66116 [details] [review]
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref
Comment 74 M. Tompsett 2017-08-16 15:11:12 UTC
Created attachment 66117 [details] [review]
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t
Comment 75 M. Tompsett 2017-08-16 15:11:16 UTC
Created attachment 66118 [details] [review]
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change
Comment 76 M. Tompsett 2017-08-16 15:43:31 UTC
Created attachment 66125 [details] [review]
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref
Comment 77 M. Tompsett 2017-08-16 15:43:35 UTC
Created attachment 66126 [details] [review]
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t
Comment 78 M. Tompsett 2017-08-16 15:43:40 UTC
Created attachment 66127 [details] [review]
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change
Comment 79 M. Tompsett 2017-08-16 15:51:08 UTC
Created attachment 66128 [details] [review]
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref
Comment 80 M. Tompsett 2017-08-16 15:51:12 UTC
Created attachment 66129 [details] [review]
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t
Comment 81 M. Tompsett 2017-08-16 15:51:16 UTC
Created attachment 66130 [details] [review]
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change
Comment 82 M. Tompsett 2017-08-16 16:09:18 UTC
Created attachment 66131 [details] [review]
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t
Comment 83 M. Tompsett 2017-08-16 16:09:22 UTC
Created attachment 66132 [details] [review]
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change
Comment 84 M. Tompsett 2017-08-18 21:56:14 UTC
Created attachment 66192 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 85 M. Tompsett 2017-08-18 21:56:20 UTC
Created attachment 66193 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 86 M. Tompsett 2017-08-18 21:56:26 UTC
Created attachment 66194 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 87 M. Tompsett 2017-08-18 21:56:33 UTC
Created attachment 66195 [details] [review]
Bug 14385: Added missing GetHiddenItems parameter change case

Without this prove t had a failure.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 88 M. Tompsett 2017-08-18 21:56:38 UTC
Created attachment 66196 [details] [review]
Bug 14385: Always use mocks, not set_preference

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 89 M. Tompsett 2017-08-18 21:56:42 UTC
Created attachment 66197 [details] [review]
Bug 14385: Tweaks so t/db_dependent/00-strict.t passes

There was a typo botcat vs borcat and borrowernumber was never
defined. Grabbing from userenv, like other code does.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 90 M. Tompsett 2017-08-18 21:56:46 UTC
Created attachment 66198 [details] [review]
Bug 14385: Tweak t/db_dependent/Items.t to fully test changes

This will test all the if structures fully in GetHiddenItemnumbers.
prove t/db_dependent/Items.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 91 M. Tompsett 2017-08-18 21:56:50 UTC
Created attachment 66199 [details] [review]
Bug 14385: Tweak borrower category code

$borrower->{categorycode} on a Koha::Patron is not the
same as $borrower->categorycode. Fixed error.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 92 M. Tompsett 2017-08-18 21:56:56 UTC
Created attachment 66200 [details] [review]
Bug 14385: Search was returning URLS for wrong interface

There was one search context place wrong. Changed it to $is_opac
as the logic for setting $is_opac was modified correctly.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 93 M. Tompsett 2017-08-18 21:57:00 UTC
Created attachment 66201 [details] [review]
Bug 14385: Corrected issues with category code.

When a user isn't logged in, $borrower is undef and causes error
when determining category code. Added conditional check.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 94 M. Tompsett 2017-08-18 21:57:05 UTC
Created attachment 66202 [details] [review]
Bug 14385: Properly trigger all changes in C4/Search.pm

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 95 M. Tompsett 2017-08-18 21:57:09 UTC
Created attachment 66203 [details] [review]
Bug 14385: Fix QA Test tool failures

C4/Search.pm had some tabs.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 96 M. Tompsett 2017-08-18 21:57:13 UTC
Created attachment 66204 [details] [review]
Bug 14385: Add some commenting to make sense of logic
Comment 97 M. Tompsett 2017-08-18 21:57:19 UTC
Created attachment 66205 [details] [review]
Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref
Comment 98 M. Tompsett 2017-08-18 21:57:24 UTC
Created attachment 66206 [details] [review]
Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t
Comment 99 M. Tompsett 2017-08-18 21:57:29 UTC
Created attachment 66207 [details] [review]
Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change
Comment 100 M. Tompsett 2017-08-18 21:57:34 UTC
Created attachment 66208 [details] [review]
Bug 14385: Add borrower category overrides

These files generally add borcat parameter to GetMarcBiblio.
Others might include correction of filtering of items
(opac-basket), or a comment as to why no changes were done
(opac-search).

In the case of opac-search, correcting the first FIXME will
likely correct the OpacHiddenItems issues on tags. As such,
that is beyond this bugs scope.

Some code had loop optimizations and fixes made, like a
'next unless $record' when the biblio shouldn't even be in
the list.
Comment 101 M. Tompsett 2017-08-18 21:57:38 UTC
Created attachment 66209 [details] [review]
Bug 14385: Modify opac-ISBDdetail and opac-MARCdetail

Both files had similar logic. They were rearranged and
optimized, so that both files would have practically identical
initial blocks of code.

Optimizations were possible, because GetMarcBiblio
returns a filtered record, so that there is no double call
(once in the opac-### file and once in GetMarcBiblio) to
GetHiddenItemnumbers.
Comment 102 M. Tompsett 2017-08-18 21:57:42 UTC
Created attachment 66210 [details] [review]
Bug 14385: Fix hiding in opac-tags

opac/opac-tags.pl was not properly hiding.

There is currently one known bug associated with tags left.
If you have two biblios tagged by different people with the
same tag, the opac-search will show the one you tagged that
is supposed to be hidden, because tag searches work differently
than regular searches. This is beyond the scope of this bug.
See the FIXME's in opac/opac-search.pl
Comment 103 M. Tompsett 2017-08-18 22:09:18 UTC
Comment 51 and 58 constitute the most recent test plan. A more complete single one to come soon.
Comment 104 M. Tompsett 2017-08-21 17:43:23 UTC
I would very much appreciate testing and feedback, while I attempt to make a more complete test plan.

Apply 19040 first.
Then this. Then test.
Comment 105 M. Tompsett 2017-08-21 23:14:57 UTC
Created attachment 66302 [details] [review]
Bug 14385: Trigger the C4::ILSDI::Services changes

prove t/db_dependent/ILSDI_Services.t
Comment 106 M. Tompsett 2017-08-22 00:29:48 UTC
Created attachment 66303 [details] [review]
Bug 14385: Added missing 'my'
Comment 107 M. Tompsett 2017-08-22 14:50:59 UTC
Created attachment 66347 [details] [review]
Bug 14385: Test C4/Labels/Label.pm changes
Comment 108 M. Tompsett 2017-08-22 16:47:17 UTC
Created attachment 66361 [details] [review]
Bug 14385: Improve C4::Record::marcrecord2csv test cases
Comment 109 M. Tompsett 2017-08-22 22:39:37 UTC
Created attachment 66372 [details] [review]
Bug 14385: Corrected opac-details searchResult call
Comment 110 M. Tompsett 2017-08-24 01:21:01 UTC
Created attachment 66401 [details] [review]
Bug 14385: Fix breaking issues constraint in ITerator test
Comment 111 M. Tompsett 2017-08-24 01:35:47 UTC
Created attachment 66403 [details] [review]
Bug 14385: Fix ILSDI_Services test when clubs with branch exist
Comment 112 M. Tompsett 2017-08-24 03:04:47 UTC
back up database
git bz apply 19040
git bz apply 14385
sudo koha-shell -c bash kohadev
./installer/data/misc/updatedatabase.pl
-- this proves the atomic update

drop the database
create an empty one
run a web install
-- this proves the sysprefs changes

restore database
./installer/data/misc/updatedatabase.pl
-- This gives non-fresh install DB to actually test on.

log into the staff client
go to the OPAC system preferences (Home -> Koha administration -> Global System Preferences -> OPAC)
scroll down to the policy section
-- there should be a new OpacHiddenItemsExpceptions system preference setting.
Set a value for the OpacHiddenItemsExceptions, and click save.
-- it should save.
Click 'Home' in the bread crumbs.
Navigate back to the OPAC system preferences
-- the value you set should be saved correctly.
-- This confirms the opac.pref file

sudo koha-shell -c bash kohadev
prove t/db_dependent/Authorities/Merge.t
-- This proves the C4/AuthoritiesMarc.pm change

prove t/db_dependent/HoldsQueue.t
-- This proves the C4/HoldsQueue.pm change

prove t/db_dependent/ILSDI_Services.t
-- This proves the C4/ILSDI/Services.pm change

prove t/db_dependent/Reserves.t t/db_dependent/Items.t
-- This prove the C4/Items.pm changes

prove t/db_dependent/Labels/t_Label.t
-- This proves the C4/Labels/Label.pm changes

prove t/db_dependent/Record/marcrecord2csv.t
-- This proves the C4/Record.pm changes

prove t/db_dependent/Serials.t
-- This proves the C4/Serials.pm change

prove t/db_dependent/ShelfBrowser.t
-- This proves the C4/ShelfBrowser.pm change

prove t/db_dependent/Koha/BiblioUtils.t
-- This proves the Koha/Biblio.pm change

prove t/db_dependent/Koha/BiblioUtils/Iterator.t
-- This proves the Koha/BiblioUtils/Iterator.pm change

prove t/db_dependent/Exporter/Record.t
-- This proves the Koha/Exporter/Record.pm changes

prove t/db_dependent/OAI/Server.t
-- This proves the Koha/OAI/Server/Repository.pm change

prove `git diff origin/master | grep ^diff | grep [.]t$ | cut -f4 -d' ' | sed -e "s#^..##"`
-- This proves all the test file changes.

./misc/migration_tools/build_oai_sets.pl -i
-- This proves this file change.

prove t/db_dependent/Search.t
-- This triggers searchResults tweaks in this file.

- Catalog any new item in the staff client. I did a new from Z39.50.
-- This will trigger any remaining EmbedItemsInMarcBiblio
   calls in the staff client.

1) Make two borrowers, one in a category that should see everything (ie Staff [S]), and another in a category that should only see certain things (ie Patron [PT])
2) Add the borrower that can see everything (the Staff) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Patron)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref.
   -- No records should show for this borrower as this item type is hidden to them.
6) Log out, and repeat the search
   -- No records should show
7) Log in as the borrower that should see everything (Staff)
8) Do the same search.
   -- There should be results from this search, as this borrower category has been specified as an exception to the hidden items

prove -v t/db_dependent/Items.t
C4::Biblio changes (EmbedItemsInMarcBiblio) tested
C4::Items changes (GetHiddentItemnumbers) tested
the Items.t is tested itself too.

perldoc C4::Items
/PERLDOC
-- finds changes

installer/data/mysql/atomicupdate/bug_14385-add_OpacHiddenItemsExceptions_syspref.sql
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
-- run an upgrade
-- log in to staff client
-- home -> koha administration -> global system preferences -> opac
-- should be an added OpacHiddenItemsExceptions under OpacHiddenItems
   in the Policy section.
-- should be able to save various values.

installer/data/mysql/sysprefs.sql
-- drop the db
-- create the db
-- fresh install
-- log in to staff client
-- home -> koha administration -> global system preferences -> opac
-- should be an added OpacHiddenItemsExceptions under OpacHiddenItems
   in the Policy section.
-- should be able to save various values.

opac/opac-search.pl
-- set the OpacHiddenItems to "itype: [BK]"
-- set the OpacHiddenItemsExceptions to "S"
-- make sure to catalogue at least two books, since you dropped everything.
-- Adding from Z39.50 is easiest.
-- do a full reindex
-- in the OPAC, search for "itype:BK"
-- there should be nothing found.
-- log in as the superlibrarian (which you probably set to S for staff)
-- in the OPAC, search for "itype:BK"
-- there should be two entries
-- change OpacHiddenItemsExceptions to "PT"
   (because your superlibrarian is probably S and not PT)
-- in the OPAC, search for "itype:BK"
-- there should be nothing found.

opac/opac-detail.pl
-- As you know there are two books, try a direct URL:
   {OPACBaseURL}/opac-detail.pl?biblionumber=1
-- it should return a 404 page
-- change OpacHiddenItemsExceptions back to "S"
-- try the direct URL again.
-- it should show up.

opac/opac-MARCdetail.pl
-- change the OpacHiddenItemsExceptions back to "PT"
-- try a direct URL:
   {OPACBaseURL}/opac-MARCdetail.pl?biblionumber=1
-- it should return a 404 page
-- change OpacHiddenItemsExceptions back to "S"
-- try the direct URL again.
-- it should show up.

opac/opac-ISBDdetail.pl
-- change the OpacHiddenItemsExceptions back to "PT"
-- try a direct URL:
   {OPACBaseURL}/opac-ISBDdetail.pl?biblionumber=1
-- it should return a 404 page
-- change OpacHiddenItemsExceptions back to "S"
-- try the direct URL again.
-- it should show up.

prove -v t/db_dependent/Search.t
the Search.t itself is proved.

catalogue/search.pl
-- Log in to the staff client
-- search for "itype:BK"
-- two results
-- change OpacHiddenItemsExceptions and OpacHiddenItems as desired
-- two results all the time, because this is the staff client!

cataloguing/addbooks.pl
-- Make sure you have records in the cataloguing reservoir
   This is done by staging at least one MARC record.
-- In the staff client, home -> Cataloguing -> Cataloguing search
-- Search for something that will find some records in the reservoir
-- Make sure results are expected regardless of hidden item
   system preferences.

To test all the EmbedItemsInMarcBiblio calls:
 prove t/db_dependent/Biblio.t
 prove t/db_dependent/Record/marcrecord2csv.t
 prove t/db_dependent/Koha/BiblioUtils/Iterator.t
 prove t/db_dependent/Exporter/Record.t
 ./misc/migration_tools/build_oai_sets.pl -i
 And catalog any new item in the staff client. I did a new from Z39.50.

 
This part of the testing is reading code and pondering if it makes sense.

git grep searchResults
-- The lines are as follows:
C4/Search.pm
- Export line
- Perldoc 2 lines
- subroutine
catalogue/search.pl
- call has interface => 'intranet' and no borrower category
cataloguing/addbooks.pl
- call has interface => 'intranet' and no borrower category
opac/opac-detail.pl
- call as variable $search_context which is
  { 'interface' => 'opac', 'category' => $borcat }
  $borcat is defined at the beginning of the program
opac/opac-search.pl
- comment
- function calls use $search_context which is defined
  before the servers loop
t/db_dependent/Search.t
- Doesn't test hiding in searchResults.

git grep GetHiddenItemnumbers
-- the lines are as follows:
C4/Biblio.pm
- a single line triggered by prove t/db_dependent/Items.t
C4/Items.pm
- Export line
- Perldoc 2 lines
- subroutine

... More explicit to come later. searchResults, GetHiddenItemnumbers, EmbedItemsInMarcBiblio, GetMarcBiblio all had major prototype changes. Grep for these and confirm the changes reflect the use of a hashref either completely (GetMarcBiblio) or as a parameter (searchResults).
Comment 113 M. Tompsett 2017-08-25 01:45:53 UTC
comment #112 represents the latest test plan that I followed. I'm calling this done. Multiple eyes and sign offs on this would be appreciated.
Comment 114 Aleisha Amohia 2017-08-27 23:40:43 UTC
Created attachment 66520 [details] [review]
[SIGNED-OFF] Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 115 Aleisha Amohia 2017-08-27 23:40:55 UTC
Created attachment 66521 [details] [review]
[SIGNED-OFF] Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 116 Aleisha Amohia 2017-08-27 23:41:04 UTC
Created attachment 66522 [details] [review]
[SIGNED-OFF] Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 117 Aleisha Amohia 2017-08-27 23:41:14 UTC
Created attachment 66523 [details] [review]
[SIGNED-OFF] Bug 14385: Added missing GetHiddenItems parameter change case

Without this prove t had a failure.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 118 Aleisha Amohia 2017-08-27 23:41:22 UTC
Created attachment 66524 [details] [review]
[SIGNED-OFF] Bug 14385: Always use mocks, not set_preference

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 119 Aleisha Amohia 2017-08-27 23:41:32 UTC
Created attachment 66525 [details] [review]
[SIGNED-OFF] Bug 14385: Tweaks so t/db_dependent/00-strict.t passes

There was a typo botcat vs borcat and borrowernumber was never
defined. Grabbing from userenv, like other code does.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 120 Aleisha Amohia 2017-08-27 23:41:41 UTC
Created attachment 66526 [details] [review]
[SIGNED-OFF] Bug 14385: Tweak t/db_dependent/Items.t to fully test changes

This will test all the if structures fully in GetHiddenItemnumbers.
prove t/db_dependent/Items.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 121 Aleisha Amohia 2017-08-27 23:41:50 UTC
Created attachment 66527 [details] [review]
[SIGNED-OFF] Bug 14385: Tweak borrower category code

$borrower->{categorycode} on a Koha::Patron is not the
same as $borrower->categorycode. Fixed error.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 122 Aleisha Amohia 2017-08-27 23:42:00 UTC
Created attachment 66528 [details] [review]
[SIGNED-OFF] Bug 14385: Search was returning URLS for wrong interface

There was one search context place wrong. Changed it to $is_opac
as the logic for setting $is_opac was modified correctly.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 123 Aleisha Amohia 2017-08-27 23:42:09 UTC
Created attachment 66529 [details] [review]
[SIGNED-OFF] Bug 14385: Corrected issues with category code.

When a user isn't logged in, $borrower is undef and causes error
when determining category code. Added conditional check.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 124 Aleisha Amohia 2017-08-27 23:42:18 UTC
Created attachment 66530 [details] [review]
[SIGNED-OFF] Bug 14385: Properly trigger all changes in C4/Search.pm

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 125 Aleisha Amohia 2017-08-27 23:42:27 UTC
Created attachment 66531 [details] [review]
[SIGNED-OFF] Bug 14385: Fix QA Test tool failures

C4/Search.pm had some tabs.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 126 Aleisha Amohia 2017-08-27 23:42:35 UTC
Created attachment 66532 [details] [review]
[SIGNED-OFF] Bug 14385: Add some commenting to make sense of logic

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 127 Aleisha Amohia 2017-08-27 23:42:43 UTC
Created attachment 66533 [details] [review]
[SIGNED-OFF] Bug 14385: Refactor EmbedItemsInMarcBiblio parameters to hashref

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 128 Aleisha Amohia 2017-08-27 23:42:51 UTC
Created attachment 66534 [details] [review]
[SIGNED-OFF] Bug 14385: Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.

prove t/db_dependent/Items.t

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 129 Aleisha Amohia 2017-08-27 23:43:03 UTC
Created attachment 66535 [details] [review]
[SIGNED-OFF] Bug 14385: Add missing test to trigger Koha/BiblioUtils/Iterator change

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 130 Aleisha Amohia 2017-08-27 23:43:13 UTC
Created attachment 66536 [details] [review]
[SIGNED-OFF] Bug 14385: Add borrower category overrides

These files generally add borcat parameter to GetMarcBiblio.
Others might include correction of filtering of items
(opac-basket), or a comment as to why no changes were done
(opac-search).

In the case of opac-search, correcting the first FIXME will
likely correct the OpacHiddenItems issues on tags. As such,
that is beyond this bugs scope.

Some code had loop optimizations and fixes made, like a
'next unless $record' when the biblio shouldn't even be in
the list.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 131 Aleisha Amohia 2017-08-27 23:43:22 UTC
Created attachment 66537 [details] [review]
[SIGNED-OFF] Bug 14385: Modify opac-ISBDdetail and opac-MARCdetail

Both files had similar logic. They were rearranged and
optimized, so that both files would have practically identical
initial blocks of code.

Optimizations were possible, because GetMarcBiblio
returns a filtered record, so that there is no double call
(once in the opac-### file and once in GetMarcBiblio) to
GetHiddenItemnumbers.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 132 Aleisha Amohia 2017-08-27 23:43:31 UTC
Created attachment 66538 [details] [review]
[SIGNED-OFF] Bug 14385: Fix hiding in opac-tags

opac/opac-tags.pl was not properly hiding.

There is currently one known bug associated with tags left.
If you have two biblios tagged by different people with the
same tag, the opac-search will show the one you tagged that
is supposed to be hidden, because tag searches work differently
than regular searches. This is beyond the scope of this bug.
See the FIXME's in opac/opac-search.pl

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 133 Aleisha Amohia 2017-08-27 23:43:40 UTC
Created attachment 66539 [details] [review]
[SIGNED-OFF] Bug 14385: Trigger the C4::ILSDI::Services changes

prove t/db_dependent/ILSDI_Services.t

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 134 Aleisha Amohia 2017-08-27 23:43:49 UTC
Created attachment 66540 [details] [review]
[SIGNED-OFF] Bug 14385: Added missing 'my'

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 135 Aleisha Amohia 2017-08-27 23:43:57 UTC
Created attachment 66541 [details] [review]
[SIGNED-OFF] Bug 14385: Test C4/Labels/Label.pm changes

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 136 Aleisha Amohia 2017-08-27 23:44:07 UTC
Created attachment 66542 [details] [review]
[SIGNED-OFF] Bug 14385: Improve C4::Record::marcrecord2csv test cases

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 137 Aleisha Amohia 2017-08-27 23:44:16 UTC
Created attachment 66543 [details] [review]
[SIGNED-OFF] Bug 14385: Corrected opac-details searchResult call

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 138 Aleisha Amohia 2017-08-27 23:44:25 UTC
Created attachment 66544 [details] [review]
[SIGNED-OFF] Bug 14385: Fix breaking issues constraint in ITerator test

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 139 Aleisha Amohia 2017-08-27 23:44:34 UTC
Created attachment 66545 [details] [review]
[SIGNED-OFF] Bug 14385: Fix ILSDI_Services test when clubs with branch exist

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 140 Aleisha Amohia 2017-08-27 23:45:09 UTC
Sorry for the spam! Signing off on all patches
Comment 141 M. Tompsett 2017-09-12 14:10:23 UTC
(In reply to Aleisha Amohia from comment #140)
> Sorry for the spam! Signing off on all patches

Thanks for the sign off. To the one that has the energy to QA this. The test plan is in comment #112.
Comment 142 Josef Moravec 2018-01-02 09:33:12 UTC
Hi,

these patchset doesn't apply. So just few comments from quick reading of the code:

You don't have to change signature of GetHiddenItemnumbers, you can ask for patron category here... And you can than leave the $search_context as is.

Also the "opac" param in GetMarcBiblio and EmbedItemsInMarcBiblio is not needed, you can use C4::Context->interface when you need to know the actual interface

And some squashing would be nice here ;)
Comment 143 M. Tompsett 2018-01-03 00:21:05 UTC
(In reply to Josef Moravec from comment #142)
> these patchset doesn't apply.

Thanks for looking and noticing this.


> So just few comments from quick reading of the
> code:
> 
> You don't have to change signature of GetHiddenItemnumbers, you can ask for
> patron category here... And you can than leave the $search_context as is.

I can't recall the reason off the top of my head, and even after rebasing this locally -- I have yet to test and put it back up. But I've got a feeling it had something to so with the nesting of some search functions which were not easily determined to be opac or intranet otherwise.


> Also the "opac" param in GetMarcBiblio and EmbedItemsInMarcBiblio is not
> needed, you can use C4::Context->interface when you need to know the
> actual interface.

It is better to pass it in my opinion to refactor the parameters list, so that test functions don't require special mocking to get the C4::Context function to return the correct value in different cases (like shell script utilities). And this makes maintenance for other changes in the future easier for others, and brings the code up to preferred coding style (https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL16:_Hashrefs_should_be_used_as_arguments)


> And some squashing would be nice here ;)

I was tweaking, testing, fixing, repeating multiple times. Also, smaller patches are easier to test individually. I will squash as much as possible. I do, however, not want to lose the fact that Aleisha Amohia put in a massive effort which helped me see how to solve my C4::Search mess problem that I had hit while working on originally in bug 10589.
Comment 144 Liz Rea 2018-01-03 00:59:16 UTC
a+ preserving attribution. :)
Comment 145 M. Tompsett 2018-01-04 01:03:28 UTC
Created attachment 70256 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 146 M. Tompsett 2018-01-04 01:03:34 UTC
Created attachment 70257 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 147 M. Tompsett 2018-01-04 01:03:40 UTC
Created attachment 70258 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 148 M. Tompsett 2018-01-04 01:03:47 UTC
Created attachment 70259 [details] [review]
Bug 14385: A lot of patches

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased!

The test plan is in comment #112 last I checked.
Comment 149 M. Tompsett 2018-01-04 01:11:20 UTC
Still in the midst of retesting after squash. Once I'm happy, I'll post back up a full test plan, and change to needs sign off, just to make sure I didn't miss something.
Comment 150 M. Tompsett 2018-03-05 16:22:26 UTC
Currently does not apply, reworking.
Comment 151 M. Tompsett 2018-03-05 19:51:44 UTC
Comment on attachment 70259 [details] [review]
Bug 14385: A lot of patches

Review of attachment 70259 [details] [review]:
-----------------------------------------------------------------

::: opac/opac-tags.pl
@@ +310,5 @@
> +        if (grep { $_->{term} eq $my_tag->{term} } @$results) {
> +            if (! $my_tag->{visible} ) {
> +                my $check_biblio = GetMarcBiblio({
> +                    biblionumber => $my_tag->{biblionumber},
> +                    embed_itesm  => 1,

Always good idea to re-check over and over. :)
Comment 152 M. Tompsett 2018-03-05 20:36:13 UTC
Created attachment 72445 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 153 M. Tompsett 2018-03-05 20:36:18 UTC
Created attachment 72446 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 154 M. Tompsett 2018-03-05 20:36:23 UTC
Created attachment 72447 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 155 M. Tompsett 2018-03-05 20:36:29 UTC
Created attachment 72448 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 156 M. Tompsett 2018-03-06 01:52:35 UTC
Created attachment 72449 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 157 M. Tompsett 2018-03-06 01:55:50 UTC
Created attachment 72450 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 158 M. Tompsett 2018-03-06 01:58:08 UTC
Created attachment 72451 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 159 M. Tompsett 2018-03-06 02:01:51 UTC
Created attachment 72452 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 160 M. Tompsett 2018-03-06 02:17:15 UTC
Created attachment 72453 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 161 M. Tompsett 2018-03-06 02:19:59 UTC
Created attachment 72454 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 162 M. Tompsett 2018-03-06 02:28:03 UTC
Today was massive rebase, and confirm changes. Still to do: test and confirm that comment #112 is the exhaustive test plan. Sorry for the noise.
Comment 163 M. Tompsett 2018-04-11 13:09:05 UTC
Created attachment 74001 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again! Rebased again -- my fault for being slow on this.

The test plan is in comment #112 last I checked.
Comment 164 M. Tompsett 2018-05-31 01:48:46 UTC
This is too huge, my mental energy keeps being too low, and I need this in sooner rather than later. Follow the test plan in comment #112.
Comment 165 M. Tompsett 2018-09-14 13:19:12 UTC
Created attachment 78654 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 166 M. Tompsett 2018-09-14 13:19:18 UTC
Created attachment 78655 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 167 M. Tompsett 2018-09-14 13:19:24 UTC
Created attachment 78656 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 168 M. Tompsett 2018-09-14 13:19:31 UTC
Created attachment 78657 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.
Comment 169 Joy Nelson 2018-09-14 22:51:25 UTC
Created attachment 78825 [details] [review]
Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 170 Joy Nelson 2018-09-14 22:51:35 UTC
Created attachment 78826 [details] [review]
Bug 14385 Followup: C4::Borrowers not found

GetMember is in C4::Members, not C4::Borrowers

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 171 Joy Nelson 2018-09-14 22:51:43 UTC
Created attachment 78827 [details] [review]
Bug 14385: Followup: changes to sysprefs.sql and atomicupdate

Run updatedatabase.pl when testing

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 172 Joy Nelson 2018-09-14 22:51:51 UTC
Created attachment 78828 [details] [review]
Bug 14385: Squash of a lot of patches rebased

- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!

The test plan is in comment #112 last I checked.