From 5b3157f70bda2ff40bb4d4f0489604d1eceb3f2d Mon Sep 17 00:00:00 2001 From: Jared Camins-Esakov Date: Thu, 12 Sep 2013 09:29:40 -0400 Subject: [PATCH] Bug 10402 follow-up: choose contacts for claims This patch makes it possible to choose a particular contact for acquisitions and serials claims. To test: 1) Select a contact to use for claiming late orders and a contact to use for claiming late issues. 2) Send a claim for a late order and a claim for a late issue. 3) Note that the claims went out to the proper people. 4) Run the unit test with: > prove t/db_dependent/Letters.t 5) Sign off. Note: the claim messages are recorded in the logs in the *Acquisitions* module, not the Letters module as you might expect This patch also fixes several perlcritic violations and centralizes contact-related unit testing in Bookseller.t. --- C4/Bookseller.pm | 4 +- C4/Bookseller/Contact.pm | 32 ++++++-- C4/Letters.pm | 16 +++- acqui/supplier.pl | 2 +- acqui/updatesupplier.pl | 14 ++-- installer/data/mysql/kohastructure.sql | 7 +- installer/data/mysql/updatedatabase.pl | 20 +++-- .../prog/en/modules/acqui/supplier.tt | 72 +++++++++++++++++- t/db_dependent/Bookseller.t | 43 +---------- t/db_dependent/Letters.t | 87 +++++++++++++++++++++- 10 files changed, 228 insertions(+), 69 deletions(-) diff --git a/C4/Bookseller.pm b/C4/Bookseller.pm index 4386bb4..702b37c 100644 --- a/C4/Bookseller.pm +++ b/C4/Bookseller.pm @@ -248,7 +248,7 @@ sub ModBookseller { discount=?,notes=?,gstrate=?,deliverytime=? WHERE id=?'; my $sth = $dbh->prepare($query); - return $sth->execute( + my $cnt = $sth->execute( $data->{'name'}, $data->{'address1'}, $data->{'address2'}, $data->{'address3'}, $data->{'address4'}, $data->{'postal'}, @@ -278,7 +278,7 @@ sub ModBookseller { } $sth = $dbh->prepare($contactquery); $sth->execute(@contactparams); - return; + return $cnt; } =head2 DelBookseller diff --git a/C4/Bookseller/Contact.pm b/C4/Bookseller/Contact.pm index 09ec8da..8816897 100644 --- a/C4/Bookseller/Contact.pm +++ b/C4/Bookseller/Contact.pm @@ -63,10 +63,21 @@ Contact's e-mail address. Notes about contact. -=item rank +=item claimacquisition -Ranking of the contact so that the contact can be given the correct -priority in display. +Whether the contact should receive acquisitions claims. + +=item claimissues + +Whether the contact should receive serials claims. + +=item acqprimary + +Whether the contact is the primary contact for acquisitions. + +=item serialsprimary + +Whether the contact is the primary contact for serials. =item bookseller @@ -81,7 +92,7 @@ use C4::Context; use base qw(Class::Accessor); -__PACKAGE__->mk_accessors(qw(id name position phone altphone fax email notes rank bookseller)); +__PACKAGE__->mk_accessors(qw(id name position phone altphone fax email notes claimacquisition claimissues acqprimary serialsprimary bookseller)); =head1 METHODS @@ -152,12 +163,19 @@ sub save { my ($self) = @_; my $query; - my @params = ($self->name, $self->position, $self->phone, $self->altphone, $self->fax, $self->email, $self->notes, $self->rank, $self->bookseller); + my @params = ( + $self->name, $self->position, + $self->phone, $self->altphone, + $self->fax, $self->email, + $self->notes, $self->acqprimary ? 1 : 0, + $self->serialsprimary ? 1 : 0, $self->claimacquisition ? 1 : 0, + $self->claimissues ? 1 : 0, $self->bookseller + ); if ($self->id) { - $query = 'UPDATE aqcontacts SET name = ?, position = ?, phone = ?, altphone = ?, fax = ?, email = ?, notes = ?, rank = ?, booksellerid = ? WHERE id = ?;'; + $query = 'UPDATE aqcontacts SET name = ?, position = ?, phone = ?, altphone = ?, fax = ?, email = ?, notes = ?, acqprimary = ?, serialsprimary = ?, claimacquisition = ?, claimissues = ?, booksellerid = ? WHERE id = ?;'; push @params, $self->id; } else { - $query = 'INSERT INTO aqcontacts (name, position, phone, altphone, fax, email, notes, rank, booksellerid) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?);'; + $query = 'INSERT INTO aqcontacts (name, position, phone, altphone, fax, email, notes, acqprimary, serialsprimary, claimacquisition, claimissues, booksellerid) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);'; } my $dbh = C4::Context->dbh; my $sth = $dbh->prepare($query); diff --git a/C4/Letters.pm b/C4/Letters.pm index 1d916e0..5fdb1c6 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -326,14 +326,24 @@ sub SendAlerts { $dbh->prepare("select * from aqbooksellers where id=?"); $sthbookseller->execute( $dataorders->[0]->{booksellerid} ); my $databookseller = $sthbookseller->fetchrow_hashref; + my $addressee = $type eq 'claimacquisition' ? 'acqprimary' : 'serialsprimary'; + my $sthcontact = + $dbh->prepare("SELECT * FROM aqcontacts WHERE booksellerid=? AND $type=1 ORDER BY $addressee DESC"); + $sthcontact->execute( $dataorders->[0]->{booksellerid} ); + my $datacontact = $sthcontact->fetchrow_hashref; my @email; + my @cc; push @email, $databookseller->{bookselleremail} if $databookseller->{bookselleremail}; - push @email, $databookseller->{contemail} if $databookseller->{contemail}; + push @email, $datacontact->{email} if ( $datacontact && $datacontact->{email} ); unless (@email) { warn "Bookseller $dataorders->[0]->{booksellerid} without emails"; return { error => "no_email" }; } + my $addlcontact; + while ($addlcontact = $sthcontact->fetchrow_hashref) { + push @cc, $addlcontact->{email} if ( $addlcontact && $addlcontact->{email} ); + } my $userenv = C4::Context->userenv; my $letter = GetPreparedLetter ( @@ -343,6 +353,7 @@ sub SendAlerts { tables => { 'branches' => $userenv->{branch}, 'aqbooksellers' => $databookseller, + 'aqcontacts' => $datacontact, }, repeat => $dataorders, want_librarian => 1, @@ -351,6 +362,7 @@ sub SendAlerts { # ... then send mail my %mail = ( To => join( ',', @email), + Cc => join( ',', @cc), From => $userenv->{emailaddress}, Subject => Encode::encode( "utf8", "" . $letter->{title} ), Message => Encode::encode( "utf8", "" . $letter->{content} ), @@ -363,7 +375,7 @@ sub SendAlerts { $type eq 'claimissues' ? "CLAIM ISSUE" : "ACQUISITION CLAIM", undef, "To=" - . $databookseller->{contemail} + . join( ',', @email ) . " Title=" . $letter->{title} . " Content=" diff --git a/acqui/supplier.pl b/acqui/supplier.pl index b1e909e..ea656a9 100755 --- a/acqui/supplier.pl +++ b/acqui/supplier.pl @@ -67,7 +67,7 @@ my $booksellerid = $query->param('booksellerid'); my $supplier = {}; if ($booksellerid) { $supplier = GetBookSellerFromId($booksellerid); - foreach ( keys $supplier ) { + foreach ( keys %{$supplier} ) { $template->{'VARS'}->{$_} = $supplier->{$_}; } $template->{'VARS'}->{'booksellerid'} = $booksellerid; diff --git a/acqui/updatesupplier.pl b/acqui/updatesupplier.pl index 7ce5633..8e887df 100755 --- a/acqui/updatesupplier.pl +++ b/acqui/updatesupplier.pl @@ -37,10 +37,12 @@ All informations regarding this supplier are listed on input parameter. Here is the list : supplier, id, company, company_postal, physical, company_phone, -physical, company_phone, company_fax, website, company_contact_name, -company_contact_position, contact_phone, contact_phone_2, contact_fax, -company_email, contact_notes, notes, status, publishers_imprints, -list_currency, gst, list_gst, invoice_gst, discount, gstrate. +physical, company_phone, company_fax, website, company_email, notes, +status, publishers_imprints, list_currency, gst, list_gst, invoice_gst, +discount, gstrate, contact_name, contact_position, contact_phone, +contact_altphone, contact_fax, contact_email, contact_notes, +contact_claimacquisition, contact_claimissues, contact_acqprimary, +contact_serialsprimary. =cut @@ -102,14 +104,14 @@ $data{'active'}=$input->param('status'); my @contacts; my %contact_info; -foreach (qw(id name position phone altphone fax email notes)) { +foreach (qw(id name position phone altphone fax email notes claimacquisition claimissues acqprimary serialsprimary)) { $contact_info{$_} = [ $input->param('contact_' . $_) ]; } for my $cnt (0..scalar(@{$contact_info{'id'}})) { my %contact; my $real_contact; - foreach (qw(id name position phone altphone fax email notes)) { + foreach (qw(id name position phone altphone fax email notes claimacquisition claimissues acqprimary serialsprimary)) { $contact{$_} = $contact_info{$_}->[$cnt]; $real_contact = 1 if $contact{$_}; } diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 19b351e..06872b6 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2974,10 +2974,13 @@ CREATE TABLE aqcontacts ( fax varchar(100) default NULL, -- contact's fax number email varchar(100) default NULL, -- contact's email address notes mediumtext, -- notes related to the contact - rank SMALLINT default 0, -- display rank for the contact + claimacquisition BOOLEAN NOT NULL DEFAULT 0, -- should this contact receive acquisitions claims + claimissues BOOLEAN NOT NULL DEFAULT 0, -- should this contact receive serial claims + acqprimary BOOLEAN NOT NULL DEFAULT 0, -- is this the primary contact for acquisitions messages + serialsprimary BOOLEAN NOT NULL DEFAULT 0, -- is this the primary contact for serials messages booksellerid int(11) not NULL, PRIMARY KEY (id), - CONSTRAINT booksellerid_fk2 FOREIGN KEY (booksellerid) + CONSTRAINT booksellerid_aqcontacts_fk FOREIGN KEY (booksellerid) REFERENCES aqbooksellers (id) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ; diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index ac5ad65..c15ae68 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8560,7 +8560,7 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } -$DBversion = "3.13.00.XXX"; +$DBversion = "3.17.00.XXX"; if ( CheckVersion($DBversion) ) { $dbh->do("CREATE TABLE aqcontacts ( id int(11) NOT NULL auto_increment, @@ -8571,20 +8571,30 @@ if ( CheckVersion($DBversion) ) { fax varchar(100) default NULL, email varchar(100) default NULL, notes mediumtext, - rank SMALLINT default 0, + claimacquisition BOOLEAN NOT NULL DEFAULT 0, + claimissues BOOLEAN NOT NULL DEFAULT 0, + acqprimary BOOLEAN NOT NULL DEFAULT 0, + serialsprimary BOOLEAN NOT NULL DEFAULT 0, booksellerid int(11) not NULL, PRIMARY KEY (id), - CONSTRAINT booksellerid_fk2 FOREIGN KEY (booksellerid) + CONSTRAINT booksellerid_aqcontacts_fk FOREIGN KEY (booksellerid) REFERENCES aqbooksellers (id) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1;"); $dbh->do("INSERT INTO aqcontacts (name, position, phone, altphone, fax, - email, notes, booksellerid) + email, notes, booksellerid, claimacquisition, claimissues, acquisitionprimary, issuesprimary) SELECT contact, contpos, contphone, contaltphone, contfax, contemail, - contnotes, id FROM aqbooksellers;"); + contnotes, id, 1, 1, 1, 1 FROM aqbooksellers;"); $dbh->do("ALTER TABLE aqbooksellers DROP COLUMN contact, DROP COLUMN contpos, DROP COLUMN contphone, DROP COLUMN contaltphone, DROP COLUMN contfax, DROP COLUMN contemail, DROP COLUMN contnotes;"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); + $dbh->do("UPDATE letter SET content = replace(content, '<>', '<>')"); print "Upgrade to $DBversion done (Bug 10402: Move bookseller contacts to separate table)\n"; SetVersion($DBversion); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt index 420214f..41745ed 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt @@ -16,6 +16,35 @@
  • +
  • + [% IF contact.acqprimary %] + + [% ELSE %] + + [% END %] + +
  • + [% IF contact.serialsprimary %] + + [% ELSE %] + + [% END %] + +
  • + [% IF contact.claimacquisition %] + + [% ELSE %] + + [% END %] + +
  • + [% IF contact.claimissues %] + + [% ELSE %] + + [% END %] + +
  • [% IF contact.id %]
  • [% END %] [% END %] @@ -32,6 +61,18 @@ [% IF ( contact.notes ) %]

    Notes: [% contact.notes %]

    [% END %] + [% IF ( contact.acqprimary ) %] +

    Primary acquisitions contact

    + [% END %] + [% IF ( contact.serialsprimary ) %] +

    Primary serials contact

    + [% END %] + [% IF ( contact.claimacquisition ) %] +

    Receives claims for late orders

    + [% END %] + [% IF ( contact.claimissues ) %] +

    Receives claims for late issues

    + [% END %] [% END %] [% INCLUDE 'doc-head-open.inc' %] @@ -59,6 +100,11 @@ function add_contact() { $(this).attr('for', $(this).attr('for') + '_' + timestamp); }); $(new_contact).insertBefore(this); + if ($('.supplier-contact').length === 2) { // First contact + $.each(['.contact_acqprimary', '.contact_serialsprimary', '.contact_claimacquisition', '.contact_claimissues'], function (idx, checkbox) { + $(checkbox, new_contact).click(); + }); + } $('input[name="contact_name"]', new_contact).focus(); return false; } @@ -76,8 +122,32 @@ function delete_contact() { ], 'sDom': 't' } ) ); - $('.delete-contact').click(delete_contact); + $('body').on('click', '.delete-contact', null, delete_contact); $('#add-contact').click(add_contact); + $('body').on('click', '.contact_acqprimary', null, function () { + if ($(this).attr('checked') === 'checked') { + $('.contact_acqprimary').filter(':checked').not(this).removeAttr('checked'); + $('.contact_acqprimary_hidden').each(function () { + $(this).val('0'); + }); + } + $(this).next('.contact_acqprimary_hidden').val('1'); + }); + $('body').on('click', '.contact_serialsprimary', null, function () { + if ($(this).attr('checked') === 'checked') { + $('.contact_serialsprimary').filter(':checked').not(this).removeAttr('checked'); + $('.contact_serialsprimary_hidden').each(function () { + $(this).val('0'); + }); + } + $(this).next('.contact_serialsprimary_hidden').val($(this).attr('checked') === 'checked' ? '1' : '0'); + }); + $('body').on('click', '.contact_claimacquisition', null, function () { + $(this).next('.contact_claimacquisition_hidden').val($(this).attr('checked') === 'checked' ? '1' : '0'); + }); + $('body').on('click', '.contact_claimissues', null, function () { + $(this).next('.contact_claimissues_hidden').val($(this).attr('checked') === 'checked' ? '1' : '0'); + }); }); //]]> diff --git a/t/db_dependent/Bookseller.t b/t/db_dependent/Bookseller.t index 334c1f9..ef4a80f 100644 --- a/t/db_dependent/Bookseller.t +++ b/t/db_dependent/Bookseller.t @@ -2,7 +2,7 @@ use Modern::Perl; -use Test::More tests => 72; +use Test::More tests => 85; use Test::MockModule; use C4::Context; use Koha::DateUtils; @@ -51,13 +51,6 @@ my $sample_supplier1 = { accountnumber => 'accountnumber1', fax => 'fax1', url => 'url1', - contact => 'contact1', - contpos => 'contpos1', - contphone => 'contphone1', - contfax => 'contefax1', - contaltphone => 'contaltphone1', - contemail => 'contemail1', - contnotes => 'contnotes1', active => 1, gstreg => 1, listincgst => 1, @@ -78,13 +71,6 @@ my $sample_supplier2 = { accountnumber => 'accountnumber2', fax => 'fax2', url => 'url2', - contact => 'contact2', - contpos => 'contpos2', - contphone => 'contphone2', - contfax => 'contefax2', - contaltphone => 'contaltphone2', - contemail => 'contemail2', - contnotes => 'contnotes2', active => 1, gstreg => 1, listincgst => 1, @@ -92,7 +78,7 @@ my $sample_supplier2 = { gstrate => '2.0000', discount => '2.0000', notes => 'notes2', - deliverytime => 2, + deliverytime => 2 }; my $id_supplier1 = C4::Bookseller::AddBookseller($sample_supplier1); @@ -232,13 +218,6 @@ $sample_supplier2 = { accountnumber => 'accountnumber2 modified', fax => 'fax2 modified', url => 'url2 modified', - contact => 'contact2 modified', - contpos => 'contpos2 modified', - contphone => 'contphone2 modified', - contfax => 'contefax2 modified', - contaltphone => 'contaltphone2 modified', - contemail => 'contemail2 modified', - contnotes => 'contnotes2 modified', active => 1, gstreg => 1, listincgst => 1, @@ -279,13 +258,6 @@ my $sample_supplier3 = { accountnumber => 'accountnumber3', fax => 'fax3', url => 'url3', - contact => 'contact3', - contpos => 'contpos3', - contphone => 'contphone3', - contfax => 'contefax3', - contaltphone => 'contaltphone3', - contemail => 'contemail3', - contnotes => 'contnotes3', active => 1, gstreg => 1, listincgst => 1, @@ -306,13 +278,6 @@ my $sample_supplier4 = { accountnumber => 'accountnumber4', fax => 'fax4', url => 'url4', - contact => 'contact4', - contpos => 'contpos4', - contphone => 'contphone4', - contfax => 'contefax4', - contaltphone => 'contaltphone4', - contemail => 'contemail4', - contnotes => 'contnotes4', active => 1, gstreg => 1, listincgst => 1, @@ -718,7 +683,7 @@ my $booksellerid = C4::Bookseller::AddBookseller( ] ); -my @booksellers = C4::Bookseller::GetBookSeller('my vendor'); +@booksellers = C4::Bookseller::GetBookSeller('my vendor'); ok( ( grep { $_->{'id'} == $booksellerid } @booksellers ), 'GetBookSeller returns correct record when passed a name' @@ -779,7 +744,7 @@ sub field_filter { 'bookselleremail', 'booksellerfax', 'booksellerurl', 'othersupplier', 'currency', 'invoiceprice', - 'listprice' + 'listprice', 'contacts' ) { diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 9f5c9ac..371653c 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -18,12 +18,28 @@ # along with Koha; if not, see . use Modern::Perl; +use Test::MockModule; + +use Test::More tests => 15; + +my %mail; +my $module = new Test::MockModule('Mail::Sendmail'); +$module->mock( + 'sendmail', + sub { + warn "Fake sendmail"; + %mail = @_; + } +); + -use Test::More tests => 5; +use_ok('C4::Context'); +use_ok('C4::Members'); +use_ok('C4::Acquisition'); +use_ok('C4::Biblio'); +use_ok('C4::Bookseller'); -use C4::Context; -use C4::Letters; -use C4::Members; +use_ok('C4::Letters'); my $dbh = C4::Context->dbh; @@ -77,4 +93,67 @@ is( 'message marked failed if tried to send SMS message for borrower with no smsalertnumber set (bug 11208)' ); +$dbh->do(q{INSERT INTO letter (module, code, name, title, content) VALUES ('claimacquisition','TESTACQCLAIM','Acquisition Claim','Item Not Received','<>|<>|Ordernumber <> (<>) (<> ordered)');}); + +my $booksellerid = C4::Bookseller::AddBookseller( + { + name => "my vendor", + address1 => "bookseller's address", + phone => "0123456", + active => 1, + deliverytime => 5, + }, + [ + { name => 'John Smith', phone => '0123456x1', claimacquisition => 1 }, + { name => 'Leo Tolstoy', phone => '0123456x2', claimissues => 1 }, + ] +); +my $basketno = NewBasket($booksellerid, 1); + +my $budgetid = C4::Budgets::AddBudget({ + budget_code => "budget_code_test_letters", + budget_name => "budget_name_test_letters", +}); + +my $ordernumber; +my $bib = MARC::Record->new(); +if (C4::Context->preference('marcflavour') eq 'UNIMARC') { + $bib->append_fields( + MARC::Field->new('200', ' ', ' ', a => 'Silence in the library'), + ); +} else { + $bib->append_fields( + MARC::Field->new('245', ' ', ' ', a => 'Silence in the library'), + ); +} + +my ($biblionumber, $biblioitemnumber) = AddBiblio($bib, ''); +( undef, $ordernumber ) = C4::Acquisition::NewOrder( + { + basketno => $basketno, + quantity => 1, + biblionumber => $biblionumber, + budget_id => $budgetid, + } +); + +C4::Acquisition::CloseBasket( $basketno ); +my $err; +eval { + warn "This test may issue a warning. Please ignore it.\n"; + $err = SendAlerts( 'claimacquisition', [ $ordernumber ], 'TESTACQCLAIM' ); +}; +is($err->{'error'}, 'no_email', "Trying to send an alert when there's no e-mail results in an error"); + +my $bookseller = C4::Bookseller::GetBookSellerFromId($booksellerid); +$bookseller->{'contacts'}->[0]->email('testemail@mydomain.com'); +C4::Bookseller::ModBookseller($bookseller); + +eval { + $err = SendAlerts( 'claimacquisition', [ $ordernumber ], 'TESTACQCLAIM' ); +}; +is($err, 1, "Successfully sent claim"); +is($mail{'To'}, 'testemail@mydomain.com'); +is($mail{'Message'}, 'my vendor|John Smith|Ordernumber ' . $ordernumber . ' (Silence in the library) (1 ordered)', 'Claim notice text constructed successfully'); + $dbh->rollback; -- 1.9.1