Switched to database-based tracking for page editor

- Works better to avoid bad assumptions when showing the editor based
  upon content type.
- Also updated some previous tests to cleaner format.

diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php
index c8217af576d84af0e1aa4aa4d98526d222c471ac..ed69bcf8b8569d2e92fe94444d541872003a849b 100644 (file)
--- a/app/Entities/Models/Page.php
+++ b/app/Entities/Models/Page.php
@@ -22,6 +22,7 @@ use Illuminate\Database\Eloquent\Relations\HasOne;
  * @property bool         $template
  * @property bool         $draft
  * @property int          $revision_count
  * @property bool         $template
  * @property bool         $draft
  * @property int          $revision_count

+ * @property string       $editor

  * @property Chapter      $chapter
  * @property Collection   $attachments
  * @property Collection   $revisions
  * @property Chapter      $chapter
  * @property Collection   $attachments
  * @property Collection   $revisions

diff --git a/app/Entities/Models/PageRevision.php b/app/Entities/Models/PageRevision.php
index 55b2ffbe805c835277f2800247a021254f8c2573..be2ac33a0958fc26dddfd12ec9795f8448818a7e 100644 (file)
--- a/app/Entities/Models/PageRevision.php
+++ b/app/Entities/Models/PageRevision.php
@@ -12,6 +12,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
  *
  * @property mixed  $id
  * @property int    $page_id
  *
  * @property mixed  $id
  * @property int    $page_id

+ * @property string $name

  * @property string $slug
  * @property string $book_slug
  * @property int    $created_by
  * @property string $slug
  * @property string $book_slug
  * @property int    $created_by


@@ -21,6 +22,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
  * @property string $summary
  * @property string $markdown
  * @property string $html
  * @property string $summary
  * @property string $markdown
  * @property string $html

+ * @property string $text

  * @property int    $revision_number
  * @property Page   $page
  * @property-read ?User $createdBy
  * @property int    $revision_number
  * @property Page   $page
  * @property-read ?User $createdBy

diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php
index d47573a6cf516c80f4798618fb370af3e0bf50ae..c106d2fd30679d6f663cdd845ddbaf1b0aee4fe5 100644 (file)
--- a/app/Entities/Repos/PageRepo.php
+++ b/app/Entities/Repos/PageRepo.php
@@ -10,6 +10,7 @@ use BookStack\Entities\Models\Page;
 use BookStack\Entities\Models\PageRevision;
 use BookStack\Entities\Tools\BookContents;
 use BookStack\Entities\Tools\PageContent;
 use BookStack\Entities\Models\PageRevision;
 use BookStack\Entities\Tools\BookContents;
 use BookStack\Entities\Tools\PageContent;

+use BookStack\Entities\Tools\PageEditorData;

 use BookStack\Entities\Tools\TrashCan;
 use BookStack\Exceptions\MoveOperationException;
 use BookStack\Exceptions\NotFoundException;
 use BookStack\Entities\Tools\TrashCan;
 use BookStack\Exceptions\MoveOperationException;
 use BookStack\Exceptions\NotFoundException;


@@ -217,11 +218,25 @@ class PageRepo
         }
 
         $pageContent = new PageContent($page);
         }
 
         $pageContent = new PageContent($page);

-        if (!empty($input['markdown'] ?? '')) {
+        $currentEditor = $page->editor ?: PageEditorData::getSystemDefaultEditor();
+        $newEditor = $currentEditor;
+
+        $haveInput = isset($input['markdown']) || isset($input['html']);
+        $inputEmpty = empty($input['markdown']) && empty($input['html']);
+
+        if ($haveInput && $inputEmpty) {
+            $pageContent->setNewHTML('');
+        } elseif (!empty($input['markdown']) && is_string($input['markdown'])) {
+            $newEditor = 'markdown';

             $pageContent->setNewMarkdown($input['markdown']);
         } elseif (isset($input['html'])) {
             $pageContent->setNewMarkdown($input['markdown']);
         } elseif (isset($input['html'])) {

+            $newEditor = 'wysiwyg';

             $pageContent->setNewHTML($input['html']);
         }
             $pageContent->setNewHTML($input['html']);
         }

