[Koha-bugs] [Bug 12446] Enable an adult to have a guarantor

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12446

--- Comment #12 from M. Tompsett <mtompset at hotmail.com> ---
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.