Added front-end toggle and testing of inline attachments

app/Uploads/Attachment.php

@@ -41,12 +41,12 @@ class Attachment extends Model
     /**
      * Get the url of this file.
      */
-    public function getUrl(): string
+    public function getUrl($openInline = false): string
     {
         if ($this->external && strpos($this->path, 'http') !== 0) {
             return $this->path;
         }
-        return url('/attachments/' . $this->id);
+        return url('/attachments/' . $this->id . ($openInline ? '?open=true' : ''));
     }
 
     /**

resources/js/components/attachments-list.js

@@ -0,0 +1,47 @@
+/**
+ * Attachments List
+ * Adds '?open=true' query to file attachment links
+ * when ctrl/cmd is pressed down.
+ * @extends {Component}
+ */
+class AttachmentsList {
+
+    setup() {
+        this.container = this.$el;
+        this.setupListeners();
+    }
+
+    setupListeners() {
+        const isExpectedKey = (event) => event.key === 'Control' || event.key === 'Meta';
+        window.addEventListener('keydown', event => {
+             if (isExpectedKey(event)) {
+                this.addOpenQueryToLinks();
+             }
+        }, {passive: true});
+        window.addEventListener('keyup', event => {
+            if (isExpectedKey(event)) {
+                this.removeOpenQueryFromLinks();
+            }
+        }, {passive: true});
+    }
+
+    addOpenQueryToLinks() {
+        const links = this.container.querySelectorAll('a.attachment-file');
+        for (const link of links) {
+            if (link.href.split('?')[1] !== 'open=true') {
+                link.href = link.href + '?open=true';
+                link.setAttribute('target', '_blank');
+            }
+        }
+    }
+
+    removeOpenQueryFromLinks() {
+        const links = this.container.querySelectorAll('a.attachment-file');
+        for (const link of links) {
+            link.href = link.href.split('?')[0];
+            link.removeAttribute('target');
+        }
+    }
+}
+
+export default AttachmentsList;

resources/js/components/index.js

@@ -2,6 +2,7 @@ import addRemoveRows from "./add-remove-rows.js"
 import ajaxDeleteRow from "./ajax-delete-row.js"
 import ajaxForm from "./ajax-form.js"
 import attachments from "./attachments.js"
+import attachmentsList from "./attachments-list.js"
 import autoSuggest from "./auto-suggest.js"
 import backToTop from "./back-to-top.js"
 import bookSort from "./book-sort.js"
@@ -56,6 +57,7 @@ const componentMapping = {
     "ajax-delete-row": ajaxDeleteRow,
     "ajax-form": ajaxForm,
     "attachments": attachments,
+    "attachments-list": attachmentsList,
     "auto-suggest": autoSuggest,
     "back-to-top": backToTop,
     "book-sort": bookSort,

resources/views/attachments/list.blade.php

@@ -1,8 +1,10 @@
-@foreach($attachments as $attachment)
+<div component="attachments-list">
-    <div class="attachment icon-list">
+    @foreach($attachments as $attachment)
-        <a class="icon-list-item py-xs" href="{{ $attachment->getUrl() }}" @if($attachment->external) target="_blank" @endif>
+        <div class="attachment icon-list">
-            <span class="icon">@icon($attachment->external ? 'export' : 'file')</span>
+            <a class="icon-list-item py-xs attachment-{{ $attachment->external ? 'link' : 'file' }}" href="{{ $attachment->getUrl() }}" @if($attachment->external) target="_blank" @endif>
-            <span>{{ $attachment->name }}</span>
+                <span class="icon">@icon($attachment->external ? 'export' : 'file')</span>
-        </a>
+                <span>{{ $attachment->name }}</span>
-    </div>
+            </a>
-@endforeach
+        </div>
+    @endforeach
+</div>

tests/Uploads/AttachmentTest.php

@@ -4,11 +4,9 @@ use BookStack\Entities\Tools\TrashCan;
 use BookStack\Entities\Repos\PageRepo;
 use BookStack\Uploads\Attachment;
 use BookStack\Entities\Models\Page;
-use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Uploads\AttachmentService;
 use Illuminate\Http\UploadedFile;
 use Tests\TestCase;
-use Tests\TestResponse;
 
 class AttachmentTest extends TestCase
 {
@@ -57,7 +55,7 @@ class AttachmentTest extends TestCase
 
     public function test_file_upload()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
         $admin = $this->getAdmin();
         $fileName = 'upload_test_file.txt';
@@ -85,7 +83,7 @@ class AttachmentTest extends TestCase
 
     public function test_file_upload_does_not_use_filename()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $fileName = 'upload_test_file.txt';
 
 
@@ -99,7 +97,7 @@ class AttachmentTest extends TestCase
 
     public function test_file_display_and_access()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
         $fileName = 'upload_test_file.txt';
 
@@ -119,7 +117,7 @@ class AttachmentTest extends TestCase
 
     public function test_attaching_link_to_page()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $admin = $this->getAdmin();
         $this->asAdmin();
 
@@ -156,7 +154,7 @@ class AttachmentTest extends TestCase
 
     public function test_attachment_updating()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
 
         $attachment = $this->createAttachment($page);
@@ -180,7 +178,7 @@ class AttachmentTest extends TestCase
 
     public function test_file_deletion()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
         $fileName = 'deletion_test.txt';
         $this->uploadFile($fileName, $page->id);
@@ -202,7 +200,7 @@ class AttachmentTest extends TestCase
 
     public function test_attachment_deletion_on_page_deletion()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
         $fileName = 'deletion_test.txt';
         $this->uploadFile($fileName, $page->id);
@@ -230,7 +228,7 @@ class AttachmentTest extends TestCase
     {
         $admin = $this->getAdmin();
         $viewer = $this->getViewer();
-        $page = Page::first(); /** @var Page $page */
+        $page = Page::query()->first(); /** @var Page $page */
 
         $this->actingAs($admin);
         $fileName = 'permission_test.txt';
@@ -253,7 +251,7 @@ class AttachmentTest extends TestCase
 
     public function test_data_and_js_links_cannot_be_attached_to_a_page()
     {
-        $page = Page::first();
+        $page = Page::query()->first();
         $this->asAdmin();
 
         $badLinks = [
@@ -291,4 +289,22 @@ class AttachmentTest extends TestCase
             ]);
         }
     }
+
+    public function test_file_access_with_open_query_param_provides_inline_response_with_correct_content_type()
+    {
+        $page = Page::query()->first();
+        $this->asAdmin();
+        $fileName = 'upload_test_file.txt';
+
+        $upload = $this->uploadFile($fileName, $page->id);
+        $upload->assertStatus(200);
+        $attachment = Attachment::query()->orderBy('id', 'desc')->take(1)->first();
+
+        $attachmentGet = $this->get($attachment->getUrl(true));
+        // http-foundation/Response does some 'fixing' of responses to add charsets to text responses.
+        $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=UTF-8');
+        $attachmentGet->assertHeader('Content-Disposition', "inline; filename=\"upload_test_file.txt\"");
+
+        $this->deleteUploads();
+    }
 }