Bug 24013 - Transferring a checked out item gives a software error
Summary: Transferring a checked out item gives a software error
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-11 20:37 UTC by Aleisha Amohia
Modified: 2019-11-25 21:19 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 24013: Add tests (2.16 KB, patch)
2019-11-20 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24013: Fix transferbook if item is checked out (1.35 KB, patch)
2019-11-20 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24013: Add tests (2.22 KB, patch)
2019-11-21 16:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24013: Fix transferbook if item is checked out (1.41 KB, patch)
2019-11-21 16:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24013: Add tests (2.22 KB, patch)
2019-11-23 07:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24013: Fix transferbook if item is checked out (1.47 KB, patch)
2019-11-23 07:47 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!