https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #1 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- This has been in the work for some time now, with the UI designed and code written both for this and on the backend as well to modernise the long overdue cron script. As the interface builds on top of the circulation triggers page, recently efforts have shifted towards progressing Bug 10190, as this is currently the main blocker to this work being completed. I will be rebasing against 10190 once finalised, and updating the code to match the more recent version of 10190 as well, as there has been quite a bit of movement and a bit of a rework will be needed here. Further updates to follow! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #2 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 199609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199609&action=edit Bug 39756: feat(staff-UI): display long overdue rules Expose the four following rules in the circulation triggers UI. For a given overdue item for a given patron, after N days (delay): - lost: which lost value, if any should be assigned to the item - charge: whether to charge the patron (create a fine) - forgive fine: whether to forgive the relevant fine (if any) - mark returned: whether to mark the item as returned These four new circulation rules were extracted from the current behaviour of the longoverdue script which is controlled by a mix of system preferences and flags, with the intend to replace the flags entirely so control of the script's behaviour occurs via the staff interface, and to override the system preferences. Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Chloé Zermatten <chloe.zermatten@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #4 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- The two commits submitted here cover the frontend part of the patch, and build on top of the most recent version of 10190. I thought I'd submit those early in case anyone fancies a look at the intended UI / has any feedback on that front :) Cypress tests updates to follow. The backend part of the piece is currently in the works: In short, so that the circulation rules introduced here can be taken into account when processing overdues, the longoverdue.pl and overdue_notices.pl scripts will be deprecated and replaced with a process_overdues.pl script (a thin cron entry point that delegates to new Koha::Overdues::* services (TriggerProcessor, RuleResolver, Repository, ActionExecutor)). I'll add a PoC version of this on here as soon as I have it working so it can be discussed, tested, amended, etc! The test plan will be added later (upon submission of the backend PoC) - that said, I'm happy to put together a quick one to cover UI behaviour if anyone's interested :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Chloé Zermatten <chloe.zermatten@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |43501 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43501 [Bug 43501] introduce Koha::Item->mark_lost -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Chloé Zermatten <chloe.zermatten@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |43508 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43508 [Bug 43508] Extract Koha::Account->add_lost_replacement_fee from chargelostitem -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Bug 39756 depends on bug 43501, which changed state. Bug 43501 Summary: introduce Koha::Item->mark_lost https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43501 What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |DUPLICATE -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Chloé Zermatten <chloe.zermatten@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Large patch Sponsorship status|--- |Sponsored -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #6 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 208162 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208162&action=edit Bug 39756: feat(staff-UI): manage long overdue rules Allow administrators to view, add, edit, reset and delete triggers containing the following new overdue circulation rules: - lost: which lost value, if any should be assigned to the item - charge: whether to charge the patron (create a fine) - forgive fine: whether to forgive the relevant fine (if any) - mark returned: whether to mark the item as returned These four new circulation rules were extracted from the current behaviour of the longoverdue script which is controlled by a mix of system preferences and flags, with the intend to replace the flags entirely so control of the script's behaviour occurs via the staff interface, and to override the system preferences. Also allows for the value of these rules to be displayed in trigger deletion and rule set reset impact previews. Test plan: 1. Apply the patch, ensure you rebuild and reload the JS code. 2. Go to Administration > Circulation triggers, and notice four new columns: Set Lost Value, Charge Replacement Cost, Mark as returned, Forgive overdue fine. 3. Add a trigger for the default context with a delay, and set: - lost value to 'Lost' - charge replacement cost to 'Yes' - mark as returned to 'No' - forgive overdue fine to 'Yes' Save, and notice the trigger shows those four values. 4. Select a specific library, and notice the values are shown as inherited from the default rule (fallback styling). 5. Edit the trigger for that library and set lost value to 'Missing'. Save, and notice the library shows 'Missing' while the default still shows 'Lost'. 6. Edit it again and set lost value to 'No lost value'. Save, and notice the library shows no lost value rather than 'Lost'. 7. Reset the library's rule set, and notice the impact preview lists the four rules among the values being removed. Confirm, and notice the library falls back to the default's values again. 8. Click delete on the default trigger, and notice the deletion preview lists the four values. Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ Assisted-by: Claude Opus 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #8 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 208164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208164&action=edit Bug 39756: feat(old-scripts): script deprecation warnings Test plan: 1. Run longoverdue.pl and overdue_notices.pl. 2. In both cases, notice the deprecation warning that shows. Note: for overdue_notices.pl, use the --test flag. longoverdue defaults to test mode and is safe to run without flags. Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ Assisted-by: Claude Opus 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #10 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 208166 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208166&action=edit Bug 39756: refactor: extract to plain js module Split the js logic used by the circulation triggers UI between a store and a plain js module, based on whether methods do or do not affect state. The test plan here is the same as for "Bug 39756: feat(staff-UI): manage long overdue rules", because this is a pure refactore with no functionality change. The intent is code maintainability. Test plan: 1. Apply the patch, ensure you rebuild and reload the JS code. 2. Go to Administration > Circulation triggers, and notice four new columns: Set Lost Value, Charge Replacement Cost, Mark as returned, Forgive overdue fine. 3. Add a trigger for the default context with a delay, and set: - lost value to 'Lost' - charge replacement cost to 'Yes' - mark as returned to 'No' - forgive overdue fine to 'Yes' Save, and notice the trigger shows those four values. 4. Select a specific library, and notice the values are shown as inherited from the default rule (fallback styling). 5. Edit the trigger for that library and set lost value to 'Missing'. Save, and notice the library shows 'Missing' while the default still shows 'Lost'. 6. Edit it again and set lost value to 'No lost value'. Save, and notice the library shows no lost value rather than 'Lost'. 7. Reset the library's rule set, and notice the impact preview lists the four rules among the values being removed. Confirm, and notice the library falls back to the default's values again. 8. Click delete on the default trigger, and notice the deletion preview lists the four values. Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ Assisted-by: Claude Opus 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #12 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 208168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208168&action=edit Bug 39756: feat: route actions to processing queue Part one of the process_circulation_triggers.pl script, this retrieves overdue loans that are relevant to a given script run, retrieves relevant overdue circulation rules, and enqueue the actions to be taken when the processing starts. Nothing is enacted in this patch: we are only testing that the script is detecting the right overdue loans, and retrieving the correct rules. Test plan: Preparation: pick a patron P, an item I (note its home library, itemtype), and check I out to P. Move the due date so that it is overdue by 7 days. For each scenario below, run the script with: perl misc/cronjobs/process_circulation_triggers.pl --debug Note: for simplicity, this assumes a test instance with no pre-existing overdues. 1. Test the default scenario: a. Ensure OverdueTriggersCalendar is off b. Check that CircControl is set to PatronLibrary c. Create a circulation trigger for P's library and category, and I's itemtype and set: - delay to 7 - lost value to LOST, - restrict to 'yes' d. run the process_circulation_triggers script. e. notice the following results: Matched: 1; action_batch_queue has one entry, delay 7, actions { lost, restrict } f. Update your loan to set the due date 6 days ago g. run the script once again. i. Notice the following result: Matched: 0, no dump 2. Test a missed run (--date): a. Update your loan to set the due date 10 days ago b. Run the script. c. Notice the following result: Matched: 0 d. Run the script again with --date set to 3 days ago (YYYY-MM-DD) e. Notice the following results: Trigger date: <the date you passed> Matched: 1; action_batch_queue has one entry, delay 7, actions { lost, restrict } 3. Test the calendar-adjusted delay: a. Turn OverdueTriggersCalendar on b. Set useDaysMode to 'Calendar' c. In Tools > Calendar, close P's library on each of the last 3 days (yesterday, 2 days ago, 3 days ago) d. Keep the due date at 10 days ago e. Run the script. f. Notice the following results: Matched: 1; action_batch_queue has one entry, delay 10, actions { lost, restrict } (7 open days back from today lands 10 calendar day) g. Turn OverdueTriggersCalendar off, run the script again h. Notice the following result: Matched: 0, action_batch_queue empty 4. Test a library closed on the run date: a. Turn OverdueTriggersCalendar on b. Update your loan to set the due date 7 days ago c. In Tools > Calendar, close P's library today d. Run the script. e. Notice the following result: Matched: 0, no dump 5. Test itemtype resolution under item-level_itypes: a. Set item-level_itypes to 'bibliographic record' b. Edit I's bibliographic record so its itemtype (942$c) differs from I's item type c. Create a circulation trigger specific to this new item type. Configure it with - restrict set to 'no' - delay set to 7 - lost value to 'Lost' d. Run the script. e. Notice the following result: Matched: 1, action_batch_queue contains 'lost' and crucially, restrict => 0. This shows the correct rule set was applied - the one matching the biblio item type. (Otherwise we would have seen restrict => 1.) 6. Test rule resolution under CircControl: a. Reset item-level_itypes to 'specific item' b. Ensure I's home library is not P's library c. Set CircControl to 'the library the item is from', and HomeOrHoldingBranch to 'the item's home library (homebranch)'. d. Run the script e. Notice the following result: Matched: 1, action_batch_queue empty (the trigger is on P's library; rules now resolve on I's home library) f. Create a circulation trigger for I's home library, P's category, and I's itemtype, and set: - delay to 7 - lost value to LOST g. Run the script. h. Notice the following result: action_batch_queue has one entry, delay 7, actions { lost } i. Set CircControl back to 'the library the patron is from' j. Run the script k. Notice the following result: action_batch_queue has one entry, delay 7, actions { lost, restrict } 7. Run the script with --dry-run --verbose a. Notice the DRY RUN header; nothing else differs from a normal run just yet. Also run relevant test suite, including: prove t/db_dependent/Koha/Overdues/ prove t/db_dependent/Koha/Library/Calendar.t prove t/db_dependent/Koha/CirculationRules.t Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ Assisted-by: Claude Opus 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #14 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 208170 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208170&action=edit Bug 39756: feat: enact notices - prepare and add to message queue Part three of the process_circulation_triggers.pl script. This processes the notice queue built in part two: for each patron it establishes once which transports can reach them, renders one letter per notice, transport and delay listing every item in it, and queues that letter in message_queue for SendQueuedMessages to send. Email and SMS notices for a patron who cannot receive them fall back to print, and only one print letter is produced per delay, in keep with overdue_notices.pl's behaviour. A notice already queued for the patron on the trigger date is not queued again, so a re-run on the same day is safe. With --date - the missed run recovery option - notices are dated to the day being replayed. Because we still rely on process_message_queue.pl to send out notices, you will need to wait for that script to run if you would like to see the notices being actually sent. Nothing is enacted in this patch: actions are routed but not yet carried out. Test plan: Preparation: pick a patron P with an email address, an item I, and check I out to P. Move the due date so that it is overdue by 7 days. Create a circulation trigger for P's library and category, and I's itemtype, and set: - delay to 7 - notice to ODUE - transport to email Run every step with: perl misc/cronjobs/process_circulation_triggers.pl --verbose Steps 1 and 2 add --dry-run: a notice queued today blocks the same notice for the rest of the day, so nothing is kept until step 3. Note: for simplicity, this assumes a test instance with no pre-existing overdues. 1. Test notices are added to the message queue: a. Check out an item I to a patron P1 with an email address, and move the due date so that it is overdue by 7 days b. Run the script. c. In the script output, under NOTICES ENQUEUED, notice one line: letter_code=ODUE mtt=email borrower=<P1> days_overdue=7 items=1 d. In the summary, notice: notices 1 queued, 0 suppressed e. On P1's record, Notices tab, notice one pending ODUE email listing I 2. Test a same-day re-run (expected outcome: no duplicates): a. Run the script again. b. In the script output, notice no line under NOTICES ENQUEUED c. In the summary, notice: notices 0 queued, 1 suppressed d. On P1's record, Notices tab, notice there is still only one ODUE email 3. Test one letter per notice, transport and delay: a. Create a second trigger in the same context: delay notice ODUE, transport email b. Check out three items to a patron P2 with an email I2 and I3 due 7 days ago, I4 due 14 days ago c. Run the script. d. In the script output, under NOTICES ENQUEUED, notice two lines for P2: letter_code=ODUE mtt=email borrower=<P2> days_overdue=7 items=2 letter_code=ODUE mtt=email borrower=<P2> days_over e. On P2's record, Notices tab, notice two pending ODUE emails: one listing I2 and I3, one listing I4 4. Test transport fallback: a. Edit trigger 1 so transport is both email and sms b. Check out an item I5 to a patron P3 with no email address and no SMS number, due 7 days ago c. Run the script. d. In the script output, under NOTICES ENQUEUED, notice a single line for P3: letter_code=ODUE mtt=print borrower=<P3> days_overdue=7 items=1 Email and sms both fall back to print, and only one print letter is produced per delay. (Print lines for P1 also appear: they have no SMS number either.) e. On P3's record, Notices tab, notice one pending ODUE print notice listing I5 f. Set trigger 1's transport back to email 5. Test a missed run (--date): a. Check out an item I6 to a patron P4 with an email address, due 10 days ago b. Run the script with --date set to 3 days ago (YYYY-MM-DD) c. In the script output, under NOTICES ENQUEUED, notice one line: letter_code=ODUE mtt=email borrower=<P4> days_over d. In the summary, notice: Trigger date: <the date you passed> notices 1 queued, 0 suppressed e. On P4's record, Notices tab, notice one pending ODUE email listing I6, dated 3 days ago f. Run the same command again g. In the summary, notice: notices 0 queued, 1 suppressed 6. Run the relevant test suites: prove t/db_dependent/Koha/Overdues/ Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ Assisted-by: Claude Opus 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Chloé Zermatten <chloe.zermatten@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #16 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- Created attachment 208187 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208187&action=edit Bug 39756: feat: enact actions Part four of the process_circulation_triggers.pl script. This carries out the actions routed in part one, for each overdue loan, in a fixed order when more than one exist for a given delay, for a given context. The order is specified as restrict, forgive the overdue fine, set lost, charge the replacement cost, mark returned. Actions run after notices are queued, so a letter on a trigger that also acts describes the loan as it stood before the action: the fine still outstanding, the item not yet lost. Lost is set through a new Koha::Item->mark_lost: it sets the item's lost status, moves the loan's accruing overdue fine to LOST (deleting it if it is a zero-amount line with no credits), and cancels any outstanding transfers. Since this is only useful to this script at this time, it is submitted as part of 39756 rather than as its own bug (not testable if extracted). Once a patron's loans have been marked returned, their OVERDUES restriction is lifted where AutoRemoveOverduesRestrictions allows, once per patron. Re-runs are safe: a patron holds a single OVERDUES restriction, and an item is charged its replacement cost once per loan. With --dry-run everything is rolled back, and no search index update or holds queue job is triggered. Test plan: Preparation: ensure OverdueTriggersCalendar is off and C is set to PatronLibrary. Every trigger below is created for the patrons' library and category, and the items' itemtype. Each step uses its own patron and its own delay, so steps do not interfere. Run every step with: perl misc/cronjobs/process_circulation_triggers.pl --verbose Results are read in three places: - the script output, under ACTION ENACTED: one line per action - the summary at the end of the script output - the staff interface, on the patron's and item's records Each run acts on every matching loan, so lines for patrons from earlier steps may also appear: look for the step's own patron. Note: for simplicity, this assumes a test instance with pre-existing overdues. 1. Test restrict: a. Create a default trigger: delay 7, restrict 'yes' b. Check out an item I1 to a patron P1, due 7 days ag c. Run the script d. In the script output, under ACTION ENACTED, notice: type=restrict value=1 borrower=<P1> item=<I1> days e. In the summary, notice: restricted 1 patrons f. On P1's record, notice one OVERDUES restriction g. Run the script again, and notice P1 still has a single OVERDUES restriction 2. Test lost and charge: a. Create a default trigger: delay 14, lost value 'Lost', charge 'yes' b. Give an item I2 a replacement price of 5.00, check it out to a patron P2, due 14 days ago c. Run the script d. In the script output, under ACTION ENACTED, notice, in this order: type=lost value=1 borrower=<P2> item=<I2> days_ove type=charge value=1 borrower=<P2> item=<I2> days_overdue=14 e. In the summary, notice: lost 1 items charged 1 items f. On I2's record, notice it is lost; on P2's record, Accounting tab, notice one lost item fee of 5.00 g. Run the script again, and notice P2 still has a single lost item fee, and the summary has no 'charged' line 3. Test forgiving the overdue fine: a. Make sure the circulation rules for this context charge fines (fine amount and fine charging interval set) b. Create a trigger: delay 21, forgive fine 'yes' c. Check out an item I3 to a patron P3, due 21 days ago d. Run misc/cronjobs/fines.pl, and notice an accruing overdue fine on P3's record, Accounting tab e. Run the script f. In the script output, under ACTION ENACTED, notice: type=forgive_fine value=1 borrower=<P3> item=<I3> days_overdue=21 g. In the summary, notice: forgiven 1 fines h. On P3's record, Accounting tab, notice the overdue fine is forgiven and nothing is outstanding 4. Test mark returned and restriction lifting: a. Set AutoRemoveOverduesRestrictions to 'if patron has no remaining overdue items' b. Create a trigger: delay 28, mark returned 'yes' c. Check out an item I4 to a patron P4, due 7 days ago d. Run the script, and notice P4 now has an OVERDUES restriction (from the trigger in step 1) e. Update the loan to set the due date 28 days ago f. Run the script g. In the script output, under ACTION ENACTED, notice: type=mark_returned value=1 borrower=<P4> item=<I4> h. In the summary, notice: returned 1 items lifted 1 restrictions i. On P4's record, notice I4 is no longer checked out, and the OVERDUES restriction is gone j. Set AutoRemoveOverduesRestrictions back to "don't" 5. Test notices are processed before actions: a. Edit the delay 14 trigger to also set notice ODUE, transport email b. Give an item I5 a replacement price, check it out to a patron P5 with an email address, due 14 days ago c. Run the script d. In the script output, notice the NOTICES ENQUEUED section, with a line for P5, comes before ACTION ENACTED e. On P5's record, Notices tab, notice one pending ODUE email listing I5 6. Test a dry run: a. Give an item I6 a replacement price, check it out P6, due 14 days ago b. Run the script with --dry-run c. In the script output, notice the DRY RUN header, and lost and charge lines for P6 under ACTION ENACTED d. On I6's record, notice it is not lost; on P6's record, Accounting tab, notice no lost item fee 7. Run the relevant test suites: prove t/db_dependent/Koha/Overdues/ prove t/db_dependent/Koha/Item.t Sponsored-by: Black Hills Library Consortium- http://www.rcgov.org/Library/ Assisted-by: Claude Opus 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #18 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- (In reply to Philip Orr from comment #17)

