From d6706def535fe0346429185f63f399b7a52a1ee7 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Mon, 8 Mar 2021 23:24:57 +0100 Subject: [PATCH 1/2] Reports: more internal refactor and include new attributes --- app/assets/javascripts/application.js | 2 +- app/decorators/member_report_decorator.rb | 12 +++++++++-- app/decorators/post_report_decorator.rb | 10 +++++++--- app/services/report/csv.rb | 14 ------------- app/services/report/csv/base.rb | 14 +++++++++++-- app/services/report/csv/member.rb | 2 +- app/services/report/csv/post.rb | 2 +- app/services/report/pdf.rb | 17 ---------------- app/services/report/pdf/base.rb | 9 +++++++-- app/services/report/pdf/member.rb | 2 +- app/services/report/pdf/post.rb | 2 +- app/views/reports/post_list.html.erb | 6 +++++- app/views/reports/user_list.html.erb | 20 +++++++++++++------ .../member_report_decorator_spec.rb | 12 +++++++++-- spec/decorators/post_report_decorator_spec.rb | 8 +++++--- 15 files changed, 75 insertions(+), 57 deletions(-) delete mode 100644 app/services/report/csv.rb delete mode 100644 app/services/report/pdf.rb diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index d2af575af..32ae40c1f 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -3,7 +3,7 @@ $(document).on('click', 'a[data-popup]', function(event) { event.preventDefault(); - window.open($(this).attr('href'), 'popup', 'width=600,height=600'); + window.open($(this).attr('href'), 'popup', 'width=800,height=600'); }); $(document).on('click', 'span.show-password', function(event) { diff --git a/app/decorators/member_report_decorator.rb b/app/decorators/member_report_decorator.rb index 0cd13fc5a..93cc9dd93 100644 --- a/app/decorators/member_report_decorator.rb +++ b/app/decorators/member_report_decorator.rb @@ -17,7 +17,11 @@ def headers User.human_attribute_name(:username), User.human_attribute_name(:email), User.human_attribute_name(:phone), - User.human_attribute_name(:alt_phone) + User.human_attribute_name(:alt_phone), + User.human_attribute_name(:created_at), + User.human_attribute_name(:last_sign_in_at), + User.human_attribute_name(:locale), + Account.human_attribute_name(:balance) ] end @@ -28,7 +32,11 @@ def rows member.user.username, member.user.email_if_real, member.user.phone, - member.user.alt_phone + member.user.alt_phone, + member.user.created_at.to_s, + member.user.last_sign_in_at.to_s, + member.user.locale, + member.account_balance.to_s ] end end diff --git a/app/decorators/post_report_decorator.rb b/app/decorators/post_report_decorator.rb index 3ebb3985f..ba64f3a61 100644 --- a/app/decorators/post_report_decorator.rb +++ b/app/decorators/post_report_decorator.rb @@ -16,7 +16,9 @@ def headers [ "", @type.model_name.human, - User.model_name.human + Post.human_attribute_name(:tag_list), + User.model_name.human, + Post.human_attribute_name(:created_at) ] end @@ -24,13 +26,15 @@ def rows grouped_rows = [] @collection.each do |category, posts| - grouped_rows << ["", category.try(:name) || "-", ""] + grouped_rows << ["", category.try(:name) || "-", "", "", ""] posts.each do |post| grouped_rows << [ post.id, post.title, - "#{post.user} (#{post.member_uid})" + post.tag_list.to_s, + "#{post.user} (#{post.member_uid})", + post.created_at.to_s ] end end diff --git a/app/services/report/csv.rb b/app/services/report/csv.rb deleted file mode 100644 index bbfa4c963..000000000 --- a/app/services/report/csv.rb +++ /dev/null @@ -1,14 +0,0 @@ -require "csv" - -module Report - module Csv - def self.run(headers, rows) - ::CSV.generate do |csv| - csv << headers - rows.each do |row| - csv << row - end - end - end - end -end diff --git a/app/services/report/csv/base.rb b/app/services/report/csv/base.rb index 4e9bedd45..b10452491 100644 --- a/app/services/report/csv/base.rb +++ b/app/services/report/csv/base.rb @@ -1,8 +1,12 @@ +require "csv" + module Report module Csv class Base + attr_accessor :decorator + def name - @decorator.name(:csv) + decorator.name(:csv) end def mime_type @@ -10,7 +14,13 @@ def mime_type end def run - Report::Csv.run(@decorator.headers, @decorator.rows) + ::CSV.generate do |csv| + csv << decorator.headers + + decorator.rows.each do |row| + csv << row + end + end end end end diff --git a/app/services/report/csv/member.rb b/app/services/report/csv/member.rb index d1f823d7b..83880ece2 100644 --- a/app/services/report/csv/member.rb +++ b/app/services/report/csv/member.rb @@ -2,7 +2,7 @@ module Report module Csv class Member < Base def initialize(org, collection) - @decorator = MemberReportDecorator.new(org, collection) + self.decorator = MemberReportDecorator.new(org, collection) end end end diff --git a/app/services/report/csv/post.rb b/app/services/report/csv/post.rb index c7bda2483..99dc78065 100644 --- a/app/services/report/csv/post.rb +++ b/app/services/report/csv/post.rb @@ -2,7 +2,7 @@ module Report module Csv class Post < Base def initialize(org, collection, type) - @decorator = PostReportDecorator.new(org, collection, type) + self.decorator = PostReportDecorator.new(org, collection, type) end end end diff --git a/app/services/report/pdf.rb b/app/services/report/pdf.rb deleted file mode 100644 index 1bf8f88f3..000000000 --- a/app/services/report/pdf.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Report - module Pdf - def self.run(headers, rows) - pdf = Prawn::Document.new(options) - pdf.table [headers] + rows - - pdf.render - end - - def self.options - { - page_size: "A4", - margin: 30 - } - end - end -end diff --git a/app/services/report/pdf/base.rb b/app/services/report/pdf/base.rb index db4601e4b..8401533e7 100644 --- a/app/services/report/pdf/base.rb +++ b/app/services/report/pdf/base.rb @@ -1,8 +1,10 @@ module Report module Pdf class Base + attr_accessor :decorator + def name - @decorator.name(:pdf) + decorator.name(:pdf) end def mime_type @@ -10,7 +12,10 @@ def mime_type end def run - Report::Pdf.run(@decorator.headers, @decorator.rows) + pdf = Prawn::Document.new({ page_size: "A4", margin: 30 }) + pdf.table [decorator.headers] + decorator.rows + + pdf.render end end end diff --git a/app/services/report/pdf/member.rb b/app/services/report/pdf/member.rb index 8f35969bc..650c2184a 100644 --- a/app/services/report/pdf/member.rb +++ b/app/services/report/pdf/member.rb @@ -2,7 +2,7 @@ module Report module Pdf class Member < Base def initialize(org, collection) - @decorator = MemberReportDecorator.new(org, collection) + self.decorator = MemberReportDecorator.new(org, collection) end end end diff --git a/app/services/report/pdf/post.rb b/app/services/report/pdf/post.rb index ed1f04fc9..1232ee3e4 100644 --- a/app/services/report/pdf/post.rb +++ b/app/services/report/pdf/post.rb @@ -2,7 +2,7 @@ module Report module Pdf class Post < Base def initialize(org, collection, type) - @decorator = PostReportDecorator.new(org, collection, type) + self.decorator = PostReportDecorator.new(org, collection, type) end end end diff --git a/app/views/reports/post_list.html.erb b/app/views/reports/post_list.html.erb index 6d656dbe2..ead0a6e8a 100644 --- a/app/views/reports/post_list.html.erb +++ b/app/views/reports/post_list.html.erb @@ -4,19 +4,23 @@ <%= @post_type.model_name.human %> + <%= Post.human_attribute_name(:tag_list) %> <%= User.model_name.human %> + <%= Post.human_attribute_name :created_at %> <% @posts.each do |cat, ps| %> -

