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.
Created attachment 92801 [details] [review] Bug 23608: Update return value of Koha::Object::Limit::Library
Need to think about the UI a little here.. the 'All libraries' selection working in the inverse adds some fun.
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;
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?
The POD says "It returns undef if no library limits defined."
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 ) %]
(admin/credit_types.pl and admin/debit_types.pl should use the same construct by the way)
Is this still an issue?