Many libraries in my system lend DVDs out for a shorter period than their normal checkout period, like 2 days versus 14 or 21 days for other items. They also charge larger fines on DVDs than other items. They would like to be able to generate overdue notices for DVDs on the first day overdue, unlike the first notice generation of 7 days on their other items. So this is an enhancement request to allow libraries to generate an overdue notice for a particular item type separate from the three notice triggers.
This is standard behavior now, right?
Right now we can only set the overdue notices by patron category, but we can't differentiate by item type. I think this is still valid.
Here in 2017 still wishing for greater flexibility for overdue notices based on Item Type, not only Patron Category -- in our case (school library) for better communication about items on Reserve (in our case loan period is 2 hours).
+1
This is important in our consortium of mainly public libraries as our Library of Things (LoT) items go out for shorter periods of time and are overdue on a different time schedule to regular items like books. We would like overdues to be based on item type not just patron type. We also have one college member library in our consortium and their items go out on for different time periods again based on Item type not just patron type.
Matt and I are working on this together.. we're not too far off submitting code as per the spec above.. getting the API patches linked above through would certainly help it keep marching forward.
Created attachment 170357 [details] [review] Bug 10190: DB migration for overduerules This patch adds the data migration for overduerules to circulation rules. We attempt to create the default rules based on most commonly occuring rule values and then add overrides for all other rule values we find. Sponsored-by: Glasgow Colleges Library Group
Created attachment 170358 [details] [review] Bug 10190: Fix unit test to exclude _X_ rules Sponsored-by: Glasgow Colleges Library Group
Created attachment 170359 [details] [review] Bug 10190: Add support for incrementable circulation rules This patch adds support for _X_ grouped circulation rules to both the internal and external API's for handling circulation rules. This allows us to group overdue_X_rulename rules by an 'X' integer so we can have triggers incremented and grouped together. We also add the new overdue_x_* rule definition here for overdue trigger rules. Sponsored-by: Glasgow Colleges Library Group
Created attachment 170360 [details] [review] Bug 10190: Add a vue app for circulation triggers This patch adds a new vue based page for administering circulation triggers. This aims to replace the 'Overdue notice/status triggers' page available from Tools. The new page is available from Koha administration > Circulation triggers Sponsored-by: Glasgow Colleges Library Group
Created attachment 170361 [details] [review] Bug 10190: Add components to list/add and edit circulation triggers These new components are used by the new 'Circulation triggers' administration page to display existing trigger rules and add the ability to add new rules and edit existing ones. Sponsored-by: Glasgow Colleges Library Group
Created attachment 170362 [details] [review] Bug 10190: Add a library API client This patch adds a library api client to the vue ecosystem for later use in the Circulation triggers page. Sponsored-by: Glasgow Colleges Library Group
Created attachment 170363 [details] [review] Bug 10190: Add itemtypes and patron categories to the item/patron api clients This patch adds patron and itemtype api clients for the Vue ecosystem Sponsored-by: Glasgow Colleges Library Group
Created attachment 170364 [details] [review] Bug 10190: Let the circulation_rules endpoint accept query parameters Sponsored-by: Glasgow Colleges Library Group
Created attachment 170365 [details] [review] Bug 10190: Update GetBranchcodesWithOverdueRules to use CirculationRules This patch updates the C4::Overdues method GetBranchecodesWithOverdueRules to refer to the new location of these rules in the circulation rules table. We also update the unit tests to move the mocked data into the right location. We could perhaps migrate this method in Koha::CirculationRules at a later date? Sponsored-by: Glasgow Colleges Library Group
Created attachment 170366 [details] [review] Bug 10190: Update overdue_notices script to refer to circ rules This patch updates the overdue_notices cron script to refer to the new location for overdue rules found in the circulation rules table now. Sponsored-by: Glasgow Colleges Library Group
Created attachment 170367 [details] [review] Bug 10190: Add itemtype handling to overdue_notices script This patch enhances the overdue_notices script to understand the newly introduced itemtype level for overdue rules. Sponsored-by: Glasgow Colleges Library Group
Created attachment 170368 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group
Created attachment 170369 [details] [review] Bug 10190: Add letter names to tables Sponsored-by: Glasgow Colleges Library Group
Created attachment 170370 [details] [review] Bug 10190: Set tab based on trigger edited/created Sponsored-by: Glasgow Colleges Library Group
Created attachment 170371 [details] [review] Bug 10190: Add fallback to 'Default' text for notice Sponsored-by: Glasgow Colleges Library Group
Created attachment 170372 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group
Created attachment 170373 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group
This is going to be fun to test.. Matt and I worked hard together on it, so whilst it's assigned to me we're both available to comment. I'm happy to walk through intentions in a Jistsi or whatever.. reach out on chat.koha-community.org if you want any guidance.
Created attachment 170374 [details] [review] Bug 10190: Drop overduerules and overduerules_transport_types
Created attachment 170375 [details] [review] Bug 10190: DBIC Schema update
Created attachment 170376 [details] [review] Bug 10190: Remove OverdueRule Koha Objects
Created attachment 170380 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules
Whilst I've updated the itiva script to reflect the move to circulation rules.. I've not got any way to really test this. We also need to expose itiva in the new admin page.. I'm going to ask Matt to take a look at that.
Created attachment 170449 [details] [review] Bug 10190: Add comments to overdue_notices and rename variable Trying to fully understand how this script operates.. this patch adds some notes to the script for developers and also renames a variable that I found was being re-used.
https://github.com/PTFS-Europe/koha/tree/bug_10190_submission
I'm still cleaning up the commits and Matt and I are working on adding tests. But, it's all working pretty well now and testing is ongoing in production at a customer site. I've set up a sandbox for anyone wanting to have a play: https://staff-overdues.sandboxes.ptfs-europe.co.uk/ Test plan: 1) Head on over to either 'Tools > Overdue notice/status triggers' or 'Administration > Circulation triggers' (They're the same page) 2) Add a base ALL, ALL, ALL rule 3) Add rule overrides for various combinations of Library, Patron category and Item type
Created attachment 171727 [details] [review] Bug 10190: DB migration for overduerules This patch adds the data migration for overduerules to circulation rules. We attempt to create the default rules based on most commonly occuring rule values and then add overrides for all other rule values we find. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171728 [details] [review] Bug 10190: DBIC Schema update
Created attachment 171729 [details] [review] Bug 10190: Remove OverdueRule Koha Objects
Created attachment 171730 [details] [review] Bug 10190: Fix unit test to exclude _X_ rules Sponsored-by: Glasgow Colleges Library Group
Created attachment 171731 [details] [review] Bug 10190: Add support for incrementable circulation rules This patch adds support for _X_ grouped circulation rules to both the internal and external API's for handling circulation rules. This allows us to group overdue_X_rulename rules by an 'X' integer so we can have triggers incremented and grouped together. We also add the new overdue_x_* rule definition here for overdue trigger rules. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171732 [details] [review] Bug 10190: Add a vue app for circulation triggers This patch adds a new vue based page for administering circulation triggers. This aims to replace the 'Overdue notice/status triggers' page available from Tools. The new page is available from Koha administration > Circulation triggers Sponsored-by: Glasgow Colleges Library Group
Created attachment 171733 [details] [review] Bug 10190: Add components to list/add and edit circulation triggers These new components are used by the new 'Circulation triggers' administration page to display existing trigger rules and add the ability to add new rules and edit existing ones. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171734 [details] [review] Bug 10190: Add a library API client This patch adds a library api client to the vue ecosystem for later use in the Circulation triggers page. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171735 [details] [review] Bug 10190: Add itemtypes and patron categories to the item/patron api clients This patch adds patron and itemtype api clients for the Vue ecosystem Sponsored-by: Glasgow Colleges Library Group
Created attachment 171736 [details] [review] Bug 10190: Let the circulation_rules endpoint accept query parameters Sponsored-by: Glasgow Colleges Library Group
Created attachment 171737 [details] [review] Bug 10190: Update GetBranchcodesWithOverdueRules to use CirculationRules This patch updates the C4::Overdues method GetBranchecodesWithOverdueRules to refer to the new location of these rules in the circulation rules table. We also update the unit tests to move the mocked data into the right location. We could perhaps migrate this method in Koha::CirculationRules at a later date? Sponsored-by: Glasgow Colleges Library Group
Created attachment 171738 [details] [review] Bug 10190: Update overdue_notices script to refer to circ rules This patch updates the overdue_notices cron script to refer to the new location for overdue rules found in the circulation rules table now. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171739 [details] [review] Bug 10190: Add itemtype handling to overdue_notices script This patch enhances the overdue_notices script to understand the newly introduced itemtype level for overdue rules. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171740 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group
Created attachment 171741 [details] [review] Bug 10190: Add letter names to tables Sponsored-by: Glasgow Colleges Library Group
Created attachment 171742 [details] [review] Bug 10190: Set tab based on trigger edited/created Sponsored-by: Glasgow Colleges Library Group
Created attachment 171743 [details] [review] Bug 10190: Add fallback to 'Default' text for notice Sponsored-by: Glasgow Colleges Library Group
Created attachment 171744 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group
Created attachment 171745 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group
Created attachment 171746 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules
Created attachment 171747 [details] [review] Bug 10190: Add comments to overdue_notices and rename variable Trying to fully understand how this script operates.. this patch adds some notes to the script for developers and also renames a variable that I found was being re-used.
Created attachment 171748 [details] [review] Bug 10190: Rework logic of overdue_notices The original logic for digesting overdues in the overdue notices script was flawed given the additional itemtype handling introduced in this patchset and the way in which it handles effective fallbacks for rules. This patch tidies the file and reworks the logic so that we fetch overdues by patron category and branch and then iterate through those comparing to the correct effective rules and combing them into digest groups based on notice + trigger level.
Created attachment 171749 [details] [review] Bug 10190: Display effective values and highlight them
Created attachment 171750 [details] [review] Bug 10190: Restore 'edit' from 'Add/Edit' Modal
Created attachment 171751 [details] [review] Bug 10190: Modal form improvements We now fetch all rules on modal open and the effective rule on context selection. This allows us to compare the specific values of the rule we're adding/editing to the fallback values present in the all rules set. We use the fallback rule values as placeholders in the input boxes to highlight to the end user what value will be used should they choose to wipe the input. We also allow wiping inputs and pass 'null' to the API appropriately. Finally, we set a minimum value for delay when adding a new rule to highlight to the user that the new trigger will be added after existing triggers.
Created attachment 171752 [details] [review] Bug 10190: Add custom input for delay This custom input allows for dynamic min/max rules and it updates the input styling to match other inputs on the page including a 'clear' button.
Created attachment 171753 [details] [review] Bug 10190: Clarify workflow This patch re-arranges the edit/add modal form to clarify the need to select context, then decide on editing an existing trigger or adding a new one and then, and only then, exposes the actual rule inputs
Created attachment 171754 [details] [review] Bug 10190: Update for bootstrap 5
Created attachment 171755 [details] [review] Bug 10190: Rebase for rspack
This is all ready for testing now and can be done using the sandbox above, a fresh sandbox or a local dev env.
Installed on sandboxes : updatedatabase error, problem of foreign key constraint
Notes from Nick Clemens at KohaCon24 * Use syspref for 'All libraries' or 'Logged in library' by default - DefaultToLoggedInLibraryOverdueTriggers * We should not be able to save a notice without a transport type (including aking into account fallbacks) * Note about whether we're talking aobut 'patrons home library' or 'items home library' * If a notice template is only set up for one library only display it in the notice dropdown when in that context * Can we expose somewhere the list of all notice trigger context combinations..
Created attachment 172588 [details] [review] Bug 10190: DB migration for overduerules This patch adds the data migration for overduerules to circulation rules. We attempt to create the default rules based on most commonly occuring rule values and then add overrides for all other rule values we find. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172589 [details] [review] Bug 10190: DBIC Schema update
Created attachment 172590 [details] [review] Bug 10190: Remove OverdueRule Koha Objects
Created attachment 172591 [details] [review] Bug 10190: Fix unit test to exclude _X_ rules Sponsored-by: Glasgow Colleges Library Group
Created attachment 172592 [details] [review] Bug 10190: Add support for incrementable circulation rules This patch adds support for _X_ grouped circulation rules to both the internal and external API's for handling circulation rules. This allows us to group overdue_X_rulename rules by an 'X' integer so we can have triggers incremented and grouped together. We also add the new overdue_x_* rule definition here for overdue trigger rules. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172593 [details] [review] Bug 10190: Add a vue app for circulation triggers This patch adds a new vue based page for administering circulation triggers. This aims to replace the 'Overdue notice/status triggers' page available from Tools. The new page is available from Koha administration > Circulation triggers Sponsored-by: Glasgow Colleges Library Group
Created attachment 172594 [details] [review] Bug 10190: Add components to list/add and edit circulation triggers These new components are used by the new 'Circulation triggers' administration page to display existing trigger rules and add the ability to add new rules and edit existing ones. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172595 [details] [review] Bug 10190: Add a library API client This patch adds a library api client to the vue ecosystem for later use in the Circulation triggers page. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172596 [details] [review] Bug 10190: Add itemtypes and patron categories to the item/patron api clients This patch adds patron and itemtype api clients for the Vue ecosystem Sponsored-by: Glasgow Colleges Library Group
Created attachment 172597 [details] [review] Bug 10190: Let the circulation_rules endpoint accept query parameters Sponsored-by: Glasgow Colleges Library Group
Created attachment 172598 [details] [review] Bug 10190: Update GetBranchcodesWithOverdueRules to use CirculationRules This patch updates the C4::Overdues method GetBranchecodesWithOverdueRules to refer to the new location of these rules in the circulation rules table. We also update the unit tests to move the mocked data into the right location. We could perhaps migrate this method in Koha::CirculationRules at a later date? Sponsored-by: Glasgow Colleges Library Group
Created attachment 172599 [details] [review] Bug 10190: Update overdue_notices script to refer to circ rules This patch updates the overdue_notices cron script to refer to the new location for overdue rules found in the circulation rules table now. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172600 [details] [review] Bug 10190: Add itemtype handling to overdue_notices script This patch enhances the overdue_notices script to understand the newly introduced itemtype level for overdue rules. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172601 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group
Created attachment 172602 [details] [review] Bug 10190: Add letter names to tables Sponsored-by: Glasgow Colleges Library Group
Created attachment 172603 [details] [review] Bug 10190: Set tab based on trigger edited/created Sponsored-by: Glasgow Colleges Library Group
Created attachment 172604 [details] [review] Bug 10190: Add fallback to 'Default' text for notice Sponsored-by: Glasgow Colleges Library Group
Created attachment 172605 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group
Created attachment 172606 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group
Created attachment 172607 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules
Created attachment 172608 [details] [review] Bug 10190: Add comments to overdue_notices and rename variable Trying to fully understand how this script operates.. this patch adds some notes to the script for developers and also renames a variable that I found was being re-used.
Created attachment 172609 [details] [review] Bug 10190: Rework logic of overdue_notices The original logic for digesting overdues in the overdue notices script was flawed given the additional itemtype handling introduced in this patchset and the way in which it handles effective fallbacks for rules. This patch tidies the file and reworks the logic so that we fetch overdues by patron category and branch and then iterate through those comparing to the correct effective rules and combing them into digest groups based on notice + trigger level.
Created attachment 172612 [details] [review] Bug 10190: Display effective values and highlight them
Created attachment 172614 [details] [review] Bug 10190: Restore 'edit' from 'Add/Edit' Modal
Created attachment 172617 [details] [review] Bug 10190: Modal form improvements We now fetch all rules on modal open and the effective rule on context selection. This allows us to compare the specific values of the rule we're adding/editing to the fallback values present in the all rules set. We use the fallback rule values as placeholders in the input boxes to highlight to the end user what value will be used should they choose to wipe the input. We also allow wiping inputs and pass 'null' to the API appropriately. Finally, we set a minimum value for delay when adding a new rule to highlight to the user that the new trigger will be added after existing triggers.
Created attachment 172619 [details] [review] Bug 10190: Add custom input for delay This custom input allows for dynamic min/max rules and it updates the input styling to match other inputs on the page including a 'clear' button.
Created attachment 172621 [details] [review] Bug 10190: Clarify workflow This patch re-arranges the edit/add modal form to clarify the need to select context, then decide on editing an existing trigger or adding a new one and then, and only then, exposes the actual rule inputs
Created attachment 172622 [details] [review] Bug 10190: Update for bootstrap 5
Created attachment 172623 [details] [review] Bug 10190: Rebase for rspack
Created attachment 172624 [details] [review] Bug 10190: Rename 'All *' to 'Default rule for *' Try to clarify that the 'All' rules are actually fallback/default rules for when there's not a more specific rule for the context. We need to expose these clearly for each 'Library' view too which we're not currently doing.
Created attachment 172625 [details] [review] Bug 10190: Take DefaultToLoggedInLibraryOverdueTriggers into account This updates the logic for the default page load to take account of DefaultToLoggedInLibraryOverdueTriggers.
Created attachment 172626 [details] [review] Bug 10190: Only display default and library specific letters This patch filters the list of letter templates available to use by only those available for the context the rule is being added for (i.e. the correct library relation)
Created attachment 172630 [details] [review] Bug 10190: Display value of OverdueNoticeFrom in note
Created attachment 172631 [details] [review] Bug 10190: DB migration for overduerules This patch adds the data migration for overduerules to circulation rules. We attempt to create the default rules based on most commonly occuring rule values and then add overrides for all other rule values we find. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172632 [details] [review] Bug 10190: DBIC Schema update Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172633 [details] [review] Bug 10190: Remove OverdueRule Koha Objects Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172634 [details] [review] Bug 10190: Fix unit test to exclude _X_ rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172635 [details] [review] Bug 10190: Add support for incrementable circulation rules This patch adds support for _X_ grouped circulation rules to both the internal and external API's for handling circulation rules. This allows us to group overdue_X_rulename rules by an 'X' integer so we can have triggers incremented and grouped together. We also add the new overdue_x_* rule definition here for overdue trigger rules. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172636 [details] [review] Bug 10190: Add a vue app for circulation triggers This patch adds a new vue based page for administering circulation triggers. This aims to replace the 'Overdue notice/status triggers' page available from Tools. The new page is available from Koha administration > Circulation triggers Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172637 [details] [review] Bug 10190: Add components to list/add and edit circulation triggers These new components are used by the new 'Circulation triggers' administration page to display existing trigger rules and add the ability to add new rules and edit existing ones. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172638 [details] [review] Bug 10190: Add a library API client This patch adds a library api client to the vue ecosystem for later use in the Circulation triggers page. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172639 [details] [review] Bug 10190: Add itemtypes and patron categories to the item/patron api clients This patch adds patron and itemtype api clients for the Vue ecosystem Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172640 [details] [review] Bug 10190: Let the circulation_rules endpoint accept query parameters Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172641 [details] [review] Bug 10190: Update GetBranchcodesWithOverdueRules to use CirculationRules This patch updates the C4::Overdues method GetBranchecodesWithOverdueRules to refer to the new location of these rules in the circulation rules table. We also update the unit tests to move the mocked data into the right location. We could perhaps migrate this method in Koha::CirculationRules at a later date? Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172642 [details] [review] Bug 10190: Update overdue_notices script to refer to circ rules This patch updates the overdue_notices cron script to refer to the new location for overdue rules found in the circulation rules table now. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172643 [details] [review] Bug 10190: Add itemtype handling to overdue_notices script This patch enhances the overdue_notices script to understand the newly introduced itemtype level for overdue rules. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172644 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172645 [details] [review] Bug 10190: Add letter names to tables Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172646 [details] [review] Bug 10190: Set tab based on trigger edited/created Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172647 [details] [review] Bug 10190: Add fallback to 'Default' text for notice Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172648 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172649 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172650 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172651 [details] [review] Bug 10190: Add comments to overdue_notices and rename variable Trying to fully understand how this script operates.. this patch adds some notes to the script for developers and also renames a variable that I found was being re-used. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172652 [details] [review] Bug 10190: Rework logic of overdue_notices The original logic for digesting overdues in the overdue notices script was flawed given the additional itemtype handling introduced in this patchset and the way in which it handles effective fallbacks for rules. This patch tidies the file and reworks the logic so that we fetch overdues by patron category and branch and then iterate through those comparing to the correct effective rules and combing them into digest groups based on notice + trigger level. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172653 [details] [review] Bug 10190: Display effective values and highlight them Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172654 [details] [review] Bug 10190: Restore 'edit' from 'Add/Edit' Modal Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172655 [details] [review] Bug 10190: Modal form improvements We now fetch all rules on modal open and the effective rule on context selection. This allows us to compare the specific values of the rule we're adding/editing to the fallback values present in the all rules set. We use the fallback rule values as placeholders in the input boxes to highlight to the end user what value will be used should they choose to wipe the input. We also allow wiping inputs and pass 'null' to the API appropriately. Finally, we set a minimum value for delay when adding a new rule to highlight to the user that the new trigger will be added after existing triggers. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172656 [details] [review] Bug 10190: Add custom input for delay This custom input allows for dynamic min/max rules and it updates the input styling to match other inputs on the page including a 'clear' button. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172657 [details] [review] Bug 10190: Clarify workflow This patch re-arranges the edit/add modal form to clarify the need to select context, then decide on editing an existing trigger or adding a new one and then, and only then, exposes the actual rule inputs Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172658 [details] [review] Bug 10190: Update for bootstrap 5 Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172659 [details] [review] Bug 10190: Rebase for rspack Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172660 [details] [review] Bug 10190: Rename 'All *' to 'Default rule for *' Try to clarify that the 'All' rules are actually fallback/default rules for when there's not a more specific rule for the context. We need to expose these clearly for each 'Library' view too which we're not currently doing. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172661 [details] [review] Bug 10190: Take DefaultToLoggedInLibraryOverdueTriggers into account This updates the logic for the default page load to take account of DefaultToLoggedInLibraryOverdueTriggers. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172662 [details] [review] Bug 10190: Only display default and library specific letters This patch filters the list of letter templates available to use by only those available for the context the rule is being added for (i.e. the correct library relation) Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 172663 [details] [review] Bug 10190: Display value of OverdueNoticeFrom in note Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
(In reply to Martin Renvoize (ashimema) from comment #65) > Notes from Nick Clemens at KohaCon24 > * Use syspref for 'All libraries' or 'Logged in library' by default - > DefaultToLoggedInLibraryOverdueTriggers DONE > * Note about whether we're talking aobut 'patrons home library' or 'items > home library' DONE > * If a notice template is only set up for one library only display it in the > notice dropdown when in that context DONE > * Can we expose somewhere the list of all notice trigger context > combinations.. In a follow-up bug I reckon.
Created attachment 178643 [details] [review] Bug 10190: DB migration for overduerules This patch adds the data migration for overduerules to circulation rules. We attempt to create the default rules based on most commonly occuring rule values and then add overrides for all other rule values we find. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178644 [details] [review] Bug 10190: DBIC Schema update Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178645 [details] [review] Bug 10190: Remove OverdueRule Koha Objects Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178646 [details] [review] Bug 10190: Fix unit test to exclude _X_ rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178647 [details] [review] Bug 10190: Add support for incrementable circulation rules This patch adds support for _X_ grouped circulation rules to both the internal and external API's for handling circulation rules. This allows us to group overdue_X_rulename rules by an 'X' integer so we can have triggers incremented and grouped together. We also add the new overdue_x_* rule definition here for overdue trigger rules. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178648 [details] [review] Bug 10190: Add a vue app for circulation triggers This patch adds a new vue based page for administering circulation triggers. This aims to replace the 'Overdue notice/status triggers' page available from Tools. The new page is available from Koha administration > Circulation triggers Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178649 [details] [review] Bug 10190: Add components to list/add and edit circulation triggers These new components are used by the new 'Circulation triggers' administration page to display existing trigger rules and add the ability to add new rules and edit existing ones. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178650 [details] [review] Bug 10190: Add a library API client This patch adds a library api client to the vue ecosystem for later use in the Circulation triggers page. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178651 [details] [review] Bug 10190: Add itemtypes and patron categories to the item/patron api clients This patch adds patron and itemtype api clients for the Vue ecosystem Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178652 [details] [review] Bug 10190: Let the circulation_rules endpoint accept query parameters Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178653 [details] [review] Bug 10190: Update GetBranchcodesWithOverdueRules to use CirculationRules This patch updates the C4::Overdues method GetBranchecodesWithOverdueRules to refer to the new location of these rules in the circulation rules table. We also update the unit tests to move the mocked data into the right location. We could perhaps migrate this method in Koha::CirculationRules at a later date? Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178654 [details] [review] Bug 10190: Update overdue_notices script to refer to circ rules This patch updates the overdue_notices cron script to refer to the new location for overdue rules found in the circulation rules table now. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178655 [details] [review] Bug 10190: Add itemtype handling to overdue_notices script This patch enhances the overdue_notices script to understand the newly introduced itemtype level for overdue rules. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178656 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178657 [details] [review] Bug 10190: Add letter names to tables Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178658 [details] [review] Bug 10190: Set tab based on trigger edited/created Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178659 [details] [review] Bug 10190: Add fallback to 'Default' text for notice Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178660 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178661 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178662 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178663 [details] [review] Bug 10190: Add comments to overdue_notices and rename variable Trying to fully understand how this script operates.. this patch adds some notes to the script for developers and also renames a variable that I found was being re-used. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178664 [details] [review] Bug 10190: Rework logic of overdue_notices The original logic for digesting overdues in the overdue notices script was flawed given the additional itemtype handling introduced in this patchset and the way in which it handles effective fallbacks for rules. This patch tidies the file and reworks the logic so that we fetch overdues by patron category and branch and then iterate through those comparing to the correct effective rules and combing them into digest groups based on notice + trigger level. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178665 [details] [review] Bug 10190: Display effective values and highlight them Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178666 [details] [review] Bug 10190: Restore 'edit' from 'Add/Edit' Modal Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178667 [details] [review] Bug 10190: Modal form improvements We now fetch all rules on modal open and the effective rule on context selection. This allows us to compare the specific values of the rule we're adding/editing to the fallback values present in the all rules set. We use the fallback rule values as placeholders in the input boxes to highlight to the end user what value will be used should they choose to wipe the input. We also allow wiping inputs and pass 'null' to the API appropriately. Finally, we set a minimum value for delay when adding a new rule to highlight to the user that the new trigger will be added after existing triggers. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178668 [details] [review] Bug 10190: Add custom input for delay This custom input allows for dynamic min/max rules and it updates the input styling to match other inputs on the page including a 'clear' button. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178669 [details] [review] Bug 10190: Clarify workflow This patch re-arranges the edit/add modal form to clarify the need to select context, then decide on editing an existing trigger or adding a new one and then, and only then, exposes the actual rule inputs Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178670 [details] [review] Bug 10190: Update for bootstrap 5 Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178671 [details] [review] Bug 10190: Rebase for rspack Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178672 [details] [review] Bug 10190: Rename 'All *' to 'Default rule for *' Try to clarify that the 'All' rules are actually fallback/default rules for when there's not a more specific rule for the context. We need to expose these clearly for each 'Library' view too which we're not currently doing. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178673 [details] [review] Bug 10190: Take DefaultToLoggedInLibraryOverdueTriggers into account This updates the logic for the default page load to take account of DefaultToLoggedInLibraryOverdueTriggers. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178674 [details] [review] Bug 10190: Only display default and library specific letters This patch filters the list of letter templates available to use by only those available for the context the rule is being added for (i.e. the correct library relation) Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 178675 [details] [review] Bug 10190: Display value of OverdueNoticeFrom in note Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Comment on attachment 178644 [details] [review] Bug 10190: DBIC Schema update Review of attachment 178644 [details] [review]: ----------------------------------------------------------------- ::: Koha/Schema/Result/MessageTransportType.pm @@ +133,5 @@ > ); > > > +# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-08-15 11:53:13 > +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Bx96QiqHxZ9SNbgK8dSwMg There are no manual changes to split in separate commit. Whole commit should be marked as [DO NOT PUSH] https://wiki.koha-community.org/wiki/Database_updates#Files_to_change
updatedatabase doesn't work: (have a koha on main, apply the patches and update) $ updatedatabase Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $itemtype in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 229. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 240. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 251. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $itemtype in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 229. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 240. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 251. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $itemtype in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 229. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 240. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 251. DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl [23:24:08]: Bug 10190 - Migrate overduerules to circulation_rules Inserting :undef: default most frequent delay for overdue_1: Inserting :undef:undef default most frequent notice for overdue_1: Inserting :undef:undef default most frequent mtt for overdue_1: Inserting :undef:undef default most frequent restrict for overdue_1: 0 Inserting :undef: default most frequent delay for overdue_2: Inserting :undef:undef default most frequent notice for overdue_2: Inserting :undef:undef default most frequent mtt for overdue_2: Inserting :undef:undef default most frequent restrict for overdue_2: 0 Inserting :undef: default most frequent delay for overdue_3: Inserting :undef:undef default most frequent notice for overdue_3: Inserting :undef:undef default most frequent mtt for overdue_3: Inserting :undef:undef default most frequent restrict for overdue_3: 0 ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails at /kohadevbox/koha/C4/Installer.pm line 771
MariaDB 10.5.26 in case that matters.
Comment on attachment 178648 [details] [review] Bug 10190: Add a vue app for circulation triggers Review of attachment 178648 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt @@ +25,5 @@ > + <!-- this is closed in intranet-bottom.inc --> > + > + [% MACRO jsinclude BLOCK %] > + [% INCLUDE 'calendar.inc' %] > + <!-- FIXME: this shouldn't be needed --> Is the FIXME for calendar.inc above or datatables.inc bellow or the 4 includes below?
(In reply to Victor Grousset/tuxayo from comment #169) > Comment on attachment 178648 [details] [review] [review] > Bug 10190: Add a vue app for circulation triggers > > Review of attachment 178648 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt > @@ +25,5 @@ > > + <!-- this is closed in intranet-bottom.inc --> > > + > > + [% MACRO jsinclude BLOCK %] > > + [% INCLUDE 'calendar.inc' %] > > + <!-- FIXME: this shouldn't be needed --> > > Is the FIXME for calendar.inc above or datatables.inc bellow or the 4 > includes below? It was for calendar.inc and if you look at the other vue apps, it's consistent throughout them so far.
*** Bug 12772 has been marked as a duplicate of this bug. ***
I wanted to see what it looked like, but unfortunately, the patch doesn't apply anymore...
Koha's been moving so fast.. probably best to use the public branch for this: https://github.com/openfifth/koha/tree/bug_10190_submission-rebased
We've been reworking this considerably, given live use feedback. Chloe's taken over the reins, so I'm updating the assignee, but the reality is Chloe, Matt, and I have all done a lot of work between us on this one.
Created attachment 188075 [details] [review] Bug 10190: DB migration for overduerules This patch adds the data migration for overduerules to circulation rules. We attempt to create the default rules based on most commonly occuring rule values and then add overrides for all other rule values we find. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188076 [details] [review] Bug 10190: DBIC Schema update Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188077 [details] [review] Bug 10190: Remove OverdueRule Koha Objects Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188078 [details] [review] Bug 10190: Fix unit test to exclude _X_ rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188079 [details] [review] Bug 10190: Add support for incrementable circulation rules This patch adds support for _X_ grouped circulation rules to both the internal and external API's for handling circulation rules. This allows us to group overdue_X_rulename rules by an 'X' integer so we can have triggers incremented and grouped together. We also add the new overdue_x_* rule definition here for overdue trigger rules. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188080 [details] [review] Bug 10190: Update GetBranchcodesWithOverdueRules to use CirculationRules This patch updates the C4::Overdues method GetBranchecodesWithOverdueRules to refer to the new location of these rules in the circulation rules table. We also update the unit tests to move the mocked data into the right location. We could perhaps migrate this method in Koha::CirculationRules at a later date? Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188081 [details] [review] Bug 10190: Rework logic of overdue_notices This patch enhances the overdue_notices script to understand the newly introduced itemtype level for overdue rules. The original logic for digesting overdues in the overdue notices script was flawed given the additional itemtype handling introduced in this patchset and the way in which it handles effective fallbacks for rules. This patch tidies the file and reworks the logic so that we fetch overdues by patron category and branch and then iterate through those comparing to the correct effective rules and combing them into digest groups based on notice + trigger level. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188082 [details] [review] Bug 10190: Let the circulation_rules endpoint accept query parameters Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188083 [details] [review] Bug 10190: Complete Vue.js rewrite of Circulation Triggers administration interface This patch introduces a modern Vue.js-based interface for managing circulation triggers (overdue notice/status triggers), replacing the legacy Tools-based interface with a more intuitive, feature-rich administration page. New Features Enhanced Visibility & Navigation - Exhaustive effective rule lists showing all rules that apply to each context - Toggle between "all exhaustive rules" and "explicitly set rules" views - Advanced filtering by patron category and/or item type - Clear display of fallback/default rules for each context Rule Management Actions - Delete triggers (last trigger in a sequence for any given library) - Reset entire rule sets for a given context (with safeguards) - Edit existing triggers with improved context awareness - Add new triggers with intelligent delay constraints Improved User Experience - Multi-step form wizard with clear workflow progression - Context-aware letter template filtering (only shows applicable templates) - Placeholder values showing fallback rules when clearing inputs - Dynamic min/max constraints for delay values based on neighboring triggers - Real-time form validation and feedback - Partial page reloads (only reload affected sections, not entire page) - Specific loading messages for different operations Technical Improvements - Centralized Vuex store for state management - Reusable CirculationTriggersForm component - API clients for libraries, circulation rules, item types, and patron categories - Consistent async/await pattern throughout - Separation of repositories, utilities, and services - Improved naming consistency and code maintainability - "Dumb" TriggersTable component for better separation of concerns Test Plan Prerequisites 1. Log in to staff interface with appropriate permissions (tools_edit_notice_status_triggers) 2. Ensure you have test data: multiple libraries, patron categories, item types, and notice templates 3. Navigate to Administration > Circulation triggers (new interface) Part 1: Basic Navigation & Interface 1. Verify the new "Circulation triggers" link appears in Administration home under the Circulation section 2. Access the new interface and confirm it loads successfully 3. Verify the default view shows either "All libraries" or your logged-in library based on DefaultToLoggedInLibraryOverdueTriggers system preference 4. Confirm the interface displays a list of triggers organized by context (library/patron category/item type combinations) Part 2: Viewing and Filtering Rules 1. Toggle between "All exhaustive rules" and "Explicitly set rules" views - Verify "All exhaustive rules" shows inherited/fallback rules - Verify "Explicitly set rules" shows only rules you've specifically configured 2. Test the patron category filter: - Select a specific patron category - Confirm only rules for that category are displayed 3. Test the item type filter: - Select a specific item type - Confirm only rules for that item type are displayed 4. Test combined filters (patron category + item type simultaneously) 5. Clear filters and verify all rules display again Part 3: Adding New Triggers 1. Click "Add trigger" button 2. Step 1 - Confirm Context: - Select Library (required) - Select Patron category (required) - Select Item type (required) - Click "Confirm context" 3. Step 2 - Review Existing Triggers: - Verify existing triggers for this context are displayed - Note the trigger numbers and delays 4. Step 3 - Configure New Trigger: - Enter a delay value (should be constrained by neighboring triggers) - Try entering an invalid delay (too low or too high) - verify validation - Set "Restricts checkouts" (Yes/No/Fallback to default) - Note the placeholder showing the fallback value - Select a letter template - Verify only templates for the selected library context appear - Set "Debarring" options if desired - Select "Charged notice fees" if applicable 5. Submit the form 6. Verify the new trigger appears in the list without a full page reload 7. Verify success message is displayed Part 4: Editing Existing Triggers 1. Locate an existing trigger in the list 2. Click the edit icon/button 3. Verify the form pre-populates with existing values 4. Test delay modification: - Try changing the delay within valid bounds - should succeed - Try changing delay beyond neighboring triggers - should show validation error - Use the increment/decrement buttons to adjust delay 5. Test clearing values: - Clear a field (e.g., delay or restrict checkouts) - Verify placeholder shows the fallback/default value - Submit with cleared value - should save as null and inherit fallback 6. Change the letter template to a different valid template 7. Submit changes 8. Verify the trigger updates in the list without full page reload 9. Verify the changes persist after refreshing the page Part 5: Deleting Triggers 1. Identify a trigger that is the last in its sequence 2. Click the delete button/icon 3. Verify a confirmation dialog appears 4. Confirm deletion 5. Verify the trigger is removed from the list without page reload 6. Verify the deletion persists after page refresh 7. Test delete constraints: - Try to delete a trigger that's not the last in sequence - Verify appropriate error/warning message Part 6: Resetting Rule Sets 1. Locate a context with explicitly set rules (not just inherited) 2. Ensure this is not the only rule set for this trigger sequence 3. Click the "Reset" button for that context 4. Verify confirmation dialog explains what will be reset 5. Confirm the reset 6. Verify the explicit rules are removed and context now shows inherited rules 7. Verify changes persist after page refresh 8. Test reset constraints: - Try to reset when it would leave no rules for that trigger - Verify appropriate warning/prevention Part 7: Context-Specific Behavior 1. Test library-specific letter filtering: - Create/edit a trigger for a specific library - Verify only letter templates with matching branchcode or default templates appear - Switch to a different library context - Verify the letter list updates accordingly 2. Test default rules: - View rules for "All libraries" / "All patron categories" / "All item types" - Verify these are labeled as "Default rule for [context]" - Confirm these serve as fallback for more specific rules 3. Test DefaultToLoggedInLibraryOverdueTriggers preference: - Set preference to "Yes" - Reload circulation triggers page - Verify it defaults to your logged-in library - Set preference to "No" - Verify it defaults to "All libraries" Part 8: Form Workflow & Validation 1. Test required fields: - Try submitting add/edit form without selecting library - should fail - Try submitting without patron category - should fail - Try submitting without item type - should fail 2. Test stepper navigation: - Start adding a trigger - Verify you must confirm context before proceeding - Verify you see existing triggers before editing/adding - Verify trigger form only appears after context confirmation 3. Test cancellation: - Start adding/editing a trigger - Click Cancel - Verify you return to the list without changes - Verify no partial data was saved Part 9: Loading States & Messages 1. On slow connections or with many rules: - Verify "Loading context..." appears while fetching context data - Verify "Loading rule set information..." appears while fetching rules - Verify section-specific loading messages appear during updates 2. After form submission: - Verify appropriate success/error messages display - Verify loading indicators during async operations Part 10: Permissions 1. Log in as a user without tools_edit_notice_status_triggers permission 2. Verify the "Circulation triggers" link does not appear in Administration 3. Try accessing /cgi-bin/koha/admin/circulation_triggers directly 4. Verify access is denied Part 11: Data Integrity & Persistence 1. Create a complex set of rules with multiple triggers across different contexts 2. Refresh the page and verify all rules persist 3. Restart the Koha services 4. Verify all rules still display correctly 5. Check the database circulation_rules table: - Verify rules are stored with correct rule_name pattern (overdue_1_delay, etc.) - Verify null values are properly stored for inheritance - Verify the overdue_X_has_rules rule kind is present Part 12: Edge Cases 1. Test with no existing triggers - verify empty state message 2. Test with maximum trigger sequences (e.g., trigger 1, 2, 3) 3. Test rapid successive edits 4. Test with special characters in letter template names 5. Test browser back/forward buttons during form workflow Part 13: Comparison with Legacy Interface 1. Access the old interface if still available (via Tools menu) 2. Create a trigger using the new interface 3. Verify it appears correctly in the old interface (if applicable) 4. Create a trigger using the old interface (if still available) 5. Verify it appears correctly in the new interface --- Notes for QA - This is a complete UI rewrite; the underlying API remains unchanged - The interface is mobile-responsive and should be tested on various screen sizes - Vue.js components are compiled via Rspack; run yarn build after pulling - All existing circulation trigger functionality should continue to work - The URL rewrite rule ensures /admin/circulation_triggers routes correctly Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Co-authored-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> Co-authored-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188084 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188085 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188086 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188087 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 188088 [details] [review] Bug 10190: Comprehensive Cypress tests for Circulation Triggers UI This patch significantly enhances the Cypress test coverage for the new Circulation Triggers administration interface, expanding from basic breadcrumb checks to comprehensive end-to-end testing. Test Coverage: - Initial page load and API error handling - Context filtering (library, patron category, item type) - View toggling between explicit and all applicable rules - Tab navigation between multiple triggers - Full CRUD operations (Create, Read, Update, Delete) - Rule set reset functionality - Letter template filtering by library context - Form validation and constraint checking - Loading states and empty states - Error handling for all operations - Browser navigation (back/forward buttons) - Complex scenarios with inheritance hierarchies New Features: - Mock data helper functions for test fixtures - Comprehensive API interception for all endpoints - Tests for both success and error paths - Placeholder value testing for fallback rules - Increment/decrement button testing - Multi-step form workflow validation - Confirmation dialog testing for destructive operations The tests follow Koha's Cypress testing patterns established in the ERM module, using proper API mocking, clear test descriptions, and organized test suites.
As Victor Grousset/tuxayo wrote in Comment 167 (2025-02-26) updatedatabase doesn't work: (have a koha on main, apply the patches and update) $ updatedatabase Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $itemtype in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 229. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 240. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 251. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $itemtype in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 229. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 240. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 251. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $itemtype in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 218. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 229. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 240. Use of uninitialized value $branchcode_value in concatenation (.) or string at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl line 251. DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_10190.pl [08:57:11]: Bug 10190 - Migrate overduerules to circulation_rules Inserting :undef: default most frequent delay for overdue_1: Inserting :undef:undef default most frequent notice for overdue_1: Inserting :undef:undef default most frequent mtt for overdue_1: Inserting :undef:undef default most frequent restrict for overdue_1: 0 Inserting :undef: default most frequent delay for overdue_2: Inserting :undef:undef default most frequent notice for overdue_2: Inserting :undef:undef default most frequent mtt for overdue_2: Inserting :undef:undef default most frequent restrict for overdue_2: 0 Inserting :undef: default most frequent delay for overdue_3: Inserting :undef:undef default most frequent notice for overdue_3: Inserting :undef:undef default most frequent mtt for overdue_3: Inserting :undef:undef default most frequent restrict for overdue_3: 0 ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails at /kohadevbox/koha/C4/Installer.pm line 825
Created attachment 188476 [details] [review] Bug 10190: (QA follow-up): Fix removal of tables
(In reply to Pedro Amorim from comment #190) > Created attachment 188476 [details] [review] [review] > Bug 10190: (QA follow-up): Fix removal of tables This fixes the database error but does not address the undef warnings.
To have it working on a upgrade, I had to run the following: Apply patches $ updatabase $ yarn js:build $ sudo perl /kohadevbox/misc4dev/cp_debian_files.pl --instance=kohadev --koha_dir=/kohadevbox/koha --gitify_dir=/kohadevbox/gitify $ restart_all The cp_debian_files.pl is required or else the URL always returns 404: http://localhost:8081/cgi-bin/koha/admin/circulation_triggers I believe this is why applying patches and doing reset_all, but applying patches and doing restart_all does not, it's missing the cp_debian_files part, which I believe copies debian/templates/apache-shared-intranet.conf among other files. It is now producing a JavaScript error on the console though, which may or may not highlight an issue with the data migration script.
We tested several hours and I will provide more feedback this week but here are the most relevant Bugs we found: 1.) Migration scenario have some overdue rule, then apply the patch and do updatabase and yarn buid Error 404 in staff interface Please see Pedros comment 192 In your testplan in comment 183 you write in Part 11: Data Integrity & Persistence 5. Check the database circulation_rules table: - Verify the overdue_X_has_rules rule kind is present Is this the missing piece for the migrated data ? 2.) Adding a trigger for a combination that exists in circ rules it is not possible to configure an overdue trigger with a combination that already exists as a circulation rule. Example to reproduce in ktd with test data: - Go to circ rules (/cgi-bin/koha/admin/smart-rules.pl) - Set a rule for All Libraries / All patron categories / Itemtype Books - Check via SQL: select * from circulation_rules - Go to overdue rules (/cgi-bin/koha/admin/circulation_triggers ) - Set a rule for All Libraries / All patron categories / All Itemtype - Now try to edit the delay for the applied rule with combination All Libraries / All patron categories / Itemtype Books --> Message in blue box "Your changes could not be saved as this circulation trigger was updated elsewhere. Please see the updated trigger below." It is not possible to save / submit - Now edit the delay for the applied rule with combination All Libraries / Patron category Homebound / Itemtype Books --> You can save without problems, if this combination does not exist in the database table circulation_rules 3.) Permissions Error if you are not an admin The Overlay rules are part of the tools and also the permission is part of the tools section. That means that it was enough to give users the edit_notice_status_triggers permission, it was not neccessary to give any administration permission (parameters) . Now you moved the overdue triggers to administration. If you now log in as a person with edit_notice_status_triggers permission you can see the tools module and click on the menu for overdue triggers but then you get the error message "Something went wrong: Error: Authorization failure. Missing required permission(s)." 4.) Edit an existing trigger does not pull delay from database Edit an existing trigger and check that the delay value is 0 , it is not pulled from the database. This is confusing and dangerous because you see the delay value just a few centimeters above in the table. If you save the form, the value in the database is overwritten with 0. To reproduce: follow testplan in comment 183 Part 4.3
(In reply to Michaela Sieber from comment #193) > We tested several hours and I will provide more feedback this week but here > are the most relevant Bugs we found: > > 1.) Migration scenario > > have some overdue rule, then apply the patch and do updatabase and yarn buid > > Error 404 in staff interface > > Please see Pedros comment 192 > > In your testplan in comment 183 you write in > > Part 11: Data Integrity & Persistence > 5. Check the database circulation_rules table: > - Verify the overdue_X_has_rules rule kind is present > > Is this the missing piece for the migrated data ? > > > 2.) Adding a trigger for a combination that exists in circ rules > > it is not possible to configure an overdue trigger with a combination that > already exists as a circulation rule. > > Example to reproduce in ktd with test data: > > - Go to circ rules (/cgi-bin/koha/admin/smart-rules.pl) > - Set a rule for All Libraries / All patron categories / Itemtype Books > - Check via SQL: select * from circulation_rules > - Go to overdue rules (/cgi-bin/koha/admin/circulation_triggers ) > - Set a rule for All Libraries / All patron categories / All Itemtype > - Now try to edit the delay for the applied rule > with combination All Libraries / All patron categories / Itemtype Books > --> Message in blue box "Your changes could not be saved as this > circulation > trigger was updated elsewhere. Please see the updated trigger below." > It is not possible to save / submit > - Now edit the delay for the applied rule > with combination All Libraries / Patron category Homebound / Itemtype > Books > --> You can save without problems, if this combination does not exist in > the database table circulation_rules > > > 3.) Permissions Error if you are not an admin > > The Overlay rules are part of the tools and also the permission is part of > the tools section. That means that it was enough to give users the > edit_notice_status_triggers permission, it was not neccessary to give any > administration permission (parameters) . > Now you moved the overdue triggers to administration. > > If you now log in as a person with edit_notice_status_triggers permission > you can see the tools module and click on the menu for overdue triggers but > then you get the error message "Something went wrong: Error: Authorization > failure. Missing required permission(s)." > > > 4.) Edit an existing trigger does not pull delay from database > > Edit an existing trigger and check that the delay value is 0 , it is not > pulled from the database. This is confusing and dangerous because you see > the delay value just a few centimeters above in the table. > If you save the form, the value in the database is overwritten with 0. > > To reproduce: follow testplan in comment 183 Part 4.3 Thank you for the testing and feedback, much appreciated! Re - 1.) overdue_X_has_rules is a property indicating that some rule(s) exist for a given context/trigger number combination. The most recent version of the integration ensure it is created whenever any rule is created for a given context/trigger number combination. The staff clients relies on this to ascertain whether a rule set exists efficiently. I think the solution might be to have it added to each existing context/trigger number combination - would that be sensible? 2., 3., and 4.: I will do some debugging and put patches together for those, many thanks for reporting them!
Created attachment 189345 [details] [review] Bug 10190: (follow-up) Complete Vue.js rewrite of Circulation Triggers administration interface This patch aims to resolve the following issues as reported by comment 193: - missing atomic update for overde_X_has_rules: - existing delays being overwritten by the client when a rule set is displayed - inability to save circulation overdue rule sets for context for which circulations rules where already set
(In reply to Chloé Zermatten from comment #194) > (In reply to Michaela Sieber from comment #193) > > We tested several hours and I will provide more feedback this week but here > > are the most relevant Bugs we found: > > > > 1.) Migration scenario > > > > have some overdue rule, then apply the patch and do updatabase and yarn buid > > > > Error 404 in staff interface > > > > Please see Pedros comment 192 > > > > In your testplan in comment 183 you write in > > > > Part 11: Data Integrity & Persistence > > 5. Check the database circulation_rules table: > > - Verify the overdue_X_has_rules rule kind is present > > > > Is this the missing piece for the migrated data ? > > > > > > 2.) Adding a trigger for a combination that exists in circ rules > > > > it is not possible to configure an overdue trigger with a combination that > > already exists as a circulation rule. > > > > Example to reproduce in ktd with test data: > > > > - Go to circ rules (/cgi-bin/koha/admin/smart-rules.pl) > > - Set a rule for All Libraries / All patron categories / Itemtype Books > > - Check via SQL: select * from circulation_rules > > - Go to overdue rules (/cgi-bin/koha/admin/circulation_triggers ) > > - Set a rule for All Libraries / All patron categories / All Itemtype > > - Now try to edit the delay for the applied rule > > with combination All Libraries / All patron categories / Itemtype Books > > --> Message in blue box "Your changes could not be saved as this > > circulation > > trigger was updated elsewhere. Please see the updated trigger below." > > It is not possible to save / submit > > - Now edit the delay for the applied rule > > with combination All Libraries / Patron category Homebound / Itemtype > > Books > > --> You can save without problems, if this combination does not exist in > > the database table circulation_rules > > > > > > 3.) Permissions Error if you are not an admin > > > > The Overlay rules are part of the tools and also the permission is part of > > the tools section. That means that it was enough to give users the > > edit_notice_status_triggers permission, it was not neccessary to give any > > administration permission (parameters) . > > Now you moved the overdue triggers to administration. > > > > If you now log in as a person with edit_notice_status_triggers permission > > you can see the tools module and click on the menu for overdue triggers but > > then you get the error message "Something went wrong: Error: Authorization > > failure. Missing required permission(s)." > > > > > > 4.) Edit an existing trigger does not pull delay from database > > > > Edit an existing trigger and check that the delay value is 0 , it is not > > pulled from the database. This is confusing and dangerous because you see > > the delay value just a few centimeters above in the table. > > If you save the form, the value in the database is overwritten with 0. > > > > To reproduce: follow testplan in comment 183 Part 4.3 > > Thank you for the testing and feedback, much appreciated! > > > Re - 1.) overdue_X_has_rules is a property indicating that some rule(s) > exist for a given context/trigger number combination. The most recent > version of the integration ensure it is created whenever any rule is created > for a given context/trigger number combination. The staff clients relies on > this to ascertain whether a rule set exists efficiently. > I think the solution might be to have it added to each existing > context/trigger number combination - would that be sensible? > > 2., 3., and 4.: I will do some debugging and put patches together for those, > many thanks for reporting them! Quick update on this: The latest commit (Attachment #189345 [details]) intends to address point 1, 2, and 4 from the QA feedback. 3 is, to my knowledge, one for discussion as it looks to me like expected behaviour. In the meantime, if any other bugs are found, I'll be happy to look at addressing them!
Unfortunately, the patch doesn't apply: Applying: Bug 10190: Update has_restricting_overdues to refer to circulation rules Using index info to reconstruct a base tree... M Koha/Patron.pm Falling back to patching base and 3-way merge... Auto-merging Koha/Patron.pm CONFLICT (content): Merge conflict in Koha/Patron.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 10190: Update has_restricting_overdues to refer to circulation rules
Created attachment 189863 [details] [review] Bug 10190: Update has_restricting_overdues to refer to circulation rules This patch updates the Koha::Patron->has_restriction_overdues method to refer to the migrated overdue rules. We also correct an oversight in the method prior to this patch that meant that not all overdues were being correctly checked for a restriction.. we were incorrectly filtering to only overdues at the current checkin branch. Finally, we also take account of the itemtype level addition here too. NOTE: There weren't any unit tests for this Koha::Patron method.. we should add some here! Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 189864 [details] [review] Bug 10190: Migrate MarkIssueReturned test to circulation rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 189865 [details] [review] Bug 10190: Remove old overduerules interface This patch updates the link from the tools homepage to open the new circulation triggers administration page. We then remove the old overduerules controller and template, and finally we drop the no longer referenced C4::Overdues::GetOverdueMessageTransportTypes method and associated unit tests. Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 189866 [details] [review] Bug 10190: Update itiva script to reflect move to circ_rules Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk> Signed-off-by: George Harkins <George.Harkins@cityofglasgowcollege.ac.uk>
Created attachment 189867 [details] [review] Bug 10190: Comprehensive Cypress tests for Circulation Triggers UI This patch significantly enhances the Cypress test coverage for the new Circulation Triggers administration interface, expanding from basic breadcrumb checks to comprehensive end-to-end testing. Test Coverage: - Initial page load and API error handling - Context filtering (library, patron category, item type) - View toggling between explicit and all applicable rules - Tab navigation between multiple triggers - Full CRUD operations (Create, Read, Update, Delete) - Rule set reset functionality - Letter template filtering by library context - Form validation and constraint checking - Loading states and empty states - Error handling for all operations - Browser navigation (back/forward buttons) - Complex scenarios with inheritance hierarchies New Features: - Mock data helper functions for test fixtures - Comprehensive API interception for all endpoints - Tests for both success and error paths - Placeholder value testing for fallback rules - Increment/decrement button testing - Multi-step form workflow validation - Confirmation dialog testing for destructive operations The tests follow Koha's Cypress testing patterns established in the ERM module, using proper API mocking, clear test descriptions, and organized test suites.
Created attachment 189868 [details] [review] Bug 10190: (follow-up) Complete Vue.js rewrite of Circulation Triggers administration interface This patch aims to resolve the following issues as reported by comment 193: - missing atomic update for overde_X_has_rules: - existing delays being overwritten by the client when a rule set is displayed - inability to save circulation overdue rule sets for context for which circulations rules where already set