Reviewed and refactored additional editor draft save warnings

- Added testing to cover warning cases.
- Refactored logic to be simpler and move much of the business out of
  the controller.
- Added new message that's more suitable to the case this was handling.
- For detecting an outdated draft, checked the draft created_at time
  instead of updated_at to better fit the scenario being checked.
- Updated some method types to align with those potentially being used
  in the logic of the code.
- Added a cache of shown messages on the front-end to prevent them
  re-showing on every save during the session, even if dismissed.

app/Entities/Models/PageRevision.php

@@ -5,6 +5,7 @@ namespace BookStack\Entities\Models;
 use BookStack\Auth\User;
 use BookStack\Model;
 use Carbon\Carbon;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
 
 /**
  * Class PageRevision.
@@ -14,11 +15,13 @@ use Carbon\Carbon;
  * @property string $book_slug
  * @property int    $created_by
  * @property Carbon $created_at
+ * @property Carbon $updated_at
  * @property string $type
  * @property string $summary
  * @property string $markdown
  * @property string $html
  * @property int    $revision_number
+ * @property Page   $page
  */
 class PageRevision extends Model
 {
@@ -26,20 +29,16 @@ class PageRevision extends Model
 
     /**
      * Get the user that created the page revision.
-     *
-     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo
      */
-    public function createdBy()
+    public function createdBy(): BelongsTo
     {
         return $this->belongsTo(User::class, 'created_by');
     }
 
     /**
      * Get the page this revision originates from.
-     *
-     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo
      */
-    public function page()
+    public function page(): BelongsTo
     {
         return $this->belongsTo(Page::class);
     }

app/Entities/Tools/PageEditActivity.php

@@ -21,8 +21,6 @@ class PageEditActivity
 
     /**
      * Check if there's active editing being performed on this page.
-     *
-     * @return bool
      */
     public function hasActiveEditing(): bool
     {
@@ -44,21 +42,35 @@ class PageEditActivity
     }
 
     /**
-     * Check if the page has been updated since the draft has been saved.
-     *
-     * @return bool
+     * Get any editor clash warning messages to show for the given draft revision.
+     * @param PageRevision|Page $draft
+     * @return string[]
      */
-    public function hasPageBeenUpdatedSinceDraftSaved(PageRevision $draft): bool
+    public function getWarningMessagesForDraft($draft): array
     {
-        return $draft->page->updated_at->timestamp >= $draft->updated_at->timestamp;
+        $warnings = [];
+
+        if ($this->hasActiveEditing()) {
+            $warnings[] = $this->activeEditingMessage();
+        }
+
+        if ($draft instanceof PageRevision && $this->hasPageBeenUpdatedSinceDraftCreated($draft)) {
+            $warnings[] = trans('entities.pages_draft_page_changed_since_creation');
+        }
+
+        return $warnings;
+    }
+
+    /**
+     * Check if the page has been updated since the draft has been saved.
+     */
+    protected function hasPageBeenUpdatedSinceDraftCreated(PageRevision $draft): bool
+    {
+        return $draft->page->updated_at->timestamp > $draft->created_at->timestamp;
     }
 
     /**
      * Get the message to show when the user will be editing one of their drafts.
-     *
-     * @param PageRevision $draft
-     *
-     * @return string
      */
     public function getEditingActiveDraftMessage(PageRevision $draft): string
     {

app/Http/Controllers/PageController.php

@@ -4,6 +4,7 @@ namespace BookStack\Http\Controllers;
 
 use BookStack\Actions\View;
 use BookStack\Entities\Models\Page;
+use BookStack\Entities\Models\PageRevision;
 use BookStack\Entities\Repos\PageRepo;
 use BookStack\Entities\Tools\BookContents;
 use BookStack\Entities\Tools\NextPreviousContentLocator;
@@ -258,32 +259,14 @@ class PageController extends Controller
             return $this->jsonError(trans('errors.guests_cannot_save_drafts'), 500);
         }
 
-        // Check for active editing or time conflict
-        $warnings = [];
-        $jsonResponseWarning = '';
-        $editActivity = new PageEditActivity($page);
-        if ($editActivity->hasActiveEditing()) {
-            $warnings[] = $editActivity->activeEditingMessage();
-        }
-        $userDraft = $this->pageRepo->getUserDraft($page);
-        if ($userDraft !== null) {
-            if ($editActivity->hasPageBeenUpdatedSinceDraftSaved($userDraft)) {
-                $warnings[] = $editActivity->getEditingActiveDraftMessage($userDraft);
-            }
-        }
-        if (count($warnings) > 0) {
-            $jsonResponseWarning = implode("\n", $warnings);
-        }
-
         $draft = $this->pageRepo->updatePageDraft($page, $request->only(['name', 'html', 'markdown']));
-
-        $updateTime = $draft->updated_at->timestamp;
+        $warnings = (new PageEditActivity($page))->getWarningMessagesForDraft($draft);
 
         return response()->json([
             'status'    => 'success',
             'message'   => trans('entities.pages_edit_draft_save_at'),
-            'warning'   => $jsonResponseWarning,
-            'timestamp' => $updateTime,
+            'warning'   => implode("\n", $warnings),
+            'timestamp' => $draft->updated_at->timestamp,
         ]);
     }