From cd907c2b42f8b50ebe6d490aab42365e16deb258 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Fri, 17 Jul 2009 12:51:27 -0700
Subject: Change model_cache::clear() API to clear everything.  This prevents
 old ORM relationships from hanging around, which was causing problems when
 doing MPTT manipulations (resulting in incorrect permission propagation--
 very bad!)

---
 modules/gallery/helpers/item.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index f40b5c97..80c25862 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -53,7 +53,7 @@ class item_Core {
     access::required("view", $parent);
     access::required("edit", $parent);
 
-    model_cache::clear("item", $parent->album_cover_item_id);
+    model_cache::clear();
     $parent->album_cover_item_id = $item->is_album() ? $item->album_cover_item_id : $item->id;
     $parent->thumb_dirty = 1;
     $parent->save();
@@ -69,7 +69,7 @@ class item_Core {
     access::required("edit", $album);
     @unlink($album->thumb_path());
 
-    model_cache::clear("item", $album->album_cover_item_id)  ;
+    model_cache::clear();
     $album->album_cover_item_id = null;
     $album->thumb_width = 0;
     $album->thumb_height = 0;
-- 
cgit v1.2.3


From 24d7f5df8c8a18b9ac017fb7b4fc92974b5435cc Mon Sep 17 00:00:00 2001
From: Tim Almdal <tnalmdal@shaw.ca>
Date: Mon, 17 Aug 2009 23:47:39 +0800
Subject: Refactor the get maximum weight functionality into a method in the
 item helper, so that we can use it else where (i.e. the new organize module)

Signed-off-by: Tim Almdal <tnalmdal@shaw.ca>
---
 modules/gallery/helpers/item.php | 14 ++++++++++++++
 modules/gallery/models/item.php  |  9 +--------
 2 files changed, 15 insertions(+), 8 deletions(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 80c25862..5504dc95 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -137,4 +137,18 @@ class item_Core {
     $group->submit("")->value(t("Delete"));
     return $form;
   }
+
+  /**
+   * Get the next weight value
+   */
+  static function get_max_weight() {
+    // Guard against an empty result when we create the first item.  It's unfortunate that we
+    // have to check this every time.
+    // @todo: figure out a better way to bootstrap the weight.
+    $result = Database::instance()
+      ->select("weight")->from("items")
+      ->orderby("weight", "desc")->limit(1)
+      ->get()->current();
+    return ($result ? $result->weight : 0) + 1;
+  }
 }
\ No newline at end of file
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index c4b9826f..7a3a2ba7 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -351,14 +351,7 @@ class Item_Model extends ORM_MPTT {
       $this->updated = time();
       if (!$this->loaded) {
         $this->created = $this->updated;
-        // Guard against an empty result when we create the first item.  It's unfortunate that we
-        // have to check this every time.
-        // @todo: figure out a better way to bootstrap the weight.
-        $result = Database::instance()
-          ->select("weight")->from("items")
-          ->orderby("weight", "desc")->limit(1)
-          ->get()->current();
-        $this->weight = ($result ? $result->weight : 0) + 1;
+        $this->weight = item::get_max_weight();
       } else {
         $send_event = 1;
       }
-- 
cgit v1.2.3


From e3f5a8be903dd54a72b9c7694a21d63655999127 Mon Sep 17 00:00:00 2001
From: Tim Almdal <tnalmdal@shaw.ca>
Date: Tue, 18 Aug 2009 23:40:52 +0800
Subject: Change the setting of page type from "item" to "photo".  Thre is no
 page type of item.  All other references use photo for non album and non
 dynamic pages.

Signed-off-by: Tim Almdal <tnalmdal@shaw.ca>
---
 modules/gallery/helpers/item.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 5504dc95..a2d3859f 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -129,7 +129,7 @@ class item_Core {
     if (Input::instance()->get("page_type") == "album") {
       $page_type = "album";
     } else {
-      $page_type = "item";
+      $page_type = "photo";
     }
     $form = new Forge("quick/delete/$item->id?page_type=$page_type", "", "post", array("id" => "gConfirmDelete"));
     $form->hidden("_method")->value("put");
-- 
cgit v1.2.3


From 38b2efc44cf3345d97798e9637db241b05e2dded Mon Sep 17 00:00:00 2001
From: Tim Almdal <tnalmdal@shaw.ca>
Date: Sat, 29 Aug 2009 11:43:10 -0700
Subject: Fix for 641... extend viewable functionality to comments. Viewable
 unit test is not working.

---
 modules/comment/helpers/comment_rss.php    | 55 +++++++++----------
 modules/comment/models/comment.php         | 10 ++++
 modules/gallery/helpers/item.php           | 37 +++++++++++++
 modules/gallery/models/item.php            | 34 +-----------
 modules/gallery/tests/Item_Helper_Test.php | 84 ++++++++++++++++++++++++++++++
 5 files changed, 157 insertions(+), 63 deletions(-)
 create mode 100644 modules/gallery/tests/Item_Helper_Test.php

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

diff --git a/modules/comment/helpers/comment_rss.php b/modules/comment/helpers/comment_rss.php
index ab3d2283..a8171ce7 100644
--- a/modules/comment/helpers/comment_rss.php
+++ b/modules/comment/helpers/comment_rss.php
@@ -33,42 +33,37 @@ class comment_rss_Core {
       return;
     }
 
-    $comments = ORM::factory("comment")
-      ->where("state", "published")
-      ->orderby("created", "DESC");
-    $all_comments = ORM::factory("comment")
+    $comment_model = ORM::factory("comment")
+      ->viewable()
       ->where("state", "published")
       ->orderby("created", "DESC");
 
     if ($feed_id == "item") {
-      $comments->where("item_id", $id);
-      $all_comments->where("item_id", $id);
+      $comment_model->where("item_id", $id);
     }
 
-    if (!empty($comments)) {
-      $feed->view = "comment.mrss";
-      $comments = $comments->find_all($limit, $offset);
-      $feed->children = array();
-      foreach ($comments as $comment) {
-        $item = $comment->item();
-        $feed->children[] = new ArrayObject(
-          array("pub_date" => date("D, d M Y H:i:s T", $comment->created),
-                "text" => nl2br(p::purify($comment->text)),
-                "thumb_url" => $item->thumb_url(),
-                "thumb_height" => $item->thumb_height,
-                "thumb_width" => $item->thumb_width,
-                "item_uri" => url::abs_site("{$item->type}s/$item->id"),
-                "title" => p::purify($item->title),
-                "author" => p::clean($comment->author_name())),
-          ArrayObject::ARRAY_AS_PROPS);
-      }
+    $comments = $comment_model->find_all($limit, $offset);
+    $feed->view = "comment.mrss";
+    $feed->children = array();
+    foreach ($comments as $comment) {
+      $item = $comment->item();
+      $feed->children[] = new ArrayObject(
+        array("pub_date" => date("D, d M Y H:i:s T", $comment->created),
+              "text" => nl2br(p::purify($comment->text)),
+              "thumb_url" => $item->thumb_url(),
+              "thumb_height" => $item->thumb_height,
+              "thumb_width" => $item->thumb_width,
+              "item_uri" => url::abs_site("{$item->type}s/$item->id"),
+              "title" => p::purify($item->title),
+              "author" => p::clean($comment->author_name())),
+        ArrayObject::ARRAY_AS_PROPS);
+    }
 
-      $feed->max_pages = ceil($all_comments->find_all()->count() / $limit);
-      $feed->title = htmlspecialchars(t("Recent Comments"));
-      $feed->uri = url::abs_site("albums/" . (empty($id) ? "1" : $id));
-      $feed->description = t("Recent Comments");
+    $feed->max_pages = ceil($comment_model->count_all() / $limit);
+    $feed->title = htmlspecialchars(t("Recent Comments"));
+    $feed->uri = url::abs_site("albums/" . (empty($id) ? "1" : $id));
+    $feed->description = t("Recent Comments");
 
-      return $feed;
-    }
+    return $feed;
   }
-}
\ No newline at end of file
+}
diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php
index 83d0888a..de9b0cd6 100644
--- a/modules/comment/models/comment.php
+++ b/modules/comment/models/comment.php
@@ -80,4 +80,14 @@ class Comment_Model extends ORM {
 
     return $this;
   }
+
+  /**
+   * Add a set of restrictions to any following queries to restrict access only to items
+   * viewable by the active user.
+   * @chainable
+   */
+  public function viewable() {
+    $this->join("items", "items.id", "comments.item_id");
+    return item::viewable($this);
+  }
 }
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index a2d3859f..8839861f 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -151,4 +151,41 @@ class item_Core {
       ->get()->current();
     return ($result ? $result->weight : 0) + 1;
   }
