Skip to content

Commit

Permalink
Revert "Change the item rest update processing to call the
Browse files Browse the repository at this point in the history
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 2492280.
  • Loading branch information
bharat committed Jun 16, 2010
1 parent 48dc07d commit a432a43
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 22 deletions.
10 changes: 6 additions & 4 deletions modules/gallery/controllers/move.php
Expand Up @@ -34,10 +34,12 @@ public function save($source_id) {
$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",
Expand Down
15 changes: 6 additions & 9 deletions modules/gallery/helpers/item.php
Expand Up @@ -47,28 +47,27 @@ static function move($source, $target) {
$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 <b>%old_name</b> renamed to <b>%new_name</b> 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 <b>%old_name</b> renamed to <b>%new_name</b> 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 <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict",
array("old_name" => $orig_name, "new_name" => $source->name));
array("old_name" => $orig_name, "new_name" => $source->name)));
break;
}
}
Expand Down Expand Up @@ -96,8 +95,6 @@ static function move($source, $target) {
if ($target->album_cover_item_id == null) {
item::make_album_cover($source);
}

return $message;
}

static function make_album_cover($item) {
Expand Down
16 changes: 7 additions & 9 deletions modules/gallery/helpers/item_rest.php
Expand Up @@ -99,7 +99,7 @@ static function put($request) {
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) {
Expand All @@ -113,22 +113,20 @@ static function put($request) {
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)) {
$item->$key = $entity->$key;
}
}
}

// 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;
Expand Down

0 comments on commit a432a43

Please sign in to comment.