Fixing up numbers and totals

Some areas like quickstats and total needed to use count_distinct so I
created MAPS::CountLogDistinct.

diff --git a/lib/Utils.pm b/lib/Utils.pm
index 9cb99e3..ea189a1 100644 (file)
--- a/lib/Utils.pm
+++ b/lib/Utils.pm
@@ -104,9 +104,11 @@ sub CheckParms($$) {
 
   my $msg = RequiredFields($requiredFields, $rec);
 
-  my $function = (caller(1))[3];
+  my $function   = (caller(1))[3];
+  my $calledFrom = (caller(2))[3];
+  my $lineNbr    = (caller(2))[2];
 
-  croak "Internal error: $function: $msg" if $msg;
+  croak "Internal error: $function called from $calledFrom:$lineNbr\n\nThe field $msg" if $msg;
 
   return;
 } # CheckParms
@@ -255,7 +257,7 @@ STDOUT then do so in the $command passed in.
 
   chomp @output;
 
-  wantarray ? return ($status, @output) : $status;
+  return wantarray ? ($status, @output) : $status;
 } # Execute
 
 sub GetChildren(;$) {
@@ -804,6 +806,8 @@ Returns:
     print $out "$_\n";
   } # for
 
+  close $out;
+  
   return;
 } # RedirectOutput
 

