From 9e035efd50eccfa2cea3a514f1a29eae15db9176 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 6 Dec 2018 09:37:58 +0800 Subject: [PATCH] Render enterprise fee report directly in renderer --- .../enterprise_fee_summaries_controller.rb | 18 ++++++----------- .../_report.html.haml | 8 ++++---- .../renderers/csv_renderer.rb | 11 +++++----- .../renderers/html_renderer.rb | 5 +++++ .../enterprise_fee_summary/report_service.rb | 11 ++-------- .../renderers/csv_renderer_spec.rb | 20 +++++++++++++++---- .../renderers/html_renderer_spec.rb | 10 ++++++---- .../report_service_spec.rb | 6 +++--- .../reports/renderers/base.rb | 6 +----- .../reports/renderers/independent_file.rb | 11 ---------- 10 files changed, 49 insertions(+), 57 deletions(-) delete mode 100644 lib/open_food_network/reports/renderers/independent_file.rb diff --git a/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb b/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb index 7548c962b5..04346efa7a 100644 --- a/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb +++ b/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb @@ -20,9 +20,8 @@ module Spree @authorizer = report_klass::Authorizer.new(@report_parameters, @permissions) @authorizer.authorize! - @report = report_klass::ReportService.new(@permissions, @report_parameters, - report_renderer_klass) - render_report + @report = report_klass::ReportService.new(@permissions, @report_parameters) + renderer.render(self) rescue OpenFoodNetwork::Reports::Authorizer::ParameterNotAllowedError => e flash[:error] = e.message render_report_form @@ -55,15 +54,6 @@ module Spree @permissions = report_klass::Permissions.new(spree_current_user) end - def render_report - return render_html_report unless @report.renderer.independent_file? - send_data(@report.render, filename: @report.filename) - end - - def render_html_report - render action: :index - end - def report_renderer_klass case params[:report_format] when "csv" @@ -74,6 +64,10 @@ module Spree raise OpenFoodNetwork::Reports::UnsupportedReportFormatException end end + + def renderer + @renderer ||= report_renderer_klass.new(@report) + end end end end diff --git a/app/views/spree/admin/reports/enterprise_fee_summaries/_report.html.haml b/app/views/spree/admin/reports/enterprise_fee_summaries/_report.html.haml index 2877778d08..4427c511ba 100644 --- a/app/views/spree/admin/reports/enterprise_fee_summaries/_report.html.haml +++ b/app/views/spree/admin/reports/enterprise_fee_summaries/_report.html.haml @@ -2,18 +2,18 @@ %table#enterprise_fee_summary_report.report__table %thead %tr - - @report.renderer.header.each do |heading| + - @renderer.header.each do |heading| %th= heading %tbody - - @report.renderer.data_rows.each do |row| + - @renderer.data_rows.each do |row| %tr - row.each do |cell_value| %td= cell_value - - if @report.renderer.data_rows.empty? + - if @renderer.data_rows.empty? %tr - %td{colspan: @report.renderer.header.length}= t(:none) + %td{colspan: @renderer.header.length}= t(:none) - else %p.report__message = t(".select_and_search") diff --git a/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer.rb b/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer.rb index 9af42fbfc4..7479d940fd 100644 --- a/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer.rb +++ b/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer.rb @@ -1,14 +1,15 @@ require "open_food_network/reports/renderers/base" -require "open_food_network/reports/renderers/independent_file" module OrderManagement module Reports module EnterpriseFeeSummary module Renderers class CsvRenderer < OpenFoodNetwork::Reports::Renderers::Base - include OpenFoodNetwork::Reports::Renderers::IndependentFile + def render(context) + context.send_data(generate, filename: filename) + end - def render + def generate CSV.generate do |csv| render_header(csv) @@ -18,13 +19,13 @@ module OrderManagement end end + private + def filename timestamp = Time.zone.now.strftime("%Y%m%d") "enterprise_fee_summary_#{timestamp}.csv" end - private - def render_header(csv) csv << [ header_label(:fee_type), diff --git a/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer.rb b/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer.rb index 1a280ba667..fd54febc2e 100644 --- a/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer.rb +++ b/engines/order_management/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer.rb @@ -5,6 +5,11 @@ module OrderManagement module EnterpriseFeeSummary module Renderers class HtmlRenderer < OpenFoodNetwork::Reports::Renderers::Base + def render(context) + context.instance_variable_set :@renderer, self + context.render(action: :create, renderer: self) + end + def header data_row_attributes.map do |attribute| header_label(attribute) diff --git a/engines/order_management/lib/order_management/reports/enterprise_fee_summary/report_service.rb b/engines/order_management/lib/order_management/reports/enterprise_fee_summary/report_service.rb index d2b86d97e9..c40cffd308 100644 --- a/engines/order_management/lib/order_management/reports/enterprise_fee_summary/report_service.rb +++ b/engines/order_management/lib/order_management/reports/enterprise_fee_summary/report_service.rb @@ -7,14 +7,11 @@ module OrderManagement module Reports module EnterpriseFeeSummary class ReportService - delegate :render, :filename, to: :renderer + attr_accessor :permissions, :parameters - attr_accessor :permissions, :parameters, :renderer_klass - - def initialize(permissions, parameters, renderer_klass) + def initialize(permissions, parameters) @permissions = permissions @parameters = parameters - @renderer_klass = renderer_klass end def enterprise_fees_by_customer @@ -25,10 +22,6 @@ module OrderManagement ReportData::EnterpriseFeeTypeTotals.new(list: enterprise_fee_type_total_list.sort) end - def renderer - @renderer ||= renderer_klass.new(self) - end - private def permission_filters diff --git a/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer_spec.rb b/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer_spec.rb index 192e165bd9..13823d61b2 100644 --- a/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer_spec.rb +++ b/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/csv_renderer_spec.rb @@ -10,7 +10,17 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::CsvRenderer let!(:permissions) { report_klass::Permissions.new(current_user) } let!(:parameters) { report_klass::Parameters.new } - let!(:service) { report_klass::ReportService.new(permissions, parameters, described_class) } + let!(:service) { report_klass::ReportService.new(permissions, parameters) } + let!(:renderer) { described_class.new(service) } + + # Context which will be passed to the renderer. The response object is not automatically prepared, + # so this has to be assigned explicitly. + let!(:response) { ActionController::TestResponse.new } + let!(:controller) do + ActionController::Base.new.tap do |controller_mock| + controller_mock.instance_variable_set(:@_response, response) + end + end let!(:enterprise_fee_type_totals) do instance = report_klass::ReportData::EnterpriseFeeTypeTotals.new @@ -46,7 +56,8 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::CsvRenderer end it "generates CSV header" do - result = service.render + renderer.render(controller) + result = response.body csv = CSV.parse(result) header_row = csv[0] @@ -56,7 +67,8 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::CsvRenderer end it "generates CSV data rows" do - result = service.render + renderer.render(controller) + result = response.body csv = CSV.parse(result, headers: true) expect(csv.length).to eq(2) @@ -69,7 +81,7 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::CsvRenderer it "generates filename correctly" do Timecop.freeze(Time.zone.local(2018, 10, 9, 7, 30, 0)) do - filename = service.filename + filename = renderer.__send__(:filename) expect(filename).to eq("enterprise_fee_summary_20181009.csv") end end diff --git a/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb b/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb index 39158b35cd..3cc30f6abb 100644 --- a/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb +++ b/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb @@ -10,7 +10,9 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::HtmlRenderer let!(:permissions) { report_klass::Permissions.new(current_user) } let!(:parameters) { report_klass::Parameters.new } - let!(:service) { report_klass::ReportService.new(permissions, parameters, described_class) } + let!(:controller) { Spree::Admin::Reports::EnterpriseFeeSummariesController.new } + let!(:service) { report_klass::ReportService.new(permissions, parameters) } + let!(:renderer) { described_class.new(service) } let!(:enterprise_fee_type_totals) do instance = report_klass::ReportData::EnterpriseFeeTypeTotals.new @@ -46,7 +48,7 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::HtmlRenderer end it "generates header values" do - header_row = service.renderer.header + header_row = renderer.header # Test all header cells have values expect(header_row.length).to eq(8) @@ -54,8 +56,8 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::HtmlRenderer end it "generates data rows" do - header_row = service.renderer.header - result = service.renderer.data_rows + header_row = renderer.header + result = renderer.data_rows expect(result.length).to eq(2) diff --git a/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/report_service_spec.rb b/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/report_service_spec.rb index a9c7988592..8b3d522e34 100644 --- a/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/report_service_spec.rb +++ b/engines/order_management/spec/lib/order_management/reports/enterprise_fee_summary/report_service_spec.rb @@ -86,7 +86,7 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do let(:permissions) { report_klass::Permissions.new(current_user) } let(:parameters) { report_klass::Parameters.new } - let(:service) { described_class.new(permissions, parameters, nil) } + let(:service) { described_class.new(permissions, parameters) } it "groups and sorts entries correctly" do totals = service.enterprise_fee_type_totals @@ -165,7 +165,7 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do let(:permissions) { report_klass::Permissions.new(current_user) } let(:parameters) { report_klass::Parameters.new({}) } - let(:service) { described_class.new(permissions, parameters, nil) } + let(:service) { described_class.new(permissions, parameters) } context "when admin" do let!(:current_user) { create(:admin_user) } @@ -194,7 +194,7 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do describe "filters entries correctly" do let(:permissions) { report_klass::Permissions.new(current_user) } let(:parameters) { report_klass::Parameters.new(parameters_attributes) } - let(:service) { described_class.new(permissions, parameters, nil) } + let(:service) { described_class.new(permissions, parameters) } context "filtering by completion date" do let(:timestamp) { Time.zone.local(2018, 1, 5, 14, 30, 5) } diff --git a/lib/open_food_network/reports/renderers/base.rb b/lib/open_food_network/reports/renderers/base.rb index 6a60b94c46..9720202bc7 100644 --- a/lib/open_food_network/reports/renderers/base.rb +++ b/lib/open_food_network/reports/renderers/base.rb @@ -2,15 +2,11 @@ module OpenFoodNetwork module Reports module Renderers class Base - attr_accessor :report_data + attr_reader :report_data def initialize(report_data) @report_data = report_data end - - def independent_file? - false - end end end end diff --git a/lib/open_food_network/reports/renderers/independent_file.rb b/lib/open_food_network/reports/renderers/independent_file.rb deleted file mode 100644 index 3c5322a04e..0000000000 --- a/lib/open_food_network/reports/renderers/independent_file.rb +++ /dev/null @@ -1,11 +0,0 @@ -module OpenFoodNetwork - module Reports - module Renderers - module IndependentFile - def independent_file? - true - end - end - end - end -end