Bug 37016 - SIP2 renew shows old/wrong date due
Summary: SIP2 renew shows old/wrong date due
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P3 normal
Assignee: Andreas Jonsson
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 37200
  Show dependency treegraph
 
Reported: 2024-06-03 13:01 UTC by Magnus Enger
Modified: 2024-07-22 09:41 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Set correct due date in SIP2 renewal response message.
Version(s) released in:
24.11.00,24.05.02,23.11.07
Circulation function:


Attachments
Bug 37016: Invalid due date in SIP renew response (2.03 KB, patch)
2024-06-13 15:05 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 37016: Invalid due date in SIP renew response (2.19 KB, patch)
2024-06-14 12:48 UTC, HKS3 Tadeusz Sośnierz
Details | Diff | Splinter Review
Bug 37016: Unit tests (2.55 KB, patch)
2024-06-26 14:50 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37016: Invalid due date in SIP renew response (2.25 KB, patch)
2024-06-26 14:50 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2024-06-03 13:01:09 UTC
This was reported by a library running 23.11.03. 

Sometimes when patrons do renewals via SIP2, the old due date is reported as the new due date. But everything else looks OK, including the staff view and "my profile", which shows the right due date. 

It looks like this only happens some of the time, or for some patrons, but I have not been able to figure out what the difference is there. I do think I have spotted the problem, though. 

C4::Circulation::AddIssue() has this code: 

1552     my $issue;
...
1571         # find which item we issue
1572         my $item_object = Koha::Items->find({ barcode => $barcode })
1573           or return;    # if we don't get an Item, abort.
1574         my $item_unblessed = $item_object->unblessed;
1575 
1576         my $branchcode = _GetCircControlBranch( $item_object, $patron );
1577 
1578         # get actual issuing if there is one
1579         my $actualissue = $item_object->checkout;
1580 
1581         # check if we just renew the issue.
1582         if ( $actualissue and $actualissue->borrowernumber eq $patron->borrowernumber
1583                 and not $switch_onsite_checkout ) {
1584             $datedue = AddRenewal(
1585                 {
1586                     borrowernumber => $patron->borrowernumber,
1587                     itemnumber     => $item_object->itemnumber,
1588                     branch         => $branchcode,
1589                     datedue        => $datedue,
1590                     lastreneweddate =>
1591                       $issuedate,    # here interpreted as the renewal date
1592                 }
1593             );
1594             $issue = $item_object->checkout;
1595         }

We get the $item_object on line 1572. Then we get the active issue/loan/checkout from the $item_object on line 1579. Renewal is done on line 1584 (and testing shows $datedue has the right date due there). But then we update $issue with the active issue/loan/checkout from the same $item_object as earlier, which has not been updated, and contains stale data, including the original date due. 

I was able to quickfix this by replacing line 1594 with this: 

             my $updated_item_object = my $item_object = Koha::Items->find({ barcode => $barcode });
             $issue = $updated_item_object->checkout;

But I am far from confident that this is a good fix, so opinions are welcome!
Comment 1 Andreas Jonsson 2024-06-13 13:48:30 UTC
It looks like you can use the get_from_storage method:

  DB<14> x $item->checkout->unblessed;
0  HASH(0x5624163e5780)
   'auto_renew' => 0
   'auto_renew_error' => undef
   'borrowernumber' => 190
   'branchcode' => 'LA'
   'date_due' => '2024-06-13 23:59:00'
   'issue_id' => 208
   'issuedate' => '2024-06-13 15:38:17'
   'issuer_id' => undef
   'itemnumber' => 310173
   'lastreneweddate' => undef
   'note' => undef
   'notedate' => undef
   'noteseen' => undef
   'onsite_checkout' => 0
   'renewals_count' => 0
   'returndate' => undef
   'timestamp' => '2024-06-13 15:38:17'
   'unseen_renewals' => 0
  DB<15> x $item->checkout->get_from_storage->unblessed;
0  HASH(0x5624163e36c0)
   'auto_renew' => 0
   'auto_renew_error' => undef
   'borrowernumber' => 190
   'branchcode' => 'LA'
   'date_due' => '2024-06-13 23:59:00'
   'issue_id' => 208
   'issuedate' => '2024-06-13 15:38:17'
   'issuer_id' => undef
   'itemnumber' => 310173
   'lastreneweddate' => '2024-06-13 15:40:00'
   'note' => undef
   'notedate' => undef
   'noteseen' => undef
   'onsite_checkout' => 0
   'renewals_count' => 1
   'returndate' => undef
   'timestamp' => '2024-06-13 15:40:00'
   'unseen_renewals' => 0
