From c80d2da0a95a63b76f5a4c835f1a0e1022ec2f53 Mon Sep 17 00:00:00 2001 From: Romain LE DISEZ Date: Tue, 23 Jun 2009 10:09:46 +0200 Subject: Remove an useless ORDER BY. It improves compatibility with PgSQL. --- modules/gallery/models/item.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 7dce9e51..44d79d5b 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -382,8 +382,7 @@ class Item_Model extends ORM_MPTT { SELECT COUNT(*) AS position FROM {items} WHERE parent_id = {$this->id} AND {$this->sort_column} <= (SELECT {$this->sort_column} - FROM {items} WHERE id = $child_id) - ORDER BY {$this->sort_column} {$this->sort_order}"); + FROM {items} WHERE id = $child_id)"); return $result->current()->position; } -- cgit v1.2.3 From be354ab98bbb08abad0a7efd6fb1d110e11381de Mon Sep 17 00:00:00 2001 From: Romain LE DISEZ Date: Wed, 24 Jun 2009 23:40:47 +0200 Subject: Remove an useless ORDER BY. It improves compatibility with PgSQL. (Replay of c80d2da0a95a63b76f5a4c835f1a0e1022ec2f53) --- modules/gallery/models/item.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 430119b5..7dbbbcb1 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -389,8 +389,7 @@ class Item_Model extends ORM_MPTT { SELECT COUNT(*) AS position FROM {items} WHERE parent_id = {$this->id} AND `{$this->sort_column}` $comp (SELECT `{$this->sort_column}` - FROM {items} WHERE id = $child_id) - ORDER BY `{$this->sort_column}` {$this->sort_order}")->current()->position; + FROM {items} WHERE id = $child_id)")->current()->position; // We stopped short of our target value in the sort (notice that we're using a < comparator // above) because it's possible that we have duplicate values in the sort column. An -- cgit v1.2.3 From 5b3b675b6d8a1cd9a5f2b9455c551791e18d88ff Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 16 Jul 2009 11:19:34 -0700 Subject: Non-trivial changes to the event handling code: 1) The item_updated event no longer takes the old and new items. Instead we overload ORM to track the original data and make that available via the item. This will allow us to move event publishing down into the API methods which in turn will give us more stability since we won't require each controller to remember to do it. 2) ORM class now tracks the original values. It doesn't track the original relationships (no need for that, yet) 3) Added new events: item_deleted group_deleted user_deleted --- modules/akismet/helpers/akismet_event.php | 6 ++--- modules/comment/controllers/admin_comments.php | 4 ++-- modules/comment/helpers/comment_event.php | 2 +- modules/exif/helpers/exif_event.php | 2 +- modules/g2_import/helpers/g2_import_event.php | 2 +- modules/gallery/controllers/albums.php | 2 +- modules/gallery/controllers/movies.php | 2 +- modules/gallery/controllers/photos.php | 2 +- modules/gallery/helpers/gallery_event.php | 4 ++-- modules/gallery/libraries/MY_ORM.php | 27 +++++++++++++++++++++- modules/gallery/models/item.php | 3 +++ modules/gallery/tests/Item_Model_Test.php | 16 +++++++++++++ modules/notification/helpers/notification.php | 19 ++++++++------- .../notification/helpers/notification_event.php | 10 ++++---- modules/notification/helpers/notification_menu.php | 2 +- modules/notification/views/item_updated.html.php | 16 ++++++------- modules/organize/controllers/organize.php | 4 ++-- modules/search/helpers/search_event.php | 6 ++--- modules/tag/helpers/tag_event.php | 2 +- modules/user/models/group.php | 2 ++ modules/user/models/user.php | 2 ++ 21 files changed, 91 insertions(+), 44 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/akismet/helpers/akismet_event.php b/modules/akismet/helpers/akismet_event.php index 80fe0127..bffc0fd7 100644 --- a/modules/akismet/helpers/akismet_event.php +++ b/modules/akismet/helpers/akismet_event.php @@ -40,14 +40,14 @@ class akismet_event_Core { $comment->save(); } - static function comment_updated($old, $new) { + static function comment_updated($comment) { if (!module::get_var("akismet", "api_key")) { return; } - if ($old->state != "spam" && $new->state == "spam") { + if ($comment->original("state") != "spam" && $comment->state == "spam") { akismet::submit_spam($new); - } else if ($old->state == "spam" && $new->state != "spam") { + } else if ($comment->original("state") == "spam" && $comment->state != "spam") { akismet::submit_ham($new); } } diff --git a/modules/comment/controllers/admin_comments.php b/modules/comment/controllers/admin_comments.php index 3e8d3c46..ea76b188 100644 --- a/modules/comment/controllers/admin_comments.php +++ b/modules/comment/controllers/admin_comments.php @@ -113,8 +113,8 @@ class Admin_Comments_Controller extends Admin_Controller { if ($comment->loaded) { $comment->state = $state; $comment->save(); - module::event("comment_updated", $orig, $comment); - if ($orig->state == "published" || $comment->state == "published") { + module::event("comment_updated", $comment); + if ($comment->original("state") == "published" || $comment->state == "published") { module::event("item_related_update", $comment->item()); } } diff --git a/modules/comment/helpers/comment_event.php b/modules/comment/helpers/comment_event.php index a3beb27a..3850a001 100644 --- a/modules/comment/helpers/comment_event.php +++ b/modules/comment/helpers/comment_event.php @@ -18,7 +18,7 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class comment_event_Core { - static function item_before_delete($item) { + static function item_deleted($item) { Database::instance()->delete("comments", array("item_id" => $item->id)); } } diff --git a/modules/exif/helpers/exif_event.php b/modules/exif/helpers/exif_event.php index f5677653..24243f4d 100644 --- a/modules/exif/helpers/exif_event.php +++ b/modules/exif/helpers/exif_event.php @@ -22,7 +22,7 @@ class exif_event_Core { exif::extract($item); } - static function item_before_delete($item) { + static function item_deleted($item) { Database::instance()->delete("exif_records", array("item_id" => $item->id)); } } diff --git a/modules/g2_import/helpers/g2_import_event.php b/modules/g2_import/helpers/g2_import_event.php index 13f5b1a0..77b489a7 100644 --- a/modules/g2_import/helpers/g2_import_event.php +++ b/modules/g2_import/helpers/g2_import_event.php @@ -18,7 +18,7 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class g2_import_event_Core { - static function item_before_delete($item) { + static function item_deleted($item) { Database::instance()->delete("g2_maps", array("g3_id" => $item->id)); } diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index e6d01b90..c378e3ce 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -192,7 +192,7 @@ class Albums_Controller extends Items_Controller { } $album->save(); - module::event("item_updated", $orig, $album); + module::event("item_updated", $album); log::success("content", "Updated album", "id\">view"); message::success( diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 30a5d78c..fc511082 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -91,7 +91,7 @@ class Movies_Controller extends Items_Controller { $photo->rename($form->edit_photo->filename->value); $photo->save(); - module::event("item_updated", $orig, $photo); + module::event("item_updated", $photo); log::success("content", "Updated photo", "id\">view"); message::success( diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 6a62e859..77627009 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -84,7 +84,7 @@ class Photos_Controller extends Items_Controller { $photo->rename($form->edit_photo->filename->value); $photo->save(); - module::event("item_updated", $orig, $photo); + module::event("item_updated", $photo); log::success("content", "Updated photo", "id\">view"); message::success( diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index aa11b7c0..2f3a64d3 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -23,7 +23,7 @@ class gallery_event_Core { access::add_group($group); } - static function group_before_delete($group) { + static function group_deleted($group) { access::delete_group($group); } @@ -31,7 +31,7 @@ class gallery_event_Core { access::add_item($item); } - static function item_before_delete($item) { + static function item_deleted($item) { access::delete_item($item); } diff --git a/modules/gallery/libraries/MY_ORM.php b/modules/gallery/libraries/MY_ORM.php index 2bd9b4eb..319cbe09 100644 --- a/modules/gallery/libraries/MY_ORM.php +++ b/modules/gallery/libraries/MY_ORM.php @@ -18,6 +18,9 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class ORM extends ORM_Core { + // Track the original value of this ORM instance so that we can look it up in ORM::original() + protected $original = null; + public function open_paren() { $this->db->open_paren(); return $this; @@ -30,7 +33,29 @@ class ORM extends ORM_Core { public function save() { model_cache::clear($this->object_name, $this->{$this->primary_key}, $this->primary_key); - return parent::save(); + $result = parent::save(); + $this->original = $this->object; + return $result; + } + + public function __set($column, $value) { + if (!isset($this->original)) { + $this->original = $this->object; + } + + return parent::__set($column, $value); + } + + public function __unset($column) { + if (!isset($this->original)) { + $this->original = $this->object; + } + + return parent::__unset($column); + } + + public function original($column) { + return $this->original[$column]; } } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 51037073..80f19d26 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -93,6 +93,7 @@ class Item_Model extends ORM_MPTT { } public function delete() { + $old = clone $this; module::event("item_before_delete", $this); $parent = $this->parent(); @@ -114,6 +115,8 @@ class Item_Model extends ORM_MPTT { @unlink($resize_path); @unlink($thumb_path); } + + module::event("item_deleted", $old); } /** diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php index 615b8997..a21cdc13 100644 --- a/modules/gallery/tests/Item_Model_Test.php +++ b/modules/gallery/tests/Item_Model_Test.php @@ -140,4 +140,20 @@ class Item_Model_Test extends Unit_Test_Case { } $this->assert_false(true, "Item_Model::rename should not accept / characters"); } + + public function save_original_values_test() { + print "START\n"; + $item = $this->create_random_item(); + $item->title = "ORIGINAL_VALUE"; + $item->save(); + + print "CHANGE\n"; + $item->title = "NEW_VALUE"; + + //printf("
%s
",print_r($item,1));flush(); + + print "COMPARE\n"; + $this->assert_same("ORIGINAL_VALUE", $item->original("title")); + $this->assert_same("NEW_VALUE", $item->title); + } } diff --git a/modules/notification/helpers/notification.php b/modules/notification/helpers/notification.php index 8ee0c6ba..e246af2c 100644 --- a/modules/notification/helpers/notification.php +++ b/modules/notification/helpers/notification.php @@ -82,17 +82,16 @@ class notification { return $subscribers; } - static function send_item_updated($old, $new) { + static function send_item_updated($item) { $v = new View("item_updated.html"); - $v->old = $old; - $v->new = $new; - $v->subject = $old->is_album() ? - t("Album %title updated", array("title" => $old->title)) : - ($old->is_photo() ? - t("Photo %title updated", array("title" => $old->title)) - : t("Movie %title updated", array("title" => $old->title))); - - self::_notify_subscribers($old, $v->render(), $v->subject); + $v->item = $item; + $v->subject = $item->is_album() ? + t("Album %title updated", array("title" => $item->original("title"))) : + ($item->is_photo() ? + t("Photo %title updated", array("title" => $item->original("title"))) + : t("Movie %title updated", array("title" => $item->original("title")))); + + self::_notify_subscribers($item, $v->render(), $v->subject); } static function send_item_add($item) { diff --git a/modules/notification/helpers/notification_event.php b/modules/notification/helpers/notification_event.php index 1cf9ff58..536557c6 100644 --- a/modules/notification/helpers/notification_event.php +++ b/modules/notification/helpers/notification_event.php @@ -18,15 +18,15 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class notification_event_Core { - static function item_updated($old, $new) { - notification::send_item_updated($old, $new); + static function item_updated($item) { + notification::send_item_updated($item); } static function item_created($item) { notification::send_item_add($item); } - static function item_before_delete($item) { + static function item_deleted($item) { notification::send_item_deleted($item); if (notification::is_watching($item)) { @@ -40,8 +40,8 @@ class notification_event_Core { } } - static function comment_updated($old, $new) { - if ($new->state == "published" && $old->state != "published") { + static function comment_updated($item) { + if ($item->state == "published" && $item->original("state") != "published") { notification::send_comment_published($new); } } diff --git a/modules/notification/helpers/notification_menu.php b/modules/notification/helpers/notification_menu.php index 696aad62..87478b8a 100644 --- a/modules/notification/helpers/notification_menu.php +++ b/modules/notification/helpers/notification_menu.php @@ -21,7 +21,7 @@ class notification_menu_Core { static function site($menu, $theme) { if (!user::active()->guest) { $item = $theme->item(); - + if ($item && $item->is_album()) { $watching = notification::is_watching($item); diff --git a/modules/notification/views/item_updated.html.php b/modules/notification/views/item_updated.html.php index 0620c50c..39f9113b 100644 --- a/modules/notification/views/item_updated.html.php +++ b/modules/notification/views/item_updated.html.php @@ -7,27 +7,27 @@

