Created attachment 26005 [details] [review] 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: 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.
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?
Created attachment 26046 [details] [review] 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.
It appends when DEBUG=0.
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>
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
(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.