Skip to content
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

update_at value with dashboard #2208

Closed
bleuchtang opened this issue Oct 24, 2016 · 10 comments
Closed

update_at value with dashboard #2208

bleuchtang opened this issue Oct 24, 2016 · 10 comments
Assignees
Milestone

Comments

@bleuchtang
Copy link

When I use the dashboard, created_at and updated_at are always equals.

My the typical workflow is:
Create an incident with the dashboard, and query the database:

mysql> select * from incidents where id=2\G
*************************** 1. row ***************************
id: 2
component_id: 0
name: test_update_at
status: 1
visible: 1
message: Test update_at
scheduled_at: NULL
created_at: 2016-10-24 14:09:15
updated_at: 2016-10-24 14:09:15
deleted_at: NULL
1 row in set (0.00 sec)

Then I update the incident with the API (here I change the status):
curl -X PUT -H "Content-Type: application/json" -H "X-Cachet-Token: xxxx" http://status.com/api/v1/incidents/2 -d '{"status":2}'

Now database have different values for created_at and updated_at:
mysql> select * from incidents where id=2\G
*************************** 1. row ***************************
id: 2
component_id: 0
name: test_update_at
status: 2
visible: 1
message: Test update_at
scheduled_at: NULL
created_at: 2016-10-24 14:09:15
updated_at: 2016-10-24 14:14:08
deleted_at: NULL
1 row in set (0.00 sec)

Now if I edit the incident again with the dashboard an query the database:

mysql> select * from incidents where id=2\G
*************************** 1. row ***************************
id: 2
component_id: 0
name: test_update_at
status: 2
visible: 1
message: Test update_at
scheduled_at: NULL
created_at: 2016-10-24 14:09:00
updated_at: 2016-10-24 14:09:00
deleted_at: NULL
1 row in set (0.01 sec)

It seems that the dashboard use the same value for created_at and updated_at.

@ConnorVG
Copy link
Contributor

@jbrooksuk
Copy link
Member

Yeah, this is something that seems to be recurring...

I think the best idea here is to introduce a new field such as reported_at which is the canonical data in which an incident occurred not the created_at date.

created_at would then become the date which the Incident was created, and updated_at will always update when the incident is changed.

Thoughts?

@jbrooksuk jbrooksuk added this to the V2.4.0 milestone Oct 24, 2016
@jbrooksuk jbrooksuk self-assigned this Oct 24, 2016
@jbrooksuk
Copy link
Member

Maybe not reported_at actually, that sounds like a synonym for created_at.. Hmm.

@SunilKumar3
Copy link

By the detailed given in the problem statement and by debugging the related update code. we are explicitly updating 'created_at' and 'updated_at' which is not required. By simply avoiding explicitly we can fix this issue.

Please share me your thoughts. Thank you.

@bleuchtang
Copy link
Author

For me the correct behavior is; created_at set at the creation of the incident, updated_at set every time we update the incident (what the API do).

Anyway, I think it's a good thing that the dashboard and the API give exactly the same result.

@jbrooksuk
Copy link
Member

jbrooksuk commented Oct 26, 2016

Okay, to reconfirm what I'm planning on changing here...

  • created_at will only be set at the time the incident is created, whether that's through the dashboard or the API. It'll never change once it's set.
  • updated_at this will be modified any time an incident is modified in some way.
  • And then I'll add a new field, which I originally called reported_at but it'll now be occurred_at which is either going to be the same as the created_at field, or since you can backport incidents that happened in the past, it'll be whatever the user supplies as the time of the incident. This is the timestamp that'll use to order incidents by in the API and the timeline.

@SunilKumar3
Copy link

According to my Analysis and test , I commented updated_at in update method controller for my application in Laravel which is almost similar to above code. Framework will handle the updated_at column. so , by simply avoiding explicitly created_at and updated_at column in code. we can fix this issue and I've done it.

https://github.com/CachetHQ/Cachet/blob/2.4/app/Bus/Handlers/Commands/Incident/UpdateIncidentCommandHandler.php#L72

/* $incidentDate = $emp->created_at;

     $emp->update([
            'created_at' => $incidentDate,
            'updated_at' => $incidentDate
        ]); */

@jbrooksuk
Copy link
Member

@SunilKumar3 that doesn't work, if somebody wants to change when an incident happens, that's why I've said about adding a new column.

Please see my PR #2212

@SunilKumar3
Copy link

@jbrooksuk Your idea of having occurred_at column to capture actual incident date is very much required. This we need to add.

I have made code changes for this, the changes are as follows : added a new column 'occurred_at' to incidents table, made changes for updating occurred_at in updateIncidentCommandHandler.php and also removed 'created_at' , 'updated_at' columns update because this is automatically handled by Framework.

so, these changes will capture occurred_at and allows framework to handle created_at and updated_at.

@jbrooksuk
Copy link
Member

@SunilKumar3 so you copied the changes from what I did in #2212 then? Does it work for you? It's a bigger job than it looks though, because we have to account for feeds and the status timeline etc.

jbrooksuk added a commit that referenced this issue Oct 29, 2016
Add incident column for when an incident occurred at. Closes #2208
[ci skip] [skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants