Fix some validation checks to check to see if the original was loaded

before deciding whether or not we changed a value.

Change valid_name to be cascading, not parallel.

2 files changed, 19 insertions, 15 deletions

diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index a7f73d0e..58ff86ed 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -782,21 +782,15 @@ class Item_Model extends ORM_MPTT {
     if (strpos($this->name, "/") !== false) {
       $v->add_error("name", "no_slashes");
       return;
-    }
-
-    if (rtrim($this->name, ".") !== $this->name) {
+    } else if (rtrim($this->name, ".") !== $this->name) {
       $v->add_error("name", "no_trailing_period");
-      return;
-    }
-
-    if ($this->is_movie() || $this->is_photo()) {
-      if ($this->loaded()) {
+    } else if ($this->is_movie() || $this->is_photo()) {
+      if ($this->original()->loaded()) {
         // Existing items can't change their extension
         $new_ext = pathinfo($this->name, PATHINFO_EXTENSION);
         $old_ext = pathinfo($this->original()->name, PATHINFO_EXTENSION);
         if (strcasecmp($new_ext, $old_ext)) {
           $v->add_error("name", "illegal_data_file_extension");
-          return;
         }
       } else {
         // New items must have an extension
@@ -804,9 +798,7 @@ class Item_Model extends ORM_MPTT {
           $v->add_error("name", "illegal_data_file_extension");
         }
       }
-    }
-
-    if (db::build()
+    } else if (db::build()
         ->from("items")
         ->where("parent_id", "=", $this->parent_id)
         ->where("name", "=", $this->name)
@@ -908,7 +900,7 @@ class Item_Model extends ORM_MPTT {
    * This field cannot be changed after it's been set.
    */
   public function read_only(Validation $v, $field) {
-    if ($this->loaded() && $this->original()->$field != $this->$field) {
+    if ($this->original()->loaded() && $this->original()->$field != $this->$field) {
       $v->add_error($field, "read_only");
     }
   }
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index afb131fc..284491a0 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -320,9 +320,8 @@ class Item_Model_Test extends Unit_Test_Case {
   }
 
   public function slug_is_url_safe_test() {
-    $album = test::random_album_unsaved();
-
     try {
+      $album = test::random_album_unsaved();
       $album->slug = "illegal chars! !@#@#$!@~";
       $album->save();
       $this->assert_true(false, "Shouldn't be able to save");
@@ -334,4 +333,17 @@ class Item_Model_Test extends Unit_Test_Case {
     $album->slug = "the_quick_brown_fox";
     $album->save();
   }
+
+  public function cant_change_item_type_test() {
+    $photo = test::random_photo();
+    try {
+      $photo->type = "movie";
+      $photo->mime_type = "video/x-flv";
+      $photo->save();
+    } catch (ORM_Validation_Exception $e) {
+      $this->assert_same(array("type" => "read_only"), $e->validation->errors());
+      return;  // pass
+    }
+    $this->assert_true(false, "Shouldn't get here");
+  }
 }