Long Overdue processing needs org unit settings separate from Lost Processing

Bug #1331174 reported by Michele Morgan on 2014-06-17
30
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Unassigned

Bug Description

Evergreen 2.5.5

This report is related to the Long-overdue processing functionality using Action Triggers which was introduced in Release 2.5:

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

The original tech specs for the development:

http://yeti.esilibrary.com/dev/pub/techspecs/long-overdue.html

as well as the release notes for Release 2.5:

http://evergreen-ils.org/documentation/release/RELEASE_NOTES_2_5.html

and the Evergreen Documentation:

http://docs.evergreen-ils.org/dev/_mark_an_item_long_overdue.html

all 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.

In the current functionality, there are no org unit settings that apply exclusively to automatic Long Overdue processing. The org unit settings that apply are the same as those for automatic Lost processing. We would like to use both the Long Overdue processing functionality as well as automatic Lost processing functionality.

Below is a use case that could employ the Long Overdue processing in conjunction with Lost Processing:

1. Automatic Long Overdue processing would happen at approximately six weeks after the due date. This would:

- Mark circulations as Long Overdue
- Assign associated items the status of Long Overdue.
- The cost of the item would not be applied to the patron record at that time.
- The PATRON_EXCEEDS_LONG_OVERDUE_COUNT penalty would be employed to block the patron from performing certain actions.

Marking circulations long overdue would help staff members easily distinguish between mildly overdue and severely overdue items. It would also allow blocking of some patron transactions.

2. Automatic Lost processing would happen at a much later date, for example, one year. This would:

- Mark circulations Lost
- Assign associated items the status of Lost.
- Apply the cost of the item to the patron records.
- The PATRON_EXCEEDS_LOST_COUNT penalty would be employed to block the patron from performing any transactions.

When enough time passes making it is likely that items will not be returned, the bill for the item can be applied to the patron record and the patron blocked.

Below are references to an email thread and IRC log with more background information:

http://georgialibraries.markmail.org/thread/j2ria247probmlmd

http://irc.evergreen-ils.org/evergreen/2014-06-16#i_105694

Bill Erickson (berick) wrote :

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

Michele Morgan (mmorgan) wrote :

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.

Blake GH (bmagic) wrote :

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

tags: added: pullrequest
Blake GH (bmagic) wrote :
Liam Whalen (whalen-ld) on 2015-03-02
Changed in evergreen:
assignee: nobody → Liam Whalen (whalen-ld)
Liam Whalen (whalen-ld) wrote :

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

Michele Morgan (mmorgan) on 2015-03-19
tags: added: signedoff
Kathy Lussier (klussier) on 2015-03-20
Changed in evergreen:
assignee: Liam Whalen (whalen-ld) → nobody
importance: Undecided → Wishlist
milestone: none → 2.next
status: New → Confirmed
Blake GH (bmagic) wrote :

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?

Blake GH (bmagic) wrote :

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

Blake GH (bmagic) wrote :

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

Blake GH (bmagic) wrote :

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

Blake GH (bmagic) wrote :

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

Kathy Lussier (klussier) wrote :

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

tags: added: needsreleasenote
removed: signedoff
Blake GH (bmagic) wrote :

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.

Kathy Lussier (klussier) wrote :

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.

Kathy Lussier (klussier) wrote :

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

Blake GH (bmagic) wrote :

Kathy,

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

Blake GH (bmagic) wrote :

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.

Kathy Lussier (klussier) wrote :

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.

tags: added: needstest
removed: needsreleasenote
Liam Whalen (whalen-ld) on 2015-11-17
tags: added: test-writing-day-1
Michele Morgan (mmorgan) on 2015-12-09
Changed in evergreen:
assignee: nobody → Michele Morgan (mmorgan)
Michele Morgan (mmorgan) wrote :

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!

Changed in evergreen:
assignee: Michele Morgan (mmorgan) → nobody
Kathy Lussier (klussier) wrote :

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?

Blake GH (bmagic) wrote :

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?

Jeff Godin (jgodin) wrote :

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';

Blake GH (bmagic) wrote :

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

Blake GH (bmagic) wrote :

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?

Dan Wells (dbw2) on 2018-05-01
tags: added: billing
Dan Wells (dbw2) on 2019-02-06
tags: removed: pullrequest
Blake GH (bmagic) wrote :

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.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers