Bug 24013

Summary: Transferring a checked out item gives a software error
Product: Koha Reporter: Aleisha Amohia <aleisha>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: gmcharlt, jonathan.druart, kyle.m.hall, lucas, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00, 19.05.11
Attachments: Bug 24013: Add tests
Bug 24013: Fix transferbook if item is checked out
Bug 24013: Add tests
Bug 24013: Fix transferbook if item is checked out
Bug 24013: Add tests
Bug 24013: Fix transferbook if item is checked out

Description Aleisha Amohia 2019-11-11 20:37:05 UTC
To reproduce:
1. check out an item
2. Go to circulation -> transfer and put in the barcode of the item you just checked out
3. error: Can't call method "borrowernumber" on unblessed reference at /home/vagrant/kohaclone/C4/Circulation.pm line 350

This occurs in the C4::Circulation::transferbook subroutine. It happens because Bug 18276 accidentally reintroduces an old subroutine (GetOpenIssue) that returns an unblessed reference. This line should use Koha Objects instead.

Code causing an error:

my $issue = GetOpenIssue($itemnumber);
# check if it is still issued to someone, return it...
if ( $issue ) {
    AddReturn( $barcode, $fbr );
    $messages->{'WasReturned'} = $issue->borrowernumber;
}

Suggested fix:

my $issue = Koha::Checkouts->find({ itemnumber => $itemnumber });
Comment 1 Jonathan Druart 2019-11-20 10:42:18 UTC
Upping severity.
Comment 2 Jonathan Druart 2019-11-20 10:56:26 UTC
Created attachment 95608 [details] [review]
Bug 24013: Add tests
Comment 3 Jonathan Druart 2019-11-20 10:56:30 UTC
Created attachment 95609 [details] [review]
Bug 24013: Fix transferbook if item is checked out

Wrong conflict resolution, the following
  commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
  Bug 18276: Remove GetBiblioFromItemNumber - Easy ones
restored a previous change from
  commit 546379cc92b733cb29a0b70247a72c770afdad26
  Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls

Not that "easy" or "simple"...

Test plan:
Run the test before and after this patch.
Comment 4 Jonathan Druart 2019-11-20 10:58:33 UTC
Hello Aleisha,

I removed the Academy kw and provided 2 patches, a bugfix and the tests to cover the changes. If you need this bug report to use as example, I can obsolete the bugfix and keep the tests.

It sounded quite urgent to fix as it impact stable releases.

Cheers
Comment 5 Martin Renvoize 2019-11-21 16:41:38 UTC
Created attachment 95695 [details] [review]
Bug 24013: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2019-11-21 16:41:42 UTC
Created attachment 95696 [details] [review]
Bug 24013: Fix transferbook if item is checked out

Wrong conflict resolution, the following
  commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
  Bug 18276: Remove GetBiblioFromItemNumber - Easy ones
restored a previous change from
  commit 546379cc92b733cb29a0b70247a72c770afdad26
  Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls

Not that "easy" or "simple"...

Test plan:
Run the test before and after this patch.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2019-11-21 16:42:23 UTC
Thanks for the quick fix, works well.. Signing off.
Comment 8 Katrin Fischer 2019-11-23 07:47:45 UTC
Created attachment 95755 [details] [review]
Bug 24013: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Katrin Fischer 2019-11-23 07:47:48 UTC
Created attachment 95756 [details] [review]
Bug 24013: Fix transferbook if item is checked out

Wrong conflict resolution, the following
  commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
  Bug 18276: Remove GetBiblioFromItemNumber - Easy ones
restored a previous change from
  commit 546379cc92b733cb29a0b70247a72c770afdad26
  Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls

Not that "easy" or "simple"...

Test plan:
Run the test before and after this patch.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Martin Renvoize 2019-11-25 08:43:56 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Aleisha Amohia 2019-11-25 21:19:36 UTC
(In reply to Jonathan Druart from comment #4)
> Hello Aleisha,
> 
> I removed the Academy kw and provided 2 patches, a bugfix and the tests to
> cover the changes. If you need this bug report to use as example, I can
> obsolete the bugfix and keep the tests.
> 
> It sounded quite urgent to fix as it impact stable releases.
> 
> Cheers

No worries Jonathan, thank you for fixing!
Comment 12 Josef Moravec 2020-04-30 14:50:59 UTC
This affects 19.05 too and applies cleanly, can you push it Lucas?
Comment 13 Lucas Gass 2020-04-30 16:01:15 UTC
backported to 19.05.x for 19.05.11