diff --git a/Gemfile b/Gemfile index 8c1cc92db0a..21b1aa0ca10 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,7 @@ gem 'barby', '~> 0.6.8' gem 'base32-crockford' gem 'bootsnap', '~> 1.0', require: false gem 'browser' +gem 'caxlsx', require: false gem 'concurrent-ruby' gem 'connection_pool' gem 'cssbundling-rails' @@ -128,6 +129,7 @@ group :test do gem 'rspec-retry' gem 'rspec_junit_formatter' gem 'shoulda-matchers', '~> 4.0', require: false + gem 'simple_xlsx_reader', require: false gem 'tableparser', require: false gem 'webmock' gem 'zonebie' diff --git a/Gemfile.lock b/Gemfile.lock index f66eef47051..424f4cf061b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -220,6 +220,11 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) + caxlsx (3.4.1) + htmlentities (~> 4.3, >= 4.3.4) + marcel (~> 1.0) + nokogiri (~> 1.10, >= 1.10.4) + rubyzip (>= 1.3.0, < 3) cbor (0.5.9.6) chunky_png (1.4.0) coderay (1.1.3) @@ -614,6 +619,9 @@ GEM simple_form (5.1.0) actionpack (>= 5.2) activemodel (>= 5.2) + simple_xlsx_reader (5.0.0) + nokogiri + rubyzip simplecov (0.22.0) docile (~> 1.1) simplecov-html (~> 0.11) @@ -720,6 +728,7 @@ DEPENDENCIES bullet (~> 7.0) bundler-audit capybara-webmock! + caxlsx concurrent-ruby connection_pool cssbundling-rails @@ -799,6 +808,7 @@ DEPENDENCIES scrypt shoulda-matchers (~> 4.0) simple_form (>= 5.0.2) + simple_xlsx_reader simplecov (~> 0.22.0) simplecov-cobertura simplecov_json_formatter diff --git a/app/jobs/reports/authentication_report.rb b/app/jobs/reports/authentication_report.rb index c530048f643..efd31063938 100644 --- a/app/jobs/reports/authentication_report.rb +++ b/app/jobs/reports/authentication_report.rb @@ -22,6 +22,7 @@ def perform(report_date) subject:, message:, tables:, + attachment_format: :csv, ).deliver_now end end diff --git a/app/jobs/reports/monthly_key_metrics_report.rb b/app/jobs/reports/monthly_key_metrics_report.rb index 246114b0d23..25a0a8bda48 100644 --- a/app/jobs/reports/monthly_key_metrics_report.rb +++ b/app/jobs/reports/monthly_key_metrics_report.rb @@ -57,6 +57,7 @@ def perform(date = Time.zone.today) subject: "Monthly Key Metrics Report - #{date}", message: email_message, tables: email_tables, + attachment_format: :xlsx, ).deliver_now end diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index 55f8f1f2d19..46bff5aa33f 100644 --- a/app/mailers/report_mailer.rb +++ b/app/mailers/report_mailer.rb @@ -1,4 +1,5 @@ require 'csv' +require 'caxlsx' class ReportMailer < ActionMailer::Base include Mailable @@ -38,12 +39,20 @@ def warn_error(email:, error:, env: Rails.env) # @param [String] email # @param [String] subject # @param [String] env name of current deploy environment + # @param [:csv,:xlsx] attachment_format # @param [Array>>] tables # an array of tables (which are arrays of rows (arrays of strings)) # each table can have a first "row" that is a hash with options # @option opts [Boolean] :float_as_percent whether or not to render floats as percents # @option opts [Boolean] :title title of the table - def tables_report(email:, subject:, message:, tables:, env: Identity::Hostdata.env || 'local') + def tables_report( + email:, + subject:, + message:, + tables:, + attachment_format:, + env: Identity::Hostdata.env || 'local' + ) @message = message @tables = tables.map(&:dup).each_with_index.map do |table, index| @@ -54,16 +63,35 @@ def tables_report(email:, subject:, message:, tables:, env: Identity::Hostdata.e [options, *table] end - @tables.each do |options_and_table| - options, *table = options_and_table + case attachment_format + when :csv + @tables.each do |options_and_table| + options, *table = options_and_table - title = "#{options[:title].parameterize}.csv" + title = "#{options[:title].parameterize}.csv" - attachments[title] = CSV.generate do |csv| - table.each do |row| - csv << row + attachments[title] = CSV.generate do |csv| + table.each do |row| + csv << row + end end end + when :xlsx + Axlsx::Package.new do |package| + @tables.each do |options_and_table| + options, *table = options_and_table + + package.workbook.add_worksheet(name: options[:title].byteslice(0...31)) do |sheet| + table.each do |row| + sheet.add_row(row) + end + end + end + + attachments['report.xlsx'] = package.to_stream.read + end + else + raise ArgumentError, "unknown attachment_format=#{attachment_format}" end mail(to: email, subject: "[#{env}] #{subject}") diff --git a/spec/jobs/reports/authentication_report_spec.rb b/spec/jobs/reports/authentication_report_spec.rb index 5bc2d870aff..f3efd34bd16 100644 --- a/spec/jobs/reports/authentication_report_spec.rb +++ b/spec/jobs/reports/authentication_report_spec.rb @@ -62,6 +62,7 @@ subject: "Weekly Authentication Report - #{report_date}", message: "Report: authentication-report #{report_date}", tables:, + attachment_format: :csv, ) subject.perform(report_date) diff --git a/spec/jobs/reports/monthly_key_metrics_report_spec.rb b/spec/jobs/reports/monthly_key_metrics_report_spec.rb index 66ef072c34b..dc6cb98fb5a 100644 --- a/spec/jobs/reports/monthly_key_metrics_report_spec.rb +++ b/spec/jobs/reports/monthly_key_metrics_report_spec.rb @@ -59,6 +59,7 @@ email: [agnes_email], subject: 'Monthly Key Metrics Report - 2021-03-02', tables: anything, + attachment_format: :xlsx, ).and_call_original subject.perform(report_date) @@ -72,6 +73,7 @@ email: [agnes_email, feds_email], subject: 'Monthly Key Metrics Report - 2021-03-01', tables: anything, + attachment_format: :xlsx, ).and_call_original subject.perform(first_of_month_date) @@ -83,12 +85,7 @@ expect_any_instance_of(Reporting::AccountReuseAndTotalIdentitiesReport). not_to receive(:total_identities_report) - expect(ReportMailer).not_to receive(:tables_report).with( - message: 'Report: monthly-key-metrics-report 2021-03-02', - email: [''], - subject: 'Monthly Key Metrics Report - 2021-03-02', - tables: anything, - ).and_call_original + expect(ReportMailer).not_to receive(:tables_report) subject.perform(report_date) end diff --git a/spec/mailers/previews/report_mailer_preview.rb b/spec/mailers/previews/report_mailer_preview.rb index d4bfdb255ec..2f533da743c 100644 --- a/spec/mailers/previews/report_mailer_preview.rb +++ b/spec/mailers/previews/report_mailer_preview.rb @@ -13,6 +13,7 @@ def monthly_key_metrics_report email: 'test@example.com', subject: 'Example Key Metrics Report', message: 'Key Metrics Report February 2021', + attachment_format: :xlsx, tables: [ [ { title: 'February 2021 Active Users' }, @@ -64,6 +65,7 @@ def tables_report email: 'test@example.com', subject: 'Example Report', message: 'Sample Message', + attachment_format: :csv, tables: [ [ ['Some', 'String'], diff --git a/spec/mailers/previews/report_mailer_preview_spec.rb b/spec/mailers/previews/report_mailer_preview_spec.rb index 72368761d6d..a487d07d892 100644 --- a/spec/mailers/previews/report_mailer_preview_spec.rb +++ b/spec/mailers/previews/report_mailer_preview_spec.rb @@ -4,15 +4,11 @@ RSpec.describe ReportMailerPreview do subject(:mailer_preview) { ReportMailerPreview.new } - describe '#warn_error' do - it 'generates a warn_error email' do - expect { mailer_preview.warn_error }.to_not raise_error - end - end - - describe '#tables_report' do - it 'generates a tables_report email' do - expect { mailer_preview.tables_report }.to_not raise_error + ReportMailerPreview.instance_methods(false).each do |mailer_method| + describe "##{mailer_method}" do + it 'generates a preview without blowing up' do + expect { mailer_preview.public_send(mailer_method).body }.to_not raise_error + end end end end diff --git a/spec/mailers/previews/user_mailer_preview_spec.rb b/spec/mailers/previews/user_mailer_preview_spec.rb index 1ce068b8a33..f0ae9d81b88 100644 --- a/spec/mailers/previews/user_mailer_preview_spec.rb +++ b/spec/mailers/previews/user_mailer_preview_spec.rb @@ -5,7 +5,7 @@ UserMailerPreview.instance_methods(false).each do |mailer_method| describe "##{mailer_method}" do it 'generates a preview without blowing up' do - expect { UserMailerPreview.new.public_send(mailer_method) }.to_not raise_error + expect { UserMailerPreview.new.public_send(mailer_method).body }.to_not raise_error end end end diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb index f643a0ee376..29ec6a53aba 100644 --- a/spec/mailers/report_mailer_spec.rb +++ b/spec/mailers/report_mailer_spec.rb @@ -1,4 +1,5 @@ require 'rails_helper' +require 'simple_xlsx_reader' RSpec.describe ReportMailer, type: :mailer do let(:user) { build(:user) } @@ -55,6 +56,31 @@ describe '#tables_report' do let(:env) { 'prod' } + let(:attachment_format) { :csv } + + let(:first_table) do + [ + ['Some', 'String'], + ['a', 'b'], + ['c', 'd'], + ] + end + + let(:second_table) do + [ + ['Float', 'Int', 'Float'], + ['Row 1', 1, 0.5], + ['Row 2', 1, 1.5], + ] + end + + let(:third_table) do + [ + ['Float As Percent', 'Gigantic Int', 'Float'], + ['Row 1', 100_000_000, 1.0], + ['Row 2', 123_456_789, 1.5], + ] + end let(:mail) do ReportMailer.tables_report( @@ -62,23 +88,16 @@ subject: 'My Report', message: 'My Report - Today', env: env, + attachment_format: attachment_format, tables: [ - [ - ['Some', 'String'], - ['a', 'b'], - ['c', 'd'], - ], + first_table, [ { float_as_percent: true, title: 'Custom Table 2' }, - ['Float', 'Int', 'Float'], - ['Row 1', 1, 0.5], - ['Row 2', 1, 1.5], + *second_table, ], [ - { float_as_percent: false, title: 'Custom Table 3' }, - ['Float As Percent', 'Gigantic Int', 'Float'], - ['Row 1', 100_000_000, 1.0], - ['Row 2', 123_456_789, 1.5], + { float_as_percent: false, title: 'Custom Table 3 With Very Long Name' }, + *third_table, ], ], ) @@ -88,10 +107,11 @@ expect(mail.attachments.map(&:filename)).to_not include('logo.png') end - it 'renders the tables in HTML and attaches them as CSVs', aggregate_failures: true do + it 'renders the tables in HTML', aggregate_failures: true do doc = Nokogiri::HTML(mail.html_part.body.to_s) - expect(doc.css('h2').map(&:text)).to eq(['Table 1', 'Custom Table 2', 'Custom Table 3']) + expect(doc.css('h2').map(&:text)). + to eq(['Table 1', 'Custom Table 2', 'Custom Table 3 With Very Long Name']) _first_table, percent_table, float_table = doc.css('table') @@ -106,5 +126,53 @@ big_int_cell = float_table.at_css('tbody tr:nth-child(1) td:nth-child(2)') expect(big_int_cell.text.strip).to eq('100,000,000') end + + context 'with attachment_format: :csv' do + let(:attachment_format) { :csv } + + it 'renders each table as a separate CSV', aggregate_failures: true do + expect(mail.attachments.map(&:filename)).to eq( + [ + 'table-1.csv', + 'custom-table-2.csv', + 'custom-table-3-with-very-long-name.csv', + ], + ) + + tables = mail.attachments.map { |a| CSV.parse(a.read) } + + expect(tables).to eq( + [first_table, second_table, third_table]. + map { |table| table.map { |row| row.map(&:to_s) } }, + ) + end + end + + context 'with attachment_format: :xlsx' do + let(:attachment_format) { :xlsx } + + it 'combines all the tables into one .xlsx', aggregate_failures: true do + expect(mail.attachments.size).to eq(1) + + attachment = mail.attachments.first + expect(attachment.filename).to eq('report.xlsx') + + xlsx = SimpleXlsxReader.parse(attachment.read) + expect(xlsx.sheets.map(&:name)).to( + eq(['Table 1', 'Custom Table 2', 'Custom Table 3 With Very Long N']), + 'truncates sheet names to fit within 31-byte XLSX limit', + ) + + expect(xlsx.sheets.first.rows.to_a).to eq(first_table) + end + end + + context 'another attachment format' do + let(:attachment_format) { :pdf } + + it 'throws' do + expect { mail.read }.to raise_error(ArgumentError, 'unknown attachment_format=pdf') + end + end end end