From 94f58e8b65b78cafed8f07f70a48b7b271cfc212 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Fri, 15 Jan 2010 10:48:39 -0800
Subject: Move setting Item_Model::rand_key into Item_Model::save() since it's
 business logic.

---
 modules/gallery/helpers/photo.php | 1 -
 1 file changed, 1 deletion(-)

(limited to 'modules/gallery/helpers/photo.php')

diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 4e20e610..aeae7f56 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -84,7 +84,6 @@ class photo_Core {
     $photo->resize_dirty = 1;
     $photo->sort_column = "weight";
     $photo->slug = $slug;
-    $photo->rand_key = ((float)mt_rand()) / (float)mt_getrandmax();
 
     // Randomize the name or slug if there's a conflict
     // @todo Improve this.  Random numbers are not user friendly
-- 
cgit v1.2.3


From bf085a1a176f32546f86988049e0c3f809842ce7 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 00:51:31 -0800
Subject: Convert photo uploading over to the new model based validation
 approach.

- Rearrange Simple_Uploader_Controller::add_photo() to validate
  the form early in the process, and switch to using model based
  validation.

- Move thumbnail generation into gallery_event::item_created() so
  that it's decoupled from the model.

- Delete photo::create() and move all of its logic into
  Item_Model::save().

- Add Item_Model::$data_file to track the data file associated
  with new movies and photos.

- Do some cleanup on the validation callbacks -- it turns out the
  2nd argument is the field name not the value.
---
 modules/gallery/controllers/simple_uploader.php |  40 ++++---
 modules/gallery/helpers/gallery_event.php       |  16 +++
 modules/gallery/helpers/photo.php               | 112 -------------------
 modules/gallery/models/item.php                 | 139 +++++++++++++++++++-----
 4 files changed, 151 insertions(+), 156 deletions(-)

(limited to 'modules/gallery/helpers/photo.php')

diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php
index 5d32e35f..7a7e7557 100644
--- a/modules/gallery/controllers/simple_uploader.php
+++ b/modules/gallery/controllers/simple_uploader.php
@@ -40,39 +40,45 @@ class Simple_Uploader_Controller extends Controller {
     access::required("add", $album);
     access::verify_csrf();
 
+    // The Flash uploader not call /start directly, so simulate it here for now.
+    if (!batch::in_progress()) {
+      batch::start();
+    }
+
+    $form = $this->_get_add_form($album);
+
+    // Uploadify adds its own field to the form, so validate that separately.
     $file_validation = new Validation($_FILES);
     $file_validation->add_rules(
       "Filedata", "upload::valid",  "upload::required", "upload::type[gif,jpg,jpeg,png,flv,mp4]");
-    if ($file_validation->validate()) {
-      // SimpleUploader.swf does not yet call /start directly, so simulate it here for now.
-      if (!batch::in_progress()) {
-        batch::start();
-      }
 
+    if ($form->validate() && $file_validation->validate()) {
       $temp_filename = upload::save("Filedata");
       try {
-        $name = substr(basename($temp_filename), 10);  // Skip unique identifier Kohana adds
-        $title = item::convert_filename_to_title($name);
+        $item = ORM::factory("item");
+        $item->name = substr(basename($temp_filename), 10);  // Skip unique identifier Kohana adds
+        $item->title = item::convert_filename_to_title($item->name);
+        $item->parent_id = $album->id;
+        $item->set_data_file($temp_filename);
+
         $path_info = @pathinfo($temp_filename);
         if (array_key_exists("extension", $path_info) &&
             in_array(strtolower($path_info["extension"]), array("flv", "mp4"))) {
-          $item = movie::create($album, $temp_filename, $name, $title);
+          $item->type = "movie";
+          $item->save();
           log::success("content", t("Added a movie"),
                        html::anchor("movies/$item->id", t("view movie")));
         } else {
-          $item = photo::create($album, $temp_filename, $name, $title);
+          $item->type = "photo";
+          $item->save();
           log::success("content", t("Added a photo"),
                        html::anchor("photos/$item->id", t("view photo")));
         }
 
-        // We currently have no way of showing errors if validation fails, so only call our event
-        // handlers if validation passes.
-        $form = $this->_get_add_form($album);
-        if ($form->validate()) {
-          module::event("add_photos_form_completed", $item, $form);
-        }
+        module::event("add_photos_form_completed", $item, $form);
       } catch (Exception $e) {
-        Kohana_Log::add("alert", $e->__toString());
+        // The Flash uploader has no good way of reporting complex errors, so just keep it simple.
+        Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString());
         if (file_exists($temp_filename)) {
           unlink($temp_filename);
         }
@@ -84,7 +90,7 @@ class Simple_Uploader_Controller extends Controller {
       print "FILEID: $item->id";
     } else {
       header("HTTP/1.1 400 Bad Request");
-      print "ERROR: " . t("Invalid Upload");
+      print "ERROR: " . t("Invalid upload");
     }
   }
 
diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php
index 679d65c2..9452e855 100644
--- a/modules/gallery/helpers/gallery_event.php
+++ b/modules/gallery/helpers/gallery_event.php
@@ -73,6 +73,22 @@ class gallery_event_Core {
 
   static function item_created($item) {
     access::add_item($item);
+
+    if ($item->is_photo() || $item->is_movie()) {
+      // Build our thumbnail/resizes.
+      try {
+        graphics::generate($item);
+      } catch (Exception $e) {
+        log::failure("Unable to create a thumbnail for item id {$item->id}");
+        Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString());
+      }
+
+      // If the parent has no cover item, make this it.
+      $parent = $item->parent();
+      if (access::can("edit", $parent) && $parent->album_cover_item_id == null)  {
+        item::make_album_cover($item);
+      }
+    }
   }
 
   static function item_deleted($item) {
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index aeae7f56..74e30409 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -24,118 +24,6 @@
  * Note: by design, this class does not do any permission checking.
  */
 class photo_Core {
-  /**
-   * Create a new photo.
-   * @param integer $parent parent album
-   * @param string  $filename path to the photo file on disk
-   * @param string  $name the filename to use for this photo in the album
-   * @param integer $title the title of the new photo
-   * @param string  $description (optional) the longer description of this photo
-   * @param string  $slug (optional) the url component for this photo
-   * @return Item_Model
-   */
-  static function create($parent, $filename, $name, $title,
-                         $description=null, $owner_id=null, $slug=null) {
-    if (!$parent->loaded() || !$parent->is_album()) {
-      throw new Exception("@todo INVALID_PARENT");
-    }
-
-    if (!is_file($filename)) {
-      throw new Exception("@todo MISSING_IMAGE_FILE");
-    }
-
-    if (strpos($name, "/")) {
-      throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH");
-    }
-
-    // We don't allow trailing periods as a security measure
-    // ref: http://dev.kohanaphp.com/issues/684
-    if (rtrim($name, ".") != $name) {
-      throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD");
-    }
-
-    if (filesize($filename) == 0) {
-      throw new Exception("@todo EMPTY_INPUT_FILE");
-    }
-
-    $image_info = getimagesize($filename);
-
-    // Force an extension onto the name
-    $pi = pathinfo($filename);
-    if (empty($pi["extension"])) {
-      $pi["extension"] = image_type_to_extension($image_info[2], false);
-      $name .= "." . $pi["extension"];
-    }
-
-    if (empty($slug)) {
-      $slug = item::convert_filename_to_slug($name);
-    }
-
-    $photo = ORM::factory("item");
-    $photo->type = "photo";
-    $photo->title = $title;
-    $photo->description = $description;
-    $photo->name = $name;
-    $photo->owner_id = $owner_id ? $owner_id : identity::active_user()->id;
-    $photo->width = $image_info[0];
-    $photo->height = $image_info[1];
-    $photo->mime_type = empty($image_info['mime']) ? "application/unknown" : $image_info['mime'];
-    $photo->thumb_dirty = 1;
-    $photo->resize_dirty = 1;
-    $photo->sort_column = "weight";
-    $photo->slug = $slug;
-
-    // Randomize the name or slug if there's a conflict
-    // @todo Improve this.  Random numbers are not user friendly
-    while (ORM::factory("item")
-           ->where("parent_id", "=", $parent->id)
-           ->and_open()
-           ->where("name", "=", $photo->name)
-           ->or_where("slug", "=", $photo->slug)
-           ->close()
-           ->find()->id) {
-      $rand = rand();
-      $photo->name = "{$name}.$rand.{$pi['extension']}";
-      $photo->slug = "{$slug}-$rand";
-    }
-
-    // This saves the photo
-    $photo->add_to_parent($parent);
-
-    /*
-     * If the thumb or resize already exists then rename it. We need to do this after the save
-     * because the resize_path and thumb_path both call relative_path which caches the
-     * path. Before add_to_parent the relative path will be incorrect.
-     */
-    if (file_exists($photo->resize_path()) ||
-        file_exists($photo->thumb_path())) {
-      $photo->name = $pi["filename"] . "-" . rand() . "." . $pi["extension"];
-      $photo->save();
-    }
-
-    copy($filename, $photo->file_path());
-
-    // @todo: publish this from inside Item_Model::save() when we refactor to the point where
-    // there's only one save() happening here.
-    module::event("item_created", $photo);
-
-    // Build our thumbnail/resizes.  If we fail to build thumbnail/resize we assume that the image
-    // is bad in some way and discard it.
-    try {
-      graphics::generate($photo);
-    } catch (Exception $e) {
-      $photo->delete();
-      throw $e;
-    }
-
-    // If the parent has no cover item, make this it.
-    if (access::can("edit", $parent) && $parent->album_cover_item_id == null)  {
-      item::make_album_cover($photo);
-    }
-
-    return $photo;
-  }
-
   static function get_edit_form($photo) {
     $form = new Forge("photos/update/$photo->id", "", "post", array("id" => "g-edit-photo-form"));
     $form->hidden("from_id");
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 46b0304e..977b9771 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -20,13 +20,13 @@
 class Item_Model extends ORM_MPTT {
   protected $children = 'items';
   protected $sorting = array();
+  protected $data_file = null;
 
   var $rules = array(
     "name"        => array("rules" => array("length[0,255]", "required")),
     "title"       => array("rules" => array("length[0,255]", "required")),
     "slug"        => array("rules" => array("length[0,255]", "required")),
     "description" => array("rules" => array("length[0,65535]")),
-    "parent_id"   => array("rules" => array("Item_Model::valid_parent")),
     "type"        => array("rules" => array("Item_Model::valid_type")),
   );
 
@@ -174,6 +174,14 @@ class Item_Model extends ORM_MPTT {
     return $this;
   }
 
+  /**
+   * Specify the path to the data file associated with this item.  To actually associate it,
+   * you still have to call save().
+   */
+  public function set_data_file($data_file) {
+    $this->data_file = $data_file;
+  }
+
   /**
    * Return the server-relative url to this item, eg:
    *   /gallery3/index.php/BobsWedding?page=2
@@ -304,7 +312,7 @@ class Item_Model extends ORM_MPTT {
     }
     $this->relative_path_cache = implode($names, "/");
     $this->relative_url_cache = implode($slugs, "/");
-    $this->save();
+    return $this;
   }
 
   /**
@@ -319,7 +327,7 @@ class Item_Model extends ORM_MPTT {
     }
 
     if (!isset($this->relative_path_cache)) {
-      $this->_build_relative_caches();
+      $this->_build_relative_caches()->save();
     }
     return $this->relative_path_cache;
   }
@@ -334,7 +342,7 @@ class Item_Model extends ORM_MPTT {
     }
 
     if (!isset($this->relative_url_cache)) {
-      $this->_build_relative_caches();
+      $this->_build_relative_caches()->save();
     }
     return $this->relative_url_cache;
   }
@@ -368,6 +376,7 @@ class Item_Model extends ORM_MPTT {
     unset($significant_changes["relative_url_cache"]);
     unset($significant_changes["relative_path_cache"]);
 
+
     if (!empty($this->changed) && $significant_changes) {
       $this->updated = time();
       if (!$this->loaded()) {
@@ -386,15 +395,37 @@ class Item_Model extends ORM_MPTT {
         if (empty($this->owner_id)) {
           $this->owner_id = identity::active_user()->id;
         }
+
+        // Make an url friendly slug from the name, if necessary
         if (empty($this->slug)) {
           $tmp = pathinfo($this->name, PATHINFO_FILENAME);
           $tmp = preg_replace("/[^A-Za-z0-9-_]+/", "-", $tmp);
           $this->slug = trim($tmp, "-");
         }
 
-        // Randomize the name or slug if there's a conflict
+        if ($this->is_movie() || $this->is_photo()) {
+          $image_info = getimagesize($this->data_file);
+
+          if ($this->is_photo()) {
+            $this->width = $image_info[0];
+            $this->height = $image_info[1];
+            $this->mime_type =
+              empty($image_info['mime']) ? "application/unknown" : $image_info['mime'];
+          }
+
+          // Force an extension onto the name if necessary
+          $pi = pathinfo($this->data_file);
+          if (empty($pi["extension"])) {
+            $pi["extension"] = image_type_to_extension($image_info[2], false);
+            $this->name .= "." . $pi["extension"];
+          }
+
+        }
+
+        // Randomize the name or slug if there's a conflict.  Preserve the extension.
         // @todo Improve this.  Random numbers are not user friendly
-        $base_name = $this->name;
+        $base_name = pathinfo($this->name, PATHINFO_FILENAME);
+        $base_ext = pathinfo($this->name, PATHINFO_EXTENSION);
         $base_slug = $this->slug;
         while (ORM::factory("item")
                ->where("parent_id", "=", $this->parent_id)
@@ -404,19 +435,46 @@ class Item_Model extends ORM_MPTT {
                ->close()
                ->find()->id) {
           $rand = rand();
-          $this->name = "$base_name-$rand";
+          if ($base_ext) {
+            $this->name = "$base_name-$rand.$base_ext";
+          } else {
+            $this->name = "$base_name-$rand";
+          }
           $this->slug = "$base_slug-$rand";
         }
 
         parent::save();
 
-        // Call this after we finish saving so that the paths are correct.
-        if ($this->is_album()) {
+        // Build our url caches and save again.  If we could depend on a save happening later we
+        // could defer this 2nd save.
+        $this->_build_relative_caches();
+        parent::save();
+
+        // Take any actions that we can only do once all our paths are set correctly after saving.
+        switch ($this->type) {
+        case "album":
           mkdir($this->file_path());
           mkdir(dirname($this->thumb_path()));
           mkdir(dirname($this->resize_path()));
+          break;
+
+        case "photo":
+          // The thumb or resize may already exist in the case where a movie and a photo generate
+          // a thumbnail of the same name (eg, foo.flv movie and foo.jpg photo will generate
+          // foo.jpg thumbnail).  If that happens, randomize and save again.
+          if (file_exists($this->resize_path()) ||
+              file_exists($this->thumb_path())) {
+            $pi = pathinfo($this->name);
+            $this->name = $pi["filename"] . "-" . rand() . "." . $pi["extension"];
+            parent::save();
+          }
+
+          copy($this->data_file, $this->file_path());
+          break;
         }
 
+        // This will almost definitely trigger another save, so put it at the end so that we're
+        // tail recursive.
         module::event("item_created", $this);
       } else {
         // Update an existing item
@@ -691,8 +749,8 @@ class Item_Model extends ORM_MPTT {
     if (!$array) {
       // The root item has different rules for the name and slug.
       if ($this->id == 1) {
-        $this->rules["name"]["rules"][] = "length[0]";
-        $this->rules["slug"]["rules"][] = "length[0]";
+        $this->rules["name"] = array("rules" => array("length[0]"));
+        $this->rules["slug"] = array("rules" => array("length[0]"));
       }
 
       // Names and slugs can't conflict
@@ -700,20 +758,28 @@ class Item_Model extends ORM_MPTT {
       $this->rules["slug"]["callbacks"][] = array($this, "valid_slug");
     }
 
+    // Movies and photos must have data files
+    if ($this->is_photo() || $this->is_movie() && !$this->loaded()) {
+      $this->rules["name"]["callbacks"][] = array($this, "valid_data_file");
+    }
+
+    // All items must have a legal parent
+    $this->rules["parent_id"]["callbacks"][] = array($this, "valid_parent");
+
     parent::validate($array);
   }
 
   /**
    * Validate that the desired slug does not conflict.
    */
-  public function valid_slug(Validation $v, $value) {
-    if (preg_match("/[^A-Za-z0-9-_]/", $value)) {
+  public function valid_slug(Validation $v, $field) {
+    if (preg_match("/[^A-Za-z0-9-_]/", $this->slug)) {
       $v->add_error("slug", "not_url_safe");
     } else if (db::build()
         ->from("items")
         ->where("parent_id", "=", $this->parent_id)
         ->where("id", "<>", $this->id)
-        ->where("slug", "=", $value)
+        ->where("slug", "=", $this->slug)
         ->count_records()) {
       $v->add_error("slug", "conflict");
     }
@@ -723,36 +789,55 @@ class Item_Model extends ORM_MPTT {
    * Validate the item name.  It can't conflict with other names, can't contain slashes or
    * trailing periods.
    */
-  public function valid_name(Validation $v, $value) {
-    if (strpos($value, "/") !== false) {
+  public function valid_name(Validation $v, $field) {
+    if (strpos($this->name, "/") !== false) {
       $v->add_error("name", "no_slashes");
-    } else if (rtrim($value, ".") !== $value) {
+    } else if (rtrim($this->name, ".") !== $this->name) {
       $v->add_error("name", "no_trailing_period");
     } else if (db::build()
                ->from("items")
                ->where("parent_id", "=", $this->parent_id)
                ->where("id", "<>", $this->id)
-               ->where("name", "=", $value)
+               ->where("name", "=", $this->name)
                ->count_records()) {
       $v->add_error("name", "conflict");
     }
   }
 
   /**
-   * Make sure that the type is valid.
+   * Make sure that the data file is well formed (it exists and isn't empty).
    */
-  static function valid_type($value) {
-    return in_array($value, array("album", "photo", "movie"));
+  public function valid_data_file(Validation $v, $field) {
+    if (!is_file($this->data_file)) {
+      $v->add_error("file", "bad_path");
+    } else if (filesize($this->data_file) == 0) {
+      $v->add_error("file", "empty_file");
+    }
   }
 
   /**
    * Make sure that the parent id refers to an album.
    */
-  static function valid_parent($value) {
-    return db::build()
-      ->from("items")
-      ->where("id", "=", $value)
-      ->where("type", "=", "album")
-      ->count_records() == 1;
+  public function valid_parent(Validation $v, $field) {
+    if ($this->id == 1) {
+      if ($this->parent_id != 0) {
+        $v->add_error("parent_id", "invalid");
+      }
+    } else {
+      if (db::build()
+          ->from("items")
+          ->where("id", "=", $this->parent_id)
+          ->where("type", "=", "album")
+          ->count_records() != 1) {
+        $v->add_error("parent_id", "invalid");
+      }
+    }
+  }
+
+  /**
+   * Make sure that the type is valid.
+   */
+  static function valid_type($value) {
+    return in_array($value, array("album", "photo", "movie"));
   }
 }
-- 
cgit v1.2.3


From 5a8449f16d3c0db8fb47acf515d319d6eb9e87f4 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Sat, 16 Jan 2010 11:12:27 -0800
Subject: Convert Photos_Controller::update() to use model based validation.

---
 modules/gallery/controllers/photos.php | 55 ++++++++++------------------------
 modules/gallery/helpers/photo.php      | 10 ++-----
 2 files changed, 17 insertions(+), 48 deletions(-)

(limited to 'modules/gallery/helpers/photo.php')

diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php
index 56b454ce..98f2126d 100644
--- a/modules/gallery/controllers/photos.php
+++ b/modules/gallery/controllers/photos.php
@@ -61,48 +61,25 @@ class Photos_Controller extends Items_Controller {
     access::required("edit", $photo);
 
     $form = photo::get_edit_form($photo);
-    $valid = $form->validate();
-
-    if ($valid) {
-      $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION);
-      $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION);
-      if (strcasecmp($new_ext, $old_ext)) {
-        $form->edit_item->filename->add_error("illegal_extension", 1);
-        $valid = false;
-      }
-    }
-
-    if ($valid) {
-      if ($form->edit_item->filename->value != $photo->name ||
-          $form->edit_item->slug->value != $photo->slug) {
-        // Make sure that there's not a name or slug conflict
-        if ($row = db::build()
-            ->select(array("name", "slug"))
-            ->from("items")
-            ->where("parent_id", "=", $photo->parent_id)
-            ->where("id", "<>", $photo->id)
-            ->and_open()
-            ->where("name", "=", $form->edit_item->filename->value)
-            ->or_where("slug", "=", $form->edit_item->slug->value)
-            ->close()
-            ->execute()
-            ->current()) {
-          if ($row->name == $form->edit_item->filename->value) {
-            $form->edit_item->filename->add_error("name_conflict", 1);
-          }
-          if ($row->slug == $form->edit_item->slug->value) {
-            $form->edit_item->slug->add_error("slug_conflict", 1);
-          }
-          $valid = false;
+    try {
+      $valid = $form->validate();
+      $photo->title = $form->edit_item->title->value;
+      $photo->description = $form->edit_item->description->value;
+      $photo->slug = $form->edit_item->slug->value;
+      $photo->name = $form->edit_item->filename->value;
+      $photo->validate();
+    } catch (ORM_Validation_Exception $e) {
+      // Translate ORM validation errors into form error messages
+      foreach ($e->validation->errors() as $key => $error) {
+        if ($key == "name") {
+          $key = "filename";
         }
+        $form->edit_item->inputs[$key]->add_error($error, 1);
       }
+      $valid = false;
     }
 
     if ($valid) {
-      $photo->title = $form->edit_item->title->value;
-      $photo->description = $form->edit_item->description->value;
-      $photo->slug = $form->edit_item->slug->value;
-      $photo->rename($form->edit_item->filename->value);
       $photo->save();
       module::event("item_edit_form_completed", $photo, $form);
 
@@ -118,9 +95,7 @@ class Photos_Controller extends Items_Controller {
         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));
     }
   }
 
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 74e30409..cb94772e 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -31,18 +31,13 @@ class photo_Core {
     $group->input("title")->label(t("Title"))->value($photo->title);
     $group->textarea("description")->label(t("Description"))->value($photo->description);
     $group->input("filename")->label(t("Filename"))->value($photo->name)
-      ->rules("required")
-      ->error_messages(
-        "name_conflict", t("There is already a movie, photo or album with this name"))
-      ->callback("item::validate_no_slashes")
+      ->error_messages("conflict", t("There is already a movie, photo or album with this name"))
       ->error_messages("no_slashes", t("The photo name can't contain a \"/\""))
-      ->callback("item::validate_no_trailing_period")
       ->error_messages("no_trailing_period", t("The photo name can't end in \".\""))
       ->error_messages("illegal_extension", t("You cannot change the filename extension"));
     $group->input("slug")->label(t("Internet Address"))->value($photo->slug)
-      ->callback("item::validate_url_safe")
       ->error_messages(
-        "slug_conflict", t("There is already a movie, photo or album with this internet address"))
+        "conflict", t("There is already a movie, photo or album with this internet address"))
       ->error_messages(
         "not_url_safe",
         t("The internet address should contain only letters, numbers, hyphens and underscores"));
@@ -51,7 +46,6 @@ class photo_Core {
 
     $group = $form->group("buttons")->label("");
     $group->submit("")->value(t("Modify"));
-    $form->add_rules_from(ORM::factory("item"));
     return $form;
   }
 
-- 
cgit v1.2.3


From e02675b730fb814105e1cb9dceb0e25fdcbd3e27 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 19 Jan 2010 19:31:01 -0800
Subject: Change "filename" to "name" in the edit album form.  I'd rather have
 consistency between field names than deal with underlying issues with Forge
 bitching about the "name" property.

---
 modules/gallery/controllers/movies.php           | 5 +----
 modules/gallery/controllers/photos.php           | 5 +----
 modules/gallery/helpers/movie.php                | 2 +-
 modules/gallery/helpers/photo.php                | 2 +-
 modules/gallery/tests/Photos_Controller_Test.php | 4 ++--
 5 files changed, 6 insertions(+), 12 deletions(-)

(limited to 'modules/gallery/helpers/photo.php')

diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php
index 0908e281..b51282b3 100644
--- a/modules/gallery/controllers/movies.php
+++ b/modules/gallery/controllers/movies.php
@@ -66,14 +66,11 @@ class Movies_Controller extends Items_Controller {
       $movie->title = $form->edit_item->title->value;
       $movie->description = $form->edit_item->description->value;
       $movie->slug = $form->edit_item->slug->value;
-      $movie->name = $form->edit_item->filename->value;
+      $movie->name = $form->edit_item->inputs["name"]->value;
       $movie->validate();
     } catch (ORM_Validation_Exception $e) {
       // Translate ORM validation errors into form error messages
       foreach ($e->validation->errors() as $key => $error) {
-        if ($key == "name") {
-          $key = "filename";
-        }
         $form->edit_item->inputs[$key]->add_error($error, 1);
       }
       $valid = false;
diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php
index 98f2126d..b5da3884 100644
--- a/modules/gallery/controllers/photos.php
+++ b/modules/gallery/controllers/photos.php
@@ -66,14 +66,11 @@ class Photos_Controller extends Items_Controller {
       $photo->title = $form->edit_item->title->value;
       $photo->description = $form->edit_item->description->value;
       $photo->slug = $form->edit_item->slug->value;
-      $photo->name = $form->edit_item->filename->value;
+      $photo->name = $form->edit_item->inputs["name"]->value;
       $photo->validate();
     } catch (ORM_Validation_Exception $e) {
       // Translate ORM validation errors into form error messages
       foreach ($e->validation->errors() as $key => $error) {
-        if ($key == "name") {
-          $key = "filename";
-        }
         $form->edit_item->inputs[$key]->add_error($error, 1);
       }
       $valid = false;
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index b2e846d3..b07a9e69 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -30,7 +30,7 @@ class movie_Core {
     $group = $form->group("edit_item")->label(t("Edit Movie"));
     $group->input("title")->label(t("Title"))->value($movie->title);
     $group->textarea("description")->label(t("Description"))->value($movie->description);
-    $group->input("filename")->label(t("Filename"))->value($movie->name)
+    $group->input("name")->label(t("Filename"))->value($movie->name)
       ->error_messages(
         "conflict", t("There is already a movie, photo or album with this name"))
       ->error_messages("no_slashes", t("The movie name can't contain a \"/\""))
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index cb94772e..9bd277bc 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -30,7 +30,7 @@ class photo_Core {
     $group = $form->group("edit_item")->label(t("Edit Photo"));
     $group->input("title")->label(t("Title"))->value($photo->title);
     $group->textarea("description")->label(t("Description"))->value($photo->description);
-    $group->input("filename")->label(t("Filename"))->value($photo->name)
+    $group->input("name")->label(t("Filename"))->value($photo->name)
       ->error_messages("conflict", t("There is already a movie, photo or album with this name"))
       ->error_messages("no_slashes", t("The photo name can't contain a \"/\""))
       ->error_messages("no_trailing_period", t("The photo name can't end in \".\""))
diff --git a/modules/gallery/tests/Photos_Controller_Test.php b/modules/gallery/tests/Photos_Controller_Test.php
index 31e0bc21..f548b40d 100644
--- a/modules/gallery/tests/Photos_Controller_Test.php
+++ b/modules/gallery/tests/Photos_Controller_Test.php
@@ -31,7 +31,7 @@ class Photos_Controller_Test extends Unit_Test_Case {
     $controller = new Photos_Controller();
     $photo = test::random_photo();
 
-    $_POST["filename"] = "new name.jpg";
+    $_POST["name"] = "new name.jpg";
     $_POST["title"] = "new title";
     $_POST["description"] = "new description";
     $_POST["slug"] = "new-slug";
@@ -55,7 +55,7 @@ class Photos_Controller_Test extends Unit_Test_Case {
     $controller = new Photos_Controller();
     $photo = test::random_photo();
 
-    $_POST["filename"] = "new name.jpg";
+    $_POST["name"] = "new name.jpg";
     $_POST["title"] = "new title";
     $_POST["description"] = "new description";
     $_POST["slug"] = "new slug";
-- 
cgit v1.2.3


From e5b25983a67a53ad209aa4b8c251afa3276853fa Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 27 Jan 2010 23:00:29 -0800
Subject: Localize all error messages.

---
 modules/gallery/helpers/photo.php | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

(limited to 'modules/gallery/helpers/photo.php')

diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 9bd277bc..bbdf2e3b 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -28,19 +28,25 @@ class photo_Core {
     $form = new Forge("photos/update/$photo->id", "", "post", array("id" => "g-edit-photo-form"));
     $form->hidden("from_id");
     $group = $form->group("edit_item")->label(t("Edit Photo"));
-    $group->input("title")->label(t("Title"))->value($photo->title);
+    $group->input("title")->label(t("Title"))->value($photo->title)
+      ->error_messages("required", t("You must provide a title"))
+      ->error_messages("length", t("Your title is too long"));
     $group->textarea("description")->label(t("Description"))->value($photo->description);
     $group->input("name")->label(t("Filename"))->value($photo->name)
       ->error_messages("conflict", t("There is already a movie, photo or album with this name"))
       ->error_messages("no_slashes", t("The photo name can't contain a \"/\""))
       ->error_messages("no_trailing_period", t("The photo name can't end in \".\""))
-      ->error_messages("illegal_extension", t("You cannot change the filename extension"));
+      ->error_messages("illegal_data_file_extension", t("You cannot change the photo file extension"))
+      ->error_messages("required", t("You must provide a photo file name"))
+      ->error_messages("length", t("Your photo file name is too long"));
     $group->input("slug")->label(t("Internet Address"))->value($photo->slug)
       ->error_messages(
         "conflict", t("There is already a movie, photo or album with this internet address"))
       ->error_messages(
         "not_url_safe",
-        t("The internet address should contain only letters, numbers, hyphens and underscores"));
+        t("The internet address should contain only letters, numbers, hyphens and underscores"))
+      ->error_messages("required", t("You must provide an internet address"))
+      ->error_messages("length", t("Your internet address is too long"));
 
     module::event("item_edit_form", $photo, $form);
 
-- 
cgit v1.2.3