[Bug 15739] New: Saving a new printer profile does not work
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15739 Bug ID: 15739 Summary: Saving a new printer profile does not work Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Label/patron card printing Assignee: cnighswonger@foundations.edu Reporter: aleishaamohia@hotmail.com QA Contact: testopia@bugs.koha-community.org If you leave either the Printer Name or Paper Bin field empty (ie only putting something in one of those fields and leaving the other empty) then you will get a software error upon clicking Save Can't call method "save" without a package or object reference at /home/vagrant/kohaclone/patroncards/edit-profile.pl line 72. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15739 Chris Nighswonger <cnighswonger@foundations.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion --- Comment #1 from Chris Nighswonger <cnighswonger@foundations.edu> --- Granted the error trapping here and feedback to the user leaves much to be desired, however, the behavior is as expected. Looking here: http://tinyurl.com/z3t845z will show that what we do is run a parameter check when asked to create a new Profile object. If all required parameters are not passed in, rather than returning a new object, -1 is returned which causes the attempt to invoke the save method to fail. Its old code, and I never go back to adding proper error trapping. It really should not return -1 in any case. In light of that, this bug should probably be closed RESOLVED WORKSFORME and another opened which addresses error trapping in the labels/patroncards code in general. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15739 --- Comment #2 from Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> --- 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15739 Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |RESOLVED Resolution|--- |DUPLICATE --- Comment #3 from Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> --- *** This bug has been marked as a duplicate of bug 21052 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org