#1946, 1947 - Make altered names/slugs more user-friendly, make conflict-finding code check filenames with no extensions

- Reduced from four places that alter names/slugs to two (one to populate empty slugs, one to handle conflicting names/slugs).
- For empty slugs, fill with generic human-readable name (e.g. "photo") instead of random value.
- For conflicting names/slugs, add suffix that's sequential (e.g. "-01"), only using random after 99 conflicts.
- Made conflict-finding code check filenames with no extensions.
- Renamed _randomize_name_or_slug_on_conflict to _check_and_fix_conflicts.
- Added unit tests.  Also cleaned up existing unit tests to reflect new logic and removed a redundant test.
- Added installer logic to correct existing items now considered in conflict.
- Revised gallery_task to look for duplicate names based on new criteria.

1 files changed, 18 insertions, 45 deletions

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index b2c4cadf..386eeb08 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -39,55 +39,28 @@ class item_Core {
       }
     }
 
-    $source->parent_id = $target->id;
-
-    // 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.
     $orig_name = $source->name;
-    $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME);
-    $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION);
-    $orig_slug = $source->slug;
-    for ($i = 0; $i < 5; $i++) {
-      try {
-        $source->save();
-        if ($orig_name != $source->name) {
-          switch ($source->type) {
-          case "album":
-            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)));
-            break;
-
-          case "photo":
-            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)));
-            break;
+    $source->parent_id = $target->id;
+    $source->save();
+    if ($orig_name != $source->name) {
+      switch ($source->type) {
+      case "album":
+        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)));
+        break;
 
-          case "movie":
-            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)));
-            break;
-          }
-        }
+      case "photo":
+        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)));
         break;
-      } catch (ORM_Validation_Exception $e) {
-        $rand = rand(10, 99);
-        $errors = $e->validation->errors();
-        if (isset($errors["name"])) {
-          $source->name = $orig_name_filename . "-{$rand}." . $orig_name_extension;
-          unset($errors["name"]);
-        }
-        if (isset($errors["slug"])) {
-          $source->slug = $orig_slug . "-{$rand}";
-          unset($errors["slug"]);
-        }
 
-        if ($errors) {
-          // There were other validation issues-- we don't know how to handle those
-          throw $e;
-        }
+      case "movie":
+        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)));
+        break;
       }
     }