Bug 37034

Summary: Add cronjob to alert patron with bookings ended and not been converted into a loan
Product: Koha Reporter: Thibaud Guillot (thibaud_g) <thibaud.guillot>
Component: ToolsAssignee: Thibaud Guillot (thibaud_g) <thibaud.guillot>
Status: Failed QA --- QA Contact: Victor Grousset/tuxayo <victor>
Severity: enhancement    
Priority: P5 - low CC: david, marie.hedbom, victor
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function: bookings
Bug Depends on: 36915    
Bug Blocks:    
Attachments: Bug 37034: Add script to alerts patron when booking ended a day before
Bug 37034: Add script to alerts patron when booking ended a day before
Bug 37034: Add script to alerts patron when booking ended a day before

Description Thibaud Guillot (thibaud_g) 2024-06-05 09:46:14 UTC
Hello,

It could be useful to alert patron with some bookings ended a day before but not been converted into a loan. Based on notice template.
Comment 1 Thibaud Guillot (thibaud_g) 2024-06-05 09:55:19 UTC
Created attachment 167434 [details] [review]
Bug 37034: Add script to alerts patron when booking ended a day before

Test plan:

1) Write a notice template to use datas from this script.
For example :
"
Hello <<borrowers.firstname>>, <<borrowers.surname>>

Some bookings ended yesterday:
[% FOREACH booking IN bookings %]
  Booking number [% booking.booking_id %] linked to [% booking.biblio.title %] which was to be
  taken from the library [% booking.pickup_library.branchname %], ended [% booking.end_date %].
[% END %]
"

2) This template can have custom lettercode but don't forget to use it
   when you launch it. By default (if no lettercode is send to script,
'BOOKINGS_OUTDATED' code will be used)

3) Launch 'perl misc/cronjobs/bookings/bookings_outdated.pl' with args
   or not
4) Go on message_queue and you can normally see your message.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 2 Thibaud Guillot (thibaud_g) 2024-06-05 10:14:46 UTC
Created attachment 167435 [details] [review]
Bug 37034: Add script to alerts patron when booking ended a day before

Test plan:

1) Write a notice template to use datas from this script.
For example :
"
Hello <<borrowers.firstname>>, <<borrowers.surname>>

Some bookings ended yesterday:
[% FOREACH booking IN bookings %]
  Booking number [% booking.booking_id %] linked to [% booking.biblio.title %] which was to be
  taken from the library [% booking.pickup_library.branchname %], ended [% booking.end_date %].
[% END %]
"

2) This template can have custom lettercode but don't forget to use it
   when you launch it. By default (if no lettercode is send to script,
'BOOKINGS_OUTDATED' code will be used)

3) Launch 'perl misc/cronjobs/bookings/bookings_outdated.pl' with args
   or not
4) Go on message_queue and you can normally see your message.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 3 David Nind 2024-07-05 01:10:09 UTC
Created attachment 168523 [details] [review]
Bug 37034: Add script to alerts patron when booking ended a day before

Test plan:

1) Write a notice template to use datas from this script.
For example :
"
Hello <<borrowers.firstname>>, <<borrowers.surname>>

Some bookings ended yesterday:
[% FOREACH booking IN bookings %]
  Booking number [% booking.booking_id %] linked to [% booking.biblio.title %] which was to be
  taken from the library [% booking.pickup_library.branchname %], ended [% booking.end_date %].
[% END %]
"

2) This template can have custom lettercode but don't forget to use it
   when you launch it. By default (if no lettercode is send to script,
'BOOKINGS_OUTDATED' code will be used)

3) Launch 'perl misc/cronjobs/bookings/bookings_outdated.pl' with args
   or not
4) Go on message_queue and you can normally see your message.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-07-05 01:11:36 UTC
I've tested, it works, and I've signed it off.

Some suggestions:
1. Add a sample notice similar to Bug 36915
2. Rename to booking_expired.pl and BOOKING_EXPIRED (that makes more sense to me in English)

I'll leave it up to you and the QA Team to decide.

Testing notes (using KTD):

1. Add a sample booking notice.
2. Apply the patch.
3. Add an email address to the patron you will make the booking for (for example, Mary Burton, mary@example.com)
4. Add a booking:
   . Find a record
   . Go to Items > Priority
   . Change "Bookable" to "Yes" then click "Update"
   . Use the "Place booking" action for the record
   . Enter the details to create a booking (using the patron you added an email address to)
