Bug 19899

Summary: The float items feature is broken - cannot checkin
Product: Koha Reporter: Mohd Hafiz Yusoff <mhby87>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: fridolin.somers, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, nick, sandboxes
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:
Bug Depends on: 18276    
Bug Blocks:    
Attachments: The method noreturn is not covered by tests!
Bug 19899: Restore Item floats feature - do not crash
Error after edit code line 279
Bug 19899: Restore Item floats feature - do not crash
Successful fix the problem, thank you
Bug 19899: Restore Item floats feature - do not crash
Bug 19899: Restore Item floats feature - do not crash

Description Mohd Hafiz Yusoff 2017-12-31 13:49:00 UTC
Created attachment 70217 [details]
The method noreturn is not covered by tests!
Comment 1 Mohd Hafiz Yusoff 2017-12-31 13:51:07 UTC
Software Error while upgrade from KOHA 16.05 to 17.11. This error while check in and I got error message "The method noreturn is not covered by tests!"
Comment 2 Mohd Hafiz Yusoff 2018-01-04 16:51:21 UTC
Anybody can help?
Comment 3 Jonathan Druart 2018-01-04 18:06:55 UTC
Hi Mohd,

How did you install Koha? Are you sure it is not a customized version of Koha?

What returns the following commands?
  cd /usr/share/koha; grep -r noreturn | grep -v translator

I do not find any relevant occurrences of "noreturn" in our code that could produce such error.

Do you have something else in the Koha log file?
Comment 4 Mohd Hafiz Yusoff 2018-01-05 12:23:21 UTC
Fresh install koha using this instruction: https://wiki.koha-community.org/wiki/Koha_on_ubuntu_-_packages

After install, I restore old database (from KOHA 16.05).

And then upgrade schema "koha-upgrade-schema {instance}"

Last reindex "koha-rebuild-zebra -v -f {instance}"
Comment 5 Mohd Hafiz Yusoff 2018-01-05 12:27:21 UTC
Return from command

lib/C4/Circulation.pm:  noreturn: do not return, let item remain where checked in (floating collections)
lib/C4/Circulation.pm:        # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch)
intranet/htdocs/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:                            [% IF ( default_returnbranch == 'noreturn' ) %]
intranet/htdocs/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:                            <option value="noreturn" selected="selected">
intranet/htdocs/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:                            <option value="noreturn">
intranet/htdocs/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:                            [% ELSIF ( branch_item_rule_loo.returnbranch == 'noreturn' ) %]
intranet/htdocs/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:                            <option value="noreturn">Item floats</option>
intranet/cgi-bin/installer/data/mysql/kohastructure.sql:  `returnbranch` varchar(15) default NULL, -- the branch the item returns to (homebranch, holdingbranch, noreturn)
man/man3/C4::Circulation.3pm:  noreturn: do not return, let item remain where checked in (floating collections)
Comment 6 Jonathan Druart 2018-01-05 14:22:21 UTC
Do you have something else in the Koha log file?

I will need the file and line number where the error occurs.
Comment 7 Jonathan Druart 2018-01-05 14:26:32 UTC
Ok I recreate the problem, it happens when "Return policy" is "Item floats" (value noreturn) in circ rules.
Comment 8 Jonathan Druart 2018-01-05 14:32:24 UTC
Certainly caused by commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones
Comment 9 Jonathan Druart 2018-01-05 14:47:38 UTC
Created attachment 70290 [details] [review]
Bug 19899: Restore Item floats feature - do not crash

Error is "The method noreturn is not covered by tests!"
caused by commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

When the "Return policy" (var returnbranch) is set to "Item floats" (value noreturn)
the library must be the current library (from userenv).
This behaviour is used in C4::Circulation::AddReturn

On bug 18276 I assumed that the value was either holdingbranch or
homebranch, and forgot it could be "noreturn"

Test plan:
Set Return policy to Item floats
Check an item in
=> without this patch it explodes
=> With this patch applied it will check the item in
Comment 10 Mohd Hafiz Yusoff 2018-01-06 16:52:06 UTC
Created attachment 70316 [details]
Error after edit code line 279

