From e1e67949ae576e229d1350df665fd1f9c549c932 Mon Sep 17 00:00:00 2001 From: Quentin Renard Date: Wed, 19 Feb 2020 23:31:03 -0500 Subject: [PATCH] Some cleanup and best practices from phpstan feedback --- phpstan.neon | 21 +++----- src/Commands/ModuleMake.php | 40 +++++++++++++++ .../Controllers/Admin/FeaturedController.php | 4 +- .../Admin/FileLibraryController.php | 38 +++++++++----- .../Controllers/Admin/LoginController.php | 7 +++ .../Admin/MediaLibraryController.php | 26 ++++++---- .../Controllers/Admin/ModuleController.php | 51 +++++++++++++++++-- .../Admin/ResetPasswordController.php | 7 +++ src/Repositories/Behaviors/HandleBrowsers.php | 9 ++-- src/Repositories/BlockRepository.php | 2 +- src/Repositories/ModuleRepository.php | 7 ++- src/Repositories/SettingRepository.php | 2 + src/Services/MediaLibrary/Glide.php | 8 +-- src/TwillServiceProvider.php | 4 +- src/ValidationServiceProvider.php | 2 + tests/integration/FileLibraryTest.php | 2 +- tests/integration/MediaLibraryTest.php | 2 +- 17 files changed, 174 insertions(+), 58 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 8c693d1af..bb5308bfa 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -6,28 +6,19 @@ parameters: paths: - src - routes - - migrations - config - tests # 8 is the highest level - level: 6 + level: 1 + + autoload_directories: + - migrations autoload_files: - - migrations/add_locale_column_to_twill_mediables.php - - migrations/add_locale_column_to_twill_mediables.php - - migrations/add_two_factor_auth_columns_to_twill_users.php - - migrations/change_locale_column_in_twill_fileables.php - - migrations/create_blocks_table.php - - migrations/create_features_table.php - - migrations/create_files_tables.php - - migrations/create_medias_tables.php - - migrations/create_related_table.php - - migrations/create_settings_table.php - - migrations/create_tags_tables.php - - migrations/create_twill_activity_log_table.php - - migrations/create_twill_users_tables.php + - tests/helpers.php ignoreErrors: excludes_analyse: + - tests/stubs/* diff --git a/src/Commands/ModuleMake.php b/src/Commands/ModuleMake.php index 0888eaede..834b4c839 100644 --- a/src/Commands/ModuleMake.php +++ b/src/Commands/ModuleMake.php @@ -58,6 +58,46 @@ class ModuleMake extends Command */ protected $config; + /** + * @var bool + */ + protected $blockable; + + /** + * @var bool + */ + protected $translatable; + + /** + * @var bool + */ + protected $sluggable; + + /** + * @var bool + */ + protected $mediable; + + /** + * @var bool + */ + protected $fileable; + + /** + * @var bool + */ + protected $sortable; + + /** + * @var bool + */ + protected $revisionable; + + /** + * @var bool + */ + protected $defaultsAnswserToNo; + /** * @param Filesystem $files * @param Composer $composer diff --git a/src/Http/Controllers/Admin/FeaturedController.php b/src/Http/Controllers/Admin/FeaturedController.php index 23bdb775a..919d5df22 100644 --- a/src/Http/Controllers/Admin/FeaturedController.php +++ b/src/Http/Controllers/Admin/FeaturedController.php @@ -147,7 +147,7 @@ private function getFeaturedSources(Request $request, $featuredSection, $search $featuredSources = []; Collection::make($featuredSection['buckets'])->map(function ($bucket, $bucketKey) use (&$fetchedModules, $search, $request) { - return Collection::make($bucket['bucketables'])->mapWithKeys(function ($bucketable) use (&$fetchedModules, $bucketKey, $search, $request) { + return Collection::make($bucket['bucketables'])->mapWithKeys(function ($bucketable) use (&$fetchedModules, $search, $request) { $module = $bucketable['module']; $repository = $this->getRepository($module, $bucketable['repository'] ?? null); @@ -177,7 +177,7 @@ private function getFeaturedSources(Request $request, $featuredSection, $search ]]; }); })->each(function ($bucketables, $bucket) use (&$featuredSources) { - $bucketables->each(function ($bucketableData, $bucketable) use ($bucket, &$featuredSources) { + $bucketables->each(function ($bucketableData, $bucketable) use (&$featuredSources) { $featuredSources[$bucketable]['name'] = $bucketableData['name']; $featuredSources[$bucketable]['maxPage'] = $bucketableData['items']->lastPage(); $featuredSources[$bucketable]['offset'] = $bucketableData['items']->perPage(); diff --git a/src/Http/Controllers/Admin/FileLibraryController.php b/src/Http/Controllers/Admin/FileLibraryController.php index deb6ae55a..d8fe5eae0 100644 --- a/src/Http/Controllers/Admin/FileLibraryController.php +++ b/src/Http/Controllers/Admin/FileLibraryController.php @@ -51,14 +51,28 @@ class FileLibraryController extends ModuleController implements SignUploadListen */ protected $endpointType; + /** + * @var Illuminate\Routing\UrlGenerator + */ + protected $urlGenerator; + + /** + * @var Illuminate\Routing\ResponseFactory + */ + protected $responseFactory; + + /** + * @var Illuminate\Config\Repository + */ + protected $config; + public function __construct( Application $app, Request $request, UrlGenerator $urlGenerator, ResponseFactory $responseFactory, Config $config - ) - { + ) { parent::__construct($app, $request); $this->urlGenerator = $urlGenerator; $this->responseFactory = $responseFactory; @@ -108,14 +122,14 @@ public function getIndexData($prependScope = []) private function buildFile($item) { return $item->toCmsArray() + [ - 'tags' => $item->tags->map(function ($tag) { - return $tag->name; - }), - 'deleteUrl' => $item->canDeleteSafely() ? moduleRoute($this->moduleName, $this->routePrefix, 'destroy', $item->id) : null, - 'updateUrl' => $this->urlGenerator->route('admin.file-library.files.single-update'), - 'updateBulkUrl' => $this->urlGenerator->route('admin.file-library.files.bulk-update'), - 'deleteBulkUrl' => $this->urlGenerator->route('admin.file-library.files.bulk-delete'), - ]; + 'tags' => $item->tags->map(function ($tag) { + return $tag->name; + }), + 'deleteUrl' => $item->canDeleteSafely() ? moduleRoute($this->moduleName, $this->routePrefix, 'destroy', $item->id) : null, + 'updateUrl' => $this->urlGenerator->route('admin.file-library.files.single-update'), + 'updateBulkUrl' => $this->urlGenerator->route('admin.file-library.files.bulk-update'), + 'deleteBulkUrl' => $this->urlGenerator->route('admin.file-library.files.bulk-delete'), + ]; } /** @@ -265,8 +279,8 @@ public function signAzureUpload(Request $request, SignAzureUpload $signAzureUplo public function uploadIsSigned($signature, $isJsonResponse = true) { return $isJsonResponse - ? $this->responseFactory->json($signature, 200) - : $this->responseFactory->make($signature, 200, ['Content-Type' => 'text/plain']); + ? $this->responseFactory->json($signature, 200) + : $this->responseFactory->make($signature, 200, ['Content-Type' => 'text/plain']); } /** diff --git a/src/Http/Controllers/Admin/LoginController.php b/src/Http/Controllers/Admin/LoginController.php index efdcfaa12..236da190d 100644 --- a/src/Http/Controllers/Admin/LoginController.php +++ b/src/Http/Controllers/Admin/LoginController.php @@ -56,6 +56,13 @@ class LoginController extends Controller */ protected $viewFactory; + /** + * The path the user should be redirected to. + * + * @var string + */ + protected $redirectTo; + public function __construct( Config $config, AuthManager $authManager, diff --git a/src/Http/Controllers/Admin/MediaLibraryController.php b/src/Http/Controllers/Admin/MediaLibraryController.php index 62dd421d8..d61f6f007 100644 --- a/src/Http/Controllers/Admin/MediaLibraryController.php +++ b/src/Http/Controllers/Admin/MediaLibraryController.php @@ -8,7 +8,6 @@ use A17\Twill\Services\Uploader\SignS3Upload; use A17\Twill\Services\Uploader\SignUploadListener; use Illuminate\Config\Repository as Config; -use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Contracts\Foundation\Application; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -59,13 +58,22 @@ class MediaLibraryController extends ModuleController implements SignUploadListe */ protected $customFields; + /** + * @var Illuminate\Routing\ResponseFactory + */ + protected $responseFactory; + + /** + * @var Illuminate\Config\Repository + */ + protected $config; + public function __construct( Application $app, Config $config, Request $request, ResponseFactory $responseFactory - ) - { + ) { parent::__construct($app, $request); $this->responseFactory = $responseFactory; $this->config = $config; @@ -224,7 +232,7 @@ public function bulkUpdate() return is_null($meta); })->toArray(); - $extraMetadatas = array_diff_key($metadatasFromRequest, array_flip((array)$this->request->get('fieldsRemovedFromBulkEditing', []))); + $extraMetadatas = array_diff_key($metadatasFromRequest, array_flip((array) $this->request->get('fieldsRemovedFromBulkEditing', []))); if (in_array('tags', $this->request->get('fieldsRemovedFromBulkEditing', []))) { $this->repository->addIgnoreFieldsBeforeSave('bulk_tags'); @@ -235,9 +243,9 @@ public function bulkUpdate() foreach ($ids as $id) { $this->repository->update($id, [ - 'bulk_tags' => $newTags ?? [], - 'previous_common_tags' => $previousCommonTags ?? [], - ] + $extraMetadatas); + 'bulk_tags' => $newTags ?? [], + 'previous_common_tags' => $previousCommonTags ?? [], + ] + $extraMetadatas); } $scopes = $this->filterScope(['id' => $ids]); @@ -279,8 +287,8 @@ public function signAzureUpload(Request $request, SignAzureUpload $signAzureUplo public function uploadIsSigned($signature, $isJsonResponse = true) { return $isJsonResponse - ? $this->responseFactory->json($signature, 200) - : $this->responseFactory->make($signature, 200, ['Content-Type' => 'text/plain']); + ? $this->responseFactory->json($signature, 200) + : $this->responseFactory->make($signature, 200, ['Content-Type' => 'text/plain']); } /** diff --git a/src/Http/Controllers/Admin/ModuleController.php b/src/Http/Controllers/Admin/ModuleController.php index f0252165e..5b9b2178e 100644 --- a/src/Http/Controllers/Admin/ModuleController.php +++ b/src/Http/Controllers/Admin/ModuleController.php @@ -31,6 +31,11 @@ abstract class ModuleController extends Controller */ protected $request; + /** + * @var string + */ + protected $namespace; + /** * @var string */ @@ -46,6 +51,11 @@ abstract class ModuleController extends Controller */ protected $modelName; + /** + * @var string + */ + protected $modelTitle; + /** * @var \A17\Twill\Repositories\ModuleRepository */ @@ -173,6 +183,41 @@ abstract class ModuleController extends Controller */ protected $disableEditor = false; + /** + * @var array + */ + protected $indexOptions; + + /** + * @var array + */ + protected $indexColumns; + + /** + * @var array + */ + protected $browserColumns; + + /** + * @var string + */ + protected $permalinkBase; + + /** + * @var array + */ + protected $defaultFilters; + + /** + * @var string + */ + protected $viewPrefix; + + /** + * @var string + */ + protected $previewView; + /** * List of permissions keyed by a request field. Can be used to prevent unauthorized field updates. * @@ -1024,7 +1069,7 @@ protected function getIndexUrls($moduleName, $routePrefix) 'feature', 'bulkFeature', 'bulkDelete', - ])->mapWithKeys(function ($endpoint) use ($moduleName, $routePrefix) { + ])->mapWithKeys(function ($endpoint) { return [ $endpoint . 'Url' => $this->getIndexOption($endpoint) ? moduleRoute( $this->moduleName, $this->routePrefix, $endpoint, @@ -1096,7 +1141,7 @@ protected function getBrowserTableData($items) $withImage = $this->moduleHas('medias'); return $items->map(function ($item) use ($withImage) { - $columnsData = Collection::make($this->browserColumns)->mapWithKeys(function ($column) use ($item, $withImage) { + $columnsData = Collection::make($this->browserColumns)->mapWithKeys(function ($column) use ($item) { return $this->getItemColumnData($item, $column); })->toArray(); @@ -1240,7 +1285,7 @@ protected function form($id, $item = null) 'permalinkPrefix' => $this->getPermalinkPrefix($baseUrl), 'saveUrl' => $this->getModuleRoute($item->id, 'update'), 'editor' => $this->moduleHas('revisions') && $this->moduleHas('blocks') && !$this->disableEditor, - 'blockPreviewUrl' => Route::has('admin.blocks.preview')? URL::route('admin.blocks.preview') : '#', + 'blockPreviewUrl' => Route::has('admin.blocks.preview') ? URL::route('admin.blocks.preview') : '#', 'revisions' => $this->moduleHas('revisions') ? $item->revisionsArray() : null, ] + (Route::has($previewRouteName) ? [ 'previewUrl' => moduleRoute($this->moduleName, $this->routePrefix, 'preview', $item->id), diff --git a/src/Http/Controllers/Admin/ResetPasswordController.php b/src/Http/Controllers/Admin/ResetPasswordController.php index 39d61da8a..6e634e164 100644 --- a/src/Http/Controllers/Admin/ResetPasswordController.php +++ b/src/Http/Controllers/Admin/ResetPasswordController.php @@ -38,6 +38,13 @@ class ResetPasswordController extends Controller */ protected $config; + /** + * The path the user should be redirected to. + * + * @var string + */ + protected $redirectTo; + /** * @var ViewFactory */ diff --git a/src/Repositories/Behaviors/HandleBrowsers.php b/src/Repositories/Behaviors/HandleBrowsers.php index 39b84a306..9ffbd9525 100644 --- a/src/Repositories/Behaviors/HandleBrowsers.php +++ b/src/Repositories/Behaviors/HandleBrowsers.php @@ -2,17 +2,18 @@ namespace A17\Twill\Repositories\Behaviors; +use A17\Twill\Models\Behaviors\HasMedias; use Illuminate\Support\Str; trait HandleBrowsers { /** - * All browsers used in the model, as an array of browser names: + * All browsers used in the model, as an array of browser names: * [ * 'books', * 'publications' * ]. - * + * * When only the browser name is given here, its rest information will be inferred from the name. * Each browser's detail can also be override with an array * [ @@ -141,8 +142,8 @@ public function getFormFieldsForRelatedBrowser($object, $relation) })->values()->toArray(); } - /** - * Get all browser' detail info from the $browsers attribute. + /** + * Get all browser' detail info from the $browsers attribute. * The missing information will be inferred by convention of Twill. * * @return Illuminate\Support\Collection diff --git a/src/Repositories/BlockRepository.php b/src/Repositories/BlockRepository.php index 49e730e8e..41726bbe0 100644 --- a/src/Repositories/BlockRepository.php +++ b/src/Repositories/BlockRepository.php @@ -45,7 +45,7 @@ public function hydrate($object, $fields) if (Schema::hasTable(config('twill.related_table', 'twill_related'))) { $relatedItems = Collection::make(); - Collection::make($fields['browsers'])->each(function ($items, $browserName) use ($object, &$relatedItems) { + Collection::make($fields['browsers'])->each(function ($items, $browserName) use (&$relatedItems) { Collection::make($items)->each(function ($item) use ($browserName, &$relatedItems) { try { $repository = $this->getModelRepository($item['endpointType'] ?? $browserName); diff --git a/src/Repositories/ModuleRepository.php b/src/Repositories/ModuleRepository.php index 29fe41a1d..3ff6e3c74 100644 --- a/src/Repositories/ModuleRepository.php +++ b/src/Repositories/ModuleRepository.php @@ -2,12 +2,11 @@ namespace A17\Twill\Repositories; -use A17\Twill\Models\Behaviors\HasMedias; use A17\Twill\Models\Behaviors\Sortable; -use A17\Twill\Repositories\Behaviors\HandleDates; use A17\Twill\Repositories\Behaviors\HandleBrowsers; -use A17\Twill\Repositories\Behaviors\HandleRepeaters; +use A17\Twill\Repositories\Behaviors\HandleDates; use A17\Twill\Repositories\Behaviors\HandleFieldsGroups; +use A17\Twill\Repositories\Behaviors\HandleRepeaters; use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\App; @@ -20,7 +19,7 @@ abstract class ModuleRepository { use HandleDates, HandleBrowsers, HandleRepeaters, HandleFieldsGroups; - + /** * @var \A17\Twill\Models\Model */ diff --git a/src/Repositories/SettingRepository.php b/src/Repositories/SettingRepository.php index 8204b153e..289824ca0 100644 --- a/src/Repositories/SettingRepository.php +++ b/src/Repositories/SettingRepository.php @@ -88,6 +88,8 @@ public function saveAll($settingsFields, $section = null) } if (isset($settingsTranslated) && !empty($settingsTranslated)) { + $settings = []; + foreach ($settingsTranslated as $key => $values) { Arr::set($settings, $key, ['key' => $key] + $section + $values); } diff --git a/src/Services/MediaLibrary/Glide.php b/src/Services/MediaLibrary/Glide.php index 8d910eb5f..3e7021094 100644 --- a/src/Services/MediaLibrary/Glide.php +++ b/src/Services/MediaLibrary/Glide.php @@ -32,12 +32,12 @@ class Glide implements ImageServiceInterface protected $request; /** - * @var League\Glide\Server + * @var \League\Glide\Server */ private $server; /** - * @var UrlBuilder + * @var \League\Glide\Urls\UrlBuilder */ private $urlBuilder; @@ -75,7 +75,7 @@ public function __construct(Config $config, Application $app, Request $request) /** * @param string $path - * @return StreamedResponse + * @return mixed */ public function render($path) { @@ -179,7 +179,7 @@ public function getPresetUrl($id, $preset) /** * @param string $id - * @return array|null + * @return string */ public function getRawUrl($id) { diff --git a/src/TwillServiceProvider.php b/src/TwillServiceProvider.php index a480293ad..4a0d43312 100644 --- a/src/TwillServiceProvider.php +++ b/src/TwillServiceProvider.php @@ -314,11 +314,11 @@ private function extendBlade() null != $data ? $data : "get_defined_vars()"); }); - $blade->directive('formField', function ($expression) use ($blade) { + $blade->directive('formField', function ($expression) { return $this->includeView('partials.form._', $expression); }); - $blade->directive('partialView', function ($expression) use ($blade) { + $blade->directive('partialView', function ($expression) { $expressionAsArray = str_getcsv($expression, ',', '\''); diff --git a/src/ValidationServiceProvider.php b/src/ValidationServiceProvider.php index 9fc303c3a..ae6ed5de4 100644 --- a/src/ValidationServiceProvider.php +++ b/src/ValidationServiceProvider.php @@ -33,6 +33,8 @@ public function boot() }); Validator::extend('validBlocks', function ($attribute, $value, $parameters, $validator) { + $blockMessages = []; + foreach ($value as $block) { $cmsBlock = $this->app->make(BlockRepository::class)->buildFromCmsArray($block, false); diff --git a/tests/integration/FileLibraryTest.php b/tests/integration/FileLibraryTest.php index 74fd59266..342e5ca6f 100644 --- a/tests/integration/FileLibraryTest.php +++ b/tests/integration/FileLibraryTest.php @@ -131,7 +131,7 @@ public function testCanUpdateInBulk() $crawler->assertStatus(200); $tags = collect( - $files->reduce(function ($carry, $file) use ($files) { + $files->reduce(function ($carry, $file) { return $carry + $file->tags->pluck('slug')->toArray(); }, []) ) diff --git a/tests/integration/MediaLibraryTest.php b/tests/integration/MediaLibraryTest.php index fd331c854..6c7d20327 100644 --- a/tests/integration/MediaLibraryTest.php +++ b/tests/integration/MediaLibraryTest.php @@ -131,7 +131,7 @@ public function testCanUpdateInBulk() $crawler->assertStatus(200); $tags = collect( - $medias->reduce(function ($carry, $media) use ($medias) { + $medias->reduce(function ($carry, $media) { return $carry + $media->tags->pluck('slug')->toArray(); }, []) )