5. Update the booking in the database so that it is expired:
   . koha-mysql kohadev
   . select * from bookings;
   . using the booking_id, update the start and end dates for the booking:
     update bookings set start_date = "2024-07-03 12:00:00" where booking_id="XXX"; (where XXX is the booking_id)
     update bookings set end_date = "2024-07-04 12:00:00" where booking_id="XXX"; (where XXX is the booking_id); (update end_date to yesterday's date)
   . Check the bookings for the record - there should be no current bookings
6. Run the cron job manually:
   perl misc/cronjobs/bookings/bookings_outdated.pl -v
   1 bookings outdated between 2024-07-04 00:00:00 and 2024-07-04 23:59:59 
   borrower Burton, Mary has 1 booking(s) outdated.
7. Check that a notice was generated:
   . Go to the notices section for the patron
   OR
   . Check the message queue in the database:
     - koha-mysql kohadev
     - select * from message_queue;
Comment 5 Victor Grousset/tuxayo 2024-11-06 06:30:32 UTC
Thanks David for the testing and feedback.

(In reply to David Nind from comment #4)
> I've tested, it works, and I've signed it off.
> 
> Some suggestions:
> 1. Add a sample notice similar to Bug 36915
> 2. Rename to booking_expired.pl and BOOKING_EXPIRED (that makes more sense
> to me in English)

+1
(wait, not sure about bookings => booking)

About the sample notice how about this Thibaud?:
It just some changes to BOOKING_CANCELLATION from sample_notices.yml

        - module: bookings
          code: BOOKING_EXPIRED
          branchcode: ""
          name: "Booking expiration notice"
          is_html: 1
          title: "A library booking expired"
          message_transport_type: email
          lang: default
          content:
            - "[%- PROCESS 'html_helpers.inc' -%]"
            - "[%- USE KohaDates -%]"
            - "Dear [%- INCLUDE 'patron-title.inc' patron => booking.patron -%],<br>"
            - "<br>"
            - "We regret to inform you, that the following booking has expired.<br>"
            - "<br>"
            - "Title: [%- INCLUDE 'biblio-title.inc' biblio=booking.biblio link = 0 -%]<br>"
            - "<br>"
            - "Dates: [% booking.start_date | $KohaDates %] to [% booking.end_date | $KohaDates %]<br>"
            - "<br>"
            - "Kind regards<br>"
            - "<br>"
            - "[% booking.pickup_library.branchname %]"
Comment 6 Victor Grousset/tuxayo 2024-11-07 06:49:37 UTC
Without a notice existing, it fails with
Can't "next" outside a loop block at misc/cronjobs/bookings/bookings_outdated.pl line 130.

code:
unless ($template_exists) {
    $verbose and print qq|Message '$lettercode' content not found\n|;
    next;
}


It's indeed not in a loop. And it should output the message even if it's not verbose since it's a fatal error.

------

-lettercode param doesn't match terminology.
https://wiki.koha-community.org/wiki/Terminology#N
«Use "notice", not "letter".»

Would `-notice-code` work well? (I can spot naming issues but not always propose names being sure they have no issue :P)

-----

POD contains "bookings_overdated.pl" (5 occurrences)


--------

There is redundancy when running --help

```
Usage:
    bookings_overdated.pl -lettercode <notice to send> [ -c ][ -v ]

     Options:
       -help                          brief help message
       -v                             verbose
       -lettercode   <lettercode>     predefined notice to use, default is BOOKING_OUTDATED

Options:
    -help   Print a brief help message and exits.

    -v      Verbose. Without this flag set, only fatal errors are reported.

    -lettercode
            Optional parameter, choose a notice to use. Default is
            'BOOKING_OUTDATED'.
```

You can look at misc/cronjobs/sftp_file.pl there is something (I didn't found what exactly ^^") that makes =head1 OPTIONS not being outputted. So no duplication with the " Options:" in SYNOPSIS

-------------


What is [ -c ] ?
Is that a rest from a --confirm that came from another cron job?
Comment 7 Victor Grousset/tuxayo 2024-11-07 07:26:26 UTC
I asked David for confirmation about the naming and they meant "bookings_expired.pl" as a better sounding term. Not "booking_expired.pl"
So a preference for this.

(Though someone else confirmed me that "booking_expired.pl" would have been ok also.)
Comment 8 Victor Grousset/tuxayo 2024-11-12 22:53:15 UTC
Above is attached a follow-up to put a sample notice. Turns out a simple copy paste of BOOKING_CANCELLATION doesn't work. It needed the patron object and a loop to handle multiple items.

Here is the output:

======================
Dear ****** ****** (******),

We regret to inform you, that one or more bookings have expired.

Title: Linux kernel development /
Dates: 07/03/2024 to 11/10/2024

Title: Linux kernel development /
Dates: 07/03/2024 to 11/10/2024

Title: Linux system programming /
Dates: 07/03/2024 to 11/10/2024

Title: The Linux programming interface :
Dates: 07/03/2024 to 11/10/2024

Kind regards

Centerville
==================


-----------------------


How does one make a record level booking? To be sure it work there too.


-----------------


POD/--help should say it's only meant to be run daily because it only looks for the bookings expired the previous day.


-----------------


POD:
> This script is designed to alert patrons with bookings
> ended a day before and not been converted into a loan.

↓↓↓ would be clearer with something like:

[...]with bookings that expired on the previous day due to not being converted to a checkout.


Another thing from terminology:
https://wiki.koha-community.org/wiki/Terminology#I
loan => checkout