[Bug 35685] New: ILL - OPAC request creation error if submitted empty while ILLModuleDisclaimerByType is in use
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Bug ID: 35685 Summary: ILL - OPAC request creation error if submitted empty while ILLModuleDisclaimerByType is in use Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: ILL Assignee: pedro.amorim@ptfs-europe.com Reporter: pedro.amorim@ptfs-europe.com CC: pedro.amorim@ptfs-europe.com Depends on: 33716 To reproduce: 1) Install FreeForm and enable ILLModule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Configure ILLModuleDisclaimerByType, visit: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType Click "edit" and paste the example code into the sys pref value 3) Create a new OPAC ILL request, visit: /cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm 4) Dont put a type or branch, click 'Create', on the next screen click 'Submit' 5) Notice it explodes 6) Apply patch and repeat. It no longer explodes and now shows an error message. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33716 [Bug 33716] ILL - Allow for a disclaimer stage per request type -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 --- Comment #1 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 160456 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160456&action=edit Bug 35685: Fix after creation actions Test plan: 1) Install FreeForm and enable ILLModule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Configure ILLModuleDisclaimerByType, visit: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType Click "edit" and paste the example code into the sys pref value 3) Create a new OPAC ILL request, visit: /cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm 4) Dont put a type or branch, click 'Create', on the next screen click 'Submit' 5) Notice it explodes 6) Apply patch and repeat. It no longer explodes and now shows an error message. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |magnus@libriotech.no, | |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 David Nind <david@davidnind.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=35685 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160456|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 160563 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160563&action=edit Bug 35685: Fix after creation actions Test plan: 1) Install FreeForm and enable ILLModule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Configure ILLModuleDisclaimerByType, visit: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType Click "edit" and paste the example code into the sys pref value 3) Create a new OPAC ILL request, visit: /cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm 4) Dont put a type or branch, click 'Create', on the next screen click 'Submit' 5) Notice it explodes 6) Apply patch and repeat. It no longer explodes and now shows an error message. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- This does fix the issue, and I have signed off. However, some comments - not sure how hard these are to do or whether it depends on the ILL plugin (I'm assuming this is the case): 1. If the "Destination library" is not selected, then the message at the top says: "Please Note: Branch is a mandatory field.". In my view the error should match the UI, that is, instead of "Branch" it should say "Destination library is required" 2. The normal UI for forms in the OPAC (from what I can see) is to have "Required" under the field name, and then a message if mandatory fields are not completed. I assume that this depends on how the ILL plugin works/coded though. Testing notes (using KTD): 1. Make the ILL request using the OPAC! . Go to the OPAC . Log in as a patron . Make the request using the "Interlibrary loan requests" tab/section -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to David Nind from comment #3)
This does fix the issue, and I have signed off.
However, some comments - not sure how hard these are to do or whether it depends on the ILL plugin (I'm assuming this is the case):
1. If the "Destination library" is not selected, then the message at the top says: "Please Note: Branch is a mandatory field.". In my view the error should match the UI, that is, instead of "Branch" it should say "Destination library is required"
2. The normal UI for forms in the OPAC (from what I can see) is to have "Required" under the field name, and then a message if mandatory fields are not completed. I assume that this depends on how the ILL plugin works/coded though.
Hi David, thanks for testing! Your assumptions are correct. The form and its behavior is dependent on the specific ILL backend which the request is being created to, in this example it's FreeForm. In bug 35570 I'm submitting work to add FreeForm to Koha core, to become a backend available out of the box instead of it being required to install an external one in order to make use of the ILL module. I mention bug 35570 because once that's gone through, these sort of enhancements can be worked directly into Koha, and have at least the Koha native backend be consistent with everything else. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 --- Comment #5 from David Nind <david@davidnind.com> --- (In reply to Pedro Amorim from comment #4) ...
Hi David, thanks for testing! Your assumptions are correct. The form and its behavior is dependent on the specific ILL backend which the request is being created to, in this example it's FreeForm.
In bug 35570 I'm submitting work to add FreeForm to Koha core, to become a backend available out of the box instead of it being required to install an external one in order to make use of the ILL module. I mention bug 35570 because once that's gone through, these sort of enhancements can be worked directly into Koha, and have at least the Koha native backend be consistent with everything else.
Thanks Pedro! I'll look at signing that one off over the next few days. I'll keep an eye out for other ILL bugs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- print $query->redirect('/cgi-bin/koha/opac-illrequests.pl?message=2'); # After creation actions if ( $params->{type_disclaimer_submitted} ) { $type_disclaimer->after_request_created( $params, $request ); print $query->redirect('/cgi-bin/koha/opac-illrequests.pl?message=2'); Possibly, two redirects in a row ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |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=35685 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160563|0 |1 is obsolete| | --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 161503 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161503&action=edit Bug 35685: Fix after creation actions Test plan: 1) Install FreeForm and enable ILLModule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Configure ILLModuleDisclaimerByType, visit: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType Click "edit" and paste the example code into the sys pref value 3) Create a new OPAC ILL request, visit: /cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm 4) Dont put a type or branch, click 'Create', on the next screen click 'Submit' 5) Notice it explodes 6) Apply patch and repeat. It no longer explodes and now shows an error message. 7) (After FQA): Create a normal request, input type and library, submit the type disclaimer 8) Run: echo "select * from illrequestattributes;" | koha-mysql kohadev 9) Notice type_disclaimer_date and type_disclaimer_value are correctly saved Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Marcel de Rooy from comment #6)
print $query->redirect('/cgi-bin/koha/opac-illrequests.pl?message=2');
# After creation actions if ( $params->{type_disclaimer_submitted} ) { $type_disclaimer->after_request_created( $params, $request ); print $query->redirect('/cgi-bin/koha/opac-illrequests.pl?message=2');
Possibly, two redirects in a row ?
Thank you Marcel, certainly not great code! Thanks for pointing it out. I've amended the original patch and updated the test plan to reinforce that there are no regressions in how the data is saved. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com, | |victor@tuxayo.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161503|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 164457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164457&action=edit Bug 35685: Fix after creation actions Test plan: 1) Install FreeForm and enable ILLModule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Configure ILLModuleDisclaimerByType, visit: /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType Click "edit" and paste the example code into the sys pref value 3) Create a new OPAC ILL request, visit: /cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm 4) Dont put a type or branch, click 'Create', on the next screen click 'Submit' 5) Notice it explodes 6) Apply patch and repeat. It no longer explodes and now shows an error message. 7) (After FQA): Create a normal request, input type and library, submit the type disclaimer 8) Run: echo "select * from illrequestattributes;" | koha-mysql kohadev 9) Notice type_disclaimer_date and type_disclaimer_value are correctly saved Signed-off-by: David Nind <david@davidnind.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=35685 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=35685 --- Comment #10 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=35685 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|24.05.00 |24.05.00,23.11.06 released in| | CC| |fridolin.somers@biblibre.co | |m --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED CC| |lucas@bywatersolutions.com Resolution|--- |FIXED --- Comment #12 from Lucas Gass <lucas@bywatersolutions.com> --- Missing 23.05.x dependencies, no backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35685 Bug 35685 depends on bug 33716, which changed state. Bug 33716 Summary: ILL - Allow for a disclaimer stage per request type https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33716 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org