Merge pull request #1629 from senid231/admin-api-audit

admin api fix audit whodunnit

app/controllers/api/rest/admin/base_controller.rb

@@ -4,6 +4,12 @@ class Api::Rest::Admin::BaseController < Api::RestController
   include JSONAPI::ActsAsResourceController
   include AdminApiAuthorizable
 
+  before_action :set_paper_trail_whodunnit # must be after `include AdminApiAuthorizable`
+
+  def user_for_paper_trail
+    current_admin_user&.id || 'Unknown user'
+  end
+
   def handle_exceptions(e)
     capture_error(e) unless e.is_a?(JSONAPI::Exceptions::Error)
     super

spec/controllers/api/rest/admin/accounts_controller_spec.rb

@@ -81,10 +81,31 @@
   end
 
   describe 'POST create' do
-    before do
+    subject do
       post :create, params: request_payload
     end
 
+    shared_examples :creates_account do
+      it 'creates account' do
+        expect { subject }.to change { Account.count }.by(1)
+                          .and change { AccountBalanceNotificationSetting.count }.by(1)
+      end
+
+      include_examples :creates_audit_log, qty: 1, ordered: true do
+        let(:audit_log_attrs) do
+          item = Account.last
+          { event: 'create', item_id: item.id, item_type: item.class.name, whodunnit: admin_user.id.to_s }
+        end
+      end
+    end
+
+    shared_examples :does_not_create_account do
+      it 'creates account' do
+        expect { subject }.not_to change { [Account.count, AccountBalanceNotificationSetting.count] }
+      end
+    end
+
+    let!(:contractor) { create(:contractor, vendor: true) }
     let(:request_payload) do
       {
         data: {
@@ -115,23 +136,21 @@
     let(:relationships) do
       {
         timezone: wrap_relationship(:timezones, 1),
-        contractor: wrap_relationship(:contractors, create(:contractor, vendor: true).id)
+        contractor: wrap_relationship(:contractors, contractor.id)
       }
     end
 
     context 'when attributes and relationships are valid' do
-      it { expect(response.status).to eq(201) }
-      it { expect(Account.count).to eq(1) }
-      it { expect(AccountBalanceNotificationSetting.count).to eq(1) }
+      include_examples :responds_with_status, 201
+      include_examples :creates_account
     end
 
     context 'when attributes and relationships are invalid' do
       let(:attributes) { { name: 'name', 'max-balance': -1 } }
       let(:relationships) { {} }
 
-      it { expect(response.status).to eq(422) }
-      it { expect(Account.count).to eq(0) }
-      it { expect(AccountBalanceNotificationSetting.count).to eq(0) }
+      include_examples :responds_with_status, 422
+      include_examples :does_not_create_account
     end
 
     context 'when only required attributes and relationships' do
@@ -142,13 +161,12 @@
       end
       let(:relationships) do
         {
-          contractor: wrap_relationship(:contractors, create(:contractor, vendor: true).id)
+          contractor: wrap_relationship(:contractors, contractor.id)
         }
       end
 
-      it { expect(response.status).to eq(201) }
-      it { expect(Account.count).to eq(1) }
-      it { expect(AccountBalanceNotificationSetting.count).to eq(1) }
+      include_examples :responds_with_status, 201
+      include_examples :creates_account
     end
 
     context 'with attributes balance-low-threshold = balance-high-threshold' do
@@ -157,8 +175,8 @@
                       'balance-high-threshold': 90
       end
 
-      it { expect(response.status).to eq(422) }
-      it { expect(Account.count).to eq(0) }
+      include_examples :responds_with_status, 422
+      include_examples :does_not_create_account
     end
 
     context 'with attributes balance-low-threshold > balance-high-threshold' do
@@ -167,8 +185,8 @@
                       'balance-high-threshold': 90
       end
 
-      it { expect(response.status).to eq(422) }
-      it { expect(Account.count).to eq(0) }
+      include_examples :responds_with_status, 422
+      include_examples :does_not_create_account
     end
   end
 

spec/controllers/api/rest/admin/balance_controller_spec.rb

@@ -23,9 +23,8 @@
       it 'should change balance' do
         expect { subject }.to change { account.reload.balance }.from(balance).to(attributes[:balance])
       end
-      it 'should skip audit log' do
-        expect { subject }.not_to change { AuditLogItem.count }
-      end
+
+      include_examples :does_not_create_audit_log
 
       context 'response' do
         before { subject }

spec/support/examples/audit_logs/creates_audit_log.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+# @example usage:
+#   include_examples :creates_audit_log do
+#     let(:audit_log_attrs) do
+#       item = Account.last
+#       { event: 'create', item_id: item.id, item_type: item.class.name, whodunnit: admin_user.id.to_s }
+#     end
+#   end
+#  # when several items are created at once
+#  include_examples :creates_audit_log, qty: 2 do
+#    let(:audit_log_attrs) do
+#      [Account.last, CustomersAuth.last].map do |item|
+#        { event: 'create', item_id: item.id, item_type: item.class.name, whodunnit: admin_user.id.to_s }
+#      end
+#    end
+#  end
+RSpec.shared_examples :creates_audit_log do |qty: 1, ordered: false|
+  # Note: we didn't calculate qty by audit_log_attrs, because we want it to be called only after subject,
+  # so we could use subject result in audit_log_attrs.
+
+  it 'creates audit log', :aggregate_failures do
+    expect { subject }.to change { AuditLogItem.count }.by(qty)
+    logs = AuditLogItem.last(qty)
+    actual_attrs_list = logs.map { |log| log.attributes.symbolize_keys }
+    expected_attrs_list = Array.wrap(audit_log_attrs).map { |attrs| hash_including(attrs) }
+
+    if ordered
+      expected_attrs_list.each_with_index do |expected_attrs, index|
+        expect(actual_attrs_list[index]).to match(expected_attrs)
+      end
+    else
+      expect(actual_attrs_list).to match_array(expected_attrs_list)
+    end
+  end
+end

spec/support/examples/audit_logs/does_not_create_audit_log.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+# @example usage:
+#   include_examples :does_not_create_audit_log
+RSpec.shared_examples :does_not_create_audit_log do
+  it 'dos not create audit log' do
+    expect { subject }.not_to change { AuditLogItem.count }
+  end
+end