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.
Created attachment 82093 [details] [review] Bug 18530: Remove event attributes from patron clubs templates This patch removes event attributes like "onclick" from patron clubs templates in favor of putting these event handlers in the JavaScript. To test, apply the patch and go to Tools -> Patron clubs. - Test deletion of both club templates and clubs. Deletion confirmation dialogs should work as expected, both for OK and Cancel. - Open a patron record for checkout. - Test enrolling and un-enrolling the patron. - Test the same process from the patron detail page.
Created attachment 82100 [details] [review] Bug 18530: Remove event attributes from patron clubs templates This patch removes event attributes like "onclick" from patron clubs templates in favor of putting these event handlers in the JavaScript. To test, apply the patch and go to Tools -> Patron clubs. - Test deletion of both club templates and clubs. Deletion confirmation dialogs should work as expected, both for OK and Cancel. - Test editing of a club template. Under "Club fields," the "Add new field" and "Delete field" links should work correctly. - Open a patron record for checkout. - Test enrolling and un-enrolling the patron. - Test the same process from the patron detail page.
Created attachment 82332 [details] [review] Bug 18530: Remove event attributes from patron clubs templates This patch removes event attributes like "onclick" from patron clubs templates in favor of putting these event handlers in the JavaScript. To test, apply the patch and go to Tools -> Patron clubs. - Test deletion of both club templates and clubs. Deletion confirmation dialogs should work as expected, both for OK and Cancel. - Test editing of a club template. Under "Club fields," the "Add new field" and "Delete field" links should work correctly. - Open a patron record for checkout. - Test enrolling and un-enrolling the patron. - Test the same process from the patron detail page.
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.
(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?
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
(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."
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.
(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?
Created attachment 83771 [details] [review] Bug 18530: Remove event attributes from patron clubs templates This patch removes event attributes like "onclick" from patron clubs templates in favor of putting these event handlers in the JavaScript. To test, apply the patch and go to Tools -> Patron clubs. - Test deletion of both club templates and clubs. Deletion confirmation dialogs should work as expected, both for OK and Cancel. - Test editing of a club template. Under "Club fields," the "Add new field" and "Delete field" links should work correctly. - Open a patron record for checkout. - Test enrolling and un-enrolling the patron. - Test the same process from the patron detail page.
(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.
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.