feat: implement file rejection handling and improve media policies

dfo-osdt · Nov 28, 2024 · d05f1af · d05f1af
1 parent 0a1e7a8
commit d05f1af
Show file tree

Hide file tree

Showing 12 changed files with 121 additions and 63 deletions.
diff --git a/app/Enums/MediaCollection.php b/app/Enums/MediaCollection.php
@@ -7,4 +7,4 @@ enum MediaCollection: string
     case MANUSCRIPT = 'manuscript';
     case SUPPLEMENTARY_FILE = 'supplementary_file';
     case PUBLICATION = 'publication';
-}
+}
diff --git a/app/Http/Controllers/PublicationFileController.php b/app/Http/Controllers/PublicationFileController.php
@@ -67,7 +67,7 @@ public function show(Request $request, Publication $publication, string $uuid)
         $media = $publication->getPublicationFile($uuid);
 
         $download = $request->query('download', false);
-        if ($download && Gate::allows('downloadMedia', [$publication,$media])) {
+        if ($download && Gate::allows('downloadMedia', [$publication, $media])) {
             return $media;
         }
 
@@ -91,5 +91,7 @@ public function destroy(Publication $publication, string $uuid)
                 'message' => 'This file is locked.',
             ], 403);
         }
+
+        return response()->noContent();
     }
 }
diff --git a/app/Http/Controllers/PublicationSupplementaryFileController.php b/app/Http/Controllers/PublicationSupplementaryFileController.php
@@ -39,7 +39,8 @@ public function store(Request $request, Publication $publication)
             'description' => 'nullable|string',
         ]);
 
-        $media = $publication->addSupplementaryFile($validated['pdf'], $validated['supplementary_file_type'], $validated['description'] ?? null);
+        $type = SupplementaryFileType::tryFrom($validated['supplementary_file_type']);
+        $media = $publication->addSupplementaryFile($validated['pdf'], $type, $validated['description'] ?? null);
 
         activity()
             ->performedOn($publication)
@@ -55,12 +56,13 @@ public function store(Request $request, Publication $publication)
      */
     public function show(Request $request, Publication $publication, string $uuid)
     {
+
         Gate::authorize('view', $publication);
 
         $media = $publication->getSupplementaryFile($uuid);
 
         $download = $request->query('download', false);
-        if ($download && Gate::allow('viewSupplementaryPdf', $publication, $media)) {
+        if ($download && Gate::allows('downloadMedia', [$publication, $media])) {
             return $media;
         }
 
@@ -84,5 +86,7 @@ public function destroy(Publication $publication, string $uuid)
                 'message' => 'This file is locked.',
             ], 403);
         }
+
+        return response()->noContent();
     }
 }
diff --git a/app/Policies/ManuscriptRecordPolicy.php b/app/Policies/ManuscriptRecordPolicy.php
@@ -218,23 +218,22 @@ public function share(User $user, ManuscriptRecord $manuscriptRecord)
         return $user->id === $manuscriptRecord->user_id;
     }
 
-
     public function updateMedia(User $user, ManuscriptRecord $manuscriptRecord, Media $media)
     {
         return $this->update($user, $manuscriptRecord);
     }
 
     public function deleteMedia(User $user, ManuscriptRecord $manuscriptRecord, Media $media)
     {
-        if($media->getCustomProperty('locked') === true) {
+        if ($media->getCustomProperty('locked') === true) {
             return false;
         }
+
         return $this->update($user, $manuscriptRecord);
     }
 
     public function downloadMedia(User $user, ManuscriptRecord $manuscriptRecord, Media $media)
     {
         return $this->view($user, $manuscriptRecord);
     }
-
 }
