From 4e56176f35fe624d2d3a587636a4a45ea387be09 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 5 Jun 2010 23:47:47 -0700 Subject: item::random_query() doesn't need to take a "where" clause because it's returning a query, so the caller can add the where clause himself. This makes for a cleaner API. --- modules/gallery/helpers/item.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 43c93225..bbbe1058 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -209,17 +209,14 @@ class item_Core { /** * Return a query to get a random Item_Model, with optional filters - * - * @param array (optional) where tuple */ - static function random_query($where=null) { + static function random_query() { // Pick a random number and find the item that's got nearest smaller number. // This approach works best when the random numbers in the system are roughly evenly // distributed so this is going to be more efficient with larger data sets. return ORM::factory("item") ->viewable() ->where("rand_key", "<", ((float)mt_rand()) / (float)mt_getrandmax()) - ->merge_where($where) ->order_by("rand_key", "DESC"); } } \ No newline at end of file -- cgit v1.2.3 From 58b21e909d8ba628ddb8a19e732989821abb0283 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 10 Jun 2010 18:49:29 -0700 Subject: Change the pattern used to convert the file name to a title. Fixes ticket#1061 --- modules/gallery/helpers/item.php | 2 +- modules/gallery/tests/Item_Helper_Test.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index bbbe1058..15bbe977 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -136,7 +136,7 @@ class item_Core { */ static function convert_filename_to_title($filename) { $title = strtr($filename, "_", " "); - $title = preg_replace("/\..*?$/", "", $title); + $title = preg_replace("/\..{3,4}$/", "", $title); $title = preg_replace("/ +/", " ", $title); return $title; } diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php index 4771b11a..00229973 100644 --- a/modules/gallery/tests/Item_Helper_Test.php +++ b/modules/gallery/tests/Item_Helper_Test.php @@ -41,6 +41,11 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case { ORM::factory("item")->viewable()->where("id", "=", $item->id)->count_all()); } + public function convert_filename_to_title_test() { + $this->assert_equal("foo", item::convert_filename_to_title("foo.jpg")); + $this->assert_equal("foo.bar", item::convert_filename_to_title("foo.bar.jpg")); + } + public function convert_filename_to_slug_test() { $this->assert_equal("foo", item::convert_filename_to_slug("{[foo]}")); $this->assert_equal("foo-bar", item::convert_filename_to_slug("{[foo!@#!$@#^$@($!(@bar]}")); -- cgit v1.2.3 From 2492280cc0ec9eb64a8daeccc7b5698ece7fea66 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 15 Jun 2010 12:52:28 -0700 Subject: Change the item rest update processing to call the item::move(source, target) helper when the parent member has changed. Using the move method insures that names and slugs that could conflict in the target album are resolved properly. Changed the item::move method so it returns a message to be displayed if the caller chooses. And changed the move controller to display the message returned by the move if the item name was renamed as part of the move. --- modules/gallery/controllers/move.php | 10 ++++------ modules/gallery/helpers/item.php | 15 +++++++++------ modules/gallery/helpers/item_rest.php | 16 +++++++++------- 3 files changed, 22 insertions(+), 19 deletions(-) (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/controllers/move.php b/modules/gallery/controllers/move.php index f8b85b6f..3ce44546 100644 --- a/modules/gallery/controllers/move.php +++ b/modules/gallery/controllers/move.php @@ -34,12 +34,10 @@ class Move_Controller extends Controller { $source = ORM::factory("item", $source_id); $target = ORM::factory("item", Input::instance()->post("target_id")); - access::required("view", $source); - access::required("edit", $source); - access::required("view", $target); - access::required("edit", $target); - - item::move($source, $target); + $message = item::move($source, $target); + if (!empty($message)) { + message.info($message); + } print json_encode( array("result" => "success", diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 15bbe977..6a740de4 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -47,27 +47,28 @@ class item_Core { $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME); $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION); $orig_slug = $source->slug; + $message = ""; for ($i = 0; $i < 5; $i++) { try { $source->save(); if ($orig_name != $source->name) { switch ($source->type) { case "album": - message::info( + $message = t("Album %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); + array("old_name" => $orig_name, "new_name" => $source->name)); break; case "photo": - message::info( + $message = t("Photo %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); + array("old_name" => $orig_name, "new_name" => $source->name)); break; case "movie": - message::info( + $message = t("Movie %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); + array("old_name" => $orig_name, "new_name" => $source->name)); break; } } @@ -95,6 +96,8 @@ class item_Core { if ($target->album_cover_item_id == null) { item::make_album_cover($source); } + + return $message; } static function make_album_cover($item) { diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 0839b144..692d0895 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -99,7 +99,7 @@ class item_rest_Core { if ($entity = $request->params->entity) { // Only change fields from a whitelist. foreach (array("album_cover", "captured", "description", - "height", "mime_type", "name", "parent", "rand_key", "resize_dirty", + "height", "mime_type", "name", "rand_key", "resize_dirty", "resize_height", "resize_width", "slug", "sort_column", "sort_order", "thumb_dirty", "thumb_height", "thumb_width", "title", "view_count", "width") as $key) { @@ -113,11 +113,6 @@ class item_rest_Core { break; case "parent": - if (property_exists($entity, "parent")) { - $parent = rest::resolve($entity->parent); - access::required("edit", $parent); - $item->parent_id = $parent->id; - } break; default: if (property_exists($entity, $key)) { @@ -125,8 +120,15 @@ class item_rest_Core { } } } + + // There is an explicit save in item::move + if (property_exists($entity, "parent")) { + $parent = rest::resolve($entity->parent); + item::move($item, $parent); + } else { + $item->save(); + } } - $item->save(); if (isset($request->params->members) && $item->sort_column == "weight") { $weight = 0; -- cgit v1.2.3 From 9504f71efcadc7ed27f6f09e5d663e8025bf3b86 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 15 Jun 2010 14:18:23 -0700 Subject: Fix for ticket #1118. Create a item::save_with_retries helper method, which encapsulates saving an item and handling name and slug conflicts. Call this instead of doing a save directly. --- modules/gallery/controllers/simple_uploader.php | 10 +++++++--- modules/gallery/helpers/item.php | 17 +++++++++++------ modules/gallery/helpers/item_rest.php | 8 +++----- 3 files changed, 21 insertions(+), 14 deletions(-) (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php index c7e5031b..8ac1fc8b 100644 --- a/modules/gallery/controllers/simple_uploader.php +++ b/modules/gallery/controllers/simple_uploader.php @@ -65,12 +65,16 @@ class Simple_Uploader_Controller extends Controller { if (array_key_exists("extension", $path_info) && in_array(strtolower($path_info["extension"]), array("flv", "mp4"))) { $item->type = "movie"; - $item->save(); + } else { + $item->type = "photo"; + } + + item::save_with_retries($item); + + if ($item->type == "movie") { log::success("content", t("Added a movie"), html::anchor("movies/$item->id", t("view movie"))); } else { - $item->type = "photo"; - $item->save(); log::success("content", t("Added a photo"), html::anchor("photos/$item->id", t("view photo"))); } diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 6a740de4..0710d8b2 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -43,6 +43,17 @@ class item_Core { // Moving may result in name or slug conflicts. If that happens, try up to 5 times to pick a // random name (or slug) to avoid the conflict. + $message = item::save_with_retries($source); + + // If the target has no cover item, make this it. + if ($target->album_cover_item_id == null) { + item::make_album_cover($source); + } + + return $message; + } + + static function save_with_retries($source, $retries=5) { $orig_name = $source->name; $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME); $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION); @@ -91,12 +102,6 @@ class item_Core { } } } - - // If the target has no cover item, make this it. - if ($target->album_cover_item_id == null) { - item::make_album_cover($source); - } - return $message; } diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 692d0895..74fab2e7 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -112,8 +112,6 @@ class item_rest_Core { } break; - case "parent": - break; default: if (property_exists($entity, $key)) { $item->$key = $entity->$key; @@ -126,7 +124,7 @@ class item_rest_Core { $parent = rest::resolve($entity->parent); item::move($item, $parent); } else { - $item->save(); + $item::save_with_retries($item); } } @@ -157,7 +155,7 @@ class item_rest_Core { $item->title = isset($entity->title) ? $entity->title : $entity->name; $item->description = isset($entity->description) ? $entity->description : null; $item->slug = isset($entity->slug) ? $entity->slug : null; - $item->save(); + $item::save_with_retries($item); break; case "photo": @@ -172,7 +170,7 @@ class item_rest_Core { $item->title = isset($entity->title) ? $entity->title : $entity->name; $item->description = isset($entity->description) ? $entity->description : null; $item->slug = isset($entity->slug) ? $entity->slug : null; - $item->save(); + $item::save_with_retries($item); break; default: -- cgit v1.2.3 From 48dc07dbc8189eb16f97b7013b0481982286ab2c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 15 Jun 2010 17:17:25 -0700 Subject: Revert "Fix for ticket #1118. Create a item::save_with_retries helper method, which encapsulates saving an item and handling name and slug conflicts. Call this instead of doing a save directly." Rolled this back because it fails KISS. We already have an API for saving models with Item_Model::save() that's consistent with all of our other model code. Adding a new way to save items is confusing and inconsistent. This reverts commit 9504f71efcadc7ed27f6f09e5d663e8025bf3b86. --- modules/gallery/controllers/simple_uploader.php | 10 +++------- modules/gallery/helpers/item.php | 17 ++++++----------- modules/gallery/helpers/item_rest.php | 8 +++++--- 3 files changed, 14 insertions(+), 21 deletions(-) (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php index 8ac1fc8b..c7e5031b 100644 --- a/modules/gallery/controllers/simple_uploader.php +++ b/modules/gallery/controllers/simple_uploader.php @@ -65,16 +65,12 @@ class Simple_Uploader_Controller extends Controller { if (array_key_exists("extension", $path_info) && in_array(strtolower($path_info["extension"]), array("flv", "mp4"))) { $item->type = "movie"; - } else { - $item->type = "photo"; - } - - item::save_with_retries($item); - - if ($item->type == "movie") { + $item->save(); log::success("content", t("Added a movie"), html::anchor("movies/$item->id", t("view movie"))); } else { + $item->type = "photo"; + $item->save(); log::success("content", t("Added a photo"), html::anchor("photos/$item->id", t("view photo"))); } diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 0710d8b2..6a740de4 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -43,17 +43,6 @@ class item_Core { // Moving may result in name or slug conflicts. If that happens, try up to 5 times to pick a // random name (or slug) to avoid the conflict. - $message = item::save_with_retries($source); - - // If the target has no cover item, make this it. - if ($target->album_cover_item_id == null) { - item::make_album_cover($source); - } - - return $message; - } - - static function save_with_retries($source, $retries=5) { $orig_name = $source->name; $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME); $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION); @@ -102,6 +91,12 @@ class item_Core { } } } + + // If the target has no cover item, make this it. + if ($target->album_cover_item_id == null) { + item::make_album_cover($source); + } + return $message; } diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 74fab2e7..692d0895 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -112,6 +112,8 @@ class item_rest_Core { } break; + case "parent": + break; default: if (property_exists($entity, $key)) { $item->$key = $entity->$key; @@ -124,7 +126,7 @@ class item_rest_Core { $parent = rest::resolve($entity->parent); item::move($item, $parent); } else { - $item::save_with_retries($item); + $item->save(); } } @@ -155,7 +157,7 @@ class item_rest_Core { $item->title = isset($entity->title) ? $entity->title : $entity->name; $item->description = isset($entity->description) ? $entity->description : null; $item->slug = isset($entity->slug) ? $entity->slug : null; - $item::save_with_retries($item); + $item->save(); break; case "photo": @@ -170,7 +172,7 @@ class item_rest_Core { $item->title = isset($entity->title) ? $entity->title : $entity->name; $item->description = isset($entity->description) ? $entity->description : null; $item->slug = isset($entity->slug) ? $entity->slug : null; - $item::save_with_retries($item); + $item->save(); break; default: -- cgit v1.2.3 From a432a43b3b39fbec70d4cece1eb0ba5625b2679c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 15 Jun 2010 17:18:22 -0700 Subject: Revert "Change the item rest update processing to call the item::move(source, target) helper when the parent member has changed. Using the move method insures that names and slugs that could conflict in the target album are resolved properly. Changed the item::move method so it returns a message to be displayed if the caller chooses. And changed the move controller to display the message returned by the move if the item name was renamed as part of the move." Rolling this back for a couple of reasons: 1) Bug in move.php ("message.info" is not a function name) 2) Having the message come back from the API call as a side-effect is sloppy. We should find a cleaner way to do this checking. 3) having item::move() call save() on any changed values in the ORM is counter-intuitive. Move should move, save should save. I think the right approach here is to roll the move() code properly into save(). This reverts commit 2492280cc0ec9eb64a8daeccc7b5698ece7fea66. --- modules/gallery/controllers/move.php | 10 ++++++---- modules/gallery/helpers/item.php | 15 ++++++--------- modules/gallery/helpers/item_rest.php | 16 +++++++--------- 3 files changed, 19 insertions(+), 22 deletions(-) (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/controllers/move.php b/modules/gallery/controllers/move.php index 3ce44546..f8b85b6f 100644 --- a/modules/gallery/controllers/move.php +++ b/modules/gallery/controllers/move.php @@ -34,10 +34,12 @@ class Move_Controller extends Controller { $source = ORM::factory("item", $source_id); $target = ORM::factory("item", Input::instance()->post("target_id")); - $message = item::move($source, $target); - if (!empty($message)) { - message.info($message); - } + access::required("view", $source); + access::required("edit", $source); + access::required("view", $target); + access::required("edit", $target); + + item::move($source, $target); print json_encode( array("result" => "success", diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 6a740de4..15bbe977 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -47,28 +47,27 @@ class item_Core { $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME); $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION); $orig_slug = $source->slug; - $message = ""; for ($i = 0; $i < 5; $i++) { try { $source->save(); if ($orig_name != $source->name) { switch ($source->type) { case "album": - $message = + message::info( t("Album %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name)); + array("old_name" => $orig_name, "new_name" => $source->name))); break; case "photo": - $message = + message::info( t("Photo %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name)); + array("old_name" => $orig_name, "new_name" => $source->name))); break; case "movie": - $message = + message::info( t("Movie %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name)); + array("old_name" => $orig_name, "new_name" => $source->name))); break; } } @@ -96,8 +95,6 @@ class item_Core { if ($target->album_cover_item_id == null) { item::make_album_cover($source); } - - return $message; } static function make_album_cover($item) { diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 692d0895..0839b144 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -99,7 +99,7 @@ class item_rest_Core { if ($entity = $request->params->entity) { // Only change fields from a whitelist. foreach (array("album_cover", "captured", "description", - "height", "mime_type", "name", "rand_key", "resize_dirty", + "height", "mime_type", "name", "parent", "rand_key", "resize_dirty", "resize_height", "resize_width", "slug", "sort_column", "sort_order", "thumb_dirty", "thumb_height", "thumb_width", "title", "view_count", "width") as $key) { @@ -113,6 +113,11 @@ class item_rest_Core { break; case "parent": + if (property_exists($entity, "parent")) { + $parent = rest::resolve($entity->parent); + access::required("edit", $parent); + $item->parent_id = $parent->id; + } break; default: if (property_exists($entity, $key)) { @@ -120,15 +125,8 @@ class item_rest_Core { } } } - - // There is an explicit save in item::move - if (property_exists($entity, "parent")) { - $parent = rest::resolve($entity->parent); - item::move($item, $parent); - } else { - $item->save(); - } } + $item->save(); if (isset($request->params->members) && $item->sort_column == "weight") { $weight = 0; -- cgit v1.2.3 From 892727830d873a9f0a1a49f10ee14b0890088b23 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 20 Jun 2010 16:52:10 -0700 Subject: Add a loading indicator to the delete form by tagging some JS on at the end which triggers .gallery_show_loading(). Not a complete fix for #817 but it's a start and it takes care of one place where we have a long running process. --- modules/gallery/helpers/item.php | 2 ++ modules/gallery/js/item_form_delete.js | 5 +++++ 2 files changed, 7 insertions(+) create mode 100644 modules/gallery/js/item_form_delete.js (limited to 'modules/gallery/helpers/item.php') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 15bbe977..aef68c6e 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -162,6 +162,8 @@ class item_Core { "quick/delete/$item->id?page_type=$page_type", "", "post", array("id" => "g-confirm-delete")); $group = $form->group("confirm_delete")->label(t("Confirm Deletion")); $group->submit("")->value(t("Delete")); + $form->script("") + ->url(url::abs_file("modules/gallery/js/item_form_delete.js")); return $form; } diff --git a/modules/gallery/js/item_form_delete.js b/modules/gallery/js/item_form_delete.js new file mode 100644 index 00000000..fa3f24a2 --- /dev/null +++ b/modules/gallery/js/item_form_delete.js @@ -0,0 +1,5 @@ +$("#g-confirm-delete").submit( + function() { + $("#g-confirm-delete input[type=submit]").gallery_show_loading(); + } +); -- cgit v1.2.3