Changeset 297

Timestamp:

04/02/12 17:26:53 (13 years ago)

Author:

Kris Deugau

Message:

/trunk

Move action logging for user delete into delUser(). See #35
Tweak log entry/result message for userStatus() for consistency
and clarity

Location:

trunk

Files:

• DNSDB.pm (modified) (2 diffs)
• dns.cgi (modified) (1 diff)

trunk/DNSDB.pm

@@ -2235 +2235 @@
 
 ## DNSDB::delUser()
-#
+# Delete a user.
+# Takes a database handle and user ID
+# Returns a success/failure code and matching message
 sub delUser {
   my $dbh = shift;
-  return ('FAIL',"Need database handle") if !$dbh;
   my $userid = shift;
-  return ('FAIL',"Missing userid") if !defined($userid);
-
-  my $sth = $dbh->prepare("delete from users where user_id=?");
-  $sth->execute($userid);
-
-  return ('FAIL',"Couldn't remove user: ".$sth->errstr) if $sth->err;
-
-  return ('OK','OK');
-
+
+  return ('FAIL',"Bad userid") if !defined($userid);
+
+  my $userdata = getUserData($dbh, $userid);
+
+  # Allow transactions, and raise an exception on errors so we can catch it later.
+  # Use local to make sure these get "reset" properly on exiting this block
+  local $dbh->{AutoCommit} = 0;
+  local $dbh->{RaiseError} = 1;
+
+  eval {
+    $dbh->do("DELETE FROM users WHERE user_id=?", undef, ($userid));
+    _log($dbh, (group_id => $userdata->{group_id},
+        entry => "Deleted user ID $userid/".$userdata->{username}.
+                " (".$userdata->{firstname}." ".$userdata->{lastname}.")") );
+    $dbh->commit;
+  };
+  if ($@) {
+    my $msg = $@;
+    eval { $dbh->rollback; };
+    if ($config{log_failures}) {
+      _log($dbh, (group_id => $userdata->{group_id}, entry => "Error deleting user ID ".
+        "$userid/".$userdata->{username}.": $msg") );
+      $dbh->commit;
+    }
+    return ('FAIL',"Error deleting user $userid/".$userdata->{username}.": $msg");
+  }
+
+  return ('OK',"Deleted user ".$userdata->{username}." (".$userdata->{firstname}." ".$userdata->{lastname}.")");
 } # end delUser
 
@@ -2303 +2324 @@
       $dbh->do("UPDATE users SET status=? WHERE user_id=?", undef, ($newstatus, $id));
 
-      $resultstr = ($newstatus ? 'Enabled' : 'Disabled')." user ".$userdata->{username};
+      $resultstr = ($newstatus ? 'Enabled' : 'Disabled')." user ".$userdata->{username}.
+        " (".$userdata->{firstname}." ".$userdata->{lastname}.")";
 
       my %loghash;

trunk/dns.cgi

@@ -1383 +1383 @@
     $page->param(user => userFullName($dbh,$webvar{id}));
   } elsif ($webvar{del} eq 'ok') {
-##fixme: find group id user is in (for logging) *before* we delete the user
-##fixme: get other user data too for log
-    my $userref = getUserData($dbh, $webvar{id});
     my ($code,$msg) = delUser($dbh, $webvar{id});
     if ($code eq 'OK') {
       # success.  go back to the user list, do not pass "GO"
-      # actions on users have a domain id of 0, always
-      logaction(0, $session->param("username"), $curgroup, "Deleted user $webvar{id}/".$userref->{username}.
-        " (".$userref->{lastname}.", ".$userref->{firstname}.")");
-      changepage(page => "useradmin", resultmsg => "Deleted user ".$userref->{username}.
-        " (".$userref->{lastname}.", ".$userref->{firstname}.")");
+      changepage(page => "useradmin", resultmsg => $msg);
     } else {
-# need to find failure mode
-      $page->param(del_failed => 1);
-      $page->param(errmsg => $msg);
-      list_users($curgroup);
-      logaction(0, $session->param("username"), $curgroup, "Failed to delete user ".
-        "$webvar{id}/".$userref->{username}.": $msg")
-        if $config{log_failures};
+      changepage(page => "useradmin", errmsg => $msg);
     }
   } else {