diff --git a/app/Policies/MediaPolicy.php b/app/Policies/MediaPolicy.php
@@ -9,11 +9,9 @@
 /**
  * This policy class will be used to route to the correct policy
  * of the model to which it is associated.
- *
  */
 class MediaPolicy
 {
-
     public function update(User $user, Media $media)
     {
         return Gate::allows('updateMedia', [$media->model, $media]);
@@ -28,6 +26,4 @@ public function download(User $user, Media $media)
     {
         return Gate::allows('downloadMedia', [$media->model, $media]);
     }
-
-
 }
diff --git a/app/Policies/PublicationPolicy.php b/app/Policies/PublicationPolicy.php
@@ -9,7 +9,6 @@
 use App\Models\Publication;
 use App\Models\User;
 use Illuminate\Auth\Access\HandlesAuthorization;
-use Illuminate\Contracts\Container\BindingResolutionException;
 use Spatie\MediaLibrary\MediaCollections\Models\Media;
 
 class PublicationPolicy
@@ -86,8 +85,8 @@ public function downloadMedia(User $user, Publication $publication, Media $media
         }
 
         //if it's a publication file and not under embargo, then it can be downloaded
-        if($media->collection_name === MediaCollection::PUBLICATION->value) {
-            return !$publication->isUnderEmbargo();
+        if ($media->collection_name === MediaCollection::PUBLICATION->value) {
+            return ! $publication->isUnderEmbargo();
         }
 
         return false;
@@ -103,7 +102,6 @@ public function deleteMedia(User $user, Publication $publication, Media $media)
         return $this->update($user, $publication);
     }
 
-
     /**
      * Determine whether the user can create models.
      *

diff --git a/resources/src/auto-imports.d.ts b/resources/src/auto-imports.d.ts
@@ -190,6 +190,7 @@ declare global {
   const useFavicon: typeof import('@vueuse/core')['useFavicon']
   const useFetch: typeof import('@vueuse/core')['useFetch']
   const useFileDialog: typeof import('@vueuse/core')['useFileDialog']
+  const useFileRejectionHandler: typeof import('./composables/useFileRejectionHandler')['useFileRejectionHandler']
   const useFileSystemAccess: typeof import('@vueuse/core')['useFileSystemAccess']
   const useFocus: typeof import('@vueuse/core')['useFocus']
   const useFocusWithin: typeof import('@vueuse/core')['useFocusWithin']
@@ -520,6 +521,7 @@ declare module 'vue' {
     readonly useFavicon: UnwrapRef<typeof import('@vueuse/core')['useFavicon']>
     readonly useFetch: UnwrapRef<typeof import('@vueuse/core')['useFetch']>
     readonly useFileDialog: UnwrapRef<typeof import('@vueuse/core')['useFileDialog']>
+    readonly useFileRejectionHandler: UnwrapRef<typeof import('./composables/useFileRejectionHandler')['useFileRejectionHandler']>
     readonly useFileSystemAccess: UnwrapRef<typeof import('@vueuse/core')['useFileSystemAccess']>
     readonly useFocus: UnwrapRef<typeof import('@vueuse/core')['useFocus']>
     readonly useFocusWithin: UnwrapRef<typeof import('@vueuse/core')['useFocusWithin']>

diff --git a/resources/src/composables/useFileRejectionHandler.ts b/resources/src/composables/useFileRejectionHandler.ts
@@ -0,0 +1,30 @@
+import type { QRejectedEntry } from 'quasar'
+import { useQuasar } from 'quasar'
+import { useI18n } from 'vue-i18n'
+
+export function useFileRejectionHandler() {
+  const $q = useQuasar()
+  const { t } = useI18n()
+
+  function onFileRejected(rejectedEntries: QRejectedEntry[]): void {
+    console.error(rejectedEntries)
+    rejectedEntries.forEach((rejectedEntry) => {
+      if (rejectedEntry.failedPropValidation === 'max-file-size') {
+        $q.notify({
+          type: 'negative',
+          color: 'negative',
+          message: t('common.validation.file-size-is-too-large'),
+        })
+      }
+      else if (rejectedEntry.failedPropValidation === 'accept') {
+        $q.notify({
+          type: 'negative',
+          color: 'negative',
+          message: t('common.validation.file-type-is-not-accepted'),
+        })
+      }
+    })
+  }
+
+  return { onFileRejected }
+}
diff --git a/resources/src/models/ManuscriptRecord/components/ManuscriptFileManagementCard.vue b/resources/src/models/ManuscriptRecord/components/ManuscriptFileManagementCard.vue
@@ -5,7 +5,7 @@ import FormSectionStatusIcon, {
   type FormSectionStatus,
 } from '@/components/FormSectionStatusIcon.vue'
 import SensitivityLabelChip from '@/components/SensitivityLabelChip.vue'
-import { type QRejectedEntry, useQuasar } from 'quasar'
+import { useQuasar } from 'quasar'
 import {
   type ManuscriptRecordResource,
   ManuscriptRecordService,
@@ -16,7 +16,7 @@ const props = defineProps<{
 }>()
 const { t } = useI18n()
 const $q = useQuasar()
-
+const { onFileRejected } = useFileRejectionHandler()
 const maxFileSizeMB = 50
 
 const manuscriptFiles: Ref<MediaResourceList | null> = ref(null)
@@ -52,25 +52,6 @@ defineExpose({
   getFiles,
 })
 
-function onFileRejected(rejectedEntries: QRejectedEntry[]): void {
-  rejectedEntries.forEach((rejectedEntry) => {
-    if (rejectedEntry.failedPropValidation === 'max-file-size') {
-      $q.notify({
-        type: 'negative',
-        color: 'negative',
-        message: t('common.validation.file-size-is-too-large'),
-      })
-    }
-    else if (rejectedEntry.failedPropValidation === 'accept') {
-      $q.notify({
-        type: 'negative',
-        color: 'negative',
-        message: t('common.validation.file-type-is-not-accepted'),
-      })
-    }
-  })
-}
-
 const displayFileUpload = computed(() => {
   const m = props.manuscript.data
   if (!m)

diff --git a/resources/src/models/Publication/components/PublicationFileManagementCard.vue b/resources/src/models/Publication/components/PublicationFileManagementCard.vue
@@ -1,7 +1,7 @@
 <script setup lang="ts">
 import type { Media, MediaResourceList } from '@/models/Resource'
 import ContentCard from '@/components/ContentCard.vue'
-import { type QRejectedEntry, useQuasar } from 'quasar'
+import { useQuasar } from 'quasar'
 import { type PublicationResource, PublicationService } from '../Publication'
 
 const props = defineProps<{
@@ -13,6 +13,7 @@ const { t } = useI18n()
 const publicationResourceList = ref<MediaResourceList | null>(null)
 const publicationFile = ref<File | null>(null)
 const uploadingFile = ref(false)
+const { onFileRejected } = useFileRejectionHandler()
 
 onMounted(async () => {
   await getFiles()
@@ -24,26 +25,6 @@ async function getFiles() {
   })
 }
 
-function onFileRejected(rejectedEntries: QRejectedEntry[]): void {
-  console.error(rejectedEntries)
-  rejectedEntries.forEach((rejectedEntry) => {
-    if (rejectedEntry.failedPropValidation === 'max-file-size') {
-      $q.notify({
-        type: 'negative',
-        color: 'negative',
-        message: t('common.validation.file-size-is-too-large'),
-      })
-    }
-    else if (rejectedEntry.failedPropValidation === 'accept') {
-      $q.notify({
-        type: 'negative',
-        color: 'negative',
-        message: t('common.validation.file-type-is-not-accepted'),
-      })
-    }
-  })
-}
-
 async function upload() {
   // if there is no manuscript file, return
   if (publicationFile.value === null)
@@ -102,7 +83,7 @@ async function deleteFile(publicationResource: Media) {
     </p>
     <template v-if="publicationResourceList?.data">
       <q-card outlined class="q-mb-md">
-        <q-list>
+        <q-list separator>
           <q-item v-for="publicationResource in publicationResourceList.data" :key="publicationResource.data.uuid">
             <q-item-section>
               <q-item-label>
@@ -128,6 +109,7 @@ async function deleteFile(publicationResource: Media) {
                   v-if="publicationResource.can?.delete"
                   icon="mdi-delete-outline"
                   color="negative"
+                  outline
                   class="q-mr-sm"
                   @click="deleteFile(publicationResource.data)"
                 />

diff --git a/routes/api.php b/routes/api.php
@@ -19,13 +19,13 @@
 use App\Http\Controllers\OhDear\CheckStatusPageController;
 use App\Http\Controllers\OpenAI\GeneratePLSController;
 use App\Http\Controllers\Orcid\FullFlowController;
-use App\Http\Controllers\Orcid\ImplicitFlowController;
 use App\Http\Controllers\Orcid\RevokeTokenController;
 use App\Http\Controllers\OrganizationController;
 use App\Http\Controllers\PublicationAuthorController;
 use App\Http\Controllers\PublicationController;
 use App\Http\Controllers\PublicationFileController;
 use App\Http\Controllers\PublicationFundingSourceController;
+use App\Http\Controllers\PublicationSupplementaryFileController;
 use App\Http\Controllers\RegionController;
 use App\Http\Controllers\UserController;
 use App\Http\Controllers\UserManagementReviewStepsController;
@@ -55,10 +55,6 @@
         Route::get('/user/invitations', 'invitations');
     });
 
-    // ORCID OAuth routes - only one of these flows should be used.
-    // Routes for Implicit Flow
-    // Route::post('/user/orcid/verify', ImplicitFlowController::class);
-    // Route::get('/user/orcid/verify', [ImplicitFlowController::class, 'redirect']);
     // Routes for 3-legged OAuth flow
     Route::get('/user/orcid/verify', [FullFlowController::class, 'redirect']);
     Route::post('/user/orcid/revoke', RevokeTokenController::class);
@@ -205,6 +201,13 @@
         Route::delete('/publications/{publication}/files/{uuid}', 'destroy');
     });
 
+    Route::controller(PublicationSupplementaryFileController::class)->group(function () {
+        Route::get('/publications/{publication}/supplementary-files', 'index');
+        Route::post('/publications/{publication}/supplementary-files', 'store');
+        Route::get('/publications/{publication}/supplementary-files/{uuid}', 'show');
+        Route::delete('/publications/{publication}/supplementary-files/{uuid}', 'destroy');
+    });
+
     Route::controller(PublicationAuthorController::class)->group(function () {
         Route::get('/publications/{publication}/publication-authors', 'index');
         Route::get('/publications/{publication}/publication-authors/{publicationAuthor}', 'show');

diff --git a/tests/Feature/Models/PublicationTest.php b/tests/Feature/Models/PublicationTest.php
@@ -1,6 +1,7 @@
 <?php
 
 use App\Enums\PublicationStatus;
+use App\Enums\SupplementaryFileType;
 use App\Models\Journal;
 use App\Models\Publication;
 use App\Models\User;
@@ -256,3 +257,63 @@
     $response = $this->actingAs($user)->getJson('/api/publications/'.$publication->id);
     $response->assertOk();
 });
+
+test('a user can manage supplementary files for their publication record', function () {
+    $publication = Publication::factory()->create();
+    $user = $publication->user;
+
+    $fakePdfContent = <<<'PDF'
+        %PDF-1.4
+        1 0 obj<</Type/Catalog/Pages 2 0 R>>endobj
+        2 0 obj<</Type/Pages/Count 1/Kids[3 0 R]>>endobj
+        3 0 obj<</Type/Page/MediaBox[0 0 612 792]/Parent 2 0 R/Resources<<>>>>endobj
+        xref
+        0 4
+        0000000000 65535 f
+        0000000009 00000 n
+        0000000052 00000 n
+        0000000101 00000 n
+        trailer<</Size 4/Root 1 0 R>>
+        startxref
+        178
+        %%EOF
+        PDF;
+
+    // upload pdf
+    $file = UploadedFile::fake()->createWithContent('test.pdf', $fakePdfContent)->size(1000);
+
+    // can't do it if wrong user
+    $response = $this->actingAs(User::factory()->create())->postJson('/api/publications/'.$publication->id.'/supplementary-files', [
+        'pdf' => $file,
+        'supplementary_file_type' => SupplementaryFileType::MANUSCRIPT_RECORD_FORM->value,
+        'description' => 'Test Description',
+    ]);
+
+    $response->assertForbidden();
+
+    $response = $this->actingAs($user)->postJson('/api/publications/'.$publication->id.'/supplementary-files', [
+        'pdf' => $file,
+        'supplementary_file_type' => SupplementaryFileType::MANUSCRIPT_RECORD_FORM->value,
+        'description' => 'Test Description',
+    ]);
+
+    $response->assertCreated();
+    $uuid = $response->json('data.uuid');
+
+    // list the files
+    $response = $this->actingAs($user)->getJson('/api/publications/'.$publication->id.'/supplementary-files');
+    $response->assertOk();
+    $response->assertJsonCount(1, 'data');
+
+    $response2 = $this->actingAs($user)->get("/api/publications/{$publication->id}/supplementary-files/{$uuid}?download=true");
+    $response2->assertDownload('test.pdf');
+
+    // can't delete if wrong user
+    $response = $this->actingAs(User::factory()->create())->deleteJson("/api/publications/{$publication->id}/supplementary-files/{$uuid}");
+    $response->assertForbidden();
+
+    // delete the file
+    $response = $this->actingAs($user)->deleteJson("/api/publications/{$publication->id}/supplementary-files/{$uuid}");
+    $response->assertNoContent();
+
+});