Bug 18266 - Internal Server Error when paying fine for lost item
Summary: Internal Server Error when paying fine for lost item
Status: Pushed to Stable
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:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-14 14:43 UTC by Marc Véron
Modified: 2017-05-03 04:08 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18266: Fix internal error when paying fine for lost item without.. item (1.46 KB, patch)
2017-03-21 16:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18266: Fix internal error when paying fine for lost item without.. item (1.59 KB, patch)
2017-03-22 10:33 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18266: Fix internal error when paying fine for lost item without.. item (1.65 KB, patch)
2017-03-29 15:24 UTC, Nick Clemens
Details | Diff | Splinter Review
[16.11.x] Bug 18266: Fix internal error when paying fine for lost item without.. item (1.42 KB, patch)
2017-04-03 13:20 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2017-03-14 14:43:06 UTC
To reproduce:
- Create a Manual invoice for a patron, select Type: Lost Item
- Do not fill in Barcode, Description
- Give the invoice an amount that is easy to identify
- Go to tab "Pay fines"
- Identify row of new fine, click "Pay"
- On the following page, click "Confirm"
Result: "Internal Server Error"
plack error log says:
Can't call method "last_returned_by" on an undefined value at /home/marc/koha/C4/Circulation.pm line 2188.

- Repeat steps above, insert a valid Barcode of an item that is lost. Same result.
Comment 1 Marc Véron 2017-03-14 15:14:50 UTC
Following finding:

sub pay in Koha/Account.pm line 106 triggers  
C4::Circulation::ReturnLostItem ( $self->{patron_id}, $fine->itemnumber );

$fine->itemnumber is not defined.

But should payment for a lost item mark the item as returned? 

Payment for an item that is lost does not necessarily mean that the item is already replaced or will be replaced at all.
Comment 2 Marc Véron 2017-03-15 15:03:01 UTC
(In reply to Marc Véron from comment #1)
> Following finding:

> But should payment for a lost item mark the item as returned? 
> 
> Payment for an item that is lost does not necessarily mean that the item is
> already replaced or will be replaced at all.

Please ignore this comment - of course the item should be marked as returned.

I think 
- it should not be possible to create a manual invoice for a lost item or alternatively the barcode should be verified and the item should be set to LOST (not in the scope of this bug)
- paying a fine for a lost item without having a barcode should not result in an internal server error (scope of this bug)
Comment 3 Jonathan Druart 2017-03-21 15:55:09 UTC
To recreate you need StoreLastBorrower
Comment 4 Jonathan Druart 2017-03-21 16:03:06 UTC
Created attachment 61402 [details] [review]
Bug 18266: Fix internal error when paying fine for lost item without.. item

If a fine is created for a lost item but the itemnumber is not supplied,
the system will return it.
The item should not be mark as returned if there is no item linked to
the fine.

Test plan:
1. Turn StoreLastBorrower on
2. Create a manual invoice for a lost item, do not supply a barcode
3. Pay the fines 'Pay fines > Pay'

=> Without this patch applied you get
Can't call method "last_returned_by" on an undefined value at
/home/marc/koha/C4/Circulation.pm line 2188.

=> With this patch applied, you must not get the error.
Comment 5 Marc Véron 2017-03-22 10:33:38 UTC
Created attachment 61439 [details] [review]
Bug 18266: Fix internal error when paying fine for lost item without.. item

If a fine is created for a lost item but the itemnumber is not supplied,
the system will return it.
The item should not be mark as returned if there is no item linked to
the fine.

Test plan:
1. Turn StoreLastBorrower on
2. Create a manual invoice for a lost item, do not supply a barcode
3. Pay the fines 'Pay fines > Pay'

=> Without this patch applied you get
Can't call method "last_returned_by" on an undefined value at
/home/marc/koha/C4/Circulation.pm line 2188.

=> With this patch applied, you must not get the error.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 6 Nick Clemens 2017-03-29 15:24:38 UTC
Created attachment 61687 [details] [review]
Bug 18266: Fix internal error when paying fine for lost item without.. item

If a fine is created for a lost item but the itemnumber is not supplied,
the system will return it.
The item should not be mark as returned if there is no item linked to
the fine.

Test plan:
1. Turn StoreLastBorrower on
2. Create a manual invoice for a lost item, do not supply a barcode
3. Pay the fines 'Pay fines > Pay'

=> Without this patch applied you get
Can't call method "last_returned_by" on an undefined value at
/home/marc/koha/C4/Circulation.pm line 2188.

=> With this patch applied, you must not get the error.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Kyle M Hall 2017-03-31 10:46:38 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 8 Katrin Fischer 2017-04-02 16:09:43 UTC
I was able to reproduce this error on 16.11.x, but the patch doesn't apply cleanly. There is a conflict in Koha/Account.pm.

Jonathan, could you please take a look? I'd really like this for 16.11.07.
Comment 9 Jonathan Druart 2017-04-03 13:20:09 UTC
Created attachment 61795 [details] [review]
[16.11.x] Bug 18266: Fix internal error when paying fine for lost item without.. item

If a fine is created for a lost item but the itemnumber is not supplied,
the system will return it.
The item should not be mark as returned if there is no item linked to
the fine.

Test plan:
1. Turn StoreLastBorrower on
2. Create a manual invoice for a lost item, do not supply a barcode
3. Pay the fines 'Pay fines > Pay'

=> Without this patch applied you get
Can't call method "last_returned_by" on an undefined value at
/home/marc/koha/C4/Circulation.pm line 2188.

=> With this patch applied, you must not get the error.
Comment 10 Katrin Fischer 2017-04-10 21:45:43 UTC
This patch has been pushed to 16.11.x and will be in 16.11.07.
Comment 11 Julian Maurice 2017-04-21 10:44:35 UTC
Pushed to 3.22.x for 3.22.20
Comment 12 Mason James 2017-05-03 04:08:35 UTC
Pushed to 16.05.x, for 16.05.12 release