[PATCH] improve DHCP dynamic leases list usability

[Leo Hofmann <hofmann@leo-andres.de>]

[-- Attachment #1: Type: text/plain, Size: 10688 bytes --]

Some users have complained that displaying the active and expired leases in one
unordered list is confusing. This patch improves the DHCP user interface by
visually separating active and expired leases. The list is now sorted by active and
expired leases and they are divided by a horizontal line.
Sorting by IP/MAC/host/time and creating static leases remains unchanged.

See forum topic for context: https://community.ipfire.org/t/dhcp-server-expired-leases/

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>

---
  config/cfgroot/header.pl | 167 +++++++++++++++++++++++----------------
  1 file changed, 97 insertions(+), 70 deletions(-)

diff --git a/config/cfgroot/header.pl b/config/cfgroot/header.pl
index 1046f5992..b5d153a55 100644
--- a/config/cfgroot/header.pl
+++ b/config/cfgroot/header.pl
@@ -403,88 +403,110 @@ sub PrintActualLeases
  <th width='25%' align='center'><a href='$ENV{'SCRIPT_NAME'}?ETHER'><b>$tr{'mac address'}</b></a></th>
  <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?HOSTNAME'><b>$tr{'hostname'}</b></a></th>
  <th width='25%' align='center'><a href='$ENV{'SCRIPT_NAME'}?ENDTIME'><b>$tr{'lease expires'} (local time d/m/y)</b></a></th>
-<th width='5%' align='center'><b>Add to fix leases<b></th>
+<th width='5%' align='center'><b>Add to fix leases</b></th>
  </tr>
  END
  ;

      open(LEASES,"/var/state/dhcp/dhcpd.leases") or die "Can't open dhcpd.leases";
-    while ($line = <LEASES>) {
-    next if( $line =~ /^\s*#/ );
-    chomp($line);
-    @temp = split (' ', $line);
-
-    if ($line =~ /^\s*lease/) {
-        $ip = $temp[1];
-        #All field are not necessarily read. Clear everything
-        $endtime = 0;
-        $ether = "";
-        $hostname = "";
-    }
+    while (my $line = <LEASES>) {
+        next if( $line =~ /^\s*#/ );
+        chomp($line);
+        @temp = split (' ', $line);
+
+        if ($line =~ /^\s*lease/) {
+            $ip = $temp[1];
+            #All field are not necessarily read. Clear everything
+            $endtime = 0;
+            $endtime_print = "";
+            $expired = 0;
+            $ether = "";
+            $hostname = "";
+        }

-    if ($line =~ /^\s*ends/) {
-        $line =~ /(\d+)\/(\d+)\/(\d+) (\d+):(\d+):(\d+)/;
-        $endtime = timegm($6, $5, $4, $3, $2 - 1, $1 - 1900);
-    }
+        if ($line =~ /^\s*ends/) {
+            $line =~ /(\d+)\/(\d+)\/(\d+) (\d+):(\d+):(\d+)/;
+            $endtime = timegm($6, $5, $4, $3, $2 - 1, $1 - 1900);
+            ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $dst) = localtime($endtime);
+            $endtime_print = sprintf ("%02d/%02d/%d %02d:%02d:%02d",$mday,$mon+1,$year+1900,$hour,$min,$sec);
+            $expired = $endtime < time();
+        }

-    if ($line =~ /^\s*hardware ethernet/) {
-        $ether = $temp[2];
-        $ether =~ s/;//g;
-    }
+        if ($line =~ /^\s*hardware ethernet/) {
+            $ether = $temp[2];
+            $ether =~ s/;//g;
+        }

-    if ($line =~ /^\s*client-hostname/) {
-        $hostname = "$temp[1] $temp[2] $temp[3]";
-        $hostname =~ s/;//g;
-        $hostname =~ s/\"//g;
-    }
+        if ($line =~ /^\s*client-hostname/) {
+            $hostname = "$temp[1] $temp[2] $temp[3]";
+            $hostname =~ s/;//g;
+            $hostname =~ s/\"//g;
+        }

-    if ($line eq "}") {
-        @record = ('IPADDR',$ip,'ENDTIME',$endtime,'ETHER',$ether,'HOSTNAME',$hostname);
-            $record = {};                                # create a reference to empty hash
-        %{$record} = @record;                        # populate that hash with @record
-        $entries{$record->{'IPADDR'}} = $record;       # add this to a hash of hashes
-    }
+        if ($line eq "}") {
+            @record = ('IPADDR',$ip,'ENDTIME',$endtime,'ETHER',$ether,'HOSTNAME',$hostname,'endtime_print',$endtime_print,'expired',$expired);
+            $record = {};                                # create a reference to empty hash
+            %{$record} = @record;                        # populate that hash with @record
+            $entries{$record->{'IPADDR'}} = $record;    # add this to a hash of hashes
+        }
      }
      close(LEASES);

      my $id = 0;
-    my $col="";
+    my $col = "";
+    my $separator_printed = 0;
      foreach my $key (sort leasesort keys %entries) {
-    print "<form method='post' action='/cgi-bin/dhcp.cgi'>\n";
-    my $hostname = &cleanhtml($entries{$key}->{HOSTNAME},"y");
-
-    if ($id % 2) {
-        print "<tr>";
-        $col="bgcolor='$table1colour'";
-    }
-    else {
-        print "<tr>";
-        $col="bgcolor='$table2colour'";
-    }
-
-    print <<END
+        my $hostname = &cleanhtml($entries{$key}->{HOSTNAME},"y");
+        my $hostname_print = $hostname;
+        if($hostname_print eq "") { #print blank space if no hostname is found
+            $hostname_print = "&nbsp;&nbsp;&nbsp;";
+        }
+
+        # separate active and expired leases with a horizontal line
+        if(($entries{$key}->{expired}) && ($separator_printed == 0)) {
+            $separator_printed = 1;
+            if ($id % 2) {
+                print "<tr><td colspan='5' bgcolor='$table1colour'><hr size='1'></td></tr>\n";
+            } else {
+                print "<tr><td colspan='5' bgcolor='$table2colour'><hr size='1'></td></tr>\n";
+            }
+            $id++;
+        }
+
+        print "<form method='post' action='/cgi-bin/dhcp.cgi'>\n";
+        if ($id % 2) {
+            print "<tr>";
+            $col="bgcolor='$table1colour'";
+        } else {
+            print "<tr>";
+            $col="bgcolor='$table2colour'";
+        }
+
+        if($entries{$key}->{expired}) {
+            print <<END
+<td align='center' $col><input type='hidden' name='FIX_ADDR' value='$entries{$key}->{IPADDR}' /><strike><i>$entries{$key}->{IPADDR}</i></strike></td>
+<td align='center' $col><input type='hidden' name='FIX_MAC' value='$entries{$key}->{ETHER}' /><strike><i>$entries{$key}->{ETHER}</i></strike></td>
+<td align='center' $col><input type='hidden' name='FIX_REMARK' value='$hostname' /><strike><i>$hostname_print<i><strike></td>
+<td align='center' $col><input type='hidden' name='FIX_ENABLED' value='on' /><strike><i>$entries{$key}->{endtime_print}</i></strike></td>
+END
+;
+        } else {
+            print <<END
  <td align='center' $col><input type='hidden' name='FIX_ADDR' value='$entries{$key}->{IPADDR}' />$entries{$key}->{IPADDR}</td>
  <td align='center' $col><input type='hidden' name='FIX_MAC' value='$entries{$key}->{ETHER}' />$entries{$key}->{ETHER}</td>
-<td align='center' $col><input type='hidden' name='FIX_REMARK' value='$hostname' />&nbsp;$hostname</td>
-<td align='center' $col><input type='hidden' name='FIX_ENABLED' value='on' />
+<td align='center' $col><input type='hidden' name='FIX_REMARK' value='$hostname' />$hostname_print</td>
+<td align='center' $col><input type='hidden' name='FIX_ENABLED' value='on' />$entries{$key}->{endtime_print}</td>
  END
-;
-
-    ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $dst) = localtime ($entries{$key}->{ENDTIME});
-    $enddate = sprintf ("%02d/%02d/%d %02d:%02d:%02d",$mday,$mon+1,$year+1900,$hour,$min,$sec);
-
-    if ($entries{$key}->{ENDTIME} < time() ){
-        print "<strike>$enddate</strike>";
-    } else {
-        print "$enddate";
-    }
-    print <<END
-</td><td $col><input type='hidden' name='ACTION' value='$Lang::tr{'add'}2' /><input type='submit' name='SUBMIT' value='$Lang::tr{'add'}' />
-</td></tr></form>
+;
+        }
+
+        print <<END
+<td $col><input type='hidden' name='ACTION' value='$Lang::tr{'add'}2' /><input type='submit' name='SUBMIT' value='$Lang::tr{'add'}' /></td>
+</tr></form>
  END
-;
-    $id++;
-    }
+;
+        $id++;
+    }

      print "</table>";
      &closebox();
@@ -499,11 +521,14 @@ sub leasesort {
          if ($qs eq 'IPADDR') {
              @a = split(/\./,$entries{$a}->{$qs});
              @b = split(/\./,$entries{$b}->{$qs});
+            $entries{$a}->{'expired'} <=> $entries{$b}->{'expired'} ||
              ($b[0]<=>$a[0]) ||
              ($b[1]<=>$a[1]) ||
              ($b[2]<=>$a[2]) ||
              ($b[3]<=>$a[3]);
-        }else {
+            ;
+        } else {
+            $entries{$a}->{'expired'} <=> $entries{$b}->{'expired'} ||
              $entries{$b}->{$qs} cmp $entries{$a}->{$qs};
          }
      }
@@ -511,15 +536,17 @@ sub leasesort {
      {
          $qs=$dhcpsettings{'SORT_LEASELIST'};
          if ($qs eq 'IPADDR') {
-        @a = split(/\./,$entries{$a}->{$qs});
+            @a = split(/\./,$entries{$a}->{$qs});
              @b = split(/\./,$entries{$b}->{$qs});
+            $entries{$a}->{'expired'} <=> $entries{$b}->{'expired'} ||
              ($a[0]<=>$b[0]) ||
-        ($a[1]<=>$b[1]) ||
-        ($a[2]<=>$b[2]) ||
+            ($a[1]<=>$b[1]) ||
+            ($a[2]<=>$b[2]) ||
              ($a[3]<=>$b[3]);
-    }else {
+        } else {
+            $entries{$a}->{'expired'} <=> $entries{$b}->{'expired'} ||
              $entries{$a}->{$qs} cmp $entries{$b}->{$qs};
-    }
+        }
      }
  }