[Bug 17480] New: UpdateNotForLoanStatusOnCheckin preference requires space after colon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 Bug ID: 17480 Summary: UpdateNotForLoanStatusOnCheckin preference requires space after colon Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: System Administration Assignee: koha-bugs@lists.koha-community.org Reporter: oleonard@myacpl.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Depends on: 11629 YAML requires that there be a space after a colon, so the UpdateNotForLoanStatusOnCheckin preference does too: -1:0 will not work -1:<space>0 will work We should either handle this gracefully for the user (preferable, since this preference will only use numeric values), or update the preference's description to highlight the requirement. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11629 [Bug 11629] Add ability to update not for loan status on checkin -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I would be good to have a syspref integrity check (in the about page for instance). A first step could be to have a validation on prefs expecting YAML. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20930 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 jpl <data-27FC05A3CB97@office-dateien.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |data-27FC05A3CB97@office-da | |teien.de --- Comment #2 from jpl <data-27FC05A3CB97@office-dateien.de> --- This bug is still around, but it now also prevents any item from being checked in if there is no space after the colon in UpdateNotForLoanStatusOnCheckin. All checkins will fail with a 500 error, even for items that do not have any specific NOT_LOAN authorized value. This can be very frustrating for users as it is hard to spot. The following appears in plack-error.log: Can't use string ("-4:0") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Circulation.pm line 2156. Tested with Koha 22.11.00 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Should we deactivate the feature somehow if misconfigured so it doesn't break things? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 --- Comment #4 from jpl <data-27FC05A3CB97@office-dateien.de> --- I think that would be the best option. If the code that parses UpdateNotForLoanStatusOnCheckin can't make sense of it, then it should be treated as if it were empty. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #3)
Should we deactivate the feature somehow if misconfigured so it doesn't break things?
diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e190019ae1a..b4437331027 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2150,8 +2150,8 @@ sub AddReturn { $yaml = "$yaml\n\n"; # YAML is anal on ending \n. Surplus does not hurt my $rules; eval { $rules = YAML::XS::Load(Encode::encode_utf8($yaml)); }; - if ($@) { - warn "Unable to parse UpdateNotForLoanStatusOnCheckin syspref : $@"; + if ($@ || ref($rules) ne 'HASH') { + warn "Unable to parse UpdateNotForLoanStatusOnCheckin syspref " . ($@ ? " : $@" || ""); } else { foreach my $key ( keys %$rules ) { This patch does what you want, but I don't think it's a good idea. If something is misconfigured it should not be ignored. Only the warn in the logs is certainly very easy to missed. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- ($@ ? " : $@" || ""); should be ($@ ? " : $@" : ""); -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17480 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Maybe we could put something on the about page or an alert on top of the system preferences? We could also warn on saving that it was saved but is incorrect. Not sure what would be the best/easiest to do. OpacHiddenItems can also cause some serious crashing when misconfigured, so we have some candidates here. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org