Reviewed #2393, Removed image guessing and added testing

For review of meta tag additions as per PR #2393.
This commit removes any image guesswork and only uses images that have
been set by the author for the specific content.
This also adds tests to cover the expected OG tags.

diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php
index 6e521b2b83460928a7a343cd2a8f2cc3c3abb349..93fb218932cf6e9f2796fce7f3aaac8586c43490 100644 (file)
--- a/app/Entities/Models/Page.php
+++ b/app/Entities/Models/Page.php
@@ -138,23 +138,4 @@ class Page extends BookChild
         $refreshed->html = (new PageContent($refreshed))->render();
         return $refreshed;
     }
-
-    /**
-     * Returns URL to a cover image for the page.
-     */
-    public function getCoverImage()
-    {
-        //$default = $this->book->getBookCover();
-        $default = url('/logo.png');
-
-        $firstImage = (new PageContent($this))->fetchFirstImage();
-
-        try {
-            $cover = $firstImage ? $firstImage : $default;
-        } catch (\Exception $err) {
-            $cover = $default;
-        }
-        return $cover;
-    }
-    
 }

diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php
index d178dc040c075e923bd64889b7954ba4270f8fb6..381ef172b47105939740a31ae807a7fbff865f10 100644 (file)
--- a/app/Entities/Tools/PageContent.php
+++ b/app/Entities/Tools/PageContent.php
@@ -367,18 +367,4 @@ class PageContent
         $doc->loadHTML(mb_convert_encoding($html, 'HTML-ENTITIES', 'UTF-8'));
         return $doc;
     }
-
-    /**
-     * Retrieve first image in page content and return the source URL.
-     */
-    public function fetchFirstImage()
-    {
-        $htmlContent = $this->page->html;
-
-        $dom = new \DomDocument();
-        $dom->loadHTML($htmlContent);
-        $images = $dom->getElementsByTagName('img');
-
-        return $images->length > 0 ? $images[0]->getAttribute('src') : null;
-    }
 }

diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php
index 69a945cbebf6013f9c15fe9345b87dc0423824de..5879cf6a22277283180e6b3782564618bf0640b5 100644 (file)
--- a/resources/views/books/show.blade.php
+++ b/resources/views/books/show.blade.php
@@ -8,7 +8,9 @@
 
 @push('social-meta')
     <meta property="og:description" content="{{ Str::limit($book->description, 100, '...') }}">
-    <meta property="og:image" content="{{ $book->getBookCover() }}">
+    @if($book->cover)
+        <meta property="og:image" content="{{ $book->getBookCover() }}">
+    @endif
 @endpush
 
 @section('body')

diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php
index a70f0f9f2ab504693c483d1d0e1fcf4f4a47e561..e8e0f6374fe24ccdd33f97300d54919bb8861969 100644 (file)
--- a/resources/views/chapters/show.blade.php
+++ b/resources/views/chapters/show.blade.php
@@ -7,8 +7,7 @@
 @stop
 
 @push('social-meta')
-    <meta property="og:description" content="{{ Str::limit($chapter->description, 100) }}">
-    <meta property="og:image" content="{{ $chapter->book->getBookCover() }}">
+    <meta property="og:description" content="{{ Str::limit($chapter->description, 100, '...') }}">
 @endpush
 
 @section('body')

diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php
index 398c8a8530e3180408d8f49b13be0d7bc99bdc73..012454e7c4b8abf1dc464ebad37f747471711beb 100644 (file)
--- a/resources/views/pages/show.blade.php
+++ b/resources/views/pages/show.blade.php
@@ -2,7 +2,6 @@
 
 @push('social-meta')
     <meta property="og:description" content="{{ Str::limit($page->text, 100, '...') }}">
-    <meta property="og:image" content="{{ $page->getCoverImage() }}">
 @endpush
 
 @section('body')

diff --git a/resources/views/shelves/show.blade.php b/resources/views/shelves/show.blade.php
index 1205e8320f9e9618c732393edca32f119a397af4..f5920d4758d98305b5cf7268349b17c0deb26618 100644 (file)
--- a/resources/views/shelves/show.blade.php
+++ b/resources/views/shelves/show.blade.php
@@ -1,8 +1,10 @@
 @extends('tri-layout')
 
 @push('social-meta')
-    <meta property="og:description" content="{{ Str::limit($shelf->description, 100) }}">
-    <meta property="og:image" content="{{ $shelf->getBookCover() }}">
+    <meta property="og:description" content="{{ Str::limit($shelf->description, 100, '...') }}">
+    @if($shelf->cover)
+        <meta property="og:image" content="{{ $shelf->getBookCover() }}">
+    @endif
 @endpush
 
 @section('body')

