http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12446
--- Comment #12 from M. Tompsett <[email protected]> --- Comment on attachment 39744 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39744 bug 12446 - Enable adult patrons to have a guarantor Review of attachment 39744: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12446&attachment=39744) ----------------------------------------------------------------- I just skimmed the patches. The linking guarantor to a specific category code is bad. I like the notion of a canbeguaranteed code field. Some code changes aren't obvious to me. ::: C4/Utils/DataTables/Members.pm @@ +38,5 @@ > + @columns = grep { ! $seen{ $_ }++ } ( @columns, > @guarantor_attributes ); > + }; > + $_ = "borrowers.".$dbh->quote_identifier($_) for (@columns); > + > + my $select = "SELECT ${ \join(',', @columns) }, This code is horrible to read! ::: installer/data/mysql/atomicupdate/bug_12446-EnableAdultGarantee.sql @@ +9,5 @@ > +-- ********* -- > +ALTER TABLE categories ADD COLUMN `canbeguarantee` tinyint(1) NOT NULL > default '0'; > + > +-- Set defaults for new column > +UPDATE categories set canbeguarantee = 1 WHERE category_type IN ('C', 'P'); \ No newline at end of file Default behaviour should NOT activate new functionality. ::: installer/data/mysql/sysprefs.sql @@ +481,4 @@ > ('XSLTDetailsDisplay','default','','Enable XSL stylesheet control over > details page display on intranet','Free'), > ('XSLTResultsDisplay','default','','Enable XSL stylesheet control over > results page display on intranet','Free'), > ('z3950AuthorAuthFields','701,702,700',NULL,'Define the MARC biblio fields > for Personal Name Authorities to fill biblio.author','free'), > +('z3950NormalizeAuthor','0','','If ON, Personal Name Authorities will > replace authors in biblio.author','YesNo'), Why was , added at the end? ::: koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc @@ +148,4 @@ > > [% IF ( CAN_user_borrowers ) %] > [% IF ( adultborrower AND activeBorrowerRelationship ) %] > + <a id="addchild" class="btn btn-small" > href="/cgi-bin/koha/members/memberentry.pl?op=add&guarantorid=[% > borrowernumber %]&category_type=C"><i class="icon-plus"></i> Add > guarantiee</a> Is that spelt correctly? ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt @@ +25,5 @@ > $("#branches option:first").attr("selected", "selected"); > } > + $("#category_type").change(function(){ > + var selected = $(this).val(); > + $("#canbeguarantee").val( (selected == 'C' || selected == 'P') ? "1" > : "0" ); Hardcoded category codes is a bad thing. Is there a better way to do this? ::: members/memberentry.pl @@ +251,4 @@ > } > > #recover all data from guarantor address phone ,fax... > +if ( $guarantorid and ( $category_type eq 'C' || $category_type eq 'P')) { Why remove the space? @@ -259,5 @@ > if ( $op eq 'add' ) { > foreach (qw(streetnumber address streettype address2 > - zipcode country city state phone phonepro mobile fax > email emailpro branchcode > - B_streetnumber B_streettype B_address B_address2 > - B_city B_state B_zipcode B_country B_email B_phone)) > { Why were these removed? ::: members/moremember.pl @@ +204,2 @@ > if ($category_type eq 'C'){ > $template->param('C' => 1); Is this used anymore in the template file? I saw you removed one C condition. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
