[Koha-bugs] [Bug 15494] Block renewals by arbitrary item values

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15494

Marcel de Rooy <m.de.rooy at rijksmuseum.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--- Comment #21 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
+ unless ($item)    { carp "No item passed in!" && return }
Not our preferred coding style

+ return () if (! $yaml =~ /\S/ );
Just return here. The only call is in scalar context. We do not need a list.
Shorter: return if $yaml !~ /\S/;

+    eval {
+        $denyingrules = YAML::Load($yaml);
+    };
How much time are we spending on doing this? What is the performance effect? If
the pref always looks like field1: [val]\nfield2: [val2] we can just split on
newline and split on ':', etc. Give me some convincing numbers please.

+        return ();
Same as above

+        my $val;
+        if (exists $item->{$field}) {
+            $val = $item->{$field};
+        }
+        $val = '' unless defined $val;
Why not in just one line: my $val = $item->{$field} // '';

+            - How to write these rules is documented on the <a
href="http://wiki.koha-community.org/wiki/OpacHiddenItems" target="_blank">Koha
wiki</a>.
This is from the preference file. Not sure if this is the preferred way. A wiki
page could be gone just as easily. Why not just provide an example as close as
possible to the pref itself. Just another 40 chars ?

Just curious too if this supports denying renewal on an empty string? If so,
the conversion from undef to empty string might not be what we want. Can we
signal empty string or undef to be a blocking value? [No blocker for me btw :)]

Changing status. Please make some adjustments.

-- 
You are receiving this mail because:
You are watching all bug changes.