Long Overdue processing needs org unit settings separate from Lost Processing

To be clear, there are many settings that apply exclusively to long-overdue processing. There are 9 of them ,in fact, but it sounds like we're missing one important one:

The request as I understand it is that we need a setting which determines whether the cost of the item should be billed to the user when a circulation is marked as long-overdue? Ditto lost. I can confirm this is not configurable at the moment. Is this the sum of it?

---

For reference, OpenILS::Application::Cat::AssetCommon::set_item_long_overdue / set_item_lost

Yes, a setting which determines whether the item cost should be billed to the user when the circulation is marked "Long Overdue" is what's needed.

Adding this setting leaves open the option for a library to automatically mark "Lost" and charge the patron the cost of the item at a later time.

If a similar setting providing the option to charge the item cost when an item is automatically marked Lost were introduced, it should be separate from the behavior of the "Mark Lost by Patron" functionality. If "Mark Lost" and "Mark Lost by Patron" follow the same option to charge the item cost, a library could potentially set their options to make it impossible to charge the user the cost of the item.

Added a library setting "Assess Billing When Marked Long-Overdue" Added a few lines of code to respect that setting and skip some billing matters under the condition. Tested with and without setting. Lost still functions the same.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=b7b65232ebd6cbc6f06bfca0ad237b767586d4a6

I made some small spelling adjustments and sql file conformity:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=6201ec073ebafd738340b7c7d42634ef5b9f5789

I have signed off on this commit. The Long-Overdue is charged when the Assess Billing When Marked Long-Over due is true, and the charge is not made when it is false:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/ldw/LP1331174

I have been working on some of the other long overdue bugs. I realized that when the longoverdue action trigger fires, it needs to update the stop_fines column so that the penalties can be applied to the patron. If you set "Assess Billing When Marked Long-Overdue" , it will skip the billing section of the code which includes marking the circulation as "LONGOVERDUE". The original intent would let the fine generator continue to apply overdues even after it's been LONGOVERDUE. So, not updating that column was a good thing. It has the converse affect of not giving the patron the penalty which is one of the main goals.

So, I am going to re-work this one. My current thinking is:

In addition to this library setting, there needs to be another one. Perhaps:
"Continue to generate overdue fines after Long-Overdue"

The idea would be to go ahead and set the stop_fines column for the circulation to "LONGOVERDUE" which will apply penalties and put the circulations in the appropriate places in the UI's. But it also stops the fine generator. If you want the circulation to continue to accumulate overdue fines, then there needs to be another setting.

Any thoughts?

Here is the updated branch with the extra library setting
"Continue Overdue Billing When Marked Long-Overdue"

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=91a506cfb1d608171536dcaebcc8b13430ce3e73

Forgot the extra if statement to be sure and mark the circ LONGOVERDUE during action_trigger.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=e86d9f4cb6c974325fc7bd8151ee870ca68ec9bc

All of those commits were getting confusing when using the git browser. Compiled them into a single.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=f05da3ba2adadac1c136866f85805516643ea488

The OPAC will make Long overdue circulations invisible for patrons where the library elects to not have fines and also not bill on long overdue. This branch includes the fix for that

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=7088aa7bffa769e2fba3767fa640d88727c15fe3

Adding a note that this feature will need a release notes entry.

As Michele originally said apart of the bug report:

"indicate that billing the patron as part of the automatic long overdue processing is optional, controlled by Org Unit Settings. But in reality the documentation is wrong."

The new code introduces the settings that were missing. Agreed, the docs could use some TLC. The way the docs read, lead you to believe that these settings exist.

Docs updated:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/blake/LP1331174_Long_Overdue_processing_needs_org_unit_settings

I loaded this code on to a VM for Bug Squashing Day but ran across some trouble when trying to get it set up for testing. After loading the code, I no longer get the seed data for the action/trigger for marking a transaction as long overdue. This action/trigger is available if I load the VM without the code.

I planned to dig into it further to see if I could find out what was happening, but I haven't had time to look at it further. I'm posting my results here to see if anyone else can confirm this problem.

Blake,

I think I discovered what the problem is. On line 13755 of the seed data, I think you need an ending parentheses. Instead of:

oils_i18n_gettext(
         'circ.longoverdue.assess_billing',
         'When an item is marked long overdue a bill for the item will be ' ||
                 'assesed unless this setting is false. The default is true. If false ' ||
                 'void overdue setting will be ignored and processing fee will be ignored',
         'coust',
         'description'
     ), (

It would be:

oils_i18n_gettext(
         'circ.longoverdue.assess_billing',
         'When an item is marked long overdue a bill for the item will be ' ||
                 'assesed unless this setting is false. The default is true. If false ' ||
                 'void overdue setting will be ignored and processing fee will be ignored',
         'coust',
         'description'
    )
), (

I was going to update it myself, but the branch has some conflicts that I didn't feel comfortable resolving.

If you push out a new branch with the change and the conflicts resolved, I can load it on a test VM. I would love to see this code make it into 2.9!

Kathy

Kathy,

Thank you for looking at this. I setup the working repo with the changes. I also squashed it all into one commit.

Kathy,

Thanks for the fix on the seed file. I added your fix and altered Actor.pm in order to keep the long overdue circs on the OPAC for the patrons. And squashed them all into a single commit. I mentioned it here:

https://bugs.launchpad.net/evergreen/+bug/1440148

That bug was merged into master, and therefore, needs to be reversed for this LOD patch. It will make the circulation show in the patron OPAC circulation counts in various places.

Thanks Blake!

I've loaded the code on a test VM and can confirm that we no longer see the problem with the missing action trigger. We have somebody that will be testing this code. In the meantime, I'm adding a needstest tag. I asked in IRC, and gmcharlt indicated a unit test would be helpful here.

This is looking very promising, but it needs a few more tweaks.

My testing scenario was as follows:

Library settings:

Assess Billing When Marked Long-Overdue - FALSE
Void overdue fines when items are marked lost - TRUE
Use Lost and Paid copy status - TRUE
Void lost item billing when returned - TRUE
Long-overdue items are usable on checkin instead of going "home" first - TRUE
Restore overdues on Lost item return - TRUE
Void overdue fines when items are marked Lost - TRUE
Items Out Long-Overdue display setting - 5
Items Out Lost display setting - set to 5
Include Lost circulations in lump sum tallies in Patron Display - TRUE
Continue Overdue Billing When Marked Long-Overdue - TRUE

Used the following action triggers:

Mark a circulation Long Overdue after a 6 month interval
Mark a circulation Lost after a 7 month interval

Circulations were marked Long Overdue as expected by the Mark Long-Overdue action trigger.

However, the Mark Lost action trigger will not act on Long Overdue circulations without a change to action_trigger_filter.json, as discussed in IRC here:

http://irc.evergreen-ils.org/evergreen/2015-12-15#i_220027

So the addition of a longoverdue-to-lost filter and matching action trigger and cron entry would make this more usable out of the box by enabling the trigger and cron entry.

I'm changing this to unassigned in case someone else wants to pick it up, but I'm willing to take a stab at the tweaks if there are no takers.

Thanks for all your work on this, Blake!

Since it's getting close to the beta deadline, and I don't know that anyone will be able to work on those tweaks between now and Friday, do you all think this code is mergeable without the tweaks?

Kathy,

I would be happy to put a sql insert in the code for a "template" Action Trigger - but for reasons spelled out in the IRC discussion, I hesitate to edit the default action_trigger_filter.json file.

AKA { "stop_fines" : ["MAXFINES"] } needs to be { "stop_fines" : ["MAXFINES","LONGOVERDUE"] }. Someone implementing this, would/could have a separate action_trigger_filter.json config file for the cron jobs that intend to make items lost after they have already gone to longoverdue.
Cron example:
0 4 * * * . /etc/profile && /openils/bin/action_trigger_runner.pl --osrf-config /openils/conf/opensrf_core.xml --process-hooks --run-pending --custom-filters /openils/conf/action_trigger_filters_longoverdue_to_lost.json

Perhaps the documentations could bridge the gap?

Assuming code associated with bug 1378829 is merged before this code, the latest branch on this bug will contain a redundant change to seed data, and the following line should be removed from the upgrade script (even though it shouldn't actually cause harm if run after the upgrade script associated with bug 1378829):

UPDATE PERMISSION.PERM_LIST SET CODE='COPY_STATUS_LONG_OVERDUE.override' WHERE CODE='COPY_STATUS_LONGOVERDUE.override';

Just rebased with master and force pushed. I removed the UPDATE clause in the SQL upgrade script.

The code merged from bug 1661754 makes this patch inert. The line in AssetCommon.pm

if ($copy->status == $args{status} || $copy->status == $args{alt_status});

I am trying to think of a way to have it both ways. Perhaps another library setting? A situation where we want the action trigger to do it but not the staff? Perhaps adding something in the args array to teach set_item_lost_or_lod that the calling code is either staff or AT?

All,

I just pushed a fix to this branch. It's mostly the same but I added some new things:

Added a new example file for action_trigger_filters (action_trigger_filters_lod_to_lost.json.example)
Added a new library setting circ.longoverdue.to_lost.

Filled out the supporting documentation and release notes.

This patch now merges on current master cleanly.