Bug 21159 - Update item shelving location (952$c) on checkout
Summary: Update item shelving location (952$c) on checkout
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Buckley
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 14576
Blocks: 35269
  Show dependency treegraph
 
Reported: 2018-08-06 02:56 UTC by Alex Buckley
Modified: 2024-01-17 22:04 UTC (History)
15 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/819
Text to go in the release notes:
This enhancement enables libraries to automatically update an item's shelving location using the new system preference `UpdateItemLocationOnCheckout`. It accepts pairs of shelving locations. On checkout the item's location is compared to the location on the left and, like `UpdateItemLocationOnCheckin`, is updated to the location on the right. Special values for this system preference are: * *_ALL_* - used on left side only to affect all items, if it matches, but this is an easier to read option) * *_PERM_* - used on right side only to set items to their permanent location *_BLANK_* - used on either side to match on or set to blank (actual blanks will work) Syntax highlighting is used in the text area to make it easier to read, as is already possible when checking in items.
Version(s) released in:
23.11.00


Attachments
Bug 21159 - Implementing 2 sysprefs to enable blanking location (11.64 KB, patch)
2018-08-06 03:25 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing 2 sysprefs to enable blanking location (11.63 KB, patch)
2018-08-08 04:10 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing 2 sysprefs to enable blanking location (11.87 KB, patch)
2018-08-08 21:08 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. (6.89 KB, patch)
2018-08-13 22:32 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. (6.88 KB, patch)
2018-08-15 21:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. (6.88 KB, patch)
2018-08-16 04:49 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. (6.92 KB, patch)
2018-08-16 05:04 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Corrected typo (1016 bytes, patch)
2018-08-16 05:24 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing syspref to enable blanking of item shelving location on issue. (6.23 KB, patch)
2018-10-19 01:28 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Corrected typo (939 bytes, patch)
2018-10-19 01:28 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Implementing syspref to enable blanking of item shelving location on issue. (6.23 KB, patch)
2018-10-19 01:35 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Corrected typo (938 bytes, patch)
2018-10-19 01:35 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159 - Implementing 2 sysprefs to enable blanking location (6.96 KB, patch)
2019-03-26 21:58 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 21159 - Corrected typo (855 bytes, patch)
2019-03-26 21:59 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 21159: (follow up) Fixed broken indentation in circulation. pref (1.41 KB, patch)
2019-03-27 01:12 UTC, Hayley Pelham
Details | Diff | Splinter 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 (6.64 KB, patch)
2019-04-10 01:53 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 21159 - Corrected typo (910 bytes, patch)
2019-04-10 01:53 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 21159: (follow up) Fixed broken indentation in circulation. pref (1.62 KB, patch)
2019-04-10 01:53 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 21159: Database changes (6.68 KB, patch)
2022-08-14 23:26 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.03 KB, patch)
2022-08-14 23:27 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (6.53 KB, patch)
2022-08-14 23:27 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (6.88 KB, patch)
2022-08-15 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (6.88 KB, patch)
2022-08-15 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (8.20 KB, patch)
2022-08-15 02:01 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (8.24 KB, patch)
2022-08-15 02:05 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.03 KB, patch)
2022-08-15 04:17 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (12.15 KB, patch)
2022-08-15 04:18 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (11.59 KB, patch)
2022-08-15 04:25 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: (follow-up) Removing tab spaces (5.28 KB, patch)
2022-08-17 00:49 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Database changes (6.75 KB, patch)
2023-02-14 01:29 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.02 KB, patch)
2023-02-14 01:29 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (11.62 KB, patch)
2023-02-14 01:29 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Database changes (6.75 KB, patch)
2023-02-14 01:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.02 KB, patch)
2023-02-14 01:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (11.62 KB, patch)
2023-02-14 01:59 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (3.97 KB, patch)
2023-02-14 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Database changes (7.00 KB, patch)
2023-08-02 04:47 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.03 KB, patch)
2023-08-02 04:53 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (14.79 KB, patch)
2023-08-02 04:54 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (3.97 KB, patch)
2023-08-02 04:54 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (14.79 KB, patch)
2023-08-02 05:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (3.97 KB, patch)
2023-08-02 05:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (14.02 KB, patch)
2023-09-07 04:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (3.97 KB, patch)
2023-09-07 04:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21159: Database changes (7.04 KB, patch)
2023-09-18 00:07 UTC, David Nind
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.07 KB, patch)
2023-09-18 00:08 UTC, David Nind
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (14.06 KB, patch)
2023-09-18 00:08 UTC, David Nind
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (4.01 KB, patch)
2023-09-18 00:08 UTC, David Nind
Details | Diff | Splinter Review
Bug 21159: Database changes (7.28 KB, patch)
2023-10-31 17:26 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.07 KB, patch)
2023-10-31 17:26 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (13.17 KB, patch)
2023-10-31 17:27 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (4.01 KB, patch)
2023-10-31 17:27 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21159: Database changes (7.33 KB, patch)
2023-11-01 11:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21159: Template and yaml changes (4.12 KB, patch)
2023-11-01 11:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21159: Update item location on checkout (13.22 KB, patch)
2023-11-01 11:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21159: (follow-up) Remove tab space (4.07 KB, patch)
2023-11-01 11:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21159: (QA follow-up) Add say to database update, perltidy, fix tests (13.75 KB, patch)
2023-11-01 11:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21159: Remove redundant mock (1019 bytes, patch)
2023-11-06 14:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21159: Add Koha::Item->update_item_location tests (2.83 KB, patch)
2023-11-06 14:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2018-08-06 02:56:04 UTC
Items shelving location (952$c) is blanked when the item is issued (if the new syspref BlankShelvingLocationOnIssue is enabled) and returned (if the new syspref BlankShelvingLocationOnReturn is enabled). 

