From 88883b0e0343104580a19728c3427d87bd82edcd 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 | 16 +- C4/SIP/ILS/Patron.pm | 1 - Koha/Item.pm | 1 + Koha/Object.pm | 8 +- Koha/Patron.pm | 82 ++++--- Koha/Patron/Relationship.pm | 73 ++++++ Koha/Patron/Relationships.pm | 94 +++++++ Koha/Patrons.pm | 12 - Koha/Patrons/Import.pm | 23 ++ Koha/Schema/Result/Borrower.pm | 6 - Koha/Schema/Result/Relationship.pm | 132 ++++++++++ api/v1/swagger/definitions/patron.json | 4 - circ/circulation.pl | 14 +- installer/data/mysql/atomicupdate/bug_14570.sql | 17 ++ installer/data/mysql/kohastructure.sql | 33 +-- koha-tmpl/intranet-tmpl/prog/en/columns.def | 1 - .../prog/en/includes/members-toolbar.inc | 14 +- .../prog/en/modules/circ/circulation.tt | 2 +- .../prog/en/modules/members/memberentrygen.tt | 272 +++++++++++++-------- .../prog/en/modules/members/moremember-brief.tt | 23 +- .../prog/en/modules/members/moremember.tt | 23 +- koha-tmpl/intranet-tmpl/prog/js/members.js | 105 ++++---- .../bootstrap/en/modules/opac-memberentry.tt | 9 +- .../opac-tmpl/bootstrap/en/modules/opac-privacy.tt | 9 +- .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 10 +- members/deletemem.pl | 6 +- members/memberentry.pl | 128 ++++++---- members/moremember.pl | 28 +-- members/update-child.pl | 21 +- misc/cronjobs/j2a.pl | 57 ++++- opac/opac-memberentry.pl | 4 +- opac/opac-user.pl | 17 +- t/Patron.t | 24 +- .../Circulation/NoIssuesChargeGuarantees.t | 25 +- t/db_dependent/Items.t | 6 +- t/db_dependent/Koha/Patrons.t | 36 +-- t/db_dependent/Members.t | 18 +- t/db_dependent/Patron/Relationships.t | 181 ++++++++++++++ tools/import_borrowers.pl | 1 + 41 files changed, 1107 insertions(+), 432 deletions(-) create mode 100644 Koha/Patron/Relationship.pm create mode 100644 Koha/Patron/Relationships.pm create mode 100644 Koha/Schema/Result/Relationship.pm create mode 100644 installer/data/mysql/atomicupdate/bug_14570.sql create mode 100755 t/db_dependent/Patron/Relationships.t diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index bbb1094229..27d9aaebdd 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -463,7 +463,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 8e1bdc49fc..83dfded5b1 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -758,7 +758,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 a901a961bc..4817e2b5fa 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -198,7 +198,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; @@ -317,7 +317,6 @@ sub ModMember { $new_borrower->{dateexpiry} ||= undef if exists $new_borrower->{dateexpiry}; $new_borrower->{debarred} ||= undef if exists $new_borrower->{debarred}; $new_borrower->{sms_provider_id} ||= undef if exists $new_borrower->{sms_provider_id}; - $new_borrower->{guarantorid} ||= undef if exists $new_borrower->{guarantorid}; my $patron = Koha::Patrons->find( $new_borrower->{borrowernumber} ); @@ -450,7 +449,6 @@ sub AddMember { $data{'dateofbirth'} = undef if ( not $data{'dateofbirth'} ); $data{'debarred'} = undef if ( not $data{'debarred'} ); $data{'sms_provider_id'} = undef if ( not $data{'sms_provider_id'} ); - $data{'guarantorid'} = undef if ( not $data{'guarantorid'} ); # get only the columns of Borrower # FIXME Do we really need this check? @@ -657,18 +655,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 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 23c576493f..868f4d6b17 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -616,7 +616,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/Item.pm b/Koha/Item.pm index bea05f8317..ee76bf6f67 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -28,6 +28,7 @@ use C4::Context; use Koha::Checkouts; use Koha::IssuingRules; use Koha::Item::Transfer; +use Koha::Biblios; use Koha::Patrons; use Koha::Libraries; diff --git a/Koha/Object.pm b/Koha/Object.pm index e35ff0b85e..436ec1df63 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -72,8 +72,12 @@ 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); + + eval { + $self->{_result} = + $schema->resultset( $class->_type() )->new($attributes); + }; + Carp::cluck("ERROR: $@") if $@; } croak("No _type found! Koha::Object must be subclassed!") diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 35cfce4ca1..efb5a950fc 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -27,15 +27,16 @@ use Text::Unaccent qw( unac_string ); use C4::Context; use C4::Log; use Koha::Checkouts; +use Koha::Old::Checkouts; use Koha::Database; use Koha::DateUtils; use Koha::Holds; -use Koha::Old::Checkouts; +use Koha::Patrons; use Koha::Patron::Categories; +use Koha::Patron::Relationships; use Koha::Patron::HouseboundProfile; use Koha::Patron::HouseboundRole; use Koha::Patron::Images; -use Koha::Patrons; use Koha::Virtualshelves; use Koha::Club::Enrollments; use Koha::Account; @@ -141,41 +142,59 @@ 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 + +=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::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::Patrons->search( { guarantorid => $self->borrowernumber } ); + return Koha::Patron::Relationships->search( { guarantor_id => $self->id } ); } =head3 housebound_profile @@ -213,23 +232,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 diff --git a/Koha/Patron/Relationship.pm b/Koha/Patron/Relationship.pm new file mode 100644 index 0000000000..2d42ab4e1b --- /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 'Relationship'; +} + +1; diff --git a/Koha/Patron/Relationships.pm b/Koha/Patron/Relationships.pm new file mode 100644 index 0000000000..e9da572878 --- /dev/null +++ b/Koha/Patron/Relationships.pm @@ -0,0 +1,94 @@ +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 } ); + + return wantarray ? $guarantees->as_list : $guarantees; +} + +=cut + +=head3 type + +=cut + +sub _type { + return 'Relationship'; +} + +sub object_class { + return 'Koha::Patron::Relationship'; +} + +1; diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 0d2d9ec6b4..bba01b533d 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -124,18 +124,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 fbf9701e92..007b427e58 100644 --- a/Koha/Patrons/Import.pm +++ b/Koha/Patrons/Import.pm @@ -213,6 +213,15 @@ sub import_patrons { next LINE; } + my $relationship = $borrower{relationship}; + my $guarantor_id = $borrower{guarantor_id}; + my $guarantor_firstname = $borrower{guarantor_firstname}; + my $guarantor_surname = $borrower{guarantor_surname}; + delete $borrower{relationship}; + delete $borrower{guarantor_id}; + delete $borrower{guarantor_firstname}; + delete $borrower{guarantor_surname}; + if ($borrowernumber) { # borrower exists @@ -348,6 +357,20 @@ sub import_patrons { ); } } + + # Add a guarantor if we are given a relationship + if ( $relationship ) { + $guarantor_id ||= undef; + Koha::Patron::Relationship->new( + { + guarantee_id => $borrowernumber, + relationship => $relationship, + guarantor_id => $guarantor_id, + firstname => $guarantor_firstname, + surname => $guarantor_surname, + } + )->store(); + } } return { diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index 8663b8654b..e88fdd2b18 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -1419,12 +1419,6 @@ __PACKAGE__->many_to_many("ordernumbers", "aqorder_users", "ordernumber"); # Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-07-06 14:12:40 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:LaQWPXzF1Amzt8fgEEyHdg -__PACKAGE__->belongs_to( - "guarantor", - "Koha::Schema::Result::Borrower", - { borrowernumber => "guarantorid" }, -); - __PACKAGE__->add_columns( '+lost' => { is_boolean => 1 }, '+gonenoaddress' => { is_boolean => 1 } diff --git a/Koha/Schema/Result/Relationship.pm b/Koha/Schema/Result/Relationship.pm new file mode 100644 index 0000000000..c815df4df9 --- /dev/null +++ b/Koha/Schema/Result/Relationship.pm @@ -0,0 +1,132 @@ +use utf8; +package Koha::Schema::Result::Relationship; + +# Created by DBIx::Class::Schema::Loader +# DO NOT MODIFY THE FIRST PART OF THIS FILE + +=head1 NAME + +Koha::Schema::Result::Relationship + +=cut + +use strict; +use warnings; + +use base 'DBIx::Class::Core'; + +=head1 TABLE: C + +=cut + +__PACKAGE__->table("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 + +=head2 surname + + data_type: 'mediumtext' + is_nullable: 1 + +=head2 firstname + + data_type: 'mediumtext' + is_nullable: 1 + +=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 }, + "surname", + { data_type => "mediumtext", is_nullable => 1 }, + "firstname", + { data_type => "mediumtext", is_nullable => 1 }, +); + +=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.07042 @ 2016-05-03 13:19:34 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:2Nv437KMrKHvpQleXLOjYw + + +# 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 5d6ccd82c0..549b470f0b 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 1617584cf4..e82d036cb8 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -571,14 +571,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.sql b/installer/data/mysql/atomicupdate/bug_14570.sql new file mode 100644 index 0000000000..77d1eb7e5f --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_14570.sql @@ -0,0 +1,17 @@ +CREATE TABLE `relationships` ( + id INT(11) NOT NULL AUTO_INCREMENT, + guarantor_id INT(11) NULL DEFAULT NULL, + guarantee_id INT(11) NOT NULL, + relationship VARCHAR(100) COLLATE utf8_unicode_ci NOT NULL, + surname MEDIUMTEXT COLLATE utf8_unicode_ci NULL DEFAULT NULL, + firstname MEDIUMTEXT COLLATE utf8_unicode_ci NULL DEFAULT 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=utf8 COLLATE=utf8_unicode_ci; + +UPDATE borrowers LEFT JOIN borrowers guarantor ON ( borrowers.guarantorid = guarantor.borrowernumber ) SET borrowers.guarantorid = NULL WHERE guarantor.borrowernumber IS NULL; + +INSERT INTO relationships ( guarantor_id, guarantee_id, relationship, surname, firstname ) SELECT guarantorid, borrowernumber, relationship, contactname, contactfirstname FROM borrowers WHERE guarantorid IS NOT NULL OR contactname != ""; + +ALTER TABLE borrowers DROP guarantorid, DROP relationship, DROP contactname, DROP contactfirstname, DROP contacttitle; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 20b1a5716e..f842f80f52 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -579,12 +579,7 @@ CREATE TABLE `deletedborrowers` ( -- stores data related to the patrons/borrower `lost` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as having lost their card `debarred` date default NULL, -- until this date the patron can only check-in (no loans, no holds, etc.), is a fine based on days instead of money (YYYY-MM-DD) `debarredcomment` VARCHAR(255) DEFAULT NULL, -- comment on the stop of patron - `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 `password` varchar(60) default NULL, -- patron/borrower's encrypted password `flags` int(11) default NULL, -- will include a number associated with the staff member's permissions @@ -1609,12 +1604,7 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons `lost` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as having lost their card `debarred` date default NULL, -- until this date the patron can only check-in (no loans, no holds, etc.), is a fine based on days instead of money (YYYY-MM-DD) `debarredcomment` VARCHAR(255) DEFAULT NULL, -- comment on the stop of the patron - `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 `password` varchar(60) default NULL, -- patron/borrower's Bcrypt encrypted password `flags` int(11) default NULL, -- will include a number associated with the staff member's permissions @@ -1647,7 +1637,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)), @@ -3428,12 +3417,7 @@ CREATE TABLE IF NOT EXISTS `borrower_modifications` ( `lost` tinyint(1) DEFAULT NULL, `debarred` date DEFAULT NULL, `debarredcomment` varchar(255) DEFAULT NULL, - `contactname` LONGTEXT, - `contactfirstname` MEDIUMTEXT, - `contacttitle` MEDIUMTEXT, - `guarantorid` int(11) DEFAULT NULL, `borrowernotes` LONGTEXT, - `relationship` varchar(100) DEFAULT NULL, `sex` varchar(1) DEFAULT NULL, `password` varchar(30) DEFAULT NULL, `flags` int(11) DEFAULT NULL, @@ -4113,6 +4097,23 @@ CREATE TABLE IF NOT EXISTS club_fields ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; -- +-- Table structure for table 'guarantors_guarantees' +-- + +DROP TABLE IF EXISTS relationships; +CREATE TABLE `relationships` ( + id INT(11) NOT NULL AUTO_INCREMENT, + guarantor_id INT(11) NULL DEFAULT NULL, + guarantee_id INT(11) NOT NULL, + relationship VARCHAR(100) COLLATE utf8_unicode_ci NOT NULL, + surname MEDIUMTEXT COLLATE utf8_unicode_ci NULL DEFAULT NULL, + firstname MEDIUMTEXT COLLATE utf8_unicode_ci NULL DEFAULT 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 21c74f2017..10ecd7cbd1 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 1897af77ae..9acd11ec11 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,17 @@ [% SET NorwegianPatronDBEnable = Koha.Preference( 'NorwegianPatronDBEnable' ) %]
[% 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 - [% 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 a6e6ba00fa..b2d8922ef7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -807,7 +807,7 @@ No patron matched [% message | html %] [% 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 36353607b3..8eeb2fd2cc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1,4 +1,5 @@ [% USE Asset %] +[% USE To %] [% USE Koha %] [% USE KohaDates %] [% USE Branches %] @@ -297,113 +298,170 @@ [% END # hide fieldset %] -[% IF ( showguarantor ) %] - - [% UNLESS step_6 %] - - [% END %] -
    - Guarantor information -
      -[% IF ( P ) %] - [% IF ( guarantorid ) %] -
    1. - [% ELSE %] -
      2. -
    2. - - [% IF ( guarantorid ) %] - [% contactname %] - - [% ELSE %] - - [% END %] -
      3. -[% ELSE %] - [% IF ( C ) %] - [% IF ( guarantorid ) %] -
    3. - [% ELSE %] -
      4. - [% UNLESS nocontactname %] -
    4. - - [% IF ( guarantorid ) %] - [% contactname %] - - [% ELSE %] - - [% END %] -
      5. - [% END %] - [% UNLESS nocontactfirstname %] -
    5. - - [% IF ( guarantorid ) %] - [% contactfirstname %] - - [% 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. +
    +
    + +
    + Add new guarantor +
      + + + [% IF catetory_type == 'P' %] +
    1. + + +
      2. [% ELSE %] - - +
    2. + + +
      3. + +
    3. + + +
      4. + + [% IF ( possible_relationships ) %] +
    4. + + +
      5. + [% END %] [% END %] - -
      Allow guarantor of this patron to view this patron's checkouts from the OPAC
      - - [% END %] -
    -
    +
  • +   + Add guarantor + Set to patron + Clear +
    • + + [% IF relationships && Koha.Preference('AllowStaffToSetCheckoutsVisibilityForGuarantor') %] +
  • + + +
    Allow guarantors 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" %] @@ -1193,6 +1251,14 @@ update_category_code( category_code ); } [% END %] + + [% IF guarantor %] + select_user( '[% guarantor.borrowernumber %]', [% To.json( guarantor.unblessed ) %] ); + $('#guarantor_add').ready(function() { + $('#guarantor_add').click(); + }); + [% END %] + }); function update_cardnumber_warning(size){ 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 0c2270ad05..398756638a 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 @@ -47,8 +47,27 @@
  • Guarantees:
    • [% END %] [% ELSE %] - [% IF ( guarantor.borrowernumber ) %] -
  • Guarantor:[% guarantor.surname %], [% guarantor.firstname %]
    • + [% IF guarantees %] +
  • + Guarantees: + +
    • + [% ELSIF guarantor_relationships %] + [% FOREACH gr IN guarantor_relationships %] +
  • + Guarantor: + [% IF gr.guarantor_id %] + [% SET guarantor = gr.guarantor %] + [% guarantor.firstname %] [% guarantor.surname %] + [% ELSE %] + [% gr.firstname %] [% gr.surname %] + [% END %] +
    • + [% END %] [% 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 cac919f1dd..cffd512ef6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -218,15 +218,20 @@ [% 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: + [% IF gr.guarantor_id %] + [% SET guarantor = gr.guarantor %] + [% IF logged_in_user.can_see_patron_infos( guarantor ) %] + [% guarantor.firstname | html %] [% guarantor.surname | html %] + [% END %] + [% ELSE %] + [% gr.firstname | html %] [% gr.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 e586e60d23..8ef5707474 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,11 @@ 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 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
      @@ -118,7 +118,12 @@ - Your guarantor is [% guarantor.firstname %] [% guarantor.surname %] + Guaranteed by + [% FOREACH gr IN patron.guarantor_relationships %] + [% SET g = gr.guarantor %] + [% g.firstname || gr.firstname %] [% g.surname || gr.surname %] + [%- IF ! loop.last %], [% END %] + [% END %]
    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 143f4e023b..99314bd261 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 %] [% borrower.guarantor.surname %] + Guaranteed by + [% FOREACH gr IN borrower.guarantor_relationships %] + [% SET g = gr.guarantor %] + [% g.firstname || gr.firstname %] [% g.surname || gr.surname %] + [%- 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 fb904159fe..134f7230d2 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -428,24 +428,24 @@ [% FOREACH r IN relatives %] - [% FOREACH i IN r.issues %] + [% FOREACH c IN r.checkouts %] - [% i.item.biblio.title %][% IF ( i.item.enumchron ) %] [% i.item.enumchron %][% END %] + [% c.item.biblio.title %][% IF ( c.item.enumchron ) %] [% c.item.enumchron %][% END %] - [% i.date_due | $KohaDates %] + [% c.date_due | $KohaDates %] - [% i.item.barcode %] + [% c.item.barcode %] - [% i.item.itemcallnumber %] + [% c.item.itemcallnumber %] diff --git a/members/deletemem.pl b/members/deletemem.pl index bc0c92441b..ee99317e24 100755 --- a/members/deletemem.pl +++ b/members/deletemem.pl @@ -109,7 +109,7 @@ if ( $patron->is_child ) { 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 or $deletelocal == 0) { $template->param( @@ -121,8 +121,8 @@ if ( $op eq 'delete_confirm' or $countissues > 0 or $charges or $is_guarantor or if ( $charges > 0 ) { $template->param(charges => $charges); } - if ($is_guarantor) { - $template->param(guarantees => 1); + if ( $is_guarantor ) { + $template->param( guarantees => 1 ); } if ($deletelocal == 0) { $template->param(keeplocal => 1); diff --git a/members/memberentry.pl b/members/memberentry.pl index f10002a80b..fd38324e53 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->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 ); @@ -246,26 +257,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')); @@ -423,6 +414,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ if ($op eq 'insert'){ # we know it's not a duplicate borrowernumber or there would already be an error $borrowernumber = &AddMember(%newdata); + add_guarantors( $borrowernumber, $input ); $newdata{'borrowernumber'} = $borrowernumber; # If 'AutoEmailOpacUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode. @@ -520,6 +512,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ # updating any columns in the borrowers table, # which can happen if we're only editing the # patron attributes or messaging preferences sections + add_guarantors( $borrowernumber, $input ); if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); } @@ -627,11 +620,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' ); @@ -656,24 +648,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 @@ -749,7 +744,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/ ) ? 1 : 0 ); # 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 @@ -763,17 +758,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, # 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, @@ -901,6 +891,42 @@ sub patron_attributes_form { } +sub add_guarantors { + my ( $borrowernumber, $input ) = @_; + + my @new_guarantor_id = scalar $input->param('new_guarantor_id'); + my @new_guarantor_surname = scalar $input->param('new_guarantor_surname'); + my @new_guarantor_firstname = scalar $input->param('new_guarantor_firstname'); + my @new_guarantor_relationship = scalar $input->param('new_guarantor_relationship'); + + for ( my $i = 0 ; $i < scalar @new_guarantor_id; $i++ ) { + my $guarantor_id = $new_guarantor_id[$i]; + my $surname = $new_guarantor_surname[$i]; + my $firstname = $new_guarantor_firstname[$i]; + my $relationship = $new_guarantor_relationship[$i]; + + if ($guarantor_id) { + Koha::Patron::Relationship->new( + { + guarantee_id => $borrowernumber, + guarantor_id => $guarantor_id, + relationship => $relationship + } + )->store(); + } + elsif ($surname) { + Koha::Patron::Relationship->new( + { + guarantee_id => $borrowernumber, + surname => $surname, + firstname => $firstname, + relationship => $relationship + } + )->store(); + } + } +} + # Local Variables: # tab-width: 8 # End: diff --git a/members/moremember.pl b/members/moremember.pl index dbe9896377..6157069a7d 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -172,22 +172,20 @@ if ( $patron->is_child ) { } 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::Database->new()->schema()->resultset('Issue') diff --git a/members/update-child.pl b/members/update-child.pl index cc942e2704..1546691cd7 100755 --- a/members/update-child.pl +++ b/members/update-child.pl @@ -34,6 +34,7 @@ use C4::Output; use C4::Members; use Koha::Patrons; use Koha::Patron::Categories; +use Koha::Patrons; # use Smart::Comments; @@ -54,7 +55,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $borrowernumber = $input->param('borrowernumber'); my $catcode = $input->param('catcode'); my $cattype = $input->param('cattype'); -my $catcode_multi = $input->param('catcode_multi'); +my $catcode_multi = $input->param('catcode_multi'); my $op = $input->param('op'); my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; @@ -71,11 +72,12 @@ 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 } ); + $_->delete() for $patron->guarantor_relationships(); + my $member = $patron->unblessed; $member->{'guarantorid'} = 0; $member->{'categorycode'} = $catcode; @@ -85,14 +87,17 @@ elsif ( $op eq 'update' ) { delete $member->{password}; ModMember(%$member); - if ( $catcode_multi ) { + if ($catcode_multi) { $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 ba90ad0ccd..3a1a3cf4cb 100755 --- a/misc/cronjobs/j2a.pl +++ b/misc/cronjobs/j2a.pl @@ -163,12 +163,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 + $dbh->{AutoCommit} = 0; + 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 = ? @@ -177,10 +177,28 @@ 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, $mybranch, $fromcat ) + or ( $dbh->rollback && die "can't execute" ); + + $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 ( $dbh->rollback && die "can't execute" ); + + $dbh->commit; if ( $res eq '0E0' ) { print "No patrons updated\n"; @@ -191,10 +209,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 ( @@ -202,10 +217,27 @@ 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 ( $dbh->rollback && die "can't execute" ); + + $query = qq| + UPDATE borrowers + SET categorycode = ? + $where + |; + $sth = $dbh->prepare($query); my $res = $sth->execute( $tocat, $itsyourbirthday, $fromcat ) - or die "can't execute"; + or ( $dbh->rollback && die "can't execute" ); + $dbh->commit; if ( $res eq '0E0' ) { print "No patrons updated\n"; @@ -267,6 +299,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 9d3bed837e..ab833c1cd7 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -299,7 +299,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, 'modification' ), csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $cgi->cookie('CGISESSID'), @@ -320,7 +319,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 78bd30b319..ad4467bccb 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -40,6 +40,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; @@ -76,6 +77,8 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); +my $patron = Koha::Patrons->find( $borrowernumber ); + my %renewed = map { $_ => 1 } split( ':', $query->param('renewed') ); my $show_priority; @@ -338,14 +341,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 971208f494..96a5c9bff8 100644 --- a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t +++ b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t @@ -26,6 +26,7 @@ use C4::Accounts qw( manualinvoice ); use C4::Circulation qw( CanBookBeIssued ); 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,24 @@ my $patron = $builder->build_object( } } ); -my $guarantee = $builder->build( + +my $guarantee = $builder->build_object( { - source => 'Borrower', - value => { + class => 'Koha::Patrons', + value => { + patron_category => $patron_category->{categorycode}, guarantorid => $patron->borrowernumber, - categorycode => $patron_category->{categorycode}, + } + } +); + +my $r = $builder->build_object( + { + class => 'Koha::Patron::Relationships', + value => { + guarantor_id => $patron->id, + guarantee_id => $guarantee->id, + relationship => 'parent', } } ); @@ -68,11 +81,11 @@ t::lib::Mocks::mock_preference( 'AllowFineOverride', '' ); my ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->{barcode} ); is( $issuingimpossible->{DEBT_GUARANTEES}, undef, "Patron can check out item" ); -manualinvoice( $guarantee->{borrowernumber}, undef, undef, 'L', 10.00 ); +manualinvoice( $guarantee->id, undef, undef, 'L', 10.00 ); ( $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 ef098734c9..e5077fbd39 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -507,7 +507,7 @@ subtest 'SearchItems test' => sub { subtest 'Koha::Item(s) tests' => sub { - plan tests => 5; + plan tests => 7; $schema->storage->txn_begin(); @@ -546,6 +546,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 bb6e2bcf98..03e4a2bc30 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -35,6 +35,7 @@ use Koha::Holds; use Koha::Patron; use Koha::Patrons; use Koha::Patron::Categories; +use Koha::Patron::Relationship; use Koha::Database; use Koha::DateUtils; use Koha::Virtualshelves; @@ -85,23 +86,25 @@ subtest 'library' => sub { subtest 'guarantees' => sub { plan tests => 8; - 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} } )->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} } )->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; + map { $_->guarantee->delete } @guarantees; }; subtest 'category' => sub { @@ -115,15 +118,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} } )->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} } )->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} } )->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 1f9de78f2f..d8c06b9f4d 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', }); @@ -228,7 +226,6 @@ my $borrower1 = $builder->build({ categorycode=>'STAFFER', branchcode => $library3->{branchcode}, dateexpiry => '2015-01-01', - guarantorid=> undef, }, }); my $bor1inlist = $borrower1->{borrowernumber}; @@ -238,7 +235,6 @@ my $borrower2 = $builder->build({ categorycode=>'STAFFER', branchcode => $library3->{branchcode}, dateexpiry => '2015-01-01', - guarantorid=> undef, }, }); @@ -248,7 +244,6 @@ my $guarantee = $builder->build({ categorycode=>'KIDclamp', branchcode => $library3->{branchcode}, dateexpiry => '2015-01-01', - guarantorid=> undef, # will be filled later }, }); @@ -283,7 +278,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'); ModMember( borrowernumber => $bor1inlist, categorycode => 'CIVILIAN' ); -ModMember( borrowernumber => $guarantee->{borrowernumber} ,guarantorid=>$bor1inlist ); +my $relationship = Koha::Patron::Relationship->new( { guarantor_id => $bor1inlist, guarantee_id => $guarantee->{borrowernumber} } )->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'); @@ -293,7 +288,8 @@ $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'); -ModMember( borrowernumber => $guarantee->{borrowernumber}, guarantorid=>'' ); + +$relationship->delete(); $builder->build({ source => 'Issue', @@ -321,9 +317,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..4aec07af88 --- /dev/null +++ b/t/db_dependent/Patron/Relationships.t @@ -0,0 +1,181 @@ +#!/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 => 67; + +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(); +Koha::Patron::Relationship->new({ firstname => 'John', surname => 'Hall', guarantee_id => $daria->id, relationship => 'grandfather' })->store(); +Koha::Patron::Relationship->new({ firstname => 'Debra', surname => 'Hall', guarantee_id => $daria->id, relationship => 'grandmother' })->store(); +Koha::Patron::Relationship->new({ firstname => 'John', surname => 'Hall', guarantee_id => $kylie->id, relationship => 'grandfather' })->store(); +Koha::Patron::Relationship->new({ firstname => 'Debra', surname => 'Hall', guarantee_id => $kylie->id, relationship => 'grandmother' })->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, 4, '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' ); + +is( $gr[2]->guarantor_id, undef, 'Grandfather has no id, not a Koha patron' ); +is( $gr[2]->firstname, 'John', 'Grandfather has first name of John' ); +is( $gr[2]->surname, 'Hall', 'Grandfather has surname of Hall' ); +is( $gr[2]->guarantor, undef, 'Calling guarantor method for Grandfather returns undef' ); + +is( $gr[3]->guarantor_id, undef, 'Grandmother has no id, not a Koha patron' ); +is( $gr[3]->firstname, 'Debra', 'Grandmother has first name of John' ); +is( $gr[3]->surname, 'Hall', 'Grandmother has surname of Hall' ); +is( $gr[3]->guarantor, undef, 'Calling guarantor method for Grandmother returns undef' ); + +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 bba512f5f8..52b9d3aa4c 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.11.0