Bug 10402: Move contacts to separate table

This patch normalizes the data structures used for bookseller
contacts.

To test:
1) Repeat tests described on previous patch.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

C4/Bookseller.pm

@@ -260,12 +260,22 @@ sub ModBookseller {
         $data->{'id'}
     );
     $contacts ||= $data->{'contacts'};
+    my $contactquery = "DELETE FROM aqcontacts WHERE booksellerid = ?";
+    my @contactparams = ($data->{'id'});
     if ($contacts) {
-        $contacts->[0] = C4::Bookseller::Contact->new( $contacts->[0] )
+        foreach my $contact (@$contacts) {
+            $contact = C4::Bookseller::Contact->new( $contact )
                 unless ref $contacts->[0] eq 'C4::Bookseller::Contact';
-        $contacts->[0]->bookseller($data->{'id'});
-        $contacts->[0]->save();
+            $contact->bookseller($data->{'id'});
+            $contact->save();
+            push @contactparams, $contact->id if $contact->id;
         }
+        if ($#contactparams > 0) {
+            $contactquery .= ' AND id NOT IN (' . ('?, ' x ($#contactparams - 1)) . '?);';
+        }
+    }
+    $sth = $dbh->prepare($contactquery);
+    $sth->execute(@contactparams);
     return;
 }
 

C4/Bookseller/Contact.pm

@@ -63,10 +63,10 @@ Contact's e-mail address.
 
 Notes about contact.
 
-=item primary
+=item rank
 
-Flag to indicate whether a contact is "primary" or not. Initially unused since
-each bookseller can have only one contact.
+Ranking of the contact so that the contact can be given the correct
+priority in display.
 
 =item bookseller
 
@@ -81,7 +81,7 @@ use C4::Context;
 
 use base qw(Class::Accessor);
 
-__PACKAGE__->mk_accessors(qw(id name position phone altphone fax email notes primary bookseller));
+__PACKAGE__->mk_accessors(qw(id name position phone altphone fax email notes rank bookseller));
 
 =head1 METHODS
 
@@ -90,7 +90,8 @@ __PACKAGE__->mk_accessors(qw(id name position phone altphone fax email notes pri
     my @contacts = @{C4::Bookseller::Contact->get_from_bookseller($booksellerid)};
 
 Returns a reference to an array of C4::Bookseller::Contact objects for the
-specified bookseller.
+specified bookseller. This will always return at least one item, though that one
+item may be an empty contact.
 
 =cut
 
@@ -100,16 +101,16 @@ sub get_from_bookseller {
     return unless $bookseller;
 
     my @contacts;
-    my $query = "SELECT contact AS name, contpos AS position, contphone AS phone, contaltphone AS altphone, contfax AS fax, contemail AS email, contnotes AS notes, id AS bookseller FROM aqbooksellers WHERE id = ?";
-    # When we have our own table, we can use: my $query = "SELECT * FROM aqcontacts WHERE bookseller = ?";
+    my $query = "SELECT * FROM aqcontacts WHERE booksellerid = ?";
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
     $sth->execute($bookseller);
     while (my $rec = $sth->fetchrow_hashref()) {
-        $rec->{'primary'} = 1;
         push @contacts, $class->new($rec);
     }
 
+    push @contacts, $class->new() unless @contacts;
+
     return \@contacts;
 }
 
@@ -123,21 +124,21 @@ because there is no separate table from which contacts can be fetched.
 
 =cut
 
-#sub fetch {
-#    my ($class, $id) = @_;
-#
-#    my $rec = { };
-#    if ($id) {
-#        my $query = "SELECT * FROM aqcontacts WHERE id = ?";
-#        my $dbh = C4::Context->dbh;
-#        my $sth = $dbh->prepare($query);
-#        $sth->execute($id);
-#        $rec = $sth->fetchrow_hashref();
-#    }
-#    my $self = $class->SUPER::new($rec);
-#    bless $self, $class;
-#    return $self;
-#}
+sub fetch {
+    my ($class, $id) = @_;
+
+    my $rec = { };
+    if ($id) {
+        my $query = "SELECT * FROM aqcontacts WHERE id = ?";
+        my $dbh = C4::Context->dbh;
+        my $sth = $dbh->prepare($query);
+        $sth->execute($id);
+        $rec = $sth->fetchrow_hashref();
+    }
+    my $self = $class->new($rec);
+    bless $self, $class;
+    return $self;
+}
 
 =head2 save
 
@@ -151,21 +152,29 @@ sub save {
     my ($self) = @_;
 
     my $query;
-#    if ($self->id) {
-#        $query = 'UPDATE aqcontacts SET name = ?, position = ?, phone = ?, altphone = ?, fax = ?, email = ?, notes = ?, primary = ? WHERE id = ?;';
-#    } else {
-#        $query = 'INSERT INTO aqcontacts (name, position, phone, altphone, fax, email, notes, primary) VALUES (?, ?, ?, ?, ? ,? ,?, ?);';
-#    }
-    if ($self->bookseller) {
-        $query = 'UPDATE aqbooksellers SET contact = ?, contpos = ?, contphone = ?, contaltphone = ?, contfax = ?, contemail = ?, contnotes = ? WHERE id = ?;';
+    my @params = ($self->name, $self->position, $self->phone, $self->altphone, $self->fax, $self->email, $self->notes, $self->rank, $self->bookseller);
+    if ($self->id) {
+        $query = 'UPDATE aqcontacts SET name = ?, position = ?, phone = ?, altphone = ?, fax = ?, email = ?, notes = ?, rank = ?, booksellerid = ? WHERE id = ?;';
+        push @params, $self->id;
     } else {
-        return;
+        $query = 'INSERT INTO aqcontacts (name, position, phone, altphone, fax, email, notes, rank, booksellerid) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?);';
     }
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare($query);
-    $sth->execute($self->name, $self->position, $self->phone, $self->altphone, $self->fax, $self->email, $self->notes, $self->bookseller);
-    #$self->id = $dbh->last_insert_id(undef, undef, 'aqcontacts', undef);
-    return $self->bookseller;
+    $sth->execute(@params);
+    $self->id($dbh->{'mysql_insertid'}) unless $self->id;
+    return $self->id;
+}
+
+sub delete {
+    my ($self) = @_;
+
+    return unless $self->id;
+
+    my $dbh = C4::Context->dbh;
+    my $sth = $dbh->prepare("DELETE FROM aqcontacts WHERE id = ?;");
+    $sth->execute($self->id);
+    return;
 }
 
 =head1 AUTHOR

installer/data/mysql/kohastructure.sql

@@ -2864,15 +2864,8 @@ CREATE TABLE `aqbooksellers` ( -- information about the vendors listed in acquis
   `notes` mediumtext, -- order notes
   `bookselleremail` mediumtext, -- vendor email
   `booksellerurl` mediumtext, -- unused in Koha
-  `contact` varchar(100) default NULL, -- name of contact at vendor
   `postal` mediumtext, -- vendor postal address (all lines)
   `url` varchar(255) default NULL, -- vendor web address
-  `contpos` varchar(100) default NULL, -- contact person's position
-  `contphone` varchar(100) default NULL, -- contact's phone number
-  `contfax` varchar(100) default NULL,  -- contact's fax number
-  `contaltphone` varchar(100) default NULL, -- contact's alternate phone number
-  `contemail` varchar(100) default NULL, -- contact's email address
-  `contnotes` mediumtext, -- notes related to the contact
   `active` tinyint(4) default NULL, -- is this vendor active (1 for yes, 0 for no)
   `listprice` varchar(10) default NULL, -- currency code for list prices
   `invoiceprice` varchar(10) default NULL, -- currency code for invoice prices
@@ -2967,6 +2960,27 @@ CREATE TABLE `aqbudgets_planning` (
   CONSTRAINT `aqbudgets_planning_ifbk_1` FOREIGN KEY (`budget_id`) REFERENCES `aqbudgets` (`budget_id`) ON DELETE CASCADE ON UPDATE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
+--
+-- Table structure for table 'aqcontacts'
+--
+
+DROP TABLE IF EXISTS aqcontacts;
+CREATE TABLE aqcontacts (
+  id int(11) NOT NULL auto_increment, -- primary key and unique number assigned by Koha
+  name varchar(100) default NULL, -- name of contact at vendor
+  position varchar(100) default NULL, -- contact person's position
+  phone varchar(100) default NULL, -- contact's phone number
+  altphone varchar(100) default NULL, -- contact's alternate phone number
+  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
+  booksellerid int(11) not NULL,
+  PRIMARY KEY  (id),
+  CONSTRAINT booksellerid_fk2 FOREIGN KEY (booksellerid)
+       REFERENCES aqbooksellers (id) ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ;
+
 --
 -- Table structure for table 'aqcontract'
 --

installer/data/mysql/updatedatabase.pl

@@ -8630,6 +8630,36 @@ if ( CheckVersion($DBversion) ) {
     SetVersion ($DBversion);
 }
 
+$DBversion = "3.17.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("CREATE TABLE aqcontacts (
+        id int(11) NOT NULL auto_increment,
+        name varchar(100) default NULL,
+        position varchar(100) default NULL,
+        phone varchar(100) default NULL,
+        altphone varchar(100) default NULL,
+        fax varchar(100) default NULL,
+        email varchar(100) default NULL,
+        notes mediumtext,
+        rank SMALLINT default 0,
+        booksellerid int(11) not NULL,
+        PRIMARY KEY  (id),
+        CONSTRAINT booksellerid_fk2 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)
+        SELECT contact, contpos, contphone, contaltphone, contfax, contemail,
+            contnotes, id 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;");
+    print "Upgrade to $DBversion done (Bug 10402: Move bookseller contacts to separate table)\n";
+    SetVersion($DBversion);
+}
+
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)