https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052
--- Comment #8 from Chris Nighswonger <[email protected]> --- Created attachment 202689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202689&action=edit Bug 21052: Validate required fields and handle duplicate printer profiles printers_profile carries a UNIQUE key on (printer_name, template_id, paper_bin, creator). Neither profile editor validated that the printer name and paper bin were supplied, so a blank field was stored as an empty string. The first blank save succeeded; a second one collided on the unique key and the resulting Koha::Exceptions::Object::DuplicateID went unhandled, giving the user a bare 500 error page. The patron card editor passed CGI parameters through untouched. The label editor substituted 'DEFAULT PRINTER' / 'Tray 1' for blanks, which only moved the collision -- two blank saves still produced the same crash there. Validate both fields before saving and re-render the form with a warning when either is empty, catch DuplicateID and re-render with a warning rather than dying, and mark both inputs required in the templates so the browser catches the common case first. Entered values are preserved on redisplay. save() reports some database failures by returning -1 rather than by throwing, so the return value is now checked and routed to an error message instead of a redirect that would imply success. C4::Creators::Profile->new() has always defaulted printer_name to 'Default Printer' and paper_bin to 'Tray 1', but the new-profile form never rendered them: the create branch of both templates emitted an input with no value attribute, so the user was shown two empty boxes and saving an untouched form stored empty strings. Adding the value attribute -- and it has to be added, so that what the user typed survives an error redisplay -- also surfaces those defaults on first load, which is what they were for. A profile deleted between opening the edit form and saving it left retrieve() blessing an undefined value, which died outside the handler as another unhandled 500. That case now redirects back to the profile list with the standard error notice. Because the first blank save succeeded, existing installations have rows with a blank printer name or paper bin. Those fields are stored as hidden inputs on the edit form, which would make such a row unfixable and, worse, silently insert a duplicate. A blank stored value is now rendered as an editable input so the row can be repaired in place, and the two fields stay editable on any error redisplay so a repair that hits a duplicate name can still be corrected. The patron card editor also passed the offset, creep and units fields through without a default, unlike its label twin. Clearing an offset submitted an empty string into a float NOT NULL column; under strict SQL modes that raises an exception which is not DuplicateID and so would be rethrown -- the same unhandled 500 this patch removes. Both editors now apply the same defaults, trim whitespace from the two required fields, and set maxlength to match the column widths. This also fixes bug 15739, which reported the same crash reached by leaving only one of the two fields empty. Test plan: 1) Go to Tools > Patron card creator > Manage > Printer profiles > New profile. 2) Clear 'Printer name' and/or 'Paper bin' and save. (Without this patch both boxes are already empty; with it they arrive pre-filled with the defaults, so they have to be cleared first.) -- without this patch: the profile is silently stored with an empty value; saving a second such profile returns a 500 error page -- with this patch: the form redisplays with "Printer name and paper bin are both required." and nothing is saved 3) Enter only spaces in either field and save. -- the same warning appears; no profile is created 4) Enter a printer name and paper bin, save. Create a second profile with the same name and bin. -- without this patch: 500 error page -- with this patch: the form redisplays with "A printer profile with this printer name and paper bin already exists." 5) Create a profile, then edit it and clear the horizontal offset before saving. -- without this patch, on an installation using strict SQL modes: 500 error page -- with this patch: the offset saves as 0 6) Repeat 1-5 under Cataloging > Label creator > Manage > Printer profiles. Note that on a new profile form both editors now arrive pre-filled with 'Default Printer' and 'Tray 1'; without this patch the two boxes are empty. 7) Simulate a profile left behind by this bug: INSERT INTO printers_profile (printer_name, template_id, paper_bin, offset_horz, offset_vert, creep_horz, creep_vert, units, creator) VALUES ('', 0, '', 0, 0, 0, 0, 'POINT', 'Patroncards'); Edit that profile from the patron card creator. -- without this patch: the blank fields are read-only, saving trips the required-field check, and the form comes back as "New printer profile" -- filling it in inserts a second row and orphans the blank one -- with this patch: the blank fields are editable, the heading still reads "Edit printer profile", and saving repairs the existing row in place 8) Repeat step 7, but give the blank profile a printer name and paper bin that match an existing profile. -- the duplicate warning appears and both fields remain editable, so another name can be entered without leaving the page 9) Open a profile for editing, delete that same profile from the profile list in another browser tab, then save the still-open form. -- without this patch: 500 error page -- with this patch: the profile list is shown with an error notice 10) Edit an existing profile, change an offset, save. -- the profile updates as before 11) prove t/db_dependent/Labels/ t/db_dependent/Creators/ t/Creators.t AI Assistance: Claude Opus 5 (Anthropic) reproduced both bug reports in koha-testing-docker, identified the unique-key collision as the shared root cause (neither report described it correctly), implemented the validation and exception handling, and verified every save scenario across both editors before and after the change. Claude Fable 5 (Anthropic) reviewed two revisions. The first review found the unchecked -1 return from save(), the missing defaults on the patron card editor's numeric fields, and the unrepairable legacy blank rows; the second confirmed those fixes and found the unhandled deleted-profile case and a dead end where a duplicate during an in-place repair left the offending field read-only. OpenAI Codex GPT-5.5 (OpenAI) reviewed the result at the merge gate and found that the deleted-profile redirect interpolated an unvalidated profile_id into a query string, which is now built with URI->query_form. All of those findings are addressed here. The human author directed the investigation, decided to treat bug 15739 as a duplicate rather than patch it separately, and reviewed the change. Assisted-by: Claude Opus 5 (Anthropic) Assisted-by: Claude Fable 5 (Anthropic) Assisted-by: OpenAI Codex GPT-5.5 (OpenAI) -- 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/
