Bug 24393

Summary:	Remove event attributes from patron clubs list template
Product:	Koha	Reporter:	Owen Leonard <oleonard>
Component:	Templates	Assignee:	Owen Leonard <oleonard>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	david, testopia
Version:	Main
Hardware:	All
OS:	All
URL:	/cgi-bin/koha/clubs/clubs.pl
Change sponsored?:	---	Patch complexity:	Small patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	20.05.00
Circulation function:
Bug Depends on:	18530
Bug Blocks:
Attachments:	Bug 24393: Remove event attributes from patron clubs list template Bug 24393: Remove event attributes from patron clubs list template Bug 24393: Remove event attributes from patron clubs list template

Description Owen Leonard 2020-01-09 19:08:35 UTC

The template for the main list of patron clubs has some instances of "onclick," in violation of coding guidelines.

https://wiki.koha-community.org/wiki/Coding_Guidelines#JS9:_Avoid_the_use_of_event_attributes_like_.22onclick.22_to_attach_events

They should be removed in favor of defining events in JavaScript instead of markup.

Comment 1 Owen Leonard 2020-01-09 19:38:33 UTC

Created attachment 97139 [details] [review]
Bug 24393: Remove event attributes from patron clubs list template

This patch removes some instances of the "onclick" attributes from the
template that lists patron clubs and templates.

To test, apply the patch and go to Tools -> Patron clubs.

 - Test the process of deleting a club template. Clicking the "Delete"
   button should trigger a confirmation. Confirming should delete the
   template.

 - Test the process of deleting a club. In the "Clubs" section of the
   page, click the "Actions" menu next to any club. Clicking the
   "Delete" link should trigger a confirmation. Confirming should delete
   the club.

 - Also in the "Clubs" section of the page, find a club which has
   enrollments. Select "Search to hold" from the "Actions" menu. It
   should take you to the advanced search. On the search results page
   you should have the option to place a hold for the club you selected.

Comment 2 David Nind 2020-01-12 20:02:30 UTC

Something in master has broken "Search to hold" - getting this error message when trying to add a hold (just the start of the error message):

> Template process failed: undef error - Not an ARRAY reference at /kohadevbox/koha/Koha/Template/Plugin/Branches.pm line 95.
>  at /kohadevbox/koha/C4/Templates.pm line 122
> 
>     in C4::Templates::output at /kohadevbox/koha/C4/Templates.pm line 122
> 
>       119: 
>       120:     my $data;
>       121:     binmode( STDOUT, ":utf8" );
>       122:     $template->process( $self->filename, $vars, \$data )
>       123:       || die "Template process failed: ", $template->error();
>       124:     return $data;
>       125: }
> 
>     Show function arguments

Placing a hold works fine on 19.11.x.

Comment 3 Katrin Fischer 2020-01-12 20:34:25 UTC

(In reply to David Nind from comment #2)
> Something in master has broken "Search to hold" - getting this error message
> when trying to add a hold (just the start of the error message):
> 
> > Template process failed: undef error - Not an ARRAY reference at /kohadevbox/koha/Koha/Template/Plugin/Branches.pm line 95.
> >  at /kohadevbox/koha/C4/Templates.pm line 122
> > 

Hi David, this is probably bug 24350 - hopefully to be fixed soon.

Comment 4 David Nind 2020-01-13 08:37:37 UTC

Thanks Katrin! Will keep an eye out and retest once it's resolved...

Comment 5 David Nind 2020-01-17 12:17:47 UTC

Created attachment 97514 [details] [review]
Bug 24393: Remove event attributes from patron clubs list template

This patch removes some instances of the "onclick" attributes from the
template that lists patron clubs and templates.

To test, apply the patch and go to Tools -> Patron clubs.

 - Test the process of deleting a club template. Clicking the "Delete"
   button should trigger a confirmation. Confirming should delete the
   template.

 - Test the process of deleting a club. In the "Clubs" section of the
   page, click the "Actions" menu next to any club. Clicking the
   "Delete" link should trigger a confirmation. Confirming should delete
   the club.

 - Also in the "Clubs" section of the page, find a club which has
   enrollments. Select "Search to hold" from the "Actions" menu. It
   should take you to the advanced search. On the search results page
   you should have the option to place a hold for the club you selected.

Signed-off-by: David Nind <david@davidnind.com>

Comment 6 Katrin Fischer 2020-01-20 06:57:18 UTC

Created attachment 97599 [details] [review]
Bug 24393: Remove event attributes from patron clubs list template

This patch removes some instances of the "onclick" attributes from the
template that lists patron clubs and templates.

To test, apply the patch and go to Tools -> Patron clubs.

 - Test the process of deleting a club template. Clicking the "Delete"
   button should trigger a confirmation. Confirming should delete the
   template.

 - Test the process of deleting a club. In the "Clubs" section of the
   page, click the "Actions" menu next to any club. Clicking the
   "Delete" link should trigger a confirmation. Confirming should delete
   the club.

 - Also in the "Clubs" section of the page, find a club which has
   enrollments. Select "Search to hold" from the "Actions" menu. It
   should take you to the advanced search. On the search results page
   you should have the option to place a hold for the club you selected.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Comment 7 Martin Renvoize (ashimema) 2020-01-20 14:04:52 UTC

Nice work everyone!

Pushed to master for 20.05