+
+  /**
+   * Add a set of restrictions to any following queries to restrict access only to items
+   * viewable by the active user.
+   * @chainable
+   */
+  static function viewable($model) {
+    $view_restrictions = array();
+    if (!user::active()->admin) {
+      foreach (user::group_ids() as $id) {
+        // Separate the first restriction from the rest to make it easier for us to formulate
+        // our where clause below
+        if (empty($view_restrictions)) {
+          $view_restrictions[0] = "items.view_$id";
+        } else {
+          $view_restrictions[1]["items.view_$id"] = access::ALLOW;
+        }
+      }
+    }
+    switch (count($view_restrictions)) {
+    case 0:
+      break;
+
+    case 1:
+      $model->where($view_restrictions[0], access::ALLOW);
+      break;
+
+    default:
+      $model->open_paren();
+      $model->where($view_restrictions[0], access::ALLOW);
+      $model->orwhere($view_restrictions[1]);
+      $model->close_paren();
+      break;
+    }
+
+    return $model;
+  }
 }
\ No newline at end of file
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 7a3a2ba7..68e89db6 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -19,7 +19,6 @@
  */
 class Item_Model extends ORM_MPTT {
   protected $children = 'items';
-  private $view_restrictions = null;
   protected $sorting = array();
 
   var $rules = array(
@@ -34,38 +33,7 @@ class Item_Model extends ORM_MPTT {
    * @chainable
    */
   public function viewable() {
-    if (is_null($this->view_restrictions)) {
-      if (user::active()->admin) {
-        $this->view_restrictions = array();
-      } else {
-        foreach (user::group_ids() as $id) {
-          // Separate the first restriction from the rest to make it easier for us to formulate
-          // our where clause below
-          if (empty($this->view_restrictions)) {
-            $this->view_restrictions[0] = "view_$id";
-          } else {
-            $this->view_restrictions[1]["view_$id"] = access::ALLOW;
-          }
-        }
-      }
-    }
-    switch (count($this->view_restrictions)) {
-    case 0:
-      break;
-
-    case 1:
-      $this->where($this->view_restrictions[0], access::ALLOW);
-      break;
-
-    default:
-      $this->open_paren();
-      $this->where($this->view_restrictions[0], access::ALLOW);
-      $this->orwhere($this->view_restrictions[1]);
-      $this->close_paren();
-      break;
-    }
-
-    return $this;
+    return item::viewable($this);
   }
 
   /**
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
new file mode 100644
index 00000000..48fdd962
--- /dev/null
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -0,0 +1,84 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2009 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class Item_Helper_Test extends Unit_Test_Case {
+  private $_group;
+  private $_album;
+  private $_item;
+  //private $_user;
+
+  public function teardown() {
+    try {
+      $this->_group->delete();
+    } catch (Exception $e) { }
+
+    try {
+      $this->_album->delete();
+    } catch (Exception $e) { }
+
+    //try {
+    //  $this->_user->delete();
+    //} catch (Exception $e) { }
+  }
+
+  public function setup() {
+  }
+
+  public function viewable_item_test() {
+    $this->_group = group::create("access_test");
+    $root = ORM::factory("item", 1);
+    $this->_album = album::create($root, rand(), "visible_test");
+    $this->_user = user::create("visible_test", "Visible Test", "");
+    $this->_user->add($this->_group);
+    $this->_item = self::_create_random_item($this->_album);
+    comment::create($this->_item, $this->_user, "This is a comment");
+    access::deny(group::everybody(), "view", $this->_album);
+    $active = user::active();
+
+    $items = ORM::factory("item")
+      ->where("id", $this->_album->id)
+      ->find_all();
+    print Database::instance()->last_query() . "\n";
+    $items = ORM::factory("item")
+      ->where("id", $this->_album->id)
+      ->viewable()
+      ->find_all();
+    print Database::instance()->last_query() . "\n";
+  }
+
+
+  //public function viewable_one_restrictions_test() {
+  //  $item = self::create_random_item();
+  //  $this->assert_true(!empty($item->created));
+  //  $this->assert_true(!empty($item->updated));
+  //}
+  //public function viewable_multiple_restrictions_test() {
+  //  $item = self::create_random_item();
+  //  $this->assert_true(!empty($item->created));
+  //  $this->assert_true(!empty($item->updated));
+  //}
+
+  private static function _create_random_item($album) {
+    $item = ORM::factory("item");
+    /* Set all required fields (values are irrelevant) */
+    $item->name = rand();
+    $item->type = "photo";
+    return $item->add_to_parent($album);
+  }
+}
-- 
cgit v1.2.3


From 3f997562dee7fa45fec58604dd894d9c523a1113 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Mon, 7 Sep 2009 15:41:03 -0700
Subject: Add support for a per-item "slug" which will be the user-visible url
 component for that given item.  Album hierarchies are represented by nested
 slugs.  By default, we convert the filename to a slug when you create an
 album, photo or movie.

---
 modules/gallery/helpers/album.php             |  17 +++++-
 modules/gallery/helpers/gallery_installer.php |  17 +++++-
 modules/gallery/helpers/item.php              |   8 +++
 modules/gallery/helpers/movie.php             |  17 ++++--
 modules/gallery/helpers/photo.php             |  19 +++++--
 modules/gallery/models/item.php               |  72 ++++++++++++++++++--------
 modules/gallery/module.info                   |   2 +-
 modules/gallery/tests/Movie_Helper_Test.php   |  14 ++++-
 modules/gallery/tests/Photo_Helper_Test.php   |  14 ++++-
 modules/gallery/tests/test.flv                | Bin 0 -> 88722 bytes
 10 files changed, 142 insertions(+), 38 deletions(-)
 create mode 100644 modules/gallery/tests/test.flv

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

diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php
index d46f21ac..78c5062f 100644
--- a/modules/gallery/helpers/album.php
+++ b/modules/gallery/helpers/album.php
@@ -30,9 +30,10 @@ class album_Core {
    * @param string  $name the name of this new album (it will become the directory name on disk)
    * @param integer $title the title of the new album
    * @param string  $description (optional) the longer description of this album
+   * @param string  $slug (optional) the url component for this photo
    * @return Item_Model
    */
-  static function create($parent, $name, $title, $description=null, $owner_id=null) {
+  static function create($parent, $name, $title, $description=null, $owner_id=null, $slug=null) {
     if (!$parent->loaded || !$parent->is_album()) {
       throw new Exception("@todo INVALID_PARENT");
     }
@@ -47,6 +48,10 @@ class album_Core {
       throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD");
     }
 
+    if (empty($slug)) {
+      $slug = item::convert_filename_to_slug($name);
+    }
+
     $album = ORM::factory("item");
     $album->type = "album";
     $album->title = $title;
@@ -55,15 +60,23 @@ class album_Core {
     $album->owner_id = $owner_id;
     $album->thumb_dirty = 1;
     $album->resize_dirty = 1;
+    $album->slug = $slug;
     $album->rand_key = ((float)mt_rand()) / (float)mt_getrandmax();
     $album->sort_column = "created";
     $album->sort_order = "ASC";
 
+    // 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)
+           ->open_paren()
            ->where("name", $album->name)
+           ->orwhere("slug", $album->slug)
+           ->close_paren()
            ->find()->id) {
-      $album->name = "{$name}-" . rand();
+      $rand = rand();
+      $album->name = "{$name}-$rand";
+      $album->slug = "{$slug}-$rand";
     }
 
     $album = $album->add_to_parent($parent);
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index 40830bc0..91654afe 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -80,10 +80,12 @@ class gallery_installer {
                  `parent_id` int(9) NOT NULL,
                  `rand_key` float default NULL,
                  `relative_path_cache` varchar(255) default NULL,
+                 `relative_url_cache` varchar(255) default NULL,
                  `resize_dirty` boolean default 1,
                  `resize_height` int(9) default NULL,
                  `resize_width` int(9) default NULL,
                  `right_ptr` int(9) NOT NULL,
+                 `slug` varchar(255) default NULL,
                  `sort_column` varchar(64) default NULL,
                  `sort_order` char(4) default 'ASC',
                  `thumb_dirty` boolean default 1,
@@ -260,7 +262,7 @@ class gallery_installer {
     module::set_var("gallery", "show_credits", 1);
     // @todo this string needs to be picked up by l10n_scanner
     module::set_var("gallery", "credits", "Powered by <a href=\"%url\">Gallery %version</a>");
-    module::set_version("gallery", 11);
+    module::set_version("gallery", 12);
   }
 
   static function upgrade($version) {
@@ -343,7 +345,18 @@ class gallery_installer {
 
       module::set_version("gallery", $version = 11);
     }
-}
+
+    if ($version == 11) {
+      $db->query("ALTER TABLE {items} ADD COLUMN `relative_url_cache` varchar(255) DEFAULT NULL");
+      $db->query("ALTER TABLE {items} ADD COLUMN `slug` varchar(255) DEFAULT NULL");
+
+      // This is imperfect since some of the slugs may contain invalid characters, but it'll do
+      // for now because we don't want a lengthy operation here.
+      $db->query("UPDATE {items} SET `slug` = `name`");
+      $db->query("UPDATE {items} SET `relative_url_cache` = `relative_path_cache`");
+      module::set_version("gallery", $version = 12);
+    }
+  }
 
   static function uninstall() {
     $db = Database::instance();
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 8839861f..bf948731 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -120,6 +120,14 @@ class item_Core {
     return $title;
   }
 
+  /**
+   * Convert a filename into something we can use as a url component.
+   * @param string $filename
+   */
+  static function convert_filename_to_slug($filename) {
+    return preg_replace("/[^A-Za-z0-9-_]+/", "-", pathinfo($filename, PATHINFO_FILENAME));
+  }
+
   /**
    * Display delete confirmation message and form
    * @param object $item
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index ec975cb0..59bf5c19 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -31,10 +31,11 @@ class movie_Core {
    * @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) {
+                         $description=null, $owner_id=null, $slug=null) {
     if (!$parent->loaded || !$parent->is_album()) {
       throw new Exception("@todo INVALID_PARENT");
     }
@@ -67,6 +68,10 @@ class movie_Core {
       $name .= "." . $pi["extension"];
     }
 
+    if (empty($slug)) {
+      $slug = item::convert_filename_to_slug($name);
+    }
+
     $movie = ORM::factory("item");
     $movie->type = "movie";
     $movie->title = $title;
@@ -79,15 +84,21 @@ class movie_Core {
     $movie->thumb_dirty = 1;
     $movie->resize_dirty = 1;
     $movie->sort_column = "weight";
+    $movie->slug = $slug;
     $movie->rand_key = ((float)mt_rand()) / (float)mt_getrandmax();
 
     // Randomize the name if there's a conflict
+    // @todo Improve this.  Random numbers are not user friendly
     while (ORM::factory("item")
            ->where("parent_id", $parent->id)
+           ->open_paren()
            ->where("name", $movie->name)
+           ->orwhere("slug", $movie->slug)
+           ->close_paren()
            ->find()->id) {
-      // @todo Improve this.  Random numbers are not user friendly
-      $movie->name = rand() . "." . $pi["extension"];
+      $rand = rand();
+      $movie->name = "{$name}.$rand.{$pi['extension']}";
+      $movie->slug = "{$slug}-$rand";
     }
 
     // This saves the photo
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 40b645a2..c713fff9 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -31,10 +31,11 @@ class photo_Core {
    * @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) {
+                         $description=null, $owner_id=null, $slug=null) {
     if (!$parent->loaded || !$parent->is_album()) {
       throw new Exception("@todo INVALID_PARENT");
     }
@@ -66,6 +67,10 @@ class photo_Core {
       $name .= "." . $pi["extension"];
     }
 
+    if (empty($slug)) {
+      $slug = item::convert_filename_to_slug($name);
+    }
+
     $photo = ORM::factory("item");
     $photo->type = "photo";
     $photo->title = $title;
@@ -78,15 +83,21 @@ class photo_Core {
     $photo->thumb_dirty = 1;
     $photo->resize_dirty = 1;
     $photo->sort_column = "weight";
+    $photo->slug = $slug;
     $photo->rand_key = ((float)mt_rand()) / (float)mt_getrandmax();
 
-    // Randomize the name if there's a conflict
+    // 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)
+           ->open_paren()
            ->where("name", $photo->name)
+           ->orwhere("slug", $photo->slug)
+           ->close_paren()
            ->find()->id) {
-      // @todo Improve this.  Random numbers are not user friendly
-      $photo->name = rand() . "." . $pi["extension"];
+      $rand = rand();
+      $photo->name = "{$name}.$rand.{$pi['extension']}";
+      $photo->slug = "{$slug}-$rand";
     }
 
     // This saves the photo
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 8905a627..b679ebf1 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -117,7 +117,8 @@ class Item_Model extends ORM_MPTT {
       @rename(dirname($original_thumb_path), dirname($this->thumb_path()));
       Database::instance()
         ->update("items",
-                 array("relative_path_cache" => null),
+                 array("relative_path_cache" => null,
+                       "relative_url_cache" => null),
                  array("left_ptr >" => $this->left_ptr, "right_ptr <" => $this->right_ptr));
     } else {
       @rename($original_resize_path, $this->resize_path());
@@ -153,7 +154,8 @@ class Item_Model extends ORM_MPTT {
     if ($this->is_album()) {
       Database::instance()
         ->update("items",
-                 array("relative_path_cache" => null),
+                 array("relative_path_cache" => null,
+                       "relative_url_cache" => null),
                  array("left_ptr >" => $this->left_ptr, "right_ptr <" => $this->right_ptr));
     }
 
@@ -188,8 +190,8 @@ class Item_Model extends ORM_MPTT {
    * photo: http://example.com/gallery3/var/albums/album1/photo.jpg
    */
   public function file_url($full_uri=false) {
-    $relative_path = "var/albums/" . $this->relative_path();
-    return $full_uri ? url::abs_file($relative_path) : url::file($relative_path);
+    $relative_url = "var/albums/" . $this->relative_url();
+    return $full_uri ? url::abs_file($relative_url) : url::file($relative_url);
   }
 
   /**
@@ -221,8 +223,8 @@ class Item_Model extends ORM_MPTT {
    */
   public function thumb_url($full_uri=false) {
     $cache_buster = "?m=" . $this->updated;
-    $relative_path = "var/thumbs/" . $this->relative_path();
-    $base = ($full_uri ? url::abs_file($relative_path) : url::file($relative_path));
+    $relative_url = "var/thumbs/" . $this->relative_url();
+    $base = ($full_uri ? url::abs_file($relative_url) : url::file($relative_url));
     if ($this->is_photo()) {
       return $base . $cache_buster;
     } else if ($this->is_album()) {
@@ -248,11 +250,33 @@ class Item_Model extends ORM_MPTT {
    * photo: http://example.com/gallery3/var/albums/album1/photo.resize.jpg
    */
   public function resize_url($full_uri=false) {
-    $relative_path = "var/resizes/" . $this->relative_path();
-    return ($full_uri ? url::abs_file($relative_path) : url::file($relative_path)) .
+    $relative_url = "var/resizes/" . $this->relative_url();
+    return ($full_uri ? url::abs_file($relative_url) : url::file($relative_url)) .
       ($this->is_album() ? "/.album.jpg" : "");
   }
 
+  /**
+   * Rebuild the relative_path_cache and relative_url_cache.
+   */
+  private function _build_relative_caches() {
+    $names = array();
+    $slugs = array();
+    foreach (Database::instance()
+             ->select(array("name", "slug"))
+             ->from("items")
+             ->where("left_ptr <=", $this->left_ptr)
+             ->where("right_ptr >=", $this->right_ptr)
+             ->where("id <>", 1)
+             ->orderby("left_ptr", "ASC")
+             ->get() as $row) {
+      $names[] = $row->name;
+      $slugs[] = $row->slug;
+    }
+    $this->relative_path_cache = implode($names, "/");
+    $this->relative_url_cache = implode($slugs, "/");
+    $this->save();
+  }
+
   /**
    * Return the relative path to this item's file.
    * @return string
@@ -263,23 +287,26 @@ class Item_Model extends ORM_MPTT {
     }
 
     if (!isset($this->relative_path_cache)) {
-      $paths = array();
-      foreach (Database::instance()
-               ->select("name")
-               ->from("items")
-               ->where("left_ptr <=", $this->left_ptr)
-               ->where("right_ptr >=", $this->right_ptr)
-               ->where("id <>", 1)
-               ->orderby("left_ptr", "ASC")
-               ->get() as $row) {
-        $paths[] = $row->name;
-      }
-      $this->relative_path_cache = implode($paths, "/");
-      $this->save();
+      $this->_build_relative_caches();
     }
     return $this->relative_path_cache;
   }
 
+  /**
+   * Return the relative url to this item's file.
+   * @return string
+   */
+  public function relative_url() {
+    if (!$this->loaded) {
+      return;
+    }
+
+    if (!isset($this->relative_url_cache)) {
+      $this->_build_relative_caches();
+    }
+    return $this->relative_url_cache;
+  }
+
   /**
    * @see ORM::__get()
    */
@@ -302,7 +329,8 @@ class Item_Model extends ORM_MPTT {
    */
   public function __set($column, $value) {
     if ($column == "name") {
-      // Clear the relative path as it is no longer valid.
+      // Clear the relative path as it is no longer valid.  The relative url cache does not need
+      // to be flushed because it's not tightly bound to the actual name of the file.
       $this->relative_path_cache = null;
     }
     parent::__set($column, $value);
diff --git a/modules/gallery/module.info b/modules/gallery/module.info
index 6b9dd1ba..70bd91e2 100644
--- a/modules/gallery/module.info
+++ b/modules/gallery/module.info
@@ -1,3 +1,3 @@
 name = "Gallery 3"
 description = "Gallery core application"
-version = 11
+version = 12
diff --git a/modules/gallery/tests/Movie_Helper_Test.php b/modules/gallery/tests/Movie_Helper_Test.php
index 627651bb..23544934 100644
--- a/modules/gallery/tests/Movie_Helper_Test.php
+++ b/modules/gallery/tests/Movie_Helper_Test.php
@@ -22,7 +22,7 @@ class Movie_Helper_Test extends Unit_Test_Case {
     $rand = rand();
     $root = ORM::factory("item", 1);
     try {
-      $movie = movie::create($root, MODPATH . "gallery/tests/test.jpg", "$rand/.jpg", $rand, $rand);
+      $movie = movie::create($root, MODPATH . "gallery/tests/test.flv", "$rand/.flv", $rand, $rand);
     } catch (Exception $e) {
       // pass
       return;
@@ -35,7 +35,7 @@ class Movie_Helper_Test extends Unit_Test_Case {
     $rand = rand();
     $root = ORM::factory("item", 1);
     try {
-      $movie = movie::create($root, MODPATH . "gallery/tests/test.jpg", "$rand.jpg.", $rand, $rand);
+      $movie = movie::create($root, MODPATH . "gallery/tests/test.flv", "$rand.flv.", $rand, $rand);
     } catch (Exception $e) {
       $this->assert_equal("@todo NAME_CANNOT_END_IN_PERIOD", $e->getMessage());
       return;
@@ -43,4 +43,14 @@ class Movie_Helper_Test extends Unit_Test_Case {
 
     $this->assert_true(false, "Shouldn't create a movie with trailing . in the name");
   }
+
+  public function create_movie_creates_reasonable_slug_test() {
+    $rand = rand();
+    $root = ORM::factory("item", 1);
+    $album = album::create($root, $rand, $rand, $rand);
+    $movie = movie::create(
+      $album, MODPATH . "gallery/tests/test.flv", "This (is) my file%name.flv", $rand, $rand);
+
+    $this->assert_equal("This-is-my-file-name", $movie->slug);
+  }
 }
diff --git a/modules/gallery/tests/Photo_Helper_Test.php b/modules/gallery/tests/Photo_Helper_Test.php
index c0641ef4..a261693f 100644
--- a/modules/gallery/tests/Photo_Helper_Test.php
+++ b/modules/gallery/tests/Photo_Helper_Test.php
@@ -69,7 +69,7 @@ class Photo_Helper_Test extends Unit_Test_Case {
     $rand = rand();
     $root = ORM::factory("item", 1);
     $photo = photo::create($root, MODPATH . "gallery/tests/test.jpg", "$rand.jpg", $rand, $rand);
-    $this->assert_equal("http://./var/thumbs/{$rand}.jpg?m={$photo->updated}", $photo->thumb_url());
+    $this->assert_equal("http://./var/thumbs/{$rand}?m={$photo->updated}", $photo->thumb_url());
   }
 
   public function resize_url_test() {
@@ -78,7 +78,17 @@ class Photo_Helper_Test extends Unit_Test_Case {
     $album = album::create($root, $rand, $rand, $rand);
     $photo = photo::create($album, MODPATH . "gallery/tests/test.jpg", "$rand.jpg", $rand, $rand);
 
-    $this->assert_equal("http://./var/resizes/{$rand}/{$rand}.jpg", $photo->resize_url());
+    $this->assert_equal("http://./var/resizes/{$rand}/{$rand}", $photo->resize_url());
+  }
+
+  public function create_photo_creates_reasonable_slug_test() {
+    $rand = rand();
+    $root = ORM::factory("item", 1);
+    $album = album::create($root, $rand, $rand, $rand);
+    $photo = photo::create(
+      $album, MODPATH . "gallery/tests/test.jpg", "This (is) my file%name.jpg", $rand, $rand);
+
+    $this->assert_equal("This-is-my-file-name", $photo->slug);
   }
 
   public function create_photo_shouldnt_allow_names_with_slash_test() {
diff --git a/modules/gallery/tests/test.flv b/modules/gallery/tests/test.flv
new file mode 100644
index 00000000..799d137e
Binary files /dev/null and b/modules/gallery/tests/test.flv differ
-- 
cgit v1.2.3


From 2f666f4c527a89df235c7fdaab4b5483c5dd2595 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Mon, 7 Sep 2009 19:52:47 -0700
Subject: Add item::validate_url_safe() with a test.

---
 modules/gallery/helpers/item.php           |  7 +++++++
 modules/gallery/tests/Item_Helper_Test.php | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index bf948731..7ce6519e 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -90,6 +90,13 @@ class item_Core {
     }
   }
 
+  static function validate_url_safe($input) {
+    if (preg_match("/[^A-Za-z0-9-_]/", $input->value)) {
+      $input->add_error("not_url_safe", 1);
+    }
+    Kohana::log("alert",print_r($input,1));
+  }
+
   static function validate_no_name_conflict($input) {
     $itemid = Input::instance()->post("item");
     if (is_array($itemid)) {
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 3f80733f..87859565 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -38,6 +38,16 @@ class Item_Helper_Test extends Unit_Test_Case {
       ORM::factory("item")->viewable()->where("id", $item->id)->count_all());
   }
 
+  public function validate_url_safe_test() {
+    $input = new MockInput();
+    $input->value = "Ab_cd-ef-d9";
+    item::validate_url_safe($input);
+    $this->assert_true(!isset($input->not_url_safe));
+
+    $input->value = "ab&cd";
+    item::validate_url_safe($input);
+    $this->assert_equal(1, $input->not_url_safe);
+  }
 
   private static function _create_random_item($album) {
     // Set all required fields (values are irrelevant)
@@ -47,3 +57,9 @@ class Item_Helper_Test extends Unit_Test_Case {
     return $item->add_to_parent($album);
   }
 }
+
+class MockInput {
+  function add_error($error, $value) {
+    $this->$error = $value;
+  }
+}
\ No newline at end of file
-- 
cgit v1.2.3


From 83ebc2e2b053a0d9fb100b1663471127ca281bda Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Mon, 7 Sep 2009 20:59:11 -0700
Subject: Remove unused item::validate_no_name_conflict() and a debug line.

---
 modules/gallery/helpers/item.php | 19 -------------------
 1 file changed, 19 deletions(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 7ce6519e..8da88b6e 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -94,25 +94,6 @@ class item_Core {
     if (preg_match("/[^A-Za-z0-9-_]/", $input->value)) {
       $input->add_error("not_url_safe", 1);
     }
-    Kohana::log("alert",print_r($input,1));
-  }
-
-  static function validate_no_name_conflict($input) {
-    $itemid = Input::instance()->post("item");
-    if (is_array($itemid)) {
-      $itemid = $itemid[0];
-    }
-    $item = ORM::factory("item")
-      ->in("id", $itemid)
-      ->find();
-    if (Database::instance()
-        ->from("items")
-        ->where("parent_id", $item->parent_id)
-        ->where("id <>", $item->id)
-        ->where("name", $input->value)
-        ->count_records()) {
-      $input->add_error("conflict", 1);
-    }
   }
 
   /**
-- 
cgit v1.2.3


From 2aad580f53dbc06bb170c710467b47a5a532c6c8 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 8 Sep 2009 13:44:52 -0700
Subject: Move specialized (pretty) url generation back into Item_Model so that
 we're not relying on overriding url::site() to do tricks around item urls. 
 This means that you won't get item urls by doing url::site("albums/37"), for
 example, but it also means that we won't get pretty urls where we don't
 expect them (like in the action of a <form> element).

Incidentally, this will help us move over to using the slug format
because if you've got a bad character in a url, the edit forms will
now work on it since they'll be id based.
---
 modules/gallery/controllers/admin_themes.php       |  4 +-
 modules/gallery/controllers/after_install.php      |  2 +-
 modules/gallery/controllers/albums.php             | 16 ++++----
 modules/gallery/controllers/move.php               |  2 +-
 modules/gallery/controllers/movies.php             |  2 +-
 modules/gallery/controllers/photos.php             |  2 +-
 modules/gallery/controllers/quick.php              |  2 +-
 modules/gallery/helpers/MY_url.php                 | 19 ----------
 modules/gallery/helpers/gallery.php                | 44 +++++++++++-----------
 modules/gallery/helpers/gallery_rss.php            |  4 +-
 modules/gallery/helpers/item.php                   |  8 ++++
 modules/gallery/models/item.php                    | 21 +++++++++--
 .../views/admin_block_photo_stream.html.php        |  2 +-
 modules/gallery/views/upgrader.html.php            |  2 +-
 .../notification/views/comment_published.html.php  |  4 +-
 modules/notification/views/item_deleted.html.php   |  4 +-
 modules/search/views/search.html.php               |  2 +-
 modules/user/controllers/login.php                 |  2 +-
 modules/user/controllers/logout.php                |  2 +-
 modules/user/controllers/password.php              |  2 +-
 themes/admin_default/views/admin.html.php          |  6 +--
 themes/default/views/page.html.php                 |  4 +-
 22 files changed, 80 insertions(+), 76 deletions(-)

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

diff --git a/modules/gallery/controllers/admin_themes.php b/modules/gallery/controllers/admin_themes.php
index da001c55..24f91aba 100644
--- a/modules/gallery/controllers/admin_themes.php
+++ b/modules/gallery/controllers/admin_themes.php
@@ -38,7 +38,7 @@ class Admin_Themes_Controller extends Admin_Controller {
       $theme_info = new ArrayObject(parse_ini_file($file), ArrayObject::ARRAY_AS_PROPS);
       $theme_info->description = t($theme_info->description);
       $theme_info->name = t($theme_info->name);
-      
+
       $themes[$theme_name] = $theme_info;
     }
     return $themes;
@@ -54,7 +54,7 @@ class Admin_Themes_Controller extends Admin_Controller {
     if ($type == "admin") {
       $view->url = url::site("admin?theme=$theme_name");
     } else {
-      $view->url = url::site("albums/1?theme=$theme_name");
+      $view->url = item::root()->url("theme=$theme_name");
     }
     print $view;
   }
diff --git a/modules/gallery/controllers/after_install.php b/modules/gallery/controllers/after_install.php
index f066afe4..b640092f 100644
--- a/modules/gallery/controllers/after_install.php
+++ b/modules/gallery/controllers/after_install.php
@@ -20,7 +20,7 @@
 class After_Install_Controller extends Controller {
   public function index() {
     if (!user::active()->admin) {
-      url::redirect("albums/1");
+      url::redirect(item::root()->url());
     }
 
     $v = new View("after_install.html");
diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php
index b7a9f339..abcabfa6 100644
--- a/modules/gallery/controllers/albums.php
+++ b/modules/gallery/controllers/albums.php
@@ -42,9 +42,9 @@ class Albums_Controller extends Items_Controller {
       $index = $album->get_position($show);
       $page = ceil($index / $page_size);
       if ($page == 1) {
-        url::redirect("albums/$album->id");
+        url::redirect($album->url());
       } else {
-        url::redirect("albums/$album->id?page=$page");
+        url::redirect($album->url("page=$page"));
       }
     }
 
@@ -55,9 +55,9 @@ class Albums_Controller extends Items_Controller {
 
     // Make sure that the page references a valid offset
     if ($page < 1) {
-      url::redirect("albums/$album->id");
+      url::redirect($album->url());
     } else if ($page > $max_pages) {
-      url::redirect("albums/$album->id?page=$max_pages");
+      url::redirect($album->url("page=$max_pages"));
     }
 
     $template = new Theme_View("page.html", "album");
@@ -116,8 +116,8 @@ class Albums_Controller extends Items_Controller {
 
       print json_encode(
         array("result" => "success",
-              "location" => url::site("albums/$new_album->id"),
-              "resource" => url::site("albums/$new_album->id")));
+              "location" => $new_album->url(),
+              "resource" => $new_album->url()));
     } else {
       print json_encode(
         array(
@@ -149,8 +149,8 @@ class Albums_Controller extends Items_Controller {
 
       print json_encode(
         array("result" => "success",
-              "resource" => url::site("photos/$photo->id"),
-              "location" => url::site("photos/$photo->id")));
+              "resource" => $photo->url(),
+              "location" => $photo->url()));
     } else {
       print json_encode(
         array("result" => "error",
diff --git a/modules/gallery/controllers/move.php b/modules/gallery/controllers/move.php
index 93ef05a6..87b73436 100644
--- a/modules/gallery/controllers/move.php
+++ b/modules/gallery/controllers/move.php
@@ -43,7 +43,7 @@ class Move_Controller extends Controller {
 
     print json_encode(
       array("result" => "success",
-            "location" => url::site("albums/{$target->id}")));
+            "location" => $target->url()));
   }
 
   public function show_sub_tree($source_id, $target_id) {
diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php
index c549dbf8..1c266cc8 100644
--- a/modules/gallery/controllers/movies.php
+++ b/modules/gallery/controllers/movies.php
@@ -105,7 +105,7 @@ class Movies_Controller extends Items_Controller {
       $photo->save();
       module::event("item_edit_form_completed", $photo, $form);
 
-      log::success("content", "Updated photo", "<a href=\"photos/$photo->id\">view</a>");
+      log::success("content", "Updated photo", "<a href=\"{$photo->url()}\">view</a>");
       message::success(
         t("Saved photo %photo_title", array("photo_title" => $photo->title)));
 
diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php
index 959097b2..79ad674a 100644
--- a/modules/gallery/controllers/photos.php
+++ b/modules/gallery/controllers/photos.php
@@ -97,7 +97,7 @@ class Photos_Controller extends Items_Controller {
       $photo->save();
       module::event("item_edit_form_completed", $photo, $form);
 
-      log::success("content", "Updated photo", "<a href=\"photos/$photo->id\">view</a>");
+      log::success("content", "Updated photo", "<a href=\"{$photo->url()}\">view</a>");
       message::success(
                        t("Saved photo %photo_title",
                          array("photo_title" => html::purify($photo->title))));
diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php
index 20731f9c..2ac54754 100644
--- a/modules/gallery/controllers/quick.php
+++ b/modules/gallery/controllers/quick.php
@@ -121,7 +121,7 @@ class Quick_Controller extends Controller {
       print json_encode(array("result" => "success", "reload" => 1));
     } else {
       print json_encode(array("result" => "success",
-                              "location" => url::site("albums/$parent->id")));
+                              "location" => $parent->url()));
     }
   }
 
diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php
index 1ca9a58f..368c947e 100644
--- a/modules/gallery/helpers/MY_url.php
+++ b/modules/gallery/helpers/MY_url.php
@@ -18,25 +18,6 @@
  * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
  */
 class url extends url_Core {
-  static function site($uri, $protocol=false) {
-    if (($pos = strpos($uri, "?")) !== false) {
-      list ($uri, $query) = explode("?", $uri, 2);
-      $query = "?$query";
-    } else {
-      $query = "";
-    }
-
-    // @todo if we're only doing this for Item_Model, why not just put this
-    // all into Item_Model::url()?  It'd make url::site() faster.  Downside is that
-    // anywhere we refer to an item by id, eg url::site("albums/123") would have
-    // to load the item and do $item->url();
-    $parts = explode("/", $uri, 3);
-    if ($parts[0] == "albums" || $parts[0] == "photos" || $parts[0] == "movies") {
-      $uri = model_cache::get("item", $parts[1])->relative_url();
-    }
-    return parent::site($uri . $query, $protocol);
-  }
-
   static function parse_url() {
     if (Router::$controller) {
       return;
diff --git a/modules/gallery/helpers/gallery.php b/modules/gallery/helpers/gallery.php
index f72ef982..813134eb 100644
--- a/modules/gallery/helpers/gallery.php
+++ b/modules/gallery/helpers/gallery.php
@@ -82,9 +82,9 @@ class gallery_Core {
   static function site_menu($menu, $theme) {
     if ($theme->page_type != "login") {
       $menu->append(Menu::factory("link")
-              ->id("home")
-              ->label(t("Home"))
-              ->url(url::site("albums/1")));
+                    ->id("home")
+                    ->label(t("Home"))
+                    ->url(item::root()->url()));
 
       $item = $theme->item();
 
@@ -92,39 +92,39 @@ class gallery_Core {
       $can_add = $item && access::can("add", $item);
 
       if ($can_add) {
-              $menu->append($add_menu = Menu::factory("submenu")
-                    ->id("add_menu")
-                    ->label(t("Add")));
+        $menu->append($add_menu = Menu::factory("submenu")
+                      ->id("add_menu")
+                      ->label(t("Add")));
         $add_menu->append(Menu::factory("dialog")
-                    ->id("add_photos_item")
-                    ->label(t("Add photos"))
-                    ->url(url::site("simple_uploader/app/$item->id")));
+                          ->id("add_photos_item")
+                          ->label(t("Add photos"))
+                          ->url(url::site("simple_uploader/app/$item->id")));
         if ($item->is_album()) {
-                $add_menu->append(Menu::factory("dialog")
-                      ->id("add_album_item")
-                      ->label(t("Add an album"))
-                      ->url(url::site("form/add/albums/$item->id?type=album")));
-                                }
+          $add_menu->append(Menu::factory("dialog")
+                            ->id("add_album_item")
+                            ->label(t("Add an album"))
+                            ->url(url::site("form/add/albums/$item->id?type=album")));
+        }
       }
 
       $menu->append($options_menu = Menu::factory("submenu")
-              ->id("options_menu")
-              ->label(t("Photo options")));
+                    ->id("options_menu")
+                    ->label(t("Photo options")));
       if ($item && ($can_edit || $can_add)) {
         if ($can_edit) {
           $options_menu->append(Menu::factory("dialog")
-                          ->id("edit_item")
-                          ->label($item->is_album() ? t("Edit album") : t("Edit photo"))
-                          ->url(url::site("form/edit/{$item->type}s/$item->id")));
+                                ->id("edit_item")
+                                ->label($item->is_album() ? t("Edit album") : t("Edit photo"))
+                                ->url(url::site("form/edit/{$item->type}s/$item->id")));
         }
 
         if ($item->is_album()) {
           $options_menu->label(t("Album options"));
           if ($can_edit) {
             $options_menu->append(Menu::factory("dialog")
-                            ->id("edit_permissions")
-                            ->label(t("Edit permissions"))
-                            ->url(url::site("permissions/browse/$item->id")));
+                                  ->id("edit_permissions")
+                                  ->label(t("Edit permissions"))
+                                  ->url(url::site("permissions/browse/$item->id")));
           }
         }
       }
diff --git a/modules/gallery/helpers/gallery_rss.php b/modules/gallery/helpers/gallery_rss.php
index dee6ae40..f30df092 100644
--- a/modules/gallery/helpers/gallery_rss.php
+++ b/modules/gallery/helpers/gallery_rss.php
@@ -40,7 +40,7 @@ class gallery_rss_Core {
 
       $feed->max_pages = ceil($all_children->find_all()->count() / $limit);
       $feed->title = t("Recent Updates");
-      $feed->link = url::abs_site("albums/1");
+      $feed->link = item::root()->abs_url();
       $feed->description = t("Recent Updates");
       return $feed;
 
@@ -54,7 +54,7 @@ class gallery_rss_Core {
       $feed->max_pages = ceil(
         $item->viewable()->descendants_count(array("type" => "photo")) / $limit);
       $feed->title = html::purify($item->title);
-      $feed->link = url::abs_site("albums/{$item->id}");
+      $feed->link = $item->abs_url();
       $feed->description = nl2br(html::purify($item->description));
 
       return $feed;
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 8da88b6e..d907a177 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -184,4 +184,12 @@ class item_Core {
 
     return $model;
   }
+
+  /**
+   * Return the root Item_Model
+   * @return Item_Model
+   */
+  static function root() {
+    return model_cache::get("item", 1);
+  }
 }
\ No newline at end of file
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 0ec5d048..6e9debea 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -168,9 +168,24 @@ class Item_Model extends ORM_MPTT {
    *
    * @param string $query the query string (eg "show=3")
    */
-  public function url($query=array(), $full_uri=false) {
-    $url = ($full_uri ? url::abs_site("{$this->type}s/$this->id")
-            : url::site("{$this->type}s/$this->id"));
+  public function url($query=null) {
+    $relative_url = $this->relative_url();
+    $url = url::site($relative_url);
+    if ($query) {
+      $url .= "?$query";
+    }
+    return $url;
+  }
+
+  /**
+   * album: url::abs_site("albums/2")
+   * photo: url::abs_site("photos/3")
+   *
+   * @param string $query the query string (eg "show=3")
+   */
+  public function abs_url($query=null) {
+    $relative_url = $this->relative_url();
+    $url = url::abs_site($relative_url);
     if ($query) {
       $url .= "?$query";
     }
diff --git a/modules/gallery/views/admin_block_photo_stream.html.php b/modules/gallery/views/admin_block_photo_stream.html.php
index 1b9d8ff5..4968c39b 100644
--- a/modules/gallery/views/admin_block_photo_stream.html.php
+++ b/modules/gallery/views/admin_block_photo_stream.html.php
@@ -2,7 +2,7 @@
 <ul>
 <? foreach ($photos as $photo): ?>
   <li class="gItem gPhoto">
-    <a href="<?= url::site("photos/$photo->id") ?>" title="<?= html::purify($photo->title)->for_html_attr() ?>">
+    <a href="<?= $photo->url() ?>" title="<?= html::purify($photo->title)->for_html_attr() ?>">
       <img <?= photo::img_dimensions($photo->width, $photo->height, 72) ?>
         src="<?= $photo->thumb_url() ?>" alt="<?= html::purify($photo->title)->for_html_attr() ?>" />
     </a>
diff --git a/modules/gallery/views/upgrader.html.php b/modules/gallery/views/upgrader.html.php
index 04200920..5f93c2d5 100644
--- a/modules/gallery/views/upgrader.html.php
+++ b/modules/gallery/views/upgrader.html.php
@@ -18,7 +18,7 @@
             <h1> <?= t("That's it!") ?> </h1>
             <p>
               <?= t("Your <a href=\"%url\">Gallery</a> is up to date.",
-                    array("url" => html::mark_clean(url::site("albums/1")))) ?>
+                    array("url" => html::mark_clean(item::root()->url()))) ?>
             </p>
           </div>
         </div>
diff --git a/modules/notification/views/comment_published.html.php b/modules/notification/views/comment_published.html.php
index e39e39c6..a8ca1899 100644
--- a/modules/notification/views/comment_published.html.php
+++ b/modules/notification/views/comment_published.html.php
@@ -25,8 +25,8 @@
       <tr>
         <td><?= t("Url:") ?></td>
         <td>
-          <a href="<?= $comment->item()->url(array(), true) ?>#comments">
-            <?= $comment->item()->url(array(), true) ?>#comments
+          <a href="<?= $comment->item()->abs_url() ?>#comments">
+            <?= $comment->item()->abs_url() ?>#comments
           </a>
         </td>
       </tr>
diff --git a/modules/notification/views/item_deleted.html.php b/modules/notification/views/item_deleted.html.php
index a51782ff..a95cdd89 100644
--- a/modules/notification/views/item_deleted.html.php
+++ b/modules/notification/views/item_deleted.html.php
@@ -15,8 +15,8 @@
       <tr>
         <td><?= t("Url:") ?></td>
         <td>
-          <a href="<?= $item->parent()->url(array(), true) ?>">
-            <?= $item->parent()->url(array(), true) ?>
+          <a href="<?= $item->parent()->abs_url() ?>">
+            <?= $item->parent()->abs_url() ?>
           </a>
         </td>
       </tr>
diff --git a/modules/search/views/search.html.php b/modules/search/views/search.html.php
index 7963948d..91d9eec8 100644
--- a/modules/search/views/search.html.php
+++ b/modules/search/views/search.html.php
@@ -28,7 +28,7 @@
         <? $item_class = "gAlbum"; ?>
       <? endif ?>
    <li class="gItem <?= $item_class ?>">
-      <a href="<?= url::site("items/$item->id") ?>">
+      <a href="<?= $item->url() ?>">
         <?= $item->thumb_img() ?>
         <p>
     <?= html::purify($item->title) ?>
diff --git a/modules/user/controllers/login.php b/modules/user/controllers/login.php
index b81b17b2..dcac0723 100644
--- a/modules/user/controllers/login.php
+++ b/modules/user/controllers/login.php
@@ -48,7 +48,7 @@ class Login_Controller extends Controller {
 
     list ($valid, $form) = $this->_auth("login/auth_html");
     if ($valid) {
-      url::redirect("albums/1");
+      url::redirect(item::root()->url());
     } else {
       print $form;
     }
diff --git a/modules/user/controllers/logout.php b/modules/user/controllers/logout.php
index fc3ced56..3bf274a6 100644
--- a/modules/user/controllers/logout.php
+++ b/modules/user/controllers/logout.php
@@ -31,7 +31,7 @@ class Logout_Controller extends Controller {
         // Don't use url::redirect() because it'll call url::site() and munge the continue url.
         header("Location: $continue_url");
       } else {
-        url::redirect("albums/1");
+        url::redirect(item::root()->url());
       }
     }
   }
diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php
index a6522369..8c18916e 100644
--- a/modules/user/controllers/password.php
+++ b/modules/user/controllers/password.php
@@ -130,7 +130,7 @@ class Password_Controller extends Controller {
       $user->hash = null;
       $user->save();
       message::success(t("Password reset successfully"));
-      url::redirect("albums/1");
+      url::redirect(item::root()->url());
     } else {
       print $view;
     }
diff --git a/themes/admin_default/views/admin.html.php b/themes/admin_default/views/admin.html.php
index 9c5f1413..5564484c 100644
--- a/themes/admin_default/views/admin.html.php
+++ b/themes/admin_default/views/admin.html.php
@@ -43,10 +43,10 @@
       <div id="gHeader">
         <?= $theme->admin_header_top() ?>
         <ul id="gLoginMenu">
-          <li class="first"><?= html::anchor("albums/1", "&larr; ".t("Back to the Gallery")) ?></li>
-          <li id="gLogoutLink"><a href="<?= url::site("logout?continue=albums/1&amp;csrf=$csrf") ?>"><?= t("Logout") ?></a></li>
+          <li class="first"><?= html::anchor(item::root()->url(), "&larr; ".t("Back to the Gallery")) ?></li>
+          <li id="gLogoutLink"><a href="<?= url::site("logout?continue=items/1&amp;csrf=$csrf") ?>"><?= t("Logout") ?></a></li>
         </ul>
-        <a id="gLogo" href="<?= url::site("albums/1") ?>" title="<?= t("go back to the Gallery")->for_html_attr() ?>">
+        <a id="gLogo" href="<?= item::root()->url() ?>" title="<?= t("go back to the Gallery")->for_html_attr() ?>">
           &larr; <?= t("back to the ...") ?>
         </a>
         <div id="gSiteAdminMenu" style="display: none;">
diff --git a/themes/default/views/page.html.php b/themes/default/views/page.html.php
index 350ba41c..733534ea 100644
--- a/themes/default/views/page.html.php
+++ b/themes/default/views/page.html.php
@@ -81,7 +81,7 @@
           <? if ($header_text = module::get_var("gallery", "header_text")): ?>
           <?= $header_text ?>
           <? else: ?>
-          <a id="gLogo" href="<?= url::site("albums/1") ?>" title="<?= t("go back to the Gallery home")->for_html_attr() ?>">
+          <a id="gLogo" href="<?= item::root()->url() ?>" title="<?= t("go back to the Gallery home")->for_html_attr() ?>">
             <img width="107" height="48" alt="<?= t("Gallery logo: Your photos on your web site")->for_html_attr() ?>" src="<?= url::file("lib/images/logo.png") ?>" />
           </a>
           <? endif ?>
@@ -95,7 +95,7 @@
         <ul class="gBreadcrumbs">
           <? foreach ($parents as $parent): ?>
           <li>
-            <a href="<?= url::site("albums/{$parent->id}?show={$theme->item()->id}") ?>">
+            <a href="<?= $parent->url("show={$theme->item()->id}") ?>">
               <?= html::purify($parent->title) ?>
             </a>
           </li>
-- 
cgit v1.2.3


From 48becbe017758fb6fb95f0806ece879764249f37 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 8 Sep 2009 20:03:55 -0700
Subject: Update item::convert_filename_to_slug() to eliminate leading and
 trailing hyphens.

---
 modules/gallery/helpers/item.php           | 6 +++++-
 modules/gallery/tests/Item_Helper_Test.php | 5 +++++
 2 files changed, 10 insertions(+), 1 deletion(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index d907a177..e015cb72 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -113,7 +113,11 @@ class item_Core {
    * @param string $filename
    */
   static function convert_filename_to_slug($filename) {
-    return preg_replace("/[^A-Za-z0-9-_]+/", "-", pathinfo($filename, PATHINFO_FILENAME));
+    $result = pathinfo($filename, PATHINFO_FILENAME);
+    $result = preg_replace("/[^A-Za-z0-9-_]+/", "-", $result);
+    $result = preg_replace("/^-+/", "", $result);
+    $result = preg_replace("/-+$/", "", $result);
+    return $result;
   }
 
   /**
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 87859565..33fcdb73 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -49,6 +49,11 @@ class Item_Helper_Test extends Unit_Test_Case {
     $this->assert_equal(1, $input->not_url_safe);
   }
 
+  public function convert_filename_to_slug_test() {
+    $this->assert_equal("foo", item::convert_filename_to_slug("{[foo]}"));
+    $this->assert_equal("foo-bar", item::convert_filename_to_slug("{[foo!@#!$@#^$@($!(@bar]}"));
+  }
+
   private static function _create_random_item($album) {
     // Set all required fields (values are irrelevant)
     $item = ORM::factory("item");
-- 
cgit v1.2.3


From 77bd3d3d40e3a51bce9f41f996222107fc8a7427 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 8 Sep 2009 21:49:09 -0700
Subject: Replace two preg_replace() calls with a single trim() call.

---
 modules/gallery/helpers/item.php | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index e015cb72..3d02d239 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -115,9 +115,7 @@ class item_Core {
   static function convert_filename_to_slug($filename) {
     $result = pathinfo($filename, PATHINFO_FILENAME);
     $result = preg_replace("/[^A-Za-z0-9-_]+/", "-", $result);
-    $result = preg_replace("/^-+/", "", $result);
-    $result = preg_replace("/-+$/", "", $result);
-    return $result;
+    return trim($result, "-");
   }
 
   /**
-- 
cgit v1.2.3


From 0df4b7d7058a9b02faa8ff11993873f7e31a979d Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 8 Sep 2009 21:49:28 -0700
Subject: Fix line > 100 chars.

---
 modules/gallery/helpers/item.php | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 3d02d239..588c08d4 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -129,7 +129,8 @@ class item_Core {
     } else {
       $page_type = "photo";
     }
-    $form = new Forge("quick/delete/$item->id?page_type=$page_type", "", "post", array("id" => "gConfirmDelete"));
+    $form = new Forge(
+      "quick/delete/$item->id?page_type=$page_type", "", "post", array("id" => "gConfirmDelete"));
     $form->hidden("_method")->value("put");
     $group = $form->group("confirm_delete")->label(t("Confirm Deletion"));
     $group->submit("")->value(t("Delete"));
-- 
cgit v1.2.3