Bug 18530 - Remove event attributes from patron clubs templates
Summary: Remove event attributes from patron clubs templates
Status: ASSIGNED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 12461 24391
Blocks: 24393
  Show dependency treegraph
 
Reported: 2017-05-03 14:58 UTC by Owen Leonard
Modified: 2020-01-15 00:55 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18530: Remove event attributes from patron clubs templates (7.58 KB, patch)
2018-11-08 15:29 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18530: Remove event attributes from patron clubs templates (10.25 KB, patch)
2018-11-08 16:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18530: Remove event attributes from patron clubs templates (16.54 KB, patch)
2018-11-14 13:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18530: Remove event attributes from patron clubs templates (16.53 KB, patch)
2019-01-10 15:31 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2017-05-03 14:58:01 UTC
The patron clubs templates add a lot of 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 2018-11-08 15:29:20 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2018-11-08 16:20:12 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2018-11-14 13:42:12 UTC Comment hidden (obsolete)
Comment 4 Candice Hope 2018-12-17 17:49:15 UTC
I was testing this patch in the sandboxes, and while editing the club template, it would not allow me to save a new enrollment field without an error.

Everything else tested well.
Comment 5 Owen Leonard 2018-12-17 17:59:47 UTC
(In reply to Candice Hope from comment #4)
> while editing the club
> template, it would not allow me to save a new enrollment field without an
> error.

I don't see this error when I test. Can you describe the steps to reproduce it? What is the error?
Comment 6 Candice Hope 2018-12-17 18:16:44 UTC
I just made a new sandbox to test it again, and the same error results.

What I did:
1. Create a new club template.
2. Create a new club.
3. Edit the club template. 
4. Add a club field.
5. Add an enrollment field.
6. Error

DBIx::Class::Storage::DBI::_dbh_execute(): Incorrect integer value: '' for column 'id' at row 1 at /kohadevbox/koha/Koha/Object.pm line 125
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
Comment 7 Owen Leonard 2018-12-17 18:50:37 UTC
(In reply to Candice Hope from comment #6)
> 
> DBIx::Class::Storage::DBI::_dbh_execute(): Incorrect integer value: ''

I don't think that error is related to my patch, so I'm going to set this back to "Needs signoff."
Comment 8 Mark Tompsett 2019-01-09 19:17:47 UTC
you missed line 128 or so in koha-tmpl/intranet-tmpl/prog/en/modules/clubs/templates-add-modify.tt
There is still an onclick.

Also, there seems to be a problem with using every authorized value in the drop down. I chose the birthday reason one, and it didn't say, in either the club or the other field. Though, I think that might be beyond the scope of this bug, and yet fixing it first would make testing this bug easier.
Comment 9 Mark Tompsett 2019-01-10 02:48:34 UTC
(In reply to Owen Leonard from comment #7)
> (In reply to Candice Hope from comment #6)
> > 
> > DBIx::Class::Storage::DBI::_dbh_execute(): Incorrect integer value: ''
> 
> I don't think that error is related to my patch, so I'm going to set this
> back to "Needs signoff."

I concur, as I did not experience errors like this. Perhaps they got logged and I didn't read them?
Comment 10 Owen Leonard 2019-01-10 15:31:02 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2019-01-10 15:32:12 UTC
(In reply to M. Tompsett from comment #8)

> I chose the birthday reason one, and it didn't say, in either the
> club or the other field.

I don't understand this description, but if the problem exists in master please file a separate bug.
Comment 12 Pierre-Marc Thibault 2019-01-11 19:23:38 UTC
I noticed there is no longer an onclick attribute for "Add new field" and "Delete field" for enrollment fields, but it's still there for club fields when I edit a club template. I put the QA Failed status because the patch doesn't seem to be complete.