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

Site wide audit log better formatting when updates object is to be displayed #4315

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Aug 2, 2024

closes #4071

Description

As discussed in the issue, there are several places where audit log can be a bit too verbose. Rather than handling idividual cases it was suggested that details should be hidden by default and a "better default formatter" should be employed.

From this, the following has been implemented:

  • Removes individual use of AuditEntryUpdates component and places it at the end
    • New autit log entries will no longer have to explicitly include this
  • By default (unless old/new are BOOLS or explicitly a permitted "named" key (e.g. name, type, enabled, version) the OLD and NEW values are NOT shown.
    • This inhibits large values from things like description fields being rendered
    • This prevents env values being show
  • There is scope for adjust the logic around what is NOT shown in the AuditEntryUpdates component as it now receives the full audit entry (e.g. there are now possibilities of inspecting the audit event name and explicitly hiding update details.)

Additionally, a monospaced font has been set and whitespace-pre on the details for updates AND errors has been further improve readability.

I have been through a whole bunch of audit log entries with updates and this PR provides a good balance of detail and readability. Future iterations can expand or reduce further based on explicit criteria.

Screenshots of before and after

As I cannot expect the reviewer to perform every action and check formatting I will update this PR with screenshots before witching out of draft.

image

image

image

image

image

Errors before and after

image

Overview before & after

image

image

Related Issue(s)

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

Copy link

codecov bot commented Aug 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.19%. Comparing base (19f6fe1) to head (89cd0ff).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4315   +/-   ##
=======================================
  Coverage   78.19%   78.19%           
=======================================
  Files         292      292           
  Lines       13440    13440           
  Branches     3007     3007           
=======================================
  Hits        10510    10510           
  Misses       2930     2930           
Flag Coverage Δ
backend 78.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Steve-Mcl Steve-Mcl marked this pull request as ready for review August 2, 2024 14:15
@Steve-Mcl Steve-Mcl requested review from joepavitt and removed request for hardillb August 2, 2024 14:17
Copy link
Contributor

@hardillb hardillb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick local test looks good.

@joepavitt joepavitt merged commit 30f93ff into main Aug 6, 2024
16 checks passed
@joepavitt joepavitt deleted the 4071-audit-log-too-verbose branch August 6, 2024 08:31
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.

Large entries in audit log
3 participants