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

Add audit log for debugging #302

Merged
merged 5 commits into from
Sep 21, 2018
Merged

Add audit log for debugging #302

merged 5 commits into from
Sep 21, 2018

Conversation

tijmenb
Copy link
Contributor

@tijmenb tijmenb commented Sep 19, 2018

This adds the papertrail gem to the app and sets it up. Paperclip will keep track of all of the changes in the models (Document, Image, User).

The reason for this is to keep an audit log of all of the changes. While we're busy building this application, we're very likely to run into a situation where a document has entered a particular state, and we're not sure how this happened. Having a full log of changes will help us a lot in debugging. And with time being linear, we won't be able to add this feature retroactively if something goes wrong.

It's worth noting that this is a distinct & separate use case from the history timeline (#270), which is a feature designed for end-users. The two shouldn't be coupled.

Downside of having this:

  • It slow all of the writes down. When a document is edited, there will be multiple versions created because of the state changes.
  • The versions table is likely to grow a lot. We'll need to make sure that we clean up if the table becomes too large. There's a ticket on the planning board for this: https://trello.com/c/XMrIPU88. This is also the reason that we need to keep the history feature and this audit trail separate, because we should be able to truncate the table without losing end-user functionality.

This PR also adds the entries to a document "debug" page.

screenshot_2018-09-19 internal metadata for hello - gov uk

@benthorner benthorner temporarily deployed to content-publisher-revie-pr-302 September 19, 2018 15:01 Inactive
@benthorner benthorner temporarily deployed to content-publisher-revie-pr-302 September 19, 2018 15:08 Inactive
@kevindew
Copy link
Member

Just to understand the context, Is this intended entirely for debugging purposes or are they any ideas about it's usage with regards to rolling back?

It seems a cool idea and I can see it being useful in the early days. I have a concern that if we're not looking at it often it could fill up a lot of data.

What sort of future do you see for this with the context of storing revisions?

@tijmenb
Copy link
Contributor Author

tijmenb commented Sep 19, 2018

@kevindew argh, tried to communicate that in the PR description but it wasn't clear enough. Papertrail should only be used as an audit log, and absolutely nothing else. This will make it easy for us to throw away the data and have a functioning application.

@benthorner
Copy link
Contributor

Can you add comment above the Papertrail lines to indicate what it's being used for. I reckon when we eventually have two kinds of history it'll confuse people unless we clearly signpost it for them.

@benthorner benthorner temporarily deployed to content-publisher-revie-pr-302 September 19, 2018 16:31 Inactive
@benthorner benthorner temporarily deployed to content-publisher-revie-pr-302 September 19, 2018 16:36 Inactive
@benthorner benthorner temporarily deployed to content-publisher-revie-pr-302 September 19, 2018 16:38 Inactive
@benthorner benthorner temporarily deployed to content-publisher-revie-pr-302 September 20, 2018 14:21 Inactive
This adds the papertrail gem to the app and sets it up. Paperclip will
keep track of all of the changes in the models (Document, Image, User).

The reason for this is to keep an audit log of all of the changes.
While we're busy building this application, we're very likely to run
into a situation where a document has entered a particular state, and
we're not sure how this happened. Having a full log of changes will
help us a lot in debugging. And with time being linear, we won't be
able to add this feature retroactively if something goes wrong.

It's worth noting that this is a distinct & separate use case from the
history timeline
(#270), which is a
feature designed for end-users. The two shouldn't be coupled.

Downside of having this:

- It slow all of the writes down. When a document is edited, there will
be multiple versions created because of the state changes.
- The versions table is likely to grow a lot. We'll need to make sure
that we clean up if the table becomes too large. There's a ticket on
the planning board for this: https://trello.com/c/XMrIPU88. This is
also the reason that we need to keep the history feature and this audit
trail separate, because we should be able to truncate the table without
losing end-user functionality.
This makes it easier to add things to it.
This displays the audit log for internal users.

- We have to cache the users from the timeline to prevent a N+1 query,
since the `whodunnit` isn't a normal Rails relationship
- Because `updated_at` is always the same as the event's `created_at`,
we won't have to display it.
This test inadvertently passed because the internal metadata component
showed "major", even if the page itself does not.
@tijmenb tijmenb merged commit 1414857 into master Sep 21, 2018
@tijmenb tijmenb deleted the add-papertrail branch September 21, 2018 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants