[Bug 36368] New: Cannot save new patron after error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Bug ID: 36368 Summary: Cannot save new patron after error Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: emmi.takkinen@koha-suomi.fi QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com If an error occurs while adding new patron, after fixing the error and hitting save, patron entry page reloads to "Modify patron" section and error "Patron not found. Return to search" is displayed. But no patron is saved. To reproduce: 1. Add new patron and cause an error (wrong age etc.). 2. Attempt to save patron, error message is displayed. 3. Fix your errors and attempt to save again. => Error message "Patron not found. Return to search" is displayed and new patron is not saved to database. -- 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=36368 --- Comment #1 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- It seems that this stems from change made somewhere in bug 34478. But I'm unsure where from there. -- 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=36368 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #2 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- We declare param "op" too early in memberentry.pl line 166. This line
$template->param( "op" => $op ); was like this before bug 34478:
$template->param( "add" => 1 ) if ( $op eq 'add' );
Add this point, $op is still "cud-insert". So when error occurs, instead of displaying add form, we display modify form. I assume this is easily fixed just by moving line 166 further in code. -- 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=36368 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |emmi.takkinen@koha-suomi.fi |ity.org | -- 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=36368 --- Comment #3 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 163526 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163526&action=edit Bug 36368: Declare template param "op" after variable $op is set If an error occurs while adding new patron, after fixing the error and hitting save, patron entry page reloads to "Modify patron" section and error "Patron not found. Return to search" is displayed. But no patron is saved. This happens because we declare template param op too early in the code and it receives value "cud-insert" instead of "add_form" as it should. To test: 1. Add new patron and cause an error (wrong age etc.). 2. Attempt to save patron, error message is displayed. 3. Fix your errors and attempt to save again. => Error message "Patron not found. Return to search" is displayed and new patron is not saved to database. 4. Apply this patch. 5. Repeat steps from 1 to 3. => Saving patron should now work. => To be save, test if modifying patron also works as it should. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major --- Comment #4 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Setting bugs importance as major since this is quite nasty bug. Feel free to change it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Emmi Takkinen from comment #4)
Setting bugs importance as major since this is quite nasty bug. Feel free to change it.
I just did the same on another CSRF related bug, thanks for catching this one - certainly not an enh. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163526|0 |1 is obsolete| | --- Comment #6 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 163624 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163624&action=edit Works as described! Thank you for the patch. Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 --- Comment #7 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Created attachment 163625 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163625&action=edit Bug 36368: Declare template param "op" after variable $op is set If an error occurs while adding new patron, after fixing the error and hitting save, patron entry page reloads to "Modify patron" section and error "Patron not found. Return to search" is displayed. But no patron is saved. This happens because we declare template param op too early in the code and it receives value "cud-insert" instead of "add_form" as it should. To test: 1. Add new patron and cause an error (wrong age etc.). 2. Attempt to save patron, error message is displayed. 3. Fix your errors and attempt to save again. => Error message "Patron not found. Return to search" is displayed and new patron is not saved to database. 4. Apply this patch. 5. Repeat steps from 1 to 3. => Saving patron should now work. => To be save, test if modifying patron also works as it should. Sponsored-by: Koha-Suomi Oy Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Laura.escamilla@bywatersolu | |tions.com Attachment #163624|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=36368 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- $op = 'add_form' if ( $op eq 'cud-insert' ); $op = 'edit_form' if ( $op eq 'cud-save' ); %data=%newdata; $template->param( updtype => ($op eq 'add' ?'I':'M')); # used to check for $op eq "cud-insert"... but we just changed $op! Looks like we are not done here yet. See above. We are looking for 'add' but I see add_form ? FQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 --- Comment #9 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Marcel de Rooy from comment #8)
$op = 'add_form' if ( $op eq 'cud-insert' ); $op = 'edit_form' if ( $op eq 'cud-save' ); %data=%newdata; $template->param( updtype => ($op eq 'add' ?'I':'M')); # used to check for $op eq "cud-insert"... but we just changed $op!
Looks like we are not done here yet. See above. We are looking for 'add' but I see add_form ?
FQA
So it seems. But I'm wondering do we really need that line? Starting from line 628 we have two if statements. First is used to check if $op equals "add_form" and if that's true updtype is "I". And in line 636 we check if $op equals "edit_form" and set updtype as "M". So why do we set updtype param already in that $template->param( updtype => ($op eq 'add' ?'I':'M'));? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163625|0 |1 is obsolete| | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164111 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164111&action=edit Bug 36368: Declare template param "op" after variable $op is set If an error occurs while adding new patron, after fixing the error and hitting save, patron entry page reloads to "Modify patron" section and error "Patron not found. Return to search" is displayed. But no patron is saved. This happens because we declare template param op too early in the code and it receives value "cud-insert" instead of "add_form" as it should. To test: 1. Add new patron and cause an error (wrong age etc.). 2. Attempt to save patron, error message is displayed. 3. Fix your errors and attempt to save again. => Error message "Patron not found. Return to search" is displayed and new patron is not saved to database. 4. Apply this patch. 5. Repeat steps from 1 to 3. => Saving patron should now work. => To be save, test if modifying patron also works as it should. Sponsored-by: Koha-Suomi Oy Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> 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=36368 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164112 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164112&action=edit Bug 36368: (QA follow-up) Remove unneeded line As Emmy stated on comment9, this line is indeed unneeded (and wrong). A successful cud-insert/cud-save does not come here. And it is set for add_form, edit_form and duplicate later on. 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=36368 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |34478 CC| |fridolin.somers@biblibre.co | |m Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34478 [Bug 34478] Full CSRF protection -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 34478 not in 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36368 Bug 36368 depends on bug 34478, which changed state. Bug 34478 Summary: Full CSRF protection https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34478 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org