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 36036e6d..1328b37c 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 = [], array $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 6bf3fa6b..fe9cec9d 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\ArrayLoader as Twig_Loader_Array; */ class CreateIncidentCommandHandler { + use StoresMeta; + /** * The authentication guard instance. * @@ -104,14 +107,7 @@ class CreateIncidentCommandHandler // Store any meta? if ($meta = $command->meta) { - foreach ($meta as $key => $value) { - 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 3e5f0ba3..dff31ccf 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\ArrayLoader as 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)) { execute(new UpdateComponentCommand( diff --git a/app/Bus/Handlers/Traits/StoresMeta.php b/app/Bus/Handlers/Traits/StoresMeta.php new file mode 100644 index 00000000..e5272ba5 --- /dev/null +++ b/app/Bus/Handlers/Traits/StoresMeta.php @@ -0,0 +1,81 @@ + $value) { + $modelInstance = call_user_func( + [$metaModel, 'firstOrNew'], + [ + 'key' => $key, + 'meta_type' => $metaType, + 'meta_id' => $metaId, + ] + ); + + $value = $this->removeEmptyValues($value); + if (!empty($value)) { + $modelInstance->setAttribute('value', $value); + $modelInstance->save(); + continue; + } + + // The value is empty, remove the row + if ($modelInstance->exists) { + $modelInstance->delete(); + } + } + } + + /** + * Determine if a Value is empty. + * + * @param mixed $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/app/Http/Controllers/Dashboard/IncidentController.php b/app/Http/Controllers/Dashboard/IncidentController.php index f3f0dd9f..f5555aa0 100644 --- a/app/Http/Controllers/Dashboard/IncidentController.php +++ b/app/Http/Controllers/Dashboard/IncidentController.php @@ -128,7 +128,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') @@ -258,7 +259,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/lang/en/forms.php b/resources/lang/en/forms.php index 339ba0b8..5c5a41a9 100644 --- a/resources/lang/en/forms.php +++ b/resources/lang/en/forms.php @@ -229,6 +229,11 @@ return [ 'timezone' => 'Select Timezone', ], + 'seo' => [ + 'title' => 'SEO Title', + 'description' => 'SEO Description', + ], + // Buttons 'add' => 'Add', 'save' => 'Save', diff --git a/resources/views/dashboard/incidents/add.blade.php b/resources/views/dashboard/incidents/add.blade.php index 85a7222a..edc98744 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') }} + +
diff --git a/resources/views/dashboard/incidents/edit.blade.php b/resources/views/dashboard/incidents/edit.blade.php index 57e3a263..525a861b 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') }} + +
diff --git a/resources/views/layout/master.blade.php b/resources/views/layout/master.blade.php index c77e7259..70162e25 100644 --- a/resources/views/layout/master.blade.php +++ b/resources/views/layout/master.blade.php @@ -15,7 +15,7 @@ - + diff --git a/resources/views/single-incident.blade.php b/resources/views/single-incident.blade.php index 311bcff5..fe3e17ba 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.' | '.$siteTitle) +@section('title', array_get($incident->meta, 'seo.title', $incident->name).' | '.$siteTitle) -@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') 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 new file mode 100644 index 00000000..f61742b4 --- /dev/null +++ b/tests/Functional/Incident/MetaSeoTest.php @@ -0,0 +1,181 @@ + + * @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 + * @param string $dataName + */ + 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); + $this->app->config->set('setting.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() + { + $expectedDescription = htmlspecialchars($this->fakerFactory->sentence); + + $incident = $this->createIncidentWithMeta(['seo' => ['description' => $expectedDescription]]); + $page = $this->get(sprintf('/incidents/%d', $incident->id)); + + $this->assertContains( + sprintf('', $expectedDescription), + $page->content() + ); + $this->assertContains( + sprintf('', $expectedDescription), + $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 = htmlspecialchars($this->fakerFactory->title); + + $incident = $this->createIncidentWithMeta(['seo' => ['title' => $title]]); + $page = $this->get(sprintf('/incidents/%d', $incident->id)); + + $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', + htmlspecialchars($incident->name), + $presenter->occurred_at_formatted + ); + + $page = $this->get(sprintf('/incidents/%d', $incident->id)); + + $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', htmlspecialchars($incident->name), $this->appName); + + $page = $this->get(sprintf('/incidents/%d', $incident->id)); + + $this->assertContains( + sprintf('', $expectedTitle), + $page->content() + ); + $this->assertContains(sprintf('%s', $expectedTitle), $page->content()); + } + + /** + * @param array $meta + * + * @return Incident + */ + protected function createIncidentWithMeta(array $meta) + { + $this->signIn(); + $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(); + } +}