Simplification of 59b04b897b8a664cd7334b017fac8158a6281434 for #1764:

- Breadcrumb::build_from_item becomes Breadcrumb::array_from_item_parents
- Eliminate Breadcrumb::$id -- it's no longer necessary
- Fold Breadcrumb::generate_show_query_strings into Breadcrumb::array_from_item_parents
- Create Breadcrumb::set_first() and Breadcrumb::set_last()
- Breadcrumb::build_from_list goes away, we just use arrays for this
- Change Search_Controller and Tag_Controller to just create an array
  of Breadcrumb instances with the first/last marked appropriately
- Breadcrumb_Test loses a bunch of complexity.

5 files changed, 45 insertions, 64 deletions

diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php
index 90071fb5..1c48c734 100644
--- a/modules/gallery/controllers/albums.php
+++ b/modules/gallery/controllers/albums.php
@@ -69,7 +69,7 @@ class Albums_Controller extends Items_Controller {
             "item" => $album,
             "children" => $album->viewable()->children($page_size, $offset),
             "parents" => $album->parents()->as_array(), // view calls empty() on this
-            "breadcrumbs" => Breadcrumb::build_from_item($album),
+            "breadcrumbs" => Breadcrumb::array_from_item_parents($album),
             "children_count" => $children_count));
     $template->content = new View("album.html");
 
diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php
index 77d92e13..0f12c3fb 100644
--- a/modules/gallery/controllers/movies.php
+++ b/modules/gallery/controllers/movies.php
@@ -43,7 +43,7 @@ class Movies_Controller extends Items_Controller {
             "children" => array(),
             "children_count" => 0,
             "parents" => $movie->parents()->as_array(),
-            "breadcrumbs" => Breadcrumb::build_from_item($movie),
+            "breadcrumbs" => Breadcrumb::array_from_item_parents($movie),
             "next_item" => $next_item,
             "previous_item" => $previous_item,
             "sibling_count" => $movie->parent()->viewable()->children_count($where),
diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php
index 5c8c7b34..af8aed16 100644
--- a/modules/gallery/controllers/photos.php
+++ b/modules/gallery/controllers/photos.php
@@ -43,7 +43,7 @@ class Photos_Controller extends Items_Controller {
             "children" => array(),
             "children_count" => 0,
             "parents" => $photo->parents()->as_array(),
-            "breadcrumbs" => Breadcrumb::build_from_item($photo),
+            "breadcrumbs" => Breadcrumb::array_from_item_parents($photo),
             "next_item" => $next_item,
             "previous_item" => $previous_item,
             "sibling_count" => $photo->parent()->viewable()->children_count($where),
diff --git a/modules/gallery/libraries/Breadcrumb.php b/modules/gallery/libraries/Breadcrumb.php
index 3a2499ba..e151890d 100644
--- a/modules/gallery/libraries/Breadcrumb.php
+++ b/modules/gallery/libraries/Breadcrumb.php
@@ -20,55 +20,51 @@
 class Breadcrumb_Core {
   public $title;
   public $url;
-  public $id;
   public $first;
   public $last;
 
-  static function build_from_item($item) {
-    $breadcrumbs = array();
-    foreach ($item->parents() as $element) {
-      $breadcrumbs[] = new Breadcrumb($element->title, $element->url(), $element->id);
-    }
-
-    if (!empty($breadcrumbs)) {
-      $breadcrumbs[] = new Breadcrumb($item->title, $item->url(), $item->id);
-    }
+  static function instance($title, $url) {
+    return new Breadcrumb($title, $url);
+  }
 
-    return self::generate_show_query_strings($breadcrumbs);
+  public function __construct($title, $url) {
+    $this->title = $title;
+    $this->url = $url;
+    $this->first = false;
+    $this->last = false;
   }
 
   /**
-   * This static function takes a list (variable arguments) of Breadcrumbs and builds a dynamic
-   * breadcrumb list.  Used to create a breadcrumb for dynamic albums. Will really be useful
-   * for the display context change.
+   * Return an array of Breadcrumb instances build from the parents of a given item.
+   * The first and last Breadcrumb instances will be marked first/last as appropriate.
+   * Each breadcrumb will have a ?show= query parameter that refers to the id of the next
+   * item in line.
+   *
+   * @return array Breadcrumb instances
    */
-  static function build_from_list() {
-    return self::generate_show_query_strings(func_get_args());
-  }
-
-  private static function generate_show_query_strings($breadcrumbs) {
-    if (!empty($breadcrumbs)) {
+  static function array_from_item_parents($item) {
+    if ($item->id == item::root()->id) {
+      return array();
+    }
 
-      end($breadcrumbs)->last = true;;
-      while ($breadcrumb = current($breadcrumbs)) {
-        if (isset($last_id) && $last_id > 0) {
-          $query = parse_url($breadcrumb->url, PHP_URL_QUERY);
-          $breadcrumb->url =  $breadcrumb->url . ($query ? "&" : "?") . "show={$last_id}";
-        }
-        $last_id = $breadcrumb->id;
-        $breadcrumb = prev($breadcrumbs);
-      }
-      $breadcrumbs[0]->first = true;
+    $bc = array_merge($item->parents()->as_array(), array($item));
+    for ($i = 0; $i < count($bc) - 1; $i++) {
+      $bc[$i] = new Breadcrumb($bc[$i]->title, $bc[$i]->url("show={$bc[$i+1]->id}"));
     }
+    $bc[$i] = new Breadcrumb($item->title, $item->url());
 
-    return $breadcrumbs;
+    $bc[0]->set_first();
+    end($bc)->set_last();
+    return $bc;
   }
 
-  public function __construct($title, $url, $id=0) {
-    $this->title = $title;
-    $this->url = $url;
-    $this->id = $id;
-    $this->first = false;
-    $this->last = false;
+  public function set_first() {
+    $this->first = true;
+    return $this;
+  }
+
+  public function set_last() {
+    $this->last = true;
+    return $this;
   }
 }
diff --git a/modules/gallery/tests/Breadcrumb_Test.php b/modules/gallery/tests/Breadcrumb_Test.php
index 9f9aeddc..ed2e5608 100644
--- a/modules/gallery/tests/Breadcrumb_Test.php
+++ b/modules/gallery/tests/Breadcrumb_Test.php
@@ -21,31 +21,16 @@ class Breadcrumb_Test extends Gallery_Unit_Test_Case {
   private $album;
   private $item;
 
-  public function setup() {
-    $this->album = test::random_album();
-    $this->item = test::random_photo($this->album);
-    $this->album->reload();
-  }
-
-  public function teardown() {
-    $this->album = null;
-    $this->item = null;
-  }
-
   public function build_breadcrumbs_for_item_test() {
-    $breadcrumbs = Breadcrumb::build_from_item($this->item);
-    $this->assert_equal("Gallery", $breadcrumbs[0]->title);
-    $this->assert_equal($this->album->title, $breadcrumbs[1]->title);
-    $this->assert_equal($this->item->title, $breadcrumbs[2]->title);
-  }
+    $album = test::random_album();
+    $item = test::random_photo($album);
 
-  public function build_breadcrumbs_from_items_test() {
-    $breadcrumbs = Breadcrumb::build_from_list(
-      new Breadcrumb(item::root()->title, "/", item::root()->id),
-      new Breadcrumb($this->album->title, $this->album->relative_path(), $this->album->id),
-      new Breadcrumb($this->item->title, $this->item->relative_path(), $this->item->id));
-    $this->assert_equal("Gallery", $breadcrumbs[0]->title);
-    $this->assert_equal($this->album->title, $breadcrumbs[1]->title);
-    $this->assert_equal($this->item->title, $breadcrumbs[2]->title);
+    $expected = array();
+    $expected[] = Breadcrumb::instance(
+      item::root()->title, item::root()->url("show={$album->id}"))->set_first();
+    $expected[] =
+      Breadcrumb::instance($album->title, $album->url("show={$item->id}"));
+    $expected[] = Breadcrumb::instance($item->title, $item->url())->set_last();
+    $this->assert_equal($expected, Breadcrumb::array_from_item_parents($item));
   }
 }
\ No newline at end of file