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

LG-14259 Update Report formatting #11166

Merged
merged 5 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
6 changes: 2 additions & 4 deletions app/jobs/reports/fraud_metrics_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def perform(date = Time.zone.yesterday.end_of_day)

ReportMailer.tables_report(
email: email_addresses,
subject: "Fraud Metrics Report - #{date.to_date}",
subject: "Fraud Metrics Report - #{report_date.to_date}",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using the accessor.

reports: reports,
message: preamble,
attachment_format: :xlsx,
Expand Down Expand Up @@ -60,9 +60,7 @@ def preamble(env: Identity::Hostdata.env || 'local')
end

def reports
@reports ||= [
fraud_metrics_lg99_report.as_emailable_reports,
]
@reports ||= fraud_metrics_lg99_report.as_emailable_reports
end

def fraud_metrics_lg99_report
Expand Down
96 changes: 80 additions & 16 deletions lib/reporting/fraud_metrics_lg99_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,30 @@ def progress?
end

def as_emailable_reports
Reporting::EmailableReport.new(
Comment on lines 53 to -54
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm glad this plural method name is now actually returning an array

title: 'LG-99 Metrics',
table: lg99_metrics_table,
filename: 'lg99_metrics',
)
[
Reporting::EmailableReport.new(
title: "Monthly LG-99 Metrics #{stats_month}",
table: lg99_metrics_table,
filename: 'lg99_metrics',
),
Reporting::EmailableReport.new(
title: "Monthly Suspended User Metrics #{stats_month}",
table: suspended_metrics_table,
filename: 'suspended_metrics',
),
Reporting::EmailableReport.new(
title: "Monthly Reinstated User Metrics #{stats_month}",
table: reinstated_metrics_table,
filename: 'reinstated_metrics',
),
]
end

def lg99_metrics_table
[
['Metric', 'Total'],
['Unique users seeing LG-99', lg99_unique_users_count.to_s],
['Unique users suspended', unique_suspended_users_count.to_s],
['Average Days Creation to Suspension', user_days_to_suspension_avg.to_s],
['Average Days Proofed to Suspension', user_days_proofed_to_suspension_avg.to_s],
['Unique users reinstated', unique_reinstated_users_count.to_s],
['Average Days to Reinstatement', user_days_to_reinstatement_avg.to_s],
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users seeing LG-99', lg99_unique_users_count.to_s, time_range.begin.to_s,
time_range.end.to_s],
]
rescue Aws::CloudWatchLogs::Errors::ThrottlingException => err
[
Expand All @@ -75,14 +83,70 @@ def lg99_metrics_table
]
end

def to_csv
CSV.generate do |csv|
lg99_metrics_table.each do |row|
csv << row
def suspended_metrics_table
[
['Metric', 'Total', 'Range Start', 'Range End'],
[
'Unique users suspended',
unique_suspended_users_count.to_s,
time_range.begin.to_s,
time_range.end.to_s,
],
[
'Average Days Creation to Suspension',
user_days_to_suspension_avg.to_s,
time_range.begin.to_s,
time_range.end.to_s,
],
[
'Average Days Proofed to Suspension',
user_days_proofed_to_suspension_avg.to_s,
time_range.begin.to_s,
time_range.end.to_s,
],
]
end

def reinstated_metrics_table
[
['Metric', 'Total', 'Range Start', 'Range End'],
[
'Unique users reinstated',
unique_reinstated_users_count.to_s,
time_range.begin.to_s,
time_range.end.to_s,
],
[
'Average Days to Reinstatement',
user_days_to_reinstatement_avg.to_s,
time_range.begin.to_s,
time_range.end.to_s,
],
]
end

def as_tables
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, this is unused too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, done. If we ever want to mimic some of the other reporting then we can just add it back.

[
lg99_metrics_table,
suspended_metrics_table,
reinstated_metrics_table,
]
end

def to_csvs
Copy link
Contributor

Choose a reason for hiding this comment

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

this method is unused now, right? let's remove it entirely

as_tables.map do |table|
CSV.generate do |csv|
table.each do |row|
csv << row
end
end
end
end

def stats_month
time_range.begin.strftime('%b-%Y')
end

# event name => set(user ids)
# @return Hash<String,Set<String>>
def data
Expand Down
34 changes: 32 additions & 2 deletions spec/jobs/reports/fraud_metrics_report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

RSpec.describe Reports::FraudMetricsReport do
let(:report_date) { Date.new(2021, 3, 2).in_time_zone('UTC').end_of_day }
let(:time_range) { report_date.all_month }
subject(:report) { Reports::FraudMetricsReport.new(report_date) }

let(:name) { 'fraud-metrics-report' }
Expand All @@ -13,6 +14,8 @@
let(:expected_s3_paths) do
[
"#{report_folder}/lg99_metrics.csv",
"#{report_folder}/suspended_metrics.csv",
"#{report_folder}/reinstated_metrics.csv",
]
end

Expand All @@ -26,8 +29,29 @@

