[freeside-commits] branch FREESIDE_4_BRANCH updated. 3431eb1f893dafd05056eb65276a8e9122f9fd31

Mitch Jackson mitch at freeside.biz
Tue Sep 25 19:40:38 PDT 2018


The branch, FREESIDE_4_BRANCH has been updated
       via  3431eb1f893dafd05056eb65276a8e9122f9fd31 (commit)
       via  087d74f7289c302b7b79a97df4ae0f354ef45922 (commit)
       via  774a522ebf772f28ccf3b19f531e2424c62fcc0f (commit)
      from  4af1201514d154f0f76c35085d6a9f33d85ecfdc (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 3431eb1f893dafd05056eb65276a8e9122f9fd31
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sun Jan 28 17:54:00 2018 -0600

    RT# 73421 Fix bug on some "Email customers" report links, docs

diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index 5ff4ed7c0..d2c4a36ed 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -3276,7 +3276,7 @@ contacts with a matching cust_contact.classnum are returned.  When a
 classnum of 0 is given, contacts with a null classnum are also included.
 
 Arguments may also contain the dest flag names 'invoice' or 'message'.
-If given, contacts who's invoice_dest and/or invoice_message flags are
+If given, contacts who's invoice_dest and/or message_dest flags are
 not set to 'Y' will be excluded.
 
 =cut
diff --git a/FS/FS/cust_main_Mixin.pm b/FS/FS/cust_main_Mixin.pm
index 169e1eb65..cceaa4bc7 100644
--- a/FS/FS/cust_main_Mixin.pm
+++ b/FS/FS/cust_main_Mixin.pm
@@ -399,10 +399,10 @@ Text body
 
 This field contains a comma-separated list.  This list may contain:
 
-- the text "invoice" indicating emails should only be sent to contact_email
-  addresses with the invoice_dest flag set
-- the text "message" indicating emails should only be sent to contact_email
-  addresses with the message_dest flag set
+- the text "invoice" indicating contacts with invoice_dest flag should
+  be included
+- the text "message" indicating contacts with message_dest flag should
+  be included
 - numbers representing classnum id values for email contact classes.
   If any classnum are present, emails should only be sent to contact_email
   addresses where contact_email.classnum contains one of these classes.
diff --git a/FS/FS/msg_template/email.pm b/FS/FS/msg_template/email.pm
index 19b9367d9..12f2b2939 100644
--- a/FS/FS/msg_template/email.pm
+++ b/FS/FS/msg_template/email.pm
@@ -214,14 +214,14 @@ A L<MIME::Entity> (or arrayref of them) to attach to the message.
 
 Set a string containing a comma-separated list.  This list may contain:
 
-- the text "invoice" indicating emails should only be sent to contact_email
-  addresses with the invoice_dest flag set
-- the text "message" indicating emails should only be sent to contact_email
-  addresses with the message_dest flag set
-  - numbers representing classnum id values for email contact classes.
-    If any classnum are present, emails should only be sent to contact_email
-    addresses where contact_email.classnum contains one of these classes.
-    The classnum 0 also includes where contact_email.classnum IS NULL
+- the text "invoice" indicating contacts with invoice_dest flag should
+  be included
+- the text "message" indicating contacts with message_dest flag should
+  be included
+- numbers representing classnum id values for email contact classes.
+  If any classnum are present, emails should only be sent to contact_email
+  addresses where contact_email.classnum contains one of these classes.
+  The classnum 0 also includes where contact_email.classnum IS NULL
 
 If neither 'invoice' nor 'message' has been specified, this method will
 behave as if 'invoice' had been selected
diff --git a/httemplate/search/cust_main.html b/httemplate/search/cust_main.html
index 30162506f..0a43a82dd 100755
--- a/httemplate/search/cust_main.html
+++ b/httemplate/search/cust_main.html
@@ -140,8 +140,14 @@ my $menubar = [];
 
 if ( $FS::CurrentUser::CurrentUser->access_right('Bulk send customer notices') ) {
 
+  # URI::query_from does not support hashref
+  #   results in: ...&contacts=HASH(0x55e16cb81da8)&...
+  my %query_hash = %search_hash;
+  delete $query_hash{contacts}
+    if exists $query_hash{contacts} && ref $query_hash{contacts};
+
   my $uri = new URI;
-  $uri->query_form( \%search_hash );
+  $uri->query_form( \%query_hash );
   my $query = $uri->query;
 
   push @$menubar, emt('Email a notice to these customers') =>

commit 087d74f7289c302b7b79a97df4ae0f354ef45922
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sun Jan 28 02:41:17 2018 -0600

    RT# 73421 Fixed E-Mail pipeline to obey contact opt-in flags

diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index aa8ae2a6a..5ff4ed7c0 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -3265,48 +3265,101 @@ sub invoicing_list_emailonly_scalar {
   join(', ', $self->invoicing_list_emailonly);
 }
 
-=item contact_list [ CLASSNUM, ... ]
+=item contact_list [ CLASSNUM, DEST_FLAG... ]
 
-Returns a list of contacts (L<FS::contact> objects) for the customer. If
-a list of contact classnums is given, returns only contacts in those
-classes. If the pseudo-classnum 'invoice' is given, returns contacts that
-are marked as invoice destinations. If '0' is given, also returns contacts
-with no class.
+Returns a list of contacts (L<FS::contact> objects) for the customer.
 
 If no arguments are given, returns all contacts for the customer.
 
+Arguments may contain classnums.  When classnums are specified, only
+contacts with a matching cust_contact.classnum are returned.  When a
+classnum of 0 is given, contacts with a null classnum are also included.
+
+Arguments may also contain the dest flag names 'invoice' or 'message'.
+If given, contacts who's invoice_dest and/or invoice_message flags are
+not set to 'Y' will be excluded.
+
 =cut
 
 sub contact_list {
   my $self = shift;
   my $search = {
     table       => 'contact',
-    select      => 'contact.*, cust_contact.invoice_dest',
+    select      => join(', ',(
+                    'contact.*',
+                    'cust_contact.invoice_dest',
+                    'cust_contact.message_dest',
+    )),
     addl_from   => ' JOIN cust_contact USING (contactnum)',
     extra_sql   => ' WHERE cust_contact.custnum = '.$self->custnum,
   };
 
-  my @orwhere;
+  # Bugfix notes:
+  #   Calling methods were relying on this method to use invoice_dest to
+  #   block e-mail messages.  Depending on parameters, this may or may not
+  #   have actually happened.
+  #
+  #   The bug could cause this SQL to be used to filter e-mail addresses:
+  #
+  #   AND (
+  #     cust_contact.classnums IN (1,2,3)
+  #     OR cust_contact.invoice_dest = 'Y'
+  #   )
+  #
+  #   improperly including everybody with the opt-in flag AND everybody
+  #   in the contact classes
+  #
+  # Possibility to introduce new bugs:
+  #   If callers of this method called it incorrectly, and didn't notice
+  #   because it seemed to send the e-mails they wanted.
+
+  # WHERE ...
+  # AND (
+  #   ( cust_contact.classnum IN (1,2,3) )
+  #   OR
+  #   ( cust_contact.classnum IS NULL )
+  #
+  #   AND (
+  #     ( cust_contact.invoice_dest = 'Y' )
+  #     OR
+  #     ( cust_contact.message_dest = 'Y' )
+  #   )
+  # )
+
+  my @and_dest;
+  my @or_classnum;
   my @classnums;
-  foreach (@_) {
-    if ( $_ eq 'invoice' ) {
-      push @orwhere, 'cust_contact.invoice_dest = \'Y\'';
-    } elsif ( $_ eq '0' ) {
-      push @orwhere, 'cust_contact.classnum is null';
+  for (@_) {
+    if ($_ eq 'invoice' || $_ eq 'message') {
+      push @and_dest, " cust_contact.${_}_dest = 'Y' ";
+    } elsif ($_ eq '0') {
+      push @or_classnum, ' cust_contact.classnum IS NULL ';
     } elsif ( /^\d+$/ ) {
       push @classnums, $_;
     } else {
-      die "bad classnum argument '$_'";
+      croak "bad classnum argument '$_'";
     }
   }
 
-  if (@classnums) {
-    push @orwhere, 'cust_contact.classnum IN ('.join(',', @classnums).')';
-  }
-  if (@orwhere) {
-    $search->{extra_sql} .= ' AND (' .
-                            join(' OR ', map "( $_ )", @orwhere) .
-                            ')';
+  push @or_classnum, 'cust_contact.classnum IN ('.join(',', at classnums).')'
+    if @classnums;
+
+  if (@or_classnum || @and_dest) { # catch, no arguments given
+    $search->{extra_sql} .= ' AND ( ';
+
+      if (@or_classnum) {
+        $search->{extra_sql} .= join ' OR ', map {" ($_) "} @or_classnum;
+        $search->{extra_sql} .= ' AND ( ' if @and_dest;
+      }
+
+      if (@and_dest) {
+        $search->{extra_sql} .= join ' OR ', map {" ($_) "} @and_dest;
+        $search->{extra_sql} .= ' ) ' if @or_classnum;
+      }
+
+    $search->{extra_sql} .= ' ) ';
+
+    warn "\$extra_sql: $search->{extra_sql} \n" if $DEBUG;
   }
 
   qsearch($search);
@@ -6027,4 +6080,3 @@ L<FS::cust_main_invoice>, L<FS::UID>, schema.html from the base documentation.
 =cut
 
 1;
-
diff --git a/FS/FS/cust_main_Mixin.pm b/FS/FS/cust_main_Mixin.pm
index 8b6569a74..169e1eb65 100644
--- a/FS/FS/cust_main_Mixin.pm
+++ b/FS/FS/cust_main_Mixin.pm
@@ -348,10 +348,21 @@ sub cust_search_sql {
 
 =item email_search_result HASHREF
 
-Emails a notice to the specified customers.  Customers without 
-invoice email destinations will be skipped.
+Emails a notice to the specified customer's contact_email addresses.
 
-Parameters: 
+
+If the user has specified "Invoice recipients" on the send e-mail screen,
+contact_email rows containing the invoice_dest flag will be included.
+This option is default, if neither 'invoice' nor 'message' are present.
+
+If the user has specified "Message recipients" on the send e-mail screen,
+contact_email rows containing the message_dest flag will be included.
+
+The selection is indicated by the presence of the text 'message' or
+'invoice' within the to_contact_classnum argument.
+
+
+Parameters:
 
 =over 4
 
@@ -386,9 +397,19 @@ Text body
 
 =item to_contact_classnum
 
-The customer contact class (or classes, as a comma-separated list) to send
-the message to. If unspecified, will be sent to any contacts that are marked
-as invoice destinations (the equivalent of specifying 'invoice').
+This field contains a comma-separated list.  This list may contain:
+
+- the text "invoice" indicating emails should only be sent to contact_email
+  addresses with the invoice_dest flag set
+- the text "message" indicating emails should only be sent to contact_email
+  addresses with the message_dest flag set
+- numbers representing classnum id values for email contact classes.
+  If any classnum are present, emails should only be sent to contact_email
+  addresses where contact_email.classnum contains one of these classes.
+  The classnum 0 also includes where contact_email.classnum IS NULL
+
+If neither 'invoice' nor 'message' has been specified, this method will
+behave as if 'invoice' had been selected
 
 =back
 
@@ -483,8 +504,8 @@ sub email_search_result {
       next; # unlinked object; nothing else we can do
     }
 
-my %to = {};
-if ($to) { $to{'to'} = $to; }
+    my %to = ();
+    if ($to) { $to{'to'} = $to; }
 
     my $cust_msg = $msg_template->prepare(
       'cust_main' => $cust_main,
@@ -736,4 +757,3 @@ L<FS::cust_main>, L<FS::Record>
 =cut
 
 1;
-
diff --git a/FS/FS/msg_template/email.pm b/FS/FS/msg_template/email.pm
index 3850edeb9..19b9367d9 100644
--- a/FS/FS/msg_template/email.pm
+++ b/FS/FS/msg_template/email.pm
@@ -210,6 +210,22 @@ go away in the future.
 
 A L<MIME::Entity> (or arrayref of them) to attach to the message.
 
+=item to_contact_classnum
+
+Set a string containing a comma-separated list.  This list may contain:
+
+- the text "invoice" indicating emails should only be sent to contact_email
+  addresses with the invoice_dest flag set
+- the text "message" indicating emails should only be sent to contact_email
+  addresses with the message_dest flag set
+  - numbers representing classnum id values for email contact classes.
+    If any classnum are present, emails should only be sent to contact_email
+    addresses where contact_email.classnum contains one of these classes.
+    The classnum 0 also includes where contact_email.classnum IS NULL
+
+If neither 'invoice' nor 'message' has been specified, this method will
+behave as if 'invoice' had been selected
+
 =cut
 
 =back
@@ -296,8 +312,16 @@ sub prepare {
 
     my $classnum = $opt{'to_contact_classnum'} || '';
     my @classes = ref($classnum) ? @$classnum : split(',', $classnum);
-    # traditional behavior: send to all invoice recipients
-    @classes = ('invoice') unless @classes;
+
+    # There are two e-mail opt-in flags per contact_email address.
+    # If neither 'invoice' nor 'message' has been specified, default
+    # to 'invoice'.
+    #
+    # This default supports the legacy behavior of
+    #    send to all invoice recipients
+    push @classes,'invoice'
+      unless grep {$_ eq 'invoice' || $_ eq 'message'} @classes;
+
     @to = $cust_main->contact_list_email(@classes);
     # not guaranteed to produce contacts, but then customers aren't
     # guaranteed to have email addresses on file. in that case, env_to
@@ -630,4 +654,3 @@ L<FS::Record>, schema.html from the base documentation.
 =cut
 
 1;
-
diff --git a/httemplate/misc/email-customers.html b/httemplate/misc/email-customers.html
index 445ede248..b3a21767c 100644
--- a/httemplate/misc/email-customers.html
+++ b/httemplate/misc/email-customers.html
@@ -171,16 +171,18 @@ Template:
  <TD>Send to contacts:</TD>
  <TD>
    <div id="contactclassesdiv">
-  <& /elements/checkboxes.html,
-    'style'               => 'display: inline; vertical-align: top',
-    'disable_links'       => 1,
-    'names_list'          => \@contact_checkboxes,
-    'element_name_prefix' => 'contact_class_',
-    'checked_callback'    => sub {
-      my($cgi, $name) = @_;
-      $name eq 'invoice' #others default to unchecked
-    },
-  &>
+     <& /elements/checkboxes.html,
+       'style'               => 'display: inline; vertical-align: top',
+       'disable_links'       => 1,
+       'names_list'          => \@contact_checkboxes,
+       'element_name_prefix' => 'contact_class_',
+       'checked_callback'    => sub {
+         # Called for each checkbox
+         # Return true to default as checked, false as unchecked
+         my($cgi, $name) = @_;
+         $name eq 'message'
+       },
+     &>
    </div>
 % if ($send_to_domain) {
    <div>
@@ -424,6 +426,8 @@ if ( !$cgi->param('preview') ) {
         push @contact_classnum, $1;
         if ( $1 eq 'invoice' ) {
           push @contact_classname, 'Invoice recipients';
+        } elsif ( $1 eq 'message' ) {
+          push @contact_classname, 'Message recipients';
         } else {
           my $contact_class = FS::contact_class->by_key($1);
           push @contact_classname, encode_entities($contact_class->classname);
@@ -434,7 +438,8 @@ if ( !$cgi->param('preview') ) {
 }
 
 my @contact_checkboxes = (
-  [ 'invoice' => { label => 'Invoice recipients' } ]
+  [ 'message' => { label => 'Message recipients' } ],
+  [ 'invoice' => { label => 'Invoice recipients' } ],
 );
 
 foreach my $class (qsearch('contact_class', { disabled => '' })) {
diff --git a/httemplate/view/cust_main/contacts_new.html b/httemplate/view/cust_main/contacts_new.html
index 45187b1ad..9252b2197 100644
--- a/httemplate/view/cust_main/contacts_new.html
+++ b/httemplate/view/cust_main/contacts_new.html
@@ -10,8 +10,8 @@
   <%$th%>Contact</TH>
   <%$th%>Email</TH>
   <%$th%>Send invoices</TH>
-  <%$th%>Self-service</TH>
   <%$th%>Send messages</TH>
+  <%$th%>Self-service</TH>
 % foreach my $phone_type (@phone_type) {
     <%$th%><% $phone_type->typename |h %></TH>
 % }

commit 774a522ebf772f28ccf3b19f531e2424c62fcc0f
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat Jan 27 21:21:59 2018 -0600

    RT# 73421 Add allow messages flag message_dest to contact_email

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 08eae6a32..873c67e22 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -1764,7 +1764,8 @@ sub tables_hashref {
         'classnum',              'int', 'NULL',  '', '', '',
         'comment',           'varchar', 'NULL', 255, '', '',
         'selfservice_access',   'char', 'NULL',   1, '', '',
-        'invoice_dest',         'char', 'NULL',       1, '', '',
+        'invoice_dest',         'char', 'NULL',   1, '', '', # Y or NULL
+        'message_dest',         'char', 'NULL',   1, '', '', # Y or NULL
       ],
       'primary_key'  => 'custcontactnum',
       'unique'       => [ [ 'custnum', 'contactnum' ], ],
diff --git a/FS/FS/contact.pm b/FS/FS/contact.pm
index fc06ca82b..9505dee35 100644
--- a/FS/FS/contact.pm
+++ b/FS/FS/contact.pm
@@ -155,7 +155,7 @@ sub insert {
   $self->custnum('');
 
   my %link_hash = ();
-  for (qw( classnum comment selfservice_access invoice_dest )) {
+  for (qw( classnum comment selfservice_access invoice_dest message_dest)) {
     $link_hash{$_} = $self->get($_);
     $self->$_('');
   }
@@ -437,7 +437,7 @@ sub replace {
   $self->custnum('');
 
   my %link_hash = ();
-  for (qw( classnum comment selfservice_access invoice_dest )) {
+  for (qw( classnum comment selfservice_access invoice_dest message_dest )) {
     $link_hash{$_} = $self->get($_);
     $self->$_('');
   }
@@ -968,7 +968,7 @@ sub cgi_contact_fields {
 
   my @contact_fields = qw(
     classnum first last title comment emailaddress selfservice_access
-    invoice_dest password
+    invoice_dest message_dest password
   );
 
   push @contact_fields, 'phonetypenum'. $_->phonetypenum
@@ -1041,4 +1041,3 @@ L<FS::Record>, schema.html from the base documentation.
 =cut
 
 1;
-
diff --git a/FS/FS/cust_contact.pm b/FS/FS/cust_contact.pm
index 118a9e000..adad46e9e 100644
--- a/FS/FS/cust_contact.pm
+++ b/FS/FS/cust_contact.pm
@@ -59,6 +59,11 @@ empty or Y
 
 'Y' if the customer should get invoices sent to this address, null if not
 
+=item message_dest
+
+'Y' if contact should get non-invoice email messages sent to this address,
+NULL if not
+
 =back
 
 =head1 METHODS
@@ -119,6 +124,7 @@ sub check {
     || $self->ut_textn('comment')
     || $self->ut_enum('selfservice_access', [ '', 'Y' ])
     || $self->ut_flag('invoice_dest')
+    || $self->ut_flag('message_dest')
   ;
   return $error if $error;
 
@@ -148,4 +154,3 @@ L<FS::contact>, L<FS::cust_main>, L<FS::Record>
 =cut
 
 1;
-
diff --git a/httemplate/edit/process/cust_main-contacts.html b/httemplate/edit/process/cust_main-contacts.html
index 6e94a290f..6b7f1c2db 100644
--- a/httemplate/edit/process/cust_main-contacts.html
+++ b/httemplate/edit/process/cust_main-contacts.html
@@ -1,3 +1,11 @@
+<%doc>
+
+  This form works indirectly with the tables contact_email and
+  contact_phone.  The columns are updated against an FS::contact
+  object.  The insert/update methods of FS::contact will make the
+  necessary inserts/updates to contact_email and contact_phone.
+
+</%doc>
 <% include('elements/process.html',
      'table'          => 'cust_main',
      'error_redirect' => popurl(3). 'edit/cust_main-contacts.html',
diff --git a/httemplate/elements/contact.html b/httemplate/elements/contact.html
index 31b4e49cb..909ff7893 100644
--- a/httemplate/elements/contact.html
+++ b/httemplate/elements/contact.html
@@ -46,7 +46,8 @@
 %          $value = $contact->get('_password') ? '********' : '';
 %       } elsif ( $field eq 'selfservice_access'
 %              or $field eq 'comment'
-%              or $field eq 'invoice_dest' ) {
+%              or $field eq 'invoice_dest'
+%              or $field eq 'message_dest' ) {
 %         $value = $X_contact->get($field);
 %       } else {
 %         $value = $contact->get($field);
@@ -79,7 +80,7 @@
             <SCRIPT>
               <% $js %>
             </SCRIPT>
-%         } elsif ( $field eq 'invoice_dest' ) {
+%         } elsif ( $field eq 'invoice_dest' || $field eq 'message_dest' ) {
 %           my $curr_value = $cgi->param($name . '_' . $field);
 %           $curr_value = $value if !defined($curr_value);
             <& select.html,
@@ -173,6 +174,7 @@ tie my %label, 'Tie::IxHash',
 
 unless ($opt{'for_prospect'}) {
   $label{'invoice_dest'} = 'Send invoices';
+  $label{'message_dest'} = 'Send messages';
   $label{'selfservice_access'} = 'Self-service';
   $label{'password'} = 'Password';
 }
diff --git a/httemplate/view/cust_main/contacts_new.html b/httemplate/view/cust_main/contacts_new.html
index 2209d30f3..45187b1ad 100644
--- a/httemplate/view/cust_main/contacts_new.html
+++ b/httemplate/view/cust_main/contacts_new.html
@@ -11,6 +11,7 @@
   <%$th%>Email</TH>
   <%$th%>Send invoices</TH>
   <%$th%>Self-service</TH>
+  <%$th%>Send messages</TH>
 % foreach my $phone_type (@phone_type) {
     <%$th%><% $phone_type->typename |h %></TH>
 % }
@@ -33,6 +34,7 @@
 %       my @contact_email = $contact->contact_email;
         <%$td%><% join(', ', map $_->emailaddress, @contact_email) %></TD>
         <%$td%><% $cust_contact->invoice_dest eq 'Y' ? 'Yes' : 'No' %></TD>
+        <%$td%><% $cust_contact->message_dest eq 'Y' ? 'Yes' : 'No' %></TD>
         <%$td%>
 %         if ( $cust_contact->selfservice_access ) {
             Enabled

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/Schema.pm                                 |  3 +-
 FS/FS/contact.pm                                |  7 +-
 FS/FS/cust_contact.pm                           |  7 +-
 FS/FS/cust_main.pm                              | 96 +++++++++++++++++++------
 FS/FS/cust_main_Mixin.pm                        | 38 +++++++---
 FS/FS/msg_template/email.pm                     | 29 +++++++-
 httemplate/edit/process/cust_main-contacts.html |  8 +++
 httemplate/elements/contact.html                |  6 +-
 httemplate/misc/email-customers.html            | 27 ++++---
 httemplate/search/cust_main.html                |  8 ++-
 httemplate/view/cust_main/contacts_new.html     |  2 +
 11 files changed, 177 insertions(+), 54 deletions(-)




More information about the freeside-commits mailing list