-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add incident column for when an incident occurred at #2212
Changes from 2 commits
f39970a
f5fd88e
0d73145
d8eb2ec
a6e4c2a
eacb1fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of Cachet. | ||
* | ||
* (c) Alt Three Services Limited | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace CachetHQ\Cachet\Bus\Exceptions\Incident; | ||
|
||
use CachetHQ\Cachet\Bus\Exceptions\ExceptionInterface; | ||
use Exception; | ||
|
||
/** | ||
* This is the invalid incident timestamp exception. | ||
* | ||
* @author James Brooks <james@alt-three.com> | ||
*/ | ||
class InvalidIncidentTimestampException extends Exception implements ExceptionInterface | ||
{ | ||
// | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,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\Dates\DateFactory; | ||
use CachetHQ\Cachet\Models\Component; | ||
use CachetHQ\Cachet\Models\Incident; | ||
|
@@ -61,18 +62,20 @@ public function handle(UpdateIncidentCommand $command) | |
} | ||
|
||
$incident = $command->incident; | ||
$incident->update($this->filter($command)); | ||
$incident->fill($this->filter($command)); | ||
|
||
// The incident occurred at a different time. | ||
if ($command->incident_date) { | ||
$incidentDate = $this->dates->create('d/m/Y H:i', $command->incident_date); | ||
|
||
$incident->update([ | ||
'created_at' => $incidentDate, | ||
'updated_at' => $incidentDate, | ||
]); | ||
if ($occurredAt = $command->occurredAt) { | ||
if ($date = $this->dates->create('Y-m-d H:i', $occurredAt)) { | ||
$incident->fill(['occurred_at' => $date]); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard > There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, the exception doesn't do anything here, because |
||
throw new InvalidIncidentTimestampException("Unable to pass timestamp {$occurredAt}"); | ||
} | ||
} | ||
|
||
// Rather than making lots of updates, just fill and save. | ||
$incident->save(); | ||
|
||
// Update the component. | ||
if ($component = Component::find($command->component_id)) { | ||
dispatch(new UpdateComponentCommand( | ||
|
@@ -138,7 +141,7 @@ protected function parseTemplate(IncidentTemplate $template, UpdateIncidentComma | |
'visible' => $command->visible, | ||
'notify' => $command->notify, | ||
'stickied' => $command->stickied, | ||
'incident_date' => $command->incident_date, | ||
'occurredAt' => $command->occurredAt, | ||
'component' => Component::find($command->component_id) ?: null, | ||
'component_status' => $command->component_status, | ||
], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,11 +127,11 @@ public function showDashboard() | |
*/ | ||
protected function getIncidents() | ||
{ | ||
$allIncidents = Incident::notScheduled()->whereBetween('created_at', [ | ||
$allIncidents = Incident::notScheduled()->whereBetween('occurred_at', [ | ||
$this->startDate->copy()->subDays(30)->format('Y-m-d').' 00:00:00', | ||
$this->startDate->format('Y-m-d').' 23:59:59', | ||
])->orderBy('created_at', 'desc')->get()->groupBy(function (Incident $incident) { | ||
return (new Date($incident->created_at)) | ||
])->orderBy('occurred_at', 'desc')->get()->groupBy(function (Incident $incident) { | ||
return (new Date($incident->occurred_at)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we not use the presenter here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking this when I was looking at it this morning. |
||
->setTimezone($this->dateTimeZone)->toDateString(); | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,11 @@ | |
use Illuminate\Support\Facades\Config; | ||
use Illuminate\Support\Str; | ||
|
||
/** | ||
* This is the feed controller. | ||
* | ||
* @author James Brooks <james@alt-three.com> | ||
*/ | ||
class FeedController extends Controller | ||
{ | ||
/** | ||
|
@@ -80,12 +85,12 @@ private function feedAction(ComponentGroup &$group, $isRss) | |
{ | ||
if ($group->exists) { | ||
$group->components->map(function ($component) use ($isRss) { | ||
$component->incidents()->visible()->orderBy('created_at', 'desc')->get()->map(function ($incident) use ($isRss) { | ||
$component->incidents()->visible()->orderBy('occurred_at', 'desc')->get()->map(function ($incident) use ($isRss) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Long line is long 😛 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should add a better scope for this. |
||
$this->feedAddItem($incident, $isRss); | ||
}); | ||
}); | ||
} else { | ||
Incident::visible()->orderBy('created_at', 'desc')->get()->map(function ($incident) use ($isRss) { | ||
Incident::visible()->orderBy('occurred_at', 'desc')->get()->map(function ($incident) use ($isRss) { | ||
$this->feedAddItem($incident, $isRss); | ||
}); | ||
} | ||
|
@@ -105,7 +110,7 @@ private function feedAddItem(Incident $incident, $isRss) | |
$incident->name, | ||
Config::get('setting.app_name'), | ||
Str::canonicalize(cachet_route('incident', [$incident->id])), | ||
$isRss ? $incident->created_at->toRssString() : $incident->created_at->toAtomString(), | ||
$isRss ? $incident->occurred_at->toRssString() : $incident->occurred_at->toAtomString(), | ||
$isRss ? $incident->message : Markdown::convertToHtml($incident->message) | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,11 +82,11 @@ public function showIndex() | |
|
||
$incidentVisibility = Auth::check() ? 0 : 1; | ||
|
||
$allIncidents = Incident::notScheduled()->where('visible', '>=', $incidentVisibility)->whereBetween('created_at', [ | ||
$allIncidents = Incident::notScheduled()->where('visible', '>=', $incidentVisibility)->whereBetween('occurred_at', [ | ||
$startDate->copy()->subDays($daysToShow)->format('Y-m-d').' 00:00:00', | ||
$startDate->format('Y-m-d').' 23:59:59', | ||
])->orderBy('scheduled_at', 'desc')->orderBy('created_at', 'desc')->get()->load('updates')->groupBy(function (Incident $incident) { | ||
return app(DateFactory::class)->make($incident->is_scheduled ? $incident->scheduled_at : $incident->created_at)->toDateString(); | ||
])->orderBy('scheduled_at', 'desc')->orderBy('occurred_at', 'desc')->get()->groupBy(function (Incident $incident) { | ||
return app(DateFactory::class)->make($incident->is_scheduled ? $incident->scheduled_at : $incident->occurred_at)->toDateString(); | ||
}); | ||
|
||
// Add in days that have no incidents | ||
|
@@ -109,7 +109,7 @@ public function showIndex() | |
->withDaysToShow($daysToShow) | ||
->withAllIncidents($allIncidents) | ||
->withCanPageForward((bool) $today->gt($startDate)) | ||
->withCanPageBackward(Incident::notScheduled()->where('created_at', '<', $startDate->format('Y-m-d'))->count() > 0) | ||
->withCanPageBackward(Incident::notScheduled()->where('occurred_at', '<', $startDate->format('Y-m-d'))->count() > 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we even need to format There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dunno. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because |
||
->withPreviousDate($startDate->copy()->subDays($daysToShow)->toDateString()) | ||
->withNextDate($startDate->copy()->addDays($daysToShow)->toDateString()); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reckon we should move to
camelCase
but I also reckon it should be done in it's own PR rather than just leaving random bits here and there.