Bug 23608 - Koha::Objects::Limit::Library->get_library_limits can return undef
Summary: Koha::Objects::Limit::Library->get_library_limits can return undef
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 17755
Blocks:
  Show dependency treegraph
 
Reported: 2019-09-13 14:01 UTC by Martin Renvoize
Modified: 2021-11-18 14:20 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23608: Update return value of Koha::Object::Limit::Library (1.54 KB, patch)
2019-09-13 14:05 UTC, Martin Renvoize
Details | Diff | Splinter Review

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