Description
Katrin Fischer
2019-07-08 08:46:07 UTC
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 |