Skip to content

Commit

Permalink
Vi 865 user audit logger (#20577)
Browse files Browse the repository at this point in the history
* Removes Sidekiq Enterprise and Pro dependencies

Removes enterprise-specific Sidekiq gems and their dependencies (einhorn, gserver) to simplify the application's job processing infrastructure and reduce external dependencies.

* Adds UserAuditLogger service and specs

This commit introduces a new UserAuditLogger service for tracking user actions
and their verification statuses. The implementation includes:

The UserAuditLogger provides functionality to:
- Log user actions with verification status
- Support custom status messages
- Handle missing verification cases
- Maintain consistent logging format

Files changed:
- app/services/user_audit_logger.rb
- spec/services/user_audit_logger_spec.rb
- .github/CODEOWNERS

* Removes trailing whitespace from Ruby files

* refactor(user-audit-logger): Convert to service object pattern

- Convert to instance class with initialize/perform pattern
- Accept verification objects directly instead of user objects
- Remove default status, require explicit status value
- Add validations for required fields
- Use config hash to fix parameter list length
- Pass UserActionEvent object instead of ID

The UserAuditLogger now follows service object conventions and has
stricter parameter requirements for better reliability.

* Use implicit arguments and consistent naming

* refactor: simplify UserAuditLogger specs and improve initialization

- Replace config hash with explicit keyword arguments in UserAuditLogger initialization
- Move status to a top-level let declaration for better test organization
- Update error message for subject verification to be more precise
- Update missing parameter test to match Ruby's actual ArgumentError message
- Remove redundant logger initialization in specs

* refactor: improve UserAuditLogger initialization and specs

- Update UserAuditLogger to use explicit keyword arguments

Co-authored-by: bramleyjl

* revert: remove Gemfile.lock changes

The Gemfile.lock changes were unintentionally included in commit 1255301.
These changes should be handled separately.

* revert: remove Gemfile.lock changes

The Gemfile.lock changes were unintentionally included in commit 1255301.
These changes should be handled separately.

* improve UserAuditLogger parameter handling

- Keep explicit parameters in initialize for better code clarity
- Add inline rubocop disable for ParameterLists
- Update error message for subject verification
- Use implicit hash syntax consistently
- Remove redundant status testing
- Remove unnecessary config variable

The initialize method intentionally keeps all parameters explicit to make
it clear what the class requires, with a rubocop disable comment to
acknowledge the parameter count.

* Improves Rubocop directive placement
  • Loading branch information
anjolovic authored Feb 5, 2025
1 parent a895d27 commit 7af6482
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 0 deletions.
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ app/services/mhv_logging_service.rb @department-of-veterans-affairs/octo-identit
app/services/rapid_ready_for_decision @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/services/sign_in @department-of-veterans-affairs/octo-identity
app/services/terms_of_use @department-of-veterans-affairs/octo-identity
app/services/user_audit_logger.rb @department-of-veterans-affairs/octo-identity
app/services/users @department-of-veterans-affairs/octo-identity
app/swagger/readme.md @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
app/swagger/swagger/requests/appeals @department-of-veterans-affairs/backend-review-group
Expand Down Expand Up @@ -1885,6 +1886,7 @@ spec/services/mhv_logging_service_spec.rb @department-of-veterans-affairs/octo-i
spec/services/rapid_ready_for_decision @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/services/sign_in @department-of-veterans-affairs/octo-identity
spec/services/terms_of_use @department-of-veterans-affairs/octo-identity
spec/services/user_audit_logger_spec.rb @department-of-veterans-affairs/octo-identity
spec/services/users @department-of-veterans-affairs/octo-identity
spec/simplecov_helper.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/spec_helper.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
Expand Down
59 changes: 59 additions & 0 deletions app/services/user_audit_logger.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

class UserAuditLogger
class Error < StandardError; end
class MissingSubjectVerificationError < Error; end
class MissingUserActionEventError < Error; end
class MissingStatusError < Error; end

attr_reader :user_action_event, :acting_user_verification, :subject_user_verification,
:status, :acting_ip_address, :acting_user_agent

# rubocop:disable Metrics/ParameterLists
def initialize(user_action_event:, acting_user_verification:, subject_user_verification:,
status:, acting_ip_address:, acting_user_agent:)
@user_action_event = user_action_event
@acting_user_verification = acting_user_verification
@subject_user_verification = subject_user_verification
@status = status
@acting_ip_address = acting_ip_address
@acting_user_agent = acting_user_agent
end
# rubocop:enable Metrics/ParameterLists

def perform
validate_required_fields
create_user_action
end

private

def validate_required_fields
validate_user_action_event
validate_subject_verification
validate_status
end

def validate_user_action_event
raise MissingUserActionEventError, 'User action event must be present' if user_action_event.nil?
end

def validate_subject_verification
raise MissingSubjectVerificationError, 'Subject user verification must be present' if subject_user_verification.nil?
end

def validate_status
raise MissingStatusError, 'Status must be present' if status.nil?
end

def create_user_action
UserAction.create!(
user_action_event:,
acting_user_verification:,
subject_user_verification:,
status:,
acting_ip_address:,
acting_user_agent:
)
end
end
82 changes: 82 additions & 0 deletions spec/services/user_audit_logger_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe UserAuditLogger do
describe '#perform' do
let(:user_action_event) { create(:user_action_event) }
let(:acting_user_verification) { create(:user_verification) }
let(:subject_user_verification) { create(:user_verification) }
let(:acting_ip_address) { Faker::Internet.ip_v4_address }
let(:acting_user_agent) { Faker::Internet.user_agent }
let(:status) { :initial }
let(:logger) do
described_class.new(
user_action_event:,
acting_user_verification:,
subject_user_verification:,
status:,
acting_ip_address:,
acting_user_agent:
)
end

it 'creates a user action record' do
expect { logger.perform }.to change(UserAction, :count).by(1)

user_action = UserAction.last
expect(user_action).to have_attributes(
user_action_event: user_action_event,
acting_user_verification: acting_user_verification,
subject_user_verification: subject_user_verification,
status: 'initial',
acting_ip_address: acting_ip_address,
acting_user_agent: acting_user_agent
)
end

context 'when user_action_event is nil' do
let(:user_action_event) { nil }

it 'raises a missing user action event error' do
expect { logger.perform }.to raise_error(
UserAuditLogger::MissingUserActionEventError,
'User action event must be present'
)
end
end

context 'when subject_user_verification is nil' do
let(:subject_user_verification) { nil }

it 'raises a missing verification error' do
expect { logger.perform }.to raise_error(
UserAuditLogger::MissingSubjectVerificationError,
'Subject user verification must be present'
)
end
end

context 'when status is nil' do
let(:status) { nil }

it 'raises a missing status error' do
expect { logger.perform }.to raise_error(
UserAuditLogger::MissingStatusError,
'Status must be present'
)
end
end

context 'when required parameter is not provided' do
it 'raises an argument error' do
expect do
described_class.new(
user_action_event:,
subject_user_verification:
)
end.to raise_error(ArgumentError, /missing keywords: :acting_user_verification/)
end
end
end
end

0 comments on commit 7af6482

Please sign in to comment.