Move all audit logging in templates #1709
Conversation
elinol
force-pushed
the
move-auditlogging-to-templates
branch
from
86a718e to
751fa5b
Compare
| |> Multi.run(:audit_source, fn _, _ -> | ||
| AuditLogs.audit(user, source_product, description) | ||
| |> Multi.run(:audit, fn _, _ -> | ||
| Templates.audit_device_moved(user, device, product, source_product) |
There was a problem hiding this comment.
I'm not sure this approach is the best, it gets kind of nested with a Repo.transaction inside Templates.audit_device_moved, but at least the audits are moved to a centralised place.
There was a problem hiding this comment.
Because all of this logic is encapsulated in Device.move/3 I'd argue that keeping the original implementation is fine. Centralizing logic is important when it's reused, but a one-off call, especially when all three audit logs use the same description, feels like an appropriate exception.
| @@ -1091,6 +1070,7 @@ defmodule NervesHub.Devices do | |||
| {:ok, updated} | |||
|
|
|||
| err -> | |||
| Logger.warning("Could not move device: #{inspect(err)}") | |||
There was a problem hiding this comment.
will this be logged to sentry as well? @joshk
elinol
force-pushed
the
move-auditlogging-to-templates
branch
from
751fa5b to
ca603f7
Compare
|
|
||
| require Logger | ||
|
|
||
| ### RESOURCE: DEVICE |
There was a problem hiding this comment.
This module will continue to grow over time, what do you think about splitting this out into submodules organized by resource? NervesHub.AuditLogs.DeviceTemplates, NervesHub.AuditLogs.FirmwareTemplates etc.
| |> Multi.run(:audit_source, fn _, _ -> | ||
| AuditLogs.audit(user, source_product, description) | ||
| |> Multi.run(:audit, fn _, _ -> | ||
| Templates.audit_device_moved(user, device, product, source_product) |
There was a problem hiding this comment.
Because all of this logic is encapsulated in Device.move/3 I'd argue that keeping the original implementation is fine. Centralizing logic is important when it's reused, but a one-off call, especially when all three audit logs use the same description, feels like an appropriate exception.
Moves all audit logging to the template module.
One exception is in the function
Devices.update_device_with_audit/4where the audit log description is passed via the updating action (liketag_devicefor example). I don't know if it's worth it adding this to templates, since the templates themselves (the description) are defined outside the templates module. Any input on this is welcomed!