Convert Admin_Users_Controller, User_Model and Group_Model to use

model based validation.

5 files changed, 103 insertions, 103 deletions

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 7f08f8a1..c35eba73 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -37,10 +37,9 @@ class Admin_Users_Controller extends Admin_Controller {
       $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->url = $form->edit_user->url->value;
+      $user->locale = $form->add_user->locale->value;
+      $user->admin = $form->edit_user->admin->checked;
       $user->validate();
     } catch (ORM_Validation_Exception $e) {
       // Translate ORM validation errors into form error messages
@@ -110,43 +109,34 @@ class Admin_Users_Controller extends Admin_Controller {
     }
 
     $form = $this->_get_user_edit_form_admin($user);
-    $valid = $form->validate();
-    if ($valid) {
-      $new_name = $form->edit_user->inputs["name"]->value;
-      $temp_user = user::lookup_by_name($new_name);
-      if ($new_name != $user->name &&
-          ($temp_user && $temp_user->id != $user->id)) {
-        $form->edit_user->inputs["name"]->add_error("in_use", 1);
-        $valid = false;
-      } else {
-        $user->name = $new_name;
-      }
-    }
-
-    if ($valid) {
+    try {
+      $valid = $form->validate();
+      $user->name = $form->edit_user->inputs["name"]->value;
       $user->full_name = $form->edit_user->full_name->value;
-      if ($form->edit_user->password->value) {
-        $user->password = $form->edit_user->password->value;
-      }
+      $user->password = $form->edit_user->password->value;
       $user->email = $form->edit_user->email->value;
       $user->url = $form->edit_user->url->value;
-      if ($form->edit_user->locale) {
-        $desired_locale = $form->edit_user->locale->value;
-        $user->locale = $desired_locale == "none" ? null : $desired_locale;
-      }
-
-      // An admin can change the admin status for any user but themselves
+      $user->locale = $form->edit_user->locale->value;
       if ($user->id != identity::active_user()->id) {
         $user->admin = $form->edit_user->admin->checked;
       }
+
+      $user->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->edit_user->inputs[$key]->add_error($error, 1);
+      }
+      $valid = false;
+    }
+
+    if ($valid) {
       $user->save();
       module::event("user_edit_form_admin_completed", $user, $form);
-
       message::success(t("Changed 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));
     }
   }
 
@@ -191,25 +181,26 @@ class Admin_Users_Controller extends Admin_Controller {
     access::verify_csrf();
 
     $form = $this->_get_group_add_form_admin();
-    $valid = $form->validate();
-    if ($valid) {
-      $new_name = $form->add_group->inputs["name"]->value;
-      $group = group::lookup_by_name($new_name);
-      if (!empty($group)) {
-        $form->add_group->inputs["name"]->add_error("in_use", 1);
-        $valid = false;
+    try {
+      $valid = $form->validate();
+      $group = ORM::factory("group");
+      $group->name = $form->add_group->inputs["name"]->value;
+      $group->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->add_group->inputs[$key]->add_error($error, 1);
       }
+      $valid = false;
     }
 
     if ($valid) {
-      $group = group::create($new_name);
       $group->save();
       message::success(
         t("Created group %group_name", array("group_name" => $group->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));
     }
   }
 
@@ -258,19 +249,19 @@ class Admin_Users_Controller extends Admin_Controller {
     }
 
     $form = $this->_get_group_edit_form_admin($group);
-    $valid = $form->validate();
-
-    if ($valid) {
-      $new_name = $form->edit_group->inputs["name"]->value;
-      $group = group::lookup_by_name($name);
-      if ($group->loaded()) {
-        $form->edit_group->inputs["name"]->add_error("in_use", 1);
-        $valid = false;
+    try {
+      $valid = $form->validate();
+      $group->name = $form->edit_group->inputs["name"]->value;
+      $group->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        $form->edit_group->inputs[$key]->add_error($error, 1);
       }
+      $valid = false;
     }
 
     if ($valid) {
-      $group->name = $form->edit_group->inputs["name"]->value;
       $group->save();
       message::success(
         t("Changed group %group_name", array("group_name" => $group->name)));
@@ -278,8 +269,7 @@ class Admin_Users_Controller extends Admin_Controller {
     } else {
       message::error(
         t("Failed to change group %group_name", array("group_name" => $group->name)));
-      print json_encode(array("result" => "error",
-                              "form" => $form->__toString()));
+      print json_encode(array("result" => "error", "form" => (string) $form));
     }
   }
 
@@ -308,10 +298,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $group->input("email")->label(t("Email"))->id("g-email")->value($user->email);
     $group->input("url")->label(t("URL"))->id("g-url")->value($user->url);
     $group->checkbox("admin")->label(t("Admin"))->id("g-admin")->checked($user->admin);
-    $form->add_rules_from($user);
-    $minimum_length = module::get_var("user", "mininum_password_length", 5);
-    $form->edit_user->password
-      ->rules($minimum_length ? "length[$minimum_length, 40]" : "length[40]");
 
     module::event("user_edit_form_admin", $user, $form);
     $group->submit("")->value(t("Modify User"));
@@ -342,15 +328,14 @@ class Admin_Users_Controller extends Admin_Controller {
     foreach ($locales as $locale => $display_name) {
       $locales[$locale] = SafeString::of_safe_html($display_name);
     }
-    if (count($locales) > 1) {
-      // Put "none" at the first position in the array
-      $locales = array_merge(array("" => t("« none »")), $locales);
-      $selected_locale = ($user && $user->locale) ? $user->locale : "";
-      $form->dropdown("locale")
-        ->label(t("Language Preference"))
-        ->options($locales)
-        ->selected($selected_locale);
-    }
+
+    // Put "none" at the first position in the array
+    $locales = array_merge(array("" => t("« none »")), $locales);
+    $selected_locale = ($user && $user->locale) ? $user->locale : "";
+    $form->dropdown("locale")
+      ->label(t("Language Preference"))
+      ->options($locales)
+      ->selected($selected_locale);
   }
 
   private function _get_user_delete_form_admin($user) {
@@ -370,7 +355,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group->inputs["name"]->error_messages(
       "in_use", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Save"));
-    $form->add_rules_from($group);
     return $form;
   }
 
@@ -381,7 +365,6 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group->inputs["name"]->error_messages(
       "in_use", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Add group"));
-    $form->add_rules_from(ORM::factory("group"));
     return $form;
   }
 
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index ca218393..71f9a889 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -95,14 +95,13 @@ class Users_Controller extends Controller {
     foreach ($locales as $locale => $display_name) {
       $locales[$locale] = SafeString::of_safe_html($display_name);
     }
-    if (count($locales) > 1) {
-      // Put "none" at the first position in the array
-      $locales = array_merge(array("" => t("« none »")), $locales);
-      $selected_locale = ($user && $user->locale) ? $user->locale : "";
-      $form->dropdown("locale")
-        ->label(t("Language Preference"))
-        ->options($locales)
-        ->selected($selected_locale);
-    }
+
+    // Put "none" at the first position in the array
+    $locales = array_merge(array("" => t("« none »")), $locales);
+    $selected_locale = ($user && $user->locale) ? $user->locale : "";
+    $form->dropdown("locale")
+      ->label(t("Language Preference"))
+      ->options($locales)
+      ->selected($selected_locale);
   }
 }
diff --git a/modules/user/helpers/group.php b/modules/user/helpers/group.php
index 2ada0ac1..38124b0d 100644
--- a/modules/user/helpers/group.php
+++ b/modules/user/helpers/group.php
@@ -25,23 +25,6 @@
  */
 class group_Core {
   /**
-   * Create a new group.
-   *
-   * @param string  $name
-   * @return Group_Definition the group object
-   */
-  static function create($name) {
-    $group = ORM::factory("group")->where("name", "=", $name)->find();
-    if ($group->loaded()) {
-      throw new Exception("@todo GROUP_ALREADY_EXISTS $name");
-    }
-
-    $group->name = $name;
-    $group->save();
-    return $group;
-  }
-
-  /**
    * The group of all possible visitors.  This includes the guest user.
    *
    * @return Group_Definition the group object
diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index 10f6f4b3..16d6adb7 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -20,8 +20,7 @@
 class Group_Model extends ORM implements Group_Definition {
   protected $has_and_belongs_to_many = array("users");
 
-  var $form_rules = array(
-    "name" => "required|length[4,255]");
+  var $rules = array("name" => array("rules" => array("required", "length[4,255]")));
 
   /**
    * @see ORM::delete()
@@ -37,18 +36,42 @@ class Group_Model extends ORM implements Group_Definition {
     return $this->users->find_all();
   }
 
-  public function save() {
-    if (!$this->loaded()) {
-        $created = 1;
+  /**
+   * 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"));
     }
 
-    $original = clone $this->original();
-    parent::save();
-    if (isset($created)) {
+    parent::validate($array);
+  }
+
+  public function save() {
+    if (!$this->loaded()) {
+      // New group
+      parent::save();
       module::event("group_created", $this);
     } else {
+      // Updated group
+      $original = clone $this->original();
+      parent::save();
       module::event("group_updated", $original, $this);
     }
+
     return $this;
   }
+
+  /**
+   * Validate the user name.  Make sure there are no conflicts.
+   */
+  public function valid_name(Validation $v, $field) {
+    if (db::build()->from("groups")
+        ->where("name", "=", $this->name)
+        ->where("id", "<>", $this->id)
+        ->count_records() == 1) {
+      $v->add_error("name", "in_use");
+    }
+  }
 }
\ No newline at end of file
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 12da5784..c45f88ac 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -78,6 +78,7 @@ class User_Model extends ORM implements User_Definition {
     }
 
     $this->rules["password"]["callbacks"] = array(array($this, "valid_password"));
+    $this->rules["admin"]["callbacks"] = array(array($this, "valid_admin"));
 
     parent::validate($array);
   }
@@ -131,9 +132,20 @@ class User_Model extends ORM implements User_Definition {
    * 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");
+    if (!$this->loaded() || $this->password_length) {
+      $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");
+      }
+    }
+  }
+
+  /**
+   * Validate the admin bit.
+   */
+  public function valid_admin(Validation $v, $field) {
+    if ($this->id == identity::active_user()->id && !$this->admin) {
+      $v->add_error("admin", "locked");
     }
   }
 }