From a691dcc63cb6403784e061997cc85606a8f953b3 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 19:58:55 -0800
Subject: 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().

---
 modules/user/models/user.php | 72 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 12 deletions(-)

(limited to 'modules/user/models')

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");
+    }
+  }
 }
-- 
cgit v1.2.3


From 6a4dda9bdef81bcf79abe5601fd7309e593078f3 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 21:15:12 -0800
Subject: Convert Admin_Users_Controller, User_Model and Group_Model to use
 model based validation.

---
 modules/user/controllers/admin_users.php | 115 +++++++++++++------------------
 modules/user/controllers/users.php       |  17 +++--
 modules/user/helpers/group.php           |  17 -----
 modules/user/models/group.php            |  39 ++++++++---
 modules/user/models/user.php             |  18 ++++-
 5 files changed, 103 insertions(+), 103 deletions(-)

(limited to 'modules/user/models')

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
@@ -24,23 +24,6 @@
  * Note: by design, this class does not do any permission checking.
  */
 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.
    *
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");
     }
   }
 }
-- 
cgit v1.2.3


From 9488684220cbf4121dea12a28083f6c34b648da8 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sun, 17 Jan 2010 12:30:24 -0800
Subject: Move model rules down into their validate() function for consistency.
 Change "in_use" error to "conflict" for consistency.

---
 modules/user/controllers/admin_users.php |  8 ++++----
 modules/user/models/group.php            | 10 +++++-----
 modules/user/models/user.php             | 31 ++++++++++++++-----------------
 3 files changed, 23 insertions(+), 26 deletions(-)

(limited to 'modules/user/models')

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");
       }
     }
   }
-- 
cgit v1.2.3


From b0ff4418d2d56be5eb6ac5b26255dc8a40404806 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 20 Jan 2010 22:55:22 -0800
Subject: Stop using MY_ORM::original()

---
 modules/user/models/group.php | 2 +-
 modules/user/models/user.php  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'modules/user/models')

diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index c00bf5c9..85114ede 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -55,7 +55,7 @@ class Group_Model extends ORM implements Group_Definition {
       module::event("group_created", $this);
     } else {
       // Updated group
-      $original = clone $this->original();
+      $original = ORM::factory("group")->where("id", "=", $this->id)->find();
       parent::save();
       module::event("group_updated", $original, $this);
     }
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 451b5ffb..c43ee9a1 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -96,7 +96,7 @@ class User_Model extends ORM implements User_Definition {
       module::event("user_created", $this);
     } else {
       // Updated user
-      $original = clone $this->original();
+      $original = ORM::factory("user")->where("id", "=", $this->id)->find();
       parent::save();
       module::event("user_updated", $original, $this);
     }
-- 
cgit v1.2.3


From 01dfa2988856043a71974bc509d05c8c267f0d6e Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Mon, 25 Jan 2010 20:40:44 -0800
Subject: Make some exceptions for guests: 1) They don't require email 2) Guest
 users aren't in the everybody group.

---
 modules/user/models/user.php | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

(limited to 'modules/user/models')

diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index c43ee9a1..7c97bae7 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -67,7 +67,8 @@ class User_Model extends ORM implements User_Definition {
     if (!$array) {
       $this->rules = array(
         "admin"     => array("callbacks" => array(array($this, "valid_admin"))),
-        "email"     => array("rules"     => array("length[1,255]", "required", "valid::email")),
+        "email"     => array("rules"     => array("length[1,255]", "valid::email"),
+                             "callbacks" => array(array($this, "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"),
@@ -90,7 +91,9 @@ class User_Model extends ORM implements User_Definition {
     if (!$this->loaded()) {
       // New user
       $this->add(group::everybody());
-      $this->add(group::registered_users());
+      if (!$this->guest) {
+        $this->add(group::registered_users());
+      }
 
       parent::save();
       module::event("user_created", $this);
@@ -129,6 +132,10 @@ class User_Model extends ORM implements User_Definition {
    * Validate the password.
    */
   public function valid_password(Validation $v, $field) {
+    if ($this->guest) {
+      return;
+    }
+
     if (!$this->loaded() || $this->password_length) {
       $minimum_length = module::get_var("user", "mininum_password_length", 5);
       if ($this->password_length < $minimum_length) {
@@ -145,4 +152,17 @@ class User_Model extends ORM implements User_Definition {
       $v->add_error("admin", "locked");
     }
   }
+
+  /**
+   * Validate the email field.
+   */
+  public function valid_email(Validation $v, $field) {
+    if ($this->guest) {  // guests don't require an email address
+      return;
+    }
+
+    if (empty($this->email)) {
+      $v->add_error("email", "required");
+    }
+  }
 }
-- 
cgit v1.2.3


From 4b32a71afc7650fe7bdd02ba384c8914f60538f3 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 27 Jan 2010 22:34:11 -0800
Subject: Convert back to using ORM::factory(..., $id) instead of calling
 where().

---
 modules/comment/models/comment.php            |  2 +-
 modules/comment/tests/Comment_Event_Test.php  |  2 +-
 modules/g2_import/controllers/g2.php          |  2 +-
 modules/g2_import/helpers/g2_import.php       | 11 +++++------
 modules/gallery/helpers/gallery_installer.php |  2 +-
 modules/gallery/helpers/item_rest.php         |  2 +-
 modules/gallery/libraries/ORM_MPTT.php        |  2 +-
 modules/gallery/models/item.php               |  4 ++--
 modules/server_add/controllers/server_add.php | 13 ++++++-------
 modules/tag/helpers/item_tags_rest.php        |  2 +-
 modules/tag/helpers/tag_item_rest.php         |  4 ++--
 modules/tag/helpers/tag_items_rest.php        |  2 +-
 modules/tag/helpers/tag_rest.php              |  2 +-
 modules/user/models/group.php                 |  2 +-
 modules/user/models/user.php                  |  2 +-
 15 files changed, 26 insertions(+), 28 deletions(-)

(limited to 'modules/user/models')

diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php
index 43c4148f..8be022b5 100644
--- a/modules/comment/models/comment.php
+++ b/modules/comment/models/comment.php
@@ -108,7 +108,7 @@ class Comment_Model extends ORM {
       module::event("comment_created", $this);
     } else {
       // Updated comment
-      $original = ORM::factory("comment")->where("id", "=", $this->id)->find();
+      $original = ORM::factory("comment", $this->id);
       $visible_change = $original->state == "published" || $this->state == "published";
       parent::save();
       module::event("comment_updated", $original, $this);
diff --git a/modules/comment/tests/Comment_Event_Test.php b/modules/comment/tests/Comment_Event_Test.php
index 27272055..08f55b3f 100644
--- a/modules/comment/tests/Comment_Event_Test.php
+++ b/modules/comment/tests/Comment_Event_Test.php
@@ -30,6 +30,6 @@ class Comment_Event_Test extends Gallery_Unit_Test_Case {
 
     $album->delete();
 
-    $this->assert_false(ORM::factory("comment")->where("id", "=", $comment->id)->find()->loaded());
+    $this->assert_false(ORM::factory("comment", $comment->id)->loaded());
   }
 }
diff --git a/modules/g2_import/controllers/g2.php b/modules/g2_import/controllers/g2.php
index 3e002758..5fd4400c 100644
--- a/modules/g2_import/controllers/g2.php
+++ b/modules/g2_import/controllers/g2.php
@@ -50,7 +50,7 @@ class G2_Controller extends Admin_Controller {
       throw new Kohana_404_Exception();
     }
 
-    $item = ORM::factory("item")->where("id", "=", $g2_map->g3_id)->find();
+    $item = ORM::factory("item", $g2_map->g3_id);
     if (!$item->loaded() || !access::can("view", $item)) {
       throw new Kohana_404_Exception();
     }
diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php
index 74164305..fa95e547 100644
--- a/modules/g2_import/helpers/g2_import.php
+++ b/modules/g2_import/helpers/g2_import.php
@@ -358,8 +358,7 @@ class g2_import_Core {
     if ($g2_album->getParentId() == null) {
       return t("Skipping Gallery 2 root album");
     }
-    $parent_album =
-      ORM::factory("item")->where("id", "=", self::map($g2_album->getParentId()))->find();
+    $parent_album = ORM::factory("item", self::map($g2_album->getParentId()));
 
     $album = ORM::factory("item");
     $album->type = "album";
@@ -423,8 +422,8 @@ class g2_import_Core {
       }
       $item_id = self::map($g2_source->getId());
       if ($item_id) {
-        $item = ORM::factory("item")->where("id", "=", $item_id)->find();
-        $g2_album = ORM::factory("item")->where("id", "=", $g3_album_id)->find();
+        $item = ORM::factory("item", $item_id);
+        $g2_album = ORM::factory("item", $g3_album_id);
         $g2_album->album_cover_item_id = $item->id;
         $g2_album->thumb_dirty = 1;
         $g2_album->view_count = g2(GalleryCoreApi::fetchItemViewCount($g2_album_id));
@@ -452,7 +451,7 @@ class g2_import_Core {
                array("id" => $g2_item_id, "exception" => (string)$e));
     }
 
-    $parent = ORM::factory("item")->where("id", "=", self::map($g2_item->getParentId()))->find();
+    $parent = ORM::factory("item", self::map($g2_item->getParentId()));
 
     $g2_type = $g2_item->getEntityType();
     $corrupt = 0;
@@ -633,7 +632,7 @@ class g2_import_Core {
 
     GalleryCoreApi::requireOnce("modules/tags/classes/TagsHelper.class");
     $g2_item_id = array_shift($queue);
-    $g3_item = ORM::factory("item")->where("id", "=", self::map($g2_item_id))->find();
+    $g3_item = ORM::factory("item", self::map($g2_item_id));
     if (!$g3_item->loaded()) {
       return;
     }
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index aa297236..bfab4645 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -228,7 +228,7 @@ class gallery_installer {
             "updated" => $now,
             "weight" => 1))
       ->execute();
-    $root = ORM::factory("item")->where("id", "=", 1)->find();
+    $root = ORM::factory("item", 1);
     access::add_item($root);
 
     module::set_var("gallery", "active_site_theme", "wind");
diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php
index 2236fbbb..d5ca1456 100644
--- a/modules/gallery/helpers/item_rest.php
+++ b/modules/gallery/helpers/item_rest.php
@@ -145,7 +145,7 @@ class item_rest_Core {
   }
 
   static function resolve($id) {
-    $item = ORM::factory("item")->where("id", "=", $id)->find();
+    $item = ORM::factory("item", $id);
     if (!access::can("view", $item)) {
       throw new Kohana_404_Exception();
     }
diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php
index a7bb24ea..83f9b51e 100644
--- a/modules/gallery/libraries/ORM_MPTT.php
+++ b/modules/gallery/libraries/ORM_MPTT.php
@@ -48,7 +48,7 @@ class ORM_MPTT_Core extends ORM {
   function save() {
     if (!$this->loaded()) {
       $this->lock();
-      $parent = ORM::factory("item")->where("id", "=", $this->parent_id)->find();
+      $parent = ORM::factory("item", $this->parent_id);
 
       try {
         // Make a hole in the parent for this new item
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 9706d61f..ae1b6608 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -423,7 +423,7 @@ class Item_Model extends ORM_MPTT {
 
         // If any significant fields have changed, load up a copy of the original item and
         // keep it around.
-        $original = ORM::factory("item")->where("id", "=", $this->id)->find();
+        $original = ORM::factory("item", $this->id);
         if (array_intersect($this->changed, array("parent_id", "name", "slug"))) {
           $original->_build_relative_caches();
           $this->relative_path_cache = null;
@@ -787,7 +787,7 @@ class Item_Model extends ORM_MPTT {
     if ($this->is_movie() || $this->is_photo()) {
       if ($this->loaded()) {
         // Existing items can't change their extension
-        $original = ORM::factory("item")->where("id", "=", $this->id)->find();
+        $original = ORM::factory("item", $this->id);
         $new_ext = pathinfo($this->name, PATHINFO_EXTENSION);
         $old_ext = pathinfo($original->name, PATHINFO_EXTENSION);
         if (strcasecmp($new_ext, $old_ext)) {
diff --git a/modules/server_add/controllers/server_add.php b/modules/server_add/controllers/server_add.php
index 4d6d5dfe..287855b6 100644
--- a/modules/server_add/controllers/server_add.php
+++ b/modules/server_add/controllers/server_add.php
@@ -24,7 +24,7 @@ class Server_Add_Controller extends Admin_Controller {
       $files[] = $path;
     }
 
-    $item = ORM::factory("item")->where("id", "=", $id)->find();
+    $item = ORM::factory("item", $id);
     $view = new View("server_add_tree_dialog.html");
     $view->item = $item;
     $view->tree = new View("server_add_tree.html");
@@ -78,7 +78,7 @@ class Server_Add_Controller extends Admin_Controller {
    */
   public function start() {
     access::verify_csrf();
-    $item = ORM::factory("item")->where("id", "=", Input::instance()->get("item_id"))->find();
+    $item = ORM::factory("item", Input::instance()->get("item_id"));
 
     foreach (Input::instance()->post("paths") as $path) {
       if (server_add::is_valid_path($path)) {
@@ -104,7 +104,7 @@ class Server_Add_Controller extends Admin_Controller {
   function run($task_id) {
     access::verify_csrf();
 
-    $task = ORM::factory("task")->where("id", "=", $task_id)->find();
+    $task = ORM::factory("task", $task_id);
     if (!$task->loaded() || $task->owner_id != identity::active_user()->id) {
       access::forbidden();
     }
@@ -216,12 +216,11 @@ class Server_Add_Controller extends Admin_Controller {
 
         // Look up the parent item for this entry.  By now it should exist, but if none was
         // specified, then this belongs as a child of the current item.
-        $parent_entry =
-          ORM::factory("server_add_file")->where("id", "=", $entry->parent_id)->find();
+        $parent_entry = ORM::factory("server_add_file", $entry->parent_id);
         if (!$parent_entry->loaded()) {
-          $parent = ORM::factory("item")->where("id", "=", $task->get("item_id"))->find();
+          $parent = ORM::factory("item", $task->get("item_id"));
         } else {
-          $parent = ORM::factory("item")->where("id", "=", $parent_entry->item_id)->find();
+          $parent = ORM::factory("item", $parent_entry->item_id);
         }
 
         $name = basename($entry->file);
diff --git a/modules/tag/helpers/item_tags_rest.php b/modules/tag/helpers/item_tags_rest.php
index ce814f77..43e2cef0 100644
--- a/modules/tag/helpers/item_tags_rest.php
+++ b/modules/tag/helpers/item_tags_rest.php
@@ -50,7 +50,7 @@ class item_tags_rest_Core {
   }
 
   static function resolve($id) {
-    $item = ORM::factory("item")->where("id", "=", $id)->find();
+    $item = ORM::factory("item", $id);
     if (!access::can("view", $item)) {
       throw new Kohana_404_Exception();
     }
diff --git a/modules/tag/helpers/tag_item_rest.php b/modules/tag/helpers/tag_item_rest.php
index cd9bb6fe..60d37437 100644
--- a/modules/tag/helpers/tag_item_rest.php
+++ b/modules/tag/helpers/tag_item_rest.php
@@ -35,8 +35,8 @@ class tag_item_rest_Core {
 
   static function resolve($tuple) {
     list ($tag_id, $item_id) = split(",", $tuple);
-    $tag = ORM::factory("tag")->where("id", "=", $tag_id)->find();
-    $item = ORM::factory("item")->where("id", "=", $item_id)->find();
+    $tag = ORM::factory("tag", $tag_id);
+    $item = ORM::factory("item", $item_id);
     if (!$tag->loaded() || !$item->loaded() || !$tag->has($item)) {
       throw new Kohana_404_Exception();
     }
diff --git a/modules/tag/helpers/tag_items_rest.php b/modules/tag/helpers/tag_items_rest.php
index 369a8d83..ef563ac6 100644
--- a/modules/tag/helpers/tag_items_rest.php
+++ b/modules/tag/helpers/tag_items_rest.php
@@ -52,7 +52,7 @@ class tag_items_rest_Core {
   }
 
   static function resolve($id) {
-    return ORM::factory("tag")->where("id", "=", $id)->find();
+    return ORM::factory("tag", $id);
   }
 
   static function url($tag) {
diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php
index 7143daa9..4879cf63 100644
--- a/modules/tag/helpers/tag_rest.php
+++ b/modules/tag/helpers/tag_rest.php
@@ -77,7 +77,7 @@ class tag_rest_Core {
   }
 
   static function resolve($id) {
-    $tag = ORM::factory("tag")->where("id", "=", $id)->find();
+    $tag = ORM::factory("tag", $id);
     if (!$tag->loaded()) {
       throw new Kohana_404_Exception();
     }
diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index 85114ede..851e72e6 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -55,7 +55,7 @@ class Group_Model extends ORM implements Group_Definition {
       module::event("group_created", $this);
     } else {
       // Updated group
-      $original = ORM::factory("group")->where("id", "=", $this->id)->find();
+      $original = ORM::factory("group", $this->id);
       parent::save();
       module::event("group_updated", $original, $this);
     }
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 7c97bae7..78c31047 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -99,7 +99,7 @@ class User_Model extends ORM implements User_Definition {
       module::event("user_created", $this);
     } else {
       // Updated user
-      $original = ORM::factory("user")->where("id", "=", $this->id)->find();
+      $original = ORM::factory("user", $this->id);
       parent::save();
       module::event("user_updated", $original, $this);
     }
-- 
cgit v1.2.3


From c4e360431564627003e4c7864b5dd5a07297e91e Mon Sep 17 00:00:00 2001
From: Tim Almdal <tnalmdal@shaw.ca>
Date: Fri, 29 Jan 2010 14:04:27 -0800
Subject: Strongly type the argument list to the model::validate method.

---
 modules/comment/models/comment.php | 2 +-
 modules/gallery/models/item.php    | 2 +-
 modules/user/models/group.php      | 2 +-
 modules/user/models/user.php       | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

(limited to 'modules/user/models')

diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php
index 8be022b5..add15ce8 100644
--- a/modules/comment/models/comment.php
+++ b/modules/comment/models/comment.php
@@ -56,7 +56,7 @@ class Comment_Model extends ORM {
   /**
    * Add some custom per-instance rules.
    */
-  public function validate($array=null) {
+  public function validate(Validation $array=null) {
     // validate() is recursive, only modify the rules on the outermost call.
     if (!$array) {
       $this->rules = array(
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index ae1b6608..ae6e4cc9 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -720,7 +720,7 @@ class Item_Model extends ORM_MPTT {
   /**
    * Specify our rules here so that we have access to the instance of this model.
    */
-  public function validate($array=null) {
+  public function validate(Validation $array=null) {
     if (!$array) {
       $this->rules = array(
         "album_cover_item_id" => array("callbacks" => array(array($this, "valid_album_cover"))),
diff --git a/modules/user/models/group.php b/modules/user/models/group.php
index 851e72e6..82843ad1 100644
--- a/modules/user/models/group.php
+++ b/modules/user/models/group.php
@@ -37,7 +37,7 @@ class Group_Model extends ORM implements Group_Definition {
   /**
    * Specify our rules here so that we have access to the instance of this model.
    */
-  public function validate($array=null) {
+  public function validate(Validation $array=null) {
     // validate() is recursive, only modify the rules on the outermost call.
     if (!$array) {
       $this->rules = array(
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 78c31047..0cd634ea 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -62,7 +62,7 @@ class User_Model extends ORM implements User_Definition {
   /**
    * Specify our rules here so that we have access to the instance of this model.
    */
-  public function validate($array=null) {
+  public function validate(Validation $array=null) {
     // validate() is recursive, only modify the rules on the outermost call.
     if (!$array) {
       $this->rules = array(
-- 
cgit v1.2.3


From 69897b4c66e22607162ba8a1ae8c95c8f616f03a Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 30 Jan 2010 16:20:44 -0800
Subject: Fix the valid_admin code -- it was considering all non-admins
 invalid. Fixes ticket #997 (highest prime under 1000!)

---
 modules/user/models/user.php | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'modules/user/models')

diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index 0cd634ea..baac9315 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -148,7 +148,8 @@ class User_Model extends ORM implements User_Definition {
    * Validate the admin bit.
    */
   public function valid_admin(Validation $v, $field) {
-    if ($this->id == identity::active_user()->id && !$this->admin) {
+    $active = identity::active_user();
+    if ($this->id == $active->id && $active->admin && !$this->admin) {
       $v->add_error("admin", "locked");
     }
   }
-- 
cgit v1.2.3