Added smarter page finding so changing the page name does not break old urls

Added page & book slug history to revisions so they can be looked up if a page is not found.

diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php
index 2dbdb81e787c7889e0d04008929d9d81caebf643..e78ae13e4c2972067ef9ed4232f6771426ab852d 100644 (file)
--- a/app/Http/Controllers/PageController.php
+++ b/app/Http/Controllers/PageController.php
@@ -11,6 +11,7 @@ use BookStack\Http\Requests;
 use BookStack\Repos\BookRepo;
 use BookStack\Repos\ChapterRepo;
 use BookStack\Repos\PageRepo;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Views;
 
 class PageController extends Controller
@@ -81,6 +82,8 @@ class PageController extends Controller
 
     /**
      * Display the specified page.
+     * If the page is not found via the slug the
+     * revisions are searched for a match.
      *
      * @param $bookSlug
      * @param $pageSlug
@@ -89,7 +92,15 @@ class PageController extends Controller
     public function show($bookSlug, $pageSlug)
     {
         $book = $this->bookRepo->getBySlug($bookSlug);
-        $page = $this->pageRepo->getBySlug($pageSlug, $book->id);
+
+        try {
+            $page = $this->pageRepo->getBySlug($pageSlug, $book->id);
+        } catch (NotFoundHttpException $e) {
+            $page = $this->pageRepo->findPageUsingOldSlug($pageSlug, $bookSlug);
+            if ($page === null) abort(404);
+            return redirect($page->getUrl());
+        }
+
         $sidebarTree = $this->bookRepo->getChildren($book);
         Views::add($page);
         $this->setPageTitle($page->getShortName());

diff --git a/app/Repos/PageRepo.php b/app/Repos/PageRepo.php
index 494e1e9eec62cc7ba779dd8caf10ff5ab342845b..f028a1fccfa642890a9fc1f9aa4e8aec73cdd0c6 100644 (file)
--- a/app/Repos/PageRepo.php
+++ b/app/Repos/PageRepo.php
@@ -10,6 +10,7 @@ use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Str;
 use BookStack\Page;
 use BookStack\PageRevision;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
 class PageRepo
 {
@@ -65,11 +66,28 @@ class PageRepo
     public function getBySlug($slug, $bookId)
     {
         $page = $this->page->where('slug', '=', $slug)->where('book_id', '=', $bookId)->first();
-        if ($page === null) abort(404);
+        if ($page === null) throw new NotFoundHttpException('Page not found');
         return $page;
     }
 
     /**
+     * Search through page revisions and retrieve
+     * the last page in the current book that
+     * has a slug equal to the one given.
+     * @param $pageSlug
+     * @param $bookSlug
+     * @return null | Page
+     */
+    public function findPageUsingOldSlug($pageSlug, $bookSlug)
+    {
+        $revision = $this->pageRevision->where('slug', '=', $pageSlug)
+            ->where('book_slug', '=', $bookSlug)->orderBy('created_at', 'desc')
+            ->with('page')->first();
+        return $revision !== null ? $revision->page : null;
+    }
+
+    /**
+     * Get a new Page instance from the given input.
      * @param $input
      * @return Page
      */
@@ -245,9 +263,13 @@ class PageRepo
             $this->saveRevision($page);
         }
 
+        // Prevent slug being updated if no name change
+        if ($page->name !== $input['name']) {
+            $page->slug = $this->findSuitableSlug($input['name'], $book_id, $page->id);
+        }
+
         // Update with new details
         $page->fill($input);
-        $page->slug = $this->findSuitableSlug($page->name, $book_id, $page->id);
         $page->html = $this->formatHtml($input['html']);
         $page->text = strip_tags($page->html);
         $page->updated_by = auth()->user()->id;
@@ -283,6 +305,8 @@ class PageRepo
     {
         $revision = $this->pageRevision->fill($page->toArray());
         $revision->page_id = $page->id;
+        $revision->slug = $page->slug;
+        $revision->book_slug = $page->book->slug;
         $revision->created_by = auth()->user()->id;
         $revision->created_at = $page->updated_at;
         $revision->save();

diff --git a/database/migrations/2016_02_25_184030_add_slug_to_revisions.php b/database/migrations/2016_02_25_184030_add_slug_to_revisions.php
new file mode 100644 (file)
index 0000000..0be6c79
--- /dev/null
+++ b/database/migrations/2016_02_25_184030_add_slug_to_revisions.php
@@ -0,0 +1,35 @@
+<?php
+
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Database\Migrations\Migration;
+
+class AddSlugToRevisions extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('page_revisions', function (Blueprint $table) {
+            $table->string('slug');
+            $table->index('slug');
+            $table->string('book_slug');
+            $table->index('book_slug');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('page_revisions', function (Blueprint $table) {
+            $table->dropColumn('slug');
+            $table->dropColumn('book_slug');
+        });
+    }
+}

diff --git a/tests/EntityTest.php b/tests/EntityTest.php
index c04bc6d7c850d98a1189f23d05c9ad8241ff9c65..2936fc0475abe698fdbe420a6f1ece8718dadae0 100644 (file)
--- a/tests/EntityTest.php
+++ b/tests/EntityTest.php
@@ -211,5 +211,18 @@ class EntityTest extends TestCase
             ->seeInNthElement('.entity-list .page', 0, $content['page']->name);
     }
 
+    public function test_old_page_slugs_redirect_to_new_pages()
+    {
+        $page = \BookStack\Page::all()->first();
+        $pageUrl = $page->getUrl();
+        $newPageUrl = '/books/' . $page->book->slug . '/page/super-test-page';
+        $this->asAdmin()->visit($pageUrl)
+            ->clickInElement('#content', 'Edit')
+            ->type('super test page', '#name')
+            ->press('Save Page')
+            ->seePageIs($newPageUrl)
+            ->visit($pageUrl)
+            ->seePageIs($newPageUrl);
+    }
 
 }