<%= cat || '—' %>

+

<%= cat || '—' %>

<% ps.each do |post| %> <%= post.id %> <%= post.title %> + <%= post.tag_list %> <%= "#{post.user} (#{post.member_uid})" %> + <%= l(post.created_at, format: :long) %> <% end %> <% end %> diff --git a/app/views/reports/user_list.html.erb b/app/views/reports/user_list.html.erb index ef533012c..027509630 100644 --- a/app/views/reports/user_list.html.erb +++ b/app/views/reports/user_list.html.erb @@ -7,16 +7,24 @@ <%= User.human_attribute_name :email %> <%= User.human_attribute_name :phone %> <%= User.human_attribute_name :alt_phone %> + <%= User.human_attribute_name :created_at %> + <%= User.human_attribute_name :last_sign_in_at %> + <%= User.human_attribute_name :locale %> + <%= Account.human_attribute_name :balance %> - <% @members.each do |u| %> + <% @members.each do |member| %> - <%= u.member_uid %> - <%= u.user.username %> - <%= u.user.email_if_real %> - <%= u.user.phone %> - <%= u.user.alt_phone %> + <%= member.member_uid %> + <%= member.user.username %> + <%= member.user.email_if_real %> + <%= member.user.phone %> + <%= member.user.alt_phone %> + <%= l(member.user.created_at, format: :long) %> + <%= l(member.user.last_sign_in_at, format: :long) if member.user.last_sign_in_at.present? %> + <%= member.user.locale %> + <%= member.account_balance %> <% end %> diff --git a/spec/decorators/member_report_decorator_spec.rb b/spec/decorators/member_report_decorator_spec.rb index 67a0a2a6b..0d4a996da 100644 --- a/spec/decorators/member_report_decorator_spec.rb +++ b/spec/decorators/member_report_decorator_spec.rb @@ -19,7 +19,11 @@ User.human_attribute_name(:username), User.human_attribute_name(:email), User.human_attribute_name(:phone), - User.human_attribute_name(:alt_phone) + User.human_attribute_name(:alt_phone), + User.human_attribute_name(:created_at), + User.human_attribute_name(:last_sign_in_at), + User.human_attribute_name(:locale), + Account.human_attribute_name(:balance) ]) end @@ -30,7 +34,11 @@ member.user.username, member.user.email_if_real, member.user.phone, - member.user.alt_phone + member.user.alt_phone, + member.user.created_at.to_s, + member.user.last_sign_in_at.to_s, + member.user.locale, + member.account_balance.to_s ] ]) end diff --git a/spec/decorators/post_report_decorator_spec.rb b/spec/decorators/post_report_decorator_spec.rb index f3d429d9d..9c1409e31 100644 --- a/spec/decorators/post_report_decorator_spec.rb +++ b/spec/decorators/post_report_decorator_spec.rb @@ -26,7 +26,9 @@ expect(decorator.headers).to eq([ "", Offer.model_name.human, - User.model_name.human + Post.human_attribute_name(:tag_list), + User.model_name.human, + Post.human_attribute_name(:created_at) ]) end @@ -35,8 +37,8 @@ offer = org.offers.of_active_members.active.first expect(decorator.rows).to eq([ - ["", category.name, ""], - [offer.id, offer.title, "#{offer.user} (#{offer.member_uid})"] + ["", category.name, "", "", ""], + [offer.id, offer.title, offer.tag_list.to_s, "#{offer.user} (#{offer.member_uid})", offer.created_at.to_s] ]) end end From 16c4e44f31be5ca0aaa69ea6ccd44f7eb2764833 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Mon, 8 Mar 2021 23:57:20 +0100 Subject: [PATCH 2/2] more reports controller specs --- Gemfile.lock | 14 +++++++------- app/views/reports/post_list.html.erb | 2 +- spec/controllers/reports_controller_spec.rb | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 027b1a854..e95d62ae3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -133,7 +133,7 @@ GEM fabrication (2.21.1) faker (2.16.0) i18n (>= 1.6, < 2) - ffi (1.14.2) + ffi (1.15.0) formtastic (4.0.0) actionpack (>= 5.2.0) formtastic_i18n (0.6.0) @@ -199,16 +199,16 @@ GEM method_source (1.0.0) mime-types (3.3.1) mime-types-data (~> 3.2015) - mime-types-data (3.2021.0212) + mime-types-data (3.2021.0225) mimemagic (0.3.5) mini_mime (1.0.2) mini_portile2 (2.5.0) - minitest (5.14.3) + minitest (5.14.4) net-scp (3.0.0) net-ssh (>= 2.6.5, < 7.0.0) net-ssh (6.1.0) netrc (0.11.0) - nio4r (2.5.5) + nio4r (2.5.7) nokogiri (1.11.1) mini_portile2 (~> 2.5.0) racc (~> 1.4) @@ -279,7 +279,7 @@ GEM ffi (~> 1.0) rdiscount (2.2.0.2) redis (4.2.5) - regexp_parser (2.0.3) + regexp_parser (2.1.1) responders (3.0.1) actionpack (>= 5.0) railties (>= 5.0) @@ -307,7 +307,7 @@ GEM rspec-mocks (~> 3.10) rspec-support (~> 3.10) rspec-support (3.10.2) - rubocop (1.10.0) + rubocop (1.11.0) parallel (~> 1.10) parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) @@ -390,7 +390,7 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webdrivers (4.5.0) + webdrivers (4.6.0) nokogiri (~> 1.6) rubyzip (>= 1.3.0) selenium-webdriver (>= 3.0, < 4.0) diff --git a/app/views/reports/post_list.html.erb b/app/views/reports/post_list.html.erb index ead0a6e8a..39e98c320 100644 --- a/app/views/reports/post_list.html.erb +++ b/app/views/reports/post_list.html.erb @@ -4,7 +4,7 @@ <%= @post_type.model_name.human %> - <%= Post.human_attribute_name(:tag_list) %> + <%= Post.human_attribute_name :tag_list %> <%= User.model_name.human %> <%= Post.human_attribute_name :created_at %> diff --git a/spec/controllers/reports_controller_spec.rb b/spec/controllers/reports_controller_spec.rb index 0ba68b839..82f59ca51 100644 --- a/spec/controllers/reports_controller_spec.rb +++ b/spec/controllers/reports_controller_spec.rb @@ -45,6 +45,8 @@ end describe 'GET #post_list' do + let(:report_posts) { test_organization.posts.of_active_members.group_by(&:category) } + it 'do NOT show the inactive members' do get :post_list, params: { type: 'offer' } @@ -52,6 +54,22 @@ expect(posts.size).to eq(active_organization_offers.size) expect(posts.map(&:id)).to match_array(active_organization_offers.map(&:id)) end + + it 'downloads a csv' do + get :post_list, params: { type: 'offer', format: 'csv' } + + report = Report::Csv::Post.new(test_organization, report_posts, Offer) + expect(response.body).to eq(report.run) + expect(response.media_type).to eq("text/csv") + end + + it 'downloads a pdf' do + get :post_list, params: { type: 'offer', format: 'pdf' } + + report = Report::Pdf::Post.new(test_organization, report_posts, Offer) + expect(response.body).to eq(report.run) + expect(response.media_type).to eq("application/pdf") + end end end end