Created attachment 159346 [details] [review]
Bug 35434: IndependentBranches prevents non-superlibrarians from placing holds in other libraries

I updated the hold page on the staff interface so that non-superlibrarians can't place holds in other libraries when the system preference 'IndependentBranches' is enabled.

TEST PLAN
1) Appy the patch.
2) In 'Koha administration > System preferences', set 'IndependentBranches' to 'yes'.
3) In 'Koha administration > Circulation and fines rules', add a rule by only clicking the 'Save' button (this will create a circulation rule for all patron categories and all item types, allowing unlimited holds).
4) Create a staff patron (with username and password).
    ---> In 'Patrons > + New patron > Staff', fill the required fields, add a username and password.
    ---> Remember the home library.
    ---> Click the 'Save' button.
5) Add permissions to the created patron.
    ---> In the patron detail page, click on 'More > Set permissions' and check the following boxes:
	- Staff access, allows viewing of catalogue in staff interface
	- Add, modify and view patron information
	- Place and modify holds for patrons
    ---> Click the 'Save' button.
6) Create an other patron of any patron category with the same home library as the staff patron you created.
7) Create a record with one item.
    ---> In 'Cataloging > + New record', fill the required fields.
    ---> Click the 'Save' button.
    ---> You should now be in the item form. Fill the required field (home library and current library should be the same as the patron's home library).
    ---> Click the 'Add item' button.
8) On the record details page, click 'Place hold' and search for the patron you created in step 6.
    ---> In the 'Pickup at' field, you should see all the libraries.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
9) Log out from the staff interface and login with the staff patron you created.
10) Click 'Search catalog' from the search bar and search for the record you created.
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.

Emily, can we have a unit test for this?

IIRC canreservefromotherbranches was also used in this context, but it seems missing from the test plan.

Created attachment 161509 [details] [review]
Bug 35434: IndependentBranches prevents non-superlibrarians from placing holds in other libraries

I updated the hold page on the staff interface so that non-superlibrarians can't place holds in other libraries when the system preference 'IndependentBranches' is enabled.

TEST PLAN
1) Appy the patch.
2) In 'Koha administration > System preferences', set 'IndependentBranches' to 'yes'.
3) In 'Koha administration > Circulation and fines rules', add a rule by only clicking the 'Save' button (this will create a circulation rule for all patron categories and all item types, allowing unlimited holds).
4) Create a staff patron (with username and password).
    ---> In 'Patrons > + New patron > Staff', fill the required fields, add a username and password.
    ---> Remember the home library.
    ---> Click the 'Save' button.
5) Add permissions to the created patron.
    ---> In the patron detail page, click on 'More > Set permissions' and check the following boxes:
	- Staff access, allows viewing of catalogue in staff interface
	- Add, modify and view patron information
	- Place and modify holds for patrons
    ---> Click the 'Save' button.
6) Create an other patron of any patron category with the same home library as the staff patron you created.
7) Create a record with one item.
    ---> In 'Cataloging > + New record', fill the required fields.
    ---> Click the 'Save' button.
    ---> You should now be in the item form. Fill the required field (home library and current library should be the same as the patron's home library).
    ---> Click the 'Add item' button.
8) On the record details page, click 'Place hold' and search for the patron you created in step 6.
    ---> In the 'Pickup at' field, you should see all the libraries.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
9) Log out from the staff interface and login with the staff patron you created.
10) Click 'Search catalog' from the search bar and search for the record you created.
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.

Signed-off-by: Anni Rajala <anni.rajala@turku.fi>

(In reply to Katrin Fischer from comment #3)
> IIRC canreservefromotherbranches was also used in this context, but it seems
> missing from the test plan.

Can you please reply/comment?

Emily Rose is back at school.  I'll assign this issue to another dev, they should be able to answer you shortly.

Thanks!

Here comes the updated version of plan test :

TEST PLAN
1) Appy the patch.
2) In 'Koha administration > System preferences' : 
    - set 'IndependentBranches' to 'yes'
    - set 'IndependentBranchesPatronModifications' to 'yes'
    - set 'IndependentBranchesTransfers' to 'yes'
    - set 'canreservefromotherbranches' to 'Don't allow'
3) In 'Koha administration > Circulation and fines rules', add a rule by only clicking the 'Save' button (this will create a circulation rule for all patron categories and all item types, allowing unlimited holds).
4) Create a staff patron (with username and password).
    ---> In 'Patrons > + New patron > Staff', fill the required fields, add a username and password.
    ---> Remember the home library.
    ---> Click the 'Save' button.
5) Add permissions to the created patron.
    ---> In the patron detail page, click on 'More > Set permissions' and check the following boxes:
	- Staff access, allows viewing of catalogue in staff interface
	- Add, modify and view patron information
	- Place and modify holds for patrons
    ---> Click the 'Save' button.
