From 5c76465177ef3b65232dae2ae51028369b023880 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 2 May 2016 15:00:39 +0000 Subject: [PATCH] Bug 14570: Make it possible to add multiple guarantors to a record This patch adds the ability to set an unlimited number of guarantors for a given patron. As before, each guarantor may be linked to another Koha patron, and all the behavior that applies to a given guarantor remains the same. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Find some patrons with guarantors, verify the still have their guarantor 4) Test adding and removing guarantors on a patron record, both Koha users and not 5) Verify the "Add child" button works 6) Verify NoIssuesChargeGuarantees still works 7) Verify tools/cleanborrowers.pl will not delete a guarantor 8) Verify the guarantors are displayed on moremember.pl 9) Verify the guarantor is removed by members/update-child.pl 10) Verify the guarantor is removed by misc/cronjobs/j2a.pl 11) Verify import patrons converts guarantor_id, relationship, contactfirstname, and contactsurname into a guarantor 12) prove t/Patron.t 13) prove t/db_dependent/Circulation.t 14) prove t/db_dependent/Circulation/NoIssuesChargeGuarantees.t 15) prove t/db_dependent/Items.t 16) prove t/db_dependent/Koha/Patrons.t 17) prove t/db_dependent/Members.t 18) prove t/db_dependent/Patron/Relationships.t Signed-off-by: Kim Peine Signed-off-by: Kyle M Hall --- C4/Auth_with_ldap.pm | 1 - C4/Circulation.pm | 2 +- C4/Members.pm | 14 +- C4/SIP/ILS/Patron.pm | 1 - Koha/Object.pm | 5 +- Koha/Patron.pm | 120 ++++-- Koha/Patron/Relationship.pm | 73 ++++ Koha/Patron/Relationships.pm | 99 +++++ Koha/Patrons.pm | 12 - Koha/Patrons/Import.pm | 16 + Koha/REST/V1/Auth.pm | 2 +- Koha/Schema/Result/Borrower.pm | 6 - Koha/Schema/Result/BorrowerRelationship.pm | 118 +++++ api/v1/swagger/definitions/patron.json | 4 - circ/circulation.pl | 16 +- .../data/mysql/atomicupdate/bug_14570.perl | 52 +++ installer/data/mysql/kohastructure.sql | 19 +- koha-tmpl/intranet-tmpl/prog/en/columns.def | 1 - .../prog/en/includes/members-toolbar.inc | 13 +- .../prog/en/modules/circ/circulation.tt | 2 +- .../prog/en/modules/members/memberentrygen.tt | 402 ++++++++++-------- .../en/modules/members/moremember-brief.tt | 26 +- .../prog/en/modules/members/moremember.tt | 19 +- koha-tmpl/intranet-tmpl/prog/js/members.js | 92 ++-- .../bootstrap/en/modules/opac-memberentry.tt | 11 +- .../bootstrap/en/modules/opac-privacy.tt | 9 +- .../bootstrap/en/modules/opac-user.tt | 8 +- members/deletemem.pl | 6 +- members/memberentry.pl | 118 ++--- members/moremember.pl | 28 +- members/update-child.pl | 18 +- misc/cronjobs/j2a.pl | 76 +++- opac/opac-memberentry.pl | 4 +- opac/opac-user.pl | 15 +- t/Patron.t | 24 +- .../Circulation/NoIssuesChargeGuarantees.t | 24 +- t/db_dependent/Items.t | 6 +- t/db_dependent/Koha/Patrons.t | 149 +++++-- t/db_dependent/Members.t | 17 +- t/db_dependent/Patron/Relationships.t | 167 ++++++++ tools/import_borrowers.pl | 1 + 41 files changed, 1281 insertions(+), 515 deletions(-) create mode 100644 Koha/Patron/Relationship.pm create mode 100644 Koha/Patron/Relationships.pm create mode 100644 Koha/Schema/Result/BorrowerRelationship.pm create mode 100644 installer/data/mysql/atomicupdate/bug_14570.perl create mode 100755 t/db_dependent/Patron/Relationships.t diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index 6f3b9a3828..09db190255 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -468,7 +468,6 @@ C4::Auth - Authenticates Koha users | contactname | mediumtext | YES | | NULL | | | contactfirstname | text | YES | | NULL | | | contacttitle | text | YES | | NULL | | - | guarantorid | int(11) | YES | MUL | NULL | | | borrowernotes | mediumtext | YES | | NULL | | | relationship | varchar(100) | YES | | NULL | | | ethnicity | varchar(50) | YES | | NULL | | diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e1b0ed6348..c7f6a73012 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -786,7 +786,7 @@ sub CanBookBeIssued { my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees"); $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); if ( defined $no_issues_charge_guarantees ) { - my @guarantees = $patron->guarantees(); + my @guarantees = map { $_->guarantee } $patron->guarantee_relationships(); my $guarantees_non_issues_charges; foreach my $g ( @guarantees ) { $guarantees_non_issues_charges += $g->account->non_issues_charges; diff --git a/C4/Members.pm b/C4/Members.pm index 205f45bd7b..6f2cb37562 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -176,7 +176,7 @@ sub patronflags { $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); if ( defined $no_issues_charge_guarantees ) { my $p = Koha::Patrons->find( $patroninformation->{borrowernumber} ); - my @guarantees = $p->guarantees(); + my @guarantees = map { $_->guarantee } $p->guarantee_relationships; my $guarantees_non_issues_charges; foreach my $g ( @guarantees ) { $guarantees_non_issues_charges += $g->account->non_issues_charges; @@ -397,18 +397,18 @@ sub GetBorrowersToExpunge { FROM borrowers JOIN categories USING (categorycode) LEFT JOIN ( - SELECT guarantorid - FROM borrowers - WHERE guarantorid IS NOT NULL - AND guarantorid <> 0 - ) as tmp ON borrowers.borrowernumber=tmp.guarantorid + SELECT guarantor_id + FROM borrower_relationships + WHERE guarantor_id IS NOT NULL + AND guarantor_id <> 0 + ) as tmp ON borrowers.borrowernumber=tmp.guarantor_id LEFT JOIN old_issues USING (borrowernumber) LEFT JOIN issues USING (borrowernumber)|; if ( $filterpatronlist ){ $query .= q| LEFT JOIN patron_list_patrons USING (borrowernumber)|; } $query .= q| WHERE category_type <> 'S' - AND tmp.guarantorid IS NULL + AND tmp.guarantor_id IS NULL |; my @query_params; if ( $filterbranch && $filterbranch ne "" ) { diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 34866508e3..8ed94d981b 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -627,7 +627,6 @@ __END__ | contactname | mediumtext | YES | | NULL | | | contactfirstname | text | YES | | NULL | | | contacttitle | text | YES | | NULL | | -| guarantorid | int(11) | YES | MUL | NULL | | | borrowernotes | mediumtext | YES | | NULL | | | relationship | varchar(100) | YES | | NULL | | | ethnicity | varchar(50) | YES | | NULL | | diff --git a/Koha/Object.pm b/Koha/Object.pm index 1c382f26d3..04d34c49e3 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -72,8 +72,9 @@ sub new { next if not exists $attributes->{$column_name} or defined $attributes->{$column_name}; delete $attributes->{$column_name}; } - $self->{_result} = $schema->resultset( $class->_type() ) - ->new($attributes); + + $self->{_result} = + $schema->resultset( $class->_type() )->new($attributes); } croak("No _type found! Koha::Object must be subclassed!") diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 04b54a3ed6..046e0eb22a 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -27,8 +27,10 @@ use Text::Unaccent qw( unac_string ); use C4::Context; use C4::Log; +use Koha::Account; use Koha::AuthUtils; use Koha::Checkouts; +use Koha::Club::Enrollments; use Koha::Database; use Koha::DateUtils; use Koha::Exceptions::Password; @@ -39,12 +41,11 @@ use Koha::Patron::Categories; use Koha::Patron::HouseboundProfile; use Koha::Patron::HouseboundRole; use Koha::Patron::Images; +use Koha::Patron::Relationships; use Koha::Patrons; -use Koha::Virtualshelves; -use Koha::Club::Enrollments; -use Koha::Account; use Koha::Subscription::Routinglists; use Koha::Token; +use Koha::Virtualshelves; use base qw(Koha::Object); @@ -365,41 +366,67 @@ sub category { return Koha::Patron::Category->_new_from_dbic( $self->_result->categorycode ); } -=head3 guarantor - -Returns a Koha::Patron object for this patron's guarantor +=head3 image =cut -sub guarantor { - my ( $self ) = @_; - - return unless $self->guarantorid(); - - return Koha::Patrons->find( $self->guarantorid() ); -} - sub image { my ( $self ) = @_; return scalar Koha::Patron::Images->find( $self->borrowernumber ); } +=head3 library + +Returns a Koha::Library object representing the patron's home library. + +=cut + sub library { my ( $self ) = @_; return Koha::Library->_new_from_dbic($self->_result->branchcode); } -=head3 guarantees +=head3 guarantor_relationships + +Returns Koha::Patron::Relationships object for this patron's guarantors -Returns the guarantees (list of Koha::Patron) of this patron +Returns the set of relationships for the patrons that are guarantors for this patron. + +This is returned instead of a Koha::Patron object because the guarantor +may not exist as a patron in Koha. If this is true, the guarantors name +exists in the Koha::Patron::Relationship object and will have no guarantor_id. =cut -sub guarantees { - my ( $self ) = @_; +sub guarantor_relationships { + my ($self) = @_; - return Koha::Patrons->search( { guarantorid => $self->borrowernumber }, { order_by => { -asc => ['surname','firstname'] } } ); + return Koha::Patron::Relationships->search( { guarantee_id => $self->id } ); +} + +=head3 guarantee_relationships + +Returns Koha::Patron::Relationships object for this patron's guarantors + +Returns the set of relationships for the patrons that are guarantees for this patron. + +The method returns Koha::Patron::Relationship objects for the sake +of consistency with the guantors method. +A guarantee by definition must exist as a patron in Koha. + +=cut + +sub guarantee_relationships { + my ($self) = @_; + + return Koha::Patron::Relationships->search( + { guarantor_id => $self->id }, + { + prefetch => 'guarantee', + order_by => { -asc => [ 'guarantee.surname', 'guarantee.firstname' ] }, + } + ); } =head3 housebound_profile @@ -437,23 +464,22 @@ Returns the siblings of this patron. =cut sub siblings { - my ( $self ) = @_; + my ($self) = @_; - my $guarantor = $self->guarantor; + my @guarantors = $self->guarantor_relationships()->guarantors(); - return unless $guarantor; + return unless @guarantors; - return Koha::Patrons->search( - { - guarantorid => { - '!=' => undef, - '=' => $guarantor->id, - }, - borrowernumber => { - '!=' => $self->borrowernumber, - } - } - ); + my @siblings = + map { $_->guarantee_relationships()->guarantees() } @guarantors; + + return unless @siblings; + + my %seen; + @siblings = + grep { !$seen{ $_->id }++ && ( $_->id != $self->id ) } @siblings; + + return wantarray ? @siblings : Koha::Patrons->search( { borrowernumber => { -in => [ map { $_->id } @siblings ] } } ); } =head3 merge_with @@ -1392,6 +1418,34 @@ sub _anonymize_column { $self->$col($val); } +=head3 add_guarantor + + my @relationships = $patron->add_guarantor( + { + borrowernumber => $borrowernumber, + relationships => $relationship, + } + ); + + Adds a new guarantor to a patron. + +=cut + +sub add_guarantor { + my ( $self, $params ) = @_; + + my $guarantor_id = $params->{guarantor_id}; + my $relationship = $params->{relationship}; + + return Koha::Patron::Relationship->new( + { + guarantee_id => $self->id, + guarantor_id => $guarantor_id, + relationship => $relationship + } + )->store(); +} + =head2 Internal methods =head3 _type diff --git a/Koha/Patron/Relationship.pm b/Koha/Patron/Relationship.pm new file mode 100644 index 0000000000..24994aa2b7 --- /dev/null +++ b/Koha/Patron/Relationship.pm @@ -0,0 +1,73 @@ +package Koha::Patron::Relationship; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use Carp; + +use Koha::Database; + +use base qw(Koha::Object); + +=head1 NAME + +Koha::Patron::Relationship - A class to represent relationships between patrons + +Patrons in Koha may be guarantors or guarantees. This class models that relationship +and provides a way to access those relationships. + +=head1 API + +=head2 Class Methods + +=cut + +=head3 guarantor + +Returns the Koha::Patron object for the guarantor, if there is one + +=cut + +sub guarantor { + my ( $self ) = @_; + + return unless $self->guarantor_id; + + return scalar Koha::Patrons->find( $self->guarantor_id ); +} + +=head3 guarantee + +Returns the Koha::Patron object for the guarantee + +=cut + +sub guarantee { + my ( $self ) = @_; + + return scalar Koha::Patrons->find( $self->guarantee_id ); +} + +=head3 type + +=cut + +sub _type { + return 'BorrowerRelationship'; +} + +1; diff --git a/Koha/Patron/Relationships.pm b/Koha/Patron/Relationships.pm new file mode 100644 index 0000000000..0cdfa9fb09 --- /dev/null +++ b/Koha/Patron/Relationships.pm @@ -0,0 +1,99 @@ +package Koha::Patron::Relationships; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use Carp; +use List::MoreUtils qw( uniq ); + +use Koha::Database; +use Koha::Patrons; +use Koha::Patron::Relationship; + +use base qw(Koha::Objects); + +=head1 NAME + +Koha::Patron::Relationships - Koha Patron Relationship Object set class + +=head1 API + +=head2 Class Methods + +=cut + +=head3 guarantors + +Returns all the guarantors in this set of relationships as a list of Koha::Patron objects +or as a Koha::Patrons object depending on the calling context + +=cut + +sub guarantors { + my ($self) = @_; + + my $rs = $self->_resultset(); + + my @guarantor_ids = $rs->get_column('guarantor_id')->all(); + # Guarantors may not have a guarantor_id, strip out undefs + @guarantor_ids = grep { defined $_ } @guarantor_ids; + @guarantor_ids = uniq( @guarantor_ids ); + + my $guarantors = Koha::Patrons->search( { borrowernumber => \@guarantor_ids } ); + + return wantarray ? $guarantors->as_list : $guarantors; +} + +=head3 guarantees + +Returns all the guarantees in this set of relationships as a list of Koha::Patron objects +or as a Koha::Patrons object depending on the calling context + +=cut + +sub guarantees { + my ($self) = @_; + + my $rs = $self->_resultset(); + + my @guarantee_ids = uniq( $rs->get_column('guarantee_id')->all() ); + + my $guarantees = Koha::Patrons->search( + { borrowernumber => \@guarantee_ids }, + { + order_by => { -asc => [ 'surname', 'firstname' ] } + }, + ); + + return wantarray ? $guarantees->as_list : $guarantees; +} + +=cut + +=head3 type + +=cut + +sub _type { + return 'BorrowerRelationship'; +} + +sub object_class { + return 'Koha::Patron::Relationship'; +} + +1; diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 7510093185..a38d21f971 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -125,18 +125,6 @@ sub search_upcoming_membership_expires { ); } -=head3 guarantor - -Returns a Koha::Patron object for this borrower's guarantor - -=cut - -sub guarantor { - my ( $self ) = @_; - - return Koha::Patrons->find( $self->guarantorid() ); -} - =head3 search_patrons_to_anonymise my $patrons = Koha::Patrons->search_patrons_to_anonymise( { before => $older_than_date, [ library => $library ] } ); diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm index fe63b9567e..435d33ea22 100644 --- a/Koha/Patrons/Import.pm +++ b/Koha/Patrons/Import.pm @@ -214,6 +214,11 @@ sub import_patrons { next LINE; } + my $relationship = $borrower{relationship}; + my $guarantor_id = $borrower{guarantor_id}; + delete $borrower{relationship}; + delete $borrower{guarantor_id}; + if ($borrowernumber) { # borrower exists @@ -350,6 +355,17 @@ sub import_patrons { ); } } + + # Add a guarantor if we are given a relationship + if ( $guarantor_id ) { + Koha::Patron::Relationship->new( + { + guarantee_id => $borrowernumber, + relationship => $relationship, + guarantor_id => $guarantor_id, + } + )->store(); + } } return { diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index 5e505cb47b..23c9181c4b 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -314,7 +314,7 @@ sub allow_guarantor { return; } - my $guarantees = $user->guarantees->as_list; + my $guarantees = $user->guarantee_relationships->guarantees->as_list; foreach my $guarantee (@{$guarantees}) { return 1 if check_object_ownership($c, $guarantee); } diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index 70007115d2..024a6da8da 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -1532,12 +1532,6 @@ __PACKAGE__->many_to_many("ordernumbers", "aqorder_users", "ordernumber"); # Created by DBIx::Class::Schema::Loader v0.07046 @ 2019-04-25 10:08:38 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3qd/l8OkObSn8gTKTsHrkA -__PACKAGE__->belongs_to( - "guarantor", - "Koha::Schema::Result::Borrower", - { borrowernumber => "guarantorid" }, -); - __PACKAGE__->add_columns( '+anonymized' => { is_boolean => 1 }, '+lost' => { is_boolean => 1 }, diff --git a/Koha/Schema/Result/BorrowerRelationship.pm b/Koha/Schema/Result/BorrowerRelationship.pm new file mode 100644 index 0000000000..88cef4f294 --- /dev/null +++ b/Koha/Schema/Result/BorrowerRelationship.pm @@ -0,0 +1,118 @@ +use utf8; +package Koha::Schema::Result::BorrowerRelationship; + +# Created by DBIx::Class::Schema::Loader +# DO NOT MODIFY THE FIRST PART OF THIS FILE + +=head1 NAME + +Koha::Schema::Result::BorrowerRelationship + +=cut + +use strict; +use warnings; + +use base 'DBIx::Class::Core'; + +=head1 TABLE: C + +=cut + +__PACKAGE__->table("borrower_relationships"); + +=head1 ACCESSORS + +=head2 id + + data_type: 'integer' + is_auto_increment: 1 + is_nullable: 0 + +=head2 guarantor_id + + data_type: 'integer' + is_foreign_key: 1 + is_nullable: 1 + +=head2 guarantee_id + + data_type: 'integer' + is_foreign_key: 1 + is_nullable: 0 + +=head2 relationship + + data_type: 'varchar' + is_nullable: 0 + size: 100 + +=cut + +__PACKAGE__->add_columns( + "id", + { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, + "guarantor_id", + { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, + "guarantee_id", + { data_type => "integer", is_foreign_key => 1, is_nullable => 0 }, + "relationship", + { data_type => "varchar", is_nullable => 0, size => 100 }, +); + +=head1 PRIMARY KEY + +=over 4 + +=item * L + +=back + +=cut + +__PACKAGE__->set_primary_key("id"); + +=head1 RELATIONS + +=head2 guarantee + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "guarantee", + "Koha::Schema::Result::Borrower", + { borrowernumber => "guarantee_id" }, + { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" }, +); + +=head2 guarantor + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "guarantor", + "Koha::Schema::Result::Borrower", + { borrowernumber => "guarantor_id" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "CASCADE", + on_update => "CASCADE", + }, +); + + +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2018-07-18 10:52:59 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:R8RThgcrct40Zq0UMW3TWQ + + +# You can replace this text with custom code or comments, and it will be preserved on regeneration +1; diff --git a/api/v1/swagger/definitions/patron.json b/api/v1/swagger/definitions/patron.json index 7fca15bf35..10675ac778 100644 --- a/api/v1/swagger/definitions/patron.json +++ b/api/v1/swagger/definitions/patron.json @@ -159,10 +159,6 @@ "readOnly": true, "description": "If any restriction applies to the patron" }, - "guarantor_id": { - "type": ["integer", "null"], - "description": "patron_id used for children or professionals to link them to guarantor or organizations" - }, "staff_notes": { "type": ["string", "null"], "description": "a note on the patron's account" diff --git a/circ/circulation.pl b/circ/circulation.pl index 72a38465ad..31beb2554c 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -501,7 +501,7 @@ if ( $patron ) { $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees ); if ( defined $no_issues_charge_guarantees ) { my $guarantees_non_issues_charges = 0; - my $guarantees = $patron->guarantees; + my $guarantees = $patron->guarantee_relationships->guarantees; while ( my $g = $guarantees->next ) { $guarantees_non_issues_charges += $g->account->non_issues_charges; } @@ -556,14 +556,12 @@ my $view = $batch : 'circview'; my @relatives; -if ( $borrowernumber ) { - if ( $patron ) { - if ( my $guarantor = $patron->guarantor ) { - push @relatives, $guarantor->borrowernumber; - push @relatives, $_->borrowernumber for $patron->siblings; - } else { - push @relatives, $_->borrowernumber for $patron->guarantees; - } +if ( $patron ) { + if ( my @guarantors = $patron->guarantor_relationships()->guarantors() ) { + push( @relatives, $_->id ) for @guarantors; + push( @relatives, $_->id ) for $patron->siblings(); + } else { + push( @relatives, $_->id ) for $patron->guarantee_relationships()->guarantees(); } } my $relatives_issues_count = diff --git a/installer/data/mysql/atomicupdate/bug_14570.perl b/installer/data/mysql/atomicupdate/bug_14570.perl new file mode 100644 index 0000000000..967fd34a21 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_14570.perl @@ -0,0 +1,52 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + # you can use $dbh here like: + # $dbh->do( "ALTER TABLE biblio ADD COLUMN badtaste int" ); + unless (TableExists('borrower_relationships')){ + $dbh->do(q{ + CREATE TABLE `borrower_relationships` ( + id INT(11) NOT NULL AUTO_INCREMENT, + guarantor_id INT(11) NOT NULL, + guarantee_id INT(11) NOT NULL, + relationship VARCHAR(100) NOT NULL, + PRIMARY KEY (id), + CONSTRAINT r_guarantor FOREIGN KEY ( guarantor_id ) REFERENCES borrowers ( borrowernumber ) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT r_guarantee FOREIGN KEY ( guarantee_id ) REFERENCES borrowers ( borrowernumber ) ON UPDATE CASCADE ON DELETE CASCADE + ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + }); + + $dbh->do(q{ + UPDATE borrowers + LEFT JOIN borrowers guarantor ON ( borrowers.guarantorid = guarantor.borrowernumber ) + SET borrowers.guarantorid = NULL WHERE guarantor.borrowernumber IS NULL; + }); + + $dbh->do(q{ + INSERT INTO borrower_relationships ( guarantor_id, guarantee_id, relationship ) + SELECT guarantorid, borrowernumber, relationship FROM borrowers WHERE guarantorid IS NOT NULL; + }); + + } + + if( column_exists( 'borrowers', 'guarantorid' ) ) { + $dbh->do(q{ + ALTER TABLE borrowers DROP guarantorid; + }); + } + + if( column_exists( 'deletedborrowers', 'guarantorid' ) ) { + $dbh->do(q{ + ALTER TABLE deletedborrowers DROP guarantorid; + }); + } + + if( column_exists( 'borrower_modifications', 'guarantorid' ) ) { + $dbh->do(q{ + ALTER TABLE borrower_modifications DROP guarantorid; + }); + } + + # Always end with this (adjust the bug info) + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 14570 - Make it possible to add multiple guarantors to a record)\n"; +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index da3ff83f37..258c71fb14 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -579,7 +579,6 @@ CREATE TABLE `deletedborrowers` ( -- stores data related to the patrons/borrower `contactname` LONGTEXT, -- used for children and profesionals to include surname or last name of guarantor or organization name `contactfirstname` MEDIUMTEXT, -- used for children to include first name of guarantor `contacttitle` MEDIUMTEXT, -- used for children to include title (Mr., Mrs., etc) of guarantor - `guarantorid` int(11) default NULL, -- borrowernumber used for children or professionals to link them to guarantors or organizations `borrowernotes` LONGTEXT, -- a note on the patron/borrower's account that is only visible in the staff client `relationship` varchar(100) default NULL, -- used for children to include the relationship to their guarantor `sex` varchar(1) default NULL, -- patron/borrower's gender @@ -1613,7 +1612,6 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons `contactname` LONGTEXT, -- used for children and profesionals to include surname or last name of guarantor or organization name `contactfirstname` MEDIUMTEXT, -- used for children to include first name of guarantor `contacttitle` MEDIUMTEXT, -- used for children to include title (Mr., Mrs., etc) of guarantor - `guarantorid` int(11) default NULL, -- borrowernumber used for children or professionals to link them to guarantors or organizations `borrowernotes` LONGTEXT, -- a note on the patron/borrower's account that is only visible in the staff client `relationship` varchar(100) default NULL, -- used for children to include the relationship to their guarantor `sex` varchar(1) default NULL, -- patron/borrower's gender @@ -1649,7 +1647,6 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons KEY `categorycode` (`categorycode`), KEY `branchcode` (`branchcode`), UNIQUE KEY `userid` (`userid`), - KEY `guarantorid` (`guarantorid`), KEY `surname_idx` (`surname` (191)), KEY `firstname_idx` (`firstname` (191)), KEY `othernames_idx` (`othernames` (191)), @@ -3439,7 +3436,6 @@ CREATE TABLE IF NOT EXISTS `borrower_modifications` ( `contactname` LONGTEXT, `contactfirstname` MEDIUMTEXT, `contacttitle` MEDIUMTEXT, - `guarantorid` int(11) DEFAULT NULL, `borrowernotes` LONGTEXT, `relationship` varchar(100) DEFAULT NULL, `sex` varchar(1) DEFAULT NULL, @@ -4137,6 +4133,21 @@ CREATE TABLE IF NOT EXISTS club_fields ( CONSTRAINT club_fields_ibfk_4 FOREIGN KEY (club_id) REFERENCES clubs (id) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; +-- +-- Table structure for table 'guarantors_guarantees' +-- + +DROP TABLE IF EXISTS borrower_relationships; +CREATE TABLE `borrower_relationships` ( + id INT(11) NOT NULL AUTO_INCREMENT, + guarantor_id INT(11) NULL DEFAULT NULL, + guarantee_id INT(11) NOT NULL, + relationship VARCHAR(100) NOT NULL, + PRIMARY KEY (id), + CONSTRAINT r_guarantor FOREIGN KEY ( guarantor_id ) REFERENCES borrowers ( borrowernumber ) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT r_guarantee FOREIGN KEY ( guarantee_id ) REFERENCES borrowers ( borrowernumber ) ON UPDATE CASCADE ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + -- -- Table structure for table `illrequests` -- diff --git a/koha-tmpl/intranet-tmpl/prog/en/columns.def b/koha-tmpl/intranet-tmpl/prog/en/columns.def index 05112eb704..49cdd63369 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/columns.def +++ b/koha-tmpl/intranet-tmpl/prog/en/columns.def @@ -7,7 +7,6 @@ Other name Gender Relationship -Guarantor borrower number Street number Street type Address diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc index 1ab510c5b1..981966e7eb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc @@ -6,21 +6,14 @@ [% USE AuthorisedValues %]
[% IF CAN_user_borrowers_edit_borrowers %] - [% IF ( guarantor ) %] - - [% ELSE %] - - [% END %] - Edit + Edit [% END %] [% IF CAN_user_borrowers_edit_borrowers %] [% IF patron.is_adult AND Koha.Preference("borrowerRelationship") %] - Add child - [% END %] - [% IF CAN_user_borrowers_edit_borrowers %] - Change password + Add child [% END %] + Change password Duplicate [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index 5ddcb78acf..acd84c5c17 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -832,7 +832,7 @@ [% IF relatives_issues_count %] -
  • Relatives' checkouts
    • +
  • [% relatives_issues_count %] Relatives' checkouts
    • [% END %]
  • diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index 62fc2db2de..ec200302ff 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1,5 +1,6 @@ [% USE raw %] [% USE Asset %] +[% USE To %] [% USE Koha %] [% USE KohaDates %] [% USE Branches %] @@ -317,113 +318,122 @@ [% END # hide fieldset %] -[% IF ( showguarantor ) %] - - [% UNLESS step_6 %] - - [% END %] -
    - Guarantor information -
      -[% IF ( P ) %] - [% IF ( guarantorid ) %] -
    1. - [% ELSE %] -
      2. -
    2. - - [% IF ( guarantorid ) %] - [% contactname | html %] - - [% ELSE %] - - [% END %] -
      3. -[% ELSE %] - [% IF ( C ) %] - [% IF ( guarantorid ) %] -
    3. - [% ELSE %] -
      4. - [% UNLESS nocontactname %] -
    4. - - [% IF ( guarantorid ) %] - [% contactname | html %] - - [% ELSE %] - - [% END %] -
      5. - [% END %] - [% UNLESS nocontactfirstname %] -
    5. - - [% IF ( guarantorid ) %] - [% contactfirstname | html %] - - [% ELSE %] - - [% END %] -
      6. - [% END %] - [% IF ( relshiploop ) %] -
    6. - - -
      7. - [% END %] - [% END %] -[% END %] -
    7. -   - [% IF ( guarantorid ) %] - - [% ELSE %] - - [% END %] - -
      8. - [% IF guarantorid && Koha.Preference('AllowStaffToSetCheckoutsVisibilityForGuarantor') %] -
    8. - - +
      9. + [% END %] +
    +
    + [% END # END relationships foreach %] + + +
    +
      +
    1. + Patron #: + + +
      2. + +
    2. + + +
      3. + +
    3. + + +
      4. + +
    4. + + +
      5. + +
    5. + + Cancel +
      6. +
    +
    + +
      + + + + +
    1. + Search +
      2. + + [% IF relationships && Koha.Preference('AllowStaffToSetCheckoutsVisibilityForGuarantor') %] +
    2. + + +
      Allow guarantors of this patron to view this patron's checkouts from the OPAC
      +
      3. [% END %] - -
      Allow guarantor of this patron to view this patron's checkouts from the OPAC
      - - [% END %] -
    + - [% END %] + + [% UNLESS noaddress && noaddress2 && nocity && nostate && nozipcode && nocountry %] [% IF Koha.Preference( 'AddressFormat' ) %] [% INCLUDE "member-main-address-style-${ Koha.Preference( 'AddressFormat' ) }.inc" %] @@ -431,84 +441,132 @@ [% END # nostreet && nocity etc group%] [% UNLESS nophone && nophonepro && nomobile && noemail && noemailpro && nofax %] -
    - Contact
      - [% UNLESS nophone %] -
    1. - [% IF ( mandatoryphone ) %] -
    +
    [%END # hide fieldset %] @@ -1172,6 +1230,10 @@ } [% END %] + [% IF guarantor %] + select_user( '[% guarantor.borrowernumber %]', [% To.json( guarantor.unblessed ) | $raw %] ); + [% END %] + $("#cn_max").hide(); var content; $("#cardnumber").on("keydown", function(e){ diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt index c68fd9d63c..5170914ffc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt @@ -1,3 +1,4 @@ +[% USE raw %] [% USE Koha %] [% USE KohaDates %] [% SET footerjs = 1 %] @@ -40,13 +41,26 @@ [% IF ( patron.dateofbirth ) %]
  • Date of birth:[% patron.dateofbirth | $KohaDates %]
    • [% END %] [% IF ( patron.sex ) %]
  • Gender:[% IF ( patron.sex == 'F' ) %]Female[% ELSIF ( patron.sex == 'M' ) %]Male[% ELSE %][% patron.sex | html %][% END %]
    • [% END %] [% END %] - [% IF ( guarantees ) %] -
  • Guarantees:
    • - [% END %] - [% IF ( guarantor ) %] -
  • Guarantor:[% guarantor.surname | html %], [% guarantor.firstname | html %]
    • + + [% IF guarantees %] +
  • + Guarantees: + +
    • + [% ELSIF guarantor_relationships %] + [% FOREACH gr IN guarantor_relationships %] +
  • + Guarantor: + [% SET guarantor = gr.guarantor %] + [% guarantor.firstname | html %] [% guarantor.surname | html %] +
    • + [% END %] [% END %] - +
    diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index e967d56f68..96cf8bd550 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -265,15 +265,16 @@ [% END %] - [% ELSIF guarantor %] -
  • - Guarantor: - [% IF guarantor.borrowernumber AND logged_in_user.can_see_patron_infos( guarantor ) %] - [% guarantor.firstname | html %] [% guarantor.surname | html %] - [% ELSE %] - [% guarantor.firstname | html %] [% guarantor.surname | html %] - [% END %] -
    • + [% ELSIF guarantor_relationships %] + [% FOREACH gr IN guarantor_relationships %] +
  • + Guarantor: + [% SET guarantor = gr.guarantor %] + [% IF logged_in_user.can_see_patron_infos( guarantor ) %] + [% guarantor.firstname | html %] [% guarantor.surname | html %] + [% END %] +
    • + [% END %] [% END %] [% # /div.rows %] diff --git a/koha-tmpl/intranet-tmpl/prog/js/members.js b/koha-tmpl/intranet-tmpl/prog/js/members.js index bcca8ef83c..606c394238 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/members.js +++ b/koha-tmpl/intranet-tmpl/prog/js/members.js @@ -129,10 +129,6 @@ function Dopop(link) { var newin=window.open(link,'popup','width=600,height=400,resizable=no,toolbar=false,scrollbars=no,top'); } -function Dopopguarantor(link) { - var newin=window.open(link,'popup','width=800,height=500,resizable=no,toolbar=false,scrollbars=yes,top'); -} - function clear_entry(node) { var original = $(node).parent(); $("textarea", original).attr('value', ''); @@ -170,48 +166,38 @@ function update_category_code(category_code) { } function select_user(borrowernumber, borrower) { - var form = $('#entryform').get(0); - if (form.guarantorid.value) { - $("#contact-details, #quick_add_form #contact-details").find('a').remove(); - $("#contactname, #contactfirstname, #quick_add_form #contactname, #quick_add_form #contactfirstname").parent().find('span').remove(); + $('#guarantor_id').val(borrower.borrowernumber); + $('#guarantor_surname').val(borrower.surname); + $('#guarantor_firstname').val(borrower.firstname); + + var fieldset = $('#guarantor_template').clone(); + fieldset.removeAttr('id'); + + var guarantor_id = $('#guarantor_id').val(); + if ( guarantor_id ) { + fieldset.find('.new_guarantor_id').first().val( guarantor_id ); + fieldset.find('.new_guarantor_id_text').first().text( guarantor_id ); + } else { + fieldset.find('.guarantor_id').first().hide(); } + $('#guarantor_id').val(""); + + var guarantor_surname = $('#guarantor_surname').val(); + fieldset.find('.new_guarantor_surname').first().val( guarantor_surname ); + fieldset.find('.new_guarantor_surname_text').first().text( guarantor_surname ); + $('#guarantor_surname').val(""); + + var guarantor_firstname = $('#guarantor_firstname').val(); + fieldset.find('.new_guarantor_firstname').first().val( guarantor_firstname ); + fieldset.find('.new_guarantor_firstname_text').first().text( guarantor_firstname ); + $('#guarantor_firstname').val(""); - var id = borrower.borrowernumber; - form.guarantorid.value = id; - $('#contact-details, #quick_add_form #contact-details') - .show() - .find('span') - .after('' + id + ''); - - $(form.contactname) - .val(borrower.surname) - .before('' + borrower.surname + '').get(0).type = 'hidden'; - $("#quick_add_form #contactname").val(borrower.surname).before(''+borrower.surname+'' + borrower.firstname + '').get(0).type = 'hidden'; - $("#quick_add_form #contactfirstname").val(borrower.firstname).before(''+borrower.firstname+'You typed in the wrong characters in the box before submitting. Please try again. [% END %] - [% IF borrower.guarantorid && !Koha.Preference('OPACPrivacy') && Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %] + [% IF patron.guarantor_relationships && !Koha.Preference('OPACPrivacy') && Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %]
    Privacy
      @@ -119,7 +119,12 @@ - Your guarantor is [% guarantor.firstname | html %] [% guarantor.surname | html %] + Guaranteed by + [% FOREACH gr IN patron.guarantor_relationships %] + [% SET g = gr.guarantor %] + [% g.firstname | html %] [% g.surname | html %] + [%- IF ! loop.last %], [% END %] + [% END %]
    @@ -1014,7 +1019,7 @@ } }); - [% IF borrower.guarantorid && !Koha.Preference('OPACPrivacy') && Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %] + [% IF patron.guarantor_relationships && !Koha.Preference('OPACPrivacy') && Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %] $('#update_privacy_guarantor_checkouts').click( function() { $.post( "/cgi-bin/koha/svc/patron/show_checkouts_to_relatives", { privacy_guarantor_checkouts: $('#privacy_guarantor_checkouts').val() }, null, 'json') .done(function( data ) { diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt index 84a2519d20..6583b7c040 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-privacy.tt @@ -71,7 +71,7 @@ - [% IF borrower.guarantorid && Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %] + [% IF borrower.guarantor_relationships && Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %]
    - Your guarantor is [% borrower.guarantor.firstname | html %] [% borrower.guarantor.surname | html %] + Guaranteed by + [% FOREACH gr IN borrower.guarantor_relationships %] + [% SET g = gr.guarantor %] + [% g.firstname | html %] [% g.surname | html %] + [%- IF ! loop.last %], [% END %] + [% END %]
    [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt index 12aad6bbe0..60c7039f45 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -428,7 +428,7 @@ [% FOREACH r IN relatives %] - [% FOREACH i IN r.issues %] + [% FOREACH c IN r.checkouts %] @@ -437,15 +437,15 @@ - [% i.date_due | $KohaDates %] + [% c.date_due | $KohaDates %] - [% i.item.barcode | html %] + [% c.item.barcode | html %] - [% i.item.itemcallnumber | html %] + [% c.item.itemcallnumber | html %] diff --git a/members/deletemem.pl b/members/deletemem.pl index 3d8712ede7..4efe3ddda9 100755 --- a/members/deletemem.pl +++ b/members/deletemem.pl @@ -84,7 +84,7 @@ if (C4::Context->preference("IndependentBranches")) { my $op = $input->param('op') || 'delete_confirm'; my $dbh = C4::Context->dbh; -my $is_guarantor = $dbh->selectrow_array("SELECT COUNT(*) FROM borrowers WHERE guarantorid=?", undef, $member); +my $is_guarantor = $patron->guarantee_relationships->count; if ( $op eq 'delete_confirm' or $countissues > 0 or $charges or $is_guarantor ) { $template->param( @@ -96,8 +96,8 @@ if ( $op eq 'delete_confirm' or $countissues > 0 or $charges or $is_guarantor ) if ( $charges > 0 ) { $template->param(charges => $charges); } - if ($is_guarantor) { - $template->param(guarantees => 1); + if ( $is_guarantor ) { + $template->param( guarantees => 1 ); } # This is silly written but reflect the same conditions as above diff --git a/members/memberentry.pl b/members/memberentry.pl index 166302006b..dad66b3a5a 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -83,7 +83,6 @@ if ( C4::Context->preference('SMSSendDriver') eq 'Email' ) { $template->param( sms_providers => \@providers ); } -my $guarantorid = $input->param('guarantorid'); my $actionType = $input->param('actionType') || ''; my $modify = $input->param('modify'); my $delete = $input->param('delete'); @@ -99,13 +98,25 @@ $nodouble = 1 if ($op eq 'modify' or $op eq 'duplicate'); # FIXME hack to rep # isn't a duplicate. Marking FIXME because this # script needs to be refactored. my $nok = $input->param('nok'); -my $guarantorinfo = $input->param('guarantorinfo'); my $step = $input->param('step') || 0; my @errors; my $borrower_data; my $NoUpdateLogin; my $userenv = C4::Context->userenv; +## Deal with guarantor stuff +$template->param( relationships => scalar $patron->guarantor_relationships ) if $patron; + +my $guarantor_id = $input->param('guarantor_id'); +my $guarantor = Koha::Patrons->find( $guarantor_id ) if $guarantor_id; +$template->param( guarantor => $guarantor ); + +my @delete_guarantor = $input->multi_param('delete_guarantor'); +foreach my $id ( @delete_guarantor ) { + my $r = Koha::Patron::Relationships->find( $id ); + $r->delete() if $r; +} + ## Deal with debarments $template->param( debarments => scalar GetDebarments( { borrowernumber => $borrowernumber } ) ); @@ -137,14 +148,14 @@ $template->param("uppercasesurnames" => C4::Context->preference('uppercasesurnam my $check_BorrowerMandatoryField=C4::Context->preference("BorrowerMandatoryField"); my @field_check=split(/\|/,$check_BorrowerMandatoryField); foreach (@field_check) { - $template->param( "mandatory$_" => 1); + $template->param( "mandatory$_" => 1 ); } # function to designate unwanted fields my $check_BorrowerUnwantedField=C4::Context->preference("BorrowerUnwantedField"); @field_check=split(/\|/,$check_BorrowerUnwantedField); foreach (@field_check) { next unless m/\w/o; - $template->param( "no$_" => 1); + $template->param( "no$_" => 1 ); } $template->param( "add" => 1 ) if ( $op eq 'add' ); $template->param( "quickadd" => 1 ) if ( $quickadd ); @@ -224,6 +235,10 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' ) qr/^add_debarment$/, qr/^debarred_expiration$/, # We already dealt with debarments previously qr/^housebound_chooser$/, qr/^housebound_deliverer$/, qr/^select_city$/, + qr/^new_guarantor_/, + qr/^guarantor_firstname$/, + qr/^guarantor_surname$/, + qr/^delete_guarantor$/, ); for my $regexp (@keys_to_delete) { for (keys %newdata) { @@ -248,26 +263,6 @@ if ( ( $op eq 'insert' ) and !$nodouble ) { } } - #recover all data from guarantor address phone ,fax... -if ( $guarantorid ) { - if (my $guarantor = Koha::Patrons->find( $guarantorid )) { - my $guarantordata = $guarantor->unblessed; - $category_type = $guarantordata->{categorycode} eq 'I' ? 'P' : 'C'; - $guarantorinfo=$guarantordata->{'surname'}." , ".$guarantordata->{'firstname'}; - $newdata{'contactfirstname'}= $guarantordata->{'firstname'}; - $newdata{'contactname'} = $guarantordata->{'surname'}; - $newdata{'contacttitle'} = $guarantordata->{'title'}; - 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)) { - $newdata{$_} = $guarantordata->{$_}; - } - } - } -} - ###############test to take the right zipcode, country and city name ############## # set only if parameter was passed from the form $newdata{'city'} = $input->param('city') if defined($input->param('city')); @@ -435,6 +430,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ # Lot of code will need to be removed from this script to handle exceptions raised by Koha::Patron->store warn "Patron creation failed! - $@"; # Maybe we must die instead of just warn } else { + add_guarantors( $patron, $input ); $borrowernumber = $patron->borrowernumber; $newdata{'borrowernumber'} = $borrowernumber; } @@ -542,6 +538,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ $patron->set_password({ password => $password }); } + add_guarantors( $patron, $input ); if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); } @@ -640,11 +637,10 @@ foreach my $category_type (qw(C A S P I X)) { 'categoryloop' => \@categoryloop }; } - -$template->param('typeloop' => \@typeloop, - no_categories => $no_categories); -if($no_categories){ $no_add = 1; } - +$template->param( + typeloop => \@typeloop, + no_categories => $no_categories, +); my $cities = Koha::Cities->search( {}, { order_by => 'city_name' } ); my $roadtypes = C4::Koha::GetAuthorisedValues( 'ROADTYPE' ); @@ -669,24 +665,27 @@ while (@relationships) { push(@relshipdata, \%row); } -my %flags = ( 'gonenoaddress' => ['gonenoaddress' ], - 'lost' => ['lost']); +my %flags = ( + 'gonenoaddress' => ['gonenoaddress'], + 'lost' => ['lost'] +); - my @flagdata; -foreach (keys(%flags)) { - my $key = $_; - my %row = ('key' => $key, - 'name' => $flags{$key}[0]); - if ($data{$key}) { - $row{'yes'}=' checked'; - $row{'no'}=''; - } - else { - $row{'yes'}=''; - $row{'no'}=' checked'; - } - push @flagdata,\%row; +foreach ( keys(%flags) ) { + my $key = $_; + my %row = ( + 'key' => $key, + 'name' => $flags{$key}[0] + ); + if ( $data{$key} ) { + $row{'yes'} = ' checked'; + $row{'no'} = ''; + } + else { + $row{'yes'} = ''; + $row{'no'} = ' checked'; + } + push @flagdata, \%row; } # get Branch Loop @@ -761,7 +760,7 @@ if (C4::Context->preference('EnhancedMessagingPreferences')) { $template->param(TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification")); } -$template->param( "showguarantor" => ($category_type=~/A|I|S|X/) ? 0 : 1); # associate with step to know where you are +$template->param( "show_guarantor" => ( $category_type =~ /A|I|S|X/ ) ? 0 : 1 ); # associate with step to know where you are $debug and warn "memberentry step: $step"; $template->param(%data); $template->param( "step_$step" => 1) if $step; # associate with step to know where u are @@ -775,17 +774,12 @@ $template->param( check_member => $check_member,#to know if the borrower already exist(=>1) or not (=>0) "op$op" => 1); -$guarantorid = $borrower_data->{'guarantorid'} || $guarantorid; -my $guarantor = $guarantorid ? Koha::Patrons->find( $guarantorid ) : undef; $template->param( patron => $patron ? $patron : \%newdata, # Used by address include templates now nodouble => $nodouble, borrowernumber => $borrowernumber, #register number - guarantor => $guarantor, - guarantorid => $guarantorid, relshiploop => \@relshipdata, btitle=> $default_borrowertitle, - guarantorinfo => $guarantorinfo, flagloop => \@flagdata, category_type =>$category_type, modify => $modify, @@ -913,6 +907,28 @@ sub patron_attributes_form { } +sub add_guarantors { + my ( $patron, $input ) = @_; + + my @new_guarantor_id = $input->multi_param('new_guarantor_id'); + my @new_guarantor_relationship = $input->multi_param('new_guarantor_relationship'); + + for ( my $i = 0 ; $i < scalar @new_guarantor_id; $i++ ) { + my $guarantor_id = $new_guarantor_id[$i]; + my $relationship = $new_guarantor_relationship[$i]; + + next unless $guarantor_id; + + $patron->add_guarantor( + { + guarantee_id => $patron->id, + guarantor_id => $guarantor_id, + relationship => $relationship, + } + ); + } +} + # Local Variables: # tab-width: 8 # End: diff --git a/members/moremember.pl b/members/moremember.pl index 1f63180e58..a0b365fc0d 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -99,22 +99,20 @@ if ( $patron->is_debarred ) { $template->param( flagged => 1 ) if $patron->account_locked; my @relatives; -if ( my $guarantor = $patron->guarantor ) { - $template->param( guarantor => $guarantor ); - push @relatives, $guarantor->borrowernumber; - push @relatives, $_->borrowernumber for $patron->siblings; -} elsif ( $patron->contactname || $patron->contactfirstname ) { - $template->param( - guarantor => { - firstname => $patron->contactfirstname, - surname => $patron->contactname, - } - ); -} else { - my @guarantees = $patron->guarantees; - $template->param( guarantees => \@guarantees ); - push @relatives, $_->borrowernumber for @guarantees; +my $guarantor_relationships = $patron->guarantor_relationships; +my @guarantees = $patron->guarantee_relationships->guarantees; +my @guarantors = $guarantor_relationships->guarantors; +if (@guarantors) { + push( @relatives, $_->id ) for @guarantors; + push( @relatives, $_->id ) for $patron->siblings(); } +else { + push( @relatives, $_->id ) for @guarantees; +} +$template->param( + guarantor_relationships => $guarantor_relationships, + guarantees => \@guarantees, +); my $relatives_issues_count = Koha::Checkouts->count({ borrowernumber => \@relatives }); diff --git a/members/update-child.pl b/members/update-child.pl index b0aff0d852..60d239a76c 100755 --- a/members/update-child.pl +++ b/members/update-child.pl @@ -33,6 +33,7 @@ use C4::Auth; use C4::Output; use Koha::Patrons; use Koha::Patron::Categories; +use Koha::Patrons; # use Smart::Comments; @@ -68,7 +69,6 @@ if ( $op eq 'multi' ) { ); output_html_with_http_headers $input, $cookie, $template->output; } - elsif ( $op eq 'update' ) { my $patron = Koha::Patrons->find( $borrowernumber ); output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); @@ -84,7 +84,8 @@ elsif ( $op eq 'update' ) { # But we should not hit that with a normal use of the interface die "You are doing something wrong updating this child" unless $adult_category; - $patron->guarantorid(undef); + $_->delete() for $patron->guarantor_relationships(); + $patron->categorycode($adult_category->categorycode); $patron->store; @@ -92,12 +93,15 @@ elsif ( $op eq 'update' ) { # We could redirect with a friendly message if ( $patron_categories->count > 1 ) { $template->param( - SUCCESS => 1, - borrowernumber => $borrowernumber, - ); + SUCCESS => 1, + borrowernumber => $borrowernumber, + ); output_html_with_http_headers $input, $cookie, $template->output; - } else { - print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$borrowernumber"); + } + else { + print $input->redirect( + "/cgi-bin/koha/members/moremember.pl?borrowernumber=$borrowernumber" + ); } } diff --git a/misc/cronjobs/j2a.pl b/misc/cronjobs/j2a.pl index cdbb8f81ed..a63666be39 100755 --- a/misc/cronjobs/j2a.pl +++ b/misc/cronjobs/j2a.pl @@ -96,6 +96,7 @@ C - Suggests that you read this help. :) C -b= -f= -t= - Processes a single branch, and updates the patron categories from fromcat to tocat. C -f= -t= -v -n - Processes all branches, shows all messages, and reports the patrons who would be affected. Takes no action on the database. + =cut # These variables are set by command line options. @@ -128,7 +129,9 @@ if ( not $fromcat && $tocat ) { #make sure we've specified the info we need. cronlogaction(); -my $dbh = C4::Context->dbh; +my $dbh = C4::Context->dbh; +my $database = Koha::Database->new(); +my $schema = $database->schema; #get today's date, format it and subtract upperagelimit my ( $sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst ) = @@ -164,12 +167,12 @@ $verbose and print "The age limit for category $fromcat is $agelimit\n"; my $itsyourbirthday = "$year-$mon-$mday"; if ( not $noaction ) { + # Start a transaction since we need to delete from relationships and update borrowers atomically + + my $success = 1; if ($mybranch) { #yep, we received a specific branch to work on. $verbose and print "Looking for patrons of $mybranch to update from $fromcat to $tocat that were born before $itsyourbirthday\n"; - my $query = qq| - UPDATE borrowers - SET guarantorid ='0', - categorycode = ? + my $where = qq| WHERE dateofbirth <= ? AND dateofbirth != '0000-00-00' AND branchcode = ? @@ -178,10 +181,35 @@ if ( not $noaction ) { FROM categories WHERE category_type = 'C' AND categorycode = ? - )|; + ) + |; + + $schema->storage->txn_begin; + + my $query = qq| + DELETE relationships FROM relationships + LEFT JOIN borrowers ON ( borrowers.borrowernumber = relationships.guarantee_id ) + $where + |; my $sth = $dbh->prepare($query); + $sth->execute( $itsyourbirthday, $mybranch, $fromcat ) + or $success = 0; + + $query = qq| + UPDATE borrowers + SET categorycode = ? + $where + |; + $sth = $dbh->prepare($query); my $res = $sth->execute( $tocat, $itsyourbirthday, $mybranch, $fromcat ) - or die "can't execute"; + or $success = 0; + + if ( $success ) { + $schema->storage->txn_commit; + } else { + $schema->storage->txn_rollback; + die "can't execute"; + } if ( $res eq '0E0' ) { print "No patrons updated\n"; @@ -192,10 +220,7 @@ if ( not $noaction ) { } else { # branch was not supplied, processing all branches $verbose and print "Looking in all branches for patrons to update from $fromcat to $tocat that were born before $itsyourbirthday\n"; - my $query = qq| - UPDATE borrowers - SET guarantorid = '0', - categorycode = ? + my $where = qq| WHERE dateofbirth <= ? AND dateofbirth!='0000-00-00' AND categorycode IN ( @@ -203,10 +228,34 @@ if ( not $noaction ) { FROM categories WHERE category_type = 'C' AND categorycode = ? - )|; + ) + |; + + my $query = qq| + DELETE relationships FROM relationships + LEFT JOIN borrowers ON ( borrowers.borrowernumber = relationships.guarantee_id ) + $where + |; my $sth = $dbh->prepare($query); + $sth->execute( $itsyourbirthday, $fromcat ) + or $success = 0; + + $query = qq| + UPDATE borrowers + SET categorycode = ? + $where + |; + $sth = $dbh->prepare($query); my $res = $sth->execute( $tocat, $itsyourbirthday, $fromcat ) - or die "can't execute"; + or $success = 0; + $dbh->commit; + + if ( $success ) { + $dbh->commit; + } else { + $dbh->rollback; + die "can't execute"; + } if ( $res eq '0E0' ) { print "No patrons updated\n"; @@ -268,6 +317,7 @@ else { my $sth = $dbh->prepare($query); $sth->execute( $itsyourbirthday, $fromcat ) or die "Couldn't execute statement: " . $sth->errstr; + $dbh->commit; while ( my @res = $sth->fetchrow_array() ) { my $firstname = $res[0]; diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 7fc9226c29..751f8db133 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -312,7 +312,6 @@ elsif ( $action eq 'edit' ) { #Display logged in borrower's data $template->param( borrower => $borrower, - guarantor => scalar Koha::Patrons->find($borrowernumber)->guarantor(), hidden => GetHiddenFields( $mandatory, 'edit' ), csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $cgi->cookie('CGISESSID'), @@ -333,7 +332,8 @@ my $captcha = random_string("CCCCC"); $template->param( captcha => $captcha, - captcha_digest => md5_base64($captcha) + captcha_digest => md5_base64($captcha), + patron => Koha::Patrons->find( $borrowernumber ), ); output_html_with_http_headers $cgi, $cookie, $template->output, undef, { force_no_caching => 1 }; diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 4ef0fb3c0b..177bed321c 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -39,6 +39,7 @@ use Koha::Holds; use Koha::Database; use Koha::ItemTypes; use Koha::Patron::Attribute::Types; +use Koha::Patrons; use Koha::Patron::Messages; use Koha::Patron::Discharge; use Koha::Patrons; @@ -330,14 +331,12 @@ my $patron_messages = Koha::Patron::Messages->search( if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') || C4::Context->preference('AllowStaffToSetCheckoutsVisibilityForGuarantor') ) { - my @relatives = - Koha::Database->new()->schema()->resultset("Borrower")->search( - { - privacy_guarantor_checkouts => 1, - 'me.guarantorid' => $borrowernumber - }, - { prefetch => [ { 'issues' => { 'item' => 'biblio' } } ] } - ); + my @relatives; + # Filter out guarantees that don't want guarantor to see checkouts + foreach my $gr ( $patron->guarantee_relationships() ) { + my $g = $gr->guarantee; + push( @relatives, $g ) if $g->privacy_guarantor_checkouts; + } $template->param( relatives => \@relatives ); } diff --git a/t/Patron.t b/t/Patron.t index 81b3f80931..4330e16af0 100755 --- a/t/Patron.t +++ b/t/Patron.t @@ -95,12 +95,7 @@ my $patron = Koha::Patron->new( lost => '0', debarred => '2015-04-19', debarredcomment => 'You are debarred', - contactname => 'myContactname', - contactfirstname => 'myContactfirstname', - contacttitle => 'myContacttitle', - guarantorid => '123454321', borrowernotes => 'borrowernotes', - relationship => 'myRelationship', sex => 'M', password => 'hfkurhfe976634èj!', flags => '55555', @@ -125,7 +120,7 @@ my $patron = Koha::Patron->new( # patron Accessor tests subtest 'Accessor tests' => sub { - plan tests => 65; + plan tests => 60; is( $patron->borrowernumber, '12345', 'borrowernumber accessor returns correct value' ); is( $patron->cardnumber, '1234567890', 'cardnumber accessor returns correct value' ); is( $patron->surname, 'mySurname', 'surname accessor returns correct value' ); @@ -166,12 +161,7 @@ subtest 'Accessor tests' => sub { is( $patron->lost, '0', 'lost accessor returns correct value' ); is( $patron->debarred, '2015-04-19', 'debarred accessor returns correct value' ); is( $patron->debarredcomment, 'You are debarred', 'debarredcomment accessor returns correct value' ); - is( $patron->contactname, 'myContactname', 'contactname accessor returns correct value' ); - is( $patron->contactfirstname, 'myContactfirstname', 'contactfirstname accessor returns correct value' ); - is( $patron->contacttitle, 'myContacttitle', 'contacttitle accessor returns correct value' ); - is( $patron->guarantorid, '123454321', 'guarantorid accessor returns correct value' ); is( $patron->borrowernotes, 'borrowernotes', 'borrowernotes accessor returns correct value' ); - is( $patron->relationship, 'myRelationship', 'relationship accessor returns correct value' ); is( $patron->sex, 'M', 'sex accessor returns correct value' ); is( $patron->password, 'hfkurhfe976634èj!', 'password accessor returns correct value' ); is( $patron->flags, '55555', 'flags accessor returns correct value' ); @@ -195,7 +185,7 @@ subtest 'Accessor tests' => sub { # patron Set tests subtest 'Set tests' => sub { - plan tests => 65; + plan tests => 60; $patron->set( { @@ -239,12 +229,7 @@ subtest 'Set tests' => sub { lost => '1', debarred => '2016-04-19', debarredcomment => 'You are still debarred', - contactname => 'SmyContactname', - contactfirstname => 'SmyContactfirstname', - contacttitle => 'SmyContacttitle', - guarantorid => '223454321', borrowernotes => 'Sborrowernotes', - relationship => 'SmyRelationship', sex => 'F', password => 'zerzerzer#', flags => '666666', @@ -307,12 +292,7 @@ subtest 'Set tests' => sub { is( $patron->lost, '1', 'lost field set ok' ); is( $patron->debarred, '2016-04-19', 'debarred field set ok' ); is( $patron->debarredcomment, 'You are still debarred', 'debarredcomment field set ok' ); - is( $patron->contactname, 'SmyContactname', 'contactname field set ok' ); - is( $patron->contactfirstname, 'SmyContactfirstname', 'contactfirstname field set ok' ); - is( $patron->contacttitle, 'SmyContacttitle', 'contacttitle field set ok' ); - is( $patron->guarantorid, '223454321', 'guarantorid field set ok' ); is( $patron->borrowernotes, 'Sborrowernotes', 'borrowernotes field set ok' ); - is( $patron->relationship, 'SmyRelationship', 'relationship field set ok' ); is( $patron->sex, 'F', 'sex field set ok' ); is( $patron->password, 'zerzerzer#', 'password field set ok' ); is( $patron->flags, '666666', 'flags field set ok' ); diff --git a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t index b0ba95193a..a9fa98e517 100644 --- a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t +++ b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t @@ -26,6 +26,7 @@ use C4::Circulation qw( CanBookBeIssued ); use Koha::Account; use Koha::Account::Lines; use Koha::Account::Offsets; +use Koha::Patron::Relationship; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; @@ -52,12 +53,23 @@ my $patron = $builder->build_object( } } ); -my $guarantee = $builder->build( + +my $guarantee = $builder->build_object( { - source => 'Borrower', - value => { - guarantorid => $patron->borrowernumber, - categorycode => $patron_category->{categorycode}, + class => 'Koha::Patrons', + value => { + patron_category => $patron_category->{categorycode}, + } + } +); + +my $r = $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $patron->id, + guarantee_id => $guarantee->id, + relationship => 'parent', } } ); @@ -73,7 +85,7 @@ $account->add_debit({ amount => 10.00, type => 'lost_item', interface => 'test' ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->{barcode} ); is( $issuingimpossible->{DEBT_GUARANTEES} + 0, '10.00' + 0, "Patron cannot check out item due to debt for guarantee" ); -my $accountline = Koha::Account::Lines->search({ borrowernumber => $guarantee->{borrowernumber} })->next(); +my $accountline = Koha::Account::Lines->search({ borrowernumber => $guarantee->id })->next(); is( $accountline->amountoutstanding, "10.000000", "Found 10.00 amount outstanding" ); is( $accountline->accounttype, "L", "Account type is L" ); diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 289543f4e8..ad1403cd5f 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -550,7 +550,7 @@ subtest 'SearchItems test' => sub { subtest 'Koha::Item(s) tests' => sub { - plan tests => 5; + plan tests => 7; $schema->storage->txn_begin(); @@ -589,6 +589,10 @@ subtest 'Koha::Item(s) tests' => sub { is( ref($holdingbranch), 'Koha::Library', "Got Koha::Library from holding_branch method" ); is( $holdingbranch->branchcode(), $library2->{branchcode}, "Home branch code matches holdingbranch" ); + my $biblio = $item->biblio(); + is( ref($biblio), 'Koha::Biblio', "Got Koha::Biblio from biblio method" ); + is( $biblio->title(), 'Silence in the library', 'Title matches biblio title' ); + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index c9dd9c9306..d0db3c5ab8 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -34,6 +34,7 @@ use C4::Auth qw(checkpw_hash); use Koha::Holds; use Koha::Patrons; use Koha::Patron::Categories; +use Koha::Patron::Relationship; use Koha::Database; use Koha::DateUtils; use Koha::Virtualshelves; @@ -83,21 +84,23 @@ subtest 'library' => sub { subtest 'guarantees' => sub { plan tests => 13; - my $guarantees = $new_patron_1->guarantees; - is( ref($guarantees), 'Koha::Patrons', 'Koha::Patron->guarantees should return a Koha::Patrons result set in a scalar context' ); - is( $guarantees->count, 0, 'new_patron_1 should have 0 guarantee' ); - my @guarantees = $new_patron_1->guarantees; - is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantees should return an array in a list context' ); + my $guarantees = $new_patron_1->guarantee_relationships; + is( ref($guarantees), 'Koha::Patron::Relationships', 'Koha::Patron->guarantees should return a Koha::Patrons result set in a scalar context' ); + is( $guarantees->count, 0, 'new_patron_1 should have 0 guarantee relationships' ); + my @guarantees = $new_patron_1->guarantee_relationships; + is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantee_relationships should return an array in a list context' ); is( scalar(@guarantees), 0, 'new_patron_1 should have 0 guarantee' ); - my $guarantee_1 = $builder->build({ source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber }}); - my $guarantee_2 = $builder->build({ source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber }}); + my $guarantee_1 = $builder->build({ source => 'Borrower' }); + my $relationship_1 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->id, guarantee_id => $guarantee_1->{borrowernumber}, relationship => 'test' } )->store(); + my $guarantee_2 = $builder->build({ source => 'Borrower' }); + my $relationship_2 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->id, guarantee_id => $guarantee_2->{borrowernumber}, relationship => 'test' } )->store(); - $guarantees = $new_patron_1->guarantees; - is( ref($guarantees), 'Koha::Patrons', 'Koha::Patron->guarantees should return a Koha::Patrons result set in a scalar context' ); + $guarantees = $new_patron_1->guarantee_relationships; + is( ref($guarantees), 'Koha::Patron::Relationships', 'Koha::Patron->guarantee_relationships should return a Koha::Patrons result set in a scalar context' ); is( $guarantees->count, 2, 'new_patron_1 should have 2 guarantees' ); - @guarantees = $new_patron_1->guarantees; - is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantees should return an array in a list context' ); + @guarantees = $new_patron_1->guarantee_relationships; + is( ref(\@guarantees), 'ARRAY', 'Koha::Patron->guarantee_relationships should return an array in a list context' ); is( scalar(@guarantees), 2, 'new_patron_1 should have 2 guarantees' ); $_->delete for @guarantees; @@ -107,40 +110,107 @@ subtest 'guarantees' => sub { my $guarantor = $builder->build_object( { class => 'Koha::Patrons' } ); - my $order_guarantee1 = $builder->build_object( { class => 'Koha::Patrons' , value => { - surname => 'Zebra', - guarantorid => $guarantor->borrowernumber + my $order_guarantee1 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + surname => 'Zebra', + } } - })->borrowernumber; + )->borrowernumber; + $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $guarantor->id, + guarantee_id => $order_guarantee1, + relationship => 'test', + } + } + ); - my $order_guarantee2 = $builder->build_object( { class => 'Koha::Patrons' , value => { - surname => 'Yak', - guarantorid => $guarantor->borrowernumber + my $order_guarantee2 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + surname => 'Yak', + } } - })->borrowernumber; + )->borrowernumber; + $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $guarantor->id, + guarantee_id => $order_guarantee2, + relationship => 'test', + } + } + ); - my $order_guarantee3 = $builder->build_object( { class => 'Koha::Patrons' , value => { - surname => 'Xerus', - firstname => 'Walrus', - guarantorid => $guarantor->borrowernumber + my $order_guarantee3 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + surname => 'Xerus', + firstname => 'Walrus', + } } - })->borrowernumber; + )->borrowernumber; + $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $guarantor->id, + guarantee_id => $order_guarantee3, + relationship => 'test', + } + } + ); - my $order_guarantee4 = $builder->build_object( { class => 'Koha::Patrons' , value => { - surname => 'Xerus', - firstname => 'Vulture', - guarantorid => $guarantor->borrowernumber + my $order_guarantee4 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + surname => 'Xerus', + firstname => 'Vulture', + guarantorid => $guarantor->borrowernumber + } + } + )->borrowernumber; + $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $guarantor->id, + guarantee_id => $order_guarantee4, + relationship => 'test', + } } - })->borrowernumber; + ); - my $order_guarantee5 = $builder->build_object( { class => 'Koha::Patrons' , value => { - surname => 'Xerus', - firstname => 'Unicorn', - guarantorid => $guarantor->borrowernumber + my $order_guarantee5 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + surname => 'Xerus', + firstname => 'Unicorn', + guarantorid => $guarantor->borrowernumber + } } - })->borrowernumber; + )->borrowernumber; + my $r = $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $guarantor->id, + guarantee_id => $order_guarantee5, + relationship => 'test', + } + } + ); - $guarantees = $guarantor->guarantees(); + $guarantees = $guarantor->guarantee_relationships->guarantees; is( $guarantees->next()->borrowernumber, $order_guarantee5, "Return first guarantor alphabetically" ); is( $guarantees->next()->borrowernumber, $order_guarantee4, "Return second guarantor alphabetically" ); @@ -160,15 +230,18 @@ subtest 'siblings' => sub { plan tests => 7; my $siblings = $new_patron_1->siblings; is( $siblings, undef, 'Koha::Patron->siblings should not crashed if the patron has no guarantor' ); - my $guarantee_1 = $builder->build( { source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber } } ); + my $guarantee_1 = $builder->build( { source => 'Borrower' } ); + my $relationship_1 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->borrowernumber, guarantee_id => $guarantee_1->{borrowernumber}, relationship => 'test' } )->store(); my $retrieved_guarantee_1 = Koha::Patrons->find($guarantee_1); $siblings = $retrieved_guarantee_1->siblings; is( ref($siblings), 'Koha::Patrons', 'Koha::Patron->siblings should return a Koha::Patrons result set in a scalar context' ); my @siblings = $retrieved_guarantee_1->siblings; is( ref( \@siblings ), 'ARRAY', 'Koha::Patron->siblings should return an array in a list context' ); is( $siblings->count, 0, 'guarantee_1 should not have siblings yet' ); - my $guarantee_2 = $builder->build( { source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber } } ); - my $guarantee_3 = $builder->build( { source => 'Borrower', value => { guarantorid => $new_patron_1->borrowernumber } } ); + my $guarantee_2 = $builder->build( { source => 'Borrower' } ); + my $relationship_2 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->borrowernumber, guarantee_id => $guarantee_2->{borrowernumber}, relationship => 'test' } )->store(); + my $guarantee_3 = $builder->build( { source => 'Borrower' } ); + my $relationship_3 = Koha::Patron::Relationship->new( { guarantor_id => $new_patron_1->borrowernumber, guarantee_id => $guarantee_3->{borrowernumber}, relationship => 'test' } )->store(); $siblings = $retrieved_guarantee_1->siblings; is( $siblings->count, 2, 'guarantee_1 should have 2 siblings' ); is( $guarantee_2->{borrowernumber}, $siblings->next->borrowernumber, 'guarantee_2 should exist in the guarantees' ); diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index f9595beb1b..91f4b3d200 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -27,6 +27,7 @@ use Koha::Database; use Koha::Holds; use Koha::List::Patron; use Koha::Patrons; +use Koha::Patron::Relationship; use t::lib::Mocks; use t::lib::TestBuilder; @@ -40,9 +41,6 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; my $dbh = C4::Context->dbh; -# Remove invalid guarantorid's as long as we have no FK -$dbh->do("UPDATE borrowers b1 LEFT JOIN borrowers b2 ON b2.borrowernumber=b1.guarantorid SET b1.guarantorid=NULL where b1.guarantorid IS NOT NULL AND b2.borrowernumber IS NULL"); - my $library1 = $builder->build({ source => 'Branch', }); @@ -215,7 +213,6 @@ my $borrower1 = $builder->build({ categorycode=>'STAFFER', branchcode => $library3->{branchcode}, dateexpiry => '2015-01-01', - guarantorid=> undef, }, }); my $bor1inlist = $borrower1->{borrowernumber}; @@ -225,7 +222,6 @@ my $borrower2 = $builder->build({ categorycode=>'STAFFER', branchcode => $library3->{branchcode}, dateexpiry => '2015-01-01', - guarantorid=> undef, }, }); @@ -235,7 +231,6 @@ my $guarantee = $builder->build({ categorycode=>'KIDclamp', branchcode => $library3->{branchcode}, dateexpiry => '2015-01-01', - guarantorid=> undef, # will be filled later }, }); @@ -270,7 +265,7 @@ $patstodel = GetBorrowersToExpunge( {not_borrowed_since => '2016-01-02', patron_ ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Staff patron not deleted by last issue date'); Koha::Patrons->find($bor1inlist)->set({ categorycode => 'CIVILIAN' })->store; -Koha::Patrons->find($guarantee->{borrowernumber})->set({ guarantorid => $bor1inlist })->store; +my $relationship = Koha::Patron::Relationship->new( { guarantor_id => $bor1inlist, guarantee_id => $guarantee->{borrowernumber}, relationship => 'test' } )->store(); $patstodel = GetBorrowersToExpunge( {patron_list_id => $list1->patron_list_id()} ); ok( scalar(@$patstodel)== 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Guarantor patron not deleted from list'); @@ -280,7 +275,7 @@ $patstodel = GetBorrowersToExpunge( {expired_before => '2015-01-02', patron_list ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Guarantor patron not deleted by expirationdate and list'); $patstodel = GetBorrowersToExpunge( {not_borrowed_since => '2016-01-02', patron_list_id => $list1->patron_list_id() } ); ok( scalar(@$patstodel) == 1 && $patstodel->[0]->{'borrowernumber'} eq $bor2inlist,'Guarantor patron not deleted by last issue date'); -Koha::Patrons->find($guarantee->{borrowernumber})->set({ guarantorid => undef })->store; +$relationship->delete(); $builder->build({ source => 'Issue', @@ -308,9 +303,9 @@ is( scalar(@$patstodel),2,'Borrowers without issues deleted by last issue date') # Test GetBorrowersToExpunge and TrackLastPatronActivity $dbh->do(q|UPDATE borrowers SET lastseen=NULL|); -$builder->build({ source => 'Borrower', value => { lastseen => '2016-01-01 01:01:01', categorycode => 'CIVILIAN', guarantorid => undef } } ); -$builder->build({ source => 'Borrower', value => { lastseen => '2016-02-02 02:02:02', categorycode => 'CIVILIAN', guarantorid => undef } } ); -$builder->build({ source => 'Borrower', value => { lastseen => '2016-03-03 03:03:03', categorycode => 'CIVILIAN', guarantorid => undef } } ); +$builder->build({ source => 'Borrower', value => { lastseen => '2016-01-01 01:01:01', categorycode => 'CIVILIAN' } } ); +$builder->build({ source => 'Borrower', value => { lastseen => '2016-02-02 02:02:02', categorycode => 'CIVILIAN' } } ); +$builder->build({ source => 'Borrower', value => { lastseen => '2016-03-03 03:03:03', categorycode => 'CIVILIAN' } } ); $patstodel = GetBorrowersToExpunge( { last_seen => '1999-12-12' }); is( scalar @$patstodel, 0, 'TrackLastPatronActivity - 0 patrons must be deleted' ); $patstodel = GetBorrowersToExpunge( { last_seen => '2016-02-15' }); diff --git a/t/db_dependent/Patron/Relationships.t b/t/db_dependent/Patron/Relationships.t new file mode 100755 index 0000000000..5bc5b2308b --- /dev/null +++ b/t/db_dependent/Patron/Relationships.t @@ -0,0 +1,167 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 59; + +use C4::Context; + +use t::lib::TestBuilder; + +BEGIN { + use_ok('Koha::Objects'); + use_ok('Koha::Patrons'); + use_ok('Koha::Patron::Relationship'); + use_ok('Koha::Patron::Relationships'); +} + +# Start transaction +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +my $builder = t::lib::TestBuilder->new(); + +# Father +my $kyle = Koha::Patrons->find( + $builder->build( + { + source => 'Borrower', + value => { + firstname => 'Kyle', + surname => 'Hall', + } + } + )->{borrowernumber} +); + +# Mother +my $chelsea = Koha::Patrons->find( + $builder->build( + { + source => 'Borrower', + value => { + firstname => 'Chelsea', + surname => 'Hall', + } + } + )->{borrowernumber} +); + +# Children +my $daria = Koha::Patrons->find( + $builder->build( + { + source => 'Borrower', + value => { + firstname => 'Daria', + surname => 'Hall', + } + } + )->{borrowernumber} +); + +my $kylie = Koha::Patrons->find( + $builder->build( + { + source => 'Borrower', + value => { + firstname => 'Kylie', + surname => 'Hall', + } + } + )->{borrowernumber} +); + +Koha::Patron::Relationship->new({ guarantor_id => $kyle->id, guarantee_id => $daria->id, relationship => 'father' })->store(); +Koha::Patron::Relationship->new({ guarantor_id => $kyle->id, guarantee_id => $kylie->id, relationship => 'father' })->store(); +Koha::Patron::Relationship->new({ guarantor_id => $chelsea->id, guarantee_id => $daria->id, relationship => 'mother' })->store(); +Koha::Patron::Relationship->new({ guarantor_id => $chelsea->id, guarantee_id => $kylie->id, relationship => 'mother' })->store(); + +my @gr; + +@gr = $kyle->guarantee_relationships(); +is( @gr, 2, 'Found 2 guarantee relationships for father' ); +is( $gr[0]->guarantor_id, $kyle->id, 'Guarantor matches for first relationship' ); +is( $gr[0]->guarantee_id, $daria->id, 'Guarantee matches for first relationship' ); +is( $gr[0]->relationship, 'father', 'Relationship is father' ); +is( ref($gr[0]->guarantee), 'Koha::Patron', 'Method guarantee returns a Koha::Patron' ); +is( $gr[0]->guarantee->id, $daria->id, 'Koha::Patron returned is the correct guarantee' ); +is( ref($gr[0]->guarantor), 'Koha::Patron', 'Method guarantor returns a Koha::Patron' ); +is( $gr[0]->guarantor->id, $kyle->id, 'Koha::Patron returned is the correct guarantor' ); + +is( $gr[1]->guarantor_id, $kyle->id, 'Guarantor matches for first relationship' ); +is( $gr[1]->guarantee_id, $kylie->id, 'Guarantee matches for first relationship' ); +is( $gr[1]->relationship, 'father', 'Relationship is father' ); +is( ref($gr[1]->guarantee), 'Koha::Patron', 'Method guarantee returns a Koha::Patron' ); +is( $gr[1]->guarantee->id, $kylie->id, 'Koha::Patron returned is the correct guarantee' ); +is( ref($gr[1]->guarantor), 'Koha::Patron', 'Method guarantor returns a Koha::Patron' ); +is( $gr[1]->guarantor->id, $kyle->id, 'Koha::Patron returned is the correct guarantor' ); + +@gr = $chelsea->guarantee_relationships(); +is( @gr, 2, 'Found 2 guarantee relationships for mother' ); +is( $gr[0]->guarantor_id, $chelsea->id, 'Guarantor matches for first relationship' ); +is( $gr[0]->guarantee_id, $daria->id, 'Guarantee matches for first relationship' ); +is( $gr[0]->relationship, 'mother', 'Relationship is mother' ); +is( ref($gr[0]->guarantee), 'Koha::Patron', 'Method guarantee returns a Koha::Patron' ); +is( $gr[0]->guarantee->id, $daria->id, 'Koha::Patron returned is the correct guarantee' ); +is( ref($gr[0]->guarantor), 'Koha::Patron', 'Method guarantor returns a Koha::Patron' ); +is( $gr[0]->guarantor->id, $chelsea->id, 'Koha::Patron returned is the correct guarantor' ); + +is( $gr[1]->guarantor_id, $chelsea->id, 'Guarantor matches for first relationship' ); +is( $gr[1]->guarantee_id, $kylie->id, 'Guarantee matches for first relationship' ); +is( $gr[1]->relationship, 'mother', 'Relationship is mother' ); +is( ref($gr[1]->guarantee), 'Koha::Patron', 'Method guarantee returns a Koha::Patron' ); +is( $gr[1]->guarantee->id, $kylie->id, 'Koha::Patron returned is the correct guarantee' ); +is( ref($gr[1]->guarantor), 'Koha::Patron', 'Method guarantor returns a Koha::Patron' ); +is( $gr[1]->guarantor->id, $chelsea->id, 'Koha::Patron returned is the correct guarantor' ); + +@gr = $daria->guarantor_relationships(); +is( @gr, 2, 'Found 4 guarantor relationships for child' ); +is( $gr[0]->guarantor_id, $kyle->id, 'Guarantor matches for first relationship' ); +is( $gr[0]->guarantee_id, $daria->id, 'Guarantee matches for first relationship' ); +is( $gr[0]->relationship, 'father', 'Relationship is father' ); +is( ref($gr[0]->guarantee), 'Koha::Patron', 'Method guarantee returns a Koha::Patron' ); +is( $gr[0]->guarantee->id, $daria->id, 'Koha::Patron returned is the correct guarantee' ); +is( ref($gr[0]->guarantor), 'Koha::Patron', 'Method guarantor returns a Koha::Patron' ); +is( $gr[0]->guarantor->id, $kyle->id, 'Koha::Patron returned is the correct guarantor' ); + +is( $gr[1]->guarantor_id, $chelsea->id, 'Guarantor matches for first relationship' ); +is( $gr[1]->guarantee_id, $daria->id, 'Guarantee matches for first relationship' ); +is( $gr[1]->relationship, 'mother', 'Relationship is mother' ); +is( ref($gr[1]->guarantee), 'Koha::Patron', 'Method guarantee returns a Koha::Patron' ); +is( $gr[1]->guarantee->id, $daria->id, 'Koha::Patron returned is the correct guarantee' ); +is( ref($gr[1]->guarantor), 'Koha::Patron', 'Method guarantor returns a Koha::Patron' ); +is( $gr[1]->guarantor->id, $chelsea->id, 'Koha::Patron returned is the correct guarantor' ); + +my @siblings = $daria->siblings; +is( @siblings, 1, 'Method siblings called in list context returns list' ); +is( ref($siblings[0]), 'Koha::Patron', 'List contains a Koha::Patron' ); +is( $siblings[0]->firstname, 'Kylie', 'Sibling from list first name matches correctly' ); +is( $siblings[0]->surname, 'Hall', 'Sibling from list surname matches correctly' ); +is( $siblings[0]->id, $kylie->id, 'Sibling from list patron id matches correctly' ); + +my $siblings = $daria->siblings; +my $sibling = $siblings->next(); +is( ref($siblings), 'Koha::Patrons', 'Calling siblings in scalar context results in a Koha::Patrons object' ); +is( ref($sibling), 'Koha::Patron', 'Method next returns a Koha::Patron' ); +is( $sibling->firstname, 'Kylie', 'Sibling from scalar first name matches correctly' ); +is( $sibling->surname, 'Hall', 'Sibling from scalar surname matches correctly' ); +is( $sibling->id, $kylie->id, 'Sibling from scalar patron id matches correctly' ); + +1; diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index ec784be2d9..4da2cd0d03 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -62,6 +62,7 @@ my $extended = C4::Context->preference('ExtendedPatronAttributes'); my @columnkeys = map { $_ ne 'borrowernumber' ? $_ : () } Koha::Patrons->columns(); push( @columnkeys, 'patron_attributes' ) if $extended; +push( @columnkeys, qw( relationship guarantor_id guarantor_firstname guarantor_surname ) ); my $input = CGI->new(); -- 2.20.1 (Apple Git-117)