From 58087b270a90c1880dab61fc556eab5eb2e79c82 Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Wed, 17 Apr 2024 14:20:00 -0700 Subject: [PATCH 1/7] [TM-803] Make the model interface binding middleware a bit more generically useful. --- .../ModelInterfaceBindingMiddleware.php | 54 +++++++++++++++---- routes/api_v2.php | 43 ++++++--------- 2 files changed, 60 insertions(+), 37 deletions(-) diff --git a/app/Http/Middleware/ModelInterfaceBindingMiddleware.php b/app/Http/Middleware/ModelInterfaceBindingMiddleware.php index fddbf277f..b4c884840 100644 --- a/app/Http/Middleware/ModelInterfaceBindingMiddleware.php +++ b/app/Http/Middleware/ModelInterfaceBindingMiddleware.php @@ -2,14 +2,23 @@ namespace App\Http\Middleware; +use App\Models\V2\Forms\Form; +use App\Models\V2\Forms\FormQuestionOption; +use App\Models\V2\FundingProgramme; use App\Models\V2\Nurseries\Nursery; use App\Models\V2\Nurseries\NurseryReport; +use App\Models\V2\Organisation; +use App\Models\V2\ProjectPitch; use App\Models\V2\Projects\Project; +use App\Models\V2\Projects\ProjectMonitoring; use App\Models\V2\Projects\ProjectReport; use App\Models\V2\Sites\Site; +use App\Models\V2\Sites\SiteMonitoring; use App\Models\V2\Sites\SiteReport; use Closure; use Illuminate\Http\Request; +use Illuminate\Routing\RouteRegistrar; +use Illuminate\Support\Facades\Route; /** * Implicit binding doesn't work for interfaces, so we need to figure out the concrete model class and @@ -17,27 +26,52 @@ */ class ModelInterfaceBindingMiddleware { - public const ENTITY_TYPES_PLURAL = ['projects', 'project-reports', 'sites', 'site-reports', 'nurseries', 'nursery-reports']; - public const ENTITY_TYPES_SINGULAR = ['project', 'project-report', 'site', 'site-report', 'nursery', 'nursery-report']; - private const CONCRETE_MODELS = [ - // EntityModel concrete classes + // EntityModel and MediaModel concrete classes 'projects' => Project::class, 'project' => Project::class, - 'sites' => Site::class, - 'site' => Site::class, - 'nurseries' => Nursery::class, - 'nursery' => Nursery::class, - - // ReportModel (which extends EntityModel) concrete classes 'project-reports' => ProjectReport::class, 'project-report' => ProjectReport::class, + 'sites' => Site::class, + 'site' => Site::class, 'site-reports' => SiteReport::class, 'site-report' => SiteReport::class, + 'nurseries' => Nursery::class, + 'nursery' => Nursery::class, 'nursery-reports' => NurseryReport::class, 'nursery-report' => NurseryReport::class, + + // MediaModel concrete classes + 'organisation' => Organisation::class, + 'project-pitch' => ProjectPitch::class, + 'funding-programme' => FundingProgramme::class, + 'form' => Form::class, + 'form-question-option' => FormQuestionOption::class, + 'project-monitoring' => ProjectMonitoring::class, + 'site-monitoring' => SiteMonitoring::class, ]; + private static array $typeSlugsCache = []; + + public static function with(string $interface, callable $routeGroup): RouteRegistrar + { + $typeSlugs = self::$typeSlugsCache[$interface] ?? []; + if (empty($typeSlugs)) { + foreach (self::CONCRETE_MODELS as $slug => $concrete) { + if (is_a($concrete, $interface, true)) { + $typeSlugs[] = $slug; + } + } + + self::$typeSlugsCache[$interface] = $typeSlugs; + } + + return Route::prefix('/{modelSlug}') + ->whereIn('modelSlug', $typeSlugs) + ->middleware('modelInterface') + ->group($routeGroup); + } + public function handle(Request $request, Closure $next) { $route = $request->route(); diff --git a/routes/api_v2.php b/routes/api_v2.php index 10445c3a5..cdd8c9c27 100644 --- a/routes/api_v2.php +++ b/routes/api_v2.php @@ -189,6 +189,7 @@ use App\Http\Controllers\V2\Workdays\StoreWorkdayController; use App\Http\Controllers\V2\Workdays\UpdateWorkdayController; use App\Http\Middleware\ModelInterfaceBindingMiddleware; +use App\Models\V2\EntityModel; use Illuminate\Support\Facades\Route; /* @@ -292,13 +293,10 @@ Route::get('/{entity}/export/{framework}', ExportAllMonitoredEntitiesController::class); - Route::prefix('{modelSlug}') - ->whereIn('modelSlug', ModelInterfaceBindingMiddleware::ENTITY_TYPES_PLURAL) - ->middleware('modelInterface') - ->group(function () { - Route::put('/{entity}/{status}', AdminStatusEntityController::class); - Route::delete('/{entity}', AdminSoftDeleteEntityController::class); - }); + ModelInterfaceBindingMiddleware::with(EntityModel::class, function () { + Route::put('/{entity}/{status}', AdminStatusEntityController::class); + Route::delete('/{entity}', AdminSoftDeleteEntityController::class); + }); Route::get('nursery-reports', AdminIndexNurseryReportsController::class); Route::get('site-reports', AdminIndexSiteReportsController::class); @@ -405,14 +403,11 @@ Route::get('/', IndexFormController::class); Route::get('/{form}', ViewFormController::class)->middleware('i18n'); - Route::prefix('{modelSlug}') - ->whereIn('modelSlug', ModelInterfaceBindingMiddleware::ENTITY_TYPES_PLURAL) - ->middleware('modelInterface') - ->group(function () { - Route::get('/{entity}', ViewEntityWithFormController::class)->middleware('i18n'); - Route::put('/{entity}', UpdateEntityWithFormController::class); - Route::put('/{entity}/submit', SubmitEntityWithFormController::class); - }); + ModelInterfaceBindingMiddleware::with(EntityModel::class, function () { + Route::get('/{entity}', ViewEntityWithFormController::class)->middleware('i18n'); + Route::put('/{entity}', UpdateEntityWithFormController::class); + Route::put('/{entity}/submit', SubmitEntityWithFormController::class); + }); Route::prefix('projects')->group(function () { Route::post('', CreateProjectWithFormController::class); @@ -545,12 +540,9 @@ Route::put('/{report}/nothing-to-report', NothingToReportReportController::class); }); -Route::prefix('{modelSlug}') - ->whereIn('modelSlug', ModelInterfaceBindingMiddleware::ENTITY_TYPES_PLURAL) - ->middleware('modelInterface') - ->group(function () { - Route::get('/{entity}', ViewEntityController::class); - }); +ModelInterfaceBindingMiddleware::with(EntityModel::class, function () { + Route::get('/{entity}', ViewEntityController::class); +}); Route::prefix('project-reports')->group(function () { Route::get('/{projectReport}/files', ViewProjectReportGalleryController::class); @@ -606,12 +598,9 @@ Route::get('/{updateRequest}', AdminViewUpdateRequestController::class); Route::delete('/{updateRequest}', AdminSoftDeleteUpdateRequestController::class); - Route::prefix('/{modelSlug}') - ->whereIn('modelSlug', ModelInterfaceBindingMiddleware::ENTITY_TYPES_SINGULAR) - ->middleware('modelInterface') - ->group(function () { - Route::get('/{entity}', EntityUpdateRequestsController::class); - }); + ModelInterfaceBindingMiddleware::with(EntityModel::class, function () { + Route::get('/{entity}', EntityUpdateRequestsController::class); + }); }); Route::get('/funding-programme', [FundingProgrammeController::class, 'index'])->middleware('i18n'); From b6758e5d25fc4e634592ccc7709dfe17b3d79801 Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Wed, 17 Apr 2024 14:47:58 -0700 Subject: [PATCH 2/7] [TM-803] Refactor UploadController to use the model binding middleware. --- .../Controllers/V2/Files/UploadController.php | 99 ++----------------- .../ModelInterfaceBindingMiddleware.php | 16 +-- app/Models/V2/Forms/Form.php | 4 +- app/Models/V2/Forms/FormQuestionOption.php | 4 +- app/Models/V2/FundingProgramme.php | 3 +- app/Models/V2/MediaModel.php | 14 +++ app/Models/V2/Nurseries/Nursery.php | 4 +- app/Models/V2/Nurseries/NurseryReport.php | 4 +- app/Models/V2/Organisation.php | 3 +- app/Models/V2/ProjectPitch.php | 3 +- app/Models/V2/Projects/Project.php | 4 +- app/Models/V2/Projects/ProjectMonitoring.php | 4 +- app/Models/V2/Projects/ProjectReport.php | 4 +- app/Models/V2/Sites/Site.php | 4 +- app/Models/V2/Sites/SiteMonitoring.php | 4 +- app/Models/V2/Sites/SiteReport.php | 4 +- routes/api_v2.php | 10 +- 17 files changed, 66 insertions(+), 122 deletions(-) create mode 100644 app/Models/V2/MediaModel.php diff --git a/app/Http/Controllers/V2/Files/UploadController.php b/app/Http/Controllers/V2/Files/UploadController.php index fc070a0c1..376c133fc 100644 --- a/app/Http/Controllers/V2/Files/UploadController.php +++ b/app/Http/Controllers/V2/Files/UploadController.php @@ -5,36 +5,21 @@ use App\Http\Controllers\Controller; use App\Http\Requests\V2\File\UploadRequest; use App\Http\Resources\V2\Files\FileResource; -use App\Models\V2\Forms\Form; -use App\Models\V2\Forms\FormQuestionOption; -use App\Models\V2\FundingProgramme; -use App\Models\V2\Nurseries\Nursery; -use App\Models\V2\Nurseries\NurseryReport; -use App\Models\V2\Organisation; -use App\Models\V2\ProjectPitch; -use App\Models\V2\Projects\Project; -use App\Models\V2\Projects\ProjectMonitoring; -use App\Models\V2\Projects\ProjectReport; -use App\Models\V2\Sites\Site; -use App\Models\V2\Sites\SiteMonitoring; -use App\Models\V2\Sites\SiteReport; -use Illuminate\Database\Eloquent\Model; -use Illuminate\Database\Eloquent\ModelNotFoundException; +use App\Models\V2\MediaModel; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Validator; use mysql_xdevapi\Exception; class UploadController extends Controller { - public function __invoke(UploadRequest $request, $model, $collection, $uuid) + public function __invoke(UploadRequest $request, string $collection, MediaModel $mediaModel) { - $entity = $this->getEntity($model, $uuid); - $this->authorize('uploadFiles', $entity); - $config = $this->getConfiguration($entity, $collection); + $this->authorize('uploadFiles', $mediaModel); + $config = $this->getConfiguration($mediaModel, $collection); $this->validateFile($request, $config); - $qry = $entity->addMediaFromRequest('upload_file'); - $this->prepHandler($qry, $request->all(), $entity, $config, $collection); + $qry = $mediaModel->addMediaFromRequest('upload_file'); + $this->prepHandler($qry, $request->all(), $mediaModel, $config, $collection); $details = $this->executeHandler($qry, $collection); if (Arr::has($request->all(), ['lat', 'lng'])) { @@ -46,73 +31,9 @@ public function __invoke(UploadRequest $request, $model, $collection, $uuid) return new FileResource($details); } - private function getEntity($model, $uuid): Model + private function getConfiguration(MediaModel $mediaModel, $collection): array { - switch ($model) { - case 'organisation': - $entity = Organisation::isUuid($uuid)->first(); - - break; - case 'project-pitch': - $entity = ProjectPitch::isUuid($uuid)->first(); - - break; - case 'funding-programme': - $entity = FundingProgramme::isUuid($uuid)->first(); - - break; - case 'form': - $entity = Form::isUuid($uuid)->first(); - - break; - case 'form-question-option': - $entity = FormQuestionOption::isUuid($uuid)->first(); - - break; - case 'project': - $entity = Project::isUuid($uuid)->first(); - - break; - case 'site': - $entity = Site::isUuid($uuid)->first(); - - break; - case 'nursery': - $entity = Nursery::isUuid($uuid)->first(); - - break; - case 'project-report': - $entity = ProjectReport::isUuid($uuid)->first(); - - break; - case 'site-report': - $entity = SiteReport::isUuid($uuid)->first(); - - break; - case 'nursery-report': - $entity = NurseryReport::isUuid($uuid)->first(); - - break; - case 'project-monitoring': - $entity = ProjectMonitoring::isUuid($uuid)->first(); - - break; - case 'site-monitoring': - $entity = SiteMonitoring::isUuid($uuid)->first(); - - break; - } - - if (empty($entity)) { - throw new ModelNotFoundException(); - } - - return $entity; - } - - private function getConfiguration($entity, $collection): array - { - $config = $entity->fileConfiguration[$collection]; + $config = $mediaModel->fileConfiguration[$collection]; if (empty($config)) { throw new Exception('Collection is unknown to this model.'); @@ -136,14 +57,14 @@ private function validateFile($request, $config): void $validator->validate(); } - private function prepHandler($qry, $data, $entity, $config, $collection): void + private function prepHandler($qry, $data, MediaModel $mediaModel, $config, $collection): void { if (data_get($data, 'title', false)) { $qry->usingName(data_get($data, 'title')); } if (! data_get($config, 'multiple', true)) { - $entity->clearMediaCollection($collection); + $mediaModel->clearMediaCollection($collection); } } diff --git a/app/Http/Middleware/ModelInterfaceBindingMiddleware.php b/app/Http/Middleware/ModelInterfaceBindingMiddleware.php index b4c884840..87118f1f4 100644 --- a/app/Http/Middleware/ModelInterfaceBindingMiddleware.php +++ b/app/Http/Middleware/ModelInterfaceBindingMiddleware.php @@ -53,7 +53,7 @@ class ModelInterfaceBindingMiddleware private static array $typeSlugsCache = []; - public static function with(string $interface, callable $routeGroup): RouteRegistrar + public static function with(string $interface, callable $routeGroup, string $prefix = null, string $modelParameter = null): RouteRegistrar { $typeSlugs = self::$typeSlugsCache[$interface] ?? []; if (empty($typeSlugs)) { @@ -66,13 +66,15 @@ public static function with(string $interface, callable $routeGroup): RouteRegis self::$typeSlugsCache[$interface] = $typeSlugs; } - return Route::prefix('/{modelSlug}') + $middleware = $modelParameter == null ? 'modelInterface' : "modelInterface:$modelParameter"; + + return Route::prefix("$prefix/{modelSlug}") ->whereIn('modelSlug', $typeSlugs) - ->middleware('modelInterface') + ->middleware($middleware) ->group($routeGroup); } - public function handle(Request $request, Closure $next) + public function handle(Request $request, Closure $next, $modelParameter = null) { $route = $request->route(); $parameterKeys = array_keys($route->parameters); @@ -85,8 +87,10 @@ public function handle(Request $request, Closure $next) $concreteClass = self::CONCRETE_MODELS[$modelSlug]; abort_unless($concreteClass, 404, "Concrete class not found for model interface $modelSlug"); - // assume the model key (e.g. "report") is the next param down the list from the interface name. - $modelParameter = $parameterKeys[$modelSlugIndex + 1]; + if ($modelParameter == null) { + // assume the model key (e.g. "report") is the next param down the list from the interface name. + $modelParameter = $parameterKeys[$modelSlugIndex + 1]; + } $modelId = $route->parameter($modelParameter); abort_unless($modelId, 404, "Model ID not found for $concreteClass"); diff --git a/app/Models/V2/Forms/Form.php b/app/Models/V2/Forms/Form.php index 3e75188f1..ff9199cb1 100644 --- a/app/Models/V2/Forms/Form.php +++ b/app/Models/V2/Forms/Form.php @@ -8,6 +8,7 @@ use App\Models\Traits\HasUuid; use App\Models\Traits\HasV2MediaCollections; use App\Models\V2\I18n\I18nItem; +use App\Models\V2\MediaModel; use App\Models\V2\Stages\Stage; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -15,11 +16,10 @@ use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\SoftDeletes; use Laravel\Scout\Searchable; -use Spatie\MediaLibrary\HasMedia; use Spatie\MediaLibrary\InteractsWithMedia; use Spatie\MediaLibrary\MediaCollections\Models\Media; -class Form extends Model implements HasMedia +class Form extends Model implements MediaModel { use HasFactory; use SoftDeletes; diff --git a/app/Models/V2/Forms/FormQuestionOption.php b/app/Models/V2/Forms/FormQuestionOption.php index c4856d23a..9255b84d6 100644 --- a/app/Models/V2/Forms/FormQuestionOption.php +++ b/app/Models/V2/Forms/FormQuestionOption.php @@ -6,16 +6,16 @@ use App\Models\Traits\HasUuid; use App\Models\Traits\HasV2MediaCollections; use App\Models\V2\I18n\I18nItem; +use App\Models\V2\MediaModel; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Str; -use Spatie\MediaLibrary\HasMedia; use Spatie\MediaLibrary\InteractsWithMedia; use Spatie\MediaLibrary\MediaCollections\Models\Media; -class FormQuestionOption extends Model implements HasMedia +class FormQuestionOption extends Model implements MediaModel { use HasFactory; use SoftDeletes; diff --git a/app/Models/V2/FundingProgramme.php b/app/Models/V2/FundingProgramme.php index e4a7c18a0..2afb3b51d 100644 --- a/app/Models/V2/FundingProgramme.php +++ b/app/Models/V2/FundingProgramme.php @@ -15,11 +15,10 @@ use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasManyThrough; use Illuminate\Database\Eloquent\SoftDeletes; -use Spatie\MediaLibrary\HasMedia; use Spatie\MediaLibrary\InteractsWithMedia; use Spatie\MediaLibrary\MediaCollections\Models\Media; -class FundingProgramme extends Model implements HasMedia +class FundingProgramme extends Model implements MediaModel { use HasFactory; use HasStatus; diff --git a/app/Models/V2/MediaModel.php b/app/Models/V2/MediaModel.php new file mode 100644 index 000000000..8689e01f1 --- /dev/null +++ b/app/Models/V2/MediaModel.php @@ -0,0 +1,14 @@ +middleware('i18n'); Route::get('/funding-programme/{fundingProgramme}', [FundingProgrammeController::class, 'show']); -Route::post('file/upload/{model}/{collection}/{uuid}', UploadController::class); +ModelInterfaceBindingMiddleware::with( + MediaModel::class, + function () { + Route::post('/{collection}/{mediaModel}', UploadController::class); + }, + prefix: 'file/upload', + modelParameter: 'mediaModel' +); Route::resource('files', FilePropertiesController::class); //Route::put('file/{uuid}', [FilePropertiesController::class, 'update']); From 024e56f0e98539a6d0e26b4029c1aa193638212c Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Wed, 17 Apr 2024 14:49:02 -0700 Subject: [PATCH 3/7] [TM-803] Ignore the .zip files generated during unit test runs. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 96c7b63ef..4bb685369 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /public/hot /public/storage /storage/*.key +/storage/*.zip /vendor .env /.phpunit.cache From 3a903d81f5635de08b7de426aaa9b9c6378e3649 Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Thu, 18 Apr 2024 14:52:31 -0700 Subject: [PATCH 4/7] [TM-803] Implement bulk upload of images to sites by service accounts. --- .../Migration/RolesMigrationCommand.php | 2 +- app/Exceptions/Handler.php | 5 + .../Controllers/V2/Files/UploadController.php | 61 +++++++-- .../Requests/V2/File/BulkUploadRequest.php | 47 +++++++ app/Policies/V2/Sites/SitePolicy.php | 18 ++- app/functions.php | 2 + config/wri/file-handling.php | 2 +- database/factories/UserFactory.php | 10 ++ routes/api_v2.php | 1 + tests/V2/Files/UploadControllerTest.php | 127 +++++++++++++++++- 10 files changed, 260 insertions(+), 15 deletions(-) create mode 100644 app/Http/Requests/V2/File/BulkUploadRequest.php diff --git a/app/Console/Commands/Migration/RolesMigrationCommand.php b/app/Console/Commands/Migration/RolesMigrationCommand.php index b94087e09..a0f1a4b41 100644 --- a/app/Console/Commands/Migration/RolesMigrationCommand.php +++ b/app/Console/Commands/Migration/RolesMigrationCommand.php @@ -75,7 +75,7 @@ public function handle() $role->givePermissionTo(['projects-read', 'polygons-manage', 'media-manage']); } - User::whereIn('role', ['user','admin', 'terrafund-admin'])->get() + User::whereIn('role', ['user', 'admin', 'terrafund-admin', 'service'])->get() ->each(function (User $user) { if ($user->primary_role == null) { assignSpatieRole($user); diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 87e3a9a40..9cfc7ab50 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -18,6 +18,8 @@ use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Log; use Illuminate\Validation\ValidationException; +use Spatie\MediaLibrary\MediaCollections\Exceptions\MimeTypeNotAllowed; +use Spatie\MediaLibrary\MediaCollections\Exceptions\UnreachableUrl; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -345,6 +347,9 @@ public function render($request, Throwable $exception) return JsonResponseHelper::error([], 404); case InvalidStatusException::class: return JsonResponseHelper::error($exception->getMessage(), 422); + case MimeTypeNotAllowed::class: + case UnreachableUrl::class: + return JsonResponseHelper::error([[$exception->getMessage()]], 422); default: if (config('app.env') == 'local') { return new Response($this->renderExceptionContent($exception), 500, ['Content-Type' => 'text/html']); diff --git a/app/Http/Controllers/V2/Files/UploadController.php b/app/Http/Controllers/V2/Files/UploadController.php index 376c133fc..dbf26dfa2 100644 --- a/app/Http/Controllers/V2/Files/UploadController.php +++ b/app/Http/Controllers/V2/Files/UploadController.php @@ -3,12 +3,14 @@ namespace App\Http\Controllers\V2\Files; use App\Http\Controllers\Controller; +use App\Http\Requests\V2\File\BulkUploadRequest; use App\Http\Requests\V2\File\UploadRequest; use App\Http\Resources\V2\Files\FileResource; use App\Models\V2\MediaModel; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Validator; -use mysql_xdevapi\Exception; +use Exception; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class UploadController extends Controller { @@ -22,13 +24,48 @@ public function __invoke(UploadRequest $request, string $collection, MediaModel $this->prepHandler($qry, $request->all(), $mediaModel, $config, $collection); $details = $this->executeHandler($qry, $collection); - if (Arr::has($request->all(), ['lat', 'lng'])) { - $this->saveFileCoordinates($details, $request); + $this->saveFileCoordinates($details, $request->all()); + $this->saveAdditionalFileProperties($details, $request->all(), $config); + + return new FileResource($details); + } + + public function bulkUrlUpload(BulkUploadRequest $request, string $collection, MediaModel $mediaModel) + { + $this->authorize('uploadFiles', $mediaModel); + + if ($collection != 'photos') { + // Only the photos collection is allowed for bulk upload + throw new NotFoundHttpException(); } - $this->saveAdditionalFileProperties($details, $request, $config); + $config = $this->getConfiguration($mediaModel, $collection); + $files = []; + try { + foreach ($request->getPayload() as $data) { + // The downloadable file gets shuttled through the internals of Spatie without a chance for us to run + // our own validations on them. png/jpg are the only mimes allowed for the photos collection according + // to config/file-handling.php, and we disallow other collections than 'photos' above. + $handler = $mediaModel->addMediaFromUrl($data['download_url'], 'image/png', 'image/jpg'); + + $this->prepHandler($handler, $data, $mediaModel, $config, $collection); + $details = $this->executeHandler($handler, $collection); + + $this->saveFileCoordinates($details, $data); + $this->saveAdditionalFileProperties($details, $data, $config); + + $files[] = $details; + } + } catch (Exception $exception) { + // if we get an error in the bulk upload, remove any media that did successfully get saved. + foreach ($files as $file) { + $file->delete(); + } + + throw $exception; + } - return new FileResource($details); + return FileResource::collection($files); } private function getConfiguration(MediaModel $mediaModel, $collection): array @@ -76,17 +113,19 @@ private function executeHandler($handler, $collection) ->toMediaCollection($collection); } - private function saveFileCoordinates($media, $request) + private function saveFileCoordinates($media, $data) { - $media->lat = $request->lat; - $media->lng = $request->lng; - $media->save(); + if (Arr::has($data, ['lat', 'lng'])) { + $media->lat = $data['lat']; + $media->lng = $data['lng']; + $media->save(); + } } - private function saveAdditionalFileProperties($media, $request, $config) + private function saveAdditionalFileProperties($media, $data, $config) { $media->file_type = $this->getType($media, $config); - $media->is_public = $request->is_public ?? true; + $media->is_public = $data['is_public'] ?? true; $media->save(); } diff --git a/app/Http/Requests/V2/File/BulkUploadRequest.php b/app/Http/Requests/V2/File/BulkUploadRequest.php new file mode 100644 index 000000000..de8623919 --- /dev/null +++ b/app/Http/Requests/V2/File/BulkUploadRequest.php @@ -0,0 +1,47 @@ + [ + 'prohibited', + ], + '*.download_url' => [ + 'required', + 'url', + ], + '*.title' => [ + 'sometimes', + 'nullable', + 'string', + ], + '*.lat' => [ + 'nullable', + 'numeric', + 'between:-90,90', + ], + '*.lng' => [ + 'nullable', + 'numeric', + 'between:-180,180', + ], + '*.is_public' => [ + 'sometimes', + 'nullable', + 'boolean', + ], + ]; + } +} diff --git a/app/Policies/V2/Sites/SitePolicy.php b/app/Policies/V2/Sites/SitePolicy.php index 9d910fca6..9f8370a3c 100644 --- a/app/Policies/V2/Sites/SitePolicy.php +++ b/app/Policies/V2/Sites/SitePolicy.php @@ -83,7 +83,23 @@ protected function isTheirs(?User $user, ?Site $site = null): bool public function uploadFiles(?User $user, ?Site $site = null): bool { - return $user->email_address_verified_at != null; + if ($user->email_address_verified_at == null) { + return false; + } + + if ($user->can('manage-own') && $this->isTheirs($user, $site)) { + return true; + } + + if ($user->can('framework-' . $site->framework_key)) { + return true; + } + + if ($user->can('media-manage')) { + return true; + } + + return false; } public function export(?User $user, ?Form $form = null, ?Project $project = null): bool diff --git a/app/functions.php b/app/functions.php index 7407f7590..9076847d6 100644 --- a/app/functions.php +++ b/app/functions.php @@ -165,5 +165,7 @@ function assignSpatieRole($user) $user->assignRole('admin-terrafund'); break; + case 'service': + $user->assignRole('greenhouse-service-account'); } } diff --git a/config/wri/file-handling.php b/config/wri/file-handling.php index eb58e801a..2392ccab8 100644 --- a/config/wri/file-handling.php +++ b/config/wri/file-handling.php @@ -5,7 +5,7 @@ 'logo-image' => 'file|max:3000|mimes:jpg,png', 'cover-image' => 'file|max:20000|mimes:jpg,png', 'cover-image-with-svg' => 'file|max:20000|mimes:jpg,png,svg', - 'photos' => 'file|max:25000|mimes:jpg,png', + 'photos' => 'file|max:2|mimes:jpg,png', 'pdf' => 'file|max:5000|mimes:pdf', 'documents' => 'file|mimes:pdf,xls,xlsx,csv,txt,doc', 'general-documents' => 'file|mimes:pdf,xls,xlsx,csv,txt,png,jpg,doc', diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index 864925f0d..34c203713 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -67,4 +67,14 @@ public function terrafundAdmin() ]; }); } + + public function serviceAccount() + { + return $this->state(function (array $attributes) { + return [ + 'api_key' => base64_encode(random_bytes(48)), + 'role' => 'service', + ]; + }); + } } diff --git a/routes/api_v2.php b/routes/api_v2.php index e4fe6cb88..6f441b75a 100644 --- a/routes/api_v2.php +++ b/routes/api_v2.php @@ -611,6 +611,7 @@ MediaModel::class, function () { Route::post('/{collection}/{mediaModel}', UploadController::class); + Route::post('/{collection}/{mediaModel}/bulk_url', [UploadController::class, 'bulkUrlUpload']); }, prefix: 'file/upload', modelParameter: 'mediaModel' diff --git a/tests/V2/Files/UploadControllerTest.php b/tests/V2/Files/UploadControllerTest.php index fd9b524ea..3a1eb7c40 100644 --- a/tests/V2/Files/UploadControllerTest.php +++ b/tests/V2/Files/UploadControllerTest.php @@ -15,8 +15,11 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\WithFaker; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Str; use Tests\TestCase; +use function PHPUnit\Framework\assertContains; final class UploadControllerTest extends TestCase { @@ -221,7 +224,8 @@ public function test_file_upload_for_a_project_report_is_successful() public function test_file_upload_for_a_site_is_successful() { $admin = User::factory()->admin()->create(); - $site = Site::factory()->create(); + Artisan::call('v2migration:roles'); + $site = Site::factory()->ppc()->create(); Storage::fake('uploads'); @@ -486,4 +490,125 @@ public function test_file_upload_sets_media_file_type_successfully() 'file_type' => 'documents', ]); } + + public function test_bulk_upload_validation() + { + $service = User::factory()->serviceAccount()->create(); + Artisan::call('v2migration:roles'); + $site = Site::factory()->create(); + $organisation = Organisation::factory()->create(); + // It's not ideal for the testing suite to use a real hosted image, but I haven't found a way to fake a + // http download URL in phpunit/spatie. + $url = 'https://new-wri-prod.wri-restoration-marketplace-api.com/images/V2/land-tenures/national-protected-area.png'; + + // User doesn't own the site. + $this->actingAs(User::factory()->create()) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", []) + ->assertForbidden(); + + // Service accounts can only upload to sites + $this->actingAs($service) + ->postJson("/api/v2/file/upload/organisation/photos/$organisation->uuid/bulk_url", []) + ->assertForbidden(); + + // Only the photos collection is allowed + $this->actingAs($service) + ->postJson("/api/v2/file/upload/site/pdf/$site->uuid/bulk_url", []) + ->assertStatus(404); + + // UUID isn't allowed + $content = $this->actingAs($service) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [["uuid" => "test", "download_url" => "test"]]) + ->assertStatus(422) + ->json(); + $this->assertStringContainsString('uuid field is prohibited', $content['errors'][0]['detail']); + + // Payload isn't an array of images + $this->actingAs($service) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", ["download_url" => "test"]) + ->assertStatus(422); + + // Payload has incorrect download URL format + $content = $this->actingAs($service) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [["download_url" => "test"]]) + ->assertStatus(422) + ->json(); + $this->assertStringContainsString('format is invalid', $content['errors'][0]['detail']); + + // Unreachable URL + $content = $this->actingAs($service) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [["download_url" => 'https://terramatch.org/foo.jpg']]) + ->assertStatus(422) + ->json(); + $this->assertStringContainsString('cannot be reached', $content['errors'][0]['detail']); + } + + public function test_bulk_upload_functionality() + { + $service = User::factory()->serviceAccount()->create(); + Artisan::call('v2migration:roles'); + $site = Site::factory()->create(); + // It's not ideal for the testing suite to use a real hosted image, but I haven't found a way to fake a + // http download URL in phpunit/spatie. + $url = 'https://new-wri-prod.wri-restoration-marketplace-api.com/images/V2/land-tenures/national-protected-area.png'; + $badMimeUrl = 'https://www.terramatch.org/images/landing-page-hero-banner.webp'; + + // Check a valid upload + $this->actingAs($service) + ->postJson( + "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", + [["download_url" => $url]] + ) + ->assertSuccessful(); + $site = $site->refresh(); + $this->assertEquals($site->getMedia('photos')->count(), 1); + $media = $site->getFirstMedia('photos'); + $this->assertEquals($media->mime_type, 'image/png'); + $this->assertEquals($media->file_name, 'national-protected-area.png'); + + // Check that the first file doesn't stick around in an invalid upload + $site->clearMediaCollection('photos'); + $content = $this->actingAs($service) + ->postJson( + "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", + [["download_url" => $url], ["download_url" => $badMimeUrl]]) + ->assertStatus(422) + ->json(); + $this->assertStringContainsString('File has a mime type', $content['errors'][0]['detail']); + $site = $site->refresh(); + $this->assertEquals($site->getMedia('photos')->count(), 0); + + // Check that multiple file upload works + $site->clearMediaCollection('photos'); + $this->actingAs($service) + ->postJson( + "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", + [["download_url" => $url], ["download_url" => $url]] + ) + ->assertSuccessful(); + $site = $site->refresh(); + $this->assertEquals($site->getMedia('photos')->count(), 2); + + // Check that optional fields are honored + $site->clearMediaCollection('photos'); + $this->actingAs($service) + ->postJson( + "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", + [[ + "download_url" => $url, + "title" => "Test Image", + "lat" => 42, + "lng" => -50, + "is_public" => false, + ]] + ) + ->assertSuccessful(); + $site = $site->refresh(); + $media = $site->getFirstMedia('photos'); + $this->assertNotNull($media->uuid); + $this->assertEquals($media->name, 'Test Image'); + $this->assertEquals($media->lat, 42); + $this->assertEquals($media->lng, -50); + $this->assertEquals($media->is_public, false); + } } From 307e3198824ea425b47bacf6127626188aa9a50e Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Thu, 18 Apr 2024 14:53:47 -0700 Subject: [PATCH 5/7] [TM-803] Lint fix --- .../Controllers/V2/Files/UploadController.php | 3 ++- .../Requests/V2/File/BulkUploadRequest.php | 1 - database/factories/UserFactory.php | 8 +++--- tests/V2/Files/UploadControllerTest.php | 27 +++++++++---------- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/app/Http/Controllers/V2/Files/UploadController.php b/app/Http/Controllers/V2/Files/UploadController.php index dbf26dfa2..d764e46d3 100644 --- a/app/Http/Controllers/V2/Files/UploadController.php +++ b/app/Http/Controllers/V2/Files/UploadController.php @@ -7,9 +7,9 @@ use App\Http\Requests\V2\File\UploadRequest; use App\Http\Resources\V2\Files\FileResource; use App\Models\V2\MediaModel; +use Exception; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Validator; -use Exception; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class UploadController extends Controller @@ -41,6 +41,7 @@ public function bulkUrlUpload(BulkUploadRequest $request, string $collection, Me $config = $this->getConfiguration($mediaModel, $collection); $files = []; + try { foreach ($request->getPayload() as $data) { // The downloadable file gets shuttled through the internals of Spatie without a chance for us to run diff --git a/app/Http/Requests/V2/File/BulkUploadRequest.php b/app/Http/Requests/V2/File/BulkUploadRequest.php index de8623919..462c32c2c 100644 --- a/app/Http/Requests/V2/File/BulkUploadRequest.php +++ b/app/Http/Requests/V2/File/BulkUploadRequest.php @@ -2,7 +2,6 @@ namespace App\Http\Requests\V2\File; -use App\Rules\CheckMimeTypeRule; use Illuminate\Foundation\Http\FormRequest; class BulkUploadRequest extends FormRequest diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index 34c203713..18cb951cd 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -71,10 +71,10 @@ public function terrafundAdmin() public function serviceAccount() { return $this->state(function (array $attributes) { - return [ - 'api_key' => base64_encode(random_bytes(48)), - 'role' => 'service', - ]; + return [ + 'api_key' => base64_encode(random_bytes(48)), + 'role' => 'service', + ]; }); } } diff --git a/tests/V2/Files/UploadControllerTest.php b/tests/V2/Files/UploadControllerTest.php index 3a1eb7c40..e11fe2c8c 100644 --- a/tests/V2/Files/UploadControllerTest.php +++ b/tests/V2/Files/UploadControllerTest.php @@ -17,9 +17,7 @@ use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Storage; -use Illuminate\Support\Str; use Tests\TestCase; -use function PHPUnit\Framework\assertContains; final class UploadControllerTest extends TestCase { @@ -518,26 +516,26 @@ public function test_bulk_upload_validation() // UUID isn't allowed $content = $this->actingAs($service) - ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [["uuid" => "test", "download_url" => "test"]]) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [['uuid' => 'test', 'download_url' => 'test']]) ->assertStatus(422) ->json(); $this->assertStringContainsString('uuid field is prohibited', $content['errors'][0]['detail']); // Payload isn't an array of images $this->actingAs($service) - ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", ["download_url" => "test"]) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", ['download_url' => 'test']) ->assertStatus(422); // Payload has incorrect download URL format $content = $this->actingAs($service) - ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [["download_url" => "test"]]) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [['download_url' => 'test']]) ->assertStatus(422) ->json(); $this->assertStringContainsString('format is invalid', $content['errors'][0]['detail']); // Unreachable URL $content = $this->actingAs($service) - ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [["download_url" => 'https://terramatch.org/foo.jpg']]) + ->postJson("/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [['download_url' => 'https://terramatch.org/foo.jpg']]) ->assertStatus(422) ->json(); $this->assertStringContainsString('cannot be reached', $content['errors'][0]['detail']); @@ -557,7 +555,7 @@ public function test_bulk_upload_functionality() $this->actingAs($service) ->postJson( "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", - [["download_url" => $url]] + [['download_url' => $url]] ) ->assertSuccessful(); $site = $site->refresh(); @@ -571,7 +569,8 @@ public function test_bulk_upload_functionality() $content = $this->actingAs($service) ->postJson( "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", - [["download_url" => $url], ["download_url" => $badMimeUrl]]) + [['download_url' => $url], ['download_url' => $badMimeUrl]] + ) ->assertStatus(422) ->json(); $this->assertStringContainsString('File has a mime type', $content['errors'][0]['detail']); @@ -583,7 +582,7 @@ public function test_bulk_upload_functionality() $this->actingAs($service) ->postJson( "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", - [["download_url" => $url], ["download_url" => $url]] + [['download_url' => $url], ['download_url' => $url]] ) ->assertSuccessful(); $site = $site->refresh(); @@ -595,11 +594,11 @@ public function test_bulk_upload_functionality() ->postJson( "/api/v2/file/upload/site/photos/$site->uuid/bulk_url", [[ - "download_url" => $url, - "title" => "Test Image", - "lat" => 42, - "lng" => -50, - "is_public" => false, + 'download_url' => $url, + 'title' => 'Test Image', + 'lat' => 42, + 'lng' => -50, + 'is_public' => false, ]] ) ->assertSuccessful(); From 577ff4384ae9a5456e44b9378fc92fab472baf60 Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Thu, 18 Apr 2024 15:14:04 -0700 Subject: [PATCH 6/7] [TM-803] Update docs --- openapi-src/V2/paths/_index.yml | 49 +++++++++++++++++++++- resources/docs/swagger-v2.yml | 73 ++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/openapi-src/V2/paths/_index.yml b/openapi-src/V2/paths/_index.yml index d96c9f717..9419eff25 100644 --- a/openapi-src/V2/paths/_index.yml +++ b/openapi-src/V2/paths/_index.yml @@ -887,13 +887,58 @@ in: formData - type: boolean name: is_public - default: false + default: true in: formData responses: '200': description: OK schema: $ref: '../definitions/_index.yml#/V2FileRead' +/v2/file/upload/site/photos/{UUID}/bulk_url: + post: + summary: Upload a batch of photos to a specific site + operationId: v2-post-upload-file-site-photos-uuid-bulk + tags: + - Files + consumes: + - application/json + produces: + - application/json + parameters: + - type: string + name: UUID + in: path + required: true + - description: Batch of photos to upload + in: body + name: body + required: true + schema: + type: array + items: + type: object + properties: + download_url: + type: string + title: + type: string + default: Name of image + lat: + type: integer + default: null + lng: + type: integer + default: null + is_public: + type: boolean + default: true + responses: + '200': + description: OK + schema: + type: array + items: + $ref: '../definitions/_index.yml#/V2FileRead' /v2/files/{UUID}: put: summary: Update properties of a specific file @@ -2457,4 +2502,4 @@ $ref: './Exports/get-v2-projects-uuid-entity-export.yml' /v2/{ENTITY}/{UUID}/export: get: - $ref: './Exports/get-v2-entity-export-uuid.yml' \ No newline at end of file + $ref: './Exports/get-v2-entity-export-uuid.yml' diff --git a/resources/docs/swagger-v2.yml b/resources/docs/swagger-v2.yml index 77ac99069..6219c06da 100644 --- a/resources/docs/swagger-v2.yml +++ b/resources/docs/swagger-v2.yml @@ -69825,7 +69825,7 @@ paths: in: formData - type: boolean name: is_public - default: false + default: true in: formData responses: '200': @@ -69858,6 +69858,77 @@ paths: type: boolean created_at: type: string + '/v2/file/upload/site/photos/{UUID}/bulk_url': + post: + summary: Upload a batch of photos to a specific site + operationId: v2-post-upload-file-site-photos-uuid-bulk + tags: + - Files + consumes: + - application/json + produces: + - application/json + parameters: + - type: string + name: UUID + in: path + required: true + - description: Batch of photos to upload + in: body + name: body + required: true + schema: + type: array + items: + type: object + properties: + download_url: + type: string + title: + type: string + default: Name of image + lat: + type: integer + default: null + lng: + type: integer + default: null + is_public: + type: boolean + default: true + responses: + '200': + description: OK + schema: + type: array + items: + title: V2FileRead + type: object + properties: + uuid: + type: string + url: + type: string + thumb_url: + type: string + collection_name: + type: string + title: + type: string + file_name: + type: string + mime_type: + type: string + size: + type: integer + lat: + type: integer + lng: + type: integer + is_public: + type: boolean + created_at: + type: string '/v2/files/{UUID}': put: summary: Update properties of a specific file From d43777e4801485078c3f70daf81dbb1f5202bd94 Mon Sep 17 00:00:00 2001 From: Nathan Curtis Date: Fri, 19 Apr 2024 09:39:30 -0700 Subject: [PATCH 7/7] [TM-803] Roll back testing change. --- config/wri/file-handling.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/wri/file-handling.php b/config/wri/file-handling.php index 2392ccab8..eb58e801a 100644 --- a/config/wri/file-handling.php +++ b/config/wri/file-handling.php @@ -5,7 +5,7 @@ 'logo-image' => 'file|max:3000|mimes:jpg,png', 'cover-image' => 'file|max:20000|mimes:jpg,png', 'cover-image-with-svg' => 'file|max:20000|mimes:jpg,png,svg', - 'photos' => 'file|max:2|mimes:jpg,png', + 'photos' => 'file|max:25000|mimes:jpg,png', 'pdf' => 'file|max:5000|mimes:pdf', 'documents' => 'file|mimes:pdf,xls,xlsx,csv,txt,doc', 'general-documents' => 'file|mimes:pdf,xls,xlsx,csv,txt,png,jpg,doc',