i.e. This patch will blank shelving location on circulation actions, it will utilise two system preferences rather than one so it can work however people want it to. 

The sysprefs will both be off by default.
Comment 1 Alex Buckley 2018-08-06 03:25:06 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
Comment 2 Alex Buckley 2018-08-08 04:10:34 UTC
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
Comment 3 Katrin Fischer 2018-08-08 11:23:05 UTC
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
Comment 4 Alex Buckley 2018-08-08 21:08:34 UTC
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
Comment 5 Katrin Fischer 2018-08-08 23:28:24 UTC
Hi Alex, did you see my comment#3 here? Also, please don't forget to assign yourself to the bug :)
Comment 6 Alex Buckley 2018-08-08 23:29:53 UTC
Yup thanks Katrin looking now.
Comment 7 Alex Buckley 2018-08-10 04:47:22 UTC
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
Comment 8 Alex Buckley 2018-08-13 22:32:25 UTC
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
Comment 9 Alex Buckley 2018-08-15 21:00:19 UTC
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
Comment 10 Alex Buckley 2018-08-16 04:49:23 UTC
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
Comment 11 Alex Buckley 2018-08-16 05:04:31 UTC
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
Comment 12 Alex Buckley 2018-08-16 05:20:59 UTC
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 :)
Comment 13 Alex Buckley 2018-08-16 05:24:52 UTC
Created attachment 77871 [details] [review]
Bug 21159 - Corrected typo

Same test plan as first patch.

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 14 Alex Buckley 2018-09-11 22:19:25 UTC
Any chance we can move this patch along?
Comment 15 Alex Buckley 2018-10-19 01:28:12 UTC
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
Comment 16 Alex Buckley 2018-10-19 01:28:31 UTC
Created attachment 80893 [details] [review]
Bug 21159 - Corrected typo

Same test plan as first patch.

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 17 Alex Buckley 2018-10-19 01:35:26 UTC
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
Comment 18 Alex Buckley 2018-10-19 01:35:47 UTC
Created attachment 80895 [details] [review]
Bug 21159: Corrected typo

Same test plan as first patch.

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 19 Alex Buckley 2018-10-19 01:37:23 UTC
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.
Comment 20 Brian 2019-01-09 23:53:34 UTC
Any updates on bug 21159?

Carnegie team
Comment 21 Hayley Pelham 2019-03-26 21:58:14 UTC
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
Comment 22 Hayley Pelham 2019-03-26 21:59:25 UTC
Created attachment 87038 [details] [review]
Bug 21159 - Corrected typo

Same test plan as first patch.

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 23 Hayley Pelham 2019-03-27 01:12:16 UTC
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
Comment 24 Liz Rea 2019-03-29 15:53:03 UTC
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
Comment 25 Hayley Pelham 2019-04-02 20:54:45 UTC
(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. :)
Comment 26 Liz Rea 2019-04-10 01:41:12 UTC
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
Comment 27 Liz Rea 2019-04-10 01:53:49 UTC
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
Comment 28 Liz Rea 2019-04-10 01:53:53 UTC
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
Comment 29 Liz Rea 2019-04-10 01:53:57 UTC
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
Comment 30 Liz Rea 2019-04-10 01:54:40 UTC
Well that commit message is slightly closer anyway. I'll fix it again when I sign it off lol.

Liz
Comment 31 Liz Rea 2019-04-10 14:22:27 UTC
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
Comment 32 Hayley Pelham 2019-04-11 01:38:19 UTC
(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
Comment 33 Katrin Fischer 2019-04-25 13:26:19 UTC
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.
Comment 34 Alex Buckley 2022-08-09 05:44:36 UTC
I'm going to attempt to revive this patchset so setting this to 'Assigned' and setting myself as assignee.
Comment 35 Alex Buckley 2022-08-14 23:26:35 UTC
Created attachment 139097 [details] [review]
Bug 21159: Database changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 36 Alex Buckley 2022-08-14 23:27:08 UTC
Created attachment 139098 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 37 Alex Buckley 2022-08-14 23:27:29 UTC
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
Comment 38 Alex Buckley 2022-08-15 02:00:15 UTC
Created attachment 139103 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 39 Alex Buckley 2022-08-15 02:00:44 UTC
Created attachment 139104 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 40 Alex Buckley 2022-08-15 02:01:19 UTC
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
Comment 41 Alex Buckley 2022-08-15 02:05:19 UTC
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
Comment 42 Alex Buckley 2022-08-15 04:17:56 UTC
Created attachment 139107 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 43 Alex Buckley 2022-08-15 04:18:19 UTC
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
Comment 44 Alex Buckley 2022-08-15 04:21:29 UTC
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.
Comment 45 Alex Buckley 2022-08-15 04:25:06 UTC
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
Comment 46 Alex Buckley 2022-08-17 00:49:48 UTC
Created attachment 139220 [details] [review]
Bug 21159: (follow-up) Removing tab spaces

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 47 solene.ngamga 2023-01-09 19:17:33 UTC
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
Comment 48 Alex Buckley 2023-02-14 01:29:03 UTC
Created attachment 146602 [details] [review]
Bug 21159: Database changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 49 Alex Buckley 2023-02-14 01:29:22 UTC
Created attachment 146603 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 50 Alex Buckley 2023-02-14 01:29:38 UTC
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
Comment 51 Alex Buckley 2023-02-14 01:58:31 UTC
Created attachment 146605 [details] [review]
Bug 21159: Database changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 52 Alex Buckley 2023-02-14 01:58:49 UTC
Created attachment 146606 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 53 Alex Buckley 2023-02-14 01:59:08 UTC
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
Comment 54 Alex Buckley 2023-02-14 02:00:15 UTC
Created attachment 146608 [details] [review]
Bug 21159: (follow-up) Remove tab space

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 55 Alex Buckley 2023-02-14 02:02:03 UTC
Rebased against master and ready to test again
Comment 56 Laura Escamilla 2023-07-14 13:28:40 UTC
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
Comment 57 Alex Buckley 2023-08-02 04:47:34 UTC
Created attachment 154147 [details] [review]
Bug 21159: Database changes

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 58 Alex Buckley 2023-08-02 04:53:37 UTC
Created attachment 154148 [details] [review]
Bug 21159: Template and yaml changes

Sponsored-By: Toi Ohomai Institute of Technology, New Zealand
Comment 59 Alex Buckley 2023-08-02 04:54:17 UTC
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
Comment 60 Alex Buckley 2023-08-02 04:54:45 UTC
Created attachment 154150 [details] [review]
Bug 21159: (follow-up) Remove tab space

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 61 Alex Buckley 2023-08-02 04:56:04 UTC
(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.
Comment 62 Alex Buckley 2023-08-02 05:12:18 UTC
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
Comment 63 Alex Buckley 2023-08-02 05:12:32 UTC
Created attachment 154152 [details] [review]
Bug 21159: (follow-up) Remove tab space

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 64 Sam Lau 2023-08-02 15:11:48 UTC
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
Comment 65 Alex Buckley 2023-08-02 20:49:31 UTC
Thank you for testing Sam. Taking a look now
Comment 66 Aleisha Amohia 2023-09-07 04:38:12 UTC
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
Comment 67 Aleisha Amohia 2023-09-07 04:38:16 UTC
Created attachment 155285 [details] [review]
Bug 21159: (follow-up) Remove tab space

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 68 Aleisha Amohia 2023-09-07 04:39:34 UTC
Have fixed how this was affecting some other return messages, please test again
Comment 69 David Nind 2023-09-18 00:07:56 UTC
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>
Comment 70 David Nind 2023-09-18 00:08:00 UTC
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>
Comment 71 David Nind 2023-09-18 00:08:04 UTC
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>
Comment 72 David Nind 2023-09-18 00:08:07 UTC
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>
Comment 73 Katrin Fischer 2023-10-31 15:01:25 UTC
The conflict in sysprefs.sql was easy, but I got stuck witht he conflict in Issues.t - can you please rebase?
Comment 74 Alex Buckley 2023-10-31 17:26:18 UTC
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>
Comment 75 Alex Buckley 2023-10-31 17:26:46 UTC
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>
Comment 76 Alex Buckley 2023-10-31 17:27:30 UTC
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>
Comment 77 Alex Buckley 2023-10-31 17:27:49 UTC
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>
Comment 78 Alex Buckley 2023-10-31 17:32:23 UTC
(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!
Comment 79 Katrin Fischer 2023-11-01 11:51:48 UTC
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>
Comment 80 Katrin Fischer 2023-11-01 11:51:51 UTC
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>
Comment 81 Katrin Fischer 2023-11-01 11:51:56 UTC
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>
Comment 82 Katrin Fischer 2023-11-01 11:51:59 UTC
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>
Comment 83 Katrin Fischer 2023-11-01 11:52:03 UTC
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>
Comment 84 Alex Buckley 2023-11-01 13:03:38 UTC
Thanks Katrin!
Comment 85 Tomás Cohen Arazi 2023-11-06 14:03:47 UTC
Created attachment 158521 [details] [review]
Bug 21159: Remove redundant mock

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 86 Tomás Cohen Arazi 2023-11-06 14:03:51 UTC
Created attachment 158522 [details] [review]
Bug 21159: Add Koha::Item->update_item_location tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 87 Tomás Cohen Arazi 2023-11-06 14:17:09 UTC
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).
Comment 88 Fridolin Somers 2023-11-10 07:52:26 UTC
Enhancement not pushed to 23.05.x