To reproduce: - Turn off suggestion and AnonSuggestions. - Verify there are no longer links visible in navigation and patron account to visit the OPAC suggestion page. - Try to access the page directly: .../cgi-bin/koha/opac-suggestions.pl - Verify that the page is still shown.
Confirmed. With both Sysprefs AnonSuggestions and suggestion turned off, I was redirected to the login page and after login the page "Your purchase suggestions" displayed. I could even put a suggestion.
Pattern for solution in Bug 10595
Maybe something for the Catalyst Open Source Academy... (Starting today)
I'm working on this.
Created attachment 35171 [details] [review] Bug 12702 - Suggestions still accessible in OPAC when deactivated Added check for suggestions turned off. If off user is redirected to 404 error. If patron suggestions are off guest sugestions will also redirect to 404
Test Plan: -Set preference 'suggestion' to 'Don't Allow'. Navigate to .../cgi-bin/koha/opac-suggestions.pl *You should be presented with a 404 error. This is the case even if 'AnonSuggestions' are allowed. I am assuming that if suggestions by patrons are not wanted that AnonSuggestions are also not wanted. I think this is a reasonable assumption. -Set preference 'suggestion' to 'Allow' and set preference 'AnonSuggestions' to 'Don't Allow' Navigate to .../cgi-bin/koha/opac-suggestions.pl from a non logged in user page. *You should be presented with a login screen. Log in using that page and you should be presented with the normal suggestions page. -Set preference 'suggestion' to 'Allow' and set preference 'AnonSuggestions' to 'Allow' Navigate to .../cgi-bin/koha/opac-suggestions.pl from a non logged in user page. *You should be presented with the normal suggestions page. Log in and navigate to .../cgi-bin/koha/opac-suggestions.pl *You should be presented with the normal suggestions page.
Created attachment 35175 [details] [review] [SIGNED OFF] Bug 12702 - Suggestions still accessible in OPAC when deactivated Added check for suggestions turned off. If off user is redirected to 404 error. If patron suggestions are off guest sugestions will also redirect to 404 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
OK, I'm happy with the logic here. I had to go digging a little to prove to mysefl that the assumption was valid. (Lines 530 -> 539 in commit http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt;hb=544520d6bdfacada4eba4caefedf42864db4b3a0#l529) So, this patch does indeed fix the problem.. however as a side note, I really don't like seeing this kind of logic at the template level.. it's not testable and leads to repetative code :'(. I'de rather see the template just ask the question 'to display, or not to display?', and have the actual logic involved in that question handled further up in the code, perferably in either C4/Suggestions.pm or as part of the Koha.preference plugin somehow. That way, we'd have a testable fix. Just a thought
(In reply to Martin Renvoize from comment #8) > OK, I'm happy with the logic here. I had to go digging a little to prove to > mysefl that the assumption was valid. (Lines 530 -> 539 in commit > http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=koha-tmpl/opac- > tmpl/bootstrap/en/modules/opac-results.tt; > hb=544520d6bdfacada4eba4caefedf42864db4b3a0#l529) > > So, this patch does indeed fix the problem.. however as a side note, I > really don't like seeing this kind of logic at the template level.. it's not > testable and leads to repetative code :'(. > > I'de rather see the template just ask the question 'to display, or not to > display?', and have the actual logic involved in that question handled > further up in the code, perferably in either C4/Suggestions.pm or as part of > the Koha.preference plugin somehow. That way, we'd have a testable fix. > Just a thought Should I work on this?
I think not a blocker for your patch - just some thoughts about how we could handle things better.
Created attachment 35281 [details] [review] [PASSED QA] Bug 12702 - Suggestions still accessible in OPAC when deactivated Added check for suggestions turned off. If off user is redirected to 404 error. If patron suggestions are off guest sugestions will also redirect to 404 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Agreed, not a blocker here.. it's not your job to go cleaning the code up.. this was more just a comment trying to get anyone that sees it to think a bit more when implementing such code in the first place.. had such a request been centralised in the first place this fix probably wouldn't have been needed ;) Cheers Thomas, you guys are doing a great job this year :)
Patch pushed to master. Thanks Thomas!
Pushed to 3.18.x will be in 3.18.3