+
+        if ($newEditor !== $currentEditor && userCan('editor-change')) {
+            $page->editor = $newEditor;
+        }

     }
 
     /**
     }
 
     /**


@@ -229,8 +244,12 @@ class PageRepo
      */
     protected function savePageRevision(Page $page, string $summary = null): PageRevision
     {
      */
     protected function savePageRevision(Page $page, string $summary = null): PageRevision
     {

-        $revision = new PageRevision($page->getAttributes());
+        $revision = new PageRevision();

 
 

+        $revision->name = $page->name;
+        $revision->html = $page->html;
+        $revision->markdown = $page->markdown;
+        $revision->text = $page->text;

         $revision->page_id = $page->id;
         $revision->slug = $page->slug;
         $revision->book_slug = $page->book->slug;
         $revision->page_id = $page->id;
         $revision->slug = $page->slug;
         $revision->book_slug = $page->book->slug;

diff --git a/app/Entities/Tools/PageEditorData.php b/app/Entities/Tools/PageEditorData.php
index 3e11641750c9a6cfe6c30f6d7e1ed345da27aa35..72f3391eaebfe38410596f3cbb71ee078b7959f6 100644 (file)
--- a/app/Entities/Tools/PageEditorData.php
+++ b/app/Entities/Tools/PageEditorData.php
@@ -94,10 +94,7 @@ class PageEditorData
      */
     protected function getEditorType(Page $page): string
     {
      */
     protected function getEditorType(Page $page): string
     {

-        $emptyPage = empty($page->html) && empty($page->markdown);
-        $pageType = (!empty($page->html) && empty($page->markdown)) ? 'wysiwyg' : 'markdown';
-        $systemDefault = setting('app-editor') === 'wysiwyg' ? 'wysiwyg' : 'markdown';
-        $editorType = $emptyPage ? $systemDefault : $pageType;
+        $editorType = $page->editor ?: self::getSystemDefaultEditor();

 
         // Use requested editor if valid and if we have permission
         $requestedType = explode('-', $this->requestedEditor)[0];
 
         // Use requested editor if valid and if we have permission
         $requestedType = explode('-', $this->requestedEditor)[0];


@@ -108,4 +105,12 @@ class PageEditorData
         return $editorType;
     }
 
         return $editorType;
     }
 

+    /**
+     * Get the configured system default editor.
+     */
+    public static function getSystemDefaultEditor(): string
+    {
+        return setting('app-editor') === 'markdown' ? 'markdown' : 'wysiwyg';
+    }
+

 }
\ No newline at end of file
 }
\ No newline at end of file

diff --git a/database/migrations/2022_04_17_101741_add_editor_change_field_and_permission.php b/database/migrations/2022_04_17_101741_add_editor_change_field_and_permission.php
new file mode 100644 (file)
index 0000000..e71146d
--- /dev/null
+++ b/database/migrations/2022_04_17_101741_add_editor_change_field_and_permission.php
@@ -0,0 +1,62 @@
+<?php
+
+use Carbon\Carbon;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+class AddEditorChangeFieldAndPermission extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        // Add the new 'editor' column to the pages table
+        Schema::table('pages', function(Blueprint $table) {
+            $table->string('editor', 50)->default('');
+        });
+
+        // Populate the new 'editor' column
+        // We set it to 'markdown' for pages currently with markdown content
+        DB::table('pages')->where('markdown', '!=', '')->update(['editor' => 'markdown']);
+        // We set it to 'wysiwyg' where we have HTML but no markdown
+        DB::table('pages')->where('markdown', '=', '')
+            ->where('html', '!=', '')
+            ->update(['editor' => 'wysiwyg']);
+
+        // Give the admin user permission to change the editor
+        $adminRoleId = DB::table('roles')->where('system_name', '=', 'admin')->first()->id;
+
+        $permissionId = DB::table('role_permissions')->insertGetId([
+            'name'         => 'editor-change',
+            'display_name' => 'Change page editor',
+            'created_at'   => Carbon::now()->toDateTimeString(),
+            'updated_at'   => Carbon::now()->toDateTimeString(),
+        ]);
+
+        DB::table('permission_role')->insert([
+            'role_id'       => $adminRoleId,
+            'permission_id' => $permissionId,
+        ]);
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        // Drop the new column from the pages table
+        Schema::table('pages', function(Blueprint $table) {
+            $table->dropColumn('editor');
+        });
+
+        // Remove traces of the role permission
+        DB::table('role_permissions')->where('name', '=', 'editor-change')->delete();
+    }
+}

