| Summary: | Error handling in additem.pl is too specific and insufficient | ||
|---|---|---|---|
| Product: | Koha | Reporter: | HKS3 Tadeusz Sośnierz <tadeusz> |
| Component: | Cataloging | Assignee: | Bugs List <koha-bugs> |
| Status: | NEW --- | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | jonathan.druart, m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 41696: Generalize additem.pl/tt's error handling
Bug 41696: Improve additem.pl/tt's error handling Bug 41696: Add a test |
||
|
Description
HKS3 Tadeusz Sośnierz
2026-01-23 14:01:42 UTC
Created attachment 191940 [details] [review] Bug 41696: Generalize additem.pl/tt's error handling This prevents us from losing track of errors and failing quietly. Created attachment 192092 [details] [review] Bug 41696: Improve additem.pl/tt's error handling This prevents us from losing track of errors and failing quietly. Also gets rid of hardcoded error messages for Koha::Object exceptions, defering to full_message which provides more details. Sponsored-by: Universitätsbibliothek der Andrássy Universität 1. Missing filter
[FAIL] koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt
FAIL filters
missing_filter at line 115 ( <strong>Error</strong>: [% error %].)
2. You need to keep the HTML tag around the error, or the translator script will add TT tag to the translations and it will be messy.
3. t/db_dependent/Koha/ApiKey.t and t/db_dependent/Koha/Object.t are failing, but I am expecting more
4. I've added a test to ApiKey and the result is a bit unexpected:
# Failed test at t/db_dependent/Koha/ApiKey.t line 118.
# got: 'Invalid parameter passed, patron_id= does not exist'
# expected: 'Duplicate ID for key 'patron_id''
(In reply to Jonathan Druart from comment #3) > 4. I've added a test to ApiKey and the result is a bit unexpected: > > # Failed test at t/db_dependent/Koha/ApiKey.t line 118. > # got: 'Invalid parameter passed, patron_id= does not exist' > # expected: 'Duplicate ID for key 'patron_id'' Ha, it's actually the message added by your patch that is not accurate. 81 $msg = sprintf( "Invalid parameter passed, %s=%s does not exist", $self->broken_fk, $self->value ); Note that the test also raises a warning on this line: `Use of uninitialized value in sprintf at /kohadevbox/koha/Koha/Exceptions/Object.pm line 81.` (In reply to Jonathan Druart from comment #4) > (In reply to Jonathan Druart from comment #3) > > 4. I've added a test to ApiKey and the result is a bit unexpected: > > > > # Failed test at t/db_dependent/Koha/ApiKey.t line 118. > > # got: 'Invalid parameter passed, patron_id= does not exist' > > # expected: 'Duplicate ID for key 'patron_id'' > > Ha, it's actually the message added by your patch that is not accurate. > > 81 $msg = sprintf( "Invalid parameter passed, %s=%s does not > exist", $self->broken_fk, $self->value ); > > Note that the test also raises a warning on this line: > `Use of uninitialized value in sprintf at > /kohadevbox/koha/Koha/Exceptions/Object.pm line 81.` Sorry, wrong exception, I was looking at the other one and mixed both! Created attachment 192317 [details] [review] Bug 41696: Add a test Patch from commit ede59c5 Adjusted test, still failing
# Failed test at t/db_dependent/Koha/ApiKey.t line 118.
# got: 'Invalid parameter passed, patron_id= does not exist'
# expected: 'Invalid parameter passed, patron_id=162 does not exist'
I do think the change is correct, and that we should have more description in our exceptions (i.e. more with full_message). However the patch is still not ready for inclusion. |