To reproduce: - Go to a patron's detail page - Toolbar : More : Delete - Confirm "Are you sure you want to delete..." Result: Internal Server Error Plack error log: exited nonzero: 1 at /home/marc/koha/members/deletemem.pl
Created attachment 56055 [details] [review] Bug 17403: Internal Server Error while deleting patron This patch fixes a Internal Server Error while deleting patrons. To reproduce: - Go to a patron's detail page - Toolbar : More : Delete - Confirm "Are you sure you want to delete..." Result: Internal Server Error Plack error log: exited nonzero: 1 at /home/marc/koha/members/deletemem.pl Note: Patron is deleted To test: - Apply patch - Re-start plack - Try to reproduce steps above (with other patron) Expected result: No Internal Server Error, Redirect to Home > Patrons
Created attachment 56057 [details] [review] Bug 17403: Internal Server Error while deleting patron This patch fixes a Internal Server Error while deleting patrons. To reproduce: - Go to a patron's detail page - Toolbar : More : Delete - Confirm "Are you sure you want to delete..." Result: Internal Server Error Plack error log: exited nonzero: 1 at /home/marc/koha/members/deletemem.pl Note: Patron is deleted Additional tests: Try to delete yourself, to delete a staff member without having superlibrarian permission etc. To test: - Apply patch - Re-start plack - Try to reproduce steps above (with other patron) Expected result: No Internal Server Error, Redirect to Home > Patrons Amended to include all occurences of 'exit 1'
Created attachment 56067 [details] [review] Bug 17403: Internal Server Error while deleting patron This patch fixes a Internal Server Error while deleting patrons. To reproduce: - Go to a patron's detail page - Toolbar : More : Delete - Confirm "Are you sure you want to delete..." Result: Internal Server Error Plack error log: exited nonzero: 1 at /home/marc/koha/members/deletemem.pl Note: Patron is deleted Additional tests: Try to delete yourself, to delete a staff member without having superlibrarian permission etc. To test: - Apply patch - Re-start plack - Try to reproduce steps above (with other patron) Expected result: No Internal Server Error, Redirect to Home > Patrons Amended to include all occurences of 'exit 1' Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Good catch Marc
I can't reproduce the problem under Plack - patron got deleted just fine. What am I missing?
(In reply to Katrin Fischer from comment #4) > I can't reproduce the problem under Plack - patron got deleted just fine. > What am I missing? I tried again with scenario in initial comment and reproduced on current master. Plack error log says: exited nonzero: 1 /home/marc/members/deleteitem.pl exit 1 signals that there was an error in the script, but at this place we simply want to exit (without signaling an error). With patch applied deleting works fine.
Comment on attachment 56067 [details] [review] Bug 17403: Internal Server Error while deleting patron Review of attachment 56067 [details] [review]: ----------------------------------------------------------------- ::: members/deletemem.pl @@ -98,4 @@ > if ( !C4::Context->IsSuperLibrarian() && $bor->{'branchcode'}){ > unless ($userenv->{branch} eq $bor->{'branchcode'}){ > print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member&error=CANT_DELETE_OTHERLIBRARY"); > - exit; "exit" is the same as "exit 0", so this doesn't really matter, but I'm certainly in favour of a consistent approach. Why use "exit 0" over "exit" though? Just to be more explicit?
(In reply to David Cook from comment #6) > Comment on attachment 56067 [details] [review] [review] > Bug 17403: Internal Server Error while deleting patron > > Review of attachment 56067 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: members/deletemem.pl > @@ -98,4 @@ > > if ( !C4::Context->IsSuperLibrarian() && $bor->{'branchcode'}){ > > unless ($userenv->{branch} eq $bor->{'branchcode'}){ > > print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member&error=CANT_DELETE_OTHERLIBRARY"); > > - exit; > > "exit" is the same as "exit 0", so this doesn't really matter, but I'm > certainly in favour of a consistent approach. > > Why use "exit 0" over "exit" though? Just to be more explicit? Yes. I prefer it because it clearly shows the intention. BTW, it's used at many other places as well ( git grep 'exit 0' ).
Hah! ... my Plack was turned off, can reproduce the error now.
We checked the code base - we have both, exit and exit 0. At the moment, I don't consider this a blocker here. If someone wants to standardize on one writing, they can do so in a separate patch :)
Created attachment 56186 [details] [review] [PASSED QA] Bug 17403: Internal Server Error while deleting patron This patch fixes a Internal Server Error while deleting patrons. To reproduce: - Go to a patron's detail page - Toolbar : More : Delete - Confirm "Are you sure you want to delete..." Result: Internal Server Error Plack error log: exited nonzero: 1 at /home/marc/koha/members/deletemem.pl Note: Patron is deleted Additional tests: Try to delete yourself, to delete a staff member without having superlibrarian permission etc. To test: - Apply patch - Re-start plack - Try to reproduce steps above (with other patron) Expected result: No Internal Server Error, Redirect to Home > Patrons Amended to include all occurences of 'exit 1' Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Good catch Marc Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Pushed to Master - Should be in the November 16.11 release. Thanks!
Pushed in 16.05. Will be in 16.05.05.
Pushed to 3.22.x, will be in 3.22.12