Comment 2 Andreas Jonsson 2024-06-13 15:05:45 UTC
Created attachment 167688 [details] [review]
Bug 37016: Invalid due date in SIP renew response

Test plan using koha-testing-docker:

1) Make sure SIP is running.  You may need to edit
   /etc/koha/sites/SIPconfig.xml and remove the 8023 connector and
   restart the SIP-server (koha-sip --restart kohadev)
2) Find a patron, say 23529000197047
3) Set a password by selecting "change password", set it to
   "Password1234"
4) Find a book, say 39999000000856
5) Issue book to patron with sip-client:
   sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
                      --address localhost --port 6001 -t cr \
                      --su term1 --sp term1 --message checkout \
                      --location CPL --item 39999000000856 \
                      --patron 23529000197047 --password Password1234"\
                      kohadev
6) Note the AH-header in the response which for example:
   'AH20240619    235900'
7) Make a renewal with:
   sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
                      --address localhost --port 6001 -t cr \
                      --su term1 --sp term1 --message renew \
                      --location CPL --item 39999000000856 \
                      --patron 23529000197047 --password Password1234"\
                      kohadev
8) Make sure the AH-header in the response is different from the
   response to the checkout, for example: 'AH20240624 235900'
Comment 3 HKS3 Tadeusz Sośnierz 2024-06-14 12:48:01 UTC
Created attachment 167712 [details] [review]
Bug 37016: Invalid due date in SIP renew response

Test plan using koha-testing-docker:

1) Make sure SIP is running.  You may need to edit
   /etc/koha/sites/SIPconfig.xml and remove the 8023 connector and
   restart the SIP-server (koha-sip --restart kohadev)
2) Find a patron, say 23529000197047
3) Set a password by selecting "change password", set it to
   "Password1234"
4) Find a book, say 39999000000856
5) Issue book to patron with sip-client:
   sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
                      --address localhost --port 6001 -t cr \
                      --su term1 --sp term1 --message checkout \
                      --location CPL --item 39999000000856 \
                      --patron 23529000197047 --password Password1234"\
                      kohadev
6) Note the AH-header in the response which for example:
   'AH20240619    235900'
7) Make a renewal with:
   sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
                      --address localhost --port 6001 -t cr \
                      --su term1 --sp term1 --message renew \
                      --location CPL --item 39999000000856 \
                      --patron 23529000197047 --password Password1234"\
                      kohadev
8) Make sure the AH-header in the response is different from the
   response to the checkout, for example: 'AH20240624 235900'

Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Comment 4 Nick Clemens (kidclamp) 2024-06-26 14:50:53 UTC
Created attachment 168152 [details] [review]
Bug 37016: Unit tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens (kidclamp) 2024-06-26 14:50:55 UTC
Created attachment 168153 [details] [review]
Bug 37016: Invalid due date in SIP renew response

Test plan using koha-testing-docker:

1) Make sure SIP is running.  You may need to edit
   /etc/koha/sites/SIPconfig.xml and remove the 8023 connector and
   restart the SIP-server (koha-sip --restart kohadev)
2) Find a patron, say 23529000197047
3) Set a password by selecting "change password", set it to
   "Password1234"
4) Find a book, say 39999000000856
5) Issue book to patron with sip-client:
   sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
                      --address localhost --port 6001 -t cr \
                      --su term1 --sp term1 --message checkout \
                      --location CPL --item 39999000000856 \
                      --patron 23529000197047 --password Password1234"\
                      kohadev
6) Note the AH-header in the response which for example:
   'AH20240619    235900'
7) Make a renewal with:
   sudo koha-shell -c "/usr/share/koha/bin/sip_cli_emulator.pl \
                      --address localhost --port 6001 -t cr \
                      --su term1 --sp term1 --message renew \
                      --location CPL --item 39999000000856 \
                      --patron 23529000197047 --password Password1234"\
                      kohadev
8) Make sure the AH-header in the response is different from the
   response to the checkout, for example: 'AH20240624 235900'

Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Katrin Fischer 2024-06-27 09:15:40 UTC
Test plan is really nice, but please also include a little summary about the problem and the solution :)
Comment 7 Katrin Fischer 2024-06-27 09:52:51 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 8 Lucas Gass 2024-07-19 14:32:11 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 9 Fridolin Somers 2024-07-22 09:41:10 UTC
Pushed to 23.11.x for 23.11.07