Add incident column for when an incident occurred at #2212

jbrooksuk · 2016-10-27T06:35:17Z

This does change the API requests, as we're now looking for a field called occurred_at instead of incident_date, or whatever.

[ci skip] [skip ci]

ConnorVG · 2016-10-27T08:42:57Z

app/Bus/Commands/Incident/ReportIncidentCommand.php

@@ -110,7 +110,7 @@
        'component_status' => 'nullable|required_with:component_id|int|min:0|max:4',
        'notify'           => 'nullable|bool',
        'stickied'         => 'required|bool',
-        'incident_date'    => 'nullable|string',
+        'occurredAt'       => 'nullable|string',


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.

ConnorVG · 2016-10-27T08:43:48Z

app/Bus/Handlers/Commands/Incident/UpdateIncidentCommandHandler.php

+        if ($occurredAt = $command->occurredAt) {
+            if ($date = $this->dates->create('Y-m-d H:i', $occurredAt)) {
+                $incident->fill(['occurred_at' => $date]);
+            } else {


Guard > else - looks messy imo

Also, the exception doesn't do anything here, because create will throw, so we need to catch.

ConnorVG · 2016-10-27T08:45:46Z

app/Http/Controllers/Dashboard/DashboardController.php

-        ])->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))


Could we not use the presenter here?

I was thinking this when I was looking at it this morning.

ConnorVG · 2016-10-27T08:46:28Z

app/Http/Controllers/FeedController.php

@@ -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) {


Long line is long 😛

I should add a better scope for this.

ConnorVG · 2016-10-27T08:47:40Z

app/Http/Controllers/StatusPageController.php

@@ -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)


Do we even need to format $startDate?

Yes, because occurred_at might contain a time, but we're only interested in whole days.

ConnorVG · 2016-10-27T08:48:26Z

app/Presenters/IncidentPresenter.php

+     */
+    public function occurred_at()
+    {
+        return app(DateFactory::class)->make($this->wrappedObject->occurred_at)->toDateTimeString();


Off topic but is there any reason we don't just inject the DateFactory?

ConnorVG · 2016-10-27T08:48:52Z

app/Presenters/IncidentPresenter.php

    {
-        return ucfirst(app(DateFactory::class)->make($this->wrappedObject->created_at)->format(Config::get('setting.incident_date_format', 'l jS F Y H:i:s')));
+        return ucfirst(app(DateFactory::class)->make($this->wrappedObject->occurred_at)->format(Config::get('setting.incident_date_format', 'l jS F Y H:i:s')));


This seems overly complex / long. QoL goes down.

ConnorVG · 2016-10-27T08:49:00Z

app/Presenters/IncidentPresenter.php

+     */
+    public function created_at_formatted()
+    {
+        return ucfirst(app(DateFactory::class)->make($this->wrappedObject->created_at)->format(Config::get('setting.incident_date_format', 'l jS F Y H:i:s')));


Same as previous.

ConnorVG · 2016-10-27T08:49:58Z

app/helpers.php

+     */
+    function datetime_to_moment($format)
+    {
+        $replacements = [


This is just bizarre. I see it's point tho 😆

I didn't end up using this, but I like it, so I'm keeping it because I probably will use this at some point.

ConnorVG · 2016-10-27T08:51:52Z

resources/views/single-incident.blade.php

@@ -7,7 +7,7 @@
 @stop

 @section('content')
-<h1>{{ $incident->name }} <small>{{ formatted_date($incident->created_at) }}</small></h1>
+<h1>{{ $incident->name }} <small>{{ formatted_date($incident->occurred_at) }}</small></h1>


Why are we not using the presenter here?

I noticed this too, dunno why.

[ci skip] [skip ci]

jbrooksuk · 2016-10-29T16:25:26Z

Let's merge and see what happens 👍 there might be an edge-case I've missed.

jbrooksuk and others added 2 commits October 27, 2016 07:34

Add incident column for when an incident occurred at. Closes #2208

f39970a

Applied fixes from StyleCI

f5fd88e

[ci skip] [skip ci]

jbrooksuk added the Refactor label Oct 27, 2016

jbrooksuk added this to the V2.4.0 milestone Oct 27, 2016

jbrooksuk self-assigned this Oct 27, 2016

jbrooksuk mentioned this pull request Oct 27, 2016

update_at value with dashboard #2208

Closed

ConnorVG reviewed Oct 27, 2016

View reviewed changes

jbrooksuk and others added 4 commits October 29, 2016 16:53

Use snake case still for now

0d73145

Clean up the incident presenter

d8eb2ec

Use the presenter on single pages

a6e4c2a

Applied fixes from StyleCI

eacb1fd

[ci skip] [skip ci]

jbrooksuk merged commit a0f2d66 into 2.4 Oct 29, 2016

jbrooksuk deleted the reported-at-dat branch October 29, 2016 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add incident column for when an incident occurred at #2212

Add incident column for when an incident occurred at #2212

jbrooksuk commented Oct 27, 2016 •

edited

Loading

ConnorVG Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

jbrooksuk Oct 29, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

ConnorVG Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

ConnorVG Oct 27, 2016

jbrooksuk Oct 27, 2016

jbrooksuk commented Oct 29, 2016

Add incident column for when an incident occurred at #2212

Add incident column for when an incident occurred at #2212

Conversation

jbrooksuk commented Oct 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrooksuk commented Oct 29, 2016

jbrooksuk commented Oct 27, 2016 •

edited

Loading