6) Create an other patron of any patron category with the same home library as the staff patron you created.
7) Create a record with one item.
    ---> In 'Cataloging > + New record', fill the required fields.
    ---> Click the 'Save' button.
    ---> You should now be in the item form. Fill the required field (home library and current library should be the same as the patron's home library).
    ---> Click the 'Add item' button.
8) On the record details page, click 'Place hold' and search for the patron you created in step 6.
    ---> In the 'Pickup at' field, you should see all the libraries.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
9) Log out from the staff interface and login with the staff patron you created.
10) Click 'Search catalog' from the search bar and search for the record you created.
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.

Created attachment 166648 [details] [review]
Bug 35434: IndependentBranches prevents non-superlibrarians from placing holds in other libraries

I updated the hold page on the staff interface so that non-superlibrarians can't place holds in other libraries when the system preference 'IndependentBranches' is enabled.

TEST PLAN
1) Appy the patch.
2) In 'Koha administration > System preferences', set 'IndependentBranches' to 'yes'.
3) In 'Koha administration > Circulation and fines rules', add a rule by only clicking the 'Save' button (this will create a circulation rule for all patron categories and all item types, allowing unlimited holds).
4) Create a staff patron (with username and password).
    ---> In 'Patrons > + New patron > Staff', fill the required fields, add a username and password.
    ---> Remember the home library.
    ---> Click the 'Save' button.
5) Add permissions to the created patron.
    ---> In the patron detail page, click on 'More > Set permissions' and check the following boxes:
	- Staff access, allows viewing of catalogue in staff interface
	- Add, modify and view patron information
	- Place and modify holds for patrons
    ---> Click the 'Save' button.
6) Create an other patron of any patron category with the same home library as the staff patron you created.
7) Create a record with one item.
    ---> In 'Cataloging > + New record', fill the required fields.
    ---> Click the 'Save' button.
    ---> You should now be in the item form. Fill the required field (home library and current library should be the same as the patron's home library).
    ---> Click the 'Add item' button.
8) On the record details page, click 'Place hold' and search for the patron you created in step 6.
    ---> In the 'Pickup at' field, you should see all the libraries.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
9) Log out from the staff interface and login with the staff patron you created.
10) Click 'Search catalog' from the search bar and search for the record you created.
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.

Signed-off-by: Anni Rajala <anni.rajala@turku.fi>
Signed-off-by: baptiste <baptiste.bayche@inlibro.com>

Can we get a unit test for this change?

Created attachment 169917 [details] [review]
Bug 35434: IndependentBranches prevents non-superlibrarians from placing holds in other libraries

I updated the hold page on the staff interface so that non-superlibrarians can't place holds in other libraries when the system preference 'IndependentBranches' is enabled.

TEST PLAN
1) Appy the patch.
2) In 'Koha administration > System preferences', set 'IndependentBranches' to 'yes'.
3) In 'Koha administration > Circulation and fines rules', add a rule by only clicking the 'Save' button (this will create a circulation rule for all patron categories and all item types, allowing unlimited holds).
4) Create a staff patron (with username and password).
    ---> In 'Patrons > + New patron > Staff', fill the required fields, add a username and password.
    ---> Remember the home library.
    ---> Click the 'Save' button.
5) Add permissions to the created patron.
    ---> In the patron detail page, click on 'More > Set permissions' and check the following boxes:
	- Staff access, allows viewing of catalogue in staff interface
	- Add, modify and view patron information
	- Place and modify holds for patrons
    ---> Click the 'Save' button.
6) Create an other patron of any patron category with the same home library as the staff patron you created.
7) Create a record with one item.
    ---> In 'Cataloging > + New record', fill the required fields.
    ---> Click the 'Save' button.
    ---> You should now be in the item form. Fill the required field (home library and current library should be the same as the patron's home library).
    ---> Click the 'Add item' button.
8) On the record details page, click 'Place hold' and search for the patron you created in step 6.
    ---> In the 'Pickup at' field, you should see all the libraries.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
9) Log out from the staff interface and login with the staff patron you created.
10) Click 'Search catalog' from the search bar and search for the record you created.
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.

Signed-off-by: Anni Rajala <anni.rajala@turku.fi>
Signed-off-by: baptiste <baptiste.bayche@inlibro.com>

Created attachment 169918 [details] [review]
Bug 35434: (follow-up) added canreservefromotherbranches in addition to canreservefromotherbranches to prevent non-superlibrarians from placing holds in other libraries

Created attachment 169919 [details] [review]
Bug 35434: (follow-up) Unit test

Step 11 of the plan test fails : 
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
    
KO : For the librarians having limited permissions, all libraries are still in the pickup location dropdown menu.

After looking at the code, it seems the testing plans was missing some info.

When you go and set IndependentBranches to true, you also need to set  canreservefromotherbranches  to don't allow. 

