We are planning to create levels of membership at our library and we would like to prevent some borrower types from placing purchase suggestions. Currently, the system preference for purchase suggestions is 'Allow' or 'Don't Allow' for all borrower types. We would like to set the ability to place purchase suggestion for each borrow type.
There is an old bug 9290 about this, that hasn't moved in quite a while. As this is assigned already, I think we can make the other a duplicate to give this a fresh start. Alex, if you prefer to keep the low bug number, just switch :)
*** Bug 9290 has been marked as a duplicate of this bug. ***
I think it's too late to incorporate such a common feature which may be very much required by many libraries. The same has been sponsored by the Indian community too.
Why would it be too late? There is no reason why this should not be added. The problem is that the sponsored code was never submitted to community, so there was no chance to integrate it.
(In reply to Katrin Fischer from comment #4) > Why would it be too late? There is no reason why this should not be added. > The problem is that the sponsored code was never submitted to community, so > there was no chance to integrate it. Actually long back (4/5 year), some of the library professionals from India contributed for this development to one of the developer (l2c2.ac.in) but it is not yet done as the firm is looking for some more sponser I guess. But it is very late and many felt it was very much needed during covid situations too. Hope the new discussion will motivate the l2c2.co.in to act promptly on this feature
Created attachment 139112 [details] [review] Bug 31333: Database changes Sponsored-by: Catalyst IT, New Zealand
Created attachment 139113 [details] [review] Bug 31333: Add ability to make purchase suggestions by borrower type Introduce a suggestionPatronCategoryExceptions system preference. If the suggestion syspref is enabled then libraries can stop specific borrower types from making suggestions by ticking the type in the suggestionPatronCategoryExceptions syspref. Test plan: 1. Apply patches, update database, re-start services 2. Set 'suggestion' syspref = 'Allow' 3. Confirm you can view the purchase suggestion links on OPAC biblio detail page & 'your summary' page. As well as successfully submit a suggestion. 4. Select the patron category you're logged in as in suggestionPatronCategoryExceptions syspref 5. Confirm the purchase suggestion links are hidden in the OPAC biblio detail page & 'your summary' page 6. In your browser enter the link: <OPAC base URL>/cgi-bin/koha/opac-suggestions.pl e.g. http://localhost:8080/cgi-bin/koha/opac-suggestions.pl 7. Confirm a 404 page loads 8. Confirm you can view/moderate suggestions in the staff client - even though your patron is selected in the suggestionPatronCategoryExceptions syspref 9. Untick your patron category in the suggestionPatronCategoryExceptions syspref 10. Confirm you can view the purchase suggestion links on the OPAC, as well as successfully submit a suggestion. 11. Set 'suggestion' syspref = "Don't allow" 12. Confirm the purchase suggestion links are hidden in the OPAC 13. Select all patron categories in suggestPatronCategoryExceptions syspref. View the OPAC without logging in and confirm you can perform searches and view OPAC biblio detail pages. Sponsored-by: Catalyst IT, New Zealand
Created attachment 139114 [details] [review] Bug 31333: Unit tests Test plan: 1. Run tests (Categories.t has been updated by me, and I suggest running Suggestions.t to confirm it still passes): sudo koha-shell kohadev prove t/db_dependent/Koha/Patron/Categories.t prove t/db_dependent/Koha/Suggestions.t Sponsored-by: Catalyst IT, New Zealand
I've attached a patchset that introduces a new syspref: 'suggestionPatronCategoryException'. This syspref, like the existing 'OPACHoldsIfAvailableAtPickupExceptions' syspref, is a multi-choice dropdown of patron categories. Any categories selected are not affected when the 'suggestion' syspref is enabled. --- Note: 1. I deliberately chose to add a new syspref to 'exclude' categories, rather than alter the existing 'suggestion' syspref to be the multi-choice dropdown of 'allowed' patron categories because I felt the latter could create confusion for upgrading libraries. Specifically, every time they added new patron categories they may wonder why those patrons could not submit suggestions if the library had not remembered to manually allow that category to create suggestions. 2. Another conscious decision was to place the logic for checking if a patron could create a suggestion in Koha/Patron/Category.pm. The point of this enhancement is to allow/disallow patrons from placing suggestions based on their patron category. Therefore, it felt right to have that logic in Koha/Patron/Category.pm, rather than Koha/Suggestion.pm Ready for testing.
I just completed the test plan for this bug and everything was great with one exception. As stated in the test plan, I Select the patron category you're logged in as in suggestionPatronCategoryExceptions syspref. In this case 'Staff'. That is the only category I selected. I then got to the step that says 'Untick your patron category in the suggestionPatronCategoryExceptions syspref.' and that resulted in zero categories check. When I tried to save, I got the message 'Nothing to save'. I checked to OPAC and I still was not able to place a purchase suggestion even though the 'Staff' box was unticked. I then checked another category and unticked the 'Staff' box and I was able to save. On the OPAC, I was able to place a purchase suggestion. Other than the case of trying to remove all the checks from the patron types, this functioned as expected.
Great testing, Rhonda. I wonder if this happens with other prefs of this type (patron category list) as well.
I had the same thought. This might not be a problem with this bug. It might be a system preference problem.
OK. I still had my sandbox available and I tested OPACHoldsIfAvailableAtPickupExceptions to see if the system would save changes when I updated from having something checked to unchecking everything. I got the same result as for the suggestionPatronCategoryExceptions preference. I would say this bug is fine and we have another problem with saving system preference.
Created attachment 139165 [details] [review] Bug 31333: Unit tests Test plan: 1. Run tests (Categories.t has been updated by me, and I suggest running Suggestions.t to confirm it still passes): sudo koha-shell kohadev prove t/db_dependent/Koha/Patron/Categories.t prove t/db_dependent/Koha/Suggestions.t Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
I opened a bug for the problem saving system preferences where there is a drop down as in suggestionPatronCategoryExceptions when you clear all the checked options.
Thank you for testing Rhonda. I can also replicate the inability to uncheck all checkboxes and save the OPACHoldsIfAvailableAtPickupExceptions syspref.
What is relevance of old purchase suggestions, why it can't be replaced with new? Or current can be used in different way as discussed above?
(In reply to Vinod from comment #17) > What is relevance of old purchase suggestions, why it can't be replaced with > new? Or current can be used in different way as discussed above? Hi Vinod, Are you referring to why did I add a new system preference (suggestionPatronCategoryExceptions) rather than converting the existing 'suggestion' system preference to a dropdown of patron categories ALLOWED to make suggestions? If that is what you're referring to I did not go that route for 2 reasons: Reason 1. It would have meant every time libraries added new patron categories to Koha they would have had to remember to update the system preference allowing those new patron categories to make suggestions, something easily forgotten. Whereas, adding a new system preference (suggestionPatronCategoryExceptions) defining patron categories NOT allowed to make suggestions means as new patron categories are added, by default, those patrons will be able to submit suggestions. i.e. From a library configuration perspective, I believe it is better for new patron categories to be allowed to do something by default unless explicitly denied. Reason 2: There are precedents for my approach already in Koha: - The OPACHoldsIfAvailableAtPickup and OPACHoldsIfAvailableAtPickupExceptions sysprefs. - The OpacHiddenItems and OpacHiddenItemsExceptions sysprefs. --- Also, I avoided adding a boolean flag to patron categories controlling whether they could submit suggestions, to keep all suggestion configurations managed in one place: system preferences. Alex
I am starting QA here, also filed bug 31474 for the general bug Rhonda found.
Created attachment 139897 [details] [review] Bug 31333: (QA follow-up) Combine and rephrase system preferences Combines suggestion and suggestionPatronCategoryExceptions so they can be easily set together and rephrases slightly.
Created attachment 139898 [details] [review] Bug 31333: (QA follow-up) Add the new check to OPAC result list When no result is found and at the bottom of the result list the option to make a purchase suggestion is displayed. This also needs to use the new check.
Hi Alex, I provided 2 follow-ups, but then I got stuck: The new check doesn't take AnonSuggestions into account. When you click on the link in OPAC navigation now, you get a 404 if logged in and Can't call method "category" on an undefined value at /kohadevbox/koha/opac/opac-suggestions.pl line 97 if you are logged out. I believe if AnonSuggestions is set, we need to always allow suggestions independent of the other settings (should not matter if logged in or logged out).
(In reply to Katrin Fischer from comment #19) > I am starting QA here, also filed bug 31474 for the general bug Rhonda found. Missed that Rhonda had already filed it: bug 31364
(In reply to Katrin Fischer from comment #22) > Hi Alex, > > I provided 2 follow-ups, but then I got stuck: The new check doesn't take > AnonSuggestions into account. When you click on the link in OPAC navigation > now, you get a 404 if logged in and Can't call method "category" on an > undefined value at /kohadevbox/koha/opac/opac-suggestions.pl line 97 if you > are logged out. > > I believe if AnonSuggestions is set, we need to always allow suggestions > independent of the other settings (should not matter if logged in or logged > out). Hi Katrin, Thanks for QA, I will work on a fix for this. Alex
Created attachment 139935 [details] [review] Bug 31333: (follow-up) Handle anonymous patrons making suggestions Test plan: 1. Enable AnonSuggestions syspref and set AnonymousPatron = 1 2. Visit the OPAC without logging in 3. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 4. Disable the AnonSuggestions syspref 5. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 6. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a login page Sponsored-by: Catalyst IT, New Zealand
Created attachment 139936 [details] [review] Bug 31333: (follow-up) Handle anonymous patrons making suggestions Test plan: 1. Enable suggestion & AnonSuggestions sysprefs and set AnonymousPatron = 1 2. Visit the OPAC without logging in 3. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 4. Disable the AnonSuggestions syspref 5. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 6. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a login page 7. Select the category of your user in the suggestionPatronCategoryExceptions syspref 8. Log into the OPAC 9. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page 10. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a 404 error page 11. Enable AnonSuggestions syspref 12. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page 13. Disable AnonSuggestions syspref and un-check your category from suggestionPatronCategoryExeptions syspref 14. Confirm you can create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page Sponsored-by: Catalyst IT, New Zealand
(In reply to Katrin Fischer from comment #22) > Hi Alex, > > I provided 2 follow-ups, but then I got stuck: The new check doesn't take > AnonSuggestions into account. When you click on the link in OPAC navigation > now, you get a 404 if logged in and Can't call method "category" on an > undefined value at /kohadevbox/koha/opac/opac-suggestions.pl line 97 if you > are logged out. > > I believe if AnonSuggestions is set, we need to always allow suggestions > independent of the other settings (should not matter if logged in or logged > out). Hi Katrin, I have attached a follow-up patch that should, hopefully, resolve the problems you raised. Now regardless of any other setting if the AnonSuggestions & suggestion syspref is enabled then a patron can create an OPAC suggestion. I tested on master and found if you have AnonSuggestions enabled, but suggestion disabled, then opac-suggestions.pl re-directs to the 404 page. You'll notice I also tweaked C4::Auth->get_template_and_user() so it now checks whether AnonSuggestions is enabled before passing the suggestion variable to the template. As I've added that I have tweaked your check on opac-results.tt, I hope that is alright. Ready for testing again. Alex
Created attachment 140169 [details] [review] Bug 31333: Database changes Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 140170 [details] [review] Bug 31333: Add ability to make purchase suggestions by borrower type Introduce a suggestionPatronCategoryExceptions system preference. If the suggestion syspref is enabled then libraries can stop specific borrower types from making suggestions by ticking the type in the suggestionPatronCategoryExceptions syspref. Test plan: 1. Apply patches, update database, re-start services 2. Set 'suggestion' syspref = 'Allow' 3. Confirm you can view the purchase suggestion links on OPAC biblio detail page & 'your summary' page. As well as successfully submit a suggestion. 4. Select the patron category you're logged in as in suggestionPatronCategoryExceptions syspref 5. Confirm the purchase suggestion links are hidden in the OPAC biblio detail page & 'your summary' page 6. In your browser enter the link: <OPAC base URL>/cgi-bin/koha/opac-suggestions.pl e.g. http://localhost:8080/cgi-bin/koha/opac-suggestions.pl 7. Confirm a 404 page loads 8. Confirm you can view/moderate suggestions in the staff client - even though your patron is selected in the suggestionPatronCategoryExceptions syspref 9. Untick your patron category in the suggestionPatronCategoryExceptions syspref 10. Confirm you can view the purchase suggestion links on the OPAC, as well as successfully submit a suggestion. 11. Set 'suggestion' syspref = "Don't allow" 12. Confirm the purchase suggestion links are hidden in the OPAC 13. Select all patron categories in suggestPatronCategoryExceptions syspref. View the OPAC without logging in and confirm you can perform searches and view OPAC biblio detail pages. Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 140171 [details] [review] Bug 31333: Unit tests Test plan: 1. Run tests (Categories.t has been updated by me, and I suggest running Suggestions.t to confirm it still passes): sudo koha-shell kohadev prove t/db_dependent/Koha/Patron/Categories.t prove t/db_dependent/Koha/Suggestions.t Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 140172 [details] [review] Bug 31333: (QA follow-up) Combine and rephrase system preferences Combines suggestion and suggestionPatronCategoryExceptions so they can be easily set together and rephrases slightly. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 140173 [details] [review] Bug 31333: (QA follow-up) Add the new check to OPAC result list When no result is found and at the bottom of the result list the option to make a purchase suggestion is displayed. This also needs to use the new check. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 140174 [details] [review] Bug 31333: (follow-up) Handle anonymous patrons making suggestions Test plan: 1. Enable suggestion & AnonSuggestions sysprefs and set AnonymousPatron = 1 2. Visit the OPAC without logging in 3. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 4. Disable the AnonSuggestions syspref 5. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 6. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a login page 7. Select the category of your user in the suggestionPatronCategoryExceptions syspref 8. Log into the OPAC 9. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page 10. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a 404 error page 11. Enable AnonSuggestions syspref 12. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page 13. Disable AnonSuggestions syspref and un-check your category from suggestionPatronCategoryExeptions syspref 14. Confirm you can create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 140270 [details] [review] Bug 31333: Database changes Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 140271 [details] [review] Bug 31333: Add ability to make purchase suggestions by borrower type Introduce a suggestionPatronCategoryExceptions system preference. If the suggestion syspref is enabled then libraries can stop specific borrower types from making suggestions by ticking the type in the suggestionPatronCategoryExceptions syspref. Test plan: 1. Apply patches, update database, re-start services 2. Set 'suggestion' syspref = 'Allow' 3. Confirm you can view the purchase suggestion links on OPAC biblio detail page & 'your summary' page. As well as successfully submit a suggestion. 4. Select the patron category you're logged in as in suggestionPatronCategoryExceptions syspref 5. Confirm the purchase suggestion links are hidden in the OPAC biblio detail page & 'your summary' page 6. In your browser enter the link: <OPAC base URL>/cgi-bin/koha/opac-suggestions.pl e.g. http://localhost:8080/cgi-bin/koha/opac-suggestions.pl 7. Confirm a 404 page loads 8. Confirm you can view/moderate suggestions in the staff client - even though your patron is selected in the suggestionPatronCategoryExceptions syspref 9. Untick your patron category in the suggestionPatronCategoryExceptions syspref 10. Confirm you can view the purchase suggestion links on the OPAC, as well as successfully submit a suggestion. 11. Set 'suggestion' syspref = "Don't allow" 12. Confirm the purchase suggestion links are hidden in the OPAC 13. Select all patron categories in suggestPatronCategoryExceptions syspref. View the OPAC without logging in and confirm you can perform searches and view OPAC biblio detail pages. Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 140272 [details] [review] Bug 31333: Unit tests Test plan: 1. Run tests (Categories.t has been updated by me, and I suggest running Suggestions.t to confirm it still passes): sudo koha-shell kohadev prove t/db_dependent/Koha/Patron/Categories.t prove t/db_dependent/Koha/Suggestions.t Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 140273 [details] [review] Bug 31333: (QA follow-up) Combine and rephrase system preferences Combines suggestion and suggestionPatronCategoryExceptions so they can be easily set together and rephrases slightly. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 140274 [details] [review] Bug 31333: (QA follow-up) Add the new check to OPAC result list When no result is found and at the bottom of the result list the option to make a purchase suggestion is displayed. This also needs to use the new check. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 140275 [details] [review] Bug 31333: (follow-up) Handle anonymous patrons making suggestions Test plan: 1. Enable suggestion & AnonSuggestions sysprefs and set AnonymousPatron = 1 2. Visit the OPAC without logging in 3. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 4. Disable the AnonSuggestions syspref 5. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box 6. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a login page 7. Select the category of your user in the suggestionPatronCategoryExceptions syspref 8. Log into the OPAC 9. Confirm you cannot see links to make purchase suggestions on the following pages: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page 10. Confirm if you try visiting /cgi-bin/koha/opac-suggestions.pl page you are re-directed to a 404 error page 11. Enable AnonSuggestions syspref 12. Confirm you can successfully create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page 13. Disable AnonSuggestions syspref and un-check your category from suggestionPatronCategoryExeptions syspref 14. Confirm you can create a suggestion from: - Item detail page - Search result page - Masthead under the 'Library catalogue' search box - opac-user.pl ('Your summary') page Sponsored-by: Catalyst IT, New Zealand Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 140276 [details] [review] Bug 31333: (QA follow-up) Add hint about AnonSuggestions to new system preference Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 141392 [details] [review] Bug 31333: (QA follow-up) Move and improve tests This patch moves tests to the right file (singular class name). It also refactors them a bit, in order to cover more cases, including the syspref split one. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron/Category.t \ t/db_dependent/Koha/Patron/Categories.t => SUCCESS: All good. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 141393 [details] [review] Bug 31333: (QA follow-up) Simplify code The new syspref is a deny-list. So not being part of the list means you are allowed. This was not that clear reading the code, so this patch tries to simplify the logic. Better handling on undef syspref value is added as well to avoid useless warnings. This patch does the following changes: - Uses `any` instead of `grep`. This has the effect of just returning a boolean on the first match, instead of keep looping until de array Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 22.11. Nice work everyone, thanks!