|
Description
Nick Clemens (kidclamp)
2023-06-30 19:25:42 UTC
Created attachment 152913 [details] [review] Bug 34178: Cache ItemsAnyAvailableAndNotRestricted in memory and don't precalculate There are several places in the code where we precalculate ItemsAnyAvailableAndNotRestricted to avoid looping on this routine when calling IsAvailableForItemLevelRequest on a list of items form a biblio The value of ItemsAnyAvailableAndNotRestricted is only used when there is a circulation rule for 'onshelfholds' with a value of '2' (If all unavailable) Rather than calculate a value that may never be used, let's cache this value per request when we do calculate it - and reuse the cached value To test: 1 - Apply patch 2 - Set circulation rule 'On shelf holds allowed' as 'If all unavailable' make sure the rule applies to all of the items/patrons you test with 3 - Find a record with two items that are available 4 - Try to place a hold for a patron - not allowed 5 - Check out one item to another patron 6 - Attempt hold - still not allowed 7 - Check out second item to another patron 8 - Attempt hold - allowed! 9 - Apply patch 10 - Cancel and replace hold - it is allowed! 11 - Check in one item, and cancel hold 12 - Place hold - not allowed! 13 - Check in second item 14 - Place hold - not allowed! 15 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Created attachment 152914 [details] [review] Bug 34178: Adjust tests We need to clear the cache when changing rules/statuses To test: 1 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Created attachment 152981 [details] [review] Bug 34178: Cache ItemsAnyAvailableAndNotRestricted in memory and don't precalculate There are several places in the code where we precalculate ItemsAnyAvailableAndNotRestricted to avoid looping on this routine when calling IsAvailableForItemLevelRequest on a list of items form a biblio The value of ItemsAnyAvailableAndNotRestricted is only used when there is a circulation rule for 'onshelfholds' with a value of '2' (If all unavailable) Rather than calculate a value that may never be used, let's cache this value per request when we do calculate it - and reuse the cached value To test: 1 - Apply patch 2 - Set circulation rule 'On shelf holds allowed' as 'If all unavailable' make sure the rule applies to all of the items/patrons you test with 3 - Find a record with two items that are available 4 - Try to place a hold for a patron - not allowed 5 - Check out one item to another patron 6 - Attempt hold - still not allowed 7 - Check out second item to another patron 8 - Attempt hold - allowed! 9 - Apply patch 10 - Cancel and replace hold - it is allowed! 11 - Check in one item, and cancel hold 12 - Place hold - not allowed! 13 - Check in second item 14 - Place hold - not allowed! 15 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Signed-off-by: Sam Lau <samalau@gmail.com> Created attachment 152982 [details] [review] Bug 34178: Adjust tests We need to clear the cache when changing rules/statuses To test: 1 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Signed-off-by: Sam Lau <samalau@gmail.com> Upping to major, we see this cause delays of up to 1-2 minutes when loading a patron's checkouts Created attachment 153628 [details] [review] Bug 34178: Cache ItemsAnyAvailableAndNotRestricted in memory and don't precalculate There are several places in the code where we precalculate ItemsAnyAvailableAndNotRestricted to avoid looping on this routine when calling IsAvailableForItemLevelRequest on a list of items form a biblio The value of ItemsAnyAvailableAndNotRestricted is only used when there is a circulation rule for 'onshelfholds' with a value of '2' (If all unavailable) Rather than calculate a value that may never be used, let's cache this value per request when we do calculate it - and reuse the cached value To test: 1 - Apply patch 2 - Set circulation rule 'On shelf holds allowed' as 'If all unavailable' make sure the rule applies to all of the items/patrons you test with 3 - Find a record with two items that are available 4 - Try to place a hold for a patron - not allowed 5 - Check out one item to another patron 6 - Attempt hold - still not allowed 7 - Check out second item to another patron 8 - Attempt hold - allowed! 9 - Apply patch 10 - Cancel and replace hold - it is allowed! 11 - Check in one item, and cancel hold 12 - Place hold - not allowed! 13 - Check in second item 14 - Place hold - not allowed! 15 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 153629 [details] [review] Bug 34178: Adjust tests We need to clear the cache when changing rules/statuses To test: 1 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 153630 [details] [review] Bug 34178: (QA follow-up) Tidy Tidy the relevant lines to pass the new QA rules Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> All working for me, no regressions found. QA scripts happy, Passing QA Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.02 Does not apply for 22.11.x because bug 30860 and bug 31735 have not been pushed, maybe others. Not pushing. *** Bug 35225 has been marked as a duplicate of this bug. *** |