From 129030daaf60d850ce38a769b0fb29730e54f548 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Tue, 6 Mar 2018 19:38:52 +0100 Subject: [PATCH 01/14] Save a SEO title and description when creating an incident --- .../Commands/Incident/CreateIncidentCommandHandler.php | 4 ++++ app/Http/Controllers/Dashboard/IncidentController.php | 3 ++- resources/views/dashboard/incidents/add.blade.php | 8 ++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php b/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php index 4504d905..7cba6f40 100644 --- a/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php +++ b/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php @@ -104,6 +104,10 @@ class CreateIncidentCommandHandler // Store any meta? if ($meta = $command->meta) { foreach ($meta as $key => $value) { + if (empty($value)) { + continue; + } + Meta::create([ 'key' => $key, 'value' => $value, diff --git a/app/Http/Controllers/Dashboard/IncidentController.php b/app/Http/Controllers/Dashboard/IncidentController.php index cc03457d..d77e881d 100644 --- a/app/Http/Controllers/Dashboard/IncidentController.php +++ b/app/Http/Controllers/Dashboard/IncidentController.php @@ -131,7 +131,8 @@ class IncidentController extends Controller Binput::get('stickied', false), Binput::get('occurred_at'), null, - [] + [], + ['seo' => Binput::get('seo', [])] )); } catch (ValidationException $e) { return cachet_redirect('dashboard.incidents.create') diff --git a/resources/views/dashboard/incidents/add.blade.php b/resources/views/dashboard/incidents/add.blade.php index eb758484..ad931928 100644 --- a/resources/views/dashboard/incidents/add.blade.php +++ b/resources/views/dashboard/incidents/add.blade.php @@ -129,6 +129,14 @@ @endif +
+ {{ trans('forms.optional') }} + +
+
+ {{ trans('forms.optional') }} + +
From b71c61ce7d837486bbd54c2e264c70ef6ab9af34 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Tue, 6 Mar 2018 20:04:07 +0100 Subject: [PATCH 02/14] Update a SEO title and description in the incident meta --- .../Incident/UpdateIncidentCommand.php | 12 +++++- .../Incident/CreateIncidentCommandHandler.php | 16 ++------ .../Incident/UpdateIncidentCommandHandler.php | 8 ++++ app/Bus/Handlers/Traits/StoresMeta.php | 40 +++++++++++++++++++ .../Dashboard/IncidentController.php | 3 +- .../views/dashboard/incidents/edit.blade.php | 8 ++++ 6 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 app/Bus/Handlers/Traits/StoresMeta.php diff --git a/app/Bus/Commands/Incident/UpdateIncidentCommand.php b/app/Bus/Commands/Incident/UpdateIncidentCommand.php index 36036e6d..d9e13cb8 100644 --- a/app/Bus/Commands/Incident/UpdateIncidentCommand.php +++ b/app/Bus/Commands/Incident/UpdateIncidentCommand.php @@ -106,6 +106,13 @@ final class UpdateIncidentCommand */ public $template_vars; + /** + * Meta key/value pairs. + * + * @var array + */ + public $meta = []; + /** * The validation rules. * @@ -122,6 +129,7 @@ final class UpdateIncidentCommand 'stickied' => 'nullable|bool', 'occurred_at' => 'nullable|string', 'template' => 'nullable|string', + 'meta' => 'nullable|array', ]; /** @@ -139,10 +147,11 @@ final class UpdateIncidentCommand * @param string|null $occurred_at * @param string|null $template * @param array $template_vars + * @param array $meta * * @return void */ - public function __construct(Incident $incident, $name, $status, $message, $visible, $component_id, $component_status, $notify, $stickied, $occurred_at, $template, array $template_vars = []) + public function __construct(Incident $incident, $name, $status, $message, $visible, $component_id, $component_status, $notify, $stickied, $occurred_at, $template, array $template_vars = [], $meta = []) { $this->incident = $incident; $this->name = $name; @@ -156,5 +165,6 @@ final class UpdateIncidentCommand $this->occurred_at = $occurred_at; $this->template = $template; $this->template_vars = $template_vars; + $this->meta = $meta; } } diff --git a/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php b/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php index 7cba6f40..76128d9b 100644 --- a/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php +++ b/app/Bus/Handlers/Commands/Incident/CreateIncidentCommandHandler.php @@ -15,6 +15,7 @@ use CachetHQ\Cachet\Bus\Commands\Component\UpdateComponentCommand; use CachetHQ\Cachet\Bus\Commands\Incident\CreateIncidentCommand; use CachetHQ\Cachet\Bus\Events\Incident\IncidentWasCreatedEvent; use CachetHQ\Cachet\Bus\Exceptions\Incident\InvalidIncidentTimestampException; +use CachetHQ\Cachet\Bus\Handlers\Traits\StoresMeta; use CachetHQ\Cachet\Models\Component; use CachetHQ\Cachet\Models\Incident; use CachetHQ\Cachet\Models\IncidentTemplate; @@ -32,6 +33,8 @@ use Twig_Loader_Array; */ class CreateIncidentCommandHandler { + use StoresMeta; + /** * The authentication guard instance. * @@ -103,18 +106,7 @@ class CreateIncidentCommandHandler // Store any meta? if ($meta = $command->meta) { - foreach ($meta as $key => $value) { - if (empty($value)) { - continue; - } - - Meta::create([ - 'key' => $key, - 'value' => $value, - 'meta_type' => 'incidents', - 'meta_id' => $incident->id, - ]); - } + $this->storeMeta($command->meta, 'incidents', $incident->id); } // Update the component. diff --git a/app/Bus/Handlers/Commands/Incident/UpdateIncidentCommandHandler.php b/app/Bus/Handlers/Commands/Incident/UpdateIncidentCommandHandler.php index 00bf9f8d..935e1d99 100644 --- a/app/Bus/Handlers/Commands/Incident/UpdateIncidentCommandHandler.php +++ b/app/Bus/Handlers/Commands/Incident/UpdateIncidentCommandHandler.php @@ -15,6 +15,7 @@ use CachetHQ\Cachet\Bus\Commands\Component\UpdateComponentCommand; use CachetHQ\Cachet\Bus\Commands\Incident\UpdateIncidentCommand; use CachetHQ\Cachet\Bus\Events\Incident\IncidentWasUpdatedEvent; use CachetHQ\Cachet\Bus\Exceptions\Incident\InvalidIncidentTimestampException; +use CachetHQ\Cachet\Bus\Handlers\Traits\StoresMeta; use CachetHQ\Cachet\Models\Component; use CachetHQ\Cachet\Models\Incident; use CachetHQ\Cachet\Models\IncidentTemplate; @@ -30,6 +31,8 @@ use Twig_Loader_Array; */ class UpdateIncidentCommandHandler { + use StoresMeta; + /** * The authentication guard instance. * @@ -86,6 +89,11 @@ class UpdateIncidentCommandHandler // Rather than making lots of updates, just fill and save. $incident->save(); + // Store any meta? + if ($meta = $command->meta) { + $this->storeMeta($command->meta, 'incidents', $incident->id); + } + // Update the component. if ($component = Component::find($command->component_id)) { dispatch(new UpdateComponentCommand( diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php new file mode 100644 index 00000000..b41f0045 --- /dev/null +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -0,0 +1,40 @@ + $value) { + if (empty($value)) { + continue; + } + + $meta = Meta::firstOrNew([ + 'key' => $key, + 'meta_type' => $type, + 'meta_id' => $id, + ]); + + $meta->value = $value; + + $meta->save(); + } + + } +} \ No newline at end of file diff --git a/app/Http/Controllers/Dashboard/IncidentController.php b/app/Http/Controllers/Dashboard/IncidentController.php index d77e881d..4d75ff7f 100644 --- a/app/Http/Controllers/Dashboard/IncidentController.php +++ b/app/Http/Controllers/Dashboard/IncidentController.php @@ -262,7 +262,8 @@ class IncidentController extends Controller Binput::get('stickied', false), Binput::get('occurred_at'), null, - [] + [], + ['seo' => Binput::get('seo', [])] )); } catch (ValidationException $e) { return cachet_redirect('dashboard.incidents.edit', ['id' => $incident->id]) diff --git a/resources/views/dashboard/incidents/edit.blade.php b/resources/views/dashboard/incidents/edit.blade.php index 6cb199f4..02d4a1fa 100644 --- a/resources/views/dashboard/incidents/edit.blade.php +++ b/resources/views/dashboard/incidents/edit.blade.php @@ -111,6 +111,14 @@ {{ trans('forms.optional') }}
+
+ {{ trans('forms.optional') }} + +
+
+ {{ trans('forms.optional') }} + +
From a42c2bb017d2434c12e96ae51b5248d44da454f7 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Tue, 6 Mar 2018 20:11:13 +0100 Subject: [PATCH 03/14] Use the custom SEO title and descriptions in the frontend --- resources/views/single-incident.blade.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/views/single-incident.blade.php b/resources/views/single-incident.blade.php index 59d88dc1..e3443eaf 100644 --- a/resources/views/single-incident.blade.php +++ b/resources/views/single-incident.blade.php @@ -1,8 +1,8 @@ @extends('layout.master') -@section('title', $incident->name.' | '.$site_title) +@section('title', array_get($incident->meta, 'seo.title', $incident->name).' | '.$site_title) -@section('description', trans('cachet.meta.description.incident', ['name' => $incident->name, 'date' => $incident->occurred_at_formatted])) +@section('description', array_get($incident->meta, 'seo.description', trans('cachet.meta.description.incident', ['name' => $incident->name, 'date' => $incident->occurred_at_formatted]))) @section('bodyClass', 'no-padding') From da4d036883102cfbbd144828790d0d800bed5261 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Tue, 6 Mar 2018 20:17:43 +0100 Subject: [PATCH 04/14] Add translations --- resources/lang/en/forms.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/resources/lang/en/forms.php b/resources/lang/en/forms.php index cb1ed400..73924820 100644 --- a/resources/lang/en/forms.php +++ b/resources/lang/en/forms.php @@ -226,6 +226,11 @@ return [ 'timezone' => 'Select Timezone', ], + 'seo' => [ + 'title' => 'SEO title', + 'description' => 'SEO description', + ], + // Buttons 'add' => 'Add', 'save' => 'Save', From f4f77b1f1bd0ce185356d1f249ed9ad6690f3af7 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Tue, 6 Mar 2018 20:28:32 +0100 Subject: [PATCH 05/14] Fix CodeStyle --- app/Bus/Handlers/Traits/StoresMeta.php | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php index b41f0045..d22a75df 100644 --- a/app/Bus/Handlers/Traits/StoresMeta.php +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -1,5 +1,14 @@ $key, + 'key' => $key, 'meta_type' => $type, - 'meta_id' => $id, + 'meta_id' => $id, ]); $meta->value = $value; $meta->save(); } - } -} \ No newline at end of file +} From 95a76a77b7276474ace01ee60a7af82eca70a52f Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 07:22:45 +0100 Subject: [PATCH 06/14] Typehint the meta argument on the create and update incident commands --- app/Bus/Commands/Incident/CreateIncidentCommand.php | 2 +- app/Bus/Commands/Incident/UpdateIncidentCommand.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Bus/Commands/Incident/CreateIncidentCommand.php b/app/Bus/Commands/Incident/CreateIncidentCommand.php index dde666b7..4f94f2f8 100644 --- a/app/Bus/Commands/Incident/CreateIncidentCommand.php +++ b/app/Bus/Commands/Incident/CreateIncidentCommand.php @@ -140,7 +140,7 @@ final class CreateIncidentCommand * * @return void */ - public function __construct($name, $status, $message, $visible, $component_id, $component_status, $notify, $stickied, $occurred_at, $template, array $template_vars = [], $meta = []) + public function __construct($name, $status, $message, $visible, $component_id, $component_status, $notify, $stickied, $occurred_at, $template, array $template_vars = [], array $meta = []) { $this->name = $name; $this->status = $status; diff --git a/app/Bus/Commands/Incident/UpdateIncidentCommand.php b/app/Bus/Commands/Incident/UpdateIncidentCommand.php index d9e13cb8..1328b37c 100644 --- a/app/Bus/Commands/Incident/UpdateIncidentCommand.php +++ b/app/Bus/Commands/Incident/UpdateIncidentCommand.php @@ -151,7 +151,7 @@ final class UpdateIncidentCommand * * @return void */ - public function __construct(Incident $incident, $name, $status, $message, $visible, $component_id, $component_status, $notify, $stickied, $occurred_at, $template, array $template_vars = [], $meta = []) + public function __construct(Incident $incident, $name, $status, $message, $visible, $component_id, $component_status, $notify, $stickied, $occurred_at, $template, array $template_vars = [], array $meta = []) { $this->incident = $incident; $this->name = $name; From 7bb23b6108050de7251bf22ae5ec71da31d89932 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 07:32:53 +0100 Subject: [PATCH 07/14] Remove an meta model if the corresponding value is considered empty --- app/Bus/Handlers/Traits/StoresMeta.php | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php index d22a75df..30eedf8b 100644 --- a/app/Bus/Handlers/Traits/StoresMeta.php +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -21,6 +21,10 @@ trait StoresMeta * @param $meta * @param $type * @param $id + * + * @return void + * + * @throws \Exception */ public function storeMeta($meta, $type, $id) { @@ -30,19 +34,22 @@ trait StoresMeta } foreach ($meta as $key => $value) { - if (empty($value)) { - continue; - } - $meta = Meta::firstOrNew([ 'key' => $key, 'meta_type' => $type, 'meta_id' => $id, ]); - $meta->value = $value; + if (!empty($value)) { + $meta->value = $value; + $meta->save(); + continue; + } - $meta->save(); + // The value is empty, remove the row + if($meta->exists) { + $meta->delete(); + } } } } From 5e655e645ae7f620299a2426556281835b2c9980 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 07:39:58 +0100 Subject: [PATCH 08/14] Capitalize Seo Title and Description --- resources/lang/en/forms.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lang/en/forms.php b/resources/lang/en/forms.php index 73924820..a654a51c 100644 --- a/resources/lang/en/forms.php +++ b/resources/lang/en/forms.php @@ -227,8 +227,8 @@ return [ ], 'seo' => [ - 'title' => 'SEO title', - 'description' => 'SEO description', + 'title' => 'SEO Title', + 'description' => 'SEO Description', ], // Buttons From 7982e474fcbfa88984365f1e0dfbd2096781c95c Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 07:40:18 +0100 Subject: [PATCH 09/14] Also use the custom title in the og:title meta attribute --- resources/views/layout/master.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/layout/master.blade.php b/resources/views/layout/master.blade.php index 413bfd34..dd36f66c 100644 --- a/resources/views/layout/master.blade.php +++ b/resources/views/layout/master.blade.php @@ -18,7 +18,7 @@ - + From 8d8d196aac4fea034fcc9f3b3f3cc236f4e87ccc Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 09:28:03 +0100 Subject: [PATCH 10/14] Add tests for the MetaSeo fields --- app/Bus/Handlers/Traits/StoresMeta.php | 6 +- tests/Functional/Incident/MetaSeoTest.php | 184 ++++++++++++++++++++++ 2 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 tests/Functional/Incident/MetaSeoTest.php diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php index 30eedf8b..0c4a4c2f 100644 --- a/app/Bus/Handlers/Traits/StoresMeta.php +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -22,9 +22,9 @@ trait StoresMeta * @param $type * @param $id * - * @return void - * * @throws \Exception + * + * @return void */ public function storeMeta($meta, $type, $id) { @@ -47,7 +47,7 @@ trait StoresMeta } // The value is empty, remove the row - if($meta->exists) { + if ($meta->exists) { $meta->delete(); } } diff --git a/tests/Functional/Incident/MetaSeoTest.php b/tests/Functional/Incident/MetaSeoTest.php new file mode 100644 index 00000000..10d375c6 --- /dev/null +++ b/tests/Functional/Incident/MetaSeoTest.php @@ -0,0 +1,184 @@ + + * @author Graham Campbell + */ +class MetaSeoTest extends AbstractTestCase +{ + use DatabaseMigrations; + + /** + * @var \Faker\Generator + */ + protected $fakerFactory; + + /** + * @var string + */ + protected $appName; + + /** + * CreateIncidentCommandTest constructor. + * + * @param null $name + * @param array $data + */ + public function __construct($name = null, array $data = [], $dataName = '') + { + parent::__construct($name, $data, $dataName); + $this->fakerFactory = \Faker\Factory::create(); + $this->appName = 'MetaSeoTest'; + } + + /** + * Setup the application. + */ + public function setUp() + { + parent::setUp(); + $this->app->make(SettingsRepository::class)->set('app_name', $this->appName); + } + + /** + * When using a custom meta description in an incident it will be + * showed in two meta tags on the incident details page. + */ + public function testCustomSeoDescriptionOnIncidentPage() + { + $description = $this->fakerFactory->sentence; + + $incident = $this->createIncidentWithMeta(['seo' => ['description' => $description]]); + $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + + $this->assertContains( + sprintf('', $description), + $page->content() + ); + $this->assertContains( + sprintf('', $description), + $page->content() + ); + } + + /** + * When using a custom meta title in an incident it will be + * showed in two meta tags on the incident details page. + */ + public function testCustomSeoTitleOnIncidentPage() + { + $title = $this->fakerFactory->title; + + $incident = $this->createIncidentWithMeta(['seo' => ['title' => $title]]); + $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + + $this->assertContains( + sprintf('', $title, $this->appName), + $page->content() + ); + $this->assertContains( + sprintf('%s | %s', $title, $this->appName), + $page->content() + ); + } + + /** + * When using no custom meta description in an incident, the application + * default generated description will be used on the incident details page. + */ + public function testNoCustomSeoDescriptionOnIncidentPage() + { + $incident = $this->createIncidentWithMeta([]); + $presenter = $this->app->make(IncidentPresenter::class); + $presenter->setWrappedObject($incident); + + $expectedDescription = sprintf( + 'Details and updates about the %s incident that occurred on %s', + $incident->name, + $presenter->occurred_at_formatted + ); + + $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + + $this->assertContains( + sprintf('', $expectedDescription), + $page->content() + ); + $this->assertContains( + sprintf('', $expectedDescription), + $page->content() + ); + } + + /** + * When using no custom meta description in an incident, the application + * default generated description will be used on the incident details page. + */ + public function testNoCustomSeoTitleOnIncidentPage() + { + $incident = $this->createIncidentWithMeta([]); + $expectedTitle = sprintf('%s | %s', $incident->name, $this->appName); + + $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + + $this->assertContains( + sprintf('', $expectedTitle), + $page->content() + ); + $this->assertContains(sprintf('%s', $expectedTitle), $page->content()); + } + + /** + * @param array $meta + * + * @return \Illuminate\Database\Eloquent\Model|static + */ + protected function createIncidentWithMeta(array $meta) + { + $user = factory(User::class)->create(); + $user->save(); + + Auth::login($user); + $name = $this->fakerFactory->name; + $message = $this->fakerFactory->sentence; + + dispatch(new CreateIncidentCommand( + $name, + $this->fakerFactory->numberBetween(0, 3), + $message, + $this->fakerFactory->boolean, + null, + null, + false, + $this->fakerFactory->boolean, + $this->fakerFactory->date('Y-m-d H:i'), + null, + [], + $meta + )); + + return Incident::where('name', '=', $name)->where('message', '=', $message)->firstOrFail(); + } +} From abf0a0ac9c4982afda598579e3626cdaa422b8a7 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 23:47:06 +0100 Subject: [PATCH 11/14] Add tests for the StoresMeta Trait and fix the Functional MetaSeo tests --- app/Bus/Handlers/Traits/StoresMeta.php | 60 +++++++--- tests/Bus/Traits/StoresMetaTest.php | 133 ++++++++++++++++++++++ tests/Functional/Incident/MetaSeoTest.php | 15 +-- 3 files changed, 182 insertions(+), 26 deletions(-) create mode 100644 tests/Bus/Traits/StoresMetaTest.php diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php index 0c4a4c2f..b308c657 100644 --- a/app/Bus/Handlers/Traits/StoresMeta.php +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -18,38 +18,64 @@ trait StoresMeta /** * Stores all Meta values of a model. * - * @param $meta - * @param $type - * @param $id - * - * @throws \Exception + * @param array $metaData + * @param string $metaType + * @param string|int $metaId + * @param string $metaModel * * @return void */ - public function storeMeta($meta, $type, $id) + public function storeMeta($metaData, $metaType, $metaId, $metaModel = Meta::class) { // Validation required instead of type hinting because it could be passed as false or NULL - if (!is_array($meta)) { + if (!is_array($metaData)) { return; } - foreach ($meta as $key => $value) { - $meta = Meta::firstOrNew([ - 'key' => $key, - 'meta_type' => $type, - 'meta_id' => $id, - ]); + foreach ($metaData as $key => $value) { + $modelInstance = call_user_func( + [$metaModel, 'firstOrNew'], + [ + 'key' => $key, + 'meta_type' => $metaType, + 'meta_id' => $metaId, + ] + ); + $value = $this->removeEmptyValues($value); if (!empty($value)) { - $meta->value = $value; - $meta->save(); + $modelInstance->setAttribute('value', $value); + $modelInstance->save(); continue; } // The value is empty, remove the row - if ($meta->exists) { - $meta->delete(); + if ($modelInstance->exists) { + $modelInstance->delete(); } } } + + /** + * Determine if a Value is empty. + * + * @param $values + * + * @return array|mixed + */ + protected function removeEmptyValues($values) + { + if (!is_array($values)) { + return empty($values) ? null : $values; + } + + foreach ($values as $key => $value) { + if (!empty($value)) { + continue; + } + unset($values[$key]); + } + + return $values; + } } diff --git a/tests/Bus/Traits/StoresMetaTest.php b/tests/Bus/Traits/StoresMetaTest.php new file mode 100644 index 00000000..641878f8 --- /dev/null +++ b/tests/Bus/Traits/StoresMetaTest.php @@ -0,0 +1,133 @@ +markTestSkipped('This test requires Mockery'); + } + + $this->metaModel = Mockery::mock(Meta::class)->makePartial(); + $this->app->instance(Meta::class, $this->metaModel); + } + + /** + * Our Mockery expectations should count as assertions to prevent warnings from PHPUnit. + */ + public function tearDown() + { + $this->addToAssertionCount(Mockery::getContainer()->mockery_getExpectationCount()); + parent::tearDown(); + } + + /** + * Each array value passed to the MetaValues should result in a new model instance. + */ + public function testStoresMetaWithSimpleMultipleArrays() + { + $mock = $this->getMockForTrait(StoresMeta::class); + $metaData = [ + 'somekey1' => 'somevalue', + 'somekey2' => 'somevalue', + 'somekey3' => 'somevalue', + ]; + + $this->metaModel->shouldReceive('firstOrNew')->times(3)->andReturn($this->metaModel); + $this->metaModel->shouldReceive('save')->times(3); + $this->metaModel->shouldReceive('setAttribute')->times(3)->andReturnUsing(function ($key, $value) { + $this->assertEquals('value', $key); + $this->assertEquals('somevalue', $value); + + return $this->metaModel; + }); + + $mock->storeMeta($metaData, 'some_class', 1, $this->metaModel); + } + + /** + * It will pass nested arrays to the value property of the Meta model. + */ + public function testStoresNestedArraysAsSingleValue() + { + $mock = $this->getMockForTrait(StoresMeta::class); + $metaData = ['somekey1' => ['subkey' => ['second' => 'key']]]; + + $this->metaModel->shouldReceive('firstOrNew')->once()->andReturn($this->metaModel); + $this->metaModel->shouldReceive('save')->once(); + $this->metaModel->shouldReceive('setAttribute')->once()->andReturnUsing(function ($key, $value) { + $this->assertEquals('value', $key); + $this->assertEquals(['subkey' => ['second' => 'key']], $value); + + return $this->metaModel; + }); + + $mock->storeMeta($metaData, 'some_class', 1, $this->metaModel); + } + + /** + * If a value is empty or null it will be removed. + */ + public function testEmptyValuesWillBeDeleted() + { + $mock = $this->getMockForTrait(StoresMeta::class); + $metaData = ['somekey1' => '']; + + $this->metaModel->exists = true; + $this->metaModel->shouldReceive('firstOrNew')->once()->andReturn($this->metaModel); + $this->metaModel->shouldReceive('delete')->once(); + $this->metaModel->shouldReceive('setAttribute')->never(); + $this->metaModel->shouldReceive('save')->never(); + + $mock->storeMeta($metaData, 'some_class', 1, $this->metaModel); + } + + /** + * If a value is empty or null in a nested array it will be removed. + */ + public function testEmptyNestedArrayKeysAreRemoved() + { + $mock = $this->getMockForTrait(StoresMeta::class); + $metaData = ['somekey1' => ['keyWithValue' => 'value123', 'keyWithoutValue' => null]]; + + $this->metaModel->exists = true; + $this->metaModel->shouldReceive('firstOrNew')->once()->andReturn($this->metaModel); + $this->metaModel->shouldReceive('setAttribute')->once()->andReturnUsing(function ($key, $value) { + $this->assertEquals('value', $key); + $this->assertEquals(['keyWithValue' => 'value123'], $value); + + return $this->metaModel; + }); + $this->metaModel->shouldReceive('save')->once(); + $this->metaModel->shouldReceive('delete')->never(); + + $mock->storeMeta($metaData, 'some_class', 1, $this->metaModel); + } +} diff --git a/tests/Functional/Incident/MetaSeoTest.php b/tests/Functional/Incident/MetaSeoTest.php index 10d375c6..c6f6cd38 100644 --- a/tests/Functional/Incident/MetaSeoTest.php +++ b/tests/Functional/Incident/MetaSeoTest.php @@ -13,12 +13,10 @@ namespace CachetHQ\Tests\Cachet\Functional\Bus\Commands\Incident; use CachetHQ\Cachet\Bus\Commands\Incident\CreateIncidentCommand; use CachetHQ\Cachet\Models\Incident; -use CachetHQ\Cachet\Models\User; use CachetHQ\Cachet\Presenters\IncidentPresenter; use CachetHQ\Cachet\Settings\Repository as SettingsRepository; use CachetHQ\Tests\Cachet\AbstractTestCase; use Illuminate\Foundation\Testing\DatabaseMigrations; -use Illuminate\Support\Facades\Auth; /** * This is the create incident command test class. @@ -43,8 +41,9 @@ class MetaSeoTest extends AbstractTestCase /** * CreateIncidentCommandTest constructor. * - * @param null $name - * @param array $data + * @param null $name + * @param array $data + * @param string $dataName */ public function __construct($name = null, array $data = [], $dataName = '') { @@ -60,6 +59,7 @@ class MetaSeoTest extends AbstractTestCase { parent::setUp(); $this->app->make(SettingsRepository::class)->set('app_name', $this->appName); + $this->app->config->set('setting.app_name', $this->appName); } /** @@ -153,14 +153,11 @@ class MetaSeoTest extends AbstractTestCase /** * @param array $meta * - * @return \Illuminate\Database\Eloquent\Model|static + * @return Incident */ protected function createIncidentWithMeta(array $meta) { - $user = factory(User::class)->create(); - $user->save(); - - Auth::login($user); + $this->signIn(); $name = $this->fakerFactory->name; $message = $this->fakerFactory->sentence; From a6af6ada01a78b645bd644b9d472c5fc949c390f Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 19 Mar 2018 23:55:38 +0100 Subject: [PATCH 12/14] Use htmlspecialchars in the assertion to match the blade escaping --- tests/Functional/Incident/MetaSeoTest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Functional/Incident/MetaSeoTest.php b/tests/Functional/Incident/MetaSeoTest.php index c6f6cd38..c1ea4761 100644 --- a/tests/Functional/Incident/MetaSeoTest.php +++ b/tests/Functional/Incident/MetaSeoTest.php @@ -68,17 +68,17 @@ class MetaSeoTest extends AbstractTestCase */ public function testCustomSeoDescriptionOnIncidentPage() { - $description = $this->fakerFactory->sentence; + $expectedDescription = htmlspecialchars($this->fakerFactory->sentence); - $incident = $this->createIncidentWithMeta(['seo' => ['description' => $description]]); + $incident = $this->createIncidentWithMeta(['seo' => ['description' => $expectedDescription]]); $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; $this->assertContains( - sprintf('', $description), + sprintf('', $expectedDescription), $page->content() ); $this->assertContains( - sprintf('', $description), + sprintf('', $expectedDescription), $page->content() ); } @@ -89,7 +89,7 @@ class MetaSeoTest extends AbstractTestCase */ public function testCustomSeoTitleOnIncidentPage() { - $title = $this->fakerFactory->title; + $title = htmlspecialchars($this->fakerFactory->title); $incident = $this->createIncidentWithMeta(['seo' => ['title' => $title]]); $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; @@ -116,7 +116,7 @@ class MetaSeoTest extends AbstractTestCase $expectedDescription = sprintf( 'Details and updates about the %s incident that occurred on %s', - $incident->name, + htmlspecialchars($incident->name), $presenter->occurred_at_formatted ); @@ -139,7 +139,7 @@ class MetaSeoTest extends AbstractTestCase public function testNoCustomSeoTitleOnIncidentPage() { $incident = $this->createIncidentWithMeta([]); - $expectedTitle = sprintf('%s | %s', $incident->name, $this->appName); + $expectedTitle = sprintf('%s | %s', htmlspecialchars($incident->name), $this->appName); $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; From 0fe924d8681ed8517ea9c275ddf3a53f35c3ea20 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Wed, 4 Apr 2018 18:45:27 +0200 Subject: [PATCH 13/14] Typehint the removeEmptyValues function --- app/Bus/Handlers/Traits/StoresMeta.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php index b308c657..e5272ba5 100644 --- a/app/Bus/Handlers/Traits/StoresMeta.php +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -59,7 +59,7 @@ trait StoresMeta /** * Determine if a Value is empty. * - * @param $values + * @param mixed $values * * @return array|mixed */ From 33e5c34f551cbde69a9eedff37145bb2506e5081 Mon Sep 17 00:00:00 2001 From: Nico Stapelbroek Date: Mon, 2 Jul 2018 11:29:22 +0200 Subject: [PATCH 14/14] Fix metaseo tests after updating to Laravel 5.6 --- resources/views/layout/master.blade.php | 2 +- tests/Functional/Incident/MetaSeoTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/resources/views/layout/master.blade.php b/resources/views/layout/master.blade.php index d94267e6..c63ff42e 100644 --- a/resources/views/layout/master.blade.php +++ b/resources/views/layout/master.blade.php @@ -18,7 +18,7 @@ - + diff --git a/tests/Functional/Incident/MetaSeoTest.php b/tests/Functional/Incident/MetaSeoTest.php index c1ea4761..f61742b4 100644 --- a/tests/Functional/Incident/MetaSeoTest.php +++ b/tests/Functional/Incident/MetaSeoTest.php @@ -71,7 +71,7 @@ class MetaSeoTest extends AbstractTestCase $expectedDescription = htmlspecialchars($this->fakerFactory->sentence); $incident = $this->createIncidentWithMeta(['seo' => ['description' => $expectedDescription]]); - $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + $page = $this->get(sprintf('/incidents/%d', $incident->id)); $this->assertContains( sprintf('', $expectedDescription), @@ -92,7 +92,7 @@ class MetaSeoTest extends AbstractTestCase $title = htmlspecialchars($this->fakerFactory->title); $incident = $this->createIncidentWithMeta(['seo' => ['title' => $title]]); - $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + $page = $this->get(sprintf('/incidents/%d', $incident->id)); $this->assertContains( sprintf('', $title, $this->appName), @@ -120,7 +120,7 @@ class MetaSeoTest extends AbstractTestCase $presenter->occurred_at_formatted ); - $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + $page = $this->get(sprintf('/incidents/%d', $incident->id)); $this->assertContains( sprintf('', $expectedDescription), @@ -141,7 +141,7 @@ class MetaSeoTest extends AbstractTestCase $incident = $this->createIncidentWithMeta([]); $expectedTitle = sprintf('%s | %s', htmlspecialchars($incident->name), $this->appName); - $page = $this->get(sprintf('/incidents/%d', $incident->id))->response; + $page = $this->get(sprintf('/incidents/%d', $incident->id)); $this->assertContains( sprintf('', $expectedTitle),