From 653c291d94f02e3e292541fe39d9fc95bf3d22ba Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 29 Jul 2009 10:55:56 -0700 Subject: Fix for ticket #576 Add a weight index to the item table and changed the retrieval of the maximum weight to select weight from items order by weight desc limit 1. Upgrades the gallery module to version 10 --- modules/gallery/models/item.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index dcbee991..481b22bc 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -350,9 +350,21 @@ class Item_Model extends ORM_MPTT { if (!empty($this->changed) && $this->changed != array("view_count" => "view_count")) { $this->updated = time(); if (!$this->loaded) { + try { $this->created = $this->updated; - $r = ORM::factory("item")->select("MAX(weight) as max_weight")->find(); - $this->weight = $r->max_weight + 1; + Kohana::log("error", "get Weight"); + $weight = ORM::factory("item") + ->select("weight") + ->orderby("weight", "DESC") + ->limit(1) + ->find_all() + ->current()->weight; + Kohana::log("error", "Weight: $weight"); + $this->weight = $weight + 1; + } catch (Exception $e) { + Kohana::log("error", $e->__toString()); + throw $e; + } } else { $send_event = 1; } -- cgit v1.2.3 From fc7ef17e0fb08459a84ba9ba0654664e519e6a0f Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 30 Jul 2009 05:27:19 -0700 Subject: Remove try statement w/o catch --- modules/gallery/models/item.php | 1 - 1 file changed, 1 deletion(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 1ce4d340..a0598ea4 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -350,7 +350,6 @@ class Item_Model extends ORM_MPTT { if (!empty($this->changed) && $this->changed != array("view_count" => "view_count")) { $this->updated = time(); if (!$this->loaded) { - try { $this->created = $this->updated; $weight = Database::instance() ->select("weight")->from("items") -- cgit v1.2.3 From 187d4b209d6bd3fce2fa81a88e31f587d9eb4624 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 5 Aug 2009 07:38:35 -0700 Subject: Change the children methods on Item_Core and ORM_MPTT in order to specify a type parameter, so tht we can filter the children based on type (i.e. album, photo, etc). In addition, expose the sort order, so that we can specify the order we want to return the children. --- modules/gallery/libraries/ORM_MPTT.php | 16 +++++++++++----- modules/gallery/models/item.php | 9 +++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index 1917d738..cde049cd 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -146,11 +146,15 @@ class ORM_MPTT_Core extends ORM { * @chainable * @param integer SQL limit * @param integer SQL offset + * @param string type to return * @param array orderby * @return array ORM */ - function children($limit=null, $offset=0, $orderby=null) { + function children($limit=null, $offset=0, $type=null, $orderby=null) { $this->where("parent_id", $this->id); + if ($type) { + $this->where("type", $type); + } if (empty($orderby)) { $this->orderby("id", "ASC"); } else { @@ -163,16 +167,18 @@ class ORM_MPTT_Core extends ORM { * Return all of the children of this node, ordered by id. * * @chainable - * @param integer SQL limit - * @param integer SQL offset + * @param string type to return * @return array ORM */ - function children_count() { + function children_count($type=null) { + if ($type) { + $this->where("type", $type); + } return $this->where("parent_id", $this->id)->count_all(); } /** - * Return all of the children of the specified type, ordered by id. + * Return all of the decendents of the specified type, ordered by id. * * @param integer SQL limit * @param integer SQL offset diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index f3e6b8f3..3498e0db 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -526,10 +526,15 @@ class Item_Model extends ORM_MPTT { * @chainable * @param integer SQL limit * @param integer SQL offset + * @param string type to return + * @param array orderby * @return array ORM */ - function children($limit=null, $offset=0) { - return parent::children($limit, $offset, array($this->sort_column => $this->sort_order)); + function children($limit=null, $offset=0, $type=null, $orderby=null) { + if (empty($orderby)) { + $orderby = array($this->sort_column => $this->sort_order); + } + return parent::children($limit, $offset, $type, $orderby); } /** -- cgit v1.2.3 From e8c57290a2e98e3cbb7cf47875e6e5dae2e41fa2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 5 Aug 2009 10:38:53 -0700 Subject: Change the children and descendants APIs to be more consistent and to remove Gallery3 concepts from ORM_MPTT. The following API methods: ORM_MPTT::children ORM_MPTT::children_count ORM_MPTT::descendants ORM_MPTT::descendants_count All now take a $where clause that allow you to pass through additional field parameters. old API: $album->children(10, 0, "photos") $album->children_count("photos") new API: $album->children(10, 0, array("type" => "photos")) $album->children_count(array("type" => "photos")) This gives us a more flexible API and simplifies the code. While I was in there, I changed the way we deal with default orderby values so that we just assign the default value in the function definition, which allows us to get rid of all conditionals in the implementation which results in simpler code. --- modules/gallery/helpers/gallery_rss.php | 5 ++- modules/gallery/libraries/ORM_MPTT.php | 69 +++++++++++++-------------------- modules/gallery/models/item.php | 25 +++++++----- modules/gallery/tests/ORM_MPTT_Test.php | 8 ++-- 4 files changed, 51 insertions(+), 56 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/gallery_rss.php b/modules/gallery/helpers/gallery_rss.php index 7daf6170..8e887368 100644 --- a/modules/gallery/helpers/gallery_rss.php +++ b/modules/gallery/helpers/gallery_rss.php @@ -50,8 +50,9 @@ class gallery_rss_Core { $feed->children = $item ->viewable() - ->descendants($limit, $offset, "photo"); - $feed->max_pages = ceil($item->viewable()->descendants_count("photo") / $limit); + ->descendants($limit, $offset, array("type" => "photo")); + $feed->max_pages = ceil( + $item->viewable()->descendants_count(array("type" => "photo")) / $limit); $feed->title = p::purify($item->title); $feed->link = url::abs_site("albums/{$item->id}"); $feed->description = nl2br(p::purify($item->description)); diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index cde049cd..9d716d8b 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -146,35 +146,30 @@ class ORM_MPTT_Core extends ORM { * @chainable * @param integer SQL limit * @param integer SQL offset - * @param string type to return + * @param array additional where clauses * @param array orderby * @return array ORM */ - function children($limit=null, $offset=0, $type=null, $orderby=null) { - $this->where("parent_id", $this->id); - if ($type) { - $this->where("type", $type); - } - if (empty($orderby)) { - $this->orderby("id", "ASC"); - } else { - $this->orderby($orderby); - } - return $this->find_all($limit, $offset); + function children($limit=null, $offset=0, $where=array(), $orderby=array("id", "ASC")) { + return $this + ->where("parent_id", $this->id) + ->where($where) + ->orderby($orderby) + ->find_all($limit, $offset); } /** * Return all of the children of this node, ordered by id. * * @chainable - * @param string type to return + * @param array additional where clauses * @return array ORM */ - function children_count($type=null) { - if ($type) { - $this->where("type", $type); - } - return $this->where("parent_id", $this->id)->count_all(); + function children_count($where=array()) { + return $this + ->where($where) + ->where("parent_id", $this->id) + ->count_all(); } /** @@ -182,39 +177,31 @@ class ORM_MPTT_Core extends ORM { * * @param integer SQL limit * @param integer SQL offset - * @param string type to return + * @param array additional where clauses * @param array orderby * @return object ORM_Iterator */ - function descendants($limit=null, $offset=0, $type=null, $orderby=null) { - $this->where("left_ptr >", $this->left_ptr) - ->where("right_ptr <=", $this->right_ptr); - if ($type) { - $this->where("type", $type); - } - - if (empty($orderby)) { - $this->orderby("id", "ASC"); - } else { - $this->orderby($orderby); - } - - return $this->find_all($limit, $offset); + function descendants($limit=null, $offset=0, $where=array(), $orderby=array("id", "ASC")) { + return $this + ->where("left_ptr >", $this->left_ptr) + ->where("right_ptr <=", $this->right_ptr) + ->where($where) + ->orderby($orderby) + ->find_all($limit, $offset); } /** * Return the count of all the children of the specified type. * - * @param string type to count + * @param array additional where clauses * @return integer child count */ - function descendants_count($type=null) { - $this->where("left_ptr >", $this->left_ptr) - ->where("right_ptr <=", $this->right_ptr); - if ($type) { - $this->where("type", $type); - } - return $this->count_all(); + function descendants_count($where=array()) { + return $this + ->where("left_ptr >", $this->left_ptr) + ->where("right_ptr <=", $this->right_ptr) + ->where($where) + ->count_all(); } /** diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 3498e0db..c4b9826f 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -521,31 +521,38 @@ class Item_Model extends ORM_MPTT { } /** - * Return all of the children of this node, ordered by the defined sort order. + * Return all of the children of this album. Unless you specify a specific sort order, the + * results will be ordered by this album's sort order. * * @chainable * @param integer SQL limit * @param integer SQL offset - * @param string type to return + * @param array additional where clauses * @param array orderby * @return array ORM */ - function children($limit=null, $offset=0, $type=null, $orderby=null) { + function children($limit=null, $offset=0, $where=array(), $orderby=null) { if (empty($orderby)) { $orderby = array($this->sort_column => $this->sort_order); } - return parent::children($limit, $offset, $type, $orderby); + return parent::children($limit, $offset, $where, $orderby); } /** - * Return all of the children of the specified type, ordered by the defined sort order. + * Return the children of this album, and all of it's sub-albums. Unless you specify a specific + * sort order, the results will be ordered by this album's sort order. Note that this + * album's sort order is imposed on all sub-albums, regardless of their sort order. + * + * @chainable * @param integer SQL limit * @param integer SQL offset - * @param string type to return + * @param array additional where clauses * @return object ORM_Iterator */ - function descendants($limit=null, $offset=0, $type=null) { - return parent::descendants($limit, $offset, $type, - array($this->sort_column => $this->sort_order)); + function descendants($limit=null, $offset=0, $where=array(), $orderby=null) { + if (empty($orderby)) { + $orderby = array($this->sort_column => $this->sort_order); + } + return parent::descendants($limit, $offset, $where, $orderby); } } diff --git a/modules/gallery/tests/ORM_MPTT_Test.php b/modules/gallery/tests/ORM_MPTT_Test.php index 943810c3..f77f1f34 100644 --- a/modules/gallery/tests/ORM_MPTT_Test.php +++ b/modules/gallery/tests/ORM_MPTT_Test.php @@ -177,8 +177,8 @@ class ORM_MPTT_Test extends Unit_Test_Case { $parent->reload(); $this->assert_equal(3, $parent->descendants()->count()); - $this->assert_equal(2, $parent->descendants(null, 0, "photo")->count()); - $this->assert_equal(1, $parent->descendants(null, 0, "album")->count()); + $this->assert_equal(2, $parent->descendants(null, 0, array("type" => "photo"))->count()); + $this->assert_equal(1, $parent->descendants(null, 0, array("type" => "album"))->count()); } public function descendant_limit_test() { @@ -215,7 +215,7 @@ class ORM_MPTT_Test extends Unit_Test_Case { $parent->reload(); $this->assert_equal(3, $parent->descendants_count()); - $this->assert_equal(2, $parent->descendants_count("photo")); - $this->assert_equal(1, $parent->descendants_count("album")); + $this->assert_equal(2, $parent->descendants_count(array("type" => "photo"))); + $this->assert_equal(1, $parent->descendants_count(array("type" => "album"))); } } -- cgit v1.2.3 From 24d7f5df8c8a18b9ac017fb7b4fc92974b5435cc Mon Sep 17 00:00:00 2001 From: Tim Almdal 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 --- modules/gallery/helpers/item.php | 14 ++++++++++++++ modules/gallery/models/item.php | 9 +-------- 2 files changed, 15 insertions(+), 8 deletions(-) (limited to 'modules/gallery/models') 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 38b2efc44cf3345d97798e9637db241b05e2dded Mon Sep 17 00:00:00 2001 From: Tim Almdal 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/models') 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 @@ +_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