Bug 11917 - upload patron image can raise a software error
Summary: upload patron image can raise a software error
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-10 13:24 UTC by Jonathan Druart
Modified: 2014-12-07 20:02 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11917: upload image - catch error if cardnumber does not exist. (1.48 KB, patch)
2014-03-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11917: upload image - catch error if cardnumber does not exist. (1.52 KB, patch)
2014-03-11 08:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11917: upload image - catch error if cardnumber does not exist. (1.63 KB, patch)
2014-03-11 15:15 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-03-10 13:24:35 UTC

    
Comment 1 Jonathan Druart 2014-03-10 13:25:49 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2014-03-10 17:16:51 UTC
I'm not sure if I'm doing something wrong, but I get the same results before and after the patch. I see a warning box that says "Patron image failed to upload." In the table of results I see "ERROR: Image not imported because the database returned an error. Please refer to the error log for more details."

I'm not sure what this patch is supposed to change, but I wonder if we able to make the error message specific? Something about the card number not being found?
Comment 3 Jonathan Druart 2014-03-11 08:13:36 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-03-11 08:15:17 UTC
It appends when DEBUG=0.
Comment 5 Owen Leonard 2014-03-11 15:15:42 UTC
Created attachment 26117 [details] [review]
[SIGNED-OFF] Bug 11917: upload image - catch error if cardnumber does not exist.

On uploading a patron image, if the given cardnumber does not exist,
a software error is raised

DBD::mysql::st execute failed: Column 'borrowernumber' cannot be null at
/home/koha/src/C4/Members.pm line 1916.

Test plan:
SET DEBUG=1 in your apache configuration
go on tools/picture-upload.pl, choose an image and put a cardnumber
which does not exist in DB.
With the patch, you should get a friendly error message.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Marcel de Rooy 2014-03-13 13:18:37 UTC
QA Comment:
I appreciate the idea behind the patch, but do not really see the actual benefits of this code. Without DEBUG=1 (as we assume in normal operation), I do not see any difference.
I think that the routine in Members, PutPatronImage, should be addressed in this report. It combines the insert into patronimage with the select on cardnumber. It could check cardnumber first.
Note that if I see "There is a database error. See the log", I would rather see the error message right away. And the message is passed back by PutPatronImage.

Failed QA
Comment 7 Jonathan Druart 2014-03-13 13:30:32 UTC
(In reply to M. de Rooy from comment #6)

I opened this bug qaing another one. I didn't know that the issue only appeared for DEBUG=1.
I mark it as invalid, feel free to reopen it if you think it is useful.