|
Description
Pedro Amorim
2023-08-23 13:55:39 UTC
This is similar to how some libraries don't wish to block holds - as a fulfilled hold may encourage the patron to come to the library and renew their account. I think it probably should have a switch/syspref to determine whether it should eb blocked or not We’ve had an instance this week whereby an expired user was able to place an ILL request, which we assumed would have been blocked by BlockExpiredPatronOpacActions. We would want to prevent expired users from placing ILL requests. Looking through the system preferences, we did think about whether we could maybe use the LockExpiredDelay preference (and say set to 1 day), to prevent expired patrons logging into the OPAC at all, in order to prevent these instances slipping through. However, I think this preference would be too blunt, as it requires the misc/cronjobs/cleanup_database.pl to be running. Assuming this was say set to run once daily, in the event of a staff member extending the expiry date of a patron, presumably they would still be blocked from logging into the OPAC for the rest of the day until the job ran again. So for us it makes sense to see placing an ILL request included in the BlockExpiredPatronOpacActions preference, as it seems less blunt than using the LockExpiredDelay preference to prevent this action. Perhaps we could enhance BlockExpiredPatronOpacActions to be a selection list of multiple actions (place holds, place ILL requests, others) instead of just an on/off as it is currently. This would: 1) Be in line with existing behavior for those upgrading. 2) Extend existing behavior to potentially include placing an ILL request as a optional blocked action (among other possible actions). 3) Still allow libraries who wish to keep the possibility of placing holds for blocked patrons (as Nick mentioned) 4) Not be YASP. Thoughts? (In reply to Pedro Amorim from comment #3) > Perhaps we could enhance BlockExpiredPatronOpacActions to be a selection > list of multiple actions (place holds, place ILL requests, others) instead > of just an on/off as it is currently. > > This would: > 1) Be in line with existing behavior for those upgrading. > 2) Extend existing behavior to potentially include placing an ILL request as > a optional blocked action (among other possible actions). > 3) Still allow libraries who wish to keep the possibility of placing holds > for blocked patrons (as Nick mentioned) > 4) Not be YASP. > > Thoughts? +1 Good morning, all, This has become a bit of a problem in the NHS consortium for East of England (23 libraries in total). The main issue for us seems to be that on the ILL module on the staff interface an expired user does not flag as expired when an ILL article request is put through. This has meant that some users that are 'active' with ILL article requests have been deleted in the batch deletions as they are expired. This has resulted in stats being lost. Not sure if there is anything that can be developed to get these users to flag for ILL article requests as they do currently flag for ILL book requests! Thanks, Becca Created attachment 164052 [details] [review] Bug 34597: Add 'ill_request' to patron category admin page Created attachment 164053 [details] [review] Bug 34597: Add 'ill_request' to system preference options Created attachment 164054 [details] [review] Bug 34597: Add tests prove t/db_dependent/Koha/Illrequest.t prove t/db_dependent/Koha/Patron/Categories.t Created attachment 164055 [details] [review] Bug 34597: Implementation New can_patron_place_ill_in_opac method to include all rules that need checking to determine if a patron is allowed to place an ILL request on the OPAC or not. Added effective_BlockExpiredPatronOpacActions_contains rule to this new method. Test plan, k-t-d,: 1) Install FreeForm and enable ILLmodule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) Edit a patron category, visit: <staff_url>/cgi-bin/koha/admin/categories.pl 3) Set 'Placing an ILL request' for the "Block expired patrons" input config 4) Add a new patron of one of the above category, make sure this patron is expired (set an expirydate to the past). 5) Login as that user and visit ILL page in OPAC: /cgi-bin/koha/opac-illrequests.pl 6) Confirm there is no "Create a new request" button 7) Access the create a new request page url directly: <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=FreeForm 8) Confirm you get a 403 page 9) Set the 'Block expired actions' to "Follow system preference BlockExpiredPatronOpacActions" 10) Test different values of the BlockExpiredPatronOpacActions system preference and confirm the behaviour matches what's configured (In reply to R Bradshaw from comment #5) > Good morning, all, > > This has become a bit of a problem in the NHS consortium for East of England > (23 libraries in total). The main issue for us seems to be that on the ILL > module on the staff interface an expired user does not flag as expired when > an ILL article request is put through. This has meant that some users that > are 'active' with ILL article requests have been deleted in the batch > deletions as they are expired. This has resulted in stats being lost. Hi Becca, thank you so much for your input. I've added bug 36454 which I believe better addresses what you're referring to. Please correct me if I'm wrong. > Not sure if there is anything that can be developed to get these users to > flag for ILL article requests as they do currently flag for ILL book > requests! I'm not sure what you mean by "as they do currently flag for ILL book requests". I wasn't aware this was the case, can you provide more info? What ILL backend is this, what page does this show. Thank you. Thanks Pedro. Yes, it allows me to create ILL book requests for an expired user. However, at the point of checkout (i.e. after the item record has been created) it presents an error message saying there is a problem with the users account. It is extremely subtle as a message though – just a line of grey test amongst other lines of grey text. Thanks, Becca (In reply to R Bradshaw from comment #11) > Thanks Pedro. > > Yes, it allows me to create ILL book requests for an expired user. However, > at the point of checkout (i.e. after the item record has been created) it > presents an error message saying there is a problem with the users account. > It is extremely subtle as a message though – just a line of grey test > amongst other lines of grey text. > Thanks, > Becca Got it, thanks for clarifying! If/when you have the chance, please take a quick look at bug 36454 and confirm that we're on the same page regarding this or correct/add anything. Much appreciated! Created attachment 165415 [details] [review] Bug 34597: Add 'ill_request' to patron category admin page Created attachment 165416 [details] [review] Bug 34597: Add 'ill_request' to system preference options Created attachment 165417 [details] [review] Bug 34597: Add tests prove t/db_dependent/Koha/Illrequest.t prove t/db_dependent/Koha/Patron/Categories.t Created attachment 165418 [details] [review] Bug 34597: Implementation New can_patron_place_ill_in_opac method to include all rules that need checking to determine if a patron is allowed to place an ILL request on the OPAC or not. Added effective_BlockExpiredPatronOpacActions_contains rule to this new method. Test plan, k-t-d,: 1) Install FreeForm and enable ILLmodule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) Edit a patron category, visit: <staff_url>/cgi-bin/koha/admin/categories.pl 3) Set 'Placing an ILL request' for the "Block expired patrons" input config 4) Add a new patron of one of the above category, make sure this patron is expired (set an expirydate to the past). 5) Login as that user and visit ILL page in OPAC: /cgi-bin/koha/opac-illrequests.pl 6) Confirm there is no "Create a new request" button 7) Access the create a new request page url directly: <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=FreeForm 8) Confirm you get a 403 page 9) Set the 'Block expired actions' to "Follow system preference BlockExpiredPatronOpacActions" 10) Test different values of the BlockExpiredPatronOpacActions system preference and confirm the behaviour matches what's configured (In reply to Pedro Amorim from comment #16) > 3) Set 'Placing an ILL request' for the "Block expired patrons" input config This fails for me: [WARN] DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect integer value: 'ill_request' for column `koha_kohadev`.`categories`.`BlockExpiredPatronOpacActions` at row 1 at /kohadevbox/koha/Koha/Object.pm line 172 It sems like the patchset is missing a DB migration. (In reply to HKS3 Tadeusz Sośnierz from comment #17) > (In reply to Pedro Amorim from comment #16) > > 3) Set 'Placing an ILL request' for the "Block expired patrons" input config > > This fails for me: > > [WARN] DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: > DBD::mysql::st execute failed: Incorrect integer value: 'ill_request' for > column `koha_kohadev`.`categories`.`BlockExpiredPatronOpacActions` at row 1 > at /kohadevbox/koha/Koha/Object.pm line 172 > > It sems like the patchset is missing a DB migration. Nevermind; `updatedatabase` fixes the issue. It's mentioned in the dependency bug, just not in this one. Needs to be updated for a post-#35581 world (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35581). Since ac157b7a75d84a07002e7c9cecceb549b612cf98 Koha::Illrequest is called Koha::ILL::Request which breaks the changes to /opac/opa-illrequests.pl. It also breaks the entire FreeForm backend, which impacts the testing procedure. (In reply to HKS3 Tadeusz Sośnierz from comment #19) > Needs to be updated for a post-#35581 world > (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35581). Since > ac157b7a75d84a07002e7c9cecceb549b612cf98 Koha::Illrequest is called > Koha::ILL::Request which breaks the changes to /opac/opa-illrequests.pl. It > also breaks the entire FreeForm backend, which impacts the testing procedure. Hi Tadeusz, in this case please switch to "Failed QA". Everyone testing can use this status and it gets the attention of the devs. Letting you do it (for the stats on the dashboard). Created attachment 166563 [details] [review] Bug 34597: Implementation New can_patron_place_ill_in_opac method to include all rules that need checking to determine if a patron is allowed to place an ILL request on the OPAC or not. Added effective_BlockExpiredPatronOpacActions_contains rule to this new method. Test plan, k-t-d,: 1) Install FreeForm and enable ILLmodule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 1.5) Checkout FreeForm's reorganize_ill branch: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ill koha-plack --restart kohadev 2) Edit a patron category, visit: <staff_url>/cgi-bin/koha/admin/categories.pl 3) Set 'Placing an ILL request' for the "Block expired patrons" input config 4) Add a new patron of one of the above category, make sure this patron is expired (set an expirydate to the past). 5) Login as that user and visit ILL page in OPAC: /cgi-bin/koha/opac-illrequests.pl 6) Confirm there is no "Create a new request" button 7) Access the create a new request page url directly: <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=FreeForm 8) Confirm you get a 403 page 9) Set the 'Block expired actions' to "Follow system preference BlockExpiredPatronOpacActions" 10) Test different values of the BlockExpiredPatronOpacActions system preference and confirm the behaviour matches what's configured Created attachment 166564 [details] [review] Bug 34597: Implementation New can_patron_place_ill_in_opac method to include all rules that need checking to determine if a patron is allowed to place an ILL request on the OPAC or not. Added effective_BlockExpiredPatronOpacActions_contains rule to this new method. Test plan, k-t-d,: 1) Install FreeForm and enable ILLmodule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 1.5) Checkout FreeForm's reorganize_ILL branch: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL koha-plack --restart kohadev 2) Edit a patron category, visit: <staff_url>/cgi-bin/koha/admin/categories.pl 3) Set 'Placing an ILL request' for the "Block expired patrons" input config 4) Add a new patron of one of the above category, make sure this patron is expired (set an expirydate to the past). 5) Login as that user and visit ILL page in OPAC: /cgi-bin/koha/opac-illrequests.pl 6) Confirm there is no "Create a new request" button 7) Access the create a new request page url directly: <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=FreeForm 8) Confirm you get a 403 page 9) Set the 'Block expired actions' to "Follow system preference BlockExpiredPatronOpacActions" 10) Test different values of the BlockExpiredPatronOpacActions system preference and confirm the behaviour matches what's configured (In reply to HKS3 Tadeusz Sośnierz from comment #19) > Needs to be updated for a post-#35581 world > (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35581). Since > ac157b7a75d84a07002e7c9cecceb549b612cf98 Koha::Illrequest is called > Koha::ILL::Request which breaks the changes to /opac/opa-illrequests.pl. It > also breaks the entire FreeForm backend, which impacts the testing procedure. Thank you for this. I've squashed the Illrequest->ILL::Request update to the last patch and updated the test plan (notice step 1.5 to checkout FreeForm's branch). Additional info, unrelated to the above: Please note, from the dependency bug, that you should run 'yarn css:build' to see the UI element correctly (I forgot to mention this here). Created attachment 166668 [details] [review] Bug 34597: Add 'ill_request' to patron category admin page Signed-off-by: David Nind <david@davidnind.com> Created attachment 166669 [details] [review] Bug 34597: Add 'ill_request' to system preference options Signed-off-by: David Nind <david@davidnind.com> Created attachment 166670 [details] [review] Bug 34597: Add tests prove t/db_dependent/Koha/Illrequest.t prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: David Nind <david@davidnind.com> Created attachment 166671 [details] [review] Bug 34597: Implementation New can_patron_place_ill_in_opac method to include all rules that need checking to determine if a patron is allowed to place an ILL request on the OPAC or not. Added effective_BlockExpiredPatronOpacActions_contains rule to this new method. Test plan, k-t-d,: 1) Install FreeForm and enable ILLmodule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 1.5) Checkout FreeForm's reorganize_ILL branch: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL koha-plack --restart kohadev 2) Edit a patron category, visit: <staff_url>/cgi-bin/koha/admin/categories.pl 3) Set 'Placing an ILL request' for the "Block expired patrons" input config 4) Add a new patron of one of the above category, make sure this patron is expired (set an expirydate to the past). 5) Login as that user and visit ILL page in OPAC: /cgi-bin/koha/opac-illrequests.pl 6) Confirm there is no "Create a new request" button 7) Access the create a new request page url directly: <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=FreeForm 8) Confirm you get a 403 page 9) Set the 'Block expired actions' to "Follow system preference BlockExpiredPatronOpacActions" 10) Test different values of the BlockExpiredPatronOpacActions system preference and confirm the behaviour matches what's configured Signed-off-by: David Nind <david@davidnind.com> Created attachment 166672 [details] [review] Bug 34597: (follow-up) Update system preference description and order of options Update the note for the BlockExpiredPatronOpacActions system preference to make it clearer, and to follow the standard convention for notes. Order of options for 'Block expired patron OPAC actions' when editing the patron category changed to alpabetical order, to match the system preference order. Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD) (after the patches are applied): 1. Because of the dependency, after applying the patch: updatedatabase, dbic, yarn css:build, flush_memcached, restart_all 2. Tests should pass: - prove t/db_dependent/Koha/ILL/Request.t - prove t/db_dependent/Koha/Patron/Categories.t 3. For the PT patron category, set: . Block expired patron OPAC actions: make sure 'Placing an ILL request' IS NOT selected: . Can place ILL in OPAC: Yes ==> In the OPAC, logged in as a patron in the PT category, for Interlibrary loan requests: '+Create a new request' button is visible, and you can place a request without any errors 4. For the PT patron category, set: . Block expired patron OPAC actions: make sure 'Placing an ILL request' IS selected: . Can place ILL in OPAC: Yes ==> In the OPAC, logged in as a patron in the PT category, for Interlibrary loan requests: '+Create a new request' button IS NOT visible, so you can't place a request 5. Try various other combinations, and you should get the expected results. 6. You should also get the same results for steps 3 and 4 if 'Block expired patron OPAC actions' is set to 'Block actions in BlockExpiredPatronOpacActions', and the values are set using BlockExpiredPatronOpacActions. I added a follow-up: 1. System preference description: make the note a little clearer and follow the convention for notes, that is starting with NOTES: (in bold). 2. Put the order of the options when editing the patron category in alphabetical order, matching how it is displayed for the system preference. (In reply to David Nind from comment #29) > I added a follow-up: > 1. System preference description: make the note a little clearer and follow > the convention for notes, that is starting with NOTES: (in bold). > 2. Put the order of the options when editing the patron category in > alphabetical order, matching how it is displayed for the system preference. Thanks David+++ Created attachment 167481 [details] [review] Bug 34597: Add 'ill_request' to patron category admin page Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 167482 [details] [review] Bug 34597: Add 'ill_request' to system preference options Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 167483 [details] [review] Bug 34597: Add tests prove t/db_dependent/Koha/Illrequest.t prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 167484 [details] [review] Bug 34597: Implementation New can_patron_place_ill_in_opac method to include all rules that need checking to determine if a patron is allowed to place an ILL request on the OPAC or not. Added effective_BlockExpiredPatronOpacActions_contains rule to this new method. Test plan, k-t-d,: 1) Install FreeForm and enable ILLmodule, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 1.5) Checkout FreeForm's reorganize_ILL branch: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout reorganize_ILL koha-plack --restart kohadev 2) Edit a patron category, visit: <staff_url>/cgi-bin/koha/admin/categories.pl 3) Set 'Placing an ILL request' for the "Block expired patrons" input config 4) Add a new patron of one of the above category, make sure this patron is expired (set an expirydate to the past). 5) Login as that user and visit ILL page in OPAC: /cgi-bin/koha/opac-illrequests.pl 6) Confirm there is no "Create a new request" button 7) Access the create a new request page url directly: <opac_url>/cgi-bin/koha/opac-illrequests.pl?op=add_form&backend=FreeForm 8) Confirm you get a 403 page 9) Set the 'Block expired actions' to "Follow system preference BlockExpiredPatronOpacActions" 10) Test different values of the BlockExpiredPatronOpacActions system preference and confirm the behaviour matches what's configured Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 167485 [details] [review] Bug 34597: (follow-up) Update system preference description and order of options Update the note for the BlockExpiredPatronOpacActions system preference to make it clearer, and to follow the standard convention for notes. Order of options for 'Block expired patron OPAC actions' when editing the patron category changed to alpabetical order, to match the system preference order. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 167486 [details] [review] Bug 34597: (QA follow-up) Add missing database revision Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Great work, no regressions found. Passing QA Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant *** Bug 29443 has been marked as a duplicate of this bug. *** Missing 24.05.x dependencies |