Description
Pedro Amorim
2024-02-28 16:44:38 UTC
This is ready for testing. Branch at: https://github.com/PTFS-Europe/koha/commits/b_36197 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Backends.t prove t/db_dependent/selenium/opac_ill_requests.t prove t/lib/plugins/Koha/Plugin/ILL/TestPlugin.pm Sponsored-by: NHS England (National Health Service England) This is ready for testing. Branch at: https://github.com/PTFS-Europe/koha/commits/b_36197 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Backends.t prove t/db_dependent/selenium/opac_ill_requests.t Sponsored-by: NHS England (National Health Service England) I had a go at testing. I haven't signed off yet, mainly because of the error noted in note 5. Notes from testing (using KTD): 1. Step 8: When you submit the request for type 'Book' (when unauthenticated) and visit the link, you go to a newly created record. This is discoverable by searching. None of the other types creates a record. 2. Additional testing - step 3: I only tested with ILLModuleDisclaimerByType. I don't get a type of 'article', but I think that is the code for 'Journal article'? I get Book, Chapter, Journal, Journal article, Thesis, Conference, DVD, Other, Generic resource. 3. On the input form, compulsory fields have an asterisk after the field label, but nothing to say that these are mandatory (that is, an explanation for the *). When you submit without completing a mandatory field, a plain text message appears at the top. This is different from the "normal" way of presenting forms in the OPAC, but I think you are aware of that 8-). 4. Before or after submitting the form when unauthenticated, the breadcrumb link ('Interlibrary loan requests') comes up with the 'Login to your account' form. 5. If request type is DVD, I get an error trace: Template process failed: file error - /kohadevbox/koha/Koha/ILL/Backend/shared-includes/forms/dvd.inc: not found at /kohadevbox/koha/C4/Templates.pm line 134 in C4::Templates::output at /kohadevbox/koha/C4/Templates.pm line 134 .... 6. If the request type is Other, there are no additional fields to complete (you can add custom fields). 7. Tests pass. Note: need to start up KTD with Selenium (ktd --selenium up). Hi David, great testing as usual thank you so much. 1) Good point. This is default behavior (for book type) that precedes this work. I'm open to suggestions but have kept this unchanged for now. 2) Correct, 'article' is the code for 'Journal article' 3) Fair points about the form / required fields. As before, I'm open to suggestions but this is default behavior that precedes this work so I've not addressed this here. 4) Thanks for this, will look for providing a follow-up for it. 5) The DVD error, I believe is unrelated to this development and comes from the original commit that added it: https://github.com/PTFS-Europe/koha-ill-freeform/commit/9d735a2dce267d67b4396a0a33d7cb9521a1dfbe I believe a required shared-includes/forms/dvd.inc is missing here. I've added Lucas in case he wishes/is available to provide a follow-up here. 6) For the 'other' request type, having no input fields is expected I believe as there's no way what 'other' would have as input hence only considering the custom fields. (In reply to Pedro Amorim from comment #4) > Hi David, great testing as usual thank you so much. > 1) Good point. This is default behavior (for book type) that precedes this > work. I'm open to suggestions but have kept this unchanged for now. > 2) Correct, 'article' is the code for 'Journal article' > 3) Fair points about the form / required fields. As before, I'm open to > suggestions but this is default behavior that precedes this work so I've not > addressed this here. > 4) Thanks for this, will look for providing a follow-up for it. > 5) The DVD error, I believe is unrelated to this development and comes from > the original commit that added it: > https://github.com/PTFS-Europe/koha-ill-freeform/commit/ > 9d735a2dce267d67b4396a0a33d7cb9521a1dfbe > I believe a required shared-includes/forms/dvd.inc is missing here. I've > added Lucas in case he wishes/is available to provide a follow-up here. > 6) For the 'other' request type, having no input fields is expected I > believe as there's no way what 'other' would have as input hence only > considering the custom fields. Thanks Pedro! Please add my sign off to the bug. Signed-off-by: David Nind <david@davidnind.com> Created attachment 171358 [details] [review] Bug 36197: Add ILLOpacUnauthenticatedRequest sys pref Signed-off-by: David Nind <david@davidnind.com> Created attachment 171359 [details] [review] Bug 36197: Add opac_unauthenticated_ill_requests backend capability to 'Standard' backend Signed-off-by: David Nind <david@davidnind.com> Created attachment 171360 [details] [review] Bug 36197: Consider unauthenticated_ill in 'Standard' create form This also makes sure an empty left side bar exists for opac unauthenticated screens Signed-off-by: David Nind <david@davidnind.com> Created attachment 171361 [details] [review] Bug 36197: Refactor -> opac_available_backends method Signed-off-by: David Nind <david@davidnind.com> Created attachment 171362 [details] [review] Bug 36197: Add UNAUTH Standard request status Assign this new UNAUTH status for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 171363 [details] [review] Bug 36197: Update how form params are validated Validating an unauth request form requires checking for unauth info fields. Validating a regular authenticated request requires checking for borrower information. Signed-off-by: David Nind <david@davidnind.com> Created attachment 171364 [details] [review] Bug 36197: Update how the request is added: - Do not validate borrower if its an unauthenticated request - Set borrowernumber to null if its an unauthenticated request - Add staff and OPAC notes with unauthenticated request info Signed-off-by: David Nind <david@davidnind.com> Created attachment 171365 [details] [review] Bug 36197: Only check limits for regular requests Skip for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 171366 [details] [review] Bug 36197: OPAC unauthenticated screen unauth_view to show request details after its been submitted Signed-off-by: David Nind <david@davidnind.com> Created attachment 171367 [details] [review] Bug 36197: Make authentication not required if conditions are met /cgi-bin/koha/opac-illrequests.pl was an authrequired page Now, its authnotrequired if creating or viewing an unauthenticated request Signed-off-by: David Nind <david@davidnind.com> Created attachment 171368 [details] [review] Bug 36197: Unauthenticated patron can always place ILL request if ILLOpacUnauthenticatedRequest Signed-off-by: David Nind <david@davidnind.com> Created attachment 171369 [details] [review] Bug 36197: Only add cardnumber info to request is a user is logged in Signed-off-by: David Nind <david@davidnind.com> Created attachment 171370 [details] [review] Bug 36197: OPAC 'Make an ILL request' link Show the 'Make an ILL request' link for unauthenticated users in the OPAC search results page if ILLOpacUnauthenticatedRequest is enabled. Signed-off-by: David Nind <david@davidnind.com> Created attachment 171371 [details] [review] Bug 36197: Add tests 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Backends.t prove t/db_dependent/selenium/opac_ill_requests.t Sponsored-by: NHS England (National Health Service England) Signed-off-by: David Nind <david@davidnind.com> As Matthias mentioned during the Kohacon ILL workshop, there's nothing preventing bots from abusing this at the moment. But it would be similar to anonymous/unauthenticated purchase suggestions with AnonSuggestions enabled. -- That said, perhaps a magic token for EDS to include in the generated URL or a referrer check could be useful to reduce robot spam (which is an existing problem with AnonSuggestions currently). (In reply to David Cook from comment #21) > As Matthias mentioned during the Kohacon ILL workshop, there's nothing > preventing bots from abusing this at the moment. > > But it would be similar to anonymous/unauthenticated purchase suggestions > with AnonSuggestions enabled. > > -- > > That said, perhaps a magic token for EDS to include in the generated URL or > a referrer check could be useful to reduce robot spam (which is an existing > problem with AnonSuggestions currently). There is already an existing captcha system present in opac-memberentry.pl (for patron self registration). Would it make more sense to make use of this here by re utilizing it (doing any necessary refactoring), thus having a single place to extend and maintain going forward and perhaps establish a pattern for public facing forms in Koha, e.g. also apply it to unauthenticated purchase suggestions in the future? (In reply to Pedro Amorim from comment #22) > There is already an existing captcha system present in opac-memberentry.pl > (for patron self registration). > Would it make more sense to make use of this here by re utilizing it (doing > any necessary refactoring), thus having a single place to extend and > maintain going forward and perhaps establish a pattern for public facing > forms in Koha, e.g. also apply it to unauthenticated purchase suggestions in > the future? Hmm there's a thought. That would potentially help against bots. Off the top of my head, I don't think I've heard complaints of bots doing self-registrations. I just remembered that somewhere had a negative captcha, and it looks like opac-suggestions.tt is the one implementing that, but I think most bots can easily bypass that one these days. I think I still feel a bit weird about unauthenticated ILLs, but I suppose they're not really that different to unauthenticated purchase suggestions, although I don't love those either. But then in both cases they're staff mediated anyway... so I suppose that's all right. But yeah... some kind of capture is needed I think. Created attachment 174495 [details] [review] Bug 36197: Add ILLOpacUnauthenticatedRequest sys pref Signed-off-by: David Nind <david@davidnind.com> Created attachment 174496 [details] [review] Bug 36197: Add opac_unauthenticated_ill_requests backend capability to 'Standard' backend Signed-off-by: David Nind <david@davidnind.com> Created attachment 174497 [details] [review] Bug 36197: Consider unauthenticated_ill in 'Standard' create form This also makes sure an empty left side bar exists for opac unauthenticated screens Signed-off-by: David Nind <david@davidnind.com> Created attachment 174498 [details] [review] Bug 36197: Refactor -> opac_available_backends method Signed-off-by: David Nind <david@davidnind.com> Created attachment 174499 [details] [review] Bug 36197: Add UNAUTH Standard request status Assign this new UNAUTH status for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 174500 [details] [review] Bug 36197: Update how form params are validated Validating an unauth request form requires checking for unauth info fields. Validating a regular authenticated request requires checking for borrower information. Signed-off-by: David Nind <david@davidnind.com> Created attachment 174501 [details] [review] Bug 36197: Update how the request is added: - Do not validate borrower if its an unauthenticated request - Set borrowernumber to null if its an unauthenticated request - Add staff and OPAC notes with unauthenticated request info Signed-off-by: David Nind <david@davidnind.com> Created attachment 174502 [details] [review] Bug 36197: Only check limits for regular requests Skip for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 174503 [details] [review] Bug 36197: OPAC unauthenticated screen unauth_view to show request details after its been submitted Signed-off-by: David Nind <david@davidnind.com> Created attachment 174504 [details] [review] Bug 36197: Make authentication not required if conditions are met /cgi-bin/koha/opac-illrequests.pl was an authrequired page Now, its authnotrequired if creating or viewing an unauthenticated request Signed-off-by: David Nind <david@davidnind.com> Created attachment 174505 [details] [review] Bug 36197: Unauthenticated patron can always place ILL request if ILLOpacUnauthenticatedRequest Signed-off-by: David Nind <david@davidnind.com> Created attachment 174506 [details] [review] Bug 36197: Only add cardnumber info to request is a user is logged in Signed-off-by: David Nind <david@davidnind.com> Created attachment 174507 [details] [review] Bug 36197: OPAC 'Make an ILL request' link Show the 'Make an ILL request' link for unauthenticated users in the OPAC search results page if ILLOpacUnauthenticatedRequest is enabled. Signed-off-by: David Nind <david@davidnind.com> Created attachment 174508 [details] [review] Bug 36197: Add tests 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Backends.t prove t/db_dependent/selenium/opac_ill_requests.t Sponsored-by: NHS England (National Health Service England) Signed-off-by: David Nind <david@davidnind.com> Created attachment 176564 [details] [review] Bug 36197: (QA follow-up): Fix testcritic.t prove t/00-testcritic.t Created attachment 176565 [details] [review] Bug 36197: (QA follow-up): Fix PluginRoutes.t prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t Created attachment 176680 [details] [review] Bug 36197: (QA follow-up): Make ConfirmAuto::get_priority_backends respect opac_available_backends if OPAC This makes it so that the Auto ILL stage on OPAC only queries opac_available_backends. Fix is submitted here and not in bug 35604 because it's here that opac_available_backends is implemented. Created attachment 176723 [details] [review] Bug 36197: (QA follow-up): Fix backends filtering in opac_available_backends This was incorrectly keeping backends that don't support unauthenticated ILL requests if more than one existed. Created attachment 176724 [details] [review] Bug 36197: (QA follow-up): Pass the filtered OPAC backends to OPAC template The list of backends in the dropdown in this page: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?op=create If unauthenticated is enabled, this is listing all backends, instead of making use of opac_available_backends. Created attachment 176726 [details] [review] Bug 36197: (QA follow-up): Move unauthenticated_notes to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Also make these notes translatable. Created attachment 176727 [details] [review] Bug 36197: (QA follow-up): Move unauth_request_data_check to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Created attachment 179764 [details] [review] Bug 36197: Add ILLOpacUnauthenticatedRequest sys pref Signed-off-by: David Nind <david@davidnind.com> Created attachment 179765 [details] [review] Bug 36197: Add opac_unauthenticated_ill_requests backend capability to 'Standard' backend Signed-off-by: David Nind <david@davidnind.com> Created attachment 179766 [details] [review] Bug 36197: Consider unauthenticated_ill in 'Standard' create form This also makes sure an empty left side bar exists for opac unauthenticated screens Signed-off-by: David Nind <david@davidnind.com> Created attachment 179767 [details] [review] Bug 36197: Refactor -> opac_available_backends method Signed-off-by: David Nind <david@davidnind.com> Created attachment 179769 [details] [review] Bug 36197: Add UNAUTH Standard request status Assign this new UNAUTH status for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 179770 [details] [review] Bug 36197: Update how form params are validated Validating an unauth request form requires checking for unauth info fields. Validating a regular authenticated request requires checking for borrower information. Signed-off-by: David Nind <david@davidnind.com> Created attachment 179771 [details] [review] Bug 36197: Update how the request is added: - Do not validate borrower if its an unauthenticated request - Set borrowernumber to null if its an unauthenticated request - Add staff and OPAC notes with unauthenticated request info Signed-off-by: David Nind <david@davidnind.com> Created attachment 179772 [details] [review] Bug 36197: Only check limits for regular requests Skip for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 179773 [details] [review] Bug 36197: OPAC unauthenticated screen unauth_view to show request details after its been submitted Signed-off-by: David Nind <david@davidnind.com> Created attachment 179774 [details] [review] Bug 36197: Make authentication not required if conditions are met /cgi-bin/koha/opac-illrequests.pl was an authrequired page Now, its authnotrequired if creating or viewing an unauthenticated request Signed-off-by: David Nind <david@davidnind.com> Created attachment 179775 [details] [review] Bug 36197: Unauthenticated patron can always place ILL request if ILLOpacUnauthenticatedRequest Signed-off-by: David Nind <david@davidnind.com> Created attachment 179776 [details] [review] Bug 36197: Only add cardnumber info to request is a user is logged in Signed-off-by: David Nind <david@davidnind.com> Created attachment 179777 [details] [review] Bug 36197: OPAC 'Make an ILL request' link Show the 'Make an ILL request' link for unauthenticated users in the OPAC search results page if ILLOpacUnauthenticatedRequest is enabled. Signed-off-by: David Nind <david@davidnind.com> Created attachment 179778 [details] [review] Bug 36197: Add tests 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Backends.t prove t/db_dependent/selenium/opac_ill_requests.t Sponsored-by: NHS England (National Health Service England) Signed-off-by: David Nind <david@davidnind.com> Created attachment 179779 [details] [review] Bug 36197: (QA follow-up): Fix testcritic.t prove t/00-testcritic.t Created attachment 179780 [details] [review] Bug 36197: (QA follow-up): Fix PluginRoutes.t prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t Created attachment 179781 [details] [review] Bug 36197: (QA follow-up): Make ConfirmAuto::get_priority_backends respect opac_available_backends if OPAC This makes it so that the Auto ILL stage on OPAC only queries opac_available_backends. Fix is submitted here and not in bug 35604 because it's here that opac_available_backends is implemented. Created attachment 179782 [details] [review] Bug 36197: (QA follow-up): Fix backends filtering in opac_available_backends This was incorrectly keeping backends that don't support unauthenticated ILL requests if more than one existed. Created attachment 179783 [details] [review] Bug 36197: (QA follow-up): Pass the filtered OPAC backends to OPAC template The list of backends in the dropdown in this page: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?op=create If unauthenticated is enabled, this is listing all backends, instead of making use of opac_available_backends. Created attachment 179784 [details] [review] Bug 36197: (QA follow-up): Move unauthenticated_notes to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Also make these notes translatable. Created attachment 179785 [details] [review] Bug 36197: (QA follow-up): Move unauth_request_data_check to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Rebased and tidied on top of tidied bug 35604 TODO's from Hackfest's QA session: 1) Use "We'll contact you shortly" instead of "Check your email" after an unauthenticated request has been placed 2) unauthenticated fields should use their own respective illrequestattributes, removed from notes and staffnotes 3) update request page template to list all 3 unauthenticated fields 4) Update history check request matches from OPAC with Staff UI 5) Properly format placed date with Koha date tt filter for history check 6) Request history check request details branchcode, cardnumber backend need to be capitalized and properly named TODO's from Hackfest's QA session: 1) Use "We'll contact you shortly" instead of "Check your email" after an unauthenticated request has been placed 2) unauthenticated fields should use their own respective illrequestattributes, removed from notes and staffnotes 3) update request page template to list all 3 unauthenticated fields 4) Update history check request matches from OPAC with Staff UI TODO's from Hackfest's QA session: 1) Use "We'll contact you shortly" instead of "Check your email" after an unauthenticated request has been placed 2) unauthenticated fields should use their own respective illrequestattributes, removed from notes and staffnotes 3) update request page template to list all 3 unauthenticated fields Created attachment 180850 [details] [review] Bug 36197: Add ILLOpacUnauthenticatedRequest sys pref Signed-off-by: David Nind <david@davidnind.com> Created attachment 180851 [details] [review] Bug 36197: Add opac_unauthenticated_ill_requests backend capability to 'Standard' backend Signed-off-by: David Nind <david@davidnind.com> Created attachment 180852 [details] [review] Bug 36197: Consider unauthenticated_ill in 'Standard' create form This also makes sure an empty left side bar exists for opac unauthenticated screens Signed-off-by: David Nind <david@davidnind.com> Created attachment 180853 [details] [review] Bug 36197: Refactor -> opac_available_backends method Signed-off-by: David Nind <david@davidnind.com> Created attachment 180854 [details] [review] Bug 36197: Add UNAUTH Standard request status Assign this new UNAUTH status for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 180855 [details] [review] Bug 36197: Update how form params are validated Validating an unauth request form requires checking for unauth info fields. Validating a regular authenticated request requires checking for borrower information. Signed-off-by: David Nind <david@davidnind.com> Created attachment 180856 [details] [review] Bug 36197: Update how the request is added: - Do not validate borrower if its an unauthenticated request - Set borrowernumber to null if its an unauthenticated request - Add staff and OPAC notes with unauthenticated request info Signed-off-by: David Nind <david@davidnind.com> Created attachment 180857 [details] [review] Bug 36197: Only check limits for regular requests Skip for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Created attachment 180858 [details] [review] Bug 36197: OPAC unauthenticated screen unauth_view to show request details after its been submitted Signed-off-by: David Nind <david@davidnind.com> Created attachment 180859 [details] [review] Bug 36197: Make authentication not required if conditions are met /cgi-bin/koha/opac-illrequests.pl was an authrequired page Now, its authnotrequired if creating or viewing an unauthenticated request Signed-off-by: David Nind <david@davidnind.com> Created attachment 180860 [details] [review] Bug 36197: Unauthenticated patron can always place ILL request if ILLOpacUnauthenticatedRequest Signed-off-by: David Nind <david@davidnind.com> Created attachment 180861 [details] [review] Bug 36197: Only add cardnumber info to request is a user is logged in Signed-off-by: David Nind <david@davidnind.com> Created attachment 180862 [details] [review] Bug 36197: OPAC 'Make an ILL request' link Show the 'Make an ILL request' link for unauthenticated users in the OPAC search results page if ILLOpacUnauthenticatedRequest is enabled. Signed-off-by: David Nind <david@davidnind.com> Created attachment 180863 [details] [review] Bug 36197: Add tests 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Request/Config.t prove t/db_dependent/selenium/opac_ill_requests.t Sponsored-by: NHS England (National Health Service England) Signed-off-by: David Nind <david@davidnind.com> Created attachment 180864 [details] [review] Bug 36197: (QA follow-up): Fix testcritic.t prove t/00-testcritic.t Created attachment 180865 [details] [review] Bug 36197: (QA follow-up): Fix PluginRoutes.t prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t Created attachment 180866 [details] [review] Bug 36197: (QA follow-up): Make ConfirmAuto::get_priority_backends respect opac_available_backends if OPAC This makes it so that the Auto ILL stage on OPAC only queries opac_available_backends. Fix is submitted here and not in bug 35604 because it's here that opac_available_backends is implemented. Created attachment 180867 [details] [review] Bug 36197: (QA follow-up): Fix backends filtering in opac_available_backends This was incorrectly keeping backends that don't support unauthenticated ILL requests if more than one existed. Created attachment 180868 [details] [review] Bug 36197: (QA follow-up): Pass the filtered OPAC backends to OPAC template The list of backends in the dropdown in this page: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?op=create If unauthenticated is enabled, this is listing all backends, instead of making use of opac_available_backends. Created attachment 180869 [details] [review] Bug 36197: (QA follow-up): Move unauthenticated_notes to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Also make these notes translatable. Created attachment 180870 [details] [review] Bug 36197: (QA follow-up): Move unauth_request_data_check to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Created attachment 180871 [details] [review] Bug 36197: (QA follow-up): Hackfest followup WIP Moved unauth data to their respective illrequestattributes entries instead of staffnotes + opacnotes. Update confirmation message Created attachment 180872 [details] [review] Bug 36197: (QA follow-up): Add public form captcha Feedback provided by David Cook + Matthias Meusburger at KohaCon24 Created attachment 180873 [details] [review] Bug 36197: (QA follow-up): Adopt POST REDIRECT GET pattern Upon submitting the unauthenticated request, the user is now redirected to a different page and the request details are passed through a session variable. This is to prevent the user from being able to resubmit the same unauthenticated request multiple times (see security bug 39611) Hi guys, this last submission adds the following changes: 1) Rebase on top of QA changes made by Tomas to bug 35604, including moving the opac_available_backends method from the no longer existent Backends.pm file into ILL::Request::Config instead 2) Hackfest QA feedback addressed 3) Added a new captcha to the public facing form 4) Upon revisiting and testing further, I discovered the user was able to resubmit the same form multiple times, so I've addressed that in the last patch. I've also found this behavior somewhere else in Koha and have filed a security bug for that. Created attachment 181073 [details] [review] Bug 36197: Add ILLOpacUnauthenticatedRequest sys pref Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181074 [details] [review] Bug 36197: Add opac_unauthenticated_ill_requests backend capability to 'Standard' backend Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181075 [details] [review] Bug 36197: Consider unauthenticated_ill in 'Standard' create form This also makes sure an empty left side bar exists for opac unauthenticated screens Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181076 [details] [review] Bug 36197: Refactor -> opac_available_backends method Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181077 [details] [review] Bug 36197: Add UNAUTH Standard request status Assign this new UNAUTH status for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181078 [details] [review] Bug 36197: Update how form params are validated Validating an unauth request form requires checking for unauth info fields. Validating a regular authenticated request requires checking for borrower information. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181079 [details] [review] Bug 36197: Update how the request is added: - Do not validate borrower if its an unauthenticated request - Set borrowernumber to null if its an unauthenticated request - Add staff and OPAC notes with unauthenticated request info Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181080 [details] [review] Bug 36197: Only check limits for regular requests Skip for unauthenticated requests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181081 [details] [review] Bug 36197: OPAC unauthenticated screen unauth_view to show request details after its been submitted Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181082 [details] [review] Bug 36197: Make authentication not required if conditions are met /cgi-bin/koha/opac-illrequests.pl was an authrequired page Now, its authnotrequired if creating or viewing an unauthenticated request Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181083 [details] [review] Bug 36197: Unauthenticated patron can always place ILL request if ILLOpacUnauthenticatedRequest Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181084 [details] [review] Bug 36197: Only add cardnumber info to request is a user is logged in Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181085 [details] [review] Bug 36197: OPAC 'Make an ILL request' link Show the 'Make an ILL request' link for unauthenticated users in the OPAC search results page if ILLOpacUnauthenticatedRequest is enabled. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181086 [details] [review] Bug 36197: Add tests 0) If k-t-d, run updatedatabase (ignore this if testing on a sandbox) 1) Enable ILLModule syspref: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModule 2) No need to install a backend as 35570 is in the tree 3) On a different browser session, without being lgoged in, access the create ILL request OPAC page directly: {opac_url}/cgi-bin/koha/opac-illrequests.pl?method=create&backend=Standard 4) Notice you're presented with the login page, as expected 5) Enable ILLOpacUnauthenticatedRequest: {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLOpacUnauthenticatedRequest 6) Repeat step 3) Notice you now get the create ILL request form, with some additional inputs at the top specific to an unauthenticated request (first name, last name, e-mail address) 6.5) (optional) Confirm the 'Make an Interlibrary loan request' link exists at the bottom of the search results page, while unauthenticated on OPAC: {opac_url}/cgi-bin/koha/opac-search.pl?idx=&q=music&weight_search=1 This should only show for authenticated OPAC users if ILLOpacUnauthenticatedRequest is off. 7) Test the form, try submitting with some blank fields, change type and library after and before inputing data in the input fields, etc. 8) Submit the form successfully. Notice you're presented with the unauthenticated request submission confirmation screen with a message and request details. 9) Go back to Staff UI, logged in, access ILL requests and confirm the unauthenticated request submitted through the OPAC is there: {staff_url}/cgi-bin/koha/ill/ill-requests.pl 10) Test that you can add a patron afterwards, if needed, by clicking 'Edit request' on the toolbar of the manage request page, and entering a patron ID 11) Test that you're able to 'confirm' the request by clicking 'Confirm request', as if it was a regular 'NEW' request. Additional testing: Make sure existing ILL functionality interacts with this new optional workflow behavior. For this, we're using ILLModuleDisclaimerByType. ILLCheckAvailability or AutoILLBackendPriority can also be used but require more setup (plugins, credentials, etc) 1) Configure ILLModuleDisclaimerByType, visit {staff_url}/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=ILLModuleDisclaimerByType 2) Click the 'edit' link at the bottom of the syspref and copy the example code from the description into the text box. Save. 3) Repeat the original test plan from above, but make sure the request type is not 'article' (as that type is bypassed). Use 'journal', for example. Submit the request making sure you fill in all mandatory fields. 4) Notice you're now presented with the ILL request disclaimer screen, hit Submit. Notice the request is created as before. 5) Return to Staff UI, logged in, confirm this 2nd unauthenticated request has been created and the type disclaimer info is stored and presented when viewing the request. Run tests (k-t-d only): prove t/db_dependent/Koha/ILL/Request/Config.t prove t/db_dependent/selenium/opac_ill_requests.t Sponsored-by: NHS England (National Health Service England) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181087 [details] [review] Bug 36197: (QA follow-up): Fix testcritic.t prove t/00-testcritic.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181088 [details] [review] Bug 36197: (QA follow-up): Fix PluginRoutes.t prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181089 [details] [review] Bug 36197: (QA follow-up): Make ConfirmAuto::get_priority_backends respect opac_available_backends if OPAC This makes it so that the Auto ILL stage on OPAC only queries opac_available_backends. Fix is submitted here and not in bug 35604 because it's here that opac_available_backends is implemented. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181090 [details] [review] Bug 36197: (QA follow-up): Fix backends filtering in opac_available_backends This was incorrectly keeping backends that don't support unauthenticated ILL requests if more than one existed. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181091 [details] [review] Bug 36197: (QA follow-up): Pass the filtered OPAC backends to OPAC template The list of backends in the dropdown in this page: http://localhost:8080/cgi-bin/koha/opac-illrequests.pl?op=create If unauthenticated is enabled, this is listing all backends, instead of making use of opac_available_backends. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181092 [details] [review] Bug 36197: (QA follow-up): Move unauthenticated_notes to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Also make these notes translatable. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181093 [details] [review] Bug 36197: (QA follow-up): Move unauth_request_data_check to core Move this out of Standard specific logic. Other backends that support unauthenticated requests may want to make use of this and not duplicate code. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181094 [details] [review] Bug 36197: (QA follow-up): Hackfest followup WIP Moved unauth data to their respective illrequestattributes entries instead of staffnotes + opacnotes. Update confirmation message Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181095 [details] [review] Bug 36197: (QA follow-up): Add public form captcha Feedback provided by David Cook + Matthias Meusburger at KohaCon24 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181096 [details] [review] Bug 36197: (QA follow-up): Adopt POST REDIRECT GET pattern Upon submitting the unauthenticated request, the user is now redirected to a different page and the request details are passed through a session variable. This is to prevent the user from being able to resubmit the same unauthenticated request multiple times (see security bug 39611) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181097 [details] [review] Bug 36197: Atomic update style update Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 181098 [details] [review] Bug 36197: Remove incorrect import Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pedro, thanks for taking care of the feedback we provided during the hackfest. This looks great now. There's only two false negative errors. That's awesome, thank you Tomas. Thanks for the release note! (In reply to Tomás Cohen Arazi (tcohen) from comment #121) > Pedro, thanks for taking care of the feedback we provided during the > hackfest. > > This looks great now. There's only two false negative errors. Thanks for the warning about the false positive(?) errors :) Some tiny tiny stuff: 1) Spelling: E-mail vs. email It looks like the patron form and other places in Koha prefer email over e-mail, so it would be nice to standardize. There is also a mix here between some validation messages and the form for unauthenticated ILL requests. 2) Terminology: Collection library <label for="branchcode">Collection library:</label> Wondering if this should be "Pickup library" instead. Pushed for 25.05! Well done everyone, thank you! Created attachment 181592 [details] [review] Bug 36197: (QA follow-up): Update terminology: - Library input: Change 'Collection library' to 'Pickup library'. - Update 'e-mail' to 'email' Sorry for missing this earlier, picked up the follow-up now. |