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

--- Comment #2 from Chris Nighswonger 
<[email protected]> ---
Revisiting this ten years on, and correcting my own comment #1 for the
record.

I said then that the -1 came from a required-parameter check in
C4::Creators::Profile::_check_params, and that the behaviour was therefore "as
expected". That was wrong on both counts. _check_params never validated the
presence of anything -- it only checks that the keys it is handed are
recognised names. Calling new() with printer_name absent entirely still returns
a perfectly good object with the defaults filled in, so the mechanism I
described cannot produce Aleisha's error.

What actually happened was a hash shift. The controller built a flat pair-list
from $cgi->param(...) calls in list context. An absent field returns an empty
list, not undef, so the list collapsed from 14 elements to 13: every subsequent
key/value pair shifted by one, and a value landed where a key belonged. That is
what tripped _check_params -- not a missing required field, but an unrecognised
"parameter name" that was really the offset value. Hence the -1, and hence
->save() being called on a non-object. The tell-tale warnings are "Odd number
of elements in anonymous hash" and 'Unrecognized parameter type of "5"'.

So the conclusion I reached -- that -1 should never be returned that way -- was
right, but the reasoning was not, and "WORKSFORME" was the wrong disposition.
The code was not deliberately rejecting incomplete input; it was corrupting its
own argument list.

Bug 16154 (April 2016) then forced scalar context on those param calls, which
stopped the hash shift. That did not fix the underlying gap -- it changed the
symptom. Instead of dying, an empty printer name or paper bin began saving
silently as an empty string, which is exactly what bug 21052 reports. Two
reports, two months apart, one root cause: no required-field validation in
either profile editor.

Bug 21052 fixes that gap. Both fields are now trimmed and validated before
save, the duplicate-key collision that a second blank profile caused is
handled,
and save()'s -1 return -- the thing I flagged here in 2016 -- is now checked
explicitly rather than being treated as success.

Marking this a duplicate of bug 21052, where the patch is attached and awaiting
signoff. The specific route reported here (one field filled, the other empty)
is
covered by that patch and by step 2 of its test plan.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to