Bug 32563 - Reservedate becomes expirationdate in some cases
Summary: Reservedate becomes expirationdate in some cases
Status: RESOLVED DUPLICATE of bug 33611
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-04 12:17 UTC by Slava Shishkin
Modified: 2023-05-05 11:25 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 32563: HOTFIX: reservedate becomes expirationdate in some cases (1.54 KB, patch)
2023-01-04 12:19 UTC, Slava Shishkin
Details | Diff | Splinter Review
Bug 32563: HOTFIX: reservedate becomes expirationdate in some cases (1.59 KB, patch)
2023-01-23 13:41 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 32563: Cloning $self->reservedate leads to error 500 (1.72 KB, patch)
2023-04-19 10:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Slava Shishkin 2023-01-04 12:17:06 UTC
Database fields DATE/DATETIME/TIMESTAMP in DBIX-Class are automatically available as DateTime objects that we can perform operations on (for example add an interval). But there is a feature: addition or subtraction operations (->add in this case) change the main object in-place and return the new value as the result of the expression. Because of this feature, due to the fact that the address to ->reserve date was passed to the ->add function, ->reservedate itself was changed, and then the result was assigned to ->expirationdate. This implicit feature resulted in an error when both dates after the given operation were the same in the future. 

Solution: To not change the original DateTime object, it must be cloned: https://metacpan.org/pod/DateTime#$dt-%3Eclone , accordingly, the changes will be made in a temporary copy, and a new value will be assigned to expirationdate, and the original reservedate will not change.
Comment 1 Slava Shishkin 2023-01-04 12:19:18 UTC
Created attachment 145003 [details] [review]
Bug 32563: HOTFIX: reservedate becomes expirationdate in some cases

To reproduce:
1. Go to /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=DefaultHoldExpirationdate
2. Setup default expiration date to be 1 year from reserve date for example.
3. Switch to catalog to any biblio with item for example /cgi-bin/koha/catalogue/detail.pl?biblionumber=1 and place a hold for any borrower.
4. Check holds for this item, as by example /cgi-bin/koha/reserve/request.pl?biblionumber=1
5. Observe the field "Date" contains the same date as the "Expiration" which is wrong and both contains date 1 year in the future. "Date" should have a date the reservation made, i.e. now.
6. Apply the patch.
7. Make another hold, i.e. repeat steps 3-5
8. Observe the field "Date" contains correct date (today) but the "Expiration" date is 1 year in the future as set is step 2.
Comment 2 Biblibre Sandboxes 2023-01-23 13:41:34 UTC
Created attachment 145573 [details] [review]
Bug 32563: HOTFIX: reservedate becomes expirationdate in some cases

To reproduce:
1. Go to /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=DefaultHoldExpirationdate
2. Setup default expiration date to be 1 year from reserve date for example.
3. Switch to catalog to any biblio with item for example /cgi-bin/koha/catalogue/detail.pl?biblionumber=1 and place a hold for any borrower.
4. Check holds for this item, as by example /cgi-bin/koha/reserve/request.pl?biblionumber=1
5. Observe the field "Date" contains the same date as the "Expiration" which is wrong and both contains date 1 year in the future. "Date" should have a date the reservation made, i.e. now.
6. Apply the patch.
7. Make another hold, i.e. repeat steps 3-5
8. Observe the field "Date" contains correct date (today) but the "Expiration" date is 1 year in the future as set is step 2.

Signed-off-by: Amaury GAU <amaury.gau@bulac.fr>
Comment 3 Anneli Österman 2023-01-27 14:39:23 UTC
I tested this in PTFS-Europe's sandbox and noticed that if DefaultHoldExpirationdate is set and I trap a hold and revert the hold's waiting status and then try to cancel the hold, I get an error.

{UNKNOWN}: Can't locate object method "clone" via package "2023-01-27" (perhaps you forgot to load "2023-01-27"?) at /kohadevbox/koha/Koha/Hold.pm line 898. at /kohadevbox/koha/Koha/Hold.pm line 762
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

The hold's expirationdate should have been set back to the original expirationdate but it becomes empty. When I check reserves table the hold does not have expirationdate or patron_expiration_date set.

If DefaultHoldExpirationdate is not set cancelling the hold works fine after these same steps.

To reproduce:

1. Go to /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=DefaultHoldExpirationdate
2. Setup default expiration date to be 1 year from reserve date for example.
3. Switch to catalog to any biblio with item for example /cgi-bin/koha/catalogue/detail.pl?biblionumber=1 and place a hold for any borrower.
4. Check in an item from the biblio you reserved and your hold gets trapped.
5. Go to the biblio and to it's holds page and revert the waiting status of your hold.
6. Notice that expiration date is empty.
7. Cancel the hold and get an error.
Comment 4 Emmi Takkinen 2023-01-30 12:24:42 UTC
(In reply to Anneli Österman from comment #3)
> I tested this in PTFS-Europe's sandbox and noticed that if
> DefaultHoldExpirationdate is set and I trap a hold and revert the hold's
> waiting status and then try to cancel the hold, I get an error.
> 
> {UNKNOWN}: Can't locate object method "clone" via package "2023-01-27"
> (perhaps you forgot to load "2023-01-27"?) at /kohadevbox/koha/Koha/Hold.pm
> line 898. at /kohadevbox/koha/Koha/Hold.pm line 762
>  at /usr/share/perl5/DBIx/Class/Exception.pm line 77
> 
> The hold's expirationdate should have been set back to the original
> expirationdate but it becomes empty. When I check reserves table the hold
> does not have expirationdate or patron_expiration_date set.

This happens because reservedate fetched from database is not a DateTime object. Instead cloning $self->reservedate we should use dt_from_string to $self->reservedate and then clone. Then use that clone with add. Something like this:

>my $reservedate = dt_from_string( $self->reservedate );
>my $reservedate_clone = $reservedate->clone;
>$self->expirationdate(
>$reservedate_clone->add( $timeunit => $period ) );
Comment 5 Emmi Takkinen 2023-04-19 10:32:43 UTC
Created attachment 149857 [details] [review]
Bug 32563: Cloning $self->reservedate leads to error 500

If DefaultHoldExpirationdate is set and I trap a hold and revert the hold's waiting
status and then try to cancel the hold, I get an error.

To test:
1. Go to /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=DefaultHoldExpirationdate
2. Setup default expiration date to be 1 year from reserve date for example.
3. Switch to catalog to any biblio with item for example /cgi-bin/koha/catalogue/detail.pl?biblionumber=1 and place a hold for any borrower.
4. Check in an item from the biblio you reserved and your hold gets trapped.
5. Go to the biblio and to it's holds page and revert the waiting status of your hold.
6. Notice that expiration date is empty.
7. Cancel the hold and get an error.
8. Apply this patch.
9. Repeat step above, but now no error should be raised.

Also prove prove t/db_dependent/Hold.t

Sponsored-by: Koha-Suomi Oy
Comment 6 Nick Clemens 2023-05-05 11:25:32 UTC

*** This bug has been marked as a duplicate of bug 33611 ***