reconcile invoice destination contacts with multiple-customer contacts, #25536 and #27943

10 files changed, 156 insertions, 50 deletions

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 56bdfc3..1e975dc 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -1755,6 +1755,7 @@ sub tables_hashref {
         'classnum',              'int', 'NULL',  '', '', '',
         'comment',           'varchar', 'NULL', 255, '', '',
         'selfservice_access',   'char', 'NULL',   1, '', '',
+        'invoice_dest',         'char', 'NULL',       1, '', '',
       ],
       'primary_key'  => 'custcontactnum',
       'unique'       => [ [ 'custnum', 'contactnum' ], ],
diff --git a/FS/FS/contact.pm b/FS/FS/contact.pm
index e5ddcdc..a824b8e 100644
--- a/FS/FS/contact.pm
+++ b/FS/FS/contact.pm
@@ -90,10 +90,6 @@ empty or bcrypt
 
 disabled
 
-=item invoice_dest
-
-empty, or 'Y' if email invoices should be sent to this contact
-
 =back
 
 =head1 METHODS
@@ -134,6 +130,7 @@ be included in that record, if they are set on the object:
 - classnum
 - comment
 - selfservice_access
+- invoice_dest
 
 =cut
 
@@ -157,7 +154,7 @@ sub insert {
   $self->custnum('');
 
   my %link_hash = ();
-  for (qw( classnum comment selfservice_access )) {
+  for (qw( classnum comment selfservice_access invoice_dest )) {
     $link_hash{$_} = $self->get($_);
     $self->$_('');
   }
@@ -425,7 +422,7 @@ sub replace {
   $self->custnum('');
 
   my %link_hash = ();
-  for (qw( classnum comment selfservice_access )) {
+  for (qw( classnum comment selfservice_access invoice_dest )) {
     $link_hash{$_} = $self->get($_);
     $self->$_('');
   }
@@ -674,7 +671,6 @@ sub check {
     || $self->ut_textn('_password')
     || $self->ut_enum('_password_encoding', [ '', 'bcrypt'])
     || $self->ut_enum('disabled', [ '', 'Y' ])
-    || $self->ut_flag('invoice_dest')
   ;
   return $error if $error;
 
@@ -960,7 +956,7 @@ sub _upgrade_data { #class method
       $dest = $svc_acct->email;
     }
 
-    my $error = $cust_main->replace( [ $dest ] );
+    my $error = $cust_main->replace( invoicing_list => [ $dest ] );
 
     if ( $error ) {
       die "custnum $custnum, invoice destination $dest, creating contact: $error\n";
@@ -971,14 +967,14 @@ sub _upgrade_data { #class method
 
   } # while $search->fetch
 
-  unless ( FS::upgrade_journal->is_done('contact__DUPEMAIL') ) {
+  unless ( FS::upgrade_journal->is_done('contact_invoice_dest') ) {
 
     foreach my $contact (qsearch('contact', {})) {
       my $error = $contact->replace;
       die $error if $error;
     }
 
-    FS::upgrade_journal->set_done('contact__DUPEMAIL');
+    FS::upgrade_journal->set_done('contact_invoice_dest');
   }
 
 }
diff --git a/FS/FS/cust_contact.pm b/FS/FS/cust_contact.pm
index 6f899d8..f0f8bfb 100644
--- a/FS/FS/cust_contact.pm
+++ b/FS/FS/cust_contact.pm
@@ -55,6 +55,10 @@ comment
 
 empty or Y
 
+=item invoice_dest
+
+'Y' if the customer should get invoices sent to this address, null if not
+
 =back
 
 =head1 METHODS
@@ -114,6 +118,7 @@ sub check {
     || $self->ut_numbern('classnum')
     || $self->ut_textn('comment')
     || $self->ut_enum('selfservice_access', [ '', 'Y' ])
+    || $self->ut_flag('invoice_dest')
   ;
   return $error if $error;
 
diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index 4fb4f52..f6b6862 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -558,13 +558,42 @@ sub insert {
   $invoicing_list ||= $options{'invoicing_list'};
   if ( $invoicing_list ) {
 
-    $invoicing_list = join(',', @$invoicing_list) if ref $invoicing_list;
+    $invoicing_list = [ $invoicing_list ] if !ref($invoicing_list);
+
+    my $email = '';
+    foreach my $dest (@$invoicing_list ) {
+      if ($dest eq 'POST') {
+        $self->set('postal_invoice', 'Y');
+      } else {
+
+        my $contact_email = qsearchs('contact_email', { emailaddress => $dest });
+        if ( $contact_email ) {
+          my $cust_contact = FS::cust_contact->new({
+              contactnum    => $contact_email->contactnum,
+              custnum       => $self->custnum,
+          });
+          $cust_contact->set('invoice_dest', 'Y');
+          my $error = $cust_contact->contactnum ?
+                        $cust_contact->replace : $cust_contact->insert;
+          if ( $error ) {
+            $dbh->rollback if $oldAutoCommit;
+            return "$error (linking to email address $dest)";
+          }
+
+        } else {
+          # this email address is not yet linked to any contact
+          $email .= ',' if length($email);
+          $email .= $dest;
+        }
+      }
+    }
+
     my $contact = FS::contact->new({
       'custnum'       => $self->get('custnum'),
       'last'          => $self->get('last'),
       'first'         => $self->get('first'),
-      'emailaddress'  => $invoicing_list,
-      'invoice_dest'  => 'Y',
+      'emailaddress'  => $email,
+      'invoice_dest'  => 'Y', # yes, you can set this via the contact
     });
     my $error = $contact->insert;
     if ( $error ) {
@@ -1365,40 +1394,106 @@ sub replace {
 
   $invoicing_list ||= $options{invoicing_list};
 
+  my @contacts = map { $_->contact } $self->cust_contact;
+  # find a contact that matches the customer's name
+  my ($implicit_contact) = grep { $_->first eq $old->get('first')
+                              and $_->last  eq $old->get('last') }
+                            @contacts;
+  $implicit_contact ||= FS::contact->new({
+      'custnum'       => $self->custnum,
+      'locationnum'   => $self->get('bill_locationnum'),
+  });
+
+  # for any of these that are already contact emails, link to the existing
+  # contact
   if ( $invoicing_list ) {
     my $email = '';
-    foreach (@$invoicing_list) {
-      if ($_ eq 'POST') {
+
+    # kind of like process_m2m on these, except:
+    # - the other side is two tables in a join
+    # - and we might have to create new contact_emails
+    # - and possibly a new contact
+    # 
+    # Find existing invoice emails that aren't on the implicit contact.
+    # Any of these that are not on the new invoicing list will be removed.
+    my %old_email_cust_contact;
+    foreach my $cust_contact ($self->cust_contact) {
+      next if !$cust_contact->invoice_dest;
+      next if $cust_contact->contactnum == ($implicit_contact->contactnum || 0);
+
+      foreach my $contact_email ($cust_contact->contact->contact_email) {
+        $old_email_cust_contact{ $contact_email->emailaddress } = $cust_contact;
+      }
+    }
+
+    foreach my $dest (@$invoicing_list) {
+
+      if ($dest eq 'POST') {
+
         $self->set('postal_invoice', 'Y');
+
+      } elsif ( exists($old_email_cust_contact{$dest}) ) {
+
+        delete $old_email_cust_contact{$dest}; # don't need to remove it, then
+
       } else {
-        $email .= ',' if length($email);
-        $email .= $_;
+
+        # See if it belongs to some other contact; if so, link it.
+        my $contact_email = qsearchs('contact_email', { emailaddress => $dest });
+        if ( $contact_email
+             and $contact_email->contactnum != ($implicit_contact->contactnum || 0) ) {
+          my $cust_contact = qsearchs('cust_contact', {
+              contactnum  => $contact_email->contactnum,
+              custnum     => $self->custnum,
+          }) || FS::cust_contact->new({
+              contactnum    => $contact_email->contactnum,
+              custnum       => $self->custnum,
+          });
+          $cust_contact->set('invoice_dest', 'Y');
+          my $error = $cust_contact->custcontactnum ?
+                        $cust_contact->replace : $cust_contact->insert;
+          if ( $error ) {
+            $dbh->rollback if $oldAutoCommit;
+            return "$error (linking to email address $dest)";
+          }
+
+        } else {
+          # This email address is not yet linked to any contact, so it will
+          # be added to the implicit contact.
+          $email .= ',' if length($email);
+          $email .= $dest;
+        }
+      }
+    }
+
+    foreach my $remove_dest (keys %old_email_cust_contact) {
+      my $cust_contact = $old_email_cust_contact{$remove_dest};
+      # These were not in the list of requested destinations, so take them off.
+      $cust_contact->set('invoice_dest', '');
+      my $error = $cust_contact->replace;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "$error (unlinking email address $remove_dest)";
       }
     }
-    my @contacts = map { $_->contact } $self->cust_contact;
-    # if possible, use a contact that matches the customer's name
-    my ($contact) = grep { $_->first eq $old->get('first') and
-                           $_->last  eq $old->get('last') }
-                    @contacts;
-    $contact ||= FS::contact->new({
-        'custnum'       => $self->custnum,
-        'locationnum'   => $self->get('bill_locationnum'),
-    });
-    $contact->set('last', $self->get('last'));
-    $contact->set('first', $self->get('first'));
-    $contact->set('emailaddress', $email);
-    $contact->set('invoice_dest', 'Y');
+
+    # make sure it keeps up with the changed customer name, if any
+    $implicit_contact->set('last', $self->get('last'));
+    $implicit_contact->set('first', $self->get('first'));
+    $implicit_contact->set('emailaddress', $email);
+    $implicit_contact->set('invoice_dest', 'Y');
+    $implicit_contact->set('custnum', $self->custnum);
 
     my $error;
-    if ( $contact->contactnum ) {
-      $error = $contact->replace;
-    } elsif ( length($email) ) { # don't create a new contact if email is empty
-      $error = $contact->insert;
+    if ( $implicit_contact->contactnum ) {
+      $error = $implicit_contact->replace;
+    } elsif ( length($email) ) { # don't create a new contact if not needed
+      $error = $implicit_contact->insert;
     }
 
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
-      return $error;
+      return "$error (adding email address $email)";
     }
 
   }
@@ -2987,7 +3082,7 @@ sub invoicing_list_emailonly {
         addl_from => ' JOIN contact USING (contactnum) '.
                      ' JOIN contact_email USING (contactnum)',
         hashref   => { 'custnum' => $self->custnum, },
-        extra_sql => q( AND invoice_dest = 'Y'),
+        extra_sql => q( AND cust_contact.invoice_dest = 'Y'),
     });
 }
 
diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index 59d1e04..a137143 100644
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -713,14 +713,21 @@ sub insert {
 
       # slight false laziness w/ edit/process/cust_main.cgi...
       # and also slightly arbitrary behavior.
-      # if the "real name" of this account matches the first + last name
-      # of a contact, attach the email address to that person.
-      my @contacts = map { $_->contact } $cust_main->cust_contact;
-      my $myname = $self->get('finger');
-      my ($contact) =
-        grep { $_->get('first') . ' ' . $_->get('last') eq $myname } @contacts;
-      # otherwise just pick the first one
-      $contact ||= $contacts[0];
+      #
+      # this will never happen but check it anyway
+      my ($contact) = map { $_->contact }
+        qsearch('contact_email', { emailaddress => $self->email });
+
+      if (!$contact) {
+        # if the "real name" of this account matches the first + last name
+        # of a contact, attach the email address to that person.
+        my @contacts = map { $_->contact } $cust_main->cust_contact;
+        my $myname = $self->get('finger');
+        my ($contact) =
+          grep { $_->get('first') . ' ' . $_->get('last') eq $myname } @contacts;
+        # otherwise just pick the first one
+        $contact = $contacts[0];
+      }
       # if there is one
       $contact ||= FS::contact->new({
           'custnum'       => $cust_main->get('custnum'),
diff --git a/httemplate/REST/1.0/cust_main b/httemplate/REST/1.0/cust_main
index 5401195..a8b1511 100644
--- a/httemplate/REST/1.0/cust_main
+++ b/httemplate/REST/1.0/cust_main
@@ -51,7 +51,7 @@ if ( $r->method eq 'GET' ) {
                          JOIN contact USING (contactnum)
                          JOIN contact_email USING (contactnum)
                          WHERE cust_main.custnum = cust_contact.custnum
-                           AND contact.invoice_dest = 'Y'
+                           AND cust_contact.invoice_dest = 'Y'
                            AND contact_email.emailaddress = $dest
                      )
       ";
@@ -62,7 +62,7 @@ if ( $r->method eq 'GET' ) {
                          JOIN contact USING (contactnum)
                          JOIN contact_email USING (contactnum)
                          WHERE cust_main.custnum = cust_contact.custnum
-                           AND contact.invoice_dest = 'Y'
+                           AND cust_contact.invoice_dest = 'Y'
                            AND contact_email.emailaddress ILIKE $dest
                      )
       ";
diff --git a/httemplate/edit/cust_main/name.html b/httemplate/edit/cust_main/name.html
index 12d9d74..2025889 100644
--- a/httemplate/edit/cust_main/name.html
+++ b/httemplate/edit/cust_main/name.html
@@ -37,7 +37,7 @@
         : 'label' %>">Email address(es)</SPAN>
   </TH>
   <TD>
-    <INPUT TYPE="text" NAME="invoice_email"  ID="invoice_email_input"
+    <INPUT TYPE="text" NAME="invoice_email"  ID="invoice_email_input" SIZE=40
            VALUE="<% $cust_main->invoicing_list_emailonly_scalar %>">
   </TD>
 </TR>
diff --git a/httemplate/edit/process/cust_main.cgi b/httemplate/edit/process/cust_main.cgi
index 747de20..d590fde 100755
--- a/httemplate/edit/process/cust_main.cgi
+++ b/httemplate/edit/process/cust_main.cgi
@@ -165,7 +165,7 @@ if ( $cgi->param('residential_commercial') eq 'Residential' ) {
     $error = 'Email address required';
   }
 
-  $options{'invoicing_list'} = [ $email ];
+  $options{'invoicing_list'} = [ split(/\s*,\s*/, $email) ];
   # XXX really should include the phone numbers in here also
 
 } else {
diff --git a/httemplate/elements/contact.html b/httemplate/elements/contact.html
index ab14dfb..3fbcc05 100644
--- a/httemplate/elements/contact.html
+++ b/httemplate/elements/contact.html
@@ -40,7 +40,9 @@
 %         }
 %       } elsif ( $field eq 'emailaddress' ) {
 %         $value = join(', ', map $_->emailaddress, $contact->contact_email);
-%       } elsif ( $field eq 'selfservice_access' || $field eq 'comment' ) {
+%       } elsif ( $field eq 'selfservice_access'
+%              or $field eq 'comment'
+%              or $field eq 'invoice_dest' ) {
 %         $value = $X_contact->get($field);
 %       } else {
 %         $value = $contact->get($field);
diff --git a/httemplate/view/cust_main/contacts_new.html b/httemplate/view/cust_main/contacts_new.html
index a0dd301..9448867 100644
--- a/httemplate/view/cust_main/contacts_new.html
+++ b/httemplate/view/cust_main/contacts_new.html
@@ -31,7 +31,7 @@
 
 %       my @contact_email = $contact->contact_email;
         <%$td%><% join(', ', map $_->emailaddress, @contact_email) %></TD>
-        <%$td%><% $contact->invoice_dest eq 'Y' ? 'Yes' : 'No' %></TD>
+        <%$td%><% $cust_contact->invoice_dest eq 'Y' ? 'Yes' : 'No' %></TD>
         <%$td%>
 %         if ( $cust_contact->selfservice_access ) {
             Enabled