When tagging is disabled, Koha still lets you acces the tags cloud page. It displays a message that tagging is disabled, but at the same time tags in the system are displayed: .../cgi-bin/koha/opac-tags.pl I think we should handle this like other pages and not have this page accessible when the feature is diabled. But at least we shoudl not show tags, that might have been added for testing the feature.
Created attachment 95618 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. This patch introduces the blocking_errors include file at the OPAC. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you see a (ugly) short screen saying that the tag system is disabled.
When I apply the patch, but leave TagsEnabled enabled I get this message when accessing the page: response = { added: 0, deleted: 0, errors: 0 }; When I disable TagsEnabled I get a long page of error messages starting with: Can't call method "param" on an undefined value at /kohadevbox/koha/C4/Output.pm line 363 in C4::Output::output_and_exit at /kohadevbox/koha/C4/Output.pm line 363 360: 361: sub output_and_exit { 362: my ( $query, $cookie, $template, $error ) = @_; 363: $template->param( blocking_error => $error ); 364: output_html_with_http_headers ( $query, $cookie, $template->output ); 365: exit; 366: } Hide function arguments $_[0] 'CGI=HASH(0x559b4b806d98)' $_[1] undef $_[2] undef $_[3] 'tags_disabled' in (eval) at /kohadevbox/koha/opac/opac-tags.pl line 121 118: } 119: 120: 121: output_and_exit( $query, $cookie, $template, 'tags_disabled' ) 122: unless C4::Context->preference('TagsEnabled'); 123: 124: if ($add_op) {
Created attachment 95652 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. This patch introduces the blocking_errors include file at the OPAC. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you see a (ugly) short screen saying that the tag system is disabled.
Sorry David, I added to the patch a last minute change that should never have been part of it.
Created attachment 95653 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. This patch introduces the blocking_errors include file at the OPAC. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you see a (ugly) short screen saying that the tag system is disabled. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95750 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. This patch introduces the blocking_errors include file at the OPAC. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you see a (ugly) short screen saying that the tag system is disabled. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
It's really a little ugly...
Created attachment 95825 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. This patch introduces the blocking_errors include file at the OPAC. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you see a (ugly) short screen saying that the tag system is disabled. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I removed a debug statement.
I think it would be better to remove the in-page message altogether and trigger a 404 if tagging is disabled and the user navigates directly to opac-tags.pl. This is what we do with opac-topissues.pl and opac-suggestions.pl.
I am good with both approaches. I wanted to provide a patch that kept the original behavior. As well it introduces the use of blocking-errors.inc OPAC-side, which was a good move forward.
I think I agree with Owen about the 404 - especially since it seems what we do in other cases. Moving this out of 'in discussion'. I am not sure if we should add an alternate patch or can/should keep the work on blocking errors somehow?
Created attachment 105349 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. We should redirect to 404 like we do in opac-topissues.pl and opac-suggestions.pl. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you get a 404 redirect
Created attachment 105354 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. We should redirect to 404 like we do in opac-topissues.pl and opac-suggestions.pl. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you get a 404 redirect Signed-off-by: David Nind <david@davidnind.com>
Created attachment 105428 [details] [review] Bug 23276: Do not display tag if pref TagsEnabled is off If the pref TagsEnabled is off we should not display the tags at the OPAC. There is a message to tell that tags system is disabled, but the tags are displayed. We should redirect to 404 like we do in opac-topissues.pl and opac-suggestions.pl. Test plan: - Turn TagsEnabled on - Add some tags - Turn TagsEnabled off - Hit /cgi-bin/koha/opac-tags.pl => Without this patch you see a warning messaging saying that the tag system is disabled, but the tags are displayed => With this patch you get a 404 redirect Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.01
backported to 19.11.x for 19.11.07
Backported to 19.05.x branch for 19.05.13