|
Description
Alex Buckley
2018-08-06 02:56:04 UTC
Created attachment 77487 [details] [review] Bug 21159 - Implementing 2 sysprefs to enable blanking location This patch introduces two new system preferences: BlankingShelvingLocationOnIssue and BlankingShelvingLocationOnReturn. By default both are off. If BlankingShelvingLocationOnIssue is enabled then when an item is issued it's shelving location 952$c field will be set to '' in C4::Items->BlankShelvingLocation() If BlankingShelvingLocationOnReturn is enabled then when an item is returned it's shelving location field will be set to '' in C4::Items->BlankShelvingLocation(). t/db_dependent/Circulation/issue.t contains tests for BlankShelvingLocationOnIssue t/db_dependen/Circulation/Returns.t contains tests for BlankShelvingLocationOnReturn Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added two new sysprefs: BlankShelvingLocationOnIssue and BlankShelvingLocationOnReturn both of which are off by default) 5. Notice the shelving location has not changed, showing with the BlankShelvingLocationOnReturn syspref not enabled the shelving location is not changed upon return 6. Check the item out again and notice the shelving location is not changed, showing that when the BlankShelvingLocationOnIssue is not enabled the shelving location is not changed upon issue. 7. Enable both aforementioned sysprefs in the Administration->Global system preferences->Circulation interface 8. Return the item and notice the Shelving location has changed from 'Cart' to no value, showing when the BlankShelvingLocationOnReturn syspref is enabled it blanks the shelving location value on the return of the item 9. Edit the item changing the shelving location back to 'Shelving Trolley'(CART) 10. Check out the item and notice the shelving location is changed to no value, showing when the BlankShelvingLocationOnIssue syspref is enabled it blanks the shelving location on the issue of an item. 11. Run t/db_dependent/Circulation/Returns.t 12. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 77549 [details] [review] Bug 21159 - Implementing 2 sysprefs to enable blanking location This patch introduces two new system preferences: BlankingShelvingLocationOnIssue and BlankingShelvingLocationOnReturn. By default both are off. If BlankingShelvingLocationOnIssue is enabled then when an item is issued it's shelving location 952$c field will be set to '' in C4::Items->BlankShelvingLocation() If BlankingShelvingLocationOnReturn is enabled then when an item is returned it's shelving location field will be set to '' in C4::Items->BlankShelvingLocation(). t/db_dependent/Circulation/issue.t contains tests for BlankShelvingLocationOnIssue t/db_dependen/Circulation/Returns.t contains tests for BlankShelvingLocationOnReturn Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added two new sysprefs: BlankShelvingLocationOnIssue and BlankShelvingLocationOnReturn both of which are off by default) 5. Notice the shelving location has not changed, showing with the BlankShelvingLocationOnReturn syspref not enabled the shelving location is not changed upon return 6. Check the item out again and notice the shelving location is not changed, showing that when the BlankShelvingLocationOnIssue is not enabled the shelving location is not changed upon issue. 7. Enable both aforementioned sysprefs in the Administration->Global system preferences->Circulation interface 8. Return the item and notice the Shelving location has changed from 'Cart' to no value, showing when the BlankShelvingLocationOnReturn syspref is enabled it blanks the shelving location value on the return of the item 9. Edit the item changing the shelving location back to 'Shelving Trolley'(CART) 10. Check out the item and notice the shelving location is changed to no value, showing when the BlankShelvingLocationOnIssue syspref is enabled it blanks the shelving location on the issue of an item. 11. Run t/db_dependent/Circulation/Returns.t 12. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Hi Alex, I think this might be kind of a partial duplicate to bug 14576 that allows arbitrary updates on the location field on checkin. I think if that makes it in, it would make BlankingShelvingLocationOnReturn obsolete. Maybe you could take a look and see if it fits your use case? The code might be reusable for checkout too. I'd also be interesting if the use case is CART on how this works different to the existing features: NewItemsDefaultLocation, ReturnToShelvingCart, InProcessingToShelvingCart Created attachment 77594 [details] [review] Bug 21159 - Implementing 2 sysprefs to enable blanking location This patch introduces two new system preferences: BlankingShelvingLocationOnIssue and BlankingShelvingLocationOnReturn. By default both are off. If BlankingShelvingLocationOnIssue is enabled then when an item is issued it's shelving location 952$c field will be set to '' in C4::Items->BlankShelvingLocation() If BlankingShelvingLocationOnReturn is enabled then when an item is returned it's shelving location field will be set to '' in C4::Items->BlankShelvingLocation(). t/db_dependent/Circulation/issue.t contains tests for BlankShelvingLocationOnIssue t/db_dependen/Circulation/Returns.t contains tests for BlankShelvingLocationOnReturn Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added two new sysprefs: BlankShelvingLocationOnIssue and BlankShelvingLocationOnReturn both of which are off by default) 5. Notice the shelving location has not changed, showing with the BlankShelvingLocationOnReturn syspref not enabled the shelving location is not changed upon return 6. Check the item out again and notice the shelving location is not changed, showing that when the BlankShelvingLocationOnIssue is not enabled the shelving location is not changed upon issue. 7. Enable both aforementioned sysprefs in the Administration->Global system preferences->Circulation interface 8. Return the item and notice the Shelving location has changed from 'Cart' to no value, showing when the BlankShelvingLocationOnReturn syspref is enabled it blanks the shelving location value on the return of the item 9. Edit the item changing the shelving location back to 'Shelving Trolley'(CART) 10. Check out the item and notice the shelving location is changed to no value, showing when the BlankShelvingLocationOnIssue syspref is enabled it blanks the shelving location on the issue of an item. 11. Run t/db_dependent/Circulation/Returns.t 12. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Hi Alex, did you see my comment#3 here? Also, please don't forget to assign yourself to the bug :) Yup thanks Katrin looking now. Hi Katrin Sorry for the delay I think I should be able to modify this commit so it depends on (builds on) whats in 14576. In other words instead of implementing the BlankShelvingLocationOnReturn syspref which I have in this commit I can leave that out as it is being covered by 14576. However, 14576 does not set/blank the items.locations field when an item is issued as the BlankShelvingLocationOnIssue syspref in this commit does so I can leverage some of the code of 14576 to implement this functionality. Being able to blank the shelving location of an item when it is issued was the main thing our client wanted to be able to do. Thanks for bringing 14576 to my attention I will make changes to my commits so it builds on 14576. Cheers, Alex Created attachment 77775 [details] [review] Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items which are circulated. In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 77859 [details] [review] Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items which are circulated. In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 77869 [details] [review] Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items which are circulated. In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 77870 [details] [review] Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items which are circulated. In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand I think this one is ready to test. It is building on (depends on) the functionality provided by 14576. Many thanks to Katrin for informing me about 14576 so I was able to remove the duplicated functionality that my original patch had. This new patch does not deal with automatically updating the item location when the item is returned as this is covered by 14576. Instead, this new commit blanks out the items.location field when the item is checked out if the UpdateItemLocationOnCheckout syspref is enabled. This makes an easier workflow for automatically removing the items.location value if the item is issued. Please let me know if further changes are required :) Created attachment 77871 [details] [review] Bug 21159 - Corrected typo Same test plan as first patch. Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Any chance we can move this patch along? Created attachment 80892 [details] [review] Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items which are circulated. In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 80893 [details] [review] Bug 21159 - Corrected typo Same test plan as first patch. Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 80894 [details] [review] Bug 21159: Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items which are circulated. In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 80895 [details] [review] Bug 21159: Corrected typo Same test plan as first patch. Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Newly rebased patches. Also fixed warnings from qa test tool. As testing this is reliant on bug 14576 (currently set to 'patch doesnt apply' you may need to wait for the patches on 14576 to be rebased before testing this. Any updates on bug 21159? Carnegie team Created attachment 87037 [details] [review] Bug 21159 - Implementing 2 sysprefs to enable blanking location This patch introduces two new system preferences: BlankingShelvingLocationOnIssue and BlankingShelvingLocationOnReturn. By default both are off. If BlankingShelvingLocationOnIssue is enabled then when an item is issued it's shelving location 952$c field will be set to '' in C4::Items->BlankShelvingLocation() If BlankingShelvingLocationOnReturn is enabled then when an item is returned it's shelving location field will be set to '' in C4::Items->BlankShelvingLocation(). t/db_dependent/Circulation/issue.t contains tests for BlankShelvingLocationOnIssue t/db_dependen/Circulation/Returns.t contains tests for BlankShelvingLocationOnReturn Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added two new sysprefs: BlankShelvingLocationOnIssue and BlankShelvingLocationOnReturn both of which are off by default) 5. Notice the shelving location has not changed, showing with the BlankShelvingLocationOnReturn syspref not enabled the shelving location is not changed upon return 6. Check the item out again and notice the shelving location is not changed, showing that when the BlankShelvingLocationOnIssue is not enabled the shelving location is not changed upon issue. 7. Enable both aforementioned sysprefs in the Administration->Global system preferences->Circulation interface 8. Return the item and notice the Shelving location has changed from 'Cart' to no value, showing when the BlankShelvingLocationOnReturn syspref is enabled it blanks the shelving location value on the return of the item 9. Edit the item changing the shelving location back to 'Shelving Trolley'(CART) 10. Check out the item and notice the shelving location is changed to no value, showing when the BlankShelvingLocationOnIssue syspref is enabled it blanks the shelving location on the issue of an item. 11. Run t/db_dependent/Circulation/Returns.t 12. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 87038 [details] [review] Bug 21159 - Corrected typo Same test plan as first patch. Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 87041 [details] [review] Bug 21159: (follow up) Fixed broken indentation in circulation. pref Test plan: Same as comment 21. Sponsored-by: Catalyst IT I'm a little confused - one of Alex's patches builds on bug 14576, but Hayley's seems to want us to test the 2 sysprefs? It looks like it might just be a commit message problem - i.e. we need alex's test plan, and this most recently rebased patch to be put together. Have a look? I'm happy to test this once I know all the bits are in the right order. :) Liz (In reply to Liz Rea from comment #24) > I'm a little confused - one of Alex's patches builds on bug 14576, but > Hayley's seems to want us to test the 2 sysprefs? It looks like it might > just be a commit message problem - i.e. we need alex's test plan, and this > most recently rebased patch to be put together. > > Have a look? I'm happy to test this once I know all the bits are in the > right order. :) > > Liz Hi Liz, I think what happened here is I squashed the commits for simplicity, but used the first patch to squash them into rather than the follow up patch which builds on 14576. The behaviour should be the same, and testing should follow Comment 11. :) Hi Hayley, Cool, I"ll test it and if it looks all good will see if I can't get the right commit message into the patch. :) Thanks for clearing that up! Cheers, Liz Created attachment 87693 [details] [review] Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. This patch builds upon the functionality implemented in bug 14576, by allowing library staff to automatically change the item shelving location of items whic In the case of this patch it automatically blanks the item shelving locations of items when they are issued if the BlankItemLocationOnCheckout syspref is enabled. t/db_dependent/Circulation/issue.t contains tests for BlankItemLocationOnCheckout. Test plan: 1. Create a new biblio and associated item and set the value of the shelving location for the item to 'CART' 2. Checkout the item to a user and notice that the status does not change 3. Apply patches on bug report 14576, and this patch, and run ./updatedatabase.pl inside the koha shell 4. Now return the checked out item (when you updated the database you added one new syspref: UpdateItemLocationOnCheckout which is off by default) 5. Check the item out again and notice the shelving location is not changed, showing that when the UpdateItemLocationOnCheckout is not enabled and the shelving location is not changed upon issue. 6. Enable the aforementioned syspref in the Administration->Global system preferences->Circulation interface 7. Check out the item and notice the shelving location is changed to no value, showing when the UpdateItemLocationOnCheckout syspref is enabled it blanks the shelving location on the issue of an item. 8. Run t/db_dependent/Circulation/issue.t Sponsored-By: Toi Ohomai Institute of Technology, New Zealand https://bugs.koha-community.org/show_bug.cgi?id=18888 Created attachment 87694 [details] [review] Bug 21159 - Corrected typo Same test plan as first patch. Sponsored-By: Toi Ohomai Institute of Technology, New Zealand https://bugs.koha-community.org/show_bug.cgi?id=18888 Created attachment 87695 [details] [review] Bug 21159: (follow up) Fixed broken indentation in circulation. pref Test plan: Same as comment 21. Sponsored-by: Catalyst IT https://bugs.koha-community.org/show_bug.cgi?id=18888 Well that commit message is slightly closer anyway. I'll fix it again when I sign it off lol. Liz Hi Hayley, Sorry to say but the tests don't pass, and that's before even testing the functionality itself. It looks to me like they are highlighting a legit problem though (both with the tests, and with the feature). I'll be on IRC today in your morning and possibly afternoon if you want to talk about anything at all, look me up I'm wizzyrea. :) Cheers, Liz (In reply to Liz Rea from comment #31) > Hi Hayley, > > Sorry to say but the tests don't pass, and that's before even testing the > functionality itself. It looks to me like they are highlighting a legit > problem though (both with the tests, and with the feature). > > I'll be on IRC today in your morning and possibly afternoon if you want to > talk about anything at all, look me up I'm wizzyrea. :) > > Cheers, > Liz Hi Liz, This was the result of me rebasing Alex's patch, and so I fixed the conflicts and attached it with the plan to test it but I ran out of time to do so. Thank you for testing it, it is obviously one I will have to look into further before it can be completed! Cheers, Hayley Hi Hayley and Alex, bug 14576 was pushed now and I think it would make sense to have a look and make both checkout and checkin features match in functionality. I am a little worried about the blanking. Usually when CART is used you don't want to blank, but to set back to 'permanent location' (that can be blank if none was assigned before). There seems to be some mismatch here, as we still have BlankShelvingLocation but then also UpdateItemLocationOnCheckout. I feel like this needs a bit more work. I'm going to attempt to revive this patchset so setting this to 'Assigned' and setting myself as assignee. Created attachment 139097 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139098 [details] [review] Bug 21159: Template and yaml changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139099 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139103 [details] [review] Bug 21159: Template and yaml changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139104 [details] [review] Bug 21159: Template and yaml changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139105 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139106 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139107 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 139108 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand The amended patches add a new system preference UpdateItemLocationOnCheckout. This syspref shares the same logic with UpdateItemLocationOnCheckin. This logic has been moved to a shared subroutine Koha::Item->update_item_location() This means rather than being limited to blanking shelving locations on checkout, librarians can now alter shelving locations in the same way on check-in and checkout. Created attachment 139109 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 139220 [details] [review] Bug 21159: (follow-up) Removing tab spaces Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Bug 21159 - Update item shelving location (952$c) on checkout 139097 - Bug 21159: Database changes 139107 - Bug 21159: Template and yaml changes 139109 - Bug 21159: Update item location on checkout 139220 - Bug 21159: (follow-up) Removing tab spaces Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 21159: Database changes .git/rebase-apply/patch:21: trailing whitespace. warning: 1 line adds whitespace errors. Using index info to reconstruct a base tree... M installer/data/mysql/mandatory/sysprefs.sql .git/rebase-apply/patch:21: trailing whitespace. warning: 1 line applied after fixing whitespace errors. Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/mandatory/sysprefs.sql CONFLICT (content): Merge conflict in installer/data/mysql/mandatory/sysprefs.sql error: Failed to merge in the changes. Patch failed at 0001 Bug 21159: Database changes hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-21159-Database-changes-9yBvZ6.patch Created attachment 146602 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 146603 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 146604 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 146605 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 146606 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 146607 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 146608 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Rebased against master and ready to test again Applying: Bug 21159: Database changes Using index info to reconstruct a base tree... M installer/data/mysql/mandatory/sysprefs.sql Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/mandatory/sysprefs.sql Applying: Bug 21159: Template and yaml changes Applying: Bug 21159: Update item location on checkout Using index info to reconstruct a base tree... M C4/Circulation.pm M Koha/Item.pm M t/db_dependent/Circulation/issue.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Circulation/issue.t CONFLICT (content): Merge conflict in t/db_dependent/Circulation/issue.t Auto-merging Koha/Item.pm CONFLICT (content): Merge conflict in Koha/Item.pm Auto-merging C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 21159: Update item location on checkout hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-21159-Update-item-location-on-checkout-TimIX1.patch Created attachment 154147 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 154148 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Created attachment 154149 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 154150 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand (In reply to Laura Escamilla from comment #56) > Applying: Bug 21159: Database changes > Using index info to reconstruct a base tree... > M installer/data/mysql/mandatory/sysprefs.sql > Falling back to patching base and 3-way merge... > Auto-merging installer/data/mysql/mandatory/sysprefs.sql > Applying: Bug 21159: Template and yaml changes > Applying: Bug 21159: Update item location on checkout > Using index info to reconstruct a base tree... > M C4/Circulation.pm > M Koha/Item.pm > M t/db_dependent/Circulation/issue.t > Falling back to patching base and 3-way merge... > Auto-merging t/db_dependent/Circulation/issue.t > CONFLICT (content): Merge conflict in t/db_dependent/Circulation/issue.t > Auto-merging Koha/Item.pm > CONFLICT (content): Merge conflict in Koha/Item.pm > Auto-merging C4/Circulation.pm > error: Failed to merge in the changes. > Patch failed at 0001 Bug 21159: Update item location on checkout > hint: Use 'git am --show-current-patch=diff' to see the failed patch > When you have resolved this problem run "git bz apply --continue". > If you would prefer to skip this patch, instead run "git bz apply --skip". > To restore the original branch and stop patching run "git bz apply --abort". > Patch left in /tmp/Bug-21159-Update-item-location-on-checkout-TimIX1.patch Thanks for trying to test Laura. I've rebased against master, these patches should now apply cleanly. Created attachment 154151 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 154152 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Everything in your test plan is working perfectly. Failing QA because the tests are failing. # Failed test 'UpdateNotForLoanStatusOnCheckin updates notforloan value from 1 to 9 with setting "1: 9"' # at t/db_dependent/Circulation/issue.t line 432. # Failed test 'UpdateNotForLoanStatusOnCheckin does not update notforloan value from 9 with setting "1: 9"' # at t/db_dependent/Circulation/issue.t line 438. t/db_dependent/Circulation/issue.t .. 48/62 # Looks like you failed 2 tests of 62. t/db_dependent/Circulation/issue.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/62 subtests Test Summary Report ------------------- t/db_dependent/Circulation/issue.t (Wstat: 512 Tests: 62 Failed: 2) Failed tests: 27, 29 Non-zero exit status: 2 Thank you for testing Sam. Taking a look now Created attachment 155284 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Created attachment 155285 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Have fixed how this was affecting some other return messages, please test again Created attachment 155771 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 155772 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 155773 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'CART'. Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 155774 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> The conflict in sysprefs.sql was easy, but I got stuck witht he conflict in Issues.t - can you please rebase? Created attachment 158146 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 158147 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 158149 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'REF' (don't use 'CART' as this is blanked by bug 14576 on checkout). Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Created attachment 158150 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> (In reply to Katrin Fischer from comment #73) > The conflict in sysprefs.sql was easy, but I got stuck witht he conflict in > Issues.t - can you please rebase? Hi Katrin, Thank you for doing QA on this patchset, appreciate it. I've done the following: 1) I have rebased the patches against master. 2) You're right issue.t was rather tricky! but I've fixed that up 3) Run all unit tests in t/db_dependent/Circulation pass for me now 4) I've also run QA test tools - qa -c 4 -v 2 - and that passed for me. Please note: I made one tweak to step 7 of the test plan in the third patch to instruct testers to please use 'REF', not 'CART', because I noticed bug 14576 can blank items with location='CART'. Hopefully all of this is helpful! Created attachment 158169 [details] [review] Bug 21159: Database changes Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 158170 [details] [review] Bug 21159: Template and yaml changes Sponsored-By: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 158171 [details] [review] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'REF' (don't use 'CART' as this is blanked by bug 14576 on checkout). Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 158172 [details] [review] Bug 21159: (follow-up) Remove tab space Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 158173 [details] [review] Bug 21159: (QA follow-up) Add say to database update, perltidy, fix tests * Perltidies to pass the QA script * Adds missing say statement to the database update * Makes sure the tests pass on a database, where UpdateItemLocaton* system preferences are not empty Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Thanks Katrin! Created attachment 158521 [details] [review] Bug 21159: Remove redundant mock Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 158522 [details] [review] Bug 21159: Add Koha::Item->update_item_location tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I'm pushing this one. I appreciate the effort to refactor things in a good way and thanks for that. I have some remarks, though. Those I feel deserve to be treated on their own bugs but I'll post here for now. 1. I think the method name should be changed to something like `location_update_trigger`. I hope it is clear the why the name change proposal. 2. I'm not sure this method should internally call ->store. Calling it triggers a series of things like logging, and maybe others. It feels like the caller should _do all the changes to the item they need and then finally save_. 3. I don't feel like the return value is cool or either documented enough. I recognize this is inherited from the original code in the C4 namespace. Moving to Koha:: should be part of cleaning and improving, so more thought is needed here. At a very first sight, I'd say this shouldn't be calling ->store implicitly, and should be returning $self for method chainability. If we need to return some messages for the caller to be aware of, we already have a standard way to do it, using the Koha::Object::Message class, which is encapsulated inside Koha::Object which Koha::Item inherits from (e.g. see Koha::Object->add_message). Enhancement not pushed to 23.05.x |