Bug 29116

Summary: request.pl re-invents Koha::Patron::is_expired accessor
Product: Koha Reporter: Joonas Kylmälä <joonas.kylmala>
Component: Hold requestsAssignee: Joonas Kylmälä <joonas.kylmala>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jonathan.druart, kyle, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29115
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Attachments: Bug 29116: Use a pre-existing accessor instead of reimplementing it
Bug 29116: Use a pre-existing accessor instead of reimplementing it
Bug 29116: Use a pre-existing accessor instead of reimplementing it

Description Joonas Kylmälä 2021-09-26 16:09:07 UTC
reserve/request.pl duplicates the same code as what Koha::Patron::is_expired does, the code should be deleted.
Comment 1 Joonas Kylmälä 2021-09-26 16:15:57 UTC
Created attachment 125341 [details] [review]
Bug 29116: Use a pre-existing accessor instead of reimplementing it

We can use the Koha::Patron::is_expired accessor instead of re-writing
it in request.pl.

To test:
 1) Make that a warning is being shown with and without this patch
    when placing a hold to an expired patron account
Comment 2 Fridolin Somers 2021-09-28 07:22:11 UTC
Created attachment 125391 [details] [review]
Bug 29116: Use a pre-existing accessor instead of reimplementing it

We can use the Koha::Patron::is_expired accessor instead of re-writing
it in request.pl.

To test:
 1) Make that a warning is being shown with and without this patch
    when placing a hold to an expired patron account

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 3 Fridolin Somers 2021-09-28 08:08:44 UTC
Always better not to re-invent the wheel :D
Comment 4 Marcel de Rooy 2021-10-07 06:54:10 UTC
Created attachment 125860 [details] [review]
Bug 29116: Use a pre-existing accessor instead of reimplementing it

We can use the Koha::Patron::is_expired accessor instead of re-writing
it in request.pl.

To test:
 1) Make that a warning is being shown with and without this patch
    when placing a hold to an expired patron account

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Jonathan Druart 2021-10-07 09:06:44 UTC
Same file:

266         my $expiry_date = $enrollment->patron->dateexpiry;
267         $member->{expiry} = 0; # flag set if patron account has expired
268         if ($expiry_date and
269             Date_to_Days(split /-/,$date) > Date_to_Days(split /-/,$expiry_date)) {
270             $member->{expiry} = 1;
271         }

Can we fix that one as well please?
Comment 6 Joonas Kylmälä 2021-10-10 08:48:57 UTC
(In reply to Jonathan Druart from comment #5)
> Same file:
> 
> 266         my $expiry_date = $enrollment->patron->dateexpiry;
> 267         $member->{expiry} = 0; # flag set if patron account has expired
> 268         if ($expiry_date and
> 269             Date_to_Days(split /-/,$date) > Date_to_Days(split
> /-/,$expiry_date)) {
> 270             $member->{expiry} = 1;
> 271         }
> 
> Can we fix that one as well please?

There's actually more to it, I had opened earlier early a separate bug report for that with a patch: bug 29115. I'm putting this back to PQA.
Comment 7 Jonathan Druart 2021-10-12 09:18:59 UTC
Pushed to master for 21.11, thanks to everybody involved!