From a8078b22f889724684d0a65e4f596384d70b61ed Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 13 Mar 2020 10:11:46 +0000 Subject: [PATCH 1/8] Move enterprise fees summaries controller and views to ordermanagement engine --- .../enterprise_fee_summaries_controller.rb | 66 ------------------- .../enterprise_fee_summaries_controller.rb | 64 ++++++++++++++++++ .../_filters.html.haml | 0 .../_report.html.haml | 0 .../enterprise_fee_summaries/create.html.haml | 0 .../enterprise_fee_summaries/new.html.haml | 0 ...nterprise_fee_summaries_controller_spec.rb | 4 +- 7 files changed, 66 insertions(+), 68 deletions(-) delete mode 100644 app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb create mode 100644 engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb rename {app/views/spree/admin => engines/order_management/app/views/order_management}/reports/enterprise_fee_summaries/_filters.html.haml (100%) rename {app/views/spree/admin => engines/order_management/app/views/order_management}/reports/enterprise_fee_summaries/_report.html.haml (100%) rename {app/views/spree/admin => engines/order_management/app/views/order_management}/reports/enterprise_fee_summaries/create.html.haml (100%) rename {app/views/spree/admin => engines/order_management/app/views/order_management}/reports/enterprise_fee_summaries/new.html.haml (100%) rename {spec/controllers/spree/admin => engines/order_management/spec/controllers/order_management}/reports/enterprise_fee_summaries_controller_spec.rb (94%) diff --git a/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb b/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb deleted file mode 100644 index e6b750b904..0000000000 --- a/app/controllers/spree/admin/reports/enterprise_fee_summaries_controller.rb +++ /dev/null @@ -1,66 +0,0 @@ -module Spree - module Admin - module Reports - class EnterpriseFeeSummariesController < BaseController - before_filter :load_report_parameters - before_filter :load_permissions - - def new; end - - def create - return respond_to_invalid_parameters unless @report_parameters.valid? - - @report_parameters.authorize!(@permissions) - - @report = report_klass::ReportService.new(@permissions, @report_parameters) - renderer.render(self) - rescue ::Reports::Authorizer::ParameterNotAllowedError => e - flash[:error] = e.message - render_report_form - end - - private - - def respond_to_invalid_parameters - flash[:error] = I18n.t("invalid_filter_parameters", scope: i18n_scope) - render_report_form - end - - def i18n_scope - "order_management.reports.enterprise_fee_summary" - end - - def render_report_form - render action: :new - end - - def report_klass - OrderManagement::Reports::EnterpriseFeeSummary - end - - def load_report_parameters - @report_parameters = report_klass::Parameters.new(params[:report] || {}) - end - - def load_permissions - @permissions = report_klass::Permissions.new(spree_current_user) - end - - def report_renderer_klass - case params[:report_format] - when "csv" - report_klass::Renderers::CsvRenderer - when nil, "", "html" - report_klass::Renderers::HtmlRenderer - else - raise Reports::UnsupportedReportFormatException - end - end - - def renderer - @renderer ||= report_renderer_klass.new(@report) - end - end - end - end -end diff --git a/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb b/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb new file mode 100644 index 0000000000..08b6c602c5 --- /dev/null +++ b/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb @@ -0,0 +1,64 @@ +module OrderManagement + module Reports + class EnterpriseFeeSummariesController < Spree::Admin::BaseController + before_filter :load_report_parameters + before_filter :load_permissions + + def new; end + + def create + return respond_to_invalid_parameters unless @report_parameters.valid? + + @report_parameters.authorize!(@permissions) + + @report = report_klass::ReportService.new(@permissions, @report_parameters) + renderer.render(self) + rescue ::Reports::Authorizer::ParameterNotAllowedError => e + flash[:error] = e.message + render_report_form + end + + private + + def respond_to_invalid_parameters + flash[:error] = I18n.t("invalid_filter_parameters", scope: i18n_scope) + render_report_form + end + + def i18n_scope + "order_management.reports.enterprise_fee_summary" + end + + def render_report_form + render action: :new + end + + def report_klass + OrderManagement::Reports::EnterpriseFeeSummary + end + + def load_report_parameters + @report_parameters = report_klass::Parameters.new(params[:report] || {}) + end + + def load_permissions + @permissions = report_klass::Permissions.new(spree_current_user) + end + + def report_renderer_klass + case params[:report_format] + when "csv" + report_klass::Renderers::CsvRenderer + when nil, "", "html" + report_klass::Renderers::HtmlRenderer + else + raise 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/_filters.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml similarity index 100% rename from app/views/spree/admin/reports/enterprise_fee_summaries/_filters.html.haml rename to engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml diff --git a/app/views/spree/admin/reports/enterprise_fee_summaries/_report.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_report.html.haml similarity index 100% rename from app/views/spree/admin/reports/enterprise_fee_summaries/_report.html.haml rename to engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_report.html.haml diff --git a/app/views/spree/admin/reports/enterprise_fee_summaries/create.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/create.html.haml similarity index 100% rename from app/views/spree/admin/reports/enterprise_fee_summaries/create.html.haml rename to engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/create.html.haml diff --git a/app/views/spree/admin/reports/enterprise_fee_summaries/new.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/new.html.haml similarity index 100% rename from app/views/spree/admin/reports/enterprise_fee_summaries/new.html.haml rename to engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/new.html.haml diff --git a/spec/controllers/spree/admin/reports/enterprise_fee_summaries_controller_spec.rb b/engines/order_management/spec/controllers/order_management/reports/enterprise_fee_summaries_controller_spec.rb similarity index 94% rename from spec/controllers/spree/admin/reports/enterprise_fee_summaries_controller_spec.rb rename to engines/order_management/spec/controllers/order_management/reports/enterprise_fee_summaries_controller_spec.rb index d9d8962f9b..3282bd7258 100644 --- a/spec/controllers/spree/admin/reports/enterprise_fee_summaries_controller_spec.rb +++ b/engines/order_management/spec/controllers/order_management/reports/enterprise_fee_summaries_controller_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Spree::Admin::Reports::EnterpriseFeeSummariesController, type: :controller do +describe OrderManagement::Reports::EnterpriseFeeSummariesController, type: :controller do let(:report_klass) { OrderManagement::Reports::EnterpriseFeeSummary } let!(:distributor) { create(:distributor_enterprise) } @@ -76,6 +76,6 @@ describe Spree::Admin::Reports::EnterpriseFeeSummariesController, type: :control end def new_template_path - "spree/admin/reports/enterprise_fee_summaries/new" + "order_management/reports/enterprise_fee_summaries/new" end end From e209452f8b06c577ba223258a9afbf00f94d2337 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 13 Mar 2020 10:13:37 +0000 Subject: [PATCH 2/8] Make engine routes just prepend to apps routes instead of creating engine routes This makes things a bit simpler in terms of routing, we avoid a problem running specs and we can still have the engine routes separated in specific files --- config/routes.rb | 5 ----- engines/order_management/config/routes.rb | 4 ++-- engines/web/config/routes.rb | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index ae3c131ac3..dcb828b3be 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -89,11 +89,6 @@ Openfoodnetwork::Application.routes.draw do get 'sitemap.xml', to: 'sitemap#index', defaults: { format: 'xml' } - # Mount engine routes - mount Web::Engine, :at => '/' - mount Catalog::Engine, :at => '/' - mount OrderManagement::Engine, :at => '/' - # Mount Spree's routes mount Spree::Core::Engine, :at => '/' end diff --git a/engines/order_management/config/routes.rb b/engines/order_management/config/routes.rb index e221867e7e..79706c635a 100644 --- a/engines/order_management/config/routes.rb +++ b/engines/order_management/config/routes.rb @@ -1,5 +1,5 @@ -Spree::Core::Engine.routes.prepend do - namespace :admin do +Openfoodnetwork::Application.routes.prepend do + namespace :order_management do namespace :reports do resource :enterprise_fee_summary, only: [:new, :create] end diff --git a/engines/web/config/routes.rb b/engines/web/config/routes.rb index 121f3bdd9a..9126f5f621 100644 --- a/engines/web/config/routes.rb +++ b/engines/web/config/routes.rb @@ -1,4 +1,4 @@ -Web::Engine.routes.draw do +Openfoodnetwork::Application.routes.prepend do namespace :api do scope '/cookies' do resource :consent, only: [:show, :create, :destroy], controller: "cookies_consent" From 0b05312f1940055a3881079a05bc0999c7205389 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 13 Mar 2020 10:37:59 +0000 Subject: [PATCH 3/8] Move cookies spec to web engine and adapt routes to the fact they are now normal main apps routes --- engines/web/config/routes.rb | 8 +++----- .../web/spec}/features/consumer/cookies_spec.rb | 0 2 files changed, 3 insertions(+), 5 deletions(-) rename {spec => engines/web/spec}/features/consumer/cookies_spec.rb (100%) diff --git a/engines/web/config/routes.rb b/engines/web/config/routes.rb index 9126f5f621..0b56f00a6d 100644 --- a/engines/web/config/routes.rb +++ b/engines/web/config/routes.rb @@ -1,9 +1,7 @@ Openfoodnetwork::Application.routes.prepend do - namespace :api do - scope '/cookies' do - resource :consent, only: [:show, :create, :destroy], controller: "cookies_consent" - end + scope '/api/cookies' do + resource :consent, only: [:show, :create, :destroy], controller: "web/api/cookies_consent" end - get "/angular-templates/:id", to: "angular_templates#show", constraints: { name: %r{[\/\w\.]+} } + get "/angular-templates/:id", to: "web/angular_templates#show", constraints: { name: %r{[\/\w\.]+} } end diff --git a/spec/features/consumer/cookies_spec.rb b/engines/web/spec/features/consumer/cookies_spec.rb similarity index 100% rename from spec/features/consumer/cookies_spec.rb rename to engines/web/spec/features/consumer/cookies_spec.rb From 58465c4645594818ec2cfbd8d11f35f080b5148c Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 16 Mar 2020 20:19:04 +0000 Subject: [PATCH 4/8] Adapt routes placeholder in the new catalog engine to make it similar to the other engines --- engines/catalog/config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/catalog/config/routes.rb b/engines/catalog/config/routes.rb index a56f4a0b70..166898de46 100644 --- a/engines/catalog/config/routes.rb +++ b/engines/catalog/config/routes.rb @@ -1,2 +1,2 @@ -Catalog::Engine.routes.draw do +Openfoodnetwork::Application.routes.prepend do end From b4befea60651e4894d7938f1cdda43a528f73d8e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 16 Mar 2020 23:44:32 +0000 Subject: [PATCH 5/8] Fix namespace in spec --- .../enterprise_fee_summary/renderers/html_renderer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb index 5b9c62b03c..1bd591e021 100644 --- a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb +++ b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb @@ -5,7 +5,7 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::Renderers::HtmlRenderer let!(:permissions) { report_klass::Permissions.new(current_user) } let!(:parameters) { report_klass::Parameters.new } - let!(:controller) { Spree::Admin::Reports::EnterpriseFeeSummariesController.new } + let!(:controller) { OrderManagement::Reports::EnterpriseFeeSummariesController.new } let!(:service) { report_klass::ReportService.new(permissions, parameters) } let!(:renderer) { described_class.new(service) } From 3f5a964dec6d0d3add6292cb74227d7674f51e44 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 18 Mar 2020 12:09:12 +0000 Subject: [PATCH 6/8] Move enterprise_fee_summaries_spec to order_management engine, moving translation keys for the views and adapting some routes --- config/locales/en.yml | 16 ++++++++-------- .../enterprise_fee_summaries/_filters.html.haml | 2 +- .../enterprise_fee_summaries/_report.html.haml | 2 +- .../reports/enterprise_fee_summaries_spec.rb | 10 +++++----- 4 files changed, 15 insertions(+), 15 deletions(-) rename {spec/features/admin => engines/order_management/spec/features/order_management}/reports/enterprise_fee_summaries_spec.rb (93%) diff --git a/config/locales/en.yml b/config/locales/en.yml index 804ccbbcef..e407732f70 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2816,6 +2816,14 @@ See the %{link} to find out more about %{sitename}'s features and to start using order_management: reports: + enterprise_fee_summaries: + filters: + date_range: "Date Range" + report_format_csv: "Download as CSV" + generate_report: "Generate Report" + report: + none: "None" + select_and_search: "Select filters and click on GENERATE REPORT to access your data." enterprise_fee_summary: date_end_before_start_error: "must be after start" parameter_not_allowed_error: "You are not authorized to use one or more selected filters for this report." @@ -3318,14 +3326,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using bulk_coop_allocation: 'Bulk Co-op - Allocation' bulk_coop_packing_sheets: 'Bulk Co-op - Packing Sheets' bulk_coop_customer_payments: 'Bulk Co-op - Customer Payments' - enterprise_fee_summaries: - filters: - date_range: "Date Range" - report_format_csv: "Download as CSV" - generate_report: "Generate Report" - report: - none: "None" - select_and_search: "Select filters and click on GENERATE REPORT to access your data." users: index: listing_users: "Listing Users" diff --git a/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml index 744097e9a5..0aee85d71e 100644 --- a/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml +++ b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_filters.html.haml @@ -1,4 +1,4 @@ -= form_for @report_parameters, as: :report, url: spree.admin_reports_enterprise_fee_summary_path, method: :post do |f| += form_for @report_parameters, as: :report, url: main_app.order_management_reports_enterprise_fee_summary_path, method: :post do |f| .row.date-range-filter .sixteen.columns.alpha = label_tag nil, t(".date_range") diff --git a/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_report.html.haml b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_report.html.haml index 4427c511ba..332253b143 100644 --- a/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_report.html.haml +++ b/engines/order_management/app/views/order_management/reports/enterprise_fee_summaries/_report.html.haml @@ -13,7 +13,7 @@ - if @renderer.data_rows.empty? %tr - %td{colspan: @renderer.header.length}= t(:none) + %td{colspan: @renderer.header.length}= t('.none') - else %p.report__message = t(".select_and_search") diff --git a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb b/engines/order_management/spec/features/order_management/reports/enterprise_fee_summaries_spec.rb similarity index 93% rename from spec/features/admin/reports/enterprise_fee_summaries_spec.rb rename to engines/order_management/spec/features/order_management/reports/enterprise_fee_summaries_spec.rb index 57a8de346e..7930ea75a3 100644 --- a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb +++ b/engines/order_management/spec/features/order_management/reports/enterprise_fee_summaries_spec.rb @@ -41,7 +41,7 @@ feature "enterprise fee summaries", js: true do it "does not allow access to the report" do visit spree.admin_reports_path expect(page).to have_no_link(I18n.t("admin.reports.enterprise_fee_summary.name")) - visit spree.new_admin_reports_enterprise_fee_summary_path + visit main_app.new_order_management_reports_enterprise_fee_summary_path expect(page).to have_content(I18n.t("unauthorized")) end end @@ -49,7 +49,7 @@ feature "enterprise fee summaries", js: true do describe "smoke test for filters" do before do - visit spree.new_admin_reports_enterprise_fee_summary_path + visit main_app.new_order_management_reports_enterprise_fee_summary_path end context "when logged in as admin" do @@ -80,7 +80,7 @@ feature "enterprise fee summaries", js: true do describe "smoke test for generation of report based on permissions" do before do - visit spree.new_admin_reports_enterprise_fee_summary_path + visit main_app.new_order_management_reports_enterprise_fee_summary_path end context "when logged in as admin" do @@ -138,7 +138,7 @@ feature "enterprise fee summaries", js: true do let(:current_user) { create(:admin_user) } before do - visit spree.new_admin_reports_enterprise_fee_summary_path + visit main_app.new_order_management_reports_enterprise_fee_summary_path end it "generates file with data for selected order cycle" do @@ -155,6 +155,6 @@ feature "enterprise fee summaries", js: true do end def i18n_scope - "spree.admin.reports.enterprise_fee_summaries" + "order_management.reports.enterprise_fee_summaries" end end From 9994bc75ca929f0c54835a40d406bac6ed54298a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 18 Mar 2020 12:11:27 +0000 Subject: [PATCH 7/8] Adapt reports controller to handle routes of reports in the order_management engine differently --- app/controllers/spree/admin/reports_controller.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/reports_controller.rb b/app/controllers/spree/admin/reports_controller.rb index ec08bca29c..9583fe7134 100644 --- a/app/controllers/spree/admin/reports_controller.rb +++ b/app/controllers/spree/admin/reports_controller.rb @@ -298,11 +298,20 @@ module Spree end def url_for_report(report) - public_send("#{report}_admin_reports_url".to_sym) + if report_in_order_management_engine?(report) + main_app.public_send("new_order_management_reports_#{report}_url".to_sym) + else + public_send("#{report}_admin_reports_url".to_sym) + end rescue NoMethodError url_for([:new, :admin, :reports, report.to_s.singularize]) end + # List of reports that have been moved to the Order Management engine + def report_in_order_management_engine?(report) + report == :enterprise_fee_summary + end + def timestamp Time.zone.now.strftime("%Y%m%d") end From e014e6c1a4f9e9625813d8850daec2168b637acf Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 23 Mar 2020 09:12:42 +0100 Subject: [PATCH 8/8] Ensure perform_deliveries is correctly set when testing user confirmation emails --- spec/models/spree/user_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index d73042c67a..7ed818fee2 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -76,9 +76,11 @@ describe Spree.user_class do it "should send a confirmation email" do setup_email - expect do - create(:user, email: 'new_user@example.com', confirmation_sent_at: nil, confirmed_at: nil) - end.to send_confirmation_instructions + performing_deliveries do + expect do + create(:user, email: 'new_user@example.com', confirmation_sent_at: nil, confirmed_at: nil) + end.to send_confirmation_instructions + end sent_mail = ActionMailer::Base.deliveries.last expect(sent_mail.to).to eq ['new_user@example.com']