diff --git a/tests/OpenGraphTest.php b/tests/OpenGraphTest.php
new file mode 100644 (file)
index 0000000..653f2e5
--- /dev/null
+++ b/tests/OpenGraphTest.php
@@ -0,0 +1,102 @@
+<?php namespace Tests;
+
+use BookStack\Entities\Models\Book;
+use BookStack\Entities\Models\Bookshelf;
+use BookStack\Entities\Models\Chapter;
+use BookStack\Entities\Models\Page;
+use BookStack\Entities\Repos\BookRepo;
+use BookStack\Entities\Repos\BookshelfRepo;
+use Illuminate\Support\Str;
+use Tests\Uploads\UsesImages;
+
+class OpenGraphTest extends TestCase
+{
+    use UsesImages;
+
+    public function test_page_tags()
+    {
+        $page = Page::query()->first();
+        $resp = $this->asEditor()->get($page->getUrl());
+        $tags = $this->getOpenGraphTags($resp);
+
+        $this->assertEquals($page->getShortName() . ' | BookStack', $tags['title']);
+        $this->assertEquals($page->getUrl(), $tags['url']);
+        $this->assertEquals(Str::limit($page->text, 100, '...'), $tags['description']);
+    }
+
+    public function test_chapter_tags()
+    {
+        $chapter = Chapter::query()->first();
+        $resp = $this->asEditor()->get($chapter->getUrl());
+        $tags = $this->getOpenGraphTags($resp);
+
+        $this->assertEquals($chapter->getShortName() . ' | BookStack', $tags['title']);
+        $this->assertEquals($chapter->getUrl(), $tags['url']);
+        $this->assertEquals(Str::limit($chapter->description, 100, '...'), $tags['description']);
+    }
+
+    public function test_book_tags()
+    {
+        $book = Book::query()->first();
+        $resp = $this->asEditor()->get($book->getUrl());
+        $tags = $this->getOpenGraphTags($resp);
+
+        $this->assertEquals($book->getShortName() . ' | BookStack', $tags['title']);
+        $this->assertEquals($book->getUrl(), $tags['url']);
+        $this->assertEquals(Str::limit($book->description, 100, '...'), $tags['description']);
+        $this->assertArrayNotHasKey('image', $tags);
+
+        // Test image set if image has cover image
+        $bookRepo = app(BookRepo::class);
+        $bookRepo->updateCoverImage($book, $this->getTestImage('image.png'));
+        $resp = $this->asEditor()->get($book->getUrl());
+        $tags = $this->getOpenGraphTags($resp);
+
+        $this->assertEquals($book->getBookCover(), $tags['image']);
+    }
+
+    public function test_shelf_tags()
+    {
+        $shelf = Bookshelf::query()->first();
+        $resp = $this->asEditor()->get($shelf->getUrl());
+        $tags = $this->getOpenGraphTags($resp);
+
+        $this->assertEquals($shelf->getShortName() . ' | BookStack', $tags['title']);
+        $this->assertEquals($shelf->getUrl(), $tags['url']);
+        $this->assertEquals(Str::limit($shelf->description, 100, '...'), $tags['description']);
+        $this->assertArrayNotHasKey('image', $tags);
+
+        // Test image set if image has cover image
+        $shelfRepo = app(BookshelfRepo::class);
+        $shelfRepo->updateCoverImage($shelf, $this->getTestImage('image.png'));
+        $resp = $this->asEditor()->get($shelf->getUrl());
+        $tags = $this->getOpenGraphTags($resp);
+
+        $this->assertEquals($shelf->getBookCover(), $tags['image']);
+    }
+
+    /**
+     * Parse the open graph tags from a test response.
+     */
+    protected function getOpenGraphTags(TestResponse $resp): array
+    {
+        $tags = [];
+
+        libxml_use_internal_errors(true);
+        $doc = new \DOMDocument();
+        $doc->loadHTML($resp->getContent());
+        $metaElems = $doc->getElementsByTagName('meta');
+        /** @var \DOMElement $elem */
+        foreach ($metaElems as $elem) {
+            $prop = $elem->getAttribute('property');
+            $name = explode(':', $prop)[1] ?? null;
+            if ($name) {
+                $tags[$name] = $elem->getAttribute('content');
+            }
+        }
+
+        return $tags;
+    }
+
+
+}
\ No newline at end of file