From b12701a9ccef560ff9c45f810a160971c8c26fa4 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 25 Jan 2024 16:26:02 -0800 Subject: [PATCH 1/2] Add investigation reason to action-account output (LG-12212) changelog: Internal, Logging, Add reason to admin action output --- lib/action_account.rb | 17 +++++--- lib/data_pull.rb | 1 + lib/script_base.rb | 15 ++++++- spec/lib/action_account_spec.rb | 70 ++++++++++++++++++--------------- 4 files changed, 66 insertions(+), 37 deletions(-) diff --git a/lib/action_account.rb b/lib/action_account.rb index 1231aaadf40..8d838fe3a92 100644 --- a/lib/action_account.rb +++ b/lib/action_account.rb @@ -17,6 +17,7 @@ def script_base stderr:, subtask_class: subtask(argv.shift), banner: banner, + reason_arg: true, ) end @@ -58,8 +59,8 @@ def subtask(name) end module LogBase - def log_message(uuid:, log:, table:, messages:) - table << [uuid, log] + def log_message(uuid:, log:, reason:, table:, messages:) + table << [uuid, log, reason] messages << '`' + uuid + '`: ' + log [table, messages] end @@ -88,7 +89,7 @@ def perform_user_action(args:, config:, action:) table = [] messages = [] uuids = args - table << %w[uuid status] + table << %w[uuid status reason] users = User.where(uuid: uuids).order(:uuid) users.each do |user| @@ -124,6 +125,7 @@ def perform_user_action(args:, config:, action:) table, messages = log_message( uuid: user.uuid, log: text, + reason: config.reason, table:, messages:, ) @@ -135,6 +137,7 @@ def perform_user_action(args:, config:, action:) table, messages = log_message( uuid: missing_uuid, log: log_text[:missing_uuid], + reason: config.reason, table:, messages:, ) @@ -164,7 +167,7 @@ def run(args:, config:) users = User.where(uuid: uuids).order(:uuid) table = [] - table << %w[uuid status] + table << %w[uuid status reason] messages = [] @@ -191,6 +194,7 @@ def run(args:, config:) table, messages = log_message( uuid: user.uuid, log: text, + reason: config.reason, table:, messages:, ) @@ -215,6 +219,7 @@ def run(args:, config:) table, messages = log_message( uuid: missing_uuid, log: log_text[:missing_uuid], + reason: config.reason, table:, messages:, ) @@ -255,7 +260,7 @@ def run(args:, config:) users = User.where(uuid: uuids).order(:uuid) table = [] - table << %w[uuid status] + table << %w[uuid status reason] messages = [] users.each do |user| @@ -289,6 +294,7 @@ def run(args:, config:) table, messages = log_message( uuid: user.uuid, log: text, + reason: config.reason, table:, messages:, ) @@ -313,6 +319,7 @@ def run(args:, config:) table, messages = log_message( uuid: missing_uuid, log: log_text[:missing_uuid], + reason: config.reason, table:, messages:, ) diff --git a/lib/data_pull.rb b/lib/data_pull.rb index 31d3c3c7a3e..dd5b211f565 100644 --- a/lib/data_pull.rb +++ b/lib/data_pull.rb @@ -16,6 +16,7 @@ def script_base stderr:, subtask_class: subtask(argv.shift), banner: banner, + reason_arg: false, ) end diff --git a/lib/script_base.rb b/lib/script_base.rb index 6d6ea2a2eb2..fcdf5890221 100644 --- a/lib/script_base.rb +++ b/lib/script_base.rb @@ -3,12 +3,17 @@ class ScriptBase attr_reader :argv, :stdout, :stderr, :subtask_class, :banner - def initialize(argv:, stdout:, stderr:, subtask_class:, banner:) + def initialize(argv:, stdout:, stderr:, subtask_class:, banner:, reason_arg:) @argv = argv @stdout = stdout @stderr = stderr @subtask_class = subtask_class @banner = banner + @reason_arg = reason_arg + end + + def reason_arg? + !!@reason_arg end Result = Struct.new( @@ -26,6 +31,7 @@ def initialize(argv:, stdout:, stderr:, subtask_class:, banner:) :show_help, :requesting_issuers, :deflate, + :reason, keyword_init: true, ) do alias_method :include_missing?, :include_missing @@ -40,6 +46,7 @@ def config show_help: false, requesting_issuers: [], deflate: false, + reason: nil, ) end @@ -127,6 +134,12 @@ def option_parser STR config.include_missing = include_missing end + + if reason_arg? + opts.on('--reason=REASON', 'Reason for command') do |reason| + config.reason = reason + end + end end end # rubocop:enable Metrics/BlockLength diff --git a/spec/lib/action_account_spec.rb b/spec/lib/action_account_spec.rb index 933a16f41cd..474954a44d7 100644 --- a/spec/lib/action_account_spec.rb +++ b/spec/lib/action_account_spec.rb @@ -10,7 +10,7 @@ subject(:action_account) { ActionAccount.new(argv:, stdout:, stderr:) } describe 'command line run' do - let(:argv) { ['review-pass', user.uuid] } + let(:argv) { ['review-pass', user.uuid, '--reason', 'INV1234'] } let(:user) { create(:user) } it 'logs UUIDs and the command name to STDERR formatted for Slack', aggregate_failures: true do @@ -60,8 +60,8 @@ expect(CSV.parse(stdout.string)).to eq( [ - ['uuid', 'status'], - [user.uuid, 'Error: User does not have a pending fraud review'], + ['uuid', 'status', 'reason'], + [user.uuid, 'Error: User does not have a pending fraud review', 'INV1234'], ], ) end @@ -74,8 +74,8 @@ expect(Tableparser.parse(stdout.string)).to eq( [ - ['uuid', 'status'], - [user.uuid, 'Error: User does not have a pending fraud review'], + ['uuid', 'status', 'reason'], + [user.uuid, 'Error: User does not have a pending fraud review', 'INV1234'], ], ) end @@ -91,6 +91,7 @@ { 'uuid' => user.uuid, 'status' => 'Error: User does not have a pending fraud review', + 'reason' => 'INV1234', }, ], ) @@ -109,6 +110,7 @@ { 'uuid' => 'does_not_exist@example.com', 'status' => 'Error: Could not find user with that UUID', + 'reason' => nil, }, ], ) @@ -143,20 +145,22 @@ let(:args) { [user.uuid, user_without_profile.uuid, 'uuid-does-not-exist'] } let(:include_missing) { true } - let(:config) { ScriptBase::Config.new(include_missing:) } + let(:config) { ScriptBase::Config.new(include_missing:, reason: 'INV1234') } subject(:result) { subtask.run(args:, config:) } it 'Reject a user that has a pending review', aggregate_failures: true do profile_fraud_review_pending_at = user.pending_profile.fraud_review_pending_at + # rubocop:disable Layout/LineLength expect(result.table).to match_array( [ - ['uuid', 'status'], - [user.uuid, "User's profile has been deactivated due to fraud rejection."], - [user_without_profile.uuid, 'Error: User does not have a pending fraud review'], - ['uuid-does-not-exist', 'Error: Could not find user with that UUID'], + ['uuid', 'status', 'reason'], + [user.uuid, "User's profile has been deactivated due to fraud rejection.", 'INV1234'], + [user_without_profile.uuid, 'Error: User does not have a pending fraud review', 'INV1234'], + ['uuid-does-not-exist', 'Error: Could not find user with that UUID', 'INV1234'], ], ) + # rubocop:enable Layout/LineLength expect(result.subtask).to eq('review-reject') expect(result.uuids).to match_array([user.uuid, user_without_profile.uuid]) @@ -201,20 +205,22 @@ let(:args) { [user.uuid, user_without_profile.uuid, 'uuid-does-not-exist'] } let(:include_missing) { true } - let(:config) { ScriptBase::Config.new(include_missing:) } + let(:config) { ScriptBase::Config.new(include_missing:, reason: 'INV1234') } subject(:result) { subtask.run(args:, config:) } it 'Pass a user that has a pending review', aggregate_failures: true do profile_fraud_review_pending_at = user.pending_profile.fraud_review_pending_at + # rubocop:disable Layout/LineLength expect(result.table).to match_array( [ - ['uuid', 'status'], - [user.uuid, "User's profile has been activated and the user has been emailed."], - [user_without_profile.uuid, 'Error: User does not have a pending fraud review'], - ['uuid-does-not-exist', 'Error: Could not find user with that UUID'], + ['uuid', 'status', 'reason'], + [user.uuid, "User's profile has been activated and the user has been emailed.", 'INV1234'], + [user_without_profile.uuid, 'Error: User does not have a pending fraud review', 'INV1234'], + ['uuid-does-not-exist', 'Error: Could not find user with that UUID', 'INV1234'], ], ) + # rubocop:enable Layout/LineLength expect(result.subtask).to eq('review-pass') expect(result.uuids).to match_array([user.uuid, user_without_profile.uuid]) @@ -253,17 +259,17 @@ let(:reinstated_user) { create(:user, :reinstated) } let(:args) { [user.uuid, suspended_user.uuid, reinstated_user.uuid, 'uuid-does-not-exist'] } let(:include_missing) { true } - let(:config) { ScriptBase::Config.new(include_missing:) } + let(:config) { ScriptBase::Config.new(include_missing:, reason: 'INV1234') } subject(:result) { subtask.run(args:, config:) } it 'suspend a user that is not suspended already', aggregate_failures: true do expect(result.table).to match_array( [ - ['uuid', 'status'], - [user.uuid, 'User has been suspended'], - [suspended_user.uuid, 'User has already been suspended'], - [reinstated_user.uuid, 'User has been suspended'], - ['uuid-does-not-exist', 'Error: Could not find user with that UUID'], + ['uuid', 'status', 'reason'], + [user.uuid, 'User has been suspended', 'INV1234'], + [suspended_user.uuid, 'User has already been suspended', 'INV1234'], + [reinstated_user.uuid, 'User has been suspended', 'INV1234'], + ['uuid-does-not-exist', 'Error: Could not find user with that UUID', 'INV1234'], ], ) @@ -281,20 +287,22 @@ let(:suspended_user) { create(:user, :suspended) } let(:args) { [user.uuid, suspended_user.uuid, 'uuid-does-not-exist'] } let(:include_missing) { true } - let(:config) { ScriptBase::Config.new(include_missing:) } + let(:config) { ScriptBase::Config.new(include_missing:, reason: 'INV1234') } subject(:result) { subtask.run(args:, config:) } it 'suspends users that are not suspended already', aggregate_failures: true do expect { result }.to(change { ActionMailer::Base.deliveries.count }.by(1)) + # rubocop:disable Layout/LineLength expect(result.table).to match_array( [ - ['uuid', 'status'], - [user.uuid, 'User is not suspended'], - [suspended_user.uuid, 'User has been reinstated and the user has been emailed'], - ['uuid-does-not-exist', 'Error: Could not find user with that UUID'], + ['uuid', 'status', 'reason'], + [user.uuid, 'User is not suspended', 'INV1234'], + [suspended_user.uuid, 'User has been reinstated and the user has been emailed', 'INV1234'], + ['uuid-does-not-exist', 'Error: Could not find user with that UUID', 'INV1234'], ], ) + # rubocop:enable Layout/LineLength expect(result.subtask).to eq('reinstate-user') expect(result.uuids).to match_array([user.uuid, suspended_user.uuid]) @@ -310,7 +318,7 @@ let(:suspended_user) { create(:user, :suspended) } let(:args) { [suspended_user.uuid, user.uuid, 'uuid-does-not-exist'] } let(:include_missing) { true } - let(:config) { ScriptBase::Config.new(include_missing:) } + let(:config) { ScriptBase::Config.new(include_missing:, reason: 'INV1234') } subject(:result) { subtask.run(args:, config:) } let(:analytics) { FakeAnalytics.new } @@ -324,10 +332,10 @@ expect(result.table).to match_array( [ - ['uuid', 'status'], - [suspended_user.uuid, 'User has been emailed'], - [user.uuid, 'User is not suspended'], - ['uuid-does-not-exist', 'Error: Could not find user with that UUID'], + ['uuid', 'status', 'reason'], + [suspended_user.uuid, 'User has been emailed', 'INV1234'], + [user.uuid, 'User is not suspended', 'INV1234'], + ['uuid-does-not-exist', 'Error: Could not find user with that UUID', 'INV1234'], ], ) From e3b4e1753d488045bb150badd3f5bbfaccbf3288 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Thu, 25 Jan 2024 17:22:21 -0800 Subject: [PATCH 2/2] missing-arg --- spec/lib/script_base_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/script_base_spec.rb b/spec/lib/script_base_spec.rb index d121f02b49a..f3e2c1379cb 100644 --- a/spec/lib/script_base_spec.rb +++ b/spec/lib/script_base_spec.rb @@ -30,6 +30,7 @@ def run(args:, config:) # rubocop:disable Lint/UnusedMethodArgument stderr:, subtask_class:, banner: '', + reason_arg: false, ) end