This should use the same pattern as bug 11629.
Created attachment 46454 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN
Created attachment 46455 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/)
Created attachment 46456 [details] [review] Bug 14576 - Unit tests
This looks useful. I'm not crazy about additions to Koha which require the user to set configurations in this manner because I think it's no very friendly. A better choice, I think, would be to build a separate page as a front-end for the preference, displaying a form with paired dropdown menus listing existing shelving locations. I see a couple of problems with the functionality as it is here: 1. I think there are bound to be conflicts with the InProcessingToShelvingCart and ReturnToShelvingCart system preferences. I'm guessing the functionality of those preferences can be fully replaced by that of this patch, but a migration system will have to be built. 2. If this patch is approved without an improved interface like I describe, it needs to be made clear what empty values are meant to do. For instance: PROC: or :PROC Should that work?
Created attachment 48258 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/)
Created attachment 48259 [details] [review] Bug 14576 - Unit tests
A minor problem: FAIL C4/Circulation.pm OK critic FAIL forbidden patterns forbidden pattern: Data::Dumper::Dumper (line 1870) forbidden pattern: Data::Dumper::Dumper (line 1872) I think this text could be improved: " Item location updated from CART (Not an authorized value) to Fiction shelf " I think that should be "Item shelving location" (since "location" could be confused with "library"). I think the text "Not an authorized value" is not helpful to the user and is potentially scary. Think "UNAUTHOIRZED VALUE!!" I still have an issue with the data-entry aspect of this feature. In testing, I was stopped right away by having not put a space after the colon: 'FIC: CART' works 'FIC:CART' fails silently I don't think it's reasonable to expect the user to know this. For that matter, I don't think it's reasonable to expect the user to know the code for all existing shelving location authorized values. I realize this is an administrative setting, but I think we can do better. At minimum we need: - Text describing explicitly the correct format. - Some way to warn the user if their entry is incorrect. - Text explaining that an "_ALL_" rule overrides all others, or better: - Some way to warn the user if they have set an "_ALL_" rule along with other rules Overall this new patch is a great improvement over the last. But I still think we can do better. I think a separate template is needed, although I wouldn't fail the patch for that unless there was some consensus.
Created attachment 50662 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/)
Created attachment 50663 [details] [review] Bug 14576 - Unit tests
Created attachment 52195 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/)
Created attachment 52196 [details] [review] Bug 14576 - Unit tests
Created attachment 52197 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values
Created attachment 52204 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org>
Created attachment 52205 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org>
Created attachment 52206 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org>
Created attachment 52222 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 52223 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 52224 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
QA comments: 1/ Prefer mock_preference to C4::Context->set_preference 2/ "my" variable $yaml masks earlier declaration in same scope at C4/Circulation.pm line 1968. 3/ Fix some the indendation in JS code I have not tested it already, but please fix that first.
Created attachment 52365 [details] [review] Bug 14576 (QA Folllowup) Address issues use mock for tests use a separate variable for yaml fix indentation
CONFLICT (content): Merge conflict in C4/Circulation.pm Please rebase. Could you answer to comment 7? Did you take the remarks into account?
Created attachment 53112 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 53113 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 53114 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 53115 [details] [review] Bug 14576 (QA Folllowup) Address issues use mock for tests use a separate variable for yaml fix indentation
(In reply to Owen Leonard from comment #7) > I think that should be "Item shelving location" (since "location" could be > confused with "library"). Changed > I think the text "Not an authorized value" is not helpful to the user and is > potentially scary. Think "UNAUTHOIRZED VALUE!!" Changed to: 'No description available' - I am happy to reword but I do think it is nice to indicate to the user that it isn't a defined authorized value somehow > - Text describing explicitly the correct format. > - Text explaining that an "_ALL_" rule overrides all others Added to syspref description in editor > - Some way to warn the user if their entry is incorrect. > - Some way to warn the user if they have set an "_ALL_" rule along with > other rules Added js check with warnings for basic syntax I opened a second bug for addition of full GUI - this patchset follows existing format in the system but I do think a full editor could be added as per bug 16698 once this feature is pushed
1/ Tests in t/db_dependent/Circulation/Returns.t does not pass with these patches applied. 2/ "This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences." Hum... git grep "ReturnToShelvingCart\|InProcessingToShelvingCart" | grep -v translator | wc -l 28
Created attachment 54281 [details] [review] Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart This patch eliminates ReturnToShelvingCart and InProcessingToShelvingCart, Deletes Returns.t (it only tested those two prefs and new tests are in issue.t) Adds the UpdateItemLocationOnCheckin to svc/checkin Remaining instances found by: git grep "ReturnToShelvingCart\|InProcessingToShelvingCart" | grep -v translator | wc -l are db update files and release notes
(In reply to Nick Clemens from comment #28) > Adds the UpdateItemLocationOnCheckin to svc/checkin Why is it needed? svc/checkin calls AddReturn (which deals with UpdateItemLocationOnCheckin).
Created attachment 54644 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 54645 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 54646 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 54647 [details] [review] Bug 14576 (QA Folllowup) Address issues use mock for tests use a separate variable for yaml fix indentation
Created attachment 54648 [details] [review] Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart This patch eliminates ReturnToShelvingCart and InProcessingToShelvingCart, Deletes Returns.t (it only tested those two prefs and new tests are in issue.t) Remaining instances found by: git grep "ReturnToShelvingCart\|InProcessingToShelvingCart" | grep -v translator | wc -l are db update files and release notes
(In reply to Jonathan Druart from comment #29) > (In reply to Nick Clemens from comment #28) > > Adds the UpdateItemLocationOnCheckin to svc/checkin > > Why is it needed? > svc/checkin calls AddReturn (which deals with UpdateItemLocationOnCheckin). Yeah....nevermind that bit, removed
Created attachment 54654 [details] [review] Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart This patch eliminates ReturnToShelvingCart and InProcessingToShelvingCart, Deletes Returns.t (it only tested those two prefs and new tests are in issue.t) Remaining instances found by: git grep "ReturnToShelvingCart\|InProcessingToShelvingCart" | grep -v translator | wc -l are db update files and release notes
Hi Nick, starting with a first code review here: 1) There seem to be a few lines doubled up in AddIssue now: $item->{'issues'}++; if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) { UpdateTotalIssues($item->{'biblionumber'}, 1); } $item->{'issues'}++; if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) { UpdateTotalIssues( $item->{'biblionumber'}, 1 ); } 2) This line is a change of behaviour: Before the change was dependent on the system preference - now it will always happen? - CartToShelf( $itemnumber ) if ( C4::Context->preference("ReturnToShelvingCart") ); + CartToShelf( $itemnumber ) if ( $item->{'location'} eq 'CART' && $item->{'permanent_location'} ne 'CART' ); 3) ReturnToShelvingCart is removed from the code, but the system preference is not deleted in the database update. 4) koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js contains strings - those can not be translated (js files can't be in general) 5) Tests are failing for me (run within koha-shell in kohadevbox): kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ perl t/db_dependent/Circulation/issue.t Global symbol "$item" requires explicit package name at /home/vagrant/kohaclone/C4/Items.pm line 635. Global symbol "$item" requires explicit package name at /home/vagrant/kohaclone/C4/Items.pm line 635. Can you please take a look?
Created attachment 55587 [details] [review] Bug 14576 (QA Followup) Fix undelcared use of $item Move strings from js files Delete duplicated lines
(In reply to Katrin Fischer from comment #37) > starting with a first code review here: > 1) There seem to be a few lines doubled up in AddIssue now: Fixed > 2) This line is a change of behaviour: Before the change was dependent on > the system preference - now it will always happen? In practice the only way to have the scenario of location=CART and permanentlocation!=CART was via the syspref and is now via the new syspref - I don't think we are acutally changing behaviour here > 3) ReturnToShelvingCart is removed from the code, but the system preference > is not deleted in the database update. Not sure what you mean? Below is in atomic update DELETE FROM systempreferences WHERE variable='ReturnToShelvingCart'; > 4) koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js contains strings - > those can not be translated (js files can't be in general) Ah, yes, moved > 5) Tests are failing for me (run within koha-shell in kohadevbox): Fixed!
Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14576 - Allow arbitrary automatic update of location on checkin Applying: Bug 14576 - Unit tests Applying: Bug 14576 - (QA followup) Fix message for use of value not in authorized values Applying: Bug 14576 (QA Folllowup) Address issues Applying: Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart fatal: sha1 information is lacking or useless (C4/Circulation.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart
Created attachment 60658 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60659 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60660 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 60661 [details] [review] Bug 14576 (QA Folllowup) Address issues use mock for tests use a separate variable for yaml fix indentation
Created attachment 60662 [details] [review] Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart This patch eliminates ReturnToShelvingCart and InProcessingToShelvingCart, Deletes Returns.t (it only tested those two prefs and new tests are in issue.t) Remaining instances found by: git grep "ReturnToShelvingCart\|InProcessingToShelvingCart" | grep -v translator | wc -l are db update files and release notes
Created attachment 60663 [details] [review] Bug 14576 (QA Followup) Fix undelcared use of $item Move strings from js files Delete duplicated lines
Tests do not pass. C4::Items::ShelfToCart is no longer in used.
Created attachment 60747 [details] [review] Bug 14576 (QA Followup) Remove C4:::Items::ShelfToCart
Created attachment 60748 [details] [review] Bug 14576 (QA Followup) Fix issue.t
(In reply to Jonathan Druart from comment #47) > C4::Items::ShelfToCart is no longer in used. Removed > Tests do not pass. Indeed, Returns.t is failing, but I have no idea why - the expected and received warning look the same to me - I will try again tomorrow
Created attachment 60751 [details] [review] Bug 14576 - (QA Followup) Fix Returns.t Two warnings, not one
There are now a lot of warnings from t/db_dependent/Circulation/issue.t: item-level_itypes set but no itemtype set for item (xxxx)
Created attachment 61940 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 61941 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 61942 [details] [review] Bug 14576 - (QA followup) Fix message for use of value not in authorized values Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 61944 [details] [review] Bug 14576 (QA Folllowup) Address issues use mock for tests use a separate variable for yaml fix indentation
Created attachment 61945 [details] [review] Bug 14576 - (QA Followup) Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart This patch eliminates ReturnToShelvingCart and InProcessingToShelvingCart, Deletes Returns.t (it only tested those two prefs and new tests are in issue.t) Remaining instances found by: git grep "ReturnToShelvingCart\|InProcessingToShelvingCart" | grep -v translator | wc -l are db update files and release notes
Created attachment 61946 [details] [review] Bug 14576 (QA Followup) Fix undelcared use of $item Move strings from js files Delete duplicated lines
Created attachment 61947 [details] [review] Bug 14576 (QA Followup) Remove C4:::Items::ShelfToCart
Created attachment 61948 [details] [review] Bug 14576 (QA Followup) Fix issue.t
Created attachment 61949 [details] [review] Bug 14576 - (QA Followup) Fix Returns.t Two warnings, not one
Created attachment 61950 [details] [review] Bug 14576 (QA Followup) Fix warnings in tests
Could you squash the QA follow-ups ?
Created attachment 64656 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 64657 [details] [review] Bug 14576 - Unit tests Signed-off-by: Cathi Wiggins <cwiggins@ArcadiaCA.gov> Signed-off-by: Megan Wianecki <mwianecki@mtpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 64658 [details] [review] Bug 14576 - (QA followups squashed) Fix message for use of value not in authorized values use mock for tests use a separate variable for yaml fix indentation Remove other instances of ReturnToShelvingCart/InprocessingToShelvingCart Deletes Returns.t (it only tested those two prefs and new tests are in issue.t) Fix undelcared use of $item Move strings from js files Delete duplicated lines Remove C4:::Items::ShelfToCart Fix issue.t Fix Returns.t - Two warnings, not one Fix warnings in tests
Created attachment 67610 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/)
Created attachment 67611 [details] [review] Bug 14576: Unit tests
Hi Nick A couple of points I have found whilst testing your first patch; 1) Can you clarify the _ALL_ value because when I set this on the left hand side e.g _ALL_: FIC FIC: GEN The only item that was set to FIC was the item was was checking in, not all items. 2) I entered incorrect syntax such as !!! into the pref and I was not warned instead this invalid syntax was saved
(In reply to Alex Buckley from comment #69) > 1) Can you clarify the _ALL_ value because when I set this on the left hand > side e.g /_ALL_: FIC/ FIC: GEN > The only item that was set to FIC was the item was was checking in, not all > items. That is the correct behaviour - all items that are checked in, meaning regardless of original location, set the location to 'FIC' (it does NOT mean 'all items on the record') > 2) I entered incorrect syntax such as !!! into the pref and I was not warned > instead this invalid syntax was saved Hmm..I am warned everytime - but you are only warned on change of pref and not prevented from saving. On bad data the pref will simply not work any more and a warn in the logs. I think this should be sufficient - the text is very explanatory, the warning should give you pause, and it only works if set right
With the patch applied the message "Not checked out." appears twice when returning an item that has not been checked out. QA tools also highlight some issues: FAIL C4/Circulation.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling FAIL valid "my" variable $item masks earlier declaration in same scope FAIL C4/Items.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK spelling OK valid FAIL C4/Reserves.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling FAIL valid "my" variable $item masks earlier declaration in same scope OK C4/UsageStats.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling OK valid FAIL circ/returns.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid "my" variable $item masks earlier declaration in same scope OK installer/data/mysql/sysprefs.sql OK git manipulation OK semicolon OK sysprefs_order OK koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template OK koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template OK koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/authorised_values.tt OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template FAIL t/db_dependent/Circulation/Returns.t OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid "my" variable $item masks earlier declaration in same scope FAIL t/db_dependent/Circulation/issue.t OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid "my" variable $item masks earlier declaration in same scope FAIL t/db_dependent/UsageStats.t OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid "my" variable $item masks earlier declaration in same scope
Created attachment 68297 [details] [review] Bug 14576: Followup Fix (remove) extra call for item Fix rebase error code duplication
(In reply to Katrin Fischer from comment #71) > With the patch applied the message "Not checked out." appears twice when > returning an item that has not been checked out. Ah, rebase duplication, cleared > QA tools also highlight some issues: Fixed, pod coverage is false positive (I remove POD because I remove a function)
Hi Nick, can you please rebase? JavaScript on the preferences template has since be moved to the bottom. Thx!
Created attachment 70795 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/)
Created attachment 70796 [details] [review] Bug 14576: Unit tests
Created attachment 70797 [details] [review] Bug 14576: Followup Fix (remove) extra call for item Fix rebase error code duplication
Created attachment 72032 [details] [review] Bug 14576 - Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/)
Created attachment 72033 [details] [review] Bug 14576: Unit tests
Created attachment 72034 [details] [review] Bug 14576: Followup Fix (remove) extra call for item Fix rebase error code duplication
Created attachment 75661 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/)
Created attachment 75662 [details] [review] Bug 14576: Unit tests
Created attachment 75663 [details] [review] Bug 14576: (follow-up) Fix (remove) extra call for item Fix rebase error code duplication
does not apply :( M C4/Circulation.pm M C4/Items.pm M C4/Reserves.pm M C4/UsageStats.pm M circ/returns.pl M installer/data/mysql/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref M koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt M koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/authorised_values.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/authorised_values.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt Auto-merging installer/data/mysql/sysprefs.sql Auto-merging circ/returns.pl Auto-merging C4/UsageStats.pm Auto-merging C4/Reserves.pm Auto-merging C4/Items.pm Auto-merging C4/Circulation.pm Failed to merge in the changes. Patch failed at 0001 Bug 14576: Allow arbitrary automatic update of location on checkin
Created attachment 79245 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/)
Created attachment 79246 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass
Created attachment 79995 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 79996 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Nick, why do you prefer to _ALL_ rule override everything? Shouldn't the most specific rule be the first taken into account and _ALL_ rule used as fallback? I imagine it could simplify the setting on systems with many locations.
Hi Nick Can you please rebase? The conflict against master to be resolved is only small but git-bz fails to merge it automatically. Cheers, Alex
Applying: Bug 14576: Allow arbitrary automatic update of location on checkin Using index info to reconstruct a base tree... M C4/Items.pm M C4/Reserves.pm M installer/data/mysql/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref M koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging installer/data/mysql/sysprefs.sql Auto-merging C4/Reserves.pm Auto-merging C4/Items.pm CONFLICT (content): Merge conflict in C4/Items.pm Failed to merge in the changes. Patch failed at 0001 Bug 14576: Allow arbitrary automatic update of location on checkin The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/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-14576-Allow-arbitrary-automatic-update-of-loca-FkmjWf.patch
Created attachment 83113 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 83114 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 83115 [details] [review] Bug 14576: Update to use get_yaml_pref_hash and perl update statement
(In reply to Josef Moravec from comment #89) > Nick, why do you prefer to _ALL_ rule override everything? Shouldn't the > most specific rule be the first taken into account and _ALL_ rule used as > fallback? I imagine it could simplify the setting on systems with many > locations. I suppose my thought was that _ALL_ could be used to circumvent all others in a scenario where you had a closing or something. I am not sure I envisioned a scenario where ALL would be used constantly
Created attachment 83696 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 83697 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 83698 [details] [review] Bug 14576: Update to use get_yaml_pref_hash
Starting another round of review with reading code and comments: 1) This example in the system preference is wrong (appears in a few spots): E.g. ''PROC: FIC'' will cause an item that was set to ''Book Cart'' Should e: CART: FIC 2) Missing dot, also: PROC not? Items in the CART location will be returned to their permanent location on checkout 3) Use <b> instead of ** to highlight: **Use of an _ALL_ rule will override/ignore any other values** 4) Not really about this patch, but stil trying to see how it all fits together: NewItemsDefaultLocation referst to the cart_to_shelf cronjob, but I think this might be incorrect. I will continue testing, just some first notes and all easy to fix!
Created attachment 85406 [details] [review] Bug 14576: (follow-up) fix some typos and style
(In reply to Katrin Fischer from comment #99) > 1) This example in the system preference is wrong (appears in a few spots): Updated the description to say 'Processing center' > 2) Missing dot, also: PROC not? CART are always returned to permanent on checkout - proc is only update if included in this pref now (used to be syspref controlled by InProcessingToShelvingCart) kout > 3) Use <b> instead of ** to highlight: Done > 4) Not really about this patch, but stil trying to see how it all fits > together: > NewItemsDefaultLocation referst to the cart_to_shelf cronjob, but I think > this might be incorrect. cart_to_shelf stil exists - if you set items to be returned to the cart then you still need to reset them to original location after they are returned (or add a rule to move from CART to PERM and check them in again)
> > 4) Not really about this patch, but stil trying to see how it all fits > > together: > > NewItemsDefaultLocation referst to the cart_to_shelf cronjob, but I think > > this might be incorrect. > cart_to_shelf stil exists - if you set items to be returned to the cart then > you still need to reset them to original location after they are returned > (or add a rule to move from CART to PERM and check them in again) I know it does still exist :) But I think this pref isn't about the cart? What if I chose 'new item shelf' as my default location? Or PROC.
(In reply to Katrin Fischer from comment #102) > > > 4) Not really about this patch, but stil trying to see how it all fits > > > together: > > > NewItemsDefaultLocation referst to the cart_to_shelf cronjob, but I think > > > this might be incorrect. > > cart_to_shelf stil exists - if you set items to be returned to the cart then > > you still need to reset them to original location after they are returned > > (or add a rule to move from CART to PERM and check them in again) > > I know it does still exist :) But I think this pref isn't about the cart? > What if I chose 'new item shelf' as my default location? Or PROC? Discussion for a different bug.
Sorry, Nick, can you please rebase?
Created attachment 85850 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 85851 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 85852 [details] [review] Bug 14576: Update to use get_yaml_pref_hash
Created attachment 85853 [details] [review] Bug 14576: (follow-up) fix some typos and style
(In reply to Katrin Fischer from comment #104) > Sorry, Nick, can you please rebase? Rebased. Please note: QA complaint of POD is false positive (coverage decreased because subroutine removed) The ModItem calls made for location change are logged - some calls were silenced recently to avoid spamming the logs, if we are updating because of a location change though it should be logged I believe
Nick, can you rebase it again? I get: Patch failed at 0001 Bug 14576: Allow arbitrary automatic update of location on checkin The copy of the patch that failed is found in: .git/rebase-apply/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-14576-Allow-arbitrary-automatic-update-of-loca-9ePdA7.patch Thank you.
Created attachment 86812 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 86813 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 86814 [details] [review] Bug 14576: Update to use get_yaml_pref_hash
Created attachment 86815 [details] [review] Bug 14576: (follow-up) fix some typos and style
Created attachment 86816 [details] [review] Bug 14576: Rebase fixes Fix test Rebase code
This patch fails when checking in an item, due to use of a deprecated method GetItem (removed in patch 21206) in C4/Items.pm. This method is also called in C4/Reserves.pm. This should be changed to Koha::Items->find function.
Created attachment 87040 [details] [review] Bug 14576: Replaced calls using deprecated method GetItem with Koha::Items->find Test plan: 1) Apply all patches except this one 2) Checkout out an item to a patron 3) Check the item in - note that this will fail 4) Apply this patch, and repeat steps 1-3, noting instead that the the check is instead successful 5) Verify also that reserves act as expected 5) Sign off Sponsored-by: Catalyst IT
Created attachment 87061 [details] [review] Bug 14576: (follow-up) Using item objects, not hashrefs
Created attachment 87144 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87145 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87146 [details] [review] Bug 14576: Update to use get_yaml_pref_hash Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87147 [details] [review] Bug 14576: (follow-up) fix some typos and style Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87148 [details] [review] Bug 14576: Rebase fixes Fix test Rebase code Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87149 [details] [review] Bug 14576: Replaced calls using deprecated method GetItem with Koha::Items->find Test plan: 1) Apply all patches except this one 2) Checkout out an item to a patron 3) Check the item in - note that this will fail 4) Apply this patch, and repeat steps 1-3, noting instead that the the check is instead successful 5) Verify also that reserves act as expected 5) Sign off Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87150 [details] [review] Bug 14576: (follow-up) Using item objects, not hashrefs Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 87151 [details] [review] Bug 14576: (QA Follow up) Tidy up feature documentation To test: Apply this patch with all of the others run the updatedatabase, it should work install a new Koha, this pref should be installed and look fine in the interface. Check the syspref, make sure the text looks ok and makes sense. Bonus points: for checking to make sure PROC: _PERM_ appears in the new pereference if InProcessingToShelvingCart was set for checking to make sure _ALL_: CART appears in the new preferenec if ReturnToShelvingCart was previously set.
I note there is a QA tool false positive here: FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 POD coverage decreased because a sub was removed.
Hi Liz, can you please write (QA follow-up)? The other spelling makes the QA tools unhappy (consistency...).
Created attachment 87925 [details] [review] Bug 14576: (QA follow-up) Tidy up feature documentation To test: Apply this patch with all of the others run the updatedatabase, it should work install a new Koha, this pref should be installed and look fine in the interface. Check the syspref, make sure the text looks ok and makes sense. Bonus points: for checking to make sure PROC: _PERM_ appears in the new pereference if InProcessingToShelvingCart was set for checking to make sure _ALL_: CART appears in the new preferenec if ReturnToShelvingCart was previously set.
Created attachment 88451 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88452 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88453 [details] [review] Bug 14576: Update to use get_yaml_pref_hash Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88454 [details] [review] Bug 14576: (follow-up) fix some typos and style Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88455 [details] [review] Bug 14576: Rebase fixes Fix test Rebase code Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88456 [details] [review] Bug 14576: Replaced calls using deprecated method GetItem with Koha::Items->find Test plan: 1) Apply all patches except this one 2) Checkout out an item to a patron 3) Check the item in - note that this will fail 4) Apply this patch, and repeat steps 1-3, noting instead that the the check is instead successful 5) Verify also that reserves act as expected 5) Sign off Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88457 [details] [review] Bug 14576: (follow-up) Using item objects, not hashrefs Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88458 [details] [review] Bug 14576: (QA follow-up) Tidy up feature documentation To test: Apply this patch with all of the others run the updatedatabase, it should work install a new Koha, this pref should be installed and look fine in the interface. Check the syspref, make sure the text looks ok and makes sense. Bonus points: for checking to make sure PROC: _PERM_ appears in the new pereference if InProcessingToShelvingCart was set for checking to make sure _ALL_: CART appears in the new preferenec if ReturnToShelvingCart was previously set.
Created attachment 88459 [details] [review] Bug 14576: (follow-up) Add code highlighting to system preference UpdateItemLocationOnCheckin Test plan: 1) Go to system preferences and find preference UpdateItemLocationOnCheckin 2) Ensure the yaml code higlighting is used on textarea when editing this setting
Rebased and added code highlighting on syspref UpdateItemLocationOnCheckin
Created attachment 88646 [details] [review] Bug 14576: Allow arbitrary automatic update of location on checkin This patch adds a new syspref "UpdateItemLocationOnCheckin" which accepts pairs of shelving locations. On check-in the items location is compared ot the location on the left and, if it matches, is updated to the location on the left. This preference replaces ReturnToShelvingCart and InProcessingToShelvingCart preferences. The update statement should insert values that replciate these functions. Note existing functionality of all items in PROC location being returned to permanent_location is preserved by default. Also, any items issued from CART location will be returned to their permanent location on issue (if it differs) Special values for this pref are: _ALL_ - used on left side only to affect all items _BLANK_ - used on either side to match on/set to blank (actual blanks will work, but this is an easier to read option) _PERM_ - used on right side only to return items to permanent location Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Set the new system preference UpdateitemLocationOnCheckin to the following (assuming sample data): NEW: FIC FIC: GEN 4) Create an item, set its location to NEW 5) Check in the item, note its location is now FIC 6) Check in the item again, note its location is now GEN 7) Check in the item again, note its location remains GEN 8) Test using _ALL_, _BLANK_ and _PERM_ for updates 9) Try entering various incorrect syntax in the pref and note you are warned Sponsored by: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88647 [details] [review] Bug 14576: Unit tests To test: 1 - prove -v t/db_dependent/Circulation/Returns.t 2 - prove -v t/db_dependent/Circulation/issue.t 3 - prove -v t/db_dependent/UsageStats.t 4 - All should pass Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88648 [details] [review] Bug 14576: Update to use get_yaml_pref_hash Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88649 [details] [review] Bug 14576: (follow-up) fix some typos and style Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88650 [details] [review] Bug 14576: Rebase fixes Fix test Rebase code Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88651 [details] [review] Bug 14576: Replaced calls using deprecated method GetItem with Koha::Items->find Test plan: 1) Apply all patches except this one 2) Checkout out an item to a patron 3) Check the item in - note that this will fail 4) Apply this patch, and repeat steps 1-3, noting instead that the the check is instead successful 5) Verify also that reserves act as expected 5) Sign off Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88652 [details] [review] Bug 14576: (follow-up) Using item objects, not hashrefs Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88653 [details] [review] Bug 14576: (QA follow-up) Tidy up feature documentation To test: Apply this patch with all of the others run the updatedatabase, it should work install a new Koha, this pref should be installed and look fine in the interface. Check the syspref, make sure the text looks ok and makes sense. Bonus points: for checking to make sure PROC: _PERM_ appears in the new pereference if InProcessingToShelvingCart was set for checking to make sure _ALL_: CART appears in the new preferenec if ReturnToShelvingCart was previously set. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 88654 [details] [review] Bug 14576: (follow-up) Add code highlighting to system preference UpdateItemLocationOnCheckin Test plan: 1) Go to system preferences and find preference UpdateItemLocationOnCheckin 2) Ensure the yaml code higlighting is used on textarea when editing this setting Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 19.05
Enhancement will not be backported to 18.11.x series.
Not all the sponsored develo are shown in the release notes - from comment 140 should include: Arcadia Public Library (http://library.ci.arcadia.ca.us/) Middletown Township Public Library (http://www.mtpl.org/) Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/)
(In reply to David Nind from comment #151) > Not all the sponsored develo are shown in the release notes - from comment > 140 should include: > Arcadia Public Library (http://library.ci.arcadia.ca.us/) > Middletown Township Public Library (http://www.mtpl.org/) > Round Rock Public Library > (https://www.roundrocktexas.gov/departments/library/) I guess it should have been 3 different lines to have made them retrieved correctly from the release notes script.
https://koha-community.org/koha-19-05-release/ NONSENSICAL explanation in release note: " On check in the item’s location is compared to the location on the left and, if it matches, is updated to the location on the left. " (2x left) in system preferences rules is better version: "General rule: if the location value on the left of the colon (:) matches the item's current location, it will be updated to match the location value on the right of the colon (:)."
Fixed the 'release notes' field here in bugzilla, not sure we can do more at this point.
Somewhere an extra defined test on item->location has been introduced to this feature, which effectively causes location NULL items to be skipped from marking them with CART. Probably only with the good intention to remove a warn, but adding this side-effect.
(In reply to Marcel de Rooy from comment #155) > Somewhere an extra defined test on item->location has been introduced to > this feature, which effectively causes location NULL items to be skipped > from marking them with CART. > Probably only with the good intention to remove a warn, but adding this > side-effect. This has been fixed on bug 28472