From 7997300f966103d8f945422fc0a38bef9f5bdbaa Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 6 Jun 2021 13:55:56 +0100 Subject: [PATCH] Added front-end toggle and testing of inline attachments --- app/Uploads/Attachment.php | 4 +- resources/js/components/attachments-list.js | 47 +++++++++++++++++++++ resources/js/components/index.js | 2 + resources/views/attachments/list.blade.php | 18 ++++---- tests/Uploads/AttachmentTest.php | 38 ++++++++++++----- 5 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 resources/js/components/attachments-list.js diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php index d1060477d..474d68998 100644 --- a/app/Uploads/Attachment.php +++ b/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' : '')); } /** diff --git a/resources/js/components/attachments-list.js b/resources/js/components/attachments-list.js new file mode 100644 index 000000000..34979c2e7 --- /dev/null +++ b/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; \ No newline at end of file diff --git a/resources/js/components/index.js b/resources/js/components/index.js index 91ccdaf3a..010ee04ba 100644 --- a/resources/js/components/index.js +++ b/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, diff --git a/resources/views/attachments/list.blade.php b/resources/views/attachments/list.blade.php index 8c9be8290..f0a1354ea 100644 --- a/resources/views/attachments/list.blade.php +++ b/resources/views/attachments/list.blade.php @@ -1,8 +1,10 @@ -@foreach($attachments as $attachment) -
- external) target="_blank" @endif> - @icon($attachment->external ? 'export' : 'file') - {{ $attachment->name }} - -
-@endforeach \ No newline at end of file +
+ @foreach($attachments as $attachment) +
+ external) target="_blank" @endif> + @icon($attachment->external ? 'export' : 'file') + {{ $attachment->name }} + +
+ @endforeach +
\ No newline at end of file diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 1ca9ea23b..55a5aa84f 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/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(); + } }