- title != $new->title): ?> + original("title") != $item->title): ?> - + - + - + - description != $new->description): ?> + original("description") != $item->description): ?> - + - description)): ?> + description)): ?> - +
title) ?>title) ?> title) ?>title) ?>
url(array(), true) ?>url(array(), true) ?>
description) ?>description) ?>
description) ?>description) ?>
diff --git a/modules/organize/controllers/organize.php b/modules/organize/controllers/organize.php index 6792573d..54e04071 100644 --- a/modules/organize/controllers/organize.php +++ b/modules/organize/controllers/organize.php @@ -279,7 +279,7 @@ class Organize_Controller extends Controller { $item->rename($form->dirname->value); $item->save(); - module::event("item_updated", $orig, $item); + module::event("item_updated", $item); if ($item->is_album()) { log::success("content", "Updated album", "id\">view"); @@ -322,7 +322,7 @@ class Organize_Controller extends Controller { $item->sort_order = $form->direction->value; $item->save(); - module::event("item_updated", $orig, $item); + module::event("item_updated", $item); log::success("content", "Updated album", "id\">view"); $message = t("Saved album %album_title", array("album_title" => p::purify($item->title))); diff --git a/modules/search/helpers/search_event.php b/modules/search/helpers/search_event.php index b9657395..764fdd18 100644 --- a/modules/search/helpers/search_event.php +++ b/modules/search/helpers/search_event.php @@ -22,11 +22,11 @@ class search_event_Core { search::update($item); } - static function item_updated($old_item, $new_item) { - search::update($new_item); + static function item_updated($item) { + search::update($item); } - static function item_before_delete($item) { + static function item_deleted($item) { ORM::factory("search_record") ->where("item_id", $item->id) ->delete_all(); diff --git a/modules/tag/helpers/tag_event.php b/modules/tag/helpers/tag_event.php index 7a170bf8..0164f556 100644 --- a/modules/tag/helpers/tag_event.php +++ b/modules/tag/helpers/tag_event.php @@ -59,7 +59,7 @@ class tag_event_Core { return; } - static function item_before_delete($item) { + static function item_deleted($item) { $db = Database::instance(); $db->query("UPDATE {tags} SET `count` = `count` - 1 WHERE `count` > 0 " . "AND `id` IN (SELECT `tag_id` from {items_tags} WHERE `item_id` = $item->id)"); diff --git a/modules/user/models/group.php b/modules/user/models/group.php index 45948887..e0724e30 100644 --- a/modules/user/models/group.php +++ b/modules/user/models/group.php @@ -27,7 +27,9 @@ class Group_Model extends ORM { * @see ORM::delete() */ public function delete($id=null) { + $old = clone $this; module::event("group_before_delete", $this); parent::delete($id); + module::event("group_deleted", $old); } } \ No newline at end of file diff --git a/modules/user/models/user.php b/modules/user/models/user.php index b447892e..e3260270 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -44,8 +44,10 @@ class User_Model extends ORM { * @see ORM::delete() */ public function delete($id=null) { + $old = clone $this; module::event("user_before_delete", $this); parent::delete($id); + module::event("user_deleted", $old); } /** -- cgit v1.2.3 From 0f766b149d0cee7af664f2321fddc6f04cda70ac Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 16 Jul 2009 12:29:16 -0700 Subject: Second non-trivial change to the event code. We now publish model related events from within the model handling code. The only exception to this currently is item_created which is challenging because we have to save the item using ORM_MPTT::add_to_parent() before the object itself is fully set up. When we get that down to one call to save() we can publish that event from within the model also. --- modules/comment/controllers/admin_comments.php | 4 ---- modules/comment/controllers/comments.php | 1 - modules/comment/helpers/comment.php | 5 ----- modules/comment/models/comment.php | 17 ++++++++++++++++- modules/exif/helpers/exif_event.php | 4 +++- modules/gallery/controllers/albums.php | 3 --- modules/gallery/controllers/movies.php | 3 --- modules/gallery/controllers/photos.php | 3 --- modules/gallery/helpers/album.php | 2 ++ modules/gallery/helpers/movie.php | 2 ++ modules/gallery/helpers/photo.php | 2 ++ modules/gallery/models/item.php | 7 ++++++- modules/organize/controllers/organize.php | 4 ---- modules/user/helpers/group.php | 1 - modules/user/helpers/user.php | 1 - modules/user/models/group.php | 13 +++++++++++++ modules/user/models/user.php | 13 +++++++++++++ 17 files changed, 57 insertions(+), 28 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/comment/controllers/admin_comments.php b/modules/comment/controllers/admin_comments.php index ea76b188..a164f79f 100644 --- a/modules/comment/controllers/admin_comments.php +++ b/modules/comment/controllers/admin_comments.php @@ -113,10 +113,6 @@ class Admin_Comments_Controller extends Admin_Controller { if ($comment->loaded) { $comment->state = $state; $comment->save(); - module::event("comment_updated", $comment); - if ($comment->original("state") == "published" || $comment->state == "published") { - module::event("item_related_update", $comment->item()); - } } } diff --git a/modules/comment/controllers/comments.php b/modules/comment/controllers/comments.php index 02c38491..9fb4796e 100644 --- a/modules/comment/controllers/comments.php +++ b/modules/comment/controllers/comments.php @@ -152,7 +152,6 @@ class Comments_Controller extends REST_Controller { $comment->url = $form->edit_comment->url->value; $comment->text = $form->edit_comment->text->value; $comment->save(); - module::event("comment_updated", $comment); print json_encode( array("result" => "success", diff --git a/modules/comment/helpers/comment.php b/modules/comment/helpers/comment.php index 08cba096..3d743325 100644 --- a/modules/comment/helpers/comment.php +++ b/modules/comment/helpers/comment.php @@ -61,11 +61,6 @@ class comment_Core { $comment->server_remote_port = substr($input->server("REMOTE_PORT"), 0, 16); $comment->save(); - module::event("comment_created", $comment); - if ($comment->state == "published") { - module::event("item_related_update", $comment->item()); - } - return $comment; } diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php index 22c465df..551fb245 100644 --- a/modules/comment/models/comment.php +++ b/modules/comment/models/comment.php @@ -61,8 +61,23 @@ class Comment_Model extends ORM { $this->updated = time(); if (!$this->loaded && empty($this->created)) { $this->created = $this->updated; + $created = true; } } - return parent::save(); + parent::save(); + + if (isset($created)) { + module::event("comment_created", $this); + } else { + module::event("comment_updated", $this); + } + + // We only notify on the related items if we're making a visible change, which means moving in + // or out of a published state + if ($this->original("state") == "published" || $this->state == "published") { + module::event("item_related_update", $this->item()); + } + + return $this; } } diff --git a/modules/exif/helpers/exif_event.php b/modules/exif/helpers/exif_event.php index 24243f4d..826ec959 100644 --- a/modules/exif/helpers/exif_event.php +++ b/modules/exif/helpers/exif_event.php @@ -19,7 +19,9 @@ */ class exif_event_Core { static function item_created($item) { - exif::extract($item); + if (!$item->is_album()) { + exif::extract($item); + } } static function item_deleted($item) { diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index c378e3ce..9980b676 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -182,7 +182,6 @@ class Albums_Controller extends Items_Controller { } if ($valid) { - $orig = clone $album; $album->title = $form->edit_album->title->value; $album->description = $form->edit_album->description->value; $album->sort_column = $form->edit_album->sort_order->column->value; @@ -192,8 +191,6 @@ class Albums_Controller extends Items_Controller { } $album->save(); - module::event("item_updated", $album); - log::success("content", "Updated album", "id\">view"); message::success( t("Saved album %album_title", array("album_title" => p::clean($album->title)))); diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index fc511082..d954ad8d 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -85,14 +85,11 @@ class Movies_Controller extends Items_Controller { } if ($valid) { - $orig = clone $photo; $photo->title = $form->edit_photo->title->value; $photo->description = $form->edit_photo->description->value; $photo->rename($form->edit_photo->filename->value); $photo->save(); - module::event("item_updated", $photo); - log::success("content", "Updated photo", "id\">view"); message::success( t("Saved photo %photo_title", array("photo_title" => p::clean($photo->title)))); diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 77627009..9ce6ed23 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -78,14 +78,11 @@ class Photos_Controller extends Items_Controller { } if ($valid) { - $orig = clone $photo; $photo->title = $form->edit_photo->title->value; $photo->description = $form->edit_photo->description->value; $photo->rename($form->edit_photo->filename->value); $photo->save(); - module::event("item_updated", $photo); - log::success("content", "Updated photo", "id\">view"); message::success( t("Saved photo %photo_title", array("photo_title" => p::clean($photo->title)))); diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 1197f243..f1a6c060 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -71,6 +71,8 @@ class album_Core { mkdir(dirname($album->thumb_path())); mkdir(dirname($album->resize_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", $album); return $album; diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index d62ead76..4f4169d5 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -102,6 +102,8 @@ class movie_Core { copy($filename, $movie->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", $movie); // Build our thumbnail diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index e8a4f357..ce964c14 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -105,6 +105,8 @@ class photo_Core { 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 diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 80f19d26..94e2fcf7 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -350,9 +350,14 @@ class Item_Model extends ORM_MPTT { $this->created = $this->updated; $r = ORM::factory("item")->select("MAX(weight) as max_weight")->find(); $this->weight = $r->max_weight + 1; + $created = 1; } } - return parent::save(); + parent::save(); + if (!isset($created)) { + module::event("item_updated", $this); + } + return $this; } /** diff --git a/modules/organize/controllers/organize.php b/modules/organize/controllers/organize.php index 54e04071..27852904 100644 --- a/modules/organize/controllers/organize.php +++ b/modules/organize/controllers/organize.php @@ -279,8 +279,6 @@ class Organize_Controller extends Controller { $item->rename($form->dirname->value); $item->save(); - module::event("item_updated", $item); - if ($item->is_album()) { log::success("content", "Updated album", "id\">view"); $message = t("Saved album %album_title", array("album_title" => p::purify($item->title))); @@ -322,8 +320,6 @@ class Organize_Controller extends Controller { $item->sort_order = $form->direction->value; $item->save(); - module::event("item_updated", $item); - log::success("content", "Updated album", "id\">view"); $message = t("Saved album %album_title", array("album_title" => p::purify($item->title))); print json_encode(array("form" => $form->__toString(), "message" => $message)); diff --git a/modules/user/helpers/group.php b/modules/user/helpers/group.php index 1dace840..04e6efd6 100644 --- a/modules/user/helpers/group.php +++ b/modules/user/helpers/group.php @@ -39,7 +39,6 @@ class group_Core { $group->name = $name; $group->save(); - module::event("group_created", $group); return $group; } diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php index a59588f8..4105d745 100644 --- a/modules/user/helpers/user.php +++ b/modules/user/helpers/user.php @@ -202,7 +202,6 @@ class user_Core { $user->add(group::registered_users()); $user->save(); - module::event("user_created", $user); return $user; } diff --git a/modules/user/models/group.php b/modules/user/models/group.php index e0724e30..bb3fb58b 100644 --- a/modules/user/models/group.php +++ b/modules/user/models/group.php @@ -32,4 +32,17 @@ class Group_Model extends ORM { parent::delete($id); module::event("group_deleted", $old); } + + public function save() { + if (!$this->loaded) { + $created = 1; + } + parent::save(); + if (isset($created)) { + module::event("group_created", $this); + } else { + module::event("group_updated", $this); + } + return $this; + } } \ No newline at end of file diff --git a/modules/user/models/user.php b/modules/user/models/user.php index e3260270..0234f186 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -59,4 +59,17 @@ class User_Model extends ORM { return sprintf("http://www.gravatar.com/avatar/%s.jpg?s=%d&r=pg%s", md5($this->email), $size, $default ? "&d=" . urlencode($default) : ""); } + + public function save() { + if (!$this->loaded) { + $created = 1; + } + parent::save(); + if (isset($created)) { + module::event("user_created", $this); + } else { + module::event("user_updated", $this); + } + return $this; + } } \ No newline at end of file -- cgit v1.2.3 From 8a6556b30bc34d69284df6246f4010a8835f3bc2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 17 Jul 2009 08:14:08 -0700 Subject: Fix a bug where moved items don't properly inherit permissions from their new target. After each move, recalculate the permissions for the new parent's hierarchy. Fixes ticket #552 --- modules/gallery/helpers/access.php | 15 ++++++++++++ modules/gallery/helpers/gallery_event.php | 4 ++++ modules/gallery/models/item.php | 6 +++-- modules/gallery/tests/Access_Helper_Test.php | 36 ++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index c48f0b79..5dd1e465 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -243,6 +243,21 @@ class access_Core { self::_set($group, $perm_name, $item, null); } + /** + * Recalculate the permissions for a given item and its hierarchy. $item must be an album. + */ + static function recalculate_permissions($item) { + foreach (self::_get_all_groups() as $group) { + foreach (ORM::factory("permission")->find_all() as $perm) { + if ($perm->name == "view") { + self::_update_access_view_cache($group, $item); + } else { + self::_update_access_non_view_cache($group, $perm->name, $item); + } + } + } + } + /** * Register a permission so that modules can use it. * diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 2f3a64d3..1cd96372 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -35,6 +35,10 @@ class gallery_event_Core { access::delete_item($item); } + static function item_moved($item, $old_parent) { + access::recalculate_permissions($item->parent()); + } + static function user_login($user) { // If this user is an admin, check to see if there are any post-install tasks that we need // to run and take care of those now. diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 94e2fcf7..6512e9e5 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -122,8 +122,8 @@ class Item_Model extends ORM_MPTT { /** * Move this item to the specified target. * @chainable - * @param Item_Model $target Target item (must be an album - * @return ORM_MTPP + * @param Item_Model $target Target item (must be an album) + * @return ORM_MPTT */ function move_to($target) { if (!$target->is_album()) { @@ -137,6 +137,7 @@ class Item_Model extends ORM_MPTT { $original_path = $this->file_path(); $original_resize_path = $this->resize_path(); $original_thumb_path = $this->thumb_path(); + $original_parent = $this->parent(); parent::move_to($target, true); $this->relative_path_cache = null; @@ -154,6 +155,7 @@ class Item_Model extends ORM_MPTT { @rename($original_thumb_path, $this->thumb_path()); } + module::event("item_moved", $this, $original_parent); return $this; } diff --git a/modules/gallery/tests/Access_Helper_Test.php b/modules/gallery/tests/Access_Helper_Test.php index d71bf971..1352b493 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -324,4 +324,40 @@ class Access_Helper_Test extends Unit_Test_Case { $this->assert_false(file_exists($album->resize_path() . "/.htaccess")); $this->assert_false(file_exists($album->thumb_path() . "/.htaccess")); } + + public function moved_items_inherit_new_permissions_test() { + user::set_active(user::lookup_by_name("admin")); + + $root = ORM::factory("item", 1); + $public_album = album::create($root, rand(), "public album"); + $public_photo = photo::create($public_album, MODPATH . "gallery/images/gallery.png", "", ""); + access::allow(group::everybody(), "view", $public_album); + + $root->reload(); // Account for MPTT changes + + $private_album = album::create($root, rand(), "private album"); + access::deny(group::everybody(), "view", $private_album); + $private_photo = photo::create($private_album, MODPATH . "gallery/images/gallery.png", "", ""); + + // Make sure that we now have a public photo and private photo. + $this->assert_true(access::group_can(group::everybody(), "view", $public_photo)); + $this->assert_false(access::group_can(group::everybody(), "view", $private_photo)); + + // Swap the photos + item::move($public_photo, $private_album); + $private_album->reload(); // Reload to get new MPTT pointers and cached perms. + $public_album->reload(); + $private_photo->reload(); + $public_photo->reload(); + + item::move($private_photo, $public_album); + $private_album->reload(); // Reload to get new MPTT pointers and cached perms. + $public_album->reload(); + $private_photo->reload(); + $public_photo->reload(); + + // Make sure that the public_photo is now private, and the private_photo is now public. + $this->assert_false(access::group_can(group::everybody(), "view", $public_photo)); + $this->assert_true(access::group_can(group::everybody(), "view", $private_photo)); + } } -- cgit v1.2.3 From cd907c2b42f8b50ebe6d490aab42365e16deb258 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta 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/access.php | 4 ++++ modules/gallery/helpers/item.php | 4 ++-- modules/gallery/helpers/model_cache.php | 6 ++---- modules/gallery/libraries/MY_ORM.php | 2 +- modules/gallery/libraries/ORM_MPTT.php | 1 + modules/gallery/models/item.php | 1 + 6 files changed, 11 insertions(+), 7 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 5dd1e465..63324e5d 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -205,6 +205,7 @@ class access_Core { } self::_update_htaccess_files($album, $group, $perm_name, $value); + model_cache::clear(); } /** @@ -256,6 +257,7 @@ class access_Core { } } } + model_cache::clear(); } /** @@ -426,6 +428,7 @@ class access_Core { $cache_table = $perm_name == "view" ? "items" : "access_caches"; $db->query("ALTER TABLE {{$cache_table}} DROP `$field`"); $db->query("ALTER TABLE {access_intents} DROP `$field`"); + model_cache::clear(); ORM::factory("access_intent")->clear_cache(); } @@ -443,6 +446,7 @@ class access_Core { $db->query("ALTER TABLE {{$cache_table}} ADD `$field` SMALLINT NOT NULL DEFAULT 0"); $db->query("ALTER TABLE {access_intents} ADD `$field` BOOLEAN DEFAULT NULL"); $db->update("access_intents", array($field => 0), array("item_id" => 1)); + model_cache::clear(); ORM::factory("access_intent")->clear_cache(); } 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; diff --git a/modules/gallery/helpers/model_cache.php b/modules/gallery/helpers/model_cache.php index 2649fdbd..a3e09862 100644 --- a/modules/gallery/helpers/model_cache.php +++ b/modules/gallery/helpers/model_cache.php @@ -32,10 +32,8 @@ class model_cache_Core { return self::$cache->$model_name->$field_name->$id; } - static function clear($model_name, $id, $field_name="id") { - if (!empty(self::$cache->$model_name->$field_name->$id)) { - unset(self::$cache->$model_name->$field_name->$id); - } + static function clear() { + self::$cache = new stdClass(); } static function set($model) { diff --git a/modules/gallery/libraries/MY_ORM.php b/modules/gallery/libraries/MY_ORM.php index 319cbe09..1d3c1ef3 100644 --- a/modules/gallery/libraries/MY_ORM.php +++ b/modules/gallery/libraries/MY_ORM.php @@ -32,7 +32,7 @@ class ORM extends ORM_Core { } public function save() { - model_cache::clear($this->object_name, $this->{$this->primary_key}, $this->primary_key); + model_cache::clear(); $result = parent::save(); $this->original = $this->object; return $result; diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index 46280d95..e371f159 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -285,6 +285,7 @@ class ORM_MPTT_Core extends ORM { // Lets reload to get the changes. $this->reload(); + $target->reload(); return $this; } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 6512e9e5..05c4e656 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -140,6 +140,7 @@ class Item_Model extends ORM_MPTT { $original_parent = $this->parent(); parent::move_to($target, true); + model_cache::clear(); $this->relative_path_cache = null; rename($original_path, $this->file_path()); -- cgit v1.2.3 From c46188111015a128ac4ffa1882a63924cd49b25e Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 19 Jul 2009 13:20:08 -0700 Subject: Don't post the item_updated() event if we're just updating the view count. --- modules/gallery/models/item.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 05c4e656..129bd77f 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -353,11 +353,12 @@ class Item_Model extends ORM_MPTT { $this->created = $this->updated; $r = ORM::factory("item")->select("MAX(weight) as max_weight")->find(); $this->weight = $r->max_weight + 1; - $created = 1; + } else { + $send_event = 1; } } parent::save(); - if (!isset($created)) { + if (isset($send_event)) { module::event("item_updated", $this); } return $this; -- cgit v1.2.3 From 23bb6eb7e35637c8a2124216dbb6d3246ad3d702 Mon Sep 17 00:00:00 2001 From: Romain LE DISEZ Date: Sat, 25 Jul 2009 19:06:54 +0200 Subject: Rename columns that use reserved SQL words : items.left and items.right --- installer/install.sql | 72 +------------------- modules/gallery/helpers/access.php | 40 +++++------ modules/gallery/helpers/gallery_installer.php | 16 +++-- modules/gallery/libraries/ORM_MPTT.php | 84 ++++++++++++------------ modules/gallery/models/item.php | 10 +-- modules/gallery/module.info | 2 +- modules/gallery/tests/Database_Test.php | 8 +-- modules/gallery/tests/Gallery_Installer_Test.php | 10 +-- modules/gallery/tests/ORM_MPTT_Test.php | 14 ++-- modules/gallery/tests/Photo_Helper_Test.php | 4 +- modules/notification/helpers/notification.php | 4 +- 11 files changed, 102 insertions(+), 162 deletions(-) (limited to 'modules/gallery/models') diff --git a/installer/install.sql b/installer/install.sql index 36ccaaff..fc5f967a 100755 --- a/installer/install.sql +++ b/installer/install.sql @@ -1,6 +1,4 @@ DROP TABLE IF EXISTS {access_caches}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {access_caches} ( `id` int(9) NOT NULL auto_increment, `item_id` int(9) default NULL, @@ -12,11 +10,8 @@ CREATE TABLE {access_caches} ( `add_2` binary(1) NOT NULL default '0', PRIMARY KEY (`id`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {access_caches} VALUES (1,1,'1','0','0','1','0','0'); DROP TABLE IF EXISTS {access_intents}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {access_intents} ( `id` int(9) NOT NULL auto_increment, `item_id` int(9) default NULL, @@ -30,11 +25,8 @@ CREATE TABLE {access_intents} ( `add_2` binary(1) default NULL, PRIMARY KEY (`id`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {access_intents} VALUES (1,1,'1','1','0','0','1','1','0','0'); DROP TABLE IF EXISTS {caches}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {caches} ( `id` int(9) NOT NULL auto_increment, `key` varchar(255) NOT NULL, @@ -44,10 +36,7 @@ CREATE TABLE {caches} ( PRIMARY KEY (`id`), KEY `tags` (`tags`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {comments}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {comments} ( `author_id` int(9) default NULL, `created` int(9) NOT NULL, @@ -73,10 +62,7 @@ CREATE TABLE {comments} ( `updated` int(9) NOT NULL, PRIMARY KEY (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {graphics_rules}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {graphics_rules} ( `id` int(9) NOT NULL auto_increment, `active` tinyint(1) default '0', @@ -87,12 +73,9 @@ CREATE TABLE {graphics_rules} ( `target` varchar(32) NOT NULL, PRIMARY KEY (`id`) ) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {graphics_rules} VALUES (1,1,'a:3:{s:5:\"width\";i:200;s:6:\"height\";i:200;s:6:\"master\";i:2;}','gallery','resize',100,'thumb'); INSERT INTO {graphics_rules} VALUES (2,1,'a:3:{s:5:\"width\";i:640;s:6:\"height\";i:480;s:6:\"master\";i:2;}','gallery','resize',100,'resize'); DROP TABLE IF EXISTS {groups}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {groups} ( `id` int(9) NOT NULL auto_increment, `name` char(64) default NULL, @@ -100,25 +83,19 @@ CREATE TABLE {groups} ( PRIMARY KEY (`id`), UNIQUE KEY `name` (`name`) ) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {groups} VALUES (1,'Everybody',1); INSERT INTO {groups} VALUES (2,'Registered Users',1); DROP TABLE IF EXISTS {groups_users}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {groups_users} ( `group_id` int(9) NOT NULL, `user_id` int(9) NOT NULL, PRIMARY KEY (`group_id`,`user_id`), UNIQUE KEY `user_id` (`user_id`,`group_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {groups_users} VALUES (1,1); INSERT INTO {groups_users} VALUES (1,2); INSERT INTO {groups_users} VALUES (2,2); DROP TABLE IF EXISTS {incoming_translations}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {incoming_translations} ( `id` int(9) NOT NULL auto_increment, `key` char(32) NOT NULL, @@ -130,10 +107,7 @@ CREATE TABLE {incoming_translations} ( UNIQUE KEY `key` (`key`,`locale`), KEY `locale_key` (`locale`,`key`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {items}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {items} ( `id` int(9) NOT NULL auto_increment, `album_cover_item_id` int(9) default NULL, @@ -141,7 +115,7 @@ CREATE TABLE {items} ( `created` int(9) default NULL, `description` varchar(2048) default NULL, `height` int(9) default NULL, - `left` int(9) NOT NULL, + `left_ptr` int(9) NOT NULL, `level` int(9) NOT NULL, `mime_type` varchar(64) default NULL, `name` varchar(255) default NULL, @@ -152,7 +126,7 @@ CREATE TABLE {items} ( `resize_dirty` tinyint(1) default '1', `resize_height` int(9) default NULL, `resize_width` int(9) default NULL, - `right` int(9) NOT NULL, + `right_ptr` int(9) NOT NULL, `sort_column` varchar(64) default NULL, `sort_order` char(4) default 'ASC', `thumb_dirty` tinyint(1) default '1', @@ -171,11 +145,8 @@ CREATE TABLE {items} ( KEY `type` (`type`), KEY `random` (`rand_key`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {items} VALUES (1,NULL,NULL,UNIX_TIMESTAMP(),'',NULL,1,1,NULL,NULL,NULL,0,NULL,'',1,NULL,NULL,2,'weight','ASC',1,NULL,NULL,'Gallery','album',UNIX_TIMESTAMP(),0,1,NULL,'1','1'); DROP TABLE IF EXISTS {items_tags}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {items_tags} ( `id` int(9) NOT NULL auto_increment, `item_id` int(9) NOT NULL, @@ -184,10 +155,7 @@ CREATE TABLE {items_tags} ( KEY `tag_id` (`tag_id`,`id`), KEY `item_id` (`item_id`,`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {logs}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {logs} ( `id` int(9) NOT NULL auto_increment, `category` varchar(64) default NULL, @@ -200,10 +168,7 @@ CREATE TABLE {logs} ( `user_id` int(9) default '0', PRIMARY KEY (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {messages}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {messages} ( `id` int(9) NOT NULL auto_increment, `key` varchar(255) default NULL, @@ -212,10 +177,7 @@ CREATE TABLE {messages} ( PRIMARY KEY (`id`), UNIQUE KEY `key` (`key`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {modules}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {modules} ( `id` int(9) NOT NULL auto_increment, `active` tinyint(1) default '0', @@ -224,8 +186,7 @@ CREATE TABLE {modules} ( PRIMARY KEY (`id`), UNIQUE KEY `name` (`name`) ) ENGINE=InnoDB AUTO_INCREMENT=10 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; -INSERT INTO {modules} VALUES (1,1,'gallery',7); +INSERT INTO {modules} VALUES (1,1,'gallery',9); INSERT INTO {modules} VALUES (2,1,'user',1); INSERT INTO {modules} VALUES (3,1,'comment',2); INSERT INTO {modules} VALUES (4,1,'organize',1); @@ -235,8 +196,6 @@ INSERT INTO {modules} VALUES (7,1,'search',1); INSERT INTO {modules} VALUES (8,1,'slideshow',1); INSERT INTO {modules} VALUES (9,1,'tag',1); DROP TABLE IF EXISTS {outgoing_translations}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {outgoing_translations} ( `id` int(9) NOT NULL auto_increment, `base_revision` int(9) default NULL, @@ -248,10 +207,7 @@ CREATE TABLE {outgoing_translations} ( UNIQUE KEY `key` (`key`,`locale`), KEY `locale_key` (`locale`,`key`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {permissions}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {permissions} ( `id` int(9) NOT NULL auto_increment, `display_name` varchar(64) default NULL, @@ -259,14 +215,11 @@ CREATE TABLE {permissions} ( PRIMARY KEY (`id`), UNIQUE KEY `name` (`name`) ) ENGINE=InnoDB AUTO_INCREMENT=5 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {permissions} VALUES (1,'View','view'); INSERT INTO {permissions} VALUES (2,'View Full Size','view_full'); INSERT INTO {permissions} VALUES (3,'Edit','edit'); INSERT INTO {permissions} VALUES (4,'Add','add'); DROP TABLE IF EXISTS {search_records}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {search_records} ( `id` int(9) NOT NULL auto_increment, `item_id` int(9) default NULL, @@ -276,21 +229,15 @@ CREATE TABLE {search_records} ( KEY `item_id` (`item_id`), FULLTEXT KEY `data` (`data`) ) ENGINE=MyISAM AUTO_INCREMENT=2 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {search_records} VALUES (1,1,0,' Gallery'); DROP TABLE IF EXISTS {sessions}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {sessions} ( `session_id` varchar(127) NOT NULL, `data` text NOT NULL, `last_activity` int(10) unsigned NOT NULL, PRIMARY KEY (`session_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {tags}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {tags} ( `id` int(9) NOT NULL auto_increment, `name` varchar(64) NOT NULL, @@ -298,10 +245,7 @@ CREATE TABLE {tags} ( PRIMARY KEY (`id`), UNIQUE KEY `name` (`name`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {tasks}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {tasks} ( `id` int(9) NOT NULL auto_increment, `callback` varchar(128) default NULL, @@ -316,10 +260,7 @@ CREATE TABLE {tasks} ( PRIMARY KEY (`id`), KEY `owner_id` (`owner_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; DROP TABLE IF EXISTS {themes}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {themes} ( `id` int(9) NOT NULL auto_increment, `name` varchar(64) default NULL, @@ -327,12 +268,9 @@ CREATE TABLE {themes} ( PRIMARY KEY (`id`), UNIQUE KEY `name` (`name`) ) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {themes} VALUES (1,'default',1); INSERT INTO {themes} VALUES (2,'admin_default',1); DROP TABLE IF EXISTS {users}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {users} ( `id` int(9) NOT NULL auto_increment, `name` varchar(32) NOT NULL, @@ -350,12 +288,9 @@ CREATE TABLE {users} ( UNIQUE KEY `name` (`name`), UNIQUE KEY `hash` (`hash`) ) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {users} VALUES (1,'guest','Guest User','',0,0,NULL,0,1,NULL,NULL,NULL); INSERT INTO {users} VALUES (2,'admin','Gallery Administrator','',0,0,NULL,1,0,NULL,NULL,NULL); DROP TABLE IF EXISTS {vars}; -SET @saved_cs_client = @@character_set_client; -SET character_set_client = utf8; CREATE TABLE {vars} ( `id` int(9) NOT NULL auto_increment, `module_name` varchar(64) NOT NULL, @@ -364,7 +299,6 @@ CREATE TABLE {vars} ( PRIMARY KEY (`id`), UNIQUE KEY `module_name` (`module_name`,`name`) ) ENGINE=InnoDB AUTO_INCREMENT=27 DEFAULT CHARSET=utf8; -SET character_set_client = @saved_cs_client; INSERT INTO {vars} VALUES (1,'gallery','active_site_theme','default'); INSERT INTO {vars} VALUES (2,'gallery','active_admin_theme','admin_default'); INSERT INTO {vars} VALUES (3,'gallery','page_size','9'); diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index c84527f4..949aea84 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -166,8 +166,8 @@ class access_Core { // For view permissions, if any parent is self::DENY, then those parents lock this one. // Return $lock = ORM::factory("item") - ->where("`left` <= $item->left") - ->where("`right` >= $item->right") + ->where("`left_ptr` <= $item->left_ptr") + ->where("`right_ptr` >= $item->right_ptr") ->where("items.id <> $item->id") ->join("access_intents", "items.id", "access_intents.item_id") ->where("access_intents.view_$group->id", self::DENY) @@ -490,11 +490,11 @@ class access_Core { // item, then its safe to propagate from here. if ($access->$field !== self::DENY) { $tmp_item = ORM::factory("item") - ->where("left <", $item->left) - ->where("right >", $item->right) + ->where("left_ptr <", $item->left_ptr) + ->where("right_ptr >", $item->right_ptr) ->join("access_intents", "access_intents.item_id", "items.id") ->where("access_intents.$field", self::DENY) - ->orderby("left", "DESC") + ->orderby("left_ptr", "DESC") ->limit(1) ->find(); if ($tmp_item->loaded) { @@ -507,13 +507,13 @@ class access_Core { // them according the rule above. So mark every permission below this level as UNKNOWN so // that we can tell which permissions have been changed, and which ones need to be updated. $db->update("items", array($field => self::UNKNOWN), - array("left >=" => $item->left, "right <=" => $item->right)); + array("left_ptr >=" => $item->left_ptr, "right_ptr <=" => $item->right_ptr)); $query = ORM::factory("access_intent") - ->select(array("access_intents.$field", "items.left", "items.right", "items.id")) + ->select(array("access_intents.$field", "items.left_ptr", "items.right_ptr", "items.id")) ->join("items", "items.id", "access_intents.item_id") - ->where("left >=", $item->left) - ->where("right <=", $item->right) + ->where("left_ptr >=", $item->left_ptr) + ->where("right_ptr <=", $item->right_ptr) ->where("type", "album") ->where("access_intents.$field IS NOT", self::INHERIT) ->orderby("level", "DESC") @@ -522,11 +522,11 @@ class access_Core { if ($row->$field == self::ALLOW) { // Propagate ALLOW for any row that is still UNKNOWN. $db->update("items", array($field => $row->$field), - array($field => self::UNKNOWN, "left >=" => $row->left, "right <=" => $row->right)); + array($field => self::UNKNOWN, "left_ptr >=" => $row->left_ptr, "right_ptr <=" => $row->right_ptr)); } else if ($row->$field == self::DENY) { // DENY overwrites everything below it $db->update("items", array($field => $row->$field), - array("left >=" => $row->left, "right <=" => $row->right)); + array("left_ptr >=" => $row->left_ptr, "right_ptr <=" => $row->right_ptr)); } } @@ -534,7 +534,7 @@ class access_Core { // DENY parent in the hierarchy to propagate from. So we'll still have a UNKNOWN values in // the hierarchy, and all of those are safe to change to ALLOW. $db->update("items", array($field => self::ALLOW), - array($field => self::UNKNOWN, "left >=" => $item->left, "right <=" => $item->right)); + array($field => self::UNKNOWN, "left_ptr >=" => $item->left_ptr, "right_ptr <=" => $item->right_ptr)); } /** @@ -562,10 +562,10 @@ class access_Core { if ($access->$field === self::INHERIT) { $tmp_item = ORM::factory("item") ->join("access_intents", "items.id", "access_intents.item_id") - ->where("left <", $item->left) - ->where("right >", $item->right) + ->where("left_ptr <", $item->left_ptr) + ->where("right_ptr >", $item->right_ptr) ->where("$field IS NOT", self::UNKNOWN) - ->orderby("left", "DESC") + ->orderby("left_ptr", "DESC") ->limit(1) ->find(); if ($tmp_item->loaded) { @@ -576,10 +576,10 @@ class access_Core { // With non-view permissions, each level can override any permissions that came above it // so start at the top and work downwards, overlaying permissions as we go. $query = ORM::factory("access_intent") - ->select(array("access_intents.$field", "items.left", "items.right")) + ->select(array("access_intents.$field", "items.left_ptr", "items.right_ptr")) ->join("items", "items.id", "access_intents.item_id") - ->where("left >=", $item->left) - ->where("right <=", $item->right) + ->where("left_ptr >=", $item->left_ptr) + ->where("right_ptr <=", $item->right_ptr) ->where("$field IS NOT", self::INHERIT) ->orderby("level", "ASC") ->find_all(); @@ -589,8 +589,8 @@ class access_Core { "UPDATE {access_caches} SET `$field` = $value " . "WHERE `item_id` IN " . " (SELECT `id` FROM {items} " . - " WHERE `left` >= $row->left " . - " AND `right` <= $row->right)"); + " WHERE `left_ptr` >= $row->left_ptr " . + " AND `right_ptr` <= $row->right_ptr)"); } } diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index db13307f..760bec31 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -72,7 +72,7 @@ class gallery_installer { `created` int(9) default NULL, `description` varchar(2048) default NULL, `height` int(9) default NULL, - `left` int(9) NOT NULL, + `left_ptr` int(9) NOT NULL, `level` int(9) NOT NULL, `mime_type` varchar(64) default NULL, `name` varchar(255) default NULL, @@ -83,7 +83,7 @@ class gallery_installer { `resize_dirty` boolean default 1, `resize_height` int(9) default NULL, `resize_width` int(9) default NULL, - `right` int(9) NOT NULL, + `right_ptr` int(9) NOT NULL, `sort_column` varchar(64) default NULL, `sort_order` char(4) default 'ASC', `thumb_dirty` boolean default 1, @@ -204,8 +204,8 @@ class gallery_installer { $root->type = "album"; $root->title = "Gallery"; $root->description = ""; - $root->left = 1; - $root->right = 2; + $root->left_ptr = 1; + $root->right_ptr = 2; $root->parent_id = 0; $root->level = 1; $root->thumb_dirty = 1; @@ -258,7 +258,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 Gallery %version"); - module::set_version("gallery", 7); + module::set_version("gallery", 9); } static function upgrade($version) { @@ -323,6 +323,12 @@ class gallery_installer { } module::set_version("gallery", $version = 8); } + + if ($version == 8) { + $db->query("ALTER TABLE {items} CHANGE COLUMN `left` `left_ptr` INT(9) NOT NULL;"); + $db->query("ALTER TABLE {items} CHANGE COLUMN `right` `right_ptr` INT(9) NOT NULL;"); + module::set_version("gallery", $version = 9); + } } static function uninstall() { diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index e371f159..1917d738 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -52,14 +52,14 @@ class ORM_MPTT_Core extends ORM { try { // Make a hole in the parent for this new item $this->db->query( - "UPDATE {{$this->table_name}} SET `left` = `left` + 2 WHERE `left` >= {$parent->right}"); + "UPDATE {{$this->table_name}} SET `left_ptr` = `left_ptr` + 2 WHERE `left_ptr` >= {$parent->right_ptr}"); $this->db->query( - "UPDATE {{$this->table_name}} SET `right` = `right` + 2 WHERE `right` >= {$parent->right}"); - $parent->right += 2; + "UPDATE {{$this->table_name}} SET `right_ptr` = `right_ptr` + 2 WHERE `right_ptr` >= {$parent->right_ptr}"); + $parent->right_ptr += 2; // Insert this item into the hole - $this->left = $parent->right - 2; - $this->right = $parent->right - 1; + $this->left_ptr = $parent->right_ptr - 2; + $this->right_ptr = $parent->right_ptr - 1; $this->parent_id = $parent->id; $this->level = $parent->level + 1; $this->save(); @@ -81,7 +81,7 @@ class ORM_MPTT_Core extends ORM { if ($children) { foreach ($this->children() as $item) { // Deleting children affects the MPTT tree, so we have to reload each child before we - // delete it so that we have current left/right pointers. This is inefficient. + // delete it so that we have current left_ptr/right_ptr pointers. This is inefficient. // @todo load each child once, not twice. $item->reload()->delete(); } @@ -93,9 +93,9 @@ class ORM_MPTT_Core extends ORM { $this->lock(); try { $this->db->query( - "UPDATE {{$this->table_name}} SET `left` = `left` - 2 WHERE `left` > {$this->right}"); + "UPDATE {{$this->table_name}} SET `left_ptr` = `left_ptr` - 2 WHERE `left_ptr` > {$this->right_ptr}"); $this->db->query( - "UPDATE {{$this->table_name}} SET `right` = `right` - 2 WHERE `right` > {$this->right}"); + "UPDATE {{$this->table_name}} SET `right_ptr` = `right_ptr` - 2 WHERE `right_ptr` > {$this->right_ptr}"); } catch (Exception $e) { $this->unlock(); throw $e; @@ -111,7 +111,7 @@ class ORM_MPTT_Core extends ORM { * @return boolean */ function is_descendant($target) { - return ($this->left <= $target->left && $this->right >= $target->right); + return ($this->left_ptr <= $target->left_ptr && $this->right_ptr >= $target->right_ptr); } /** @@ -133,10 +133,10 @@ class ORM_MPTT_Core extends ORM { */ function parents() { return $this - ->where("`left` <= {$this->left}") - ->where("`right` >= {$this->right}") + ->where("`left_ptr` <= {$this->left_ptr}") + ->where("`right_ptr` >= {$this->right_ptr}") ->where("id <> {$this->id}") - ->orderby("left", "ASC") + ->orderby("left_ptr", "ASC") ->find_all(); } @@ -181,8 +181,8 @@ class ORM_MPTT_Core extends ORM { * @return object ORM_Iterator */ function descendants($limit=null, $offset=0, $type=null, $orderby=null) { - $this->where("left >", $this->left) - ->where("right <=", $this->right); + $this->where("left_ptr >", $this->left_ptr) + ->where("right_ptr <=", $this->right_ptr); if ($type) { $this->where("type", $type); } @@ -203,8 +203,8 @@ class ORM_MPTT_Core extends ORM { * @return integer child count */ function descendants_count($type=null) { - $this->where("left >", $this->left) - ->where("right <=", $this->right); + $this->where("left_ptr >", $this->left_ptr) + ->where("right_ptr <=", $this->right_ptr); if ($type) { $this->where("type", $type); } @@ -219,16 +219,16 @@ class ORM_MPTT_Core extends ORM { * @return ORM_MTPP */ function move_to($target) { - if ($this->left <= $target->left && - $this->right >= $target->right) { + if ($this->left_ptr <= $target->left_ptr && + $this->right_ptr >= $target->right_ptr) { throw new Exception("@todo INVALID_TARGET can't move item inside itself"); } - $number_to_move = (int)(($this->right - $this->left) / 2 + 1); + $number_to_move = (int)(($this->right_ptr - $this->left_ptr) / 2 + 1); $size_of_hole = $number_to_move * 2; - $original_left = $this->left; - $original_right = $this->right; - $target_right = $target->right; + $original_left_ptr = $this->left_ptr; + $original_right_ptr = $this->right_ptr; + $target_right_ptr = $target->right_ptr; $level_delta = ($target->level + 1) - $this->level; $this->lock(); @@ -237,45 +237,45 @@ class ORM_MPTT_Core extends ORM { // Update the levels for the to-be-moved items $this->db->query( "UPDATE {{$this->table_name}} SET `level` = `level` + $level_delta" . - " WHERE `left` >= $original_left AND `right` <= $original_right"); + " WHERE `left_ptr` >= $original_left_ptr AND `right_ptr` <= $original_right_ptr"); } // Make a hole in the target for the move $target->db->query( - "UPDATE {{$this->table_name}} SET `left` = `left` + $size_of_hole" . - " WHERE `left` >= $target_right"); + "UPDATE {{$this->table_name}} SET `left_ptr` = `left_ptr` + $size_of_hole" . + " WHERE `left_ptr` >= $target_right_ptr"); $target->db->query( - "UPDATE {{$this->table_name}} SET `right` = `right` + $size_of_hole" . - " WHERE `right` >= $target_right"); + "UPDATE {{$this->table_name}} SET `right_ptr` = `right_ptr` + $size_of_hole" . + " WHERE `right_ptr` >= $target_right_ptr"); // Change the parent. $this->db->query( "UPDATE {{$this->table_name}} SET `parent_id` = {$target->id}" . " WHERE `id` = {$this->id}"); - // If the source is to the right of the target then we just adjusted its left and right above. - $left = $original_left; - $right = $original_right; - if ($original_left > $target_right) { - $left += $size_of_hole; - $right += $size_of_hole; + // If the source is to the right of the target then we just adjusted its left_ptr and right_ptr above. + $left_ptr = $original_left_ptr; + $right_ptr = $original_right_ptr; + if ($original_left_ptr > $target_right_ptr) { + $left_ptr += $size_of_hole; + $right_ptr += $size_of_hole; } - $new_offset = $target->right - $left; + $new_offset = $target->right_ptr - $left_ptr; $this->db->query( "UPDATE {{$this->table_name}}" . - " SET `left` = `left` + $new_offset," . - " `right` = `right` + $new_offset" . - " WHERE `left` >= $left" . - " AND `right` <= $right"); + " SET `left_ptr` = `left_ptr` + $new_offset," . + " `right_ptr` = `right_ptr` + $new_offset" . + " WHERE `left_ptr` >= $left_ptr" . + " AND `right_ptr` <= $right_ptr"); // Close the hole in the source's parent after the move $this->db->query( - "UPDATE {{$this->table_name}} SET `left` = `left` - $size_of_hole" . - " WHERE `left` > $right"); + "UPDATE {{$this->table_name}} SET `left_ptr` = `left_ptr` - $size_of_hole" . + " WHERE `left_ptr` > $right_ptr"); $this->db->query( - "UPDATE {{$this->table_name}} SET `right` = `right` - $size_of_hole" . - " WHERE `right` > $right"); + "UPDATE {{$this->table_name}} SET `right_ptr` = `right_ptr` - $size_of_hole" . + " WHERE `right_ptr` > $right_ptr"); } catch (Exception $e) { $this->unlock(); throw $e; diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 58ac8f18..d9dd88f5 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -150,7 +150,7 @@ class Item_Model extends ORM_MPTT { Database::instance() ->update("items", array("relative_path_cache" => null), - array("left >" => $this->left, "right <" => $this->right)); + array("left_ptr >" => $this->left_ptr, "right_ptr <" => $this->right_ptr)); } else { @rename($original_resize_path, $this->resize_path()); @rename($original_thumb_path, $this->thumb_path()); @@ -186,7 +186,7 @@ class Item_Model extends ORM_MPTT { Database::instance() ->update("items", array("relative_path_cache" => null), - array("left >" => $this->left, "right <" => $this->right)); + array("left_ptr >" => $this->left_ptr, "right_ptr <" => $this->right_ptr)); } return $this; @@ -302,10 +302,10 @@ class Item_Model extends ORM_MPTT { foreach (Database::instance() ->select("name") ->from("items") - ->where("left <=", $this->left) - ->where("right >=", $this->right) + ->where("left_ptr <=", $this->left_ptr) + ->where("right_ptr >=", $this->right_ptr) ->where("id <>", 1) - ->orderby("left", "ASC") + ->orderby("left_ptr", "ASC") ->get() as $row) { $paths[] = $row->name; } diff --git a/modules/gallery/module.info b/modules/gallery/module.info index ba367878..c5a9d25d 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,3 +1,3 @@ name = "Gallery 3" description = "Gallery core application" -version = 8 +version = 9 diff --git a/modules/gallery/tests/Database_Test.php b/modules/gallery/tests/Database_Test.php index bd3d2f53..d83212ad 100644 --- a/modules/gallery/tests/Database_Test.php +++ b/modules/gallery/tests/Database_Test.php @@ -103,15 +103,15 @@ class Database_Test extends Unit_Test_Case { $sql = "UPDATE {test_tables} SET `name` = '{test string}' " . "WHERE `item_id` IN " . " (SELECT `id` FROM {items} " . - " WHERE `left` >= 1 " . - " AND `right` <= 6)"; + " WHERE `left_ptr` >= 1 " . + " AND `right_ptr` <= 6)"; $sql = $db->add_table_prefixes($sql); $expected = "UPDATE g3test_test_tables SET `name` = '{test string}' " . "WHERE `item_id` IN " . " (SELECT `id` FROM g3test_items " . - " WHERE `left` >= 1 " . - " AND `right` <= 6)"; + " WHERE `left_ptr` >= 1 " . + " AND `right_ptr` <= 6)"; $this->assert_same($expected, $sql); } diff --git a/modules/gallery/tests/Gallery_Installer_Test.php b/modules/gallery/tests/Gallery_Installer_Test.php index 27157d6e..36ced2bb 100644 --- a/modules/gallery/tests/Gallery_Installer_Test.php +++ b/modules/gallery/tests/Gallery_Installer_Test.php @@ -34,13 +34,13 @@ class Gallery_Installer_Test extends Unit_Test_Case { } public function install_creates_root_item_test() { - $max_right = ORM::factory("item") - ->select("MAX(`right`) AS `right`") - ->find()->right; + $max_right_ptr = ORM::factory("item") + ->select("MAX(`right_ptr`) AS `right_ptr`") + ->find()->right_ptr; $root = ORM::factory('item')->find(1); $this->assert_equal("Gallery", $root->title); - $this->assert_equal(1, $root->left); - $this->assert_equal($max_right, $root->right); + $this->assert_equal(1, $root->left_ptr); + $this->assert_equal($max_right_ptr, $root->right_ptr); $this->assert_equal(null, $root->parent_id); $this->assert_equal(1, $root->level); } diff --git a/modules/gallery/tests/ORM_MPTT_Test.php b/modules/gallery/tests/ORM_MPTT_Test.php index 200c8a74..943810c3 100644 --- a/modules/gallery/tests/ORM_MPTT_Test.php +++ b/modules/gallery/tests/ORM_MPTT_Test.php @@ -33,8 +33,8 @@ class ORM_MPTT_Test extends Unit_Test_Case { $album->sort_order = "ASC"; $album->add_to_parent($root); - $this->assert_equal($album->parent()->right - 2, $album->left); - $this->assert_equal($album->parent()->right - 1, $album->right); + $this->assert_equal($album->parent()->right_ptr - 2, $album->left_ptr); + $this->assert_equal($album->parent()->right_ptr - 1, $album->right_ptr); $this->assert_equal($album->parent()->level + 1, $album->level); $this->assert_equal($album->parent()->id, $album->parent_id); } @@ -48,10 +48,10 @@ class ORM_MPTT_Test extends Unit_Test_Case { $album1_1_2 = self::create_item_and_add_to_parent($album1_1); $album1->reload(); - $this->assert_equal(9, $album1->right - $album1->left); + $this->assert_equal(9, $album1->right_ptr - $album1->left_ptr); $album1_1->reload(); - $this->assert_equal(5, $album1_1->right - $album1_1->left); + $this->assert_equal(5, $album1_1->right_ptr - $album1_1->left_ptr); } public function delete_hierarchy_test() { @@ -66,7 +66,7 @@ class ORM_MPTT_Test extends Unit_Test_Case { $album1->reload(); // Now album1 contains only album1_2 - $this->assert_equal(3, $album1->right - $album1->left); + $this->assert_equal(3, $album1->right_ptr - $album1->left_ptr); } public function move_to_test() { @@ -85,8 +85,8 @@ class ORM_MPTT_Test extends Unit_Test_Case { $album1_1->reload(); $album1_2->reload(); - $this->assert_equal(3, $album1_1->right - $album1_1->left); - $this->assert_equal(3, $album1_2->right - $album1_2->left); + $this->assert_equal(3, $album1_1->right_ptr - $album1_1->left_ptr); + $this->assert_equal(3, $album1_2->right_ptr - $album1_2->left_ptr); $this->assert_equal( array($album1_1_2->id => "move_to_test_1_1_2"), diff --git a/modules/gallery/tests/Photo_Helper_Test.php b/modules/gallery/tests/Photo_Helper_Test.php index cc1f20da..c0641ef4 100644 --- a/modules/gallery/tests/Photo_Helper_Test.php +++ b/modules/gallery/tests/Photo_Helper_Test.php @@ -43,8 +43,8 @@ class Photo_Helper_Test extends Unit_Test_Case { $this->assert_equal($image_info[0], $photo->width); $this->assert_equal($image_info[1], $photo->height); - $this->assert_equal($photo->parent()->right - 2, $photo->left); - $this->assert_equal($photo->parent()->right - 1, $photo->right); + $this->assert_equal($photo->parent()->right_ptr - 2, $photo->left_ptr); + $this->assert_equal($photo->parent()->right_ptr - 1, $photo->right_ptr); } public function create_conflicting_photo_test() { diff --git a/modules/notification/helpers/notification.php b/modules/notification/helpers/notification.php index d91a37e8..92c40d4f 100644 --- a/modules/notification/helpers/notification.php +++ b/modules/notification/helpers/notification.php @@ -73,8 +73,8 @@ class notification { ->join("subscriptions", "users.id", "subscriptions.user_id") ->join("items", "subscriptions.item_id", "items.id") ->where("email IS NOT", null) - ->where("items.left <=", $item->left) - ->where("items.right >", $item->right) + ->where("items.left_ptr <=", $item->left_ptr) + ->where("items.right_ptr >", $item->right_ptr) ->find_all(); $subscribers = array(); -- cgit v1.2.3 From 809e52d80cbf3beb75b238fddb0da3951fb9a8e7 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Sat, 25 Jul 2009 23:45:48 -0700 Subject: Fix for ticket #452 1) Change access_Core::user_can to return true for all permissions if the owner is the specified user. 2) Change Item_Model::viewable to set the owner_id is the first view_restriction This allowed simplification of the generating the where clause to a single $this->orwhere instead of a where and an orwhere. --- modules/gallery/helpers/access.php | 2 +- modules/gallery/models/item.php | 22 ++++------------------ 2 files changed, 5 insertions(+), 19 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index c84527f4..2faa922b 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -95,7 +95,7 @@ class access_Core { return false; } - if ($user->admin) { + if ($user->admin && $item->owner_id == $user->id) { return true; } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 129bd77f..99d1ca6d 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -38,31 +38,17 @@ class Item_Model extends ORM_MPTT { if (user::active()->admin) { $this->view_restrictions = array(); } else { + $this->view_restrictions["owner_id"] = user::active()->id; 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; - } + $this->view_restrictions["view_$id"] = access::ALLOW; } } } - switch (count($this->view_restrictions)) { - case 0: - break; - case 1: - $this->where($this->view_restrictions[0], access::ALLOW); - break; - - default: + if (!empty($this->view_restrictions)) { $this->open_paren(); - $this->where($this->view_restrictions[0], access::ALLOW); - $this->orwhere($this->view_restrictions[1]); + $this->orwhere($this->view_restrictions); $this->close_paren(); - break; } return $this; -- cgit v1.2.3 From 4edf86f0ebfedbbdfda3daf71ed55a461edf9c6c Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 27 Jul 2009 11:14:03 -0700 Subject: Revert "Fix for ticket #452" This reverts commit 809e52d80cbf3beb75b238fddb0da3951fb9a8e7. --- modules/gallery/helpers/access.php | 2 +- modules/gallery/models/item.php | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 8c6f5d54..949aea84 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -95,7 +95,7 @@ class access_Core { return false; } - if ($user->admin && $item->owner_id == $user->id) { + if ($user->admin) { return true; } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 45561380..d9dd88f5 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -38,17 +38,31 @@ class Item_Model extends ORM_MPTT { if (user::active()->admin) { $this->view_restrictions = array(); } else { - $this->view_restrictions["owner_id"] = user::active()->id; foreach (user::group_ids() as $id) { - $this->view_restrictions["view_$id"] = access::ALLOW; + // 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; - if (!empty($this->view_restrictions)) { + case 1: + $this->where($this->view_restrictions[0], access::ALLOW); + break; + + default: $this->open_paren(); - $this->orwhere($this->view_restrictions); + $this->where($this->view_restrictions[0], access::ALLOW); + $this->orwhere($this->view_restrictions[1]); $this->close_paren(); + break; } return $this; -- cgit v1.2.3 From d18f31ab218c8d79d97b19079ac0078e6155bc39 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Mon, 27 Jul 2009 11:25:23 -0700 Subject: Revert "Remove an useless ORDER BY."... the order by wasn't all that useless. This reverts commit c80d2da0a95a63b76f5a4c835f1a0e1022ec2f53. Conflicts: modules/gallery/models/item.php --- modules/gallery/models/item.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 45561380..c1d11a96 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -387,7 +387,8 @@ class Item_Model extends ORM_MPTT { SELECT COUNT(*) AS position FROM {items} WHERE parent_id = {$this->id} AND `{$this->sort_column}` $comp (SELECT `{$this->sort_column}` - FROM {items} WHERE id = $child_id)")->current()->position; + FROM {items} WHERE id = $child_id) + ORDER BY `{$this->sort_column}` {$this->sort_order}")->current()->position; // We stopped short of our target value in the sort (notice that we're using a < comparator // above) because it's possible that we have duplicate values in the sort column. An -- cgit v1.2.3 From 7efb4b4cdfba4c0280b18d1980cd8ad011360b87 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 27 Jul 2009 15:17:14 -0700 Subject: Remove the unnecessary ORDER BY on $this->sort_column in get_position(), and instead apply an ORDER BY on `id` in the 2nd query so that we have stability among the equal elements. This should result in cheaper (and more sensible) queries. --- modules/gallery/models/item.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 038e11fb..dcbee991 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -399,10 +399,10 @@ class Item_Model extends ORM_MPTT { $db = Database::instance(); $position = $db->query(" SELECT COUNT(*) AS position FROM {items} - WHERE parent_id = {$this->id} + WHERE `parent_id` = {$this->id} AND `{$this->sort_column}` $comp (SELECT `{$this->sort_column}` - FROM {items} WHERE id = $child_id) - ORDER BY `{$this->sort_column}` {$this->sort_order}")->current()->position; + FROM {items} WHERE `id` = $child_id)") + ->current()->position; // We stopped short of our target value in the sort (notice that we're using a < comparator // above) because it's possible that we have duplicate values in the sort column. An @@ -414,9 +414,10 @@ class Item_Model extends ORM_MPTT { // our base value. $result = $db->query(" SELECT id FROM {items} - WHERE parent_id = {$this->id} + WHERE `parent_id` = {$this->id} AND `{$this->sort_column}` = (SELECT `{$this->sort_column}` - FROM {items} WHERE id = $child_id)"); + FROM {items} WHERE `id` = $child_id) + ORDER BY `id` ASC"); foreach ($result as $row) { $position++; if ($row->id == $child_id) { -- cgit v1.2.3 From ead6a61d9e96d37ca3a0aecb93d18755099e341c Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 30 Jul 2009 01:55:56 +0800 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 Signed-off-by: Tim Almdal --- modules/gallery/helpers/gallery_installer.php | 21 +++++++++++++++++++-- modules/gallery/models/item.php | 16 ++++++++++++++-- modules/gallery/module.info | 2 +- 3 files changed, 34 insertions(+), 5 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 760bec31..2322110e 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -98,9 +98,20 @@ class gallery_installer { PRIMARY KEY (`id`), KEY `parent_id` (`parent_id`), KEY `type` (`type`), - KEY `random` (`rand_key`)) + KEY `random` (`rand_key`), + KEY `weight` (`weight` DESC)) ENGINE=InnoDB DEFAULT CHARSET=utf8;"); + $db->query("DELIMITER | + CREATE TRIGGER setweight BEFORE INSERT ON {items} + FOR EACH ROW BEGIN + DECLARE new_weight int(9); + SELECT weight+1 FROM {items} + ORDER BY weight LIMIT 1 INTO new_weight; + SET NEW.weight = new_weight; + END;| + DELIMITER ;"); + $db->query("CREATE TABLE {logs} ( `id` int(9) NOT NULL auto_increment, `category` varchar(64) default NULL, @@ -329,7 +340,13 @@ class gallery_installer { $db->query("ALTER TABLE {items} CHANGE COLUMN `right` `right_ptr` INT(9) NOT NULL;"); module::set_version("gallery", $version = 9); } - } + + if ($version == 9) { + $db->query("ALTER TABLE {items} ADD KEY `weight` (`weight` DESC);"); + + module::set_version("gallery", $version = 10); + } +} static function uninstall() { $db = Database::instance(); 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; } diff --git a/modules/gallery/module.info b/modules/gallery/module.info index c5a9d25d..dfb1a7a2 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,3 +1,3 @@ name = "Gallery 3" description = "Gallery core application" -version = 9 +version = 10 -- cgit v1.2.3 From 7438a9c8893a5f584df3fb6a1552262c7da39b86 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 29 Jul 2009 16:40:01 -0700 Subject: Remove some scary debug code. --- modules/gallery/helpers/gallery_installer.php | 10 ---------- modules/gallery/models/item.php | 16 ++-------------- 2 files changed, 2 insertions(+), 24 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 2322110e..12031ccb 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -102,16 +102,6 @@ class gallery_installer { KEY `weight` (`weight` DESC)) ENGINE=InnoDB DEFAULT CHARSET=utf8;"); - $db->query("DELIMITER | - CREATE TRIGGER setweight BEFORE INSERT ON {items} - FOR EACH ROW BEGIN - DECLARE new_weight int(9); - SELECT weight+1 FROM {items} - ORDER BY weight LIMIT 1 INTO new_weight; - SET NEW.weight = new_weight; - END;| - DELIMITER ;"); - $db->query("CREATE TABLE {logs} ( `id` int(9) NOT NULL auto_increment, `category` varchar(64) default NULL, diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 481b22bc..dcbee991 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -350,21 +350,9 @@ 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; - 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; - } + $r = ORM::factory("item")->select("MAX(weight) as max_weight")->find(); + $this->weight = $r->max_weight + 1; } else { $send_event = 1; } -- cgit v1.2.3 From ca96d4719608d4a8a62457ecf3e54ac8ca29629a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 29 Jul 2009 16:58:29 -0700 Subject: Oops, I removed too much in my last change. Put talmdal's query back, but change it to use Database instead of ORM for a little more efficiency. --- modules/gallery/models/item.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index dcbee991..a0598ea4 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -351,8 +351,11 @@ class Item_Model extends ORM_MPTT { $this->updated = time(); if (!$this->loaded) { $this->created = $this->updated; - $r = ORM::factory("item")->select("MAX(weight) as max_weight")->find(); - $this->weight = $r->max_weight + 1; + $weight = Database::instance() + ->select("weight")->from("items") + ->orderby("weight", "desc")->limit(1) + ->get()->current()->weight; + $this->weight = $weight + 1; } else { $send_event = 1; } -- cgit v1.2.3 From fc3273da4d0b34310ba304310396527430c98ce2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 30 Jul 2009 08:02:54 -0700 Subject: Add some code to guard the weight calculation against zero rows when we're doing an initial install. --- modules/gallery/models/item.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index a0598ea4..b3c7998b 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -351,11 +351,14 @@ class Item_Model extends ORM_MPTT { $this->updated = time(); if (!$this->loaded) { $this->created = $this->updated; - $weight = Database::instance() + // 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()->weight; - $this->weight = $weight + 1; + ->get()->current(); + $this->weight = ($result ? $result->weight : 0) + 1; } else { $send_event = 1; } -- cgit v1.2.3 From 7ad0808a117fd1db4e94da8d7763ccca1d69350a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 2 Aug 2009 12:09:00 -0700 Subject: Change the API for getting to the original state of an ORM. Old API: $obj->original("field_name") New API: $obj->original()->field_name This allows us to revert the varous xxx_updated events back to passing an original ORM as well as the the updated one. This makes for a cleaner event API. Old API: comment_updated($comment) { $comment->original("field_name") } Old API: comment_updated($old, $new) { $old->field_name } --- modules/akismet/helpers/akismet_event.php | 6 +++--- modules/comment/models/comment.php | 8 ++++---- modules/gallery/libraries/MY_ORM.php | 12 ++++++------ modules/gallery/models/item.php | 2 +- modules/gallery/tests/Item_Model_Test.php | 2 +- modules/notification/helpers/notification_event.php | 8 ++++---- modules/search/helpers/search_event.php | 4 ++-- modules/user/models/group.php | 2 +- modules/user/models/user.php | 2 +- 9 files changed, 23 insertions(+), 23 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/akismet/helpers/akismet_event.php b/modules/akismet/helpers/akismet_event.php index d6cde222..cec6d95d 100644 --- a/modules/akismet/helpers/akismet_event.php +++ b/modules/akismet/helpers/akismet_event.php @@ -40,14 +40,14 @@ class akismet_event_Core { $comment->save(); } - static function comment_updated($comment) { + static function comment_updated($original, $new) { if (!module::get_var("akismet", "api_key")) { return; } - if ($comment->original("state") != "spam" && $comment->state == "spam") { + if ($original->state != "spam" && $new->state == "spam") { akismet::submit_spam($new); - } else if ($comment->original("state") == "spam" && $comment->state != "spam") { + } else if ($original->state == "spam" && $new->state != "spam") { akismet::submit_ham($new); } } diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php index d052a39c..83d0888a 100644 --- a/modules/comment/models/comment.php +++ b/modules/comment/models/comment.php @@ -64,17 +64,17 @@ class Comment_Model extends ORM { $created = true; } } + $visible_change = $this->original()->state == "published" || $this->state == "published"; parent::save(); if (isset($created)) { module::event("comment_created", $this); } else { - module::event("comment_updated", $this); + module::event("comment_updated", $this->original(), $this); } - // We only notify on the related items if we're making a visible change, which means moving in - // or out of a published state - if ($this->original("state") == "published" || $this->state == "published") { + // We only notify on the related items if we're making a visible change. + if ($visible_change) { module::event("item_related_update", $this->item()); } diff --git a/modules/gallery/libraries/MY_ORM.php b/modules/gallery/libraries/MY_ORM.php index 1d3c1ef3..de8adc1d 100644 --- a/modules/gallery/libraries/MY_ORM.php +++ b/modules/gallery/libraries/MY_ORM.php @@ -18,7 +18,7 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class ORM extends ORM_Core { - // Track the original value of this ORM instance so that we can look it up in ORM::original() + // Track the original value of this ORM so that we can look it up in ORM::original() protected $original = null; public function open_paren() { @@ -34,13 +34,13 @@ class ORM extends ORM_Core { public function save() { model_cache::clear(); $result = parent::save(); - $this->original = $this->object; + $this->original = clone $this; return $result; } public function __set($column, $value) { if (!isset($this->original)) { - $this->original = $this->object; + $this->original = clone $this; } return parent::__set($column, $value); @@ -48,14 +48,14 @@ class ORM extends ORM_Core { public function __unset($column) { if (!isset($this->original)) { - $this->original = $this->object; + $this->original = clone $this; } return parent::__unset($column); } - public function original($column) { - return $this->original[$column]; + public function original() { + return $this->original; } } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index b3c7998b..f3e6b8f3 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -365,7 +365,7 @@ class Item_Model extends ORM_MPTT { } parent::save(); if (isset($send_event)) { - module::event("item_updated", $this); + module::event("item_updated", $this->original(), $this); } return $this; } diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php index c2773097..0940d076 100644 --- a/modules/gallery/tests/Item_Model_Test.php +++ b/modules/gallery/tests/Item_Model_Test.php @@ -147,7 +147,7 @@ class Item_Model_Test extends Unit_Test_Case { $item->save(); $item->title = "NEW_VALUE"; - $this->assert_same("ORIGINAL_VALUE", $item->original("title")); + $this->assert_same("ORIGINAL_VALUE", $item->original()->title); $this->assert_same("NEW_VALUE", $item->title); } } diff --git a/modules/notification/helpers/notification_event.php b/modules/notification/helpers/notification_event.php index c6e770a7..d1b76e93 100644 --- a/modules/notification/helpers/notification_event.php +++ b/modules/notification/helpers/notification_event.php @@ -18,8 +18,8 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class notification_event_Core { - static function item_updated($item) { - notification::send_item_updated($item); + static function item_updated($original, $new) { + notification::send_item_updated($new); } static function item_created($item) { @@ -40,8 +40,8 @@ class notification_event_Core { } } - static function comment_updated($item) { - if ($item->state == "published" && $item->original("state") != "published") { + static function comment_updated($original, $new) { + if ($new->state == "published" && $original->state != "published") { notification::send_comment_published($new); } } diff --git a/modules/search/helpers/search_event.php b/modules/search/helpers/search_event.php index 764fdd18..b65763af 100644 --- a/modules/search/helpers/search_event.php +++ b/modules/search/helpers/search_event.php @@ -22,8 +22,8 @@ class search_event_Core { search::update($item); } - static function item_updated($item) { - search::update($item); + static function item_updated($original, $new) { + search::update($new); } static function item_deleted($item) { diff --git a/modules/user/models/group.php b/modules/user/models/group.php index bb3fb58b..8af78012 100644 --- a/modules/user/models/group.php +++ b/modules/user/models/group.php @@ -41,7 +41,7 @@ class Group_Model extends ORM { if (isset($created)) { module::event("group_created", $this); } else { - module::event("group_updated", $this); + module::event("group_updated", $this->original(), $this); } return $this; } diff --git a/modules/user/models/user.php b/modules/user/models/user.php index def65a6f..4b43adff 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -68,7 +68,7 @@ class User_Model extends ORM { if (isset($created)) { module::event("user_created", $this); } else { - module::event("user_updated", $this); + module::event("user_updated", $this->original(), $this); } return $this; } -- 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 From a09a6a06bef1ff08e09a2855b308e20ea739ca1e Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 2 Sep 2009 11:28:41 -0700 Subject: Refactor how we use $this->relative_path() so that we're not calling it twice on both sides of a ternary operator. --- modules/gallery/models/item.php | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 68e89db6..8905a627 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -188,9 +188,8 @@ class Item_Model extends ORM_MPTT { * photo: http://example.com/gallery3/var/albums/album1/photo.jpg */ public function file_url($full_uri=false) { - return $full_uri ? - url::abs_file("var/albums/" . $this->relative_path()) : - url::file("var/albums/" . $this->relative_path()); + $relative_path = "var/albums/" . $this->relative_path(); + return $full_uri ? url::abs_file($relative_path) : url::file($relative_path); } /** @@ -222,9 +221,8 @@ class Item_Model extends ORM_MPTT { */ public function thumb_url($full_uri=false) { $cache_buster = "?m=" . $this->updated; - $base = ($full_uri ? - url::abs_file("var/thumbs/" . $this->relative_path()) : - url::file("var/thumbs/" . $this->relative_path())); + $relative_path = "var/thumbs/" . $this->relative_path(); + $base = ($full_uri ? url::abs_file($relative_path) : url::file($relative_path)); if ($this->is_photo()) { return $base . $cache_buster; } else if ($this->is_album()) { @@ -250,9 +248,8 @@ class Item_Model extends ORM_MPTT { * photo: http://example.com/gallery3/var/albums/album1/photo.resize.jpg */ public function resize_url($full_uri=false) { - return ($full_uri ? - url::abs_file("var/resizes/" . $this->relative_path()) : - url::file("var/resizes/" . $this->relative_path())) . + $relative_path = "var/resizes/" . $this->relative_path(); + return ($full_uri ? url::abs_file($relative_path) : url::file($relative_path)) . ($this->is_album() ? "/.album.jpg" : ""); } -- cgit v1.2.3 From 3f997562dee7fa45fec58604dd894d9c523a1113 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta 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/models') 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 Gallery %version"); - 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 dccfce4c00024fa170a060b1c4b1d2bb227964ae Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 7 Sep 2009 21:00:52 -0700 Subject: Correct a mental mistake in the prior change. All the xxx_url() methods actually refer to the url to the file on disk, not the logical item. That's relative_url(). Perhaps some API renaming is in order? --- modules/gallery/models/item.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index b679ebf1..ed1294e4 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -190,7 +190,7 @@ class Item_Model extends ORM_MPTT { * photo: http://example.com/gallery3/var/albums/album1/photo.jpg */ public function file_url($full_uri=false) { - $relative_url = "var/albums/" . $this->relative_url(); + $relative_url = "var/albums/" . $this->relative_path(); return $full_uri ? url::abs_file($relative_url) : url::file($relative_url); } @@ -223,8 +223,8 @@ class Item_Model extends ORM_MPTT { */ public function thumb_url($full_uri=false) { $cache_buster = "?m=" . $this->updated; - $relative_url = "var/thumbs/" . $this->relative_url(); - $base = ($full_uri ? url::abs_file($relative_url) : url::file($relative_url)); + $relative_path = "var/thumbs/" . $this->relative_path(); + $base = ($full_uri ? url::abs_file($relative_path) : url::file($relative_path)); if ($this->is_photo()) { return $base . $cache_buster; } else if ($this->is_album()) { @@ -250,8 +250,8 @@ 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_url = "var/resizes/" . $this->relative_url(); - return ($full_uri ? url::abs_file($relative_url) : url::file($relative_url)) . + $relative_path = "var/resizes/" . $this->relative_path(); + return ($full_uri ? url::abs_file($relative_path) : url::file($relative_path)) . ($this->is_album() ? "/.album.jpg" : ""); } @@ -329,9 +329,11 @@ class Item_Model extends ORM_MPTT { */ public function __set($column, $value) { if ($column == "name") { - // 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. + // Clear the relative path as it is no longer valid. $this->relative_path_cache = null; + } else if ($column == "slug") { + // Clear the relative url as it is no longer valid. + $this->relative_url_cache = null; } parent::__set($column, $value); } -- cgit v1.2.3 From 78a42d9b8397e36bdd2a84e1fd6b4f49f0873b78 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 7 Sep 2009 21:09:52 -0700 Subject: 1. Actually set the slug to the new value in the controllers. 2. Fix up an issue where we were crashing if there were no conflicting rows 3. Amend Item_Model so that if you change the slug, it flushes the cache for all children --- modules/gallery/controllers/albums.php | 36 ++++++++++++++++++---------------- modules/gallery/controllers/movies.php | 36 ++++++++++++++++++---------------- modules/gallery/controllers/photos.php | 36 ++++++++++++++++++---------------- modules/gallery/models/item.php | 13 +++++++++--- 4 files changed, 67 insertions(+), 54 deletions(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 732a1789..b7a9f339 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -172,24 +172,25 @@ class Albums_Controller extends Items_Controller { $form->edit_item->dirname->value != $album->name || $form->edit_item->slug->value != $album->slug) { // Make sure that there's not a conflict - $row = Database::instance() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", $album->parent_id) - ->where("id <>", $album->id) - ->open_paren() - ->where("name", $form->edit_item->dirname->value) - ->orwhere("slug", $form->edit_item->slug->value) - ->close_paren() - ->get() - ->current(); - if ($row->name == $form->edit_item->dirname->value) { - $form->edit_item->dirname->add_error("name_conflict", 1); + if ($row = Database::instance() + ->select(array("name", "slug")) + ->from("items") + ->where("parent_id", $album->parent_id) + ->where("id <>", $album->id) + ->open_paren() + ->where("name", $form->edit_item->dirname->value) + ->orwhere("slug", $form->edit_item->slug->value) + ->close_paren() + ->get() + ->current()) { + if ($row->name == $form->edit_item->dirname->value) { + $form->edit_item->dirname->add_error("name_conflict", 1); + } + if ($row->slug == $form->edit_item->slug->value) { + $form->edit_item->slug->add_error("slug_conflict", 1); + } + $valid = false; } - if ($row->slug == $form->edit_item->slug->value) { - $form->edit_item->slug->add_error("slug_conflict", 1); - } - $valid = false; } } @@ -201,6 +202,7 @@ class Albums_Controller extends Items_Controller { if ($album->id != 1) { $album->rename($form->edit_item->dirname->value); } + $album->slug = $form->edit_item->slug->value; $album->save(); module::event("item_edit_form_completed", $album, $form); diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 2ba6bb5b..c549dbf8 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -75,30 +75,32 @@ class Movies_Controller extends Items_Controller { 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 - $row = Database::instance() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", $photo->parent_id) - ->where("id <>", $photo->id) - ->open_paren() - ->where("name", $form->edit_item->filename->value) - ->orwhere("slug", $form->edit_item->slug->value) - ->close_paren() - ->get() - ->current(); - if ($row->name == $form->edit_item->filename->value) { - $form->edit_item->filename->add_error("name_conflict", 1); + if ($row = Database::instance() + ->select(array("name", "slug")) + ->from("items") + ->where("parent_id", $photo->parent_id) + ->where("id <>", $photo->id) + ->open_paren() + ->where("name", $form->edit_item->filename->value) + ->orwhere("slug", $form->edit_item->slug->value) + ->close_paren() + ->get() + ->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; } - if ($row->slug == $form->edit_item->slug->value) { - $form->edit_item->slug->add_error("slug_conflict", 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); diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 159501c0..959097b2 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -67,30 +67,32 @@ class Photos_Controller extends Items_Controller { 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 - $row = Database::instance() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", $photo->parent_id) - ->where("id <>", $photo->id) - ->open_paren() - ->where("name", $form->edit_item->filename->value) - ->orwhere("slug", $form->edit_item->slug->value) - ->close_paren() - ->get() - ->current(); - if ($row->name == $form->edit_item->filename->value) { - $form->edit_item->filename->add_error("name_conflict", 1); + if ($row = Database::instance() + ->select(array("name", "slug")) + ->from("items") + ->where("parent_id", $photo->parent_id) + ->where("id <>", $photo->id) + ->open_paren() + ->where("name", $form->edit_item->filename->value) + ->orwhere("slug", $form->edit_item->slug->value) + ->close_paren() + ->get() + ->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; } - if ($row->slug == $form->edit_item->slug->value) { - $form->edit_item->slug->add_error("slug_conflict", 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); diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index ed1294e4..a2c49154 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -329,11 +329,18 @@ class Item_Model extends ORM_MPTT { */ public function __set($column, $value) { if ($column == "name") { - // Clear the relative path as it is no longer valid. $this->relative_path_cache = null; } else if ($column == "slug") { - // Clear the relative url as it is no longer valid. - $this->relative_url_cache = null; + if ($this->slug != $value) { + // Clear the relative url cache for this item and all children + $this->relative_url_cache = null; + if ($this->is_album()) { + Database::instance() + ->update("items", + array("relative_url_cache" => null), + array("left_ptr >" => $this->left_ptr, "right_ptr <" => $this->right_ptr)); + } + } } parent::__set($column, $value); } -- cgit v1.2.3 From 86f88769f9f636d5fbc75cc39b6f3b78b6ed5893 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 7 Sep 2009 21:30:48 -0700 Subject: Update Item_Model::save() to consider the relative_xxx_cache fields as insignificant, in addition to the view_count field. This means that we won't trigger an event, nor will we adjust the updated field if *only* those fields are changed. --- modules/gallery/models/item.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'modules/gallery/models') diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index a2c49154..0ec5d048 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -349,7 +349,12 @@ class Item_Model extends ORM_MPTT { * @see ORM::save() */ public function save() { - if (!empty($this->changed) && $this->changed != array("view_count" => "view_count")) { + $significant_changes = $this->changed; + unset($significant_changes["view_count"]); + unset($significant_changes["relative_url_cache"]); + unset($significant_changes["relative_path_cache"]); + + if (!empty($this->changed) && $significant_changes) { $this->updated = time(); if (!$this->loaded) { $this->created = $this->updated; -- cgit v1.2.3 From 2aad580f53dbc06bb170c710467b47a5a532c6c8 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta 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
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/models') 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", "id\">view"); + log::success("content", "Updated photo", "url()}\">view"); 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", "id\">view"); + log::success("content", "Updated photo", "url()}\">view"); 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 @@