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

feature: audit logs #2703

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1167828
WIP
Paul-Bob Jan 10, 2024
8c4c54a
wip
Paul-Bob Jan 10, 2024
fc9720b
WIP
Paul-Bob Jan 13, 2024
04e7d7f
wip
Paul-Bob Jan 15, 2024
2f6dbc3
typo
Paul-Bob Jan 15, 2024
9826cdc
Merge branch 'main' into feature/audit_logs
Paul-Bob Jan 17, 2024
0070b19
WIP
Paul-Bob Jan 17, 2024
f64dfb8
WIP
Paul-Bob Jan 19, 2024
8d9248c
wip
Paul-Bob Jan 22, 2024
0771d63
Merge branch 'main' into feature/audit_logs
Paul-Bob Feb 15, 2024
2c9b896
wip
Paul-Bob Feb 16, 2024
361120a
wip
Paul-Bob Feb 16, 2024
0db29ba
Merge branch 'main' into feature/audit_logs
Paul-Bob Apr 2, 2024
4986af8
dont break if db do not exist
Paul-Bob Apr 2, 2024
16c3019
wip
Paul-Bob Apr 3, 2024
f88a17f
Merge branch 'main' into feature/audit_logs
Paul-Bob Apr 4, 2024
496f42d
rename package
Paul-Bob Apr 9, 2024
cdde642
Merge branch 'main' into feature/audit_logs
Paul-Bob Apr 11, 2024
da5f08f
Merge branch 'main' into feature/audit_logs
Paul-Bob May 13, 2024
58210f7
extract audit method and safe_call it
Paul-Bob May 13, 2024
53221df
register all activity
Paul-Bob May 14, 2024
70cd62c
Update app/components/avo/sidebar_component.html.erb
Paul-Bob May 14, 2024
f0ab0a2
Update lib/avo/configuration.rb
Paul-Bob May 14, 2024
f3c5e1c
Update lib/avo/configuration.rb
Paul-Bob May 14, 2024
8840724
Merge branch 'main' into feature/audit_logs
Paul-Bob May 14, 2024
5e614a4
Merge branch 'main' into feature/audit_logs
Paul-Bob Jun 20, 2024
bd07682
rename to avo-audit_logging
Paul-Bob Jun 20, 2024
dc2f288
feature: log attach and detach
Paul-Bob Jun 21, 2024
019ca76
Merge branch 'main' into feature/audit_logs
Paul-Bob Sep 11, 2024
6745d11
Merge branch 'main' into feature/audit_logs
Paul-Bob Sep 11, 2024
cf57fe1
Merge branch 'main' into feature/audit_logs
Paul-Bob Sep 13, 2024
6fc5c46
unbundle audits from enterprise license
Paul-Bob Sep 13, 2024
80b049a
implement gem configuration
Paul-Bob Sep 13, 2024
9595046
rm unused config attribute
Paul-Bob Sep 13, 2024
02d3595
clear template
Paul-Bob Sep 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/components/avo/sidebar_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
<% end %>
<% end %>


Paul-Bob marked this conversation as resolved.
Show resolved Hide resolved
<%= render partial: "/avo/partials/sidebar_extra" %>
</div>
</div>
Expand Down
17 changes: 14 additions & 3 deletions app/controllers/avo/actions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,23 @@ def build_background_url
def handle
resource_ids = action_params[:fields][:avo_resource_ids].split(",")

query = decrypted_query || (resource_ids.any? ? @resource.find_record(resource_ids, params: params) : [])
fields = action_params[:fields].except(:avo_resource_ids, :avo_selected_query)

safe_call :audit,
activity_class: @action.class,
payload: {
fields: fields,
resource: resource,
},
action: __method__,
records: query

performed_action = @action.handle_action(
fields: action_params[:fields].except(:avo_resource_ids, :avo_selected_query),
fields: fields,
current_user: _current_user,
resource: @resource,
query: decrypted_query ||
(resource_ids.any? ? @resource.find_record(resource_ids, params: params) : [])
query: query
)

@response = performed_action.response
Expand Down
1 change: 1 addition & 0 deletions app/controllers/avo/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ApplicationController < ::ActionController::Base
before_action :set_view
before_action :set_sidebar_open
before_action :set_stylesheet_assets_path
before_action :set_paper_trail_whodunnit, if: -> { defined? PaperTrail }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we add this ourselves in the audit logging gem?


rescue_from Avo::NotAuthorizedError, with: :render_unauthorized
rescue_from ActiveRecord::RecordInvalid, with: :exception_logger
Expand Down
16 changes: 12 additions & 4 deletions app/controllers/avo/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class BaseController < ApplicationController
before_action :set_pagy_locale, only: :index

def index
safe_call :audit, activity_class: @resource.class, payload: params, action: __method__

@page_title = @resource.plural_name.humanize
add_breadcrumb @resource.plural_name.humanize

Expand Down Expand Up @@ -58,6 +60,8 @@ def index
end

def show
safe_call :audit, activity_class: @resource.class, payload: params, action: __method__, records: @record

@resource.hydrate(record: @record, view: :show, user: _current_user, params: params).detect_fields

set_actions
Expand Down Expand Up @@ -108,6 +112,8 @@ def new
add_breadcrumb @resource.plural_name.humanize, resources_path(resource: @resource)
add_breadcrumb t("avo.new").humanize

safe_call :audit, activity_class: @resource.class, payload: params, action: __method__

set_component_for __method__, fallback_view: :edit
end

Expand Down Expand Up @@ -152,6 +158,7 @@ def create

set_component_for :edit

safe_call :audit, activity_class: @resource.class, payload: params, action: __method__, records: @record
if saved
create_success_action
else
Expand All @@ -160,12 +167,16 @@ def create
end

