-
-
Notifications
You must be signed in to change notification settings - Fork 232
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
base: main
Are you sure you want to change the base?
feature: audit logs #2703
Changes from 22 commits
1167828
8c4c54a
fc9720b
04e7d7f
2f6dbc3
9826cdc
0070b19
f64dfb8
8d9248c
0771d63
2c9b896
361120a
0db29ba
4986af8
16c3019
f88a17f
496f42d
cdde642
da5f08f
58210f7
53221df
70cd62c
f0ab0a2
f3c5e1c
8840724
5e614a4
bd07682
dc2f288
019ca76
6745d11
cf57fe1
6fc5c46
80b049a
9595046
02d3595
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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 | ||
|
@@ -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 | ||
|
||
|
@@ -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 | ||
|
@@ -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) | ||
|
@@ -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 | ||
|
@@ -549,10 +561,6 @@ def set_pagy_locale | |
@pagy_locale = locale.to_s || Avo.configuration.locale || "en" | ||
end | ||
|
||
def safe_call(method) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we need this? And shouldn't it take more args? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 = {} | ||
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? 😢 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you consider moving the audit feature to the advanced plan? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @yorsant. 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. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW you can build audit log yourself using papertrail and avo resources. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,9 @@ Avo.configure do |config| | |
# file_logger | ||
# } | ||
|
||
## == Audit == | ||
# config.audit = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Well, that's a really good idea to have a configuration file for each gem! |
||
|
||
## == Customization == | ||
# config.app_name = 'Avocadelicious' | ||
# config.timezone = 'UTC' | ||
|
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.
Can't we add this ourselves in the audit logging gem?