After edit code at line 279, the error like attachment
Comment 11 Katrin Fischer 2018-01-06 16:59:27 UTC
Hi Mohd, did you manually edit the files or apply the patches?
Comment 12 Katrin Fischer 2018-01-06 17:00:44 UTC
Hm, you could try adding a ; to the end of this line:
$returnbranch = $hbr eq 'noreturn' ? $item->$hbr : $userenv_branch;
Comment 13 Mohd Hafiz Yusoff 2018-01-06 17:08:08 UTC
I manually edit the files, after add semicolon, error same "manually edit the files"

How to apply the patches?
Comment 14 Jonathan Druart 2018-01-06 17:21:19 UTC
Oops indeed, there is a semicolon missing at the end. I am going to reattach a patch.
Comment 15 Jonathan Druart 2018-01-06 17:24:10 UTC
Created attachment 70317 [details] [review]
Bug 19899: Restore Item floats feature - do not crash

Error is "The method noreturn is not covered by tests!"
caused by commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

When the "Return policy" (var returnbranch) is set to "Item floats" (value noreturn)
the library must be the current library (from userenv).
This behaviour is used in C4::Circulation::AddReturn

On bug 18276 I assumed that the value was either holdingbranch or
homebranch, and forgot it could be "noreturn"

Test plan:
Set Return policy to Item floats
Check an item in
=> without this patch it explodes
=> With this patch applied it will check the item in
Comment 16 Mohd Hafiz Yusoff 2018-01-06 17:43:25 UTC
Created attachment 70318 [details]
Successful fix the problem, thank you

Your code has been successful fix the problem. Now I can return my book easier.

Thank you all
Comment 17 Jonathan Druart 2018-01-06 18:44:13 UTC
Thanks Mohd for the confirmation.

You should read this wiki page (https://wiki.koha-community.org/wiki/Sign_off_on_patches) to know how to apply patches (using git-bz) and learn how to sign off on these patches.
Comment 18 Biblibre Sandboxes 2018-01-07 12:09:10 UTC
Patch tested with a sandbox, by Mohd Hafiz Yusoff <mhby87@gmail.com>
Comment 19 Biblibre Sandboxes 2018-01-07 12:09:41 UTC
Created attachment 70319 [details] [review]
Bug 19899: Restore Item floats feature - do not crash

Error is "The method noreturn is not covered by tests!"
caused by commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

When the "Return policy" (var returnbranch) is set to "Item floats" (value noreturn)
the library must be the current library (from userenv).
This behaviour is used in C4::Circulation::AddReturn

On bug 18276 I assumed that the value was either holdingbranch or
homebranch, and forgot it could be "noreturn"

Test plan:
Set Return policy to Item floats
Check an item in
=> without this patch it explodes
=> With this patch applied it will check the item in

Signed-off-by: Mohd Hafiz Yusoff <mhby87@gmail.com>
Comment 20 Nick Clemens 2018-01-08 17:19:43 UTC
Created attachment 70332 [details] [review]
Bug 19899: Restore Item floats feature - do not crash

Error is "The method noreturn is not covered by tests!"
caused by commit 6eade474ed3a84a5ba372a26ac27bf9fb4bd2299
Bug 18276: Remove GetBiblioFromItemNumber - Easy ones

When the "Return policy" (var returnbranch) is set to "Item floats" (value noreturn)
the library must be the current library (from userenv).
This behaviour is used in C4::Circulation::AddReturn

On bug 18276 I assumed that the value was either holdingbranch or
homebranch, and forgot it could be "noreturn"

Test plan:
Set Return policy to Item floats
Check an item in
=> without this patch it explodes
=> With this patch applied it will check the item in

Signed-off-by: Mohd Hafiz Yusoff <mhby87@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Jonathan Druart 2018-01-09 12:55:22 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 22 Nick Clemens 2018-01-15 16:48:05 UTC
Awesome work all, pushed to stable for 17.11.02
Comment 23 Fridolin Somers 2018-01-22 10:59:13 UTC
Bug 18276 not in 17.05.x