It worked for me when i added this setting

TEST PLAN
1) Appy the patch.
2) In 'Koha administration > System preferences', set 'IndependentBranches' to 'yes' And 'canreservefromotherbranches' to 'Don't allow'.
3) In 'Koha administration > Circulation and fines rules', add a rule by only clicking the 'Save' button (this will create a circulation rule for all patron categories and all item types, allowing unlimited holds).
4) Create a staff patron (with username and password).
    ---> In 'Patrons > + New patron > Staff', fill the required fields, add a username and password.
    ---> Remember the home library.
    ---> Click the 'Save' button.
5) Add permissions to the created patron.
    ---> In the patron detail page, click on 'More > Set permissions' and check the following boxes:
	- Staff access, allows viewing of catalogue in staff interface
	- Add, modify and view patron information
	- Place and modify holds for patrons
    ---> Click the 'Save' button.
6) Create an other patron of any patron category with the same home library as the staff patron you created.
7) Create a record with one item.
    ---> In 'Cataloging > + New record', fill the required fields.
    ---> Click the 'Save' button.
    ---> You should now be in the item form. Fill the required field (home library and current library should be the same as the patron's home library).
    ---> Click the 'Add item' button.
8) On the record details page, click 'Place hold' and search for the patron you created in step 6.
    ---> In the 'Pickup at' field, you should see all the libraries.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.
9) Log out from the staff interface and login with the staff patron you created.
10) Click 'Search catalog' from the search bar and search for the record you created.
11) On the record details page, click 'Place hold', then search for the patron in step 6.
    ---> In the 'Pickup at' field, you should only see the home library of the item.
    ---> In the 'Allowed pickup locations' column of the item, you should see the same.

Feel free to change it back to failed qa if there anything else missing

Created attachment 176045 [details] [review]
Bug 35434: Prevent non-superlibrarians from placing holds in other libraries when using IndependentBranches

When IndependentBranches is enabled and canreservefromotherbranches
is set to don't allow, then non-superlibrarians should not be able to
place holds for other libraries.

Test plan:
1. Appy the patch.
2. In Koha administration > System preferences, set:
   - IndependentBranches: Yes
   - canreservefromotherbranches: Don't allow
3. In Koha administration > Circulation and fines rules, check that
   you have a rule for all patron categories and all item types, with
   unlimited holds.
4. Find (or create) a staff patron for a library other than Centerville:
   - Change the username and password so you can log in as the
     staff patron.
   - Add permissions to the staff patron (More > Set permissions):
     . Staff access, allows viewing of catalogue in staff interface
     . Add, modify and view patron information
     . Place and modify holds for patrons
5. Find (or create) a patron with the same home library as the
   staff patron in step 4.
6. Find (or create) a record with an item where the home and
   current library are the same as the staff patron in step 3 and the
   patron in step 5.
7. As a superlibrarian place a hold for the record from step 6
   using the patron from step 5:
   - If placing a hold for the next available item, the "Pickup at"
     dropdown list should list all libraies.
   - If placing a hold on a specific item, the dropdown list for the
     "Allowed pickup locations" column for each item should also list
     all libraries.
   - After placing the hold, in the "Existing holds" table, the dropdown
     list for each hold in the "Pickup library" column should
     also list all libraries.
8. Log out from the staff interface and login with the staff patron
   from step 4.
9. Find the record from step 6 and place a hold using the patron from
   step 5 - the library dropdown list should be limited to the
   item's home library:
   - If placing a hold for the next available item, the "Pickup at"
     dropdown list should only list the item's home library.
   - If placing a hold on a specific item, the dropdown list for the
     "Allowed pickup locations" column for each item should also only
     list the item's home library.
   - After placing the hold, in the "Existing holds" table, the
     "Pickup library" column should now only show the item's home
     librayr, with no dropdown list.

Signed-off-by: Anni Rajala <anni.rajala@turku.fi>
Signed-off-by: baptiste <baptiste.bayche@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 176046 [details] [review]
Bug 35434: (follow-up) Added canreservefromotherbranches

When IndependentBranches is enabled, canreservefromotherbranches
set to don't allow is also required to prevent non-superlibrarians
from placing holds in other libraries.

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 176047 [details] [review]
Bug 35434: (follow-up) Unit test

Test plan:
1. Tests should pass: prove t/db_dependent/Koha/Item.t

Signed-off-by: David Nind <david@davidnind.com>

I've changed the assignee - please correct if I have changed it to the wrong person!

I also edited the commit messages - feel free to fix up any mistakes I may have made.

Testing notes (using koha-testing-docker (KTD)):
1. Step 3 - in KTD, these are the default rules already, so no need to change.
2. Step 4 - I used Henry Acevedo, Midway.
3. Step 5 - I used Edna Acosta, Midway.
4. Step 6 - I used Programming the Perl DBI, record number 13.