[Bug 19420] New: Improve display of errors from failure of uploading file during stage import
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Bug ID: 19420 Summary: Improve display of errors from failure of uploading file during stage import Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Currently error is always: Upload status: Failed Though there are four reasons it could fail, they are codes buried in Koha/Uploader.pm -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 67685 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67685&action=edit Bug 19420: Improve display of errors from failure of uploading file during stage import To test: 1 - sudo chown www-data:www-data /tmp/koha_kohadev_upload/ (or your relevant dir and user) 2 - Attempt to stage a file 3 - Error message is: Upload status: Failed 4 - Apply patch 5 - Repeat upload 6 - Error is now: Failed - Directory is not writeable: /tmp/koha_kohadev_upload 7 - Trigger the other errors if you can and note better responses -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |katrin.fischer@bsz-bw.de --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think it would be nicer to deal with the codes in the templates to have them translatable. What do you think? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- That's fair, I can look into that, this was mostly just in response to having to chase down that code, may take a while before I get back here -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67685|0 |1 is obsolete| | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 68292 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68292&action=edit Bug 19420 - Improve display of errors from failure of uploading file during stage import To test: 1 - sudo chown www-data:www-data /tmp/koha_kohadev_upload/ (or your relevant dir and user) 2 - Attempt to stage a file 3 - Error message is: Upload status: Failed 4 - Apply patch 5 - Repeat upload 6 - Error is now: Failed - Directory is not writeable: /tmp/koha_kohadev_upload 7 - Trigger the other errors if you can and note better responses -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68292|0 |1 is obsolete| | --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 68365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68365&action=edit Bug 19420 - Improve display of errors from failure of uploading file during stage import To test: 1 - sudo chown www-data:www-data /tmp/koha_kohadev_upload/ (or your relevant dir and user) 2 - Attempt to stage a file 3 - Error message is: Upload status: Failed 4 - Apply patch 5 - Repeat upload 6 - Error is now: Failed - Directory is not writeable: /tmp/koha_kohadev_upload 7 - Trigger the other errors if you can and note better responses Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #0)
Currently error is always: Upload status: Failed Though there are four reasons it could fail, they are codes buried in Koha/Uploader.pm
Who wrote that ?! ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- t/db_dependent/Upload.t not ok 3 - Errcode 1 [already exists] reported # Failed test 'Errcode 1 [already exists] reported' # at t/db_dependent/Upload.t line 148. # got: 'HASH(0x7071fe8)' # expected: '1' # Looks like you failed 1 test of 3. not ok 6 - Add same file in same category # Failed test 'Add same file in same category' # at t/db_dependent/Upload.t line 149. Will have a look now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68909&action=edit Bug 19420: (QA Follow-up) Fix test, correct POD The test needs the code level in the error hash now. And POD is adjusted accordingly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- We need similar changes in other places: git grep "file-upload.js" koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ/process_koc.tt:<script type="text/javascript" src="[% interface %]/[% theme %]/js/file-upload.js"></script> koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt:<script type="text/javascript" src="[% interface %]/[% theme %]/js/file-upload.js"></script> koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt:<script type="text/javascript" src="[% interface %]/[% theme %]/js/file-upload.js"></script> koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload.tt:<script type="text/javascript" src="[% interface %]/[% theme %]/js/file-upload.js"></script> Please adjust -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 68910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68910&action=edit Bug 19420 - Add errors on other pages Eventually these should all be using the same code in a js file upload.tt already dealt with these errors, but has diff code, made it work with new error syntax -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Upload local cover image Upload status: Failed - (2) Root directory for uploads not defined No, the directory was not writable. This went wrong since the errcode is 2, but you forgot to add a zero entry in you errMsgs array. So they are one position out of sync. Please add a leading entry here (see also upload.tt) var errMsgs = [ _("File already exists"), _("Directory is not writeable"), _("Root directory for uploads not defined"), _("Temporary directory for uploads not defined") ]; Adjust same thing in two other places ? //Next three errors have no code path to trigger that I see I am very confident that I tested all codes when I wrote this. They may be hard to simulate, but I did. Please remove this comment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68910|0 |1 is obsolete| | --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 68941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68941&action=edit Bug 19420 - Add errors on other pages Fix error array (add '0' value) Update errors on other places using upload.js Eventually these should all be using the same code in a js file upload.tt already dealt with these errors, but has diff code, made it work with new error syntax -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #11)
Upload local cover image Upload status: Failed - (2) Root directory for uploads not defined No, the directory was not writable. This went wrong since the errcode is 2, but you forgot to add a zero entry in you errMsgs array. So they are one position out of sync. Please add a leading entry here (see also upload.tt) var errMsgs = [ _("File already exists"), _("Directory is not writeable"), _("Root directory for uploads not defined"), _("Temporary directory for uploads not defined") ]; Adjust same thing in two other places ? Adjusted :-)
//Next three errors have no code path to trigger that I see I am very confident that I tested all codes when I wrote this. They may be hard to simulate, but I did. Please remove this comment.
Removed, I though we only passed 1-4 back in the pm file, my apologies -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here again. I still found an issue. Please wait.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #13)
Removed, I though we only passed 1-4 back in the pm file, my apologies
Ah. See the last follow-up. upload.pl passed these codes. As you already said, this still needs some improvement. I crossed a TODO also. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68960&action=edit Bug 19420: Improve display of errors from failure of uploading file during stage import To test: 1 - sudo chown www-data:www-data /tmp/koha_kohadev_upload/ (or your relevant dir and user) 2 - Attempt to stage a file 3 - Error message is: Upload status: Failed 4 - Apply patch 5 - Repeat upload 6 - Error is now: Failed - Directory is not writeable: /tmp/koha_kohadev_upload 7 - Trigger the other errors if you can and note better responses Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68909|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68961&action=edit Bug 19420: (QA Follow-up) Fix test, correct POD The test needs the code level in the error hash now. And POD is adjusted accordingly. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68962&action=edit Bug 19420: (Follow-up) Add errors on other pages Fix error array (add '0' value) Update errors on other places using upload.js Eventually these should all be using the same code in a js file upload.tt already dealt with these errors, but has diff code, made it work with new error syntax Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Amended: Removed the added js comment in upload.tt -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68963&action=edit Bug 19420: (QA Follow-up) Change in error reporting affects upload.pl The change from the first patch requires a similar change in upload.pl when a few error codes are passed to the template. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested by deleting a temporary file manually and downloading or deleting the file from the upload search results. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68365|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68941|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #13)
(In reply to Marcel de Rooy from comment #11)
Upload local cover image Upload status: Failed - (2) Root directory for uploads not defined No, the directory was not writable. This went wrong since the errcode is 2, but you forgot to add a zero entry in you errMsgs array. So they are one position out of sync. Please add a leading entry here (see also upload.tt) var errMsgs = [ _("File already exists"), _("Directory is not writeable"), _("Root directory for uploads not defined"), _("Temporary directory for uploads not defined") ]; Adjust same thing in two other places ? Adjusted :-)
Works now. Thanks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- errcode should be "already_exists", "no_tmp_dir", etc. No 0, 1, 2, ... This is a terrible and error prone pattern. Not blocker as it existed before this patch set. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 17.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19420 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19633 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org