let(:mock_identity_verification_lg99_data) do
[
['Metric', 'Total'],
['Unique users seeing LG-99', 5],
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users seeing LG-99', 5, time_range.begin.to_s,
time_range.end.to_s],
]
end
let(:mock_suspended_metrics_table) do
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users suspended', 2, time_range.begin.to_s,
time_range.end.to_s],
['Average Days Creation to Suspension', 1.5, time_range.begin.to_s,
time_range.end.to_s],
['Average Days Proofed to Suspension', 2.0, time_range.begin.to_s,
time_range.end.to_s],
]
end
let(:mock_reinstated_metrics_table) do
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users reinstated', 1, time_range.begin.to_s,
time_range.end.to_s],
['Average Days to Reinstatement', 3.0, time_range.begin.to_s,
time_range.end.to_s],
]
end

Expand All @@ -54,6 +78,12 @@

allow(report.fraud_metrics_lg99_report).to receive(:lg99_metrics_table).
and_return(mock_identity_verification_lg99_data)

allow(report.fraud_metrics_lg99_report).to receive(:suspended_metrics_table).
and_return(mock_suspended_metrics_table)

allow(report.fraud_metrics_lg99_report).to receive(:reinstated_metrics_table).
and_return(mock_reinstated_metrics_table)
end

it 'sends out a report to just to team agnes' do
Expand Down
104 changes: 87 additions & 17 deletions spec/lib/reporting/fraud_metrics_lg99_report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,24 @@
let(:time_range) { Date.new(2022, 1, 1).in_time_zone('UTC').all_month }
let(:expected_lg99_metrics_table) do
[
['Metric', 'Total'],
['Unique users seeing LG-99', '5'],
['Unique users suspended', '2'],
['Average Days Creation to Suspension', '1.5'],
['Average Days Proofed to Suspension', '2.0'],
['Unique users reinstated', '1'],
['Average Days to Reinstatement', '3.0'],
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users seeing LG-99', '5', time_range.begin.to_s,
time_range.end.to_s],
]
end
let(:expected_suspended_metrics_table) do
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users suspended', '2', time_range.begin.to_s, time_range.end.to_s],
['Average Days Creation to Suspension', '1.5', time_range.begin.to_s, time_range.end.to_s],
['Average Days Proofed to Suspension', '2.0', time_range.begin.to_s, time_range.end.to_s],
]
end
let(:expected_reinstated_metrics_table) do
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users reinstated', '1', time_range.begin.to_s, time_range.end.to_s],
['Average Days to Reinstatement', '3.0', time_range.begin.to_s, time_range.end.to_s],
]
end
let!(:user6) do
Expand Down Expand Up @@ -65,6 +76,28 @@
end
end

describe '#suspended_metrics_table' do
it 'renders a suspended metrics table' do
aggregate_failures do
report.suspended_metrics_table.zip(expected_suspended_metrics_table).
each do |actual, expected|
expect(actual).to eq(expected)
end
end
end
end

describe '#reinstated_metrics_table' do
it 'renders a reinstated metrics table' do
aggregate_failures do
report.reinstated_metrics_table.zip(expected_reinstated_metrics_table).
each do |actual, expected|
expect(actual).to eq(expected)
end
end
end
end

describe '#user_days_to_suspension_avg' do
context 'when there are suspended users' do
it 'returns average time to suspension' do
Expand Down Expand Up @@ -117,28 +150,65 @@
end

describe '#as_emailable_reports' do
let(:expected_report) do
Reporting::EmailableReport.new(
title: 'LG-99 Metrics',
filename: 'lg99_metrics',
table: expected_lg99_metrics_table,
)
let(:expected_reports) do
[
Reporting::EmailableReport.new(
title: 'Monthly LG-99 Metrics Jan-2022',
filename: 'lg99_metrics',
table: expected_lg99_metrics_table,
),
Reporting::EmailableReport.new(
title: 'Monthly Suspended User Metrics Jan-2022',
filename: 'suspended_metrics',
table: expected_suspended_metrics_table,
),
Reporting::EmailableReport.new(
title: 'Monthly Reinstated User Metrics Jan-2022',
filename: 'reinstated_metrics',
table: expected_reinstated_metrics_table,
),
]
end
it 'return expected table for email' do
expect(report.as_emailable_reports).to eq expected_report
expect(report.as_emailable_reports).to eq expected_reports
end
end

describe '#to_csv' do
it 'renders a csv report' do
describe '#to_csvs' do
it 'generates a csv' do
csv_string_list = report.to_csvs
expect(csv_string_list.count).to be 3

csvs = csv_string_list.map { |csv| CSV.parse(csv) }

aggregate_failures do
report.lg99_metrics_table.zip(expected_lg99_metrics_table).each do |actual, expected|
csvs.map(&:to_a).zip(expected_tables).each do |actual, expected|
expect(actual).to eq(expected)
end
end
end
end

def expected_tables
[
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users seeing LG-99', '5', time_range.begin.to_s, time_range.end.to_s],
],
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users suspended', '2', time_range.begin.to_s, time_range.end.to_s],
['Average Days Creation to Suspension', '1.5', time_range.begin.to_s, time_range.end.to_s],
['Average Days Proofed to Suspension', '2.0', time_range.begin.to_s, time_range.end.to_s],
],
[
['Metric', 'Total', 'Range Start', 'Range End'],
['Unique users reinstated', '1', time_range.begin.to_s, time_range.end.to_s],
['Average Days to Reinstatement', '3.0', time_range.begin.to_s, time_range.end.to_s],
],
]
end

describe '#cloudwatch_client' do
let(:opts) { {} }
let(:subject) { described_class.new(time_range:, **opts) }
Expand Down