-
Notifications
You must be signed in to change notification settings - Fork 914
refactor: Make the audit log looks like a timeline #4765
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
Conversation
const styles = useStyles() | ||
// We only want the message related to the date since the time is displayed | ||
// inside of the build row | ||
const displayDate = formatRelative(date, new Date()).split("at")[0] |
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.
is this something dayjs can't do?
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.
No, it does not 😞 . I mean, I tried to find it in their docs but I didn't find anything.
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.
Ok!
@@ -37,6 +37,27 @@ const presetFilters = [ | |||
{ query: "resource_type:user action:delete", name: "Deleted users" }, | |||
] | |||
|
|||
const groupAuditLogsByDate = (auditLogs?: AuditLog[]) => { |
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.
if you're willing to use lodash I think this would just be
_.groupBy(auditLogs, (log) => new Date(log.time).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.
Yes, I think we could talk about adding lodash since we can import specific functions only instead of the entire package.
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 we already have it because it comes bundled with something else
Before:

After:
