Convert Admin_Users::add_user() to use model based validation. Get

the rules and business logic out of the form and user::create(), and
move it into User_Model::save().

3 files changed, 78 insertions, 59 deletions

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index ab747528..7f08f8a1 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -30,31 +30,33 @@ class Admin_Users_Controller extends Admin_Controller {
     access::verify_csrf();
 
     $form = $this->_get_user_add_form_admin();
-    $valid = $form->validate();
-    $name = $form->add_user->inputs["name"]->value;
-    if ($user = user::lookup_by_name($name)) {
-      $form->add_user->inputs["name"]->add_error("in_use", 1);
+    try {
+      $user = ORM::factory("user");
+      $valid = $form->validate();
+      $user->name = $form->add_user->inputs["name"]->value;
+      $user->full_name = $form->add_user->full_name->value;
+      $user->password = $form->add_user->password->value;
+      $user->email = $form->add_user->email->value;
+
+      if (!empty($form->add_user->locale->value)) {
+        $user->locale = $form->add_user->locale->value;
+      }
+      $user->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->add_user->inputs[$key]->add_error($error, 1);
+      }
       $valid = false;
     }
 
     if ($valid) {
-      $user = user::create(
-        $name, $form->add_user->full_name->value, $form->add_user->password->value);
-      $user->email = $form->add_user->email->value;
-      $user->admin = $form->add_user->admin->checked;
-
-      if ($form->add_user->locale) {
-        $desired_locale = $form->add_user->locale->value;
-        $user->locale = $desired_locale == "none" ? null : $desired_locale;
-      }
       $user->save();
       module::event("user_add_form_admin_completed", $user, $form);
-
       message::success(t("Created user %user_name", array("user_name" => $user->name)));
       print json_encode(array("result" => "success"));
     } else {
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -329,11 +331,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $group->input("url")->label(t("URL"))->id("g-url");
     self::_add_locale_dropdown($group);
     $group->checkbox("admin")->label(t("Admin"))->id("g-admin");
-    $form->add_rules_from(ORM::factory("user"));
-
-    $minimum_length = module::get_var("user", "mininum_password_length", 5);
-    $form->add_user->password
-      ->rules($minimum_length ? "required|length[$minimum_length, 40]" : "length[40]");
 
     module::event("user_add_form_admin", $user, $form);
     $group->submit("")->value(t("Add user"));
diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php
index e092aecc..3561021f 100644
--- a/modules/user/helpers/user.php
+++ b/modules/user/helpers/user.php
@@ -36,32 +36,6 @@ class user_Core {
   }
 
   /**
-   * Create a new user.
-   *
-   * @param string  $name
-   * @param string  $full_name
-   * @param string  $password
-   * @return User_Model
-   */
-  static function create($name, $full_name, $password) {
-    $user = ORM::factory("user")->where("name", "=", $name)->find();
-    if ($user->loaded()) {
-      throw new Exception("@todo USER_ALREADY_EXISTS $name");
-    }
-
-    $user->name = $name;
-    $user->full_name = $full_name;
-    $user->password = $password;
-
-    // Required groups
-    $user->add(group::everybody());
-    $user->add(group::registered_users());
-
-    $user->save();
-    return $user;
-  }
-
-  /**
    * Is the password provided correct?
    *
    * @param user User Model
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index edba2a2c..12da5784 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -19,14 +19,16 @@
  */
 class User_Model extends ORM implements User_Definition {
   protected $has_and_belongs_to_many = array("groups");
+  protected $password_length = null;
 
-  var $form_rules = array(
-    "name" => "required|length[1,32]",
-    "full_name" => "length[0,255]",
-    "email" => "required|valid_email|length[1,255]",
-    "password" => "length[1,40]",
-    "url" => "valid_url",
-    "locale" => "length[2,10]");
+  var $rules = array(
+    "name"      => array("rules" => array("length[1,32]", "required")),
+    "locale"    => array("rules" => array("length[2,10]")),
+    "password"  => array("rules" => array("length[5,40]")),  // note: overridden in validate()
+    "email"     => array("rules" => array("length[1,255]", "required", "valid::email")),
+    "full_name" => array("rules" => array("length[0,255]")),
+    "url"       => array("rules" => array("valid::url")),
+  );
 
   public function __set($column, $value) {
     switch ($column) {
@@ -35,6 +37,7 @@ class User_Model extends ORM implements User_Definition {
       break;
 
     case "password":
+      $this->password_length = strlen($value);
       $value = user::hash_password($value);
       break;
     }
@@ -65,18 +68,41 @@ class User_Model extends ORM implements User_Definition {
     return $this->groups->find_all();
   }
 
+  /**
+   * Add some custom per-instance rules.
+   */
+  public function validate($array=null) {
+    // validate() is recursive, only modify the rules on the outermost call.
+    if (!$array) {
+      $this->rules["name"]["callbacks"] = array(array($this, "valid_name"));
+    }
+
+    $this->rules["password"]["callbacks"] = array(array($this, "valid_password"));
+
+    parent::validate($array);
+  }
+
+  /**
+   * Handle any business logic necessary to create or update a user.
+   * @see ORM::save()
+   *
+   * @return ORM User_Model
+   */
   public function save() {
     if (!$this->loaded()) {
-        $created = 1;
-    }
+      // New user
+      $this->add(group::everybody());
+      $this->add(group::registered_users());
 
-    $original = clone $this->original();
-    parent::save();
-    if (isset($created)) {
+      parent::save();
       module::event("user_created", $this);
     } else {
+      // Updated user
+      $original = clone $this->original();
+      parent::save();
       module::event("user_updated", $original, $this);
     }
+
     return $this;
   }
 
@@ -88,4 +114,26 @@ class User_Model extends ORM implements User_Definition {
   public function display_name() {
     return empty($this->full_name) ? $this->name : $this->full_name;
   }
+
+  /**
+   * Validate the user name.  Make sure there are no conflicts.
+   */
+  public function valid_name(Validation $v, $field) {
+    if (db::build()->from("users")
+        ->where("name", "=", $this->name)
+        ->where("id", "<>", $this->id)
+        ->count_records() == 1) {
+      $v->add_error("name", "in_use");
+    }
+  }
+
+  /**
+   * Validate the password.
+   */
+  public function valid_password(Validation $v, $field) {
+    $minimum_length = module::get_var("user", "mininum_password_length", 5);
+    if ($this->password_length < $minimum_length || $this->password_length > 40) {
+      $v->add_error("password", "length");
+    }
+  }
 }