Description
Kyle M Hall (khall)
2012-09-27 14:29:47 UTC
Some relevant links: Initial commit adding the feature in January 2010: http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=f650aad34b00da6fd44d974b04975b2987cdaaef Fixed: Bug 6653 - [security] Rotating collections pages can be accessed without being logged in See relevant comments at the bottom here: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=circ/returns.pl;hb=HEAD#l615 Created attachment 12560 [details] [review] Bug 8836 - Resurrect Rotating Collections Apart from the security concern, the major concern is how this would play with holds initiated transfers. So that must be tested thoroughly when testing this patch The good news is that the rotating collections feature doesn't do anything except tell a library that the item is part of a rotating collection and that the collection is currently at the given library. So, there should be no worries about it messing with transfers. Patch tested with a sandbox, by jmbroust <jean-manuel.broust@univ-lyon2.fr> Created attachment 16746 [details] [review] Bug 8836 - Resurrect Rotating Collections Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Note that if you have syspref "automaticitemreturn" ON (DO automatic return...) and HomeOrHoldingBranchReturn turned to : "the library the item is from", then when the library you transfer items receives items (doing a check in) the items will go back to homelibrary automaticaly. There should be a way to avoid that (specific syspref ? to allow to keep items in the holdingbranch if they make part of a rotating collection). I note that while this patch is fairly harmless, rotating_collections/rotatingCollections.pl still has at least one problem require Exporter; I think the other scripts have this problem too. Created attachment 19118 [details] [review] Bug 8836 - Resurrect Rotating Collections - QA Followup Created attachment 19119 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Chris, is this QA follow up what you are looking for? (In reply to comment #8) > I note that while this patch is fairly harmless, > > rotating_collections/rotatingCollections.pl still has at least one problem > > require Exporter; > > I think the other scripts have this problem too. Hi Kyle, I noticed a problem in the rotating collections templates while working on the po files: "[ editColDescription %]" in editCollections.tt. Hi Kyle: Any recent work on this one? I recently got a question about rotating collections, so it would be great to see this feature make its way back into Koha. Kyle: Do you have a test plan for this patch? Others: What are the current reservations about this patch? Created attachment 19337 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 3 - Fix bad TT tag Created attachment 19339 [details] [review] Bug 8836 - Resurrect Rotating Collections - QA Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 8836 - Resurrect Rotating Collections Applying: Bug 8836 - Resurrect Rotating Collections - QA Followup Applying: Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts fatal: sha1 information is lacking or useless (C4/RotatingCollections.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Created attachment 21008 [details] [review] Bug 8836 - Resurrect Rotating Collections Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Created attachment 21009 [details] [review] Bug 8836 - Resurrect Rotating Collections - QA Followup Created attachment 21010 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Created attachment 21011 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin Hi Kyle, the QA script found something: FAIL circ/returns.pl OK pod OK forbidden patterns FAIL valid "my" variable $itemnumber masks earlier declaration in same scope OK critic Could you check? We need this functionality as well. Is there some way I can help? (In reply to Olli-Antti Kivilahti from comment #22) > We need this functionality as well. Is there some way I can help? More testing and another signoff never hurts. ;-) You could test the feature some more and add an additional sign-off :) Could I have a test plan for this feature? I have no idea where to start looking or how to operate this? Rotating collections test case. In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyöjä / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyöjä / to <original trasfer destination>" Also unit tests are required. This problem is actually an issue with Koha's transfer implementation, and is not directly related to this feature. (In reply to Olli-Antti Kivilahti from comment #26) > Rotating collections test case. > > In "Tools" -> "Rotating Collections" -> "Add/Remove items": > When adding item barcodes to the collection, the input field > <input type="text" id="barcode" name="barcode"> > should become active > automatically so it is easy to add multiple barcodes rapidly without > touching the > mouse or keyboard. > Enter-press is dealt by the barcode reader so automatic form submittal > should be handled > by the barcode reader. > > In "Rotating collections" -> "Transfer Collection": > When the collection is initially transferred, items are set to trasfer > correctly. > When the collection is transferred while items are still being transferred, > the transfer > destination library doesn't change from the original one. > The holding library changes for all items in the collection to the > destination library on > retransfers as well. > This is tricky if a user accidentally places the wrong destination. > When I try to checkin these items to their new retransfer location, I get > the following messages: > -"This item is part of a rotating collection and needs to be transferred to > <original transfer destination>" > -"Please return Valkoinen ihmissyöjä / to <original transfer destination>" > -"Print slip or Cancel transfer" > > When I checkin a Item to a arbitrary branch, I get the following messages: > "This item is part of a rotating collection and needs to be transferred to > <retransfer destination>" > "Please return Valkoinen ihmissyöjä / to <original trasfer destination>" While I agree that we should try to add unit tests, these patches do not add any new perl subroutines. There are many existing modules that do not have unit tests.
> Also unit tests are required.
Created attachment 23966 [details] [review] Bug 8836 - Resurrect Rotating Collections Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Created attachment 23967 [details] [review] Bug 8836 - Resurrect Rotating Collections - QA Followup Created attachment 23968 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Created attachment 23969 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin Created attachment 23970 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Created attachment 23971 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Created attachment 23976 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. (In reply to Kyle M Hall from comment #29) > While I agree that we should try to add unit tests, these patches do not add > any new perl subroutines. There are many existing modules that do not have > unit tests. > > > Also unit tests are required. Course you said that as a developer who didn't want to write tests, not a member of the QA team eh ? :) I think it is stretching just a bit too far to have 1 out of six patches being signed off resulting in an overall bug status of signed off. Also, follow-up 5 doesn't seem to apply. More to the point, the collections.colbranchcode column is too narrow. It should be varchar(10) and a FK linking to the branches table. Failing QA on that basis; I further request that you change the status to needs signoff, not signed off, when you submit a follow-up. Also, given the age of the original code and the fact that it has not be in general use, there's even more call to write unit tests. Created attachment 28140 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Created attachment 28141 [details] [review] Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode (In reply to Galen Charlton from comment #38) > More to the point, the collections.colbranchcode column is too narrow. It > should be varchar(10) and a FK linking to the branches table. Failing QA on > that basis; I further request that you change the status to needs signoff, > not signed off, when you submit a follow-up. Fixed! > Also, given the age of the original code and the fact that it has not be in > general use, there's even more call to write unit tests. While this is true, the Crawford County Federated Library system has been using this feature for since 2007 with no issues. I also find the existing set of rotating collections unit tests to be quite thorough. Hello, we're looking at bringing this feature into production and I've been going through this component - a trivial issue in additems.tt -template: [% IF ( previousActionRemove ) %] [% IF ( removeSuccess ) %] <div>Item with barcode '[% addedBarcode %]' Removed successfully!</div> [% ELSE %] <div>Failed to remove item with barcode '[% removedBarcode %]'!</div> <div>Reason: <strong>[% failureMessage %]</strong></div> [% END %] [% END %] The parameter addedBarcode is used here when it probably was supposed to be removedBarcode - this leads to the barcode not being displayed after a succesful item removal. Applying: Bug 8836 - Resurrect Rotating Collections Applying: Bug 8836 - Resurrect Rotating Collections - QA Followup Applying: Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Using index info to reconstruct a base tree... <stdin>:59: trailing whitespace. "INSERT INTO collections ( colId, colTitle, colDesc ) <stdin>:211: trailing whitespace. "SELECT <stdin>:373: trailing whitespace. "DELETE FROM collections_tracking <stdin>:424: trailing whitespace. "SELECT barcode FROM items, collections_tracking <stdin>:471: trailing whitespace. "SELECT holdingbranch, colBranchcode FROM items, collections, collections_tracking warning: 5 lines add whitespace errors. Falling back to patching base and 3-way merge... Auto-merging rotating_collections/transferCollection.pl CONFLICT (content): Merge conflict in rotating_collections/transferCollection.pl Auto-merging rotating_collections/rotatingCollections.pl CONFLICT (content): Merge conflict in rotating_collections/rotatingCollections.pl Auto-merging rotating_collections/editCollections.pl CONFLICT (content): Merge conflict in rotating_collections/editCollections.pl Auto-merging rotating_collections/addItems.pl CONFLICT (content): Merge conflict in rotating_collections/addItems.pl Failed to merge in the changes. Patch failed at 0001 Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts 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-8836---Resurrect-Rotating-Collections---Follow-DX51L_.patch Created attachment 30838 [details] [review] Bug 8836 - Resurrect Rotating Collections This patch has all the previous rotating collections patched squashed together for convenience. I'm posting this as I have a fairly large enhancement patch lying around for this component that revamps the main views with new features & ajax-calls for more efficient usage and fewer page reloads. We also had a requirement where the transfer location should be defined on the item level instead on the collection level. Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Could not apply on VM or sandbox Applying: Bug 8836 - Resurrect Rotating Collections - QA Followup Applying: Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts /home/nick/koha/.git/rebase-apply/patch:59: trailing whitespace. "INSERT INTO collections ( colId, colTitle, colDesc ) /home/nick/koha/.git/rebase-apply/patch:211: trailing whitespace. "SELECT /home/nick/koha/.git/rebase-apply/patch:373: trailing whitespace. "DELETE FROM collections_tracking /home/nick/koha/.git/rebase-apply/patch:424: trailing whitespace. "SELECT barcode FROM items, collections_tracking /home/nick/koha/.git/rebase-apply/patch:471: trailing whitespace. "SELECT holdingbranch, colBranchcode FROM items, collections, collections_tracking fatal: sha1 information is lacking or useless (rotating_collections/addItems.pl). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts 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-8836---Resurrect-Rotating-Collections---Follow-hvjidv.patch (In reply to Nick Clemens from comment #45) > Could not apply on VM or sandbox > > Applying: Bug 8836 - Resurrect Rotating Collections - QA Followup > Applying: Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy > rotating collections scripts > /home/nick/koha/.git/rebase-apply/patch:59: trailing whitespace. > "INSERT INTO collections ( colId, colTitle, colDesc ) > /home/nick/koha/.git/rebase-apply/patch:211: trailing whitespace. > "SELECT > /home/nick/koha/.git/rebase-apply/patch:373: trailing whitespace. > "DELETE FROM collections_tracking > /home/nick/koha/.git/rebase-apply/patch:424: trailing whitespace. > "SELECT barcode FROM items, collections_tracking > /home/nick/koha/.git/rebase-apply/patch:471: trailing whitespace. > "SELECT holdingbranch, colBranchcode FROM items, collections, > collections_tracking > fatal: sha1 information is lacking or useless > (rotating_collections/addItems.pl). > Repository lacks necessary blobs to fall back on 3-way merge. > Cannot fall back to three-way merge. > Patch failed at 0001 Bug 8836 - Resurrect Rotating Collections - Followup 2 > - Perltidy rotating collections scripts > 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-8836---Resurrect-Rotating-Collections---Follow-hvjidv.patch Did you mean to test the squashed patch? I didn't set anything obsolete because I'm unsure as to whether squashing these old patches like this is actually welcome at all - I was hoping for comments and reactions. I guess I shouldn't have a set "needs signoff" - sorry. I can fix the old separate patches if that's how they should be kept. Created attachment 30921 [details] [review] Bug 8836 - Resurrect Rotating Collections: additions This is patch is an enchancement patch based on our requirements (sending in hopes that it could be useful to someone) and is ment to be applied on top of my squashed rotating collections patch and focuses mainly on the interface, aiming to streamline workflow and and to reduce unnecessary page reloads. Core rotating collection functionality is also changed in that transfers are now handled on the item level instead of on the collection level. This allows individual items in the collection to have their own transfer branches - you can still of course have all items transferred to the same destination by using the collection transfer feature in the main view or in the additems-view. Note: as this is a fairly large change in how collections are handled, this patch cannot be used directly with collections managed by the old system. List of larger changes (of the top of my head): - transferCollection-view has been made redundant with the addition of a transfer button and branch select input in the main view. - editCollections-view has been made redundant with addition of the "edit" button in the main view. Note that "Edit collection" still exists - datatables used in all the views. - The main view table has been revamped and some columns have been moved around or removed. New columns: Owner(branch of the signed in user on creation), Items(count), Transferred(how many items in collection are transferred), Transfer collection, Edit and Delete. - New collections can be created directly from the main view using the "New collection"-button. - Item's original home branch (before a transfer) is now recorded on transfer (a new database column 'origin_branchcode' in collection_tracking). The old collection-level transfer branch in collections is no longer needed but is not touched by this patch. - Item's transfer location is now recored on item-level basis (a new database column: 'transfer_branch). - Item's tranfer status is now recorded (a new database column in collection_tracking: 'transferred'). - When removing a collection, all items (if any) in that collection get removed from collection_tracking and returned to their origin branch. - addItems-view: - Table now shows the item's origin branch, home branch and current loc. In addition to these, new columns: "Transferred", "Transfer", "Return" and "Remove". - Item's title and barcode are now links to the relevant page in catalogue. - Buttons for individual item transfer, return and removal. The transfer button can be used to transfer an individual item to a branch (branch selection happens in a bootsrap modal-view). The return button can be used to return an item to its origin branch. The remove button allows removing an item from the collection without the need for reading a barcode. - A button for whole collection transfer. Sadly, this is *not* a complete list of changes. Created attachment 31087 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> The squashing was fine, except you removed my authorship from the patches! I've uploaded a fixed version. In addition, your patch makes large changes, so I've moved it to a separate bug and made this one a blocker for it. The biggest problems with your patch are 1) There is no test plan 2) The unit tests for rotating collections fail with it applied (In reply to Juhani Seppälä from comment #46) > Did you mean to test the squashed patch? I didn't set anything obsolete > because I'm unsure as to whether squashing these old patches like this is > actually welcome at all - I was hoping for comments and reactions. I guess I > shouldn't have a set "needs signoff" - sorry. I can fix the old separate > patches if that's how they should be kept. (In reply to Kyle M Hall from comment #49) > The squashing was fine, except you removed my authorship from the patches! > I've uploaded a fixed version. In addition, your patch makes large changes, > so I've moved it to a separate bug and made this one a blocker for it. > > The biggest problems with your patch are > 1) There is no test plan > 2) The unit tests for rotating collections fail with it applied > > (In reply to Juhani Seppälä from comment #46) > > Did you mean to test the squashed patch? I didn't set anything obsolete > > because I'm unsure as to whether squashing these old patches like this is > > actually welcome at all - I was hoping for comments and reactions. I guess I > > shouldn't have a set "needs signoff" - sorry. I can fix the old separate > > patches if that's how they should be kept. The patch authorship thing was, probably among other stuff, something I completely missed at the time - sorry about that. I agree with the separate bug and am aware of the missing test stuff. Hi Kyle, there have been lots of follow-ups on this, would you be ok with asking for another sign-off? This is a bigger one and I'd feel better with some additional testing. There are also a couple of problems pointed out by the QA script at this point: FAIL circ/returns.pl OK critic OK forbidden patterns OK pod FAIL valid "my" variable $itemnumber masks earlier declaration in same scope FAIL rotating_collections/addItems.pl OK critic FAIL forbidden patterns forbidden pattern: You should not use a .tmpl extension for the template name (see bug 11349) (line 33) OK pod OK valid FAIL rotating_collections/editCollections.pl OK critic FAIL forbidden patterns forbidden pattern: You should not use a .tmpl extension for the template name (see bug 11349) (line 33) OK pod OK valid FAIL rotating_collections/rotatingCollections.pl OK critic FAIL forbidden patterns forbidden pattern: You should not use a .tmpl extension for the template name (see bug 11349) (line 32) OK pod OK valid FAIL rotating_collections/transferCollection.pl OK critic FAIL forbidden patterns forbidden pattern: You should not use a .tmpl extension for the template name (see bug 11349) (line 36) OK pod OK valid Created attachment 32361 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Created attachment 32362 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported (In reply to Katrin Fischer from comment #51) > Hi Kyle, there have been lots of follow-ups on this, would you be ok with > asking for another sign-off? This is a bigger one and I'd feel better with > some additional testing. That's fine by me! Reseting status to "Needs Signoff". I tried applying attachment 32361 [details] [review], and I got this error: Applying: Bug 8836 - Resurrect Rotating Collections error: patch failed: installer/data/mysql/updatedatabase.pl:8800 error: installer/data/mysql/updatedatabase.pl: patch does not apply Patch failed at 0001 Bug 8836 - Resurrect Rotating Collections Created attachment 32588 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Created attachment 32589 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Created attachment 32690 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Created attachment 32691 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Created attachment 32693 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Trusting the sign off on the other page, I only tested the QA changes I was just about to sign off on them both--Chris you beat me to the punch! Hi Cindy, if you want, we can add your sign off line in QA. Yes, sounds good! Do I need to do it? Sorry, my git-fu is weak! Still getting used to the signoff process. I can do it - thx for testing :) Starting work on this now - is there a good description of the Rotating collections module somewhere? Created attachment 32773 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> http://bugs.koha-community.org/show_bug.cgi?id=8835 Created attachment 32774 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Trusting the sign off on the other page, I only tested the QA changes Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> Just adding Cindy's sign-off line so we don't forget it :) Hi Kyle, starting with a code review of the new patches. 1. CODE REVIEW 1.1 QA script Problem (normal) We got a problem with perl critic: FAIL C4/Circulation.pm FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). 1.2 Code Note: Be careful with GetReserveStatus, it's not always working correctly. 'waiting' works, but other status are problematic, see bug 10697. + transferbook( $colBranchcode, $item->{barcode}, + my $ignore_reserves = 1 ) + unless ( GetReserveStatus( $item->{itemnumber} ) eq "Waiting" ); GetReserveStatus works for 'Waiting' but is problematic for any other status (see bug 10697) 1.3 Database structure Question: Why is the constraint in kohastructure.sql added by a separate statement? +-- Constraints for table `collections` +-- +ALTER TABLE `collections` + ADD CONSTRAINT `collections_ibfk_1` FOREIGN KEY (`colBranchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE 2. FEATURE TEST As this brings the Rotating collections feature back to life, I think we also need to test the feature here. I know that the code has been written a while ago, so I will add notes on what I think NEED to be fixed in order for this to go back in at the bottom, while also noting other things I see while testing this. 2.1 Documentation 2.1.1 Question: Is there a good documentation about the feature somewhere? Feeling a bit like flying blind here :) 2.1.2 Problem (normal, separate bug) We have no help pages for the Rotating collections 2.2 Navigation 2.2.1 Problem (normal) There is no entry for "Rotating collections" on the side navigation when you are on a tool's page. 2.2.2 Problem (normal) The main page of the Rotating collection page is missing the side navigation altogether. 2.2.3 Hm. In several places we have a button to return to the Rotating collections start page, maybe this could be replaced by the side navigation link? 2.3 Adding a rotating collection 2.3.1 Problem (minor) The 'new collection' form should have 'Title' marked as required. 2.3.2 Problem (normal) The error messages stem from RotatingCollections.pm and are not translatable (see bug 11595). Please fix! 2.3.3 Problem (trivial) The table shows the branch code instead of the description name, easy to fix today, with TT! 2.3.3 Hm. It feels a bit weird, that I have to use a button to get to a page where I can edit the collections. Why not have this functionality accessible from the table on the start page? (Adding "Edit" and "Delete" links to the table and a "New collection" button at the top? This way it would fit in a bit better with Koha's usual way of doing things. 2.4 Rotating collections home 2.4.1 Problem (trivial) Several capitalization 'problems' on rotatingCollections.pl. 2.4.1 Problem (trivial) The table shows the branch code instead of the description name, easy to fix today, with TT! 2.5 Adding/removing items 2.5.1 Problem (normal) The error messages stem from RotatingCollections.pm and are not translatable (see also bug 11595). Please fix! 2.5.2 Problem (trivial) Error messages should use the usual styling to stand out a bit more. Example: remove a barcode that is not in the collection. 2.5.3 Problem (trivial) I am not sure this error makes sense to the user: Reason: No Itemnumber Given I think it would be better to say: Barcode doesn't exist (or similar) 2.5.4 Enhancement idea Removing an item from the collection currently requires that you enter the barcode. It would be nice to also have a 'Delete' link in the table. 2.6 Transfers Collection goes to Troy. 2.6.1 OK Checking in an item that is "waiting" in Centerville The rotating collection message shows, but also the hold message. I confirm the hold message. The item is still 'waiting' for the patron. No transfer is generated. 2.6.2 OK Check-out the waiting item in Centerville to the borrower. 2.6.3 OK Check-in item in Centerville. There is another hold in Springfield for another patron. Confirm transfer. Transfer is generated. 2.6.4 OK Check-in of the item in Springfield. Confirm hold. Transfer is completed. 2.6.5 OK Check-out to the patron the item is waiting for. 2.6.6 OK? Check-in of the item in _Centerville_ Patron returns the item at another branch. I still see the dialog, that the item shoudl go to "Troy", but no transfer message is generated, is this the way it's supposed to work? Note: not sure, but should the rotating collections message show up, when there is also a hold/other transfer request? SUMMARY Please check everything noted as a 'Problem' specifically the perl critic and the translation issues. Some of the other things are easy to fix too :) Can you also summarize for me, what the suspected behaviour is (2.6.6)? To address 2.1.1, I can write some documentation for it. Our libraries have been using Rotating Collections for years. Hi Cindy, thanks for the offer! Maybe you can also help with 2.6? I am not sure there how it is intended to work. Created attachment 32973 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> http://bugs.koha-community.org/show_bug.cgi?id=8835 Created attachment 32974 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> http://bugs.koha-community.org/show_bug.cgi?id=8835 Created attachment 32975 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Trusting the sign off on the other page, I only tested the QA changes Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> Created attachment 32976 [details] [review] Bug 8836 [QA Followup 2] I believe this followup should fix most all your issues. There is a small visual bug with the alert divs that I'm hoping Owen can take a look at. As far as the your functionality test, I think Cindy can speak more to its accuracy, but it helps to think of the the rotating collections as something of a floating collection. For this reason, automatic transfers are disabled for rotating items. If you need to transfer an item, you must use the transfer tool from the circulation page. Does that make sense? Kyle Some issues I see: - The list of collections should show a count of the items in that collection - Should collections require unique titles? I was able to add multiple collections with the same title. - Why are descriptions required? - Transfer library list isn't in alphabetical order - No confirmation of deletion operation, including for collections which contain items. - Shouldn't the detail page for an item in a collection show that it is in a collection? - Add/edit/delete/transfer operations should redirect sensibly instead of showing a blank page with a confirmation message. For instance: After a successful collection edit the user should be redirected to the collections home page and a message displayed there. - Templates are a mess. I'm preparing a follow-up. Another issue: I happened to add to a collection an item which was already in mid-transfer between two libraries. When I transferred that collection to another library the in-progress transfer was still active. When I tried checking in that item at another library (not the collection's library, not the library to which the item had originally been transferred) I got conflicting messages: Both a message telling me where the collection belonged and also a message about the original transfer. If I understand the feature correctly the original in-progress transfer should have been cancelled when I initiated the transfer of the collection. Canceling an in-progress transport causes issues in real life. When a book is being moved between cities, it is essential to know that the item is on the move. When the Item in the rotating collection reaches the transfer destination, it is automatically sent back to the "new" home library (depending on the sysprefs). Initially the idea back here was to be able to create these Rotating Collections by utilizing the normal consortial transportation features. So we could put items from multiple branches to one rotating collection destined to one place. For example if we want to make a collection of a special topic from all the books available throughout the consortium. Ofcourse the items need to be checked-in to activate the transfer and this needs some manual work in notifying each branch about the desired items which are waiting in shelves. Items on normal circulation tend to move towards the rotating collection "home" after they are checked-in. I guess there are still some issues with the idea, but I don't think canceling transfers is the way to go. It would be better to force items in a rotating collection to be sent to the rotating collection "home" after they reach their destination. Regarding holds queue for items on a rotating collection. We thought that it will be something the librarian needs to consider, maybe as a follow up feature to block placing holds on rotating collections via syspref or via a option in the rotating collection itself (better!). I just don't feel like missing the opportunity to providing a tool to do cool things with our material, is worth the price of assuming that all the Items are immediately available when they are put to the collection. Even if that would be the easy way out. (In reply to Owen Leonard from comment #78) > Some issues I see: > > - The list of collections should show a count of the > items in that collection > true > - Should collections require unique titles? > I was able to add multiple collections with the same title. > a good idea > - Why are descriptions required? If we have a special rotating collection we want to tell about. Or maybe to tell others to not touch the material in here until 14.4.2015? Its handy to have a freeword place to leave messages and extra info. > > - Transfer library list isn't in alphabetical order > > - No confirmation of deletion operation, including > for collections which contain items. > we got complaints of that as well, I thought Juhani already fixed that? > - Shouldn't the detail page for an item in a collection show > that it is in a collection? > It should. > - Add/edit/delete/transfer operations should redirect sensibly > instead of showing a blank page with a confirmation message. > > For instance: After a successful collection edit the user > should be redirected to the collections home page and a > message displayed there. > > - Templates are a mess. I'm preparing a follow-up. I'll ask Juhani to comment on these. (In reply to Olli-Antti Kivilahti from comment #81) > > - Why are descriptions required? > If we have a special rotating collection we want to tell about. Yes, I can see why a description is a useful option. I just don't understand why the description would be a required field. Created attachment 32994 [details] [review] Bug 8836 [Template follow-up] Resurrect Rotating Collections This follow-up makes drastic changes to the templates in order to bring them into compliance with established patterns and markup guidelines. Only minor changes are made to perl scripts. Changes: - Add a toolbar include for displaying new, edit, transfer, and delete buttons. - Improve title and breadcrumbs with collection titles and better specificity. - Correct page structure which was inconsistent with the markup of similarly-structured pages. - Correct styling of error and informational messages. - Added detailed error messages for a couple of conditions which were not defined in the template. - Add link to the detail page of titles which are in a collection using the view defined in the IntranetBiblioDefaultView preference. - Add a link to remove an item from a collection directly without having to scan the barcode. - Add client-side validation to collection creation form. - In RotatingCollections.pm, add biblionumber to the list of columns returned by GetItemsInCollection. - In rotating_collections/*.pl, remove obsolete declaration of system preference variables. To test, perform all the operations associated with Rotating Collections: - Add a new collection - Edit an existing collection - Add items to a collection - Remove items from a collection (via barcode and link) - Test the behavior of all new toolbar buttons - Verify that titles and breadcrumbs look correct and links work correctly. Created attachment 32999 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> http://bugs.koha-community.org/show_bug.cgi?id=8835 Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33000 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Trusting the sign off on the other page, I only tested the QA changes Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33001 [details] [review] Bug 8836 [QA Followup 2] Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33002 [details] [review] Bug 8836 [Template follow-up] Resurrect Rotating Collections This follow-up makes drastic changes to the templates in order to bring them into compliance with established patterns and markup guidelines. Only minor changes are made to perl scripts. Changes: - Add a toolbar include for displaying new, edit, transfer, and delete buttons. - Improve title and breadcrumbs with collection titles and better specificity. - Correct page structure which was inconsistent with the markup of similarly-structured pages. - Correct styling of error and informational messages. - Added detailed error messages for a couple of conditions which were not defined in the template. - Add link to the detail page of titles which are in a collection using the view defined in the IntranetBiblioDefaultView preference. - Add a link to remove an item from a collection directly without having to scan the barcode. - Add client-side validation to collection creation form. - In RotatingCollections.pm, add biblionumber to the list of columns returned by GetItemsInCollection. - In rotating_collections/*.pl, remove obsolete declaration of system preference variables. To test, perform all the operations associated with Rotating Collections: - Add a new collection - Edit an existing collection - Add items to a collection - Remove items from a collection (via barcode and link) - Test the behavior of all new toolbar buttons - Verify that titles and breadcrumbs look correct and links work correctly. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Kivilahti, please keep in mind that Juhani's work is on a separate bug 12807 and not included here. I'd suggest to break it into well documented easily testable parts when rebasing for the changes made here. I have gathered some information from reading through the comments on this bug, could you confirm if the following assumptions are correct? 1) If I create a collection and transfer it, all available items will get automatically moved to the new library? 2) If I am using AutomaticItemReturn, it will not send a book back to its home library, but will display a note where it should go for Rotating collections? 3) All transfers for books missed in 1) have to be done manually and there will only be a note shown on checkin? (In reply to Katrin Fischer from comment #88) > Kivilahti, please keep in mind that Juhani's work is on a separate bug 12807 > and not included here. I'd suggest to break it into well documented easily > testable parts when rebasing for the changes made here. Cool! So this means I can sign off this patch. Hi Kyle, I went through my notes again and I think the templates, and adding/editing/deleting rotating collections is now ok. There is only one minor thing about the branch list not being alphabetical (2.3.5). Running out of time now, please answer my questions about transfer behaviour in comment #89 and check comment #79 from Owen, about the behaviour for an item that was 'in transit' before the rotating collections transfer. I will try to continue there tonight. 1. CODE REVIEW 1.1 QA script - OK 1.2 Code - OK, no more problems spotted. 1.3 Database structure - OK, changes are consistent. 2. FEATURE TEST As this brings the Rotating collections feature back to life, I think we also need to test the feature here. 2.1 Documentation 2.1.1 Question: Is there a good documentation about the feature somewhere? Feeling a bit like flying blind here :) 2.1.2 Problem - no blocker I have filed bug 13177 for the missing help pages. 2.2 Navigation 2.2.1 OK - FIXED There is no entry for "Rotating collections" on the side navigation when you are on a tool's page. 2.2.2 OK - FIXED The main page of the Rotating collection page is missing the side navigation altogether. 2.2.3 OK In several places we have a button to return to the Rotating collections start page, maybe this could be replaced by the side navigation link? 2.3 Adding a rotating collection 2.3.1 OK - FIXED The 'new collection' form should have 'Title' marked as required. 2.3.2 OK - FIXED The error messages stem from RotatingCollections.pm and are not translatable (see bug 11595). Please fix! 2.3.3 OK - FIXED The table shows the branch code instead of the description name, easy to fix today, with TT! 2.3.4 OK - FIXED with a new toolbar It feels a bit weird, that I have to use a button to get to a page where I can edit the collections. Why not have this functionality accessible from the table on the start page? (Adding "Edit" and "Delete" links to the table and a "New collection" button at the top? This way it would fit in a bit better with Koha's usual way of doing things. 2.3.5 Problem - trivial Branches in the list are not sorted alphabetically by description. 2.3.6 Question - could the description be optional? 2.4 Rotating collections home 2.4.1 OK - FIXED Several capitalization 'problems' on rotatingCollections.pl. 2.4.1 OK - FIXED The table shows the branch code instead of the description name, easy to fix today, with TT! 2.5 Adding/removing items 2.5.1 OK - FIXED The error messages stem from RotatingCollections.pm and are not translatable (see also bug 11595). Please fix! 2.5.2 OK - FIXED Error messages should use the usual styling to stand out a bit more. Example: remove a barcode that is not in the collection. 2.5.3 OK - FIXED I am not sure this error makes sense to the user: Reason: No Itemnumber Given I think it would be better to say: Barcode doesn't exist (or similar) 2.5.4 Enhancement idea - OK DONE! Removing an item from the collection currently requires that you enter the barcode. It would be nice to also have a 'Delete' link in the table. 2.6 Transfers Collection goes to Troy. 2.6.1 OK Checking in an item that is "waiting" in Centerville The rotating collection message shows, but also the hold message. I confirm the hold message. The item is still 'waiting' for the patron. No transfer is generated. 2.6.2 OK Check-out the waiting item in Centerville to the borrower. 2.6.3 OK Check-in item in Centerville. There is another hold in Springfield for another patron. Confirm transfer. Transfer is generated. 2.6.4 OK Check-in of the item in Springfield. Confirm hold. Transfer is completed. 2.6.5 OK Check-out to the patron the item is waiting for. 2.6.6 OK? Check-in of the item in _Centerville_ Patron returns the item at another branch. I still see the dialog, that the item shoudl go to "Troy", but no transfer message is generated, is this the way it's supposed to work? Note: I think it would be nicer if the rotating collection message wouldn't show up, when there is a hold message at the same time. 2.7 Transferring a collection 2.7.1 OK Verified that waiting and checked out items don't get transferred automatically. X. Possible enhancements - Show rotating collection information in staff detail pages - Add redirect operations after Add/edit/delete/transfer operations Since there's a lot of significant changes both to the views and also to the core behaviour of some of the features in 12807, I agree with the idea of breaking things down (and perhaps making it more modular?). (In reply to Katrin Fischer from comment #89) > I have gathered some information from reading through the comments on this > bug, could you confirm if the following assumptions are correct? > > 1) If I create a collection and transfer it, all available items will get > automatically moved to the new library? Only those items that are available to transfer at the time. > 2) If I am using AutomaticItemReturn, it will not send a book back to its > home library, but will display a note where it should go for Rotating > collections? That is correct. Any item in a rotating collection bypasses AutomaticItemReturn, as it would be contrary to the feature. > > 3) All transfers for books missed in 1) have to be done manually and there > will only be a note shown on checkin? That is correct. Created attachment 33045 [details] [review] Bug 8836 [QA Followup] - Allow collection description to be optional Created attachment 33046 [details] [review] Bug 8836 [QA Followup] - Add confirmation dialog for deleting collections Created attachment 33047 [details] [review] Bug 8836 [QA Followup] - Prevent multiple collections from having the same title Created attachment 33048 [details] [review] Bug 8836 [QA Followup] - Sort transfer branches alphabetically Created attachment 33050 [details] [review] [SIGNED-OFF] Bug 8836 [QA Followup] - Allow collection description to be optional Signed-off-by: Owen Leonard <oleonard@myacpl.org> Description is no longer made required by the template and an empty description is saved correctly. Created attachment 33051 [details] [review] [SIGNED-OFF] Bug 8836 [QA Followup] - Sort transfer branches alphabetically Signed-off-by: Owen Leonard <oleonard@myacpl.org> Libraries sort correctly now. Created attachment 33058 [details] [review] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> http://bugs.koha-community.org/show_bug.cgi?id=8835 Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33059 [details] [review] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Trusting the sign off on the other page, I only tested the QA changes Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33060 [details] [review] Bug 8836 [QA Followup 2] Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33061 [details] [review] Bug 8836 [Template follow-up] Resurrect Rotating Collections This follow-up makes drastic changes to the templates in order to bring them into compliance with established patterns and markup guidelines. Only minor changes are made to perl scripts. Changes: - Add a toolbar include for displaying new, edit, transfer, and delete buttons. - Improve title and breadcrumbs with collection titles and better specificity. - Correct page structure which was inconsistent with the markup of similarly-structured pages. - Correct styling of error and informational messages. - Added detailed error messages for a couple of conditions which were not defined in the template. - Add link to the detail page of titles which are in a collection using the view defined in the IntranetBiblioDefaultView preference. - Add a link to remove an item from a collection directly without having to scan the barcode. - Add client-side validation to collection creation form. - In RotatingCollections.pm, add biblionumber to the list of columns returned by GetItemsInCollection. - In rotating_collections/*.pl, remove obsolete declaration of system preference variables. To test, perform all the operations associated with Rotating Collections: - Add a new collection - Edit an existing collection - Add items to a collection - Remove items from a collection (via barcode and link) - Test the behavior of all new toolbar buttons - Verify that titles and breadcrumbs look correct and links work correctly. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Created attachment 33062 [details] [review] Bug 8836 [QA Followup] - Allow collection description to be optional Signed-off-by: Owen Leonard <oleonard@myacpl.org> Description is no longer made required by the template and an empty description is saved correctly. Created attachment 33063 [details] [review] Bug 8836 [QA Followup] - Sort transfer branches alphabetically Signed-off-by: Owen Leonard <oleonard@myacpl.org> Libraries sort correctly now. Created attachment 33064 [details] [review] Bug 8836 [QA Followup] - Add confirmation dialog for deleting collections Created attachment 33065 [details] [review] Bug 8836 [QA Followup] - Prevent multiple collections from having the same title Created attachment 33066 [details] [review] Bug 8836 [QA Followup] - Don't transfer in items already in transit Created attachment 33068 [details] [review] Bug 8836 [Alt QA Followup] - Add confirmation dialog for deleting collections The previous patch for adding a confirmation dialog didn't work to make the confirmation message translatable. This alternate patch uses a progressive-enhancement method recycled from Guided Reports to trigger a warning and highlight the row which was clicked. To test, apply the patch and try to delete an existing rotating collection. You should get a confirmation dialog, and the corresponding row in the table should be highlighted red. Clicking cancel should cancel the deletion. Clicking OK should complete the deletion. To test the translatability of the confirmation message: Run perl translate update [a language code, e.g. 'en-GB'] Search [lang]-i-staff-t-prog-v-3006000.po for the message string Created attachment 33070 [details] [review] [SIGNED-OFF] Bug 8836 [QA Followup] - Prevent multiple collections from having the same title Signed-off-by: Owen Leonard <oleonard@myacpl.org> This works, and so I'll sign off, but I'm not crazy about the workflow. Having the error message display on an otherwise empty page is not user friendly. The entry form should be redisplayed so that the user can modify the data they submitted. That really should be changed in a follow-up. Created attachment 33095 [details] [review] [PASSED QA] Bug 8836 - Resurrect Rotating Collections Test Plan: In "Tools" -> "Rotating Collections" -> "Add/Remove items": When adding item barcodes to the collection, the input field <input type="text" id="barcode" name="barcode"> should become active automatically so it is easy to add multiple barcodes rapidly without touching the mouse or keyboard. Enter-press is dealt by the barcode reader so automatic form submittal should be handled by the barcode reader. In "Rotating collections" -> "Transfer Collection": When the collection is initially transferred, items are set to trasfer correctly. When the collection is transferred while items are still being transferred, the transfer destination library doesn't change from the original one. The holding library changes for all items in the collection to the destination library on retransfers as well. This is tricky if a user accidentally places the wrong destination. When I try to checkin these items to their new retransfer location, I get the following messages: -"This item is part of a rotating collection and needs to be transferred to <original transfer destination>" -"Please return Valkoinen ihmissyj / to <original transfer destination>" -"Print slip or Cancel transfer" When I checkin a Item to a arbitrary branch, I get the following messages: "This item is part of a rotating collection and needs to be transferred to <retransfer destination>" "Please return Valkoinen ihmissyj / to <original trasfer destination>" Bug 8836 - Resurrect Rotating Collections - QA Followup Bug 8836 - Resurrect Rotating Collections - Followup 2 - Perltidy rotating collections scripts Bug 8836 - Resurrect Rotating Collections - Followup 3 * Fix bad TT Tag * Fix bad sql query * Fix capitalization ( HTML4 ) * Allow a rotating collection's location to keep AutomaticItemReturn from sending it back to the branch of origin * Fix bad query Bug 8836 - Resurrect Rotating Collections - Followup 4 - Autofocus on barcode field Bug 8836 - Resurrect Rotating Collections - Followup 5 - Don't transfer issued and waiting items Items in a rotating collection are automatcially transferred when a collection is transferred. This is a problem for currently checked out items and items on hold marked as "Waiting". This patch resolves this issue by skipping the transfer for those items. When the items are then returned, the librarian will be alerted to transfer the item to the library currently holding that rotating collection. Bug 8836 - Resurrect Rotating Collections - Followup 5 - Link collections.colBranchcode to branches.branchcode Signed-off-by: jmbroust <jean-manuel.broust@univ-lyon2.fr> Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> http://bugs.koha-community.org/show_bug.cgi?id=8835 Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33096 [details] [review] [PASSED QA] Bug 8836 [QA Followup] - Fix koha-qa.pl errors reported Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Trusting the sign off on the other page, I only tested the QA changes Signed-off-by: Cindy Murdock Ames <cmurdock@ccfls.org> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33097 [details] [review] [PASSED QA] Bug 8836 [QA Followup 2] Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33098 [details] [review] [PASSED QA] Bug 8836 [Template follow-up] Resurrect Rotating Collections This follow-up makes drastic changes to the templates in order to bring them into compliance with established patterns and markup guidelines. Only minor changes are made to perl scripts. Changes: - Add a toolbar include for displaying new, edit, transfer, and delete buttons. - Improve title and breadcrumbs with collection titles and better specificity. - Correct page structure which was inconsistent with the markup of similarly-structured pages. - Correct styling of error and informational messages. - Added detailed error messages for a couple of conditions which were not defined in the template. - Add link to the detail page of titles which are in a collection using the view defined in the IntranetBiblioDefaultView preference. - Add a link to remove an item from a collection directly without having to scan the barcode. - Add client-side validation to collection creation form. - In RotatingCollections.pm, add biblionumber to the list of columns returned by GetItemsInCollection. - In rotating_collections/*.pl, remove obsolete declaration of system preference variables. To test, perform all the operations associated with Rotating Collections: - Add a new collection - Edit an existing collection - Add items to a collection - Remove items from a collection (via barcode and link) - Test the behavior of all new toolbar buttons - Verify that titles and breadcrumbs look correct and links work correctly. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33099 [details] [review] [PASSED QA] Bug 8836 [QA Followup] - Allow collection description to be optional Signed-off-by: Owen Leonard <oleonard@myacpl.org> Description is no longer made required by the template and an empty description is saved correctly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33100 [details] [review] [PASSED QA] Bug 8836 [QA Followup] - Sort transfer branches alphabetically Signed-off-by: Owen Leonard <oleonard@myacpl.org> Libraries sort correctly now. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33101 [details] [review] [PASSED QA] Bug 8836 [QA Followup] - Don't transfer in items already in transit Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33102 [details] [review] [PASSED QA] Bug 8836 [Alt QA Followup] - Add confirmation dialog for deleting collections The previous patch for adding a confirmation dialog didn't work to make the confirmation message translatable. This alternate patch uses a progressive-enhancement method recycled from Guided Reports to trigger a warning and highlight the row which was clicked. To test, apply the patch and try to delete an existing rotating collection. You should get a confirmation dialog, and the corresponding row in the table should be highlighted red. Clicking cancel should cancel the deletion. Clicking OK should complete the deletion. To test the translatability of the confirmation message: Run perl translate update [a language code, e.g. 'en-GB'] Search [lang]-i-staff-t-prog-v-3006000.po for the message string Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 33103 [details] [review] [PASSED QA] Bug 8836 [QA Followup] - Prevent multiple collections from having the same title Signed-off-by: Owen Leonard <oleonard@myacpl.org> This works, and so I'll sign off, but I'm not crazy about the workflow. Having the error message display on an otherwise empty page is not user friendly. The entry form should be redisplayed so that the user can modify the data they submitted. That really should be changed in a follow-up. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Hi Kyle and Owen, thx for all the hard work on this! 1. CODE REVIEW 1.1 QA script - OK 1.2 Code - OK, no more problems spotted. 1.3 Database structure - OK, changes are consistent. 2. FEATURE TEST As this brings the Rotating collections feature back to life, I think we also need to test the feature here. 2.1 Documentation 2.1.1 Question: Is there a good documentation about the feature somewhere? Feeling a bit like flying blind here :) 2.1.2 Problem - no blocker I have filed bug 13177 for the missing help pages. 2.2 Navigation 2.2.1 OK - FIXED There is no entry for "Rotating collections" on the side navigation when you are on a tool's page. 2.2.2 OK - FIXED The main page of the Rotating collection page is missing the side navigation altogether. 2.2.3 OK In several places we have a button to return to the Rotating collections start page, maybe this could be replaced by the side navigation link? 2.3 Adding a rotating collection 2.3.1 OK - FIXED The 'new collection' form should have 'Title' marked as required. 2.3.2 OK - FIXED The error messages stem from RotatingCollections.pm and are not translatable (see bug 11595). Please fix! 2.3.3 OK - FIXED The table shows the branch code instead of the description name, easy to fix today, with TT! 2.3.4 OK - FIXED with a new toolbar It feels a bit weird, that I have to use a button to get to a page where I can edit the collections. Why not have this functionality accessible from the table on the start page? (Adding "Edit" and "Delete" links to the table and a "New collection" button at the top? This way it would fit in a bit better with Koha's usual way of doing things. 2.3.5 OK - FIXED Branches in the list are not sorted alphabetically by description. 2.3.6 OK - FIXED Description should be optional. 2.4 Rotating collections home 2.4.1 OK - FIXED Several capitalization 'problems' on rotatingCollections.pl. 2.4.1 OK - FIXED The table shows the branch code instead of the description name, easy to fix today, with TT! 2.5 Adding/removing items 2.5.1 OK - FIXED The error messages stem from RotatingCollections.pm and are not translatable (see also bug 11595). Please fix! 2.5.2 OK - FIXED Error messages should use the usual styling to stand out a bit more. Example: remove a barcode that is not in the collection. 2.5.3 OK - FIXED I am not sure this error makes sense to the user: Reason: No Itemnumber Given I think it would be better to say: Barcode doesn't exist (or similar) 2.5.4 Enhancement idea - OK DONE! Removing an item from the collection currently requires that you enter the barcode. It would be nice to also have a 'Delete' link in the table. 2.6 Transfers Collection goes to Troy. 2.6.1 OK Checking in an item that is "waiting" in Centerville The rotating collection message shows, but also the hold message. I confirm the hold message. The item is still 'waiting' for the patron. No transfer is generated. 2.6.2 OK Check-out the waiting item in Centerville to the borrower. 2.6.3 OK Check-in item in Centerville. There is another hold in Springfield for another patron. Confirm transfer. Transfer is generated. 2.6.4 OK Check-in of the item in Springfield. Confirm hold. Transfer is completed. 2.6.5 OK Check-out to the patron the item is waiting for. 2.6.6 OK? Check-in of the item in _Centerville_ Patron returns the item at another branch. I still see the dialog, that the item shoudl go to "Troy", but no transfer message is generated, is this the way it's supposed to work? 2.7 Transferring a collection 2.7.1 OK Verified that waiting and checked out items don't get transferred automatically. 2.7.2 OK Verified that 'in transit' item doesn't get rerouted by transferring the collection. 2.7.3 AutomaticItemReturn ... does not happen when your item is in a rotating collection. X. Possible enhancements - Show rotating collection information in staff detail pages Created attachment 33330 [details] [review] Bug 8836: (RM followup) fix updatedatabase.pl rebase problem Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Created attachment 33331 [details] [review] Bug 8836: Update DBIx Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Created attachment 33374 [details] [review] Bug 8836: (RM followup) unit tests should use the new API Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Patches pushed to master. Thanks Kyle! |