Bug 23608

Summary: Koha::Objects::Limit::Library->get_library_limits can return undef
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 17755    
Bug Blocks:    
Attachments: Bug 23608: Update return value of Koha::Object::Limit::Library

Description Martin Renvoize (ashimema) 2019-09-13 14:01:54 UTC
The get_library_limits routine within Koha::Objects::Limit::Library can return either a Koha::Libraries set or undef at present.

As this is often part of a method chain we should drop the requirement to always check for undef in the return and instead ensure we return an unfiltered Koha::Libraries set which can be relied upon in subsequent method chaining.
Comment 1 Martin Renvoize (ashimema) 2019-09-13 14:05:04 UTC
Created attachment 92801 [details] [review]
Bug 23608: Update return value of Koha::Object::Limit::Library
Comment 2 Martin Renvoize (ashimema) 2019-09-13 14:11:16 UTC
Need to think about the UI a little here.. the 'All libraries' selection working in the inverse adds some fun.
Comment 3 Jonathan Druart 2021-11-15 10:36:21 UTC
I was going to open a bug for that, it leads to several warnings in categories.pl

[WARN] Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt line 629.

However my patch was simply doing:

-    return unless @branchcodes;
+    return Koha::Libraries->new->empty unless @branchcodes;
Comment 4 Martin Renvoize (ashimema) 2021-11-15 17:06:02 UTC
Wow... there's a lot of superfluous noise in my patch.. must have had a tidy setting on when I committed it.

I do similar but instead of 'empty' I return 'all'..  shouldn't 'no limit' mean all rather than none.. or am I misunderstanding the function in the first place?
Comment 5 Jonathan Druart 2021-11-18 14:08:47 UTC
The POD says "It returns undef if no library limits defined."
Comment 6 Jonathan Druart 2021-11-18 14:18:05 UTC
We actually return nothing in the first place to not selected anything on the admin edit form, for item type and categories:

koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt:                        [% PROCESS options_for_libraries libraries => Branches.all( selected => category.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %]

koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt:                        [% PROCESS options_for_libraries libraries => Branches.all( selected => itemtype.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %]
Comment 7 Jonathan Druart 2021-11-18 14:19:51 UTC
(admin/credit_types.pl and admin/debit_types.pl should use the same construct by the way)
Comment 8 Martin Renvoize (ashimema) 2024-04-25 13:07:52 UTC
Is this still an issue?