Description
Andrew Fuerste-Henry
2021-06-08 17:23:55 UTC
+1 We also would like to see this functionality added in. As we move to a more floating based system, we are going to rely on CollectionsHQ to tell us how to shift. It would be a great option to see this built into the ILS. We would also like this as a feature, it would be very helpful to have this as we are also using CollectionHQ to tell us how to shift our collections. -Charlie Created attachment 160038 [details] [review] Bug 28530: Update database, add new schema file Created attachment 160039 [details] [review] Bug 28530: Update database, add new schema file Created attachment 160040 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Created attachment 160041 [details] [review] Bug 28530: Update existing schema files Created attachment 160042 [details] [review] Bug 28530: Add float limits editor Created attachment 160043 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Created attachment 160044 [details] [review] Bug 28530: Add unit tests Created attachment 160045 [details] [review] Bug 28530: Update database, add new schema file Created attachment 160046 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Created attachment 160047 [details] [review] Bug 28530: Update existing schema files Created attachment 160048 [details] [review] Bug 28530: Add float limits editor Created attachment 160049 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Created attachment 160050 [details] [review] Bug 28530: Add unit tests I created a sandbox with these patches, but it doesn't seem to have the new table. I get the following error when I go to /cgi-bin/koha/admin/float_limits.pl: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Table 'koha_floatie.library_float_limits' doesn't exist at /kohadevbox/koha/Koha/Objects.pm line 317 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 Created attachment 160053 [details] [review] Bug 28530: Update database, add new schema file Created attachment 160054 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Created attachment 160055 [details] [review] Bug 28530: Update existing schema files Created attachment 160056 [details] [review] Bug 28530: Add float limits editor Created attachment 160057 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Created attachment 160058 [details] [review] Bug 28530: Add unit tests Note for testing: I confirmed that AutomaticItemReturn does control whether or not one can decline the transfers initiated by this feature. Issues: - Library float limits needs to be included in the admin nav sidebar - Allow value of zero -- currently, if you enter a zero it gets treated as a blank and no limit is saved. As described on the screen, a blank should mean float is not limited while a limit of zero would mean items of that type should always transfer away - rename "limit" field as it causes errors in writing reports unless you refer to the field with its full name (library_floating_limits.limit). Maybe "item_limit"? Transfer triggered for checkin that *meets* limit rather than exceeding it: - have a float limit set to 1 for Branch A - have 1 item of that item type at Branch A, giving ratio of 1 - check that item in at Branch A, get notified that this would put the library over its limit and the item will be transferred Ratio not correctly calculated for branches with zero items of type: - Have defined float limits for Branches: A = 1, B = 10, C = 100 - Have 1 item of limited type at Branch A, 1 at Branch B, zero at Branch C - Check 1 item in at Branch A - Koha directs the item to transfer to Branch B (ratio of .1) rather than Branch C (ratio of zero) Seems like we're only calculating a ratio where there is at least 1 item already. > Transfer triggered for checkin that *meets* limit rather than exceeding it:
> - have a float limit set to 1 for Branch A
> - have 1 item of that item type at Branch A, giving ratio of 1
> - check that item in at Branch A, get notified that this would put the
> library over its limit and the item will be transferred
Clarifying a little -- currently the patch *does* generate a transfer when a checked in item *meets* the floating limit. And I suppose that's what the text on screen says: "if the checkin library has reached its' limit for that type of item." That said, I feel like users may expect transfers to be generated only when the limits are *exceeded*.
Created attachment 160061 [details] [review] Bug 28530: (QA follow-up): Fix qa issues Created attachment 160062 [details] [review] Bug 28530: Update database, add new schema file Created attachment 160063 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Created attachment 160064 [details] [review] Bug 28530: Update existing schema files Created attachment 160065 [details] [review] Bug 28530: Add float limits editor Created attachment 160066 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Created attachment 160067 [details] [review] Bug 28530: Add unit tests Created attachment 160068 [details] [review] Bug 28530: (QA follow-up) Fix QA issues Created attachment 160071 [details] [review] Bug 28530: Update database, add new schema file Created attachment 160072 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Created attachment 160073 [details] [review] Bug 28530: Update existing schema files Created attachment 160074 [details] [review] Bug 28530: Add float limits editor Created attachment 160075 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Created attachment 160076 [details] [review] Bug 28530: Add unit tests (In reply to Andrew Fuerste-Henry from comment #24) All issues should be fixed. All fixes merged into original patchset! (In reply to Andrew Fuerste-Henry from comment #24) > > Transfer triggered for checkin that *meets* limit rather than exceeding it: > > - have a float limit set to 1 for Branch A > > - have 1 item of that item type at Branch A, giving ratio of 1 > > - check that item in at Branch A, get notified that this would put the > > library over its limit and the item will be transferred > > Clarifying a little -- currently the patch *does* generate a transfer when a > checked in item *meets* the floating limit. And I suppose that's what the > text on screen says: "if the checkin library has reached its' limit for that > type of item." That said, I feel like users may expect transfers to be > generated only when the limits are *exceeded*. I'll change to exceeding the limit. I think that's more natural and expected. Created attachment 160077 [details] [review] Bug 28530: (QA follow-up) Require the item checkin to exceed the limit, rather than meet the limit Created attachment 160078 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 160079 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 160080 [details] [review] Bug 28530: Update existing schema files Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 160081 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 160082 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 160083 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 160084 [details] [review] Bug 28530: (QA follow-up) Require the item checkin to exceed the limit, rather than meet the limit Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Programming error - op 'set_float_limits' must start with 'cud-' for POST http://localhost:8081/intranet/admin/float_limits.pl Created attachment 163919 [details] [review] Bug 28530: (QA follow-up): Implement CSRF cud op Created attachment 168343 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 168344 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 168345 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 168346 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 168347 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 168348 [details] [review] Bug 28530: (QA follow-up) Require the item checkin to exceed the limit, rather than meet the limit Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 168349 [details] [review] Bug 28530: (QA follow-up): Implement CSRF cud op Created attachment 168350 [details] [review] Bug 28530: Update existing schema files Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Sorry Kyle, this no longer applies Created attachment 178762 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 178763 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 178764 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 178765 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 178766 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 178767 [details] [review] Bug 28530: (QA follow-up) Require the item checkin to exceed the limit, rather than meet the limit Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Created attachment 178768 [details] [review] Bug 28530: (QA follow-up): Implement CSRF cud op Created attachment 178769 [details] [review] Bug 28530: Update existing schema files Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Rebased for current main, still some noticeable problems with both CSRF and Bootstrap 5 things. Setting to FQA for now. Created attachment 178783 [details] [review] Bug 28530: Template adjustmentts for CSRF and Bootstrap 5 Created attachment 178784 [details] [review] Bug 28530: Tidy admin-menu.inc Created attachment 178785 [details] [review] Bug 28530: Fix remaining QA script failures I think this is again ready for testing, resetting to NSO. This doesn't seem to be enforcing limits correctly. I think it's checking the float limit against the total number of items of that item type in the whole system (rather than just at the current branch) and then prompting a transfer to the branch with the lowest items/limit ratio (unless that's your current branch, in which case no transfer is generated). To recreate: - set circ rules to everything floats - create 4 items of itemtype CF / Computer Files (this itype exists in the default data without any items, so it's easy to test with) - set float limit of 3 for CF items at Centerville and Fairfield branches, leave other branches without limits - play around checking your 4 items in at Centerville and Fairfield Koha should only generate a transfer when all 4 items end up at either Centerville or Fairfield. Instead, it seems to be generating a transfer whenever the branch at which you're checking in does not have the lowest ratio of items/limit. Created attachment 178934 [details] [review] Bug 28530: (QA follow-up) correct item count and limit enforcement Corrected the count of items to limit to the current branch. Also set the limit to trigger when the count of items at the branch is currently at the limit before the item to be checked in. Created attachment 178935 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178936 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178937 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178938 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178939 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178940 [details] [review] Bug 28530: (QA follow-up) Require the item checkin to exceed the limit, rather than meet the limit Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178941 [details] [review] Bug 28530: (QA follow-up): Implement CSRF cud op Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178942 [details] [review] Bug 28530: Update existing schema files Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178943 [details] [review] Bug 28530: Template adjustmentts for CSRF and Bootstrap 5 Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178944 [details] [review] Bug 28530: Tidy admin-menu.inc Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178945 [details] [review] Bug 28530: Fix remaining QA script failures Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 178946 [details] [review] Bug 28530: (QA follow-up) correct item count and limit enforcement Corrected the count of items to limit to the current branch. Also set the limit to trigger when the count of items at the branch is currently at the limit before the item to be checked in. Signed-off-by: Kris Becker <kbecker@jcls.org> Unit tests are failing here I'm afraid. Created attachment 179943 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179944 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179945 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179946 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179947 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179948 [details] [review] Bug 28530: (QA follow-up) Require the item checkin to exceed the limit, rather than meet the limit Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179949 [details] [review] Bug 28530: (QA follow-up): Implement CSRF cud op Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179950 [details] [review] Bug 28530: Update existing schema files Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179951 [details] [review] Bug 28530: Template adjustmentts for CSRF and Bootstrap 5 Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179952 [details] [review] Bug 28530: Tidy admin-menu.inc Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179953 [details] [review] Bug 28530: Fix remaining QA script failures Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179954 [details] [review] Bug 28530: (QA follow-up) correct item count and limit enforcement Corrected the count of items to limit to the current branch. Also set the limit to trigger when the count of items at the branch is currently at the limit before the item to be checked in. Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 179955 [details] [review] Bug 28530: Adjust number of tests (In reply to Martin Renvoize (ashimema) from comment #87) > Unit tests are failing here I'm afraid. It seems like it was only the number of test, I have adjusted that. Nothing else was failing for me. Created attachment 180028 [details] [review] Bug 28530: Adjust number of tests Signed-off-by: Kris Becker <kbecker@jcls.org> I'm afraid I'm seeing unit test failures still here.. I've reset_all etc and the tests still fail I'm afraid. Comment on attachment 179944 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Review of attachment 179944 [details] [review]: ----------------------------------------------------------------- ::: Koha/Library/FloatLimits.pm @@ +43,5 @@ > +sub lowest_ratio_library { > + my ( $self, $item, $branchcode ) = @_; > + > + my $field = C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype'; > + my $query = qq{ Is it deliverate to continue using plain SQL here rather than constructing a DBIC query.. and should we perhaps be returning a Koha::Library object rather than a branchcode given this is a Koha:: object method? In fact.. I'd kinda be tempted to shift that filter method into Koha::Libraries... you're basically looking for the library with the lowest_ratio.. that feels like a `Koha::Libraries->find_library_with_lowest_float_ratio` method to me...
> Is it deliverate to continue using plain SQL here rather than constructing a
> DBIC query.. and should we perhaps be returning a Koha::Library object
> rather than a branchcode given this is a Koha:: object method?
I'm not the original author but I tend to agree, let's use a DBIC query and return a library object.
Created attachment 182931 [details] [review] Bug 28530: Use DBIC query and return library object In testing this, I've run into some shortcomings in its logic. I suspect these are things that were not considered to begin with, rather than things introduced in the recent change to use DBIC and a library object. As these float limits are enforced, we count items based purely on their holdingbranch, regardless of other values. Most pointedly, we don't consider whether or not those items are either checked out or in transit to another library. The goal here is to govern floating based on how many items are actually on the shelf, so this isn't ideal. My pretend library has a branch with a floating limit of 10 for the itemtype DVD, because they're small. At this small branch: - there are 10 DVDs available -- they're at their float limit - a patron checks out 5 DVDs, leaving 5 available Per the current logic, this library is *still* at their float limit because those 5 checked-out DVDs haven't changed their holding branch, even though we have no expectation that those DVDs will come back to the same branch they were checked out from. ** When an item is checked out, it should not count toward the float limit at *any* branch. A new scenario at my pretend library, again at the small branch with the float limit of 10 DVDs: - there are 10 DVDs available -- they're at their float limit - take one of those 10 available DVDs and check it in for a local use - Koha tells you to transfer that item away to keep under your float limit. Accept that transfer - Check in one of your 9 remaining available DVDs for another local use. Koha tells you to transfer this one away, too. Because the holding branch on the previous one hasn't changed yet and we're not looking at whether or not it's in transit. ** When an item is in transit, it should count toward the float limit at the branch it is transferring to, even when it hasn't been received yet. (In reply to Andrew Fuerste-Henry from comment #108) > A new scenario at my pretend library, again at the small branch with the > float limit of 10 DVDs: > - there are 10 DVDs available -- they're at their float limit > - take one of those 10 available DVDs and check it in for a local use > - Koha tells you to transfer that item away to keep under your float limit. This is another issue. Checkin an item in for a local use prompts that item to transfer away if the library is *at* its float limit. Seems like it's not considering that the item being checked in is already here and therefore will not push the library to *exceed* its limit, which is when a transfer should happen. Created attachment 184754 [details] [review] Bug 28530: Do not count onloan items towards the float limit Created attachment 184755 [details] [review] Bug 28530: Count incoming transfers against the float limit Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio 8) Confirm checked-out items are not counted for either float limits or float ratios 9) Confirm in-transit items are not counted for either float limits or float ratios I found it helpful to have a query to count items per branch, taking into account how many are checked out or in transit. I've been resting using the Ref item type as there aren't any in the base KTD data so it's easy to set up tests. SELECT item_counts.*, transfer_incoming FROM ( SELECT i.holdingbranch, count(i.itemnumber) as count_by_holdingbranch_only, sum(if(t.branchtransfer_id is null and i.onloan is null,1,0)) as available, sum(if(i.onloan is not null,1,0)) as checked_out, sum(if(t.branchtransfer_id is not null,1,0)) as transfered_away FROM items i LEFT JOIN branchtransfers t ON (i.itemnumber=t.itemnumber and t.datesent is not null and t.datecancelled is null and t.datearrived is null) WHERE itype='ref' GROUP BY holdingbranch ) item_counts LEFT JOIN ( SELECT b.tobranch, count(b.branchtransfer_id) as transfer_incoming FROM branchtransfers b WHERE b.datesent is not null and b.datearrived is null and b.datecancelled is null GROUP BY tobranch ) incoming ON (item_counts.holdingbranch = incoming.tobranch) Created attachment 184917 [details] [review] Bug 28530: Subtract outgoing transfers from the branches float limit Created attachment 184936 [details] [review] Bug 28530: Fix sorting and pick a random branch in the event of a tie New test plan: Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section, for example: 7) Create some float limits for a single itemtype, we are testing with REF itemtypes in k-t-d as there are none by default, so add these float limits for the REF itemtype: -CPL: 2 -FFL: 2 -FPL: 2 -FRL: 2 -ITP: 2 The rest should be set 0 or remain empty. 8) Create or have the following items: -CPL: 2 REF items -FFL: 2 REF items -FPL: 2 REF items -FRL: 2 REF items -ITP: 2 REF items 9) In Circulation rules set ‘Default checkout, hold and return policy’ to ‘item floats’. 10) Ensure you are logged in as CPL. 11) Check in a CPL REF item at CPL. It checks in normally. 12) Check in a FFL item at CPL, get a pop-up modal telling you that you have exceeded float limits, this item should return to FFL. 13). Login as FFL, they should have 1 REF item in-transit and 1 REF item on the shelves. 14) Check in a ITP item, since you have exceeded float limits you should get a pop-up telling to return this item to CPL. CPL has 1 on shelf REF item, 1 in transit away. CPL is the best option for this item. 15) Check out both items from FPL at FPL. Still logged in as FFL check in one of your own items. You should be told to return it to FPL, they have the lowest float limit now. 16) Reset all of your items transfers and holding branch data: -delete from branchtransfers; -update items set holdingbranch = homebranch; 17) Set the float limits to each branch to: -CPL: 1 -FFL: 1 -FPL: 1 -FRL: 1 -ITP: 1 18) Every library is now at its float limit. Checkin one of the checked out items for step 15. It should be send back to FPL, since it has the lowest ratio now. 19) Check all items in and reset transfers and holding branch data: -delete from branchtransfers; -update items set holdingbranch = homebranch; 20) Now as FFL, checkin an FFL item. Everyone is over their limit and the ratio is a tie. The item should go to a random branch among the tied candidates. Im setting this to FQA because this needs to follow the 'Floats by library group' return policy. If that policy is active we should only do float limit transfers to branches within the group. Created attachment 184951 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184952 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184953 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184954 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184955 [details] [review] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184956 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184957 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184958 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184959 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 184960 [details] [review] Bug 28530: Implement float limits Created attachment 184961 [details] [review] Bug 28530: DBIC changes DO NOT PUSH Created attachment 184963 [details] [review] Bug 28530: If return policy is set to float in library group, remove non group branches Created attachment 184965 [details] [review] Bug 28530: If return policy is set to float in library group, remove non group branches Created attachment 184966 [details] [review] Bug 28530: If return policy is set to float in library group, remove non group branches Created attachment 184967 [details] [review] Bug 28530: If return policy is set to float in library group, remove non group branches Created attachment 185038 [details] [review] Bug 28530: Add more tests Still running into an issue here when all branches are *at* their float limit and an item moves from Branch A to Branch B. So, at step 12 of the test plan in comment 116, we've got 2 REF items at each branch and take one of the FFL items and check it in ay CPL. With these patches, Koha sees that moving this item would put CPL over its limit of 2, but declines to transfer the item because at that point in the transaction the item's holdingbranch hasn't yet changed and so Koha views all branches as being tied on their limit/item ratio. We should update the item's holdingbranch to the branch at which the checkin is happening *before* we check the float limits to see if it's acceptable for that item to stay at its new branch. Created attachment 185836 [details] [review] Bug 28530: Update database, add new schema file Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 185837 [details] [review] Bug 28530: Add Koha Object(s) for Float Limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 185838 [details] [review] Bug 28530: Add float limits editor Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 185839 [details] [review] Bug 28530: Add unit tests Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kris Becker <kbecker@jcls.org> Created attachment 185840 [details] [review] Bug 28530: Implement float limits Created attachment 185841 [details] [review] Bug 28530: DBIC changes DO NOT PUSH Created attachment 185842 [details] [review] Bug 28530: If return policy is set to float in library group, remove non group branches Created attachment 185843 [details] [review] Bug 28530: Add more tests |