def edit
safe_call :audit, activity_class: @resource.class, payload: params, action: __method__, records: @record

set_actions

set_component_for __method__
end

def update
safe_call :audit, activity_class: @resource.class, payload: params, action: __method__, records: @record

# record gets instantiated and filled in the fill_record method
saved = save_record
@resource = @resource.hydrate(record: @record, view: :edit, user: _current_user)
Expand All @@ -181,6 +192,7 @@ def update
end

def destroy
safe_call :audit, activity_class: @resource.class, payload: params, action: __method__, records: @record
if destroy_model
destroy_success_action
else
Expand Down Expand Up @@ -549,10 +561,6 @@ def set_pagy_locale
@pagy_locale = locale.to_s || Avo.configuration.locale || "en"
end

def safe_call(method)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we need this? And shouldn't it take more args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you missed this change https://github.com/avo-hq/avo/pull/2703/files#diff-27ec89c6677b158fe3a800b268c9c196b5409f439315240504626f4c8bf81bc7R140

The method was extracted to be accessible in more places

send(method) if respond_to?(method, true)
end

def pagy_query
@query
end
Expand Down
4 changes: 4 additions & 0 deletions app/helpers/avo/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ def frame_id(resource)
["frame", resource.model_name.singular, resource.record.id].compact.join("-")
end

def safe_call(method, **args)
send(method, **args) if respond_to?(method, true)
end

def chart_color(index)
Avo.configuration.branding.chart_colors[index % Avo.configuration.branding.chart_colors.length]
end
Expand Down
22 changes: 21 additions & 1 deletion lib/avo/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class Configuration
attr_writer :root_path
attr_writer :cache_store
attr_writer :logger
attr_writer :audit
attr_writer :turbo
attr_writer :pagination
attr_accessor :timezone
Expand Down Expand Up @@ -102,6 +103,7 @@ def initialize
@mount_avo_engines = true
@cache_store = computed_cache_store
@logger = default_logger
@audit = false
@turbo = default_turbo
@default_url_options = []
@pagination = {}
Expand Down Expand Up @@ -162,7 +164,9 @@ def license=(value)
def license
gems = Gem::Specification.map {|gem| gem.name}

@license ||= if gems.include?("avo-advanced")
@license ||= if gems.include?("avo-audits")
"enterprise"
Copy link

Choose a reason for hiding this comment

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

Why? 😢

Copy link

Choose a reason for hiding this comment

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

Can you consider moving the audit feature to the advanced plan?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @yorsant.
Apologies for the late reply.

The audit logging feature is, generally speaking, an enterprise feature.

As much as we want to give features away or make them cheaper, we can't do that. It takes a lot of time and effort to build Avo.
We love the self-serve product (Pro, Advanced) but the reality is that we can't survive and improve our business without having some big customers. We've heard from customers that they saved tens and even hundreds of thousands of dollars by using Avo and that they would like to pay more.
If we don't have any Enterprise features, we can't sell Enterprise subscriptions.

This all comes back to us being able to build a robust business and being able to ship more value through our products.

Thanks for asking the question.

Copy link

Choose a reason for hiding this comment

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

FWIW you can build audit log yourself using papertrail and avo resources.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This feature will not be bundled in a gem, so let's not force the license to enterprise.

elsif gems.include?("avo-advanced")
"advanced"
elsif gems.include?("avo-pro")
"pro"
Expand Down Expand Up @@ -221,6 +225,22 @@ def default_logger
}
end

def database_exists?
ActiveRecord::Base.connection
rescue ActiveRecord::NoDatabaseError
false
else
true
end


Paul-Bob marked this conversation as resolved.
Show resolved Hide resolved
Paul-Bob marked this conversation as resolved.
Show resolved Hide resolved
def audit?
Paul-Bob marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see where we use this method, probably in the gem itself?
If so, how about moving it there so there's little noise about it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm moving the audit configuration to the gem and will revert this change, thanks for pointing it

Avo.plugin_manager.installed?("avo-audits") &&
@audit &&
database_exists? &&
ActiveRecord::Base.connection.table_exists?(:avo_activities)
end

def turbo
Avo::ExecutionContext.new(target: @turbo).handle
end
Expand Down
1 change: 1 addition & 0 deletions lib/avo/current.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Avo::Current < ActiveSupport::CurrentAttributes
attribute :tool_manager
attribute :plugin_manager
attribute :locale
attribute :activity

# The tenant attributes are here so the user can add them on their own will
attribute :tenant_id
Expand Down
3 changes: 3 additions & 0 deletions lib/generators/avo/templates/initializer/avo.tt
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ Avo.configure do |config|
# file_logger
# }

## == Audit ==
# config.audit = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's think about the configuration of this gem a bit more. true/false is a bit terse and we'll for sure have more configs on that gem.
How about moving this config as config.enabled = true|false on that gem's configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about moving this config as config.enabled = true|false on that gem's configuration?

Well, that's a really good idea to have a configuration file for each gem!


## == Customization ==
# config.app_name = 'Avocadelicious'
# config.timezone = 'UTC'
Expand Down
2 changes: 1 addition & 1 deletion lib/tasks/avo_tasks.rake
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ task "avo:sym_link" do
`touch #{packages_path}/.keep`
end

["avo-advanced", "avo-pro", "avo-dynamic_filters", "avo-dashboards", "avo-menu"].each do |gem|
["avo-advanced", "avo-pro", "avo-dynamic_filters", "avo-dashboards", "avo-menu", "avo-audits"].each do |gem|
path = `bundle show #{gem} 2> /dev/null`.chomp

# If path is emty we check if package is defined outside of root (on release process it is)
Expand Down
Loading