-
Notifications
You must be signed in to change notification settings - Fork 55
Update notifications with id and timestamp #170
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 notifications with id and timestamp #170
Conversation
@@ -18,6 +18,9 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function() | |||
}; | |||
|
|||
var addNotification = function (notification) { | |||
notification.id = notification.id || _.uniqueId('notification-'); | |||
// This format matches timestamp of API objects | |||
notification.timestamp = moment.parseZone(new Date()).utc().format(); |
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 think just
new Date().toISOString();
does what we want, then we don't need to add moment.js to common
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.
And it's simpler :)
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.
True. lets do that.
42844a8
to
bc2c28b
Compare
Just updated the filters as well to take the api object type (Normal, Warning). |
@@ -18,6 +18,10 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function() | |||
}; | |||
|
|||
var addNotification = function (notification) { | |||
notification.id = notification.id || _.uniqueId('notification-'); | |||
// This format matches timestamp of API objects | |||
//notification.timestamp = moment.parseZone(new Date()).utc().format(); |
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.
Remove commented out code
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.
got it
bc2c28b
to
284898a
Compare
Per this comment in PR 2001 in origin-web-console.
@spadgett @jeff-phillips-18