diff --git a/maps/bin/add2blacklist.cgi b/maps/bin/add2blacklist.cgi
index 34fb5e9..bfa8f4d 100755 (executable)
--- a/maps/bin/add2blacklist.cgi
+++ b/maps/bin/add2blacklist.cgi
@@ -52,7 +52,7 @@ sub Add2List(%) {
 
     my ($status, $rule) = OnBlacklist($rec{sender});
 
-    if ($status != 0) {
+    if ($status) {
       print br {-class => 'error'},
         "The email address $rec{sender} is already on ${Userid}'s $rec{type} list";
     } else {

diff --git a/maps/bin/add2nulllist.cgi b/maps/bin/add2nulllist.cgi
index e222fa0..1007526 100755 (executable)
--- a/maps/bin/add2nulllist.cgi
+++ b/maps/bin/add2nulllist.cgi
@@ -53,7 +53,7 @@ sub Add2List(%) {
 
     my ($status, $rule) = OnNulllist($rec{sender});
 
-    if ($status != 0) {
+    if ($status) {
       print br {-class => 'error'},
         "The email address $rec{sender} is already on ${Userid}'s $rec{type} list";
     } else {

diff --git a/maps/bin/add2nulllist.pl b/maps/bin/add2nulllist.pl
index 13324d9..82a3714 100755 (executable)
--- a/maps/bin/add2nulllist.pl
+++ b/maps/bin/add2nulllist.pl
@@ -69,7 +69,7 @@ sub Add2List(@) {
 
     my ($status, $rule) = OnNulllist($item->{sender}, $userid);
 
-    if ($status == 0) {
+    if ($status) {
       display ' Already on list';
     } else {
       my ($message, $msg) = Add2Nulllist(%$item);
@@ -88,16 +88,16 @@ sub Add2List(@) {
 
         if ($seq->{sequence}) {
           my $count = DeleteList(
-            userid   => $item->{userid}
+            userid   => $item->{userid},
             type     => $otherlist,
-            sequence => $seq->{sequence}
+            sequence => $seq->{sequence},
           );
 
           display "Removed $item->{sender} from ${Userid}'s " . ucfirst $otherlist . ' list'
             if $count > 0;
 
           ResequenceList(
-            userid => $rec{userid},
+            userid => $item->{userid},
             type   => $otherlist,
           );
         } # if

diff --git a/maps/bin/add2whitelist.cgi b/maps/bin/add2whitelist.cgi
index 6b75398..e8cd9e2 100755 (executable)
--- a/maps/bin/add2whitelist.cgi
+++ b/maps/bin/add2whitelist.cgi
@@ -53,7 +53,7 @@ sub Add2List(%) {
 
     my ($status, $rule) = OnWhitelist($rec{sender}, $rec{userid});
 
-    if ($status != 0) {
+    if ($status) {
       print br {-class => 'error'},
         "The email address $rec{sender} is already on ${Userid}'s $rec{type} list";
     } else {

diff --git a/maps/bin/detail.cgi b/maps/bin/detail.cgi
index f4885fd..ee2d2f4 100755 (executable)
--- a/maps/bin/detail.cgi
+++ b/maps/bin/detail.cgi
@@ -240,7 +240,11 @@ sub Body($) {
       } # unless
     } # unless
 
-    my ($list, $sequence, $comment);
+    $hit_count //= '';
+
+    my $list     = '';
+    my $sequence = 0;
+    my $comment  = '';
 
     # Parse rule
     if ($rule) {
@@ -259,6 +263,7 @@ sub Body($) {
       $rule =~ s/\\@/\@/;
     } # if
 
+    $sequence //= 0;
     $next++;
 
     # Start Sender line
@@ -311,12 +316,18 @@ sub Body($) {
     }, $comment;
     print end_Tr;
 
+    my $msgnbr = 0;
+
     for my $rec (@$msgs) {
+      $msgnbr++;
+
       # We increased $next earlier so do not add 1 here
       if (($next % $lines) == (@senders % $lines)) {
         $dataclass    = 'tablebottomdata';
-        $rightclass   = 'tablebottomright';
-        $subjectclass = 'subjectbottom';
+        $rightclass   = 'tablebottomright' if $msgnbr == @$msgs;
+
+        # Only subjectbottom the last message
+        $subjectclass = 'subjectbottom' if $msgnbr == @$msgs;
       } # if
 
       if ($date eq substr ($rec->{timestamp}, 0, 10)) {
@@ -336,11 +347,11 @@ sub Body($) {
             -class   => $subjectclass,
             -colspan => 4,
           }, a {
-            -href  => "display.cgi?sender=$sender;msg_date=$rec->{timestamp}",
+            -href    => "display.cgi?sender=$sender;msg_date=$rec->{timestamp}",
            }, '    ' . $rec->{subject},
-          td {-class   => $rightclass,
-              -width   => '150',
-              -align   => 'right'}, span {-class => 'date'}, $rec->{date} . '&nbsp',
+          td {-class => $rightclass,
+              -width => '150',
+              -align => 'right'}, span {-class => 'date'}, $rec->{date} . '&nbsp',
         ];
     } # for
   } # for
@@ -388,8 +399,10 @@ if ($date eq '') {
   $condition .= "type = '$type' and timestamp > '$sod' and timestamp < '$eod'";
 } # if
 
-$total = CountLog(
+# Need to count distinct on sender
+$total = CountLogDistinct(
   userid     => $userid,
+  column     => 'sender',
   additional => $condition,
 );
 

diff --git a/maps/bin/display.cgi b/maps/bin/display.cgi
index 65be9b2..a8dc765 100755 (executable)
--- a/maps/bin/display.cgi
+++ b/maps/bin/display.cgi
@@ -170,7 +170,7 @@ sub Body($) {
             # There should be an easier way to get this but I couldn't find one.
             my $encoding = ${$subpart->{mail_inet_head}{mail_hdr_hash}{'Content-Transfer-Encoding'}[0]};
             if ($encoding =~ /base64/) {
-              $subpart->bodayhandle->print;
+              $subpart->bodyhandle->print;
             } else {
               print $subpart->print_body;
             } # if

diff --git a/maps/css/MAPSStyle.css b/maps/css/MAPSStyle.css
index ca8c491..3945309 100644 (file)
--- a/maps/css/MAPSStyle.css
+++ b/maps/css/MAPSStyle.css
@@ -81,25 +81,19 @@ body {
   margin:           2px;
   width:            125px;
 }
-
-.smallnumber a:link {
-  color:         yellow;
+.quickstats a:link {
+  color:           white;
   text-decoration: none;
 }
-
-.smallnumber a:visited {
+.quickstats a:visited {
   color: white;
-  font-weight: bold;
-  background-color: yellow;
-}
-.smallnumber a:hover {
-  color: yellow;
-  background: blue;
 }
-
 .quickstats a:hover {
-  background: blue;
-  color:      white;
+  color:      steelblue;
+  background: white;
+}
+.quickstats a:active {
+  color: Yellow;
 }
 
 .toolbar a:hover {
@@ -436,7 +430,7 @@ img {
   border-left:                   solid 3px SteelBlue;
   border-right:                  1px dotted #ccc;
   border-bottom:                 solid 3px SteelBlue;
-  font-size:                     14px;
+  font-size:                     12px;
   -moz-border-radius-bottomleft: 7px;
   border-bottom-left-radius:     7px;
 }
@@ -570,21 +564,17 @@ img {
 .menu {
   font-weight: bold;
 }
-
 .menu a:link {
   color:           White;
   text-decoration: none;
 }
-
 .menu a:visited {
   color: White;
 }
-
 .menu a:hover {
   color:      Yellow;
   background: none;
 }
-
 .menu a:active {
   color: Yellow;
 }
@@ -593,20 +583,16 @@ img {
 .intromenu {
   font-weight: bold;
 }
-
 .intromenu a:link {
   color: Red;
 }
-
 .intromenu a:visited {
   color: white;
 }
-
 .intromenu a:hover {
   color:      White;
   background: Red;
 }
-
 .intromenu a:active {
   color: Yellow;
 }

diff --git a/maps/lib/MAPS.pm b/maps/lib/MAPS.pm
index 3e0b066..7bafc97 100644 (file)
--- a/maps/lib/MAPS.pm
+++ b/maps/lib/MAPS.pm
@@ -57,6 +57,7 @@ our @EXPORT = qw(
   CountEmail
   CountList
   CountLog
+  CountLogDistinct
   Decrypt
   DeleteEmail
   DeleteList
@@ -223,6 +224,7 @@ sub Add2Whitelist(%) {
 
   # Add to white list
   $params{sequence} = 0;
+
   my ($err, $msg) = AddList(%params);
 
   return -$err, $msg if $err;
@@ -247,7 +249,7 @@ sub Add2Whitelist(%) {
   while (my $rec = $db->getnext) {
     last unless $rec->{userid};
 
-    $status = Whitelist($rec->{sender}, $rec->data);
+    $status = Whitelist($rec->{sender}, $rec->{data});
 
     last if $status;
 
@@ -632,6 +634,19 @@ sub CountLog(%) {
   return $db->count('log', $condition);
 } # CountLog
 
+sub CountLogDistinct(%) {
+  my (%params) = @_;
+
+  CheckParms(['userid', 'column'], \%params);
+
+  my ($additional_condition) = delete $params{additional} || '';
+
+  my $condition  = "userid='$userid'";
+     $condition .= " and $additional_condition" if $additional_condition;
+
+  return $db->count_distinct('log', $params{column}, $condition);
+} # CountLog
+
 sub Decrypt ($$) {
   my ($password, $userid) = @_;
 

diff --git a/maps/lib/MAPSLog.pm b/maps/lib/MAPSLog.pm
index 33ac5c8..433de2b 100644 (file)
--- a/maps/lib/MAPSLog.pm
+++ b/maps/lib/MAPSLog.pm
@@ -68,8 +68,9 @@ sub GetStats(%) {
     for (@Types) {
       my $condition = "type=\'$_\' and (timestamp > \'$sod\' and timestamp < \'$eod\')";
 
-      $stats{$_} = MAPS::CountLog(
+      $stats{$_} = MAPS::CountLogDistinct(
         userid     => $params{userid},
+        column     => 'sender',
         additional => $condition,
       );
     } # for

diff --git a/maps/lib/MAPSWeb.pm b/maps/lib/MAPSWeb.pm
index 5dd9d23..3389c6a 100644 (file)
--- a/maps/lib/MAPSWeb.pm
+++ b/maps/lib/MAPSWeb.pm
@@ -54,7 +54,7 @@ sub getquickstats(%) {
 sub displayquickstats($) {
   my ($userid) = @_;
 
-  # Quick stats are today only.
+  # Quick stats are today only
   my $today = Today2SQLDatetime;
   my $time  = substr $today, 11;
   my $date  = substr $today, 0, 10;
@@ -94,6 +94,7 @@ sub displayquickstats($) {
 
     my $value = $dates{$date}{$_};
     my $percent;
+
     if ($_ eq 'mailloop' || $_ eq 'registered') {
       $percent = 'n/a';
     } else {
@@ -101,16 +102,22 @@ sub displayquickstats($) {
         0 : $dates{$date}{$_} / $dates{$date}{processed} * 100;
       $percent = sprintf '%5.1f%s', $percent, '%';
     } # if
-    my $stat = $value == 0 ?
-      0 : a {-href => "detail.cgi?type=$_;date=$date"}, $value;
-    print
-      td {-class => 'smalllabel'}, ucfirst ($_);
-    print
-      td {-class => 'smallnumber'}, $stat;
-    print
-      td {-class => 'smallnumber'}, $percent;
+
+    my $report = ucfirst $_;
+
+    if ($value) {
+      $report  = a {-href => "detail.cgi?type=$_;date=$date"}, $report;
+      $value   = a {-href => "detail.cgi?type=$_;date=$date"}, $value;
+      $percent = a {-href => "detail.cgi?type=$_;date=$date"}, $percent;
+    } # if
+
+    print td {-class => 'smalllabel'},  $report,
+          td {-class => 'smallnumber'}, $value,
+          td {-class => 'smallnumber'}, $percent;
+
     print end_Tr;
-  } # foreach
+  } # for
+
   print end_table;
   print end_div;
 

diff --git a/rc/perlcriticrc b/rc/perlcriticrc
index 3490060..f850357 100644 (file)
--- a/rc/perlcriticrc
+++ b/rc/perlcriticrc
@@ -53,3 +53,6 @@ minimum_regex_length_to_complain_about = 50
 
 # I'm OK with homonyms
 [-Subroutines::ProhibitBuiltinHomonyms]
+
+# I'm OK with so called ambiguous names
+[-NamingConventions::ProhibitAmbiguousNames]