Hi Chloé, I'd like to test this one but I'm not sure about the best procedure to test it. It depends on 43500, 43504 and 43508 - would it be better to test those first and then come back to this one?

I tried applying this bug (that is, 39756) and failed at 43508:

``` 📋 Bug 43508 - Extract Koha::Account->add_lost_replacement_fee from chargelostitem

• 207940 - Bug 43508: refactor(Account): extract add_lost_replacement_fee • 207941 - Bug 43508: test(Account): add_lost_replacement_fee • 207942 - Bug 43508: (follow-up) Remove non-ASCII character from add_lost_replacement_fee POD

Apply? [(y)es, (n)o, (i)nteractive] y

Preparing 3 patch(es): ✓ Prepared 3 patch(es)

Applying 3 patch(es):

Patch application failed for attachment 207940 [details] [review] - Bug 43508: refactor(Account): extract add_lost_replacement_fee

Patches left in /tmp/XCidSfC3J1 for manual application if needed

To resolve: 1. Fix conflicts (use 'git mergetool' or edit files manually) 2. Stage resolved files with 'git add' 3. Continue with 'git bz apply --continue' or 'git am --continue' 4. Or skip this patch with 'git bz apply --skip' 5. Or abort with 'git bz apply --abort'

Error: Apply failed: Cannot apply cleanly patches from bug 43508. Everything will be left dirty. Git command (git am -3 /tmp/XCidSfC3J1/0001-207940.patch) failed: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch=diff' to see the failed patch hint: When you have resolved this problem, run "git am --continue". hint: If you prefer to skip this patch, run "git am --skip" instead. hint: To restore the original branch and stop patching, run "git am --abort". hint: Disable this message with "git config advice.mergeConflict false" ´´´

Hi Philip, Thanks for this, much appreciated :) Thinking about it, the issue here is probably that 43508 conflicts with 43504. Both introduce a new method in a similar location in Account.pm. And both add a test to Account.t, so that's likely complaining too. One solution could be that I make 43504 depend on 43508, so that it includes that merge conflict resolution. (Rather than keep 43504 compatible with current main). The reason I would do it that way is 43508 already has one sign off, so I reckon it might be slightly better if 43504 is the one I rebase. I wasn't sure this was a great idea seeing as there's no functional dependency between the two. But it would make testing 39756 itself much easier. If you're happy with it, I can go ahead and sort 43504 now. So as for how to proceed in general, I think the sequential, bug by bug approach is our best bet here. Thanks again! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #20 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- I've gone through all the test plans except Comment 16, which I should be able to get to tomorrow. Issues so far: On the triggers page, last line in the blue box reads "The system is currently set to match based on the cron" but rule selection is based on the CircControl syspref. As of 26.05, the longoverdue cronjob follows LostChargesControl rather than CircControl (Bug 39802 - Add CircControl equivalent system preference for lost item fees and actions). But with this patch set all circulation triggers are evaluated based on CircControl. Seems like triggers should be evaluated based on CircControl if they do not set a lost value or charge a replacement cost, but evaluated based on LostChargesControl if they do either of these things. ​ If you reload the page while editing a trigger, the page shows a modal with "Loading context" and "Loading rule set information" but never loads Comment 11, step 9: When saving the library-specific trigger with No Notice, the transport types disappeared from the edit modal. But after saving the table of existing rules still shows "Yes" for email. Comment 12: Test failure prove t/db_dependent/Koha/Overdues/ t/db_dependent/Koha/Overdues/ActionExecutor.t .... 1/26 # Failed test 'a charge blocked by the duplicate guard is not counted' # at t/db_dependent/Koha/Overdues/ActionExecutor.t line 323. # got: '2' # expected: '1' t/db_dependent/Koha/Overdues/ActionExecutor.t .... 6/26 # Looks like you failed 1 test of 6. # Failed test 'enact_charge creates LOST debit with caller-resolved branch' # at t/db_dependent/Koha/Overdues/ActionExecutor.t line 326. t/db_dependent/Koha/Overdues/ActionExecutor.t .... 25/26 # Looks like you failed 1 test of 26. t/db_dependent/Koha/Overdues/ActionExecutor.t .... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/26 subtests t/db_dependent/Koha/Overdues/Repository.t ........ ok t/db_dependent/Koha/Overdues/RuleResolver.t ...... ok t/db_dependent/Koha/Overdues/TriggerProcessor.t .. ok Test Summary Report ------------------- t/db_dependent/Koha/Overdues/ActionExecutor.t (Wstat: 256 (exited 1) Tests: 26 Failed: 1) Failed test: 6 Non-zero exit status: 1 Files=4, Tests=51, 16 wallclock secs ( 0.02 usr 0.00 sys + 12.49 cusr 2.63 csys = 15.14 CPU) Result: FAIL Comment 14: In 26.05, if the triggers say the patron should get an email but they have no email address and therefore get a print notice instead, Koha will use the email template if no print template exists. With these patches in that situation Koha does not use the email template and instead makes no notice. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #22 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- (In reply to Chloé Zermatten from comment #16)

Created attachment 208187 [details] [review] Bug 39756: feat: enact actions

Actions run after notices are queued, so a letter on a trigger that also acts describes the loan as it stood before the action: the fine still outstanding, the item not yet lost.

Ideally lost item charges and processing fees would be applied *before* the notice is generated, so that the notice can include details of those fees. In my testing I was inconsistently able to get replacement costs and processing fees to show in a notice, but I've been unable to recreate that. In retrospect, I suspect the instances where I *did* get that data in the notice must have been cases where I ran the script more than once for a given checkout. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=43705 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #25 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- (In reply to Andrew Fuerste-Henry from comment #22)

(In reply to Chloé Zermatten from comment #16)

...

Created attachment 208187 [details] [review] [review] Bug 39756: feat: enact actions

Actions run after notices are queued, so a letter on a trigger that also acts describes the loan as it stood before the action: the fine still outstanding, the item not yet lost.

Ideally lost item charges and processing fees would be applied *before* the notice is generated, so that the notice can include details of those fees.

In my testing I was inconsistently able to get replacement costs and processing fees to show in a notice, but I've been unable to recreate that. In retrospect, I suspect the instances where I *did* get that data in the notice must have been cases where I ran the script more than once for a given checkout.

That makes sense. I'd actually originally written this as a 'process the action queue first, and the notice queue second'. Off the top of my head - and I'll go do some further digging tomorrow, I think the trade-off, and the reason behind the swap, is that if you tick mark returned, then populating a notice becomes difficult, as the data it wants isn't where we expect it to be anymore: for one, the issues row would be archived and deleted. Hence the swap to draining the notice queue, preparing the notices and enqueuing those messages before moving on to action enactment. What might make sense is this: $action_executor->process_action_queue; $action_executor->process_notice_queue; $action_executor->process_return_queue; Assuming of course that the only use case where 'notice after action' is problematic is the one where mark_returned ran. It is the only one I can think of at the minute. I'll go ahead and submit that change tomorrow so it can be tried out. Happy to revert it if it turns out I was wrong! Equally happy to look into processing all actions first if we want to test that - it is a two line change. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Comma delimited| |Rapid City Public Library list of Sponsors| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=39631 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.