fix contact self-service disabled on edit (and some basic contact reports), RT#25533, RT#32171, RT#32212

10 files changed, 162 insertions, 48 deletions

diff --git a/FS/FS/AccessRight.pm b/FS/FS/AccessRight.pm
index 4e5703203..bad831a94 100644
--- a/FS/FS/AccessRight.pm
+++ b/FS/FS/AccessRight.pm
@@ -93,11 +93,9 @@ tie my %rights, 'Tie::IxHash',
     'New prospect',
     'View prospect',
     'Edit prospect',
-    'List prospects',
     'Edit contact', #!
     #'New contact',
     #'View customer contacts',
-    #'List contacts',
     'Generate quotation',
   ],
   
@@ -269,6 +267,8 @@ tie my %rights, 'Tie::IxHash',
   # report/listing rights...
   ###
   'Reporting/listing rights' => [
+    'List prospects',
+    'List contacts',
     'List customers',
     'List all customers',
     'Advanced customer search',
diff --git a/FS/FS/access_right.pm b/FS/FS/access_right.pm
index 87b052f5a..ee0c494ae 100644
--- a/FS/FS/access_right.pm
+++ b/FS/FS/access_right.pm
@@ -247,6 +247,8 @@ sub _upgrade_data { # class method
     'Add on-the-fly credit reason' => 'Add on-the-fly refund reason',
     'Configuration' => 'Edit global fee definitions',
     'Edit package definition costs' => 'View package definition costs',
+    'List prospects' => 'List contacts',
+    'List customers' => 'List contacts',
   );
 
 #  foreach my $old_acl ( keys %onetime ) {
diff --git a/FS/FS/contact.pm b/FS/FS/contact.pm
index 60c521612..3205df106 100644
--- a/FS/FS/contact.pm
+++ b/FS/FS/contact.pm
@@ -4,10 +4,11 @@ use base qw( FS::Record );
 use strict;
 use vars qw( $skip_fuzzyfiles );
 use Scalar::Util qw( blessed );
-use FS::Record qw( qsearchs dbh ); # qw( qsearch qsearchs dbh );
+use FS::Record qw( qsearch qsearchs dbh );
 use FS::contact_phone;
 use FS::contact_email;
 use FS::queue;
+use FS::phone_type; #for cgi_contact_fields
 
 $skip_fuzzyfiles = 0;
 
@@ -32,8 +33,9 @@ FS::contact - Object methods for contact records
 
 =head1 DESCRIPTION
 
-An FS::contact object represents an example.  FS::contact inherits from
-FS::Record.  The following fields are currently supported:
+An FS::contact object represents an specific contact person for a prospect or
+customer.  FS::contact inherits from FS::Record.  The following fields are
+currently supported:
 
 =over 4
 
@@ -92,15 +94,13 @@ disabled
 
 =item new HASHREF
 
-Creates a new example.  To add the example to the database, see L<"insert">.
+Creates a new contact.  To add the contact to the database, see L<"insert">.
 
 Note that this stores the hash reference, not a distinct copy of the hash it
 points to.  You can ask the object for a copy with the I<hash> method.
 
 =cut
 
-# the new method can be inherited from FS::Record, if a table method is defined
-
 sub table { 'contact'; }
 
 =item insert
@@ -194,8 +194,6 @@ Delete this record from the database.
 
 =cut
 
-# the delete method can be inherited from FS::Record
-
 sub delete {
   my $self = shift;
 
@@ -350,7 +348,15 @@ sub replace {
 
 }
 
-#i probably belong in contact_phone.pm
+=item _parse_phonestring PHONENUMBER_STRING
+
+Subroutine, takes a string and returns a list (suitable for assigning to a hash)
+with keys 'countrycode', 'phonenum' and 'extension'
+
+(Should probably be moved to contact_phone.pm, hence the initial underscore.)
+
+=cut
+
 sub _parse_phonestring {
   my $value = shift;
 
@@ -413,15 +419,12 @@ sub queue_fuzzyfiles_update {
 
 =item check
 
-Checks all fields to make sure this is a valid example.  If there is
+Checks all fields to make sure this is a valid contact.  If there is
 an error, returns the error, otherwise returns false.  Called by the insert
 and replace methods.
 
 =cut
 
-# the check method should currently be supplied - FS::Record contains some
-# data checking routines
-
 sub check {
   my $self = shift;
 
@@ -456,6 +459,13 @@ sub check {
   $self->SUPER::check;
 }
 
+=item line
+
+Returns a formatted string representing this contact, including name, title and
+comment.
+
+=cut
+
 sub line {
   my $self = shift;
   my $data = $self->first. ' '. $self->last;
@@ -466,17 +476,37 @@ sub line {
   $data;
 }
 
+=item firstlast
+
+Returns a formatted string representing this contact, with just the name.
+
+=cut
+
 sub firstlast {
   my $self = shift;
   $self->first . ' ' . $self->last;
 }
 
+=item contact_classname
+
+Returns the name of this contact's class (see L<FS::contact_class>).
+
+=cut
+
 sub contact_classname {
   my $self = shift;
   my $contact_class = $self->contact_class or return '';
   $contact_class->classname;
 }
 
+=item by_selfservice_email EMAILADDRESS
+
+Alternate search constructor (class method).  Given an email address,
+returns the contact for that address, or the empty string if no contact
+has that email address.
+
+=cut
+
 sub by_selfservice_email {
   my($class, $email) = @_;
 
@@ -624,6 +654,31 @@ sub myaccount_cache {
                        } );
 }
 
+=item cgi_contact_fields
+
+Returns a list reference containing the set of contact fields used in the web
+interface for one-line editing (i.e. excluding contactnum, prospectnum, custnum
+and locationnum, as well as password fields, but including fields for
+contact_email and contact_phone records.)
+
+=cut
+
+sub cgi_contact_fields {
+  #my $class = shift;
+
+  my @contact_fields = qw(
+    classnum first last title comment emailaddress selfservice_access
+  );
+
+  push @contact_fields, 'phonetypenum'. $_->phonetypenum
+    foreach qsearch({table=>'phone_type', order_by=>'weight'});
+
+  \@contact_fields;
+
+}
+
+use FS::phone_type;
+
 =back
 
 =head1 BUGS
diff --git a/httemplate/edit/process/cust_main-contacts.html b/httemplate/edit/process/cust_main-contacts.html
index d902dee09..10ec3638f 100644
--- a/httemplate/edit/process/cust_main-contacts.html
+++ b/httemplate/edit/process/cust_main-contacts.html
@@ -5,18 +5,8 @@
      'skip_process'   => 1, #we don't want to make any changes to cust_main
      'process_o2m' => {
        'table'  => 'contact',
-       'fields' => \@contact_fields,
+       'fields' => FS::contact->cgi_contact_fields,
      },
      'redirect' => popurl(3). 'view/cust_main.cgi?',
    )
 %>
-<%init>
-
-my @contact_fields = qw(
-  classnum first last title comment emailaddress selfservice_access
-);
-foreach my $phone_type ( qsearch({table=>'phone_type', order_by=>'weight'}) ) {
-  push @contact_fields, 'phonetypenum'.$phone_type->phonetypenum;
-}
-
-</%init>
diff --git a/httemplate/edit/process/cust_main.cgi b/httemplate/edit/process/cust_main.cgi
index 4fb8f622d..82ec50c36 100755
--- a/httemplate/edit/process/cust_main.cgi
+++ b/httemplate/edit/process/cust_main.cgi
@@ -59,6 +59,9 @@ $cgi->param('invoicing_list', join(',', @invoicing_list) );
 
 # is this actually used?  if so, we need to clone locations...
 # but I can't find anything that sets this parameter to a non-empty value
+# yes, fec48523d3cf056da08813f9b2b7d633b27aaf8d for #16582 is where it came in,
+# for "duplicate address checking for new customers".  afaict still in
+# edit/cust_main/bottomfixup.html (and working?)
 $cgi->param('duplicate_of_custnum') =~ /^(\d+)$/;
 my $duplicate_of = $1;
 
@@ -350,15 +353,11 @@ if ( $new->custnum eq '' or $duplicate_of ) {
   
 }
 
-unless ( $error ) { #XXX i guess i should be transactional... all in the insert
+unless ( $error ) { #XXX i should be transactional... all in the insert
                     # or replace call
-  my @contact_fields = qw( classnum first last title comment emailaddress );
-  foreach my $phone_type ( qsearch({table=>'phone_type', order_by=>'weight'}) ) {
-    push @contact_fields, 'phonetypenum'.$phone_type->phonetypenum;
-  }
 
   $error = $new->process_o2m( 'table'  => 'contact',
-                              'fields' => \@contact_fields,
+                              'fields' => FS::contact->cgi_contact_fields,
                               'params' => scalar($cgi->Vars),
                             );
 }
diff --git a/httemplate/search/contact.html b/httemplate/search/contact.html
index 0292fa539..193349369 100644
--- a/httemplate/search/contact.html
+++ b/httemplate/search/contact.html
@@ -8,16 +8,23 @@
                      extra_sql => $extra_sql,
                    },
   count_query   => "SELECT COUNT(*) FROM contact $extra_sql", #XXX
-  header        => [ 'First', 'Last', 'Title', 'Company', 'Self-service', ],
-  fields        => [ 'first', 'last', 'title', 'company', 'selfservice_access' ],
-  links         => [ '', '', '', $company_link, '', ],
+  header        => \@header,
+  fields        => \@fields,
+  links         => \@links,
 &>
 <%init>
 
+die "access denied"
+  unless $FS::CurrentUser::CurrentUser->access_right('List contacts');
+
 my $select = 'contact.*';
 my %hash = ();
 my $addl_from = '';
 
+my @header = ( 'First', 'Last', 'Title', );
+my @fields = ( 'first', 'last', 'title', );
+my @links = ( '', '', '' );
+
 my $company_link = '';
 
 if ( $cgi->param('selfservice_access') eq 'Y' ) {
@@ -25,18 +32,36 @@ if ( $cgi->param('selfservice_access') eq 'Y' ) {
 }
 
 my $extra_sql = '';
-if ( $cgi->param('cust_main') ) {
-  $select .= ', cust_main.company';
-  $addl_from = ' LEFT JOIN cust_main USING ( custnum )';
-  $extra_sql = ' custnum IS NOT NULL ';
-  $company_link  = [ $p.'view/cust_main.cgi?', 'custnum' ];
-} elsif ( $cgi->param('prospect_main') ) {
-  $select .= ', prospect_main.company';
-  $addl_from = ' LEFT JOIN prospect_main USING ( prospectnum )';
-  $extra_sql = ' prospectnum IS NOT NULL ';
-  $company_link  = [ $p.'view/prospect_main.html?', 'prospectnum' ];
+if ( $cgi->param('link') ) {
+
+  my $coalesce = ', COALESCE( cust_main.company,';
+  my $as       = ') AS prospect_or_customer';
+
+  if ( $cgi->param('link') eq 'cust_main' ) {
+    push @header, 'Customer';
+    $select .= "$coalesce cust_main.first||' '||cust_main.last $as";
+    $addl_from = ' LEFT JOIN cust_main USING ( custnum )';
+    $extra_sql = ' custnum IS NOT NULL ';
+    $company_link  = [ $p.'view/cust_main.cgi?', 'custnum' ];
+  } elsif ( $cgi->param('link') eq 'prospect_main' ) {
+    push @header, 'Prospect';
+    $select .= "$coalesce contact.first||'  '||contact.last $as";
+    $addl_from = ' LEFT JOIN prospect_main USING ( prospectnum )';
+    $extra_sql = ' prospectnum IS NOT NULL ';
+    $company_link  = [ $p.'view/prospect_main.html?', 'prospectnum' ];
+  } else {
+    die "don't know how to report on contacts linked to specified table";
+  }
+
+  #because right now its harder to show it for both kinds of contacts
+  push @fields, 'prospect_or_customer';
+  push @links, $company_link; 
+
 }
 
+push @header, 'Self-service';
+push @fields, 'selfservice_access';
+
 $extra_sql = (keys(%hash) ? ' AND ' : ' WHERE '). $extra_sql
  if $extra_sql;
 
diff --git a/httemplate/search/report_contact.html b/httemplate/search/report_contact.html
new file mode 100644
index 000000000..3583bb428
--- /dev/null
+++ b/httemplate/search/report_contact.html
@@ -0,0 +1,43 @@
+<& /elements/header.html, mt($title) &>
+
+<FORM ACTION="contact.html" METHOD="GET">
+
+<TABLE BGCOLOR="#cccccc" CELLSPACING=0>
+
+  <& /elements/tr-select-agent.html,
+                 'curr_value'    => scalar( $cgi->param('agentnum') ),
+                 'label'         => emt('Contacts for agent: '),
+                 'disable_empty' => 0,
+  &>
+
+  <& /elements/tr-select.html,
+       'label'      => 'Contact source', #??? not "type" - contacts have a type
+       'field'      => 'link',
+       'options'    => [ 'prospect_main', 'cust_main', '' ],
+       'labels'     => { 'prospect_main' => 'Prospect contacts',
+                         'cust_main'     => 'Customer contacts',
+                         ''              => 'All contacts',
+                       },
+       'curr_value' => scalar( $cgi->param('link') ),
+  &>
+
+</FORM>
+
+</TABLE>
+
+<BR>
+<INPUT TYPE="submit" VALUE="<% mt('Get Report') |h %>">
+
+</FORM>
+
+<& /elements/footer.html &>
+<%init>
+
+die "access denied"
+  unless $FS::CurrentUser::CurrentUser->access_right('List contacts');
+
+my $conf = new FS::Conf;
+
+my $title = 'Contact Report';
+
+</%init>
diff --git a/httemplate/search/report_cust_credit_bill_pkg.html b/httemplate/search/report_cust_credit_bill_pkg.html
index 2b9e1e69d..175403219 100644
--- a/httemplate/search/report_cust_credit_bill_pkg.html
+++ b/httemplate/search/report_cust_credit_bill_pkg.html
@@ -3,7 +3,7 @@
 <FORM ACTION="cust_credit_bill_pkg.html" METHOD="GET">
 <!--<INPUT TYPE="hidden" NAME="magic" VALUE="_date">-->
 
-<TABLE BGCOLOR="#cccccc" CELLSPACING=0
+<TABLE BGCOLOR="#cccccc" CELLSPACING=0>
 
 <& /elements/tr-select-user.html,
               'label'       => emt('Employee: '),
diff --git a/httemplate/search/report_quotation.html b/httemplate/search/report_quotation.html
index 1be904dc3..8f4da6a1f 100644
--- a/httemplate/search/report_quotation.html
+++ b/httemplate/search/report_quotation.html
@@ -5,7 +5,7 @@
 <INPUT TYPE="hidden" NAME="prospectnum" VALUE="<% $prospectnum %>">
 <INPUT TYPE="hidden" NAME="custnum" VALUE="<% $custnum %>">
 
-<TABLE BGCOLOR="#cccccc" CELLSPACING=0
+<TABLE BGCOLOR="#cccccc" CELLSPACING=0>
 
 % unless ( $custnum ) {
   <& /elements/tr-select-agent.html,
diff --git a/httemplate/search/report_sqlradius_usage.html b/httemplate/search/report_sqlradius_usage.html
index 7e54465d3..e818fb57d 100644
--- a/httemplate/search/report_sqlradius_usage.html
+++ b/httemplate/search/report_sqlradius_usage.html
@@ -2,7 +2,7 @@
 
 <FORM ACTION="sqlradius_usage.html" METHOD="GET">
 
-<TABLE BGCOLOR="#cccccc" CELLSPACING=0
+<TABLE BGCOLOR="#cccccc" CELLSPACING=0>
 
 <& /elements/tr-select-agent.html,
   'empty_label'   => 'all',