diff --git a/database/migrations/2022_04_17_101741_add_editor_change_role_permission.php b/database/migrations/2022_04_17_101741_add_editor_change_role_permission.php
deleted file mode 100644 (file)
index a9f7f83..0000000
--- a/database/migrations/2022_04_17_101741_add_editor_change_role_permission.php
+++ /dev/null
@@ -1,40 +0,0 @@
-<?php
-
-use Carbon\Carbon;
-use Illuminate\Database\Migrations\Migration;
-use Illuminate\Support\Facades\DB;
-
-class AddEditorChangeRolePermission extends Migration
-{
-    /**
-     * Run the migrations.
-     *
-     * @return void
-     */
-    public function up()
-    {
-        $adminRoleId = DB::table('roles')->where('system_name', '=', 'admin')->first()->id;
-
-        $permissionId = DB::table('role_permissions')->insertGetId([
-            'name'         => 'editor-change',
-            'display_name' => 'Change page editor',
-            'created_at'   => Carbon::now()->toDateTimeString(),
-            'updated_at'   => Carbon::now()->toDateTimeString(),
-        ]);
-
-        DB::table('permission_role')->insert([
-            'role_id'       => $adminRoleId,
-            'permission_id' => $permissionId,
-        ]);
-    }
-
-    /**
-     * Reverse the migrations.
-     *
-     * @return void
-     */
-    public function down()
-    {
-        DB::table('role_permissions')->where('name', '=', 'editor-change')->delete();
-    }
-}

diff --git a/tests/Api/PagesApiTest.php b/tests/Api/PagesApiTest.php
index b91d96d892c74402e9ac79990bbcdcad663d56f0..f857db96d83af38659e85cbb71fd348516ee98b6 100644 (file)
--- a/tests/Api/PagesApiTest.php
+++ b/tests/Api/PagesApiTest.php
@@ -252,7 +252,9 @@ class PagesApiTest extends TestCase
             'tags' => [['name' => 'Category', 'value' => 'Testing']]
         ];
 
             'tags' => [['name' => 'Category', 'value' => 'Testing']]
         ];
 

-        $this->putJson($this->baseEndpoint . "/{$page->id}", $details);
+        $resp = $this->putJson($this->baseEndpoint . "/{$page->id}", $details);
+        $resp->assertOk();
+

         $page->refresh();
         $this->assertGreaterThan(Carbon::now()->subDay()->unix(), $page->updated_at->unix());
     }
         $page->refresh();
         $this->assertGreaterThan(Carbon::now()->subDay()->unix(), $page->updated_at->unix());
     }

diff --git a/tests/Entity/PageEditorTest.php b/tests/Entity/PageEditorTest.php
index f489e6b03a6da11da989a15d64b31a08ac91dde6..85f6779c38add2c5724801217f70608ffadca0f2 100644 (file)
--- a/tests/Entity/PageEditorTest.php
+++ b/tests/Entity/PageEditorTest.php
@@ -18,36 +18,39 @@ class PageEditorTest extends TestCase
         $this->page = Page::query()->first();
     }
 
         $this->page = Page::query()->first();
     }
 

-    public function test_default_editor_is_wysiwyg()
+    public function test_default_editor_is_wysiwyg_for_new_pages()

     {
         $this->assertEquals('wysiwyg', setting('app-editor'));
     {
         $this->assertEquals('wysiwyg', setting('app-editor'));

-        $this->asAdmin()->get($this->page->getUrl() . '/edit')
-            ->assertElementExists('#html-editor');
+        $resp = $this->asAdmin()->get($this->page->book->getUrl('/create-page'));
+        $this->followRedirects($resp)->assertElementExists('#html-editor');

     }
 
     }
 

-    public function test_markdown_setting_shows_markdown_editor()
+    public function test_markdown_setting_shows_markdown_editor_for_new_pages()

     {
         $this->setSettings(['app-editor' => 'markdown']);
     {
         $this->setSettings(['app-editor' => 'markdown']);

-        $this->asAdmin()->get($this->page->getUrl() . '/edit')
+
+        $resp = $this->asAdmin()->get($this->page->book->getUrl('/create-page'));
+        $this->followRedirects($resp)

             ->assertElementNotExists('#html-editor')
             ->assertElementExists('#markdown-editor');
     }
 
     public function test_markdown_content_given_to_editor()
     {
             ->assertElementNotExists('#html-editor')
             ->assertElementExists('#markdown-editor');
     }
 
     public function test_markdown_content_given_to_editor()
     {

-        $this->setSettings(['app-editor' => 'markdown']);
-

         $mdContent = '# hello. This is a test';
         $this->page->markdown = $mdContent;
         $mdContent = '# hello. This is a test';
         $this->page->markdown = $mdContent;

+        $this->page->editor = 'markdown';

         $this->page->save();
 
         $this->page->save();
 

-        $this->asAdmin()->get($this->page->getUrl() . '/edit')
+        $this->asAdmin()->get($this->page->getUrl('/edit'))

             ->assertElementContains('[name="markdown"]', $mdContent);
     }
 
     public function test_html_content_given_to_editor_if_no_markdown()
     {
             ->assertElementContains('[name="markdown"]', $mdContent);
     }
 
     public function test_html_content_given_to_editor_if_no_markdown()
     {

-        $this->setSettings(['app-editor' => 'markdown']);
+        $this->page->editor = 'markdown';
+        $this->page->save();
+

         $this->asAdmin()->get($this->page->getUrl() . '/edit')
             ->assertElementContains('[name="markdown"]', $this->page->html);
     }
         $this->asAdmin()->get($this->page->getUrl() . '/edit')
             ->assertElementContains('[name="markdown"]', $this->page->html);
     }


