Move model rules down into their validate() function for consistency.

Change "in_use" error to "conflict" for consistency.

3 files changed, 23 insertions, 26 deletions

diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php
index 91468250..bc68d154 100644
--- a/modules/user/controllers/admin_users.php
+++ b/modules/user/controllers/admin_users.php
@@ -287,7 +287,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $group = $form->group("edit_user")->label(t("Edit user"));
     $group->input("name")->label(t("Username"))->id("g-username")->value($user->name);
     $group->inputs["name"]->error_messages(
-      "in_use", t("There is already a user with that username"));
+      "conflict", t("There is already a user with that username"));
     $group->input("full_name")->label(t("Full name"))->id("g-fullname")->value($user->full_name);
     self::_add_locale_dropdown($group, $user);
     $group->password("password")->label(t("Password"))->id("g-password");
@@ -306,7 +306,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $form = new Forge("admin/users/add_user", "", "post", array("id" => "g-add-user-form"));
     $group = $form->group("add_user")->label(t("Add user"));
     $group->input("name")->label(t("Username"))->id("g-username")
-      ->error_messages("in_use", t("There is already a user with that username"));
+      ->error_messages("conflict", t("There is already a user with that username"));
     $group->input("full_name")->label(t("Full name"))->id("g-fullname");
     $group->password("password")->label(t("Password"))->id("g-password");
     $group->password("password2")->label(t("Confirm password"))->id("g-password2")
@@ -351,7 +351,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group = $form->group("edit_group")->label(t("Edit group"));
     $form_group->input("name")->label(t("Name"))->id("g-name")->value($group->name);
     $form_group->inputs["name"]->error_messages(
-      "in_use", t("There is already a group with that name"));
+      "conflict", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Save"));
     return $form;
   }
@@ -361,7 +361,7 @@ class Admin_Users_Controller extends Admin_Controller {
     $form_group = $form->group("add_group")->label(t("Add group"));
     $form_group->input("name")->label(t("Name"))->id("g-name");
     $form_group->inputs["name"]->error_messages(
-      "in_use", t("There is already a group with that name"));
+      "conflict", t("There is already a group with that name"));
     $form_group->submit("")->value(t("Add group"));
     return $form;
   }
diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index 16d6adb7..c00bf5c9 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -20,8 +20,6 @@
 class Group_Model extends ORM implements Group_Definition {
   protected $has_and_belongs_to_many = array("users");
 
-  var $rules = array("name" => array("rules" => array("required", "length[4,255]")));
-
   /**
    * @see ORM::delete()
    */
@@ -37,12 +35,14 @@ class Group_Model extends ORM implements Group_Definition {
   }
 
   /**
-   * Add some custom per-instance rules.
+   * Specify our rules here so that we have access to the instance of this model.
    */
   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 = array(
+        "name" => array("rules" => array("required", "length[4,255]"),
+                        "callbacks" => array(array($this, "valid_name"))));
     }
 
     parent::validate($array);
@@ -71,7 +71,7 @@ class Group_Model extends ORM implements Group_Definition {
         ->where("name", "=", $this->name)
         ->where("id", "<>", $this->id)
         ->count_records() == 1) {
-      $v->add_error("name", "in_use");
+      $v->add_error("name", "conflict");
     }
   }
 }
\ No newline at end of file
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index c45f88ac..451b5ffb 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -21,15 +21,6 @@ class User_Model extends ORM implements User_Definition {
   protected $has_and_belongs_to_many = array("groups");
   protected $password_length = null;
 
-  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) {
     case "hashed_password":
@@ -69,17 +60,23 @@ class User_Model extends ORM implements User_Definition {
   }
 
   /**
-   * Add some custom per-instance rules.
+   * Specify our rules here so that we have access to the instance of this model.
    */
   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 = array(
+        "admin"     => array("callbacks" => array(array($this, "valid_admin"))),
+        "email"     => array("rules"     => array("length[1,255]", "required", "valid::email")),
+        "full_name" => array("rules"     => array("length[0,255]")),
+        "locale"    => array("rules"     => array("length[2,10]")),
+        "name"      => array("rules"     => array("length[1,32]", "required"),
+                             "callbacks" => array(array($this, "valid_name"))),
+        "password"  => array("callbacks" => array(array($this, "valid_password"))),
+        "url"       => array("rules"     => array("valid::url")),
+      );
     }
 
-    $this->rules["password"]["callbacks"] = array(array($this, "valid_password"));
-    $this->rules["admin"]["callbacks"] = array(array($this, "valid_admin"));
-
     parent::validate($array);
   }
 
@@ -124,7 +121,7 @@ class User_Model extends ORM implements User_Definition {
         ->where("name", "=", $this->name)
         ->where("id", "<>", $this->id)
         ->count_records() == 1) {
-      $v->add_error("name", "in_use");
+      $v->add_error("name", "conflict");
     }
   }
 
@@ -134,8 +131,8 @@ class User_Model extends ORM implements User_Definition {
   public function valid_password(Validation $v, $field) {
     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");
+      if ($this->password_length < $minimum_length) {
+        $v->add_error("password", "min_length");
       }
     }
   }