@@ -143,44 +146,35 @@ class PageEditorTest extends TestCase
         $resp->assertSee("<h2>A Header</h2>\n<p>Some content with <strong>bold</strong> text!</p>", true);
     }
 
         $resp->assertSee("<h2>A Header</h2>\n<p>Some content with <strong>bold</strong> text!</p>", true);
     }
 

-    public function test_page_editor_depends_on_content_type()
+    public function test_page_editor_changes_with_editor_property()

     {
     {

-        /** @var Page $page */
-        $page = Page::query()->first();
-
-        $resp = $this->asAdmin()->get($page->getUrl('/edit'));
+        $resp = $this->asAdmin()->get($this->page->getUrl('/edit'));

         $resp->assertElementExists('[component="wysiwyg-editor"]');
 
         $resp->assertElementExists('[component="wysiwyg-editor"]');
 

-        $page->html = '';
-        $page->markdown = "## A Header\n\nSome content with **bold** text!";
-        $page->save();
+        $this->page->markdown = "## A Header\n\nSome content with **bold** text!";
+        $this->page->editor = 'markdown';
+        $this->page->save();

 
 

-        $resp = $this->asAdmin()->get($page->getUrl('/edit'));
+        $resp = $this->asAdmin()->get($this->page->getUrl('/edit'));

         $resp->assertElementExists('[component="markdown-editor"]');
     }
 
     public function test_editor_type_switch_options_show()
     {
         $resp->assertElementExists('[component="markdown-editor"]');
     }
 
     public function test_editor_type_switch_options_show()
     {

-        /** @var Page $page */
-        $page = Page::query()->first();
-
-        $resp = $this->asAdmin()->get($page->getUrl('/edit'));
-        $editLink = $page->getUrl('/edit') . '?editor=';
+        $resp = $this->asAdmin()->get($this->page->getUrl('/edit'));
+        $editLink = $this->page->getUrl('/edit') . '?editor=';

         $resp->assertElementContains("a[href=\"${editLink}markdown-clean\"]", '(Clean Content)');
         $resp->assertElementContains("a[href=\"${editLink}markdown-stable\"]", '(Stable Content)');
 
         $resp->assertElementContains("a[href=\"${editLink}markdown-clean\"]", '(Clean Content)');
         $resp->assertElementContains("a[href=\"${editLink}markdown-stable\"]", '(Stable Content)');
 

-        $resp = $this->asAdmin()->get($page->getUrl('/edit?editor=markdown-stable'));
-        $editLink = $page->getUrl('/edit') . '?editor=';
+        $resp = $this->asAdmin()->get($this->page->getUrl('/edit?editor=markdown-stable'));
+        $editLink = $this->page->getUrl('/edit') . '?editor=';

         $resp->assertElementContains("a[href=\"${editLink}wysiwyg\"]", 'Switch to WYSIWYG Editor');
     }
 
     public function test_editor_type_switch_options_dont_show_if_without_change_editor_permissions()
     {
         $resp->assertElementContains("a[href=\"${editLink}wysiwyg\"]", 'Switch to WYSIWYG Editor');
     }
 
     public function test_editor_type_switch_options_dont_show_if_without_change_editor_permissions()
     {

-        /** @var Page $page */
-        $page = Page::query()->first();
-
-        $resp = $this->asEditor()->get($page->getUrl('/edit'));
-        $editLink = $page->getUrl('/edit') . '?editor=';
+        $resp = $this->asEditor()->get($this->page->getUrl('/edit'));
+        $editLink = $this->page->getUrl('/edit') . '?editor=';

         $resp->assertElementNotExists("a[href*=\"${editLink}\"]");
     }
 
         $resp->assertElementNotExists("a[href*=\"${editLink}\"]");
     }