From f9c0edf4b92255cb109b72836d99a9ee016898d9 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 9 Aug 2017 18:21:25 +0200 Subject: [PATCH 001/353] Always execute local karma This ensure the dev will run the version specified in the package.json. Besides, makes the rake task work as all rails tests, allowing you to pass a RAILS_ENV. --- lib/tasks/karma.rake | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/tasks/karma.rake b/lib/tasks/karma.rake index 3755dacc63..7cc245cfa4 100644 --- a/lib/tasks/karma.rake +++ b/lib/tasks/karma.rake @@ -1,28 +1,22 @@ +ENV["RAILS_ENV"] ||= 'test' + namespace :karma do - task :start => :environment do |task| - continue_only_in_test_env task + task :start => :environment do |_task| with_tmp_config :start end - task :run => :environment do |task| - continue_only_in_test_env task + task :run => :environment do |_task| with_tmp_config :start, "--single-run" end private - def continue_only_in_test_env task - if Rails.env != 'test' - raise "Task must be called in test environment:\n bundle exec rake #{task.name} RAILS_ENV=test" - end - end - def with_tmp_config(command, args = nil) Tempfile.open('karma_unit.js', Rails.root.join('tmp') ) do |f| f.write unit_js(application_spec_files << i18n_file) f.flush trap('SIGINT') { puts "Killing Karma"; exit } - exec "karma #{command} #{f.path} #{args}" + exec "node_modules/.bin/karma #{command} #{f.path} #{args}" end end From 1b151ee015e401313c5fb6502ccc8ff00ece0a07 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 1 Sep 2017 14:57:16 +0200 Subject: [PATCH 002/353] Add missing translations in registration wizard --- app/views/registration/steps/_contact.html.haml | 6 +++--- config/locales/en.yml | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/views/registration/steps/_contact.html.haml b/app/views/registration/steps/_contact.html.haml index 3b7d43ae72..169e98cc85 100644 --- a/app/views/registration/steps/_contact.html.haml +++ b/app/views/registration/steps/_contact.html.haml @@ -13,17 +13,17 @@ .row .small-12.columns.field %label{ for: 'enterprise_contact' } {{'enterprise_contact' | t}}: - %input.chunky.small-12.columns{ id: 'enterprise_contact', name: 'contact', required: true, placeholder: "Contact Name", ng: { model: 'enterprise.contact' } } + %input.chunky.small-12.columns{ id: 'enterprise_contact', name: 'contact', required: true, placeholder: t(:enterprise_contact_placeholder), ng: { model: 'enterprise.contact' } } %span.error.small-12.columns{ ng: { show: "contact.contact.$error.required && submitted" } } {{'enterprise_contact_required' | t}} .row .small-12.columns.field %label{ for: 'enterprise_email_address' } {{'enterprise_email_address' | t}}: - %input.chunky.small-12.columns{ id: 'enterprise_email_address', name: 'email_address', type: 'email', placeholder: "eg. charlie@thefarm.com", ng: { model: 'enterprise.email_address' } } + %input.chunky.small-12.columns{ id: 'enterprise_email_address', name: 'email_address', type: 'email', placeholder: t(:enterprise_email_placeholder), ng: { model: 'enterprise.email_address' } } .row .small-12.columns.field %label{ for: 'enterprise_phone' } {{'enterprise_phone' | t}}: - %input.chunky.small-12.columns{ id: 'enterprise_phone', name: 'phone', placeholder: "eg. (03) 1234 5678", ng: { model: 'enterprise.phone' } } + %input.chunky.small-12.columns{ id: 'enterprise_phone', name: 'phone', placeholder: t(:enterprise_phone_placeholder), ng: { model: 'enterprise.phone' } } .small-12.medium-12.large-5.hide-for-small-only .row.buttons diff --git a/config/locales/en.yml b/config/locales/en.yml index 8ce260fb77..af7953a339 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1312,9 +1312,12 @@ See the %{link} to find out more about %{sitename}'s features and to start using registration_greeting: "Greetings!" who_is_managing_enterprise: "Who is responsible for managing %{enterprise}?" enterprise_contact: "Primary Contact" + enterprise_contact_placeholder: "Contact Name" enterprise_contact_required: "You need to enter a primary contact." enterprise_email_address: "Email address" + enterprise_email_placeholder: "eg. charlie@thefarm.com" enterprise_phone: "Phone number" + enterprise_phone_placeholder: "eg. (03) 1234 5678" back: "Back" continue: "Continue" limit_reached_headline: "Oh no!" @@ -1385,6 +1388,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using registration_finished_activate_instruction_html: "We've sent a confirmation email to %{email} if it hasn't been activated before.
Please follow the instructions there to make your enterprise visible on the Open Food Network." registration_finished_action: "Open Food Network home" + registration_contact_name: 'Contact Name' registration_type_headline: "Last step to add %{enterprise}!" registration_type_question: "Are you a producer?" registration_type_producer: "Yes, I'm a producer" From 90d1d5400ac3a71f420da0fa188890609677a6d8 Mon Sep 17 00:00:00 2001 From: Leandro C Date: Sat, 2 Sep 2017 20:07:59 +0100 Subject: [PATCH 003/353] Fix i18n translation keys for New Product Page --- .../admin/products/new/replace_form.html.haml.deface | 6 +++--- app/views/spree/admin/products/_display_as.html.haml | 4 ++-- .../admin/products/_primary_taxon_form.html.haml | 2 +- config/locales/en.yml | 12 +++++++++++- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/app/overrides/spree/admin/products/new/replace_form.html.haml.deface b/app/overrides/spree/admin/products/new/replace_form.html.haml.deface index e9dc6f13c5..efcd0f1d3d 100644 --- a/app/overrides/spree/admin/products/new/replace_form.html.haml.deface +++ b/app/overrides/spree/admin/products/new/replace_form.html.haml.deface @@ -19,14 +19,14 @@ .twelve.columns.alpha{ 'ng-controller' => 'unitsCtrl' } .six.columns.alpha = f.field_container :units do - = f.label :variant_unit_with_scale, :units + = f.label :variant_unit_with_scale, t(:units) %select.select2.fullwidth{ id: 'product_variant_unit_with_scale', 'ng-model' => 'product.variant_unit_with_scale', 'ng-options' => 'unit[1] as unit[0] for unit in variant_unit_options' } %option{'value' => '', 'ng-hide' => "hasUnit(product)"} %input{ type: 'hidden', 'ng-value' => 'product.variant_unit', name: 'product[variant_unit]' } %input{ type: 'hidden', 'ng-value' => 'product.variant_unit_scale', name: 'product[variant_unit_scale]' } .three.columns = f.field_container :unit_value do - = f.label :product_unit_value_with_description, :value, 'ng-disabled' => "!hasUnit(product)" + = f.label :product_unit_value_with_description, t(:value), 'ng-disabled' => "!hasUnit(product)" %input.fullwidth{ id: 'product_unit_value_with_description', 'ng-model' => 'product.master.unit_value_with_description', :type => 'text', placeholder: "eg. 2", 'ng-disabled' => "!hasUnit(product)" } %input{ type: 'hidden', 'ng-value' => 'product.master.unit_value', name: 'product[unit_value]' } %input{ type: 'hidden', 'ng-value' => 'product.master.unit_description', name: 'product[unit_description]' } @@ -87,7 +87,7 @@ = button t('actions.create'), 'icon-ok', :submit, value: "create" %span.or = t(:or) - = button "Create And Add Another", 'icon-repeat', :submit, value: 'add_another' + = button t('actions.create_and_add_another'), 'icon-repeat', :submit, value: 'add_another' %span.or = t(:or) = link_to_with_icon 'icon-remove', t('actions.cancel'), bulk_edit_admin_products_path, :class => 'button' diff --git a/app/views/spree/admin/products/_display_as.html.haml b/app/views/spree/admin/products/_display_as.html.haml index 33beec2b86..04af12bdce 100644 --- a/app/views/spree/admin/products/_display_as.html.haml +++ b/app/views/spree/admin/products/_display_as.html.haml @@ -1,4 +1,4 @@ .three.columns.omega{ "ng-if" => "product.variant_unit_with_scale != 'items'" } = f.field_container :display_as do - = f.label :product_display_as, t(:display_as) - %input#product_display_as.fullwidth{name: "product[display_as]", placeholder: "{{ placeholder_text }}", type: "text"} \ No newline at end of file + = f.label :product_display_as, t('.display_as') + %input#product_display_as.fullwidth{name: "product[display_as]", placeholder: "{{ placeholder_text }}", type: "text"} diff --git a/app/views/spree/admin/products/_primary_taxon_form.html.haml b/app/views/spree/admin/products/_primary_taxon_form.html.haml index 8ffe5887a7..51da47f0f4 100644 --- a/app/views/spree/admin/products/_primary_taxon_form.html.haml +++ b/app/views/spree/admin/products/_primary_taxon_form.html.haml @@ -1,5 +1,5 @@ = f.field_container :primary_taxon_id do - = f.label :primary_taxon_id, t(:product_category) + = f.label :primary_taxon_id, t('.product_category') %span.required * %br = f.collection_select(:primary_taxon_id, Spree::Taxon.all, :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"}) diff --git a/config/locales/en.yml b/config/locales/en.yml index 8ce260fb77..0f3cc3aeb0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -161,7 +161,8 @@ en: powered_by: Powered by blocked_cookies_alert: "Your browser may be blocking cookies needed to use this shopfront. Click below to allow cookies and reload the page." allow_cookies: "Allow Cookies" - + actions: + create_and_add_another: "Create and Add Another" admin: # Common properties / models date: Date @@ -1484,6 +1485,9 @@ Please follow the instructions there to make your enterprise visible on the Open create: "Create" search: "Search" supplier: "Supplier" + product_name: "Product Name" + product_description: "Product Description" + units: "Unit Size" coordinator: "Coordinator" distributor: "Distributor" enterprise_fees: "Enterprise Fees" @@ -2037,6 +2041,7 @@ Please follow the instructions there to make your enterprise visible on the Open other: "You have %{count} active order cycles." manage_order_cycles: "MANAGE ORDER CYCLES" products: + unit_name_placeholder: 'eg. bunches' bulk_edit: header: title: Bulk Edit Products @@ -2044,6 +2049,11 @@ Please follow the instructions there to make your enterprise visible on the Open title: LOADING PRODUCTS no_products: "No products yet. Why don't you add some?" no_results: "Sorry, no results match" + product_name: Product Name + primary_taxon_form: + product_category: Product Category + display_as: + display_as: Display As reports: bulk_coop: bulk_coop_supplier_report: 'Bulk Co-op - Totals by Supplier' From 942ab55ddce2001d48f0f86251d0738f3917e8ec Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 7 Sep 2017 15:59:07 +0200 Subject: [PATCH 004/353] Disable create profile from signup when submitting This prevents people re-submitting the form multiple times. Although the backend validates it, we show an ugly alert message that is hard for users to understand. --- .../registration_form_controller.js.coffee | 10 +++++++++- .../services/enterprise_registration_service.js.coffee | 5 ++++- app/views/registration/steps/_type.html.haml | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/registration/registration_form_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/registration/registration_form_controller.js.coffee index fabc2c382a..546186f53d 100644 --- a/app/assets/javascripts/darkswarm/controllers/registration/registration_form_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/registration/registration_form_controller.js.coffee @@ -1,15 +1,23 @@ Darkswarm.controller "RegistrationFormCtrl", ($scope, RegistrationService, EnterpriseRegistrationService) -> $scope.submitted = false + $scope.isDisabled = false $scope.valid = (form) -> $scope.submitted = !form.$valid form.$valid $scope.create = (form) -> - EnterpriseRegistrationService.create() if $scope.valid(form) + $scope.disableButton() + EnterpriseRegistrationService.create($scope.enableButton) if $scope.valid(form) $scope.update = (nextStep, form) -> EnterpriseRegistrationService.update(nextStep) if $scope.valid(form) $scope.selectIfValid = (nextStep, form) -> RegistrationService.select(nextStep) if $scope.valid(form) + + $scope.disableButton = -> + $scope.isDisabled = true + + $scope.enableButton = -> + $scope.isDisabled = false diff --git a/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee b/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee index 1434ffa44f..de8d643711 100644 --- a/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee +++ b/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee @@ -11,7 +11,7 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService, for key, value of enterpriseAttributes @enterprise[key] = value - create: => + create: (callback) => Loading.message = t('creating') + " " + @enterprise.name $http( method: "POST" @@ -25,6 +25,7 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService, @enterprise.id = data EnterpriseImageService.configure(@enterprise) RegistrationService.select('about') + callback.call() ).error((data) => Loading.clear() if data?.errors? @@ -32,6 +33,8 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService, alert t('failed_to_create_enterprise') + "\n" + errors.join('\n') else alert(t('failed_to_create_enterprise_unknown')) + + callback.call() ) update: (step) => diff --git a/app/views/registration/steps/_type.html.haml b/app/views/registration/steps/_type.html.haml index 4c93c441f4..5fcf6391b9 100644 --- a/app/views/registration/steps/_type.html.haml +++ b/app/views/registration/steps/_type.html.haml @@ -45,4 +45,4 @@ .row.buttons .small-12.columns %input.button.secondary{ type: "button", value: "{{'back' | t}}", ng: { click: "select('contact')" } } - %input.button.primary.right{ type: "submit", value: "{{'create_profile' | t}}" } + %input.button.primary.right{ ng: { disabled: 'isDisabled' }, type: "submit", value: "{{'create_profile' | t}}" } From d3d4628e297f8685ebe8178dfdc2cea1f68dcd2b Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 8 Sep 2017 09:51:06 +0200 Subject: [PATCH 005/353] Add doc --- .../services/enterprise_registration_service.js.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee b/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee index de8d643711..7e81d53d70 100644 --- a/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee +++ b/app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee @@ -11,6 +11,10 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService, for key, value of enterpriseAttributes @enterprise[key] = value + # Creates the enterprise and redirects to the about step on success. + # + # @param callback [Function] executed at the end of the operation both in + # case of success or failure. create: (callback) => Loading.message = t('creating') + " " + @enterprise.name $http( From c70de9f73c23f74c5db243c44d4b645a8ba86b5c Mon Sep 17 00:00:00 2001 From: leandroalemao Date: Wed, 6 Sep 2017 18:32:33 +0100 Subject: [PATCH 006/353] Fix i18n translation keys for Edit Product Page --- .../products/_form/add_supplier.html.haml.deface | 2 +- .../products/_form/add_units_form.html.haml.deface | 2 +- .../spree/admin/products/_group_buy_form.html.haml | 12 ++++++------ .../admin/products/_tax_category_form.html.haml | 2 +- config/locales/en.yml | 10 +++++++++- spec/features/admin/products_spec.rb | 2 +- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface b/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface index 1cc609bef4..deba9ff633 100644 --- a/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface +++ b/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface @@ -1,7 +1,7 @@ / insert_top "[data-hook='admin_product_form_right']" = f.field_container :supplier do - = f.label :supplier + = f.label :supplier, t(:spree_admin_supplier) %br = f.collection_select(:supplier_id, @producers, :id, :name, {:include_blank => true}, {:class => "select2"}) = f.error_message_on :supplier diff --git a/app/overrides/spree/admin/products/_form/add_units_form.html.haml.deface b/app/overrides/spree/admin/products/_form/add_units_form.html.haml.deface index d20afcca61..6571fb3ab6 100644 --- a/app/overrides/spree/admin/products/_form/add_units_form.html.haml.deface +++ b/app/overrides/spree/admin/products/_form/add_units_form.html.haml.deface @@ -3,7 +3,7 @@ .variant_units_form{ 'ng-app' => 'admin.products', 'ng-controller' => 'editUnitsCtrl' } = f.field_container :units do - = f.label :variant_unit_with_scale, :units + = f.label :variant_unit_with_scale, t(:spree_admin_variant_unit_scale) %select.select2.fullwidth{ id: 'product_variant_unit_with_scale', 'ng-model' => 'variant_unit_with_scale', 'ng-change' => 'setFields()', 'ng-options' => 'unit[1] as unit[0] for unit in variant_unit_options' } %option{'value' => ''} diff --git a/app/views/spree/admin/products/_group_buy_form.html.haml b/app/views/spree/admin/products/_group_buy_form.html.haml index 8f1de2a884..f61fc7b38d 100644 --- a/app/views/spree/admin/products/_group_buy_form.html.haml +++ b/app/views/spree/admin/products/_group_buy_form.html.haml @@ -1,14 +1,14 @@ = f.field_container :group_buy do - = f.label :group_buy, 'Group buy?' + = f.label :group_buy, t('.group_buy') %br .alpha.two.columns - = f.radio_button :group_buy, '1', :checked => f.object.group_buy - = f.label :group_buy_1, 'Yes' + = f.radio_button :group_buy, '1', checked: f.object.group_buy + = f.label :group_buy_1, t(:yes) .alpha.two.columns - = f.radio_button :group_buy, '0', :checked => !f.object.group_buy - = f.label :group_buy_0, 'No' + = f.radio_button :group_buy, '0', checked: !f.object.group_buy + = f.label :group_buy_0, t(:no) %br.clear = f.field_container :group_buy_unit_size do - = f.label :group_buy_unit_size, "Bulk unit size" + = f.label :group_buy_unit_size, t('.bulk_unit_size') %br = f.text_field :group_buy_unit_size diff --git a/app/views/spree/admin/products/_tax_category_form.html.haml b/app/views/spree/admin/products/_tax_category_form.html.haml index 3d529e6562..250ba8c832 100644 --- a/app/views/spree/admin/products/_tax_category_form.html.haml +++ b/app/views/spree/admin/products/_tax_category_form.html.haml @@ -2,5 +2,5 @@ = f.label :tax_category_id, t(:tax_category) %span.required * %br - = f.collection_select(:tax_category_id, Spree::TaxCategory.all, :id, :name, {:include_blank => Spree::Config.products_require_tax_category ? false : 'None'}, {:class => "select2 fullwidth"}) + = f.collection_select(:tax_category_id, Spree::TaxCategory.all, :id, :name, {:include_blank => Spree::Config.products_require_tax_category ? false : t(:none)}, {:class => "select2 fullwidth"}) = f.error_message_on :tax_category_id diff --git a/config/locales/en.yml b/config/locales/en.yml index 0f3cc3aeb0..104999525b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -161,8 +161,10 @@ en: powered_by: Powered by blocked_cookies_alert: "Your browser may be blocking cookies needed to use this shopfront. Click below to allow cookies and reload the page." allow_cookies: "Allow Cookies" + none: None + notes: Notes actions: - create_and_add_another: "Create and Add Another" + create_and_add_another: "Create and Add Another" admin: # Common properties / models date: Date @@ -937,6 +939,7 @@ en: checkout_shipping_price: Shipping checkout_total_price: Total checkout_back_to_cart: "Back to Cart" + cost_currency: "Cost Currency" order_paid: PAID order_not_paid: NOT PAID @@ -1549,6 +1552,8 @@ Please follow the instructions there to make your enterprise visible on the Open spree_admin_unit_description: Unit Description spree_admin_variant_unit: Variant unit spree_admin_variant_unit_scale: Variant unit scale + spree_admin_supplier: Supplier + spree_admin_product_category: Product Category spree_admin_variant_unit_name: Variant unit name change_package: "Change Package" spree_admin_single_enterprise_hint: "Hint: To allow people to find you, turn on your visibility under" @@ -2052,6 +2057,9 @@ Please follow the instructions there to make your enterprise visible on the Open product_name: Product Name primary_taxon_form: product_category: Product Category + group_buy_form: + group_buy: "Group Buy?" + bulk_unit_size: Bulk unit size display_as: display_as: Display As reports: diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index 2cebd58dce..cc11e5853d 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -95,7 +95,7 @@ feature %q{ visit spree.edit_admin_product_path(product) choose 'product_group_buy_1' - fill_in 'Bulk unit size', :with => '10' + fill_in 'product_group_buy_unit_size', with: '10' click_button 'Update' From a1aac7643ac55acb1d1bea8ee48b73c7e10a1d03 Mon Sep 17 00:00:00 2001 From: Saimon Moore Date: Fri, 8 Sep 2017 12:47:53 +0200 Subject: [PATCH 007/353] Translate Entp registration modal tab titles - Add proper namespaced translations for Details and Contact tabs --- app/views/registration/steps/_steps.html.haml | 3 +- config/locales/en.yml | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/app/views/registration/steps/_steps.html.haml b/app/views/registration/steps/_steps.html.haml index 7ea0412929..3538d5ff48 100644 --- a/app/views/registration/steps/_steps.html.haml +++ b/app/views/registration/steps/_steps.html.haml @@ -1,4 +1,5 @@ %script{ type: "text/ng-template", id: "registration/steps.html" } .row#progress-bar .small-12.medium-2.columns.item{ ng: { repeat: 'step in steps', class: "{active: (currentStep() == step),'show-for-medium-up': (currentStep() != step)}" } } - {{ $index+1 + ". " + step }} + {{ $index+1 + ". " + ('enterprise.registration.modal.steps.' + step + '.title' | t) }} + diff --git a/config/locales/en.yml b/config/locales/en.yml index af7953a339..f047b059be 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1311,6 +1311,50 @@ See the %{link} to find out more about %{sitename}'s features and to start using reset_password: "Reset password" registration_greeting: "Greetings!" who_is_managing_enterprise: "Who is responsible for managing %{enterprise}?" + enterprise: + registration: + modal: + steps: + details: + title: 'Details' + headline: "Let's Get Started" + enterprise: "Woot! First we need to know a little bit about your enterprise:" + producer: "Woot! First we need to know a little bit about your farm:" + enterprise_name_field: "Enterprise Name:" + producer_name_field: "Farm Name:" + producer_name_field_placeholder: "e.g. Charlie's Awesome Farm" + producer_name_field_error: "Please choose a unique name for your enterprise" + address1_field: "Address line 1:" + address1_field_placeholder: "e.g. 123 Cranberry Drive" + address1_field_error: "Please enter an address" + address2_field: "Address line 2:" + suburb_field: "Suburb:" + suburb_field_placeholder: "e.g. Northcote" + suburb_field_error: "Please enter a suburb" + postcode_field: "Postcode:" + postcode_field_placeholder: "e.g. 3070" + postcode_field_error: "Postcode required" + state_field: "State:" + state_field_error: "State required" + country_field: "Country:" + country_field_error: "Please select a country" + contact: + title: 'Contact' + contact_field: 'Primary Contact' + contact_field_placeholder: 'Contact Name' + contact_field_required: "You need to enter a primary contact." + email_field: 'Email address' + email_field_placeholder: 'eg. charlie@thefarm.com' + phone_field: 'Phone number' + phone_field_placeholder: 'eg. (03) 1234 5678' + type: + title: 'Type' + about: + title: 'About' + images: + title: 'Images' + social: + title: 'Social' enterprise_contact: "Primary Contact" enterprise_contact_placeholder: "Contact Name" enterprise_contact_required: "You need to enter a primary contact." From d4f7efd08a15e57a699b2bea1a4c1a4cfb2f303f Mon Sep 17 00:00:00 2001 From: Saimon Moore Date: Fri, 8 Sep 2017 12:49:08 +0200 Subject: [PATCH 008/353] Use proper namespaced translations ... in enterprise registration wizard Contact step --- app/views/registration/steps/_contact.html.haml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/views/registration/steps/_contact.html.haml b/app/views/registration/steps/_contact.html.haml index 169e98cc85..dd282f4ca3 100644 --- a/app/views/registration/steps/_contact.html.haml +++ b/app/views/registration/steps/_contact.html.haml @@ -12,18 +12,18 @@ .small-12.medium-12.large-7.columns .row .small-12.columns.field - %label{ for: 'enterprise_contact' } {{'enterprise_contact' | t}}: - %input.chunky.small-12.columns{ id: 'enterprise_contact', name: 'contact', required: true, placeholder: t(:enterprise_contact_placeholder), ng: { model: 'enterprise.contact' } } + %label{ for: 'enterprise_contact' } {{'enterprise.registration.modal.steps.contact.contact_field' | t}}: + %input.chunky.small-12.columns{ id: 'enterprise_contact', name: 'contact', required: true, placeholder: "{{'enterprise.registration.modal.steps.contact.contact_field_placeholder' | t}}", ng: { model: 'enterprise.contact' } } %span.error.small-12.columns{ ng: { show: "contact.contact.$error.required && submitted" } } - {{'enterprise_contact_required' | t}} + {{'enterprise.registration.modal.steps.contact.contact_field_required' | t}} .row .small-12.columns.field - %label{ for: 'enterprise_email_address' } {{'enterprise_email_address' | t}}: - %input.chunky.small-12.columns{ id: 'enterprise_email_address', name: 'email_address', type: 'email', placeholder: t(:enterprise_email_placeholder), ng: { model: 'enterprise.email_address' } } + %label{ for: 'enterprise_email_address' } {{'enterprise.registration.modal.steps.contact.email_field' | t}}: + %input.chunky.small-12.columns{ id: 'enterprise_email_address', name: 'email_address', type: 'email', placeholder: "{{'enterprise.registration.modal.steps.contact.email_field_placeholder' | t}}", ng: { model: 'enterprise.email_address' } } .row .small-12.columns.field - %label{ for: 'enterprise_phone' } {{'enterprise_phone' | t}}: - %input.chunky.small-12.columns{ id: 'enterprise_phone', name: 'phone', placeholder: t(:enterprise_phone_placeholder), ng: { model: 'enterprise.phone' } } + %label{ for: 'enterprise_phone' } {{'enterprise.registration.modal.steps.contact.phone_field' | t}}: + %input.chunky.small-12.columns{ id: 'enterprise_phone', name: 'phone', placeholder: "{{'enterprise.registration.modal.steps.contact.phone_field_placeholder' | t}}", ng: { model: 'enterprise.phone' } } .small-12.medium-12.large-5.hide-for-small-only .row.buttons From f73f0d4bf929d749f676b4a49a07a74624b32e03 Mon Sep 17 00:00:00 2001 From: Saimon Moore Date: Fri, 8 Sep 2017 12:50:54 +0200 Subject: [PATCH 009/353] Use proper namespaced translations ... in enterprise registration wizard Details step --- .../registration/steps/_details.html.haml | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/app/views/registration/steps/_details.html.haml b/app/views/registration/steps/_details.html.haml index 4a01d92385..4fce6a4319 100644 --- a/app/views/registration/steps/_details.html.haml +++ b/app/views/registration/steps/_details.html.haml @@ -4,59 +4,59 @@ .row .small-12.columns %header - %h2 {{'registration_detail_headline' | t}} - %h5{ ng: { if: "::enterprise.type != 'own'" } } {{'registration_detail_enterprise' | t}} - %h5{ ng: { if: "::enterprise.type == 'own'" } } {{'registration_detail_producer' | t}} + %h2 {{'enterprise.registration.modal.steps.details.headline' | t}} + %h5{ ng: { if: "::enterprise.type != 'own'" } } {{'enterprise.registration.modal.steps.details.enterprise' | t}} + %h5{ ng: { if: "::enterprise.type == 'own'" } } {{'enterprise.registration.modal.steps.details.producer' | t}} %form{ name: 'details', novalidate: true, ng: { controller: "RegistrationFormCtrl", submit: "selectIfValid('contact',details)" } } .row .small-12.medium-9.large-12.columns.end .field - %label{ for: 'enterprise_name', ng: { if: "::enterprise.type != 'own'" } } {{'registration_detail_name_enterprise' | t}} - %label{ for: 'enterprise_name', ng: { if: "::enterprise.type == 'own'" } } {{'registration_detail_name_producer' | t}} - %input.chunky{ id: 'enterprise_name', name: 'name', placeholder: "{{'registration_detail_name_placeholder' | t}}", required: true, ng: { model: 'enterprise.name' } } + %label{ for: 'enterprise_name', ng: { if: "::enterprise.type != 'own'" } } {{'enterprise.registration.modal.steps.details.enterprise_name_field' | t}} + %label{ for: 'enterprise_name', ng: { if: "::enterprise.type == 'own'" } } {{'enterprise.registration.modal.steps.details.producer_name_field' | t}} + %input.chunky{ id: 'enterprise_name', name: 'name', placeholder: "{{'enterprise.registration.modal.steps.details.producer_name_field_placeholder' | t}}", required: true, ng: { model: 'enterprise.name' } } %span.error{ ng: { show: "details.name.$error.required && submitted" } } - {{'registration_detail_name_error' | t}} + {{'enterprise.registration.modal.steps.details.producer_name_field_error' | t}} .row .small-12.medium-9.large-6.columns .field - %label{ for: 'enterprise_address' } {{'registration_detail_address1' | t}} - %input.chunky{ id: 'enterprise_address', name: 'address1', required: true, placeholder: "{{'registration_detail_address1_placeholder' | t}}", required: true, ng: { model: 'enterprise.address.address1' } } + %label{ for: 'enterprise_address' } {{'enterprise.registration.modal.steps.details.address1_field' | t}} + %input.chunky{ id: 'enterprise_address', name: 'address1', required: true, placeholder: "{{'enterprise.registration.modal.steps.details.address1_field_placeholder' | t}}", required: true, ng: { model: 'enterprise.address.address1' } } %span.error{ ng: { show: "details.address1.$error.required && submitted" } } - {{'registration_detail_address1_error' | t}} + {{'enterprise.registration.modal.steps.details.address1_field_error' | t}} .field - %label{ for: 'enterprise_address2' } {{'registration_detail_address2' | t}} + %label{ for: 'enterprise_address2' } {{'enterprise.registration.modal.steps.details.address2_field' | t}} %input.chunky{ id: 'enterprise_address2', name: 'address2', required: false, placeholder: "", required: false, ng: { model: 'enterprise.address.address2' } } .small-12.medium-9.large-6.columns.end .row .small-12.medium-8.large-8.columns .field - %label{ for: 'enterprise_city' } {{'registration_detail_suburb' | t}} - %input.chunky{ id: 'enterprise_city', name: 'city', required: true, placeholder: "{{'registration_detail_suburb_placeholder' | t}}", ng: { model: 'enterprise.address.city' } } + %label{ for: 'enterprise_city' } {{'enterprise.registration.modal.steps.details.suburb_field' | t}} + %input.chunky{ id: 'enterprise_city', name: 'city', required: true, placeholder: "{{'enterprise.registration.modal.steps.details.suburb_field_placeholder' | t}}", ng: { model: 'enterprise.address.city' } } %span.error{ ng: { show: "details.city.$error.required && submitted" } } - {{'registration_detail_suburb_error' | t}} + {{'enterprise.registration.modal.steps.details.suburb_field_error' | t}} .small-12.medium-4.large-4.columns .field - %label{ for: 'enterprise_zipcode' } {{'registration_detail_postcode' | t}} - %input.chunky{ id: 'enterprise_zipcode', name: 'zipcode', required: true, placeholder: "{{'registration_detail_postcode_placeholder' | t}}", ng: { model: 'enterprise.address.zipcode' } } + %label{ for: 'enterprise_zipcode' } {{'enterprise.registration.modal.steps.details.postcode_field' | t}} + %input.chunky{ id: 'enterprise_zipcode', name: 'zipcode', required: true, placeholder: "{{'enterprise.registration.modal.steps.details.postcode_field_placeholder' | t}}", ng: { model: 'enterprise.address.zipcode' } } %span.error{ ng: { show: "details.zipcode.$error.required && submitted" } } - {{'registration_detail_postcode_error' | t}} + {{'enterprise.registration.modal.steps.details.postcode_field_error' | t}} .row .small-12.medium-4.large-4.columns .field - %label{ for: 'enterprise_state' } {{'registration_detail_state' | t}} + %label{ for: 'enterprise_state' } {{'enterprise.registration.modal.steps.details.state_field' | t}} %select.chunky{ id: 'enterprise_state', name: 'state', ng: { model: 'enterprise.address.state_id', options: 's.id as s.abbr for s in enterprise.country.states', show: 'countryHasStates()', required: 'countryHasStates()' } } %span.error{ ng: { show: "details.state.$error.required && submitted" } } - {{'registration_detail_state_error' | t}} + {{'enterprise.registration.modal.steps.details.state_field_error' | t}} .small-12.medium-8.large-8.columns .field - %label{ for: 'enterprise_country' } {{'registration_detail_country' | t}} + %label{ for: 'enterprise_country' } {{'enterprise.registration.modal.steps.details.country_field' | t}} %select.chunky{ id: 'enterprise_country', name: 'country', required: true, ng: { init: "setDefaultCountry(#{Spree::Config[:default_country_id]})", model: 'enterprise.country', options: 'c as c.name for c in countries' } } %span.error{ ng: { show: "details.country.$error.required && submitted" } } - {{'registration_detail_country_error' | t}} + {{'enterprise.registration.modal.steps.details.country_field_error' | t}} .row.buttons From 757f886af535d4e949f83cf14774fd6f81f28c28 Mon Sep 17 00:00:00 2001 From: Saimon Moore Date: Fri, 8 Sep 2017 13:35:19 +0200 Subject: [PATCH 010/353] Use proper namespaced translations ... in enterprise registration wizard Type step --- app/views/registration/steps/_type.html.haml | 14 +++++++------- config/locales/en.yml | 8 ++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/views/registration/steps/_type.html.haml b/app/views/registration/steps/_type.html.haml index 4c93c441f4..0a98d851ed 100644 --- a/app/views/registration/steps/_type.html.haml +++ b/app/views/registration/steps/_type.html.haml @@ -6,9 +6,9 @@ .row .small-12.columns %header - %h2{ "ng-bind" => "'registration_type_headline' | t:{enterprise: enterprise.name}" } + %h2{ "ng-bind" => "'enterprise.registration.modal.steps.type.headline' | t:{enterprise: enterprise.name}" } %h4 - {{'registration_type_question' | t}} + {{'enterprise.registration.modal.steps.type.question' | t}} %form{ name: 'type', novalidate: true, ng: { controller: "RegistrationFormCtrl", submit: "create(type)" } } .row#enterprise-types{ 'data-equalizer' => true, ng: { if: "::enterprise.type != 'own'" } } @@ -17,30 +17,30 @@ .small-12.medium-6.large-6.columns{ 'data-equalizer-watch' => true } %a.btnpanel#producer-panel{ href: "#", ng: { click: "enterprise.is_primary_producer = true", class: "{selected: enterprise.is_primary_producer}" } } %i.ofn-i_059-producer - %h4 {{'registration_type_producer' | t}} + %h4 {{'enterprise.registration.modal.steps.type.yes_producer' | t}} .small-12.medium-6.large-6.columns{ 'data-equalizer-watch' => true } %a.btnpanel#hub-panel{ href: "#", ng: { click: "enterprise.is_primary_producer = false", class: "{selected: enterprise.is_primary_producer == false}" } } %i.ofn-i_063-hub - %h4 {{'registration_type_no_producer' | t}} + %h4 {{'enterprise.registration.modal.steps.type.no_producer' | t}} .row .small-12.columns %input.chunky{ id: 'enterprise_is_primary_producer', name: 'is_primary_producer', hidden: true, required: true, ng: { model: 'enterprise.is_primary_producer' } } %span.error{ ng: { show: "type.is_primary_producer.$error.required && submitted" } } - {{'registration_type_error' | t}} + {{'enterprise.registration.modal.steps.type.producer_field_error' | t}} .row .small-12.columns .panel.callout .left %i.ofn-i_013-help   - %p {{'registration_type_producer_help' | t}} + %p {{'enterprise.registration.modal.steps.type.yes_producer_help' | t}} .panel.callout .left %i.ofn-i_013-help   - %p {{'registration_type_no_producer_help' | t}} + %p {{'enterprise.registration.modal.steps.type.no_producer_help' | t}} .row.buttons .small-12.columns diff --git a/config/locales/en.yml b/config/locales/en.yml index f047b059be..8b5193659b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1349,6 +1349,14 @@ See the %{link} to find out more about %{sitename}'s features and to start using phone_field_placeholder: 'eg. (03) 1234 5678' type: title: 'Type' + headline: "Last step to add %{enterprise}!" + question: "Are you a producer?" + yes_producer: "Yes, I'm a producer" + no_producer: "No, I'm not a producer" + producer_field_error: "Please choose one. Are you are producer?" + yes_producer_help: "Producers make yummy things to eat and/or drink. You're a producer if you grow it, raise it, brew it, bake it, ferment it, milk it or mould it." + no_producer_help: "If you’re not a producer, you’re probably someone who sells and distributes food. You might be a hub, coop, buying group, retailer, wholesaler or other." + about: title: 'About' images: From 09aed63cd40ccc5bb43c625b0c9b197e9d8d169a Mon Sep 17 00:00:00 2001 From: Saimon Moore Date: Fri, 8 Sep 2017 13:57:50 +0200 Subject: [PATCH 011/353] Add todos to remove redundant i18n keys --- config/locales/en.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b5193659b..28261cf103 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1363,6 +1363,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using title: 'Images' social: title: 'Social' + # TODO: Remove these once the enterprise.registration.modal keys are translated enterprise_contact: "Primary Contact" enterprise_contact_placeholder: "Contact Name" enterprise_contact_required: "You need to enter a primary contact." @@ -1370,6 +1371,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using enterprise_email_placeholder: "eg. charlie@thefarm.com" enterprise_phone: "Phone number" enterprise_phone_placeholder: "eg. (03) 1234 5678" + # END back: "Back" continue: "Continue" limit_reached_headline: "Oh no!" @@ -1441,6 +1443,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using Please follow the instructions there to make your enterprise visible on the Open Food Network." registration_finished_action: "Open Food Network home" registration_contact_name: 'Contact Name' + + # TODO: Remove these once the enterprise.registration.modal keys are translated registration_type_headline: "Last step to add %{enterprise}!" registration_type_question: "Are you a producer?" registration_type_producer: "Yes, I'm a producer" @@ -1448,9 +1452,13 @@ Please follow the instructions there to make your enterprise visible on the Open registration_type_error: "Please choose one. Are you are producer?" registration_type_producer_help: "Producers make yummy things to eat and/or drink. You're a producer if you grow it, raise it, brew it, bake it, ferment it, milk it or mould it." registration_type_no_producer_help: "If you’re not a producer, you’re probably someone who sells and distributes food. You might be a hub, coop, buying group, retailer, wholesaler or other." + # END create_profile: "Create Profile" registration_images_headline: "Thanks!" registration_images_description: "Let's upload some pretty pictures so your profile looks great! :)" + + + # TODO: Remove these once the enterprise.registration.modal keys are translated registration_detail_headline: "Let's Get Started" registration_detail_enterprise: "Woot! First we need to know a little bit about your enterprise:" registration_detail_producer: "Woot! First we need to know a little bit about your farm:" @@ -1472,6 +1480,7 @@ Please follow the instructions there to make your enterprise visible on the Open registration_detail_state_error: "State required" registration_detail_country: "Country:" registration_detail_country_error: "Please select a country" + # END shipping_method_destroy_error: "That shipping method cannot be deleted as it is referenced by an order: %{number}." accounts_and_billing_task_already_running_error: "A task is already running, please wait until it has finished" accounts_and_billing_start_task_notice: "Task Queued" From f5c4537afa49e4ed484d4d24cbe8c3c5833bc92a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Sep 2016 12:33:53 +1000 Subject: [PATCH 012/353] Start Spree upgrade step 2 --- Gemfile | 2 +- Gemfile.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index d4c55bb63f..31092561ec 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem 'i18n-js', '~> 3.0.0' gem 'nokogiri', '>= 1.6.7.1' gem 'pg' -gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step1c' +gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-2' gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable' gem 'spree_auth_devise', github: 'spree/spree_auth_devise', branch: '1-3-stable' diff --git a/Gemfile.lock b/Gemfile.lock index fcb7283ef9..f8a6b604c3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,8 +30,8 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: a4c439570b77afa50f9e36299811f293232bd281 - branch: spree-upgrade-step1c + revision: ba444ace4de81626ef0a69a44123e25084874b4a + branch: spree-upgrade-step-2 specs: spree (1.3.99) spree_api (= 1.3.99) @@ -54,7 +54,7 @@ GIT deface (>= 0.9.0) ffaker (~> 1.15.0) highline (= 1.6.11) - jquery-rails (~> 2.0) + jquery-rails (~> 2.1.4) json (>= 1.5.5) kaminari (= 0.13.0) money (= 5.0.0) @@ -426,7 +426,7 @@ GEM ipaddress (0.8.0) journey (1.0.4) jquery-migrate-rails (1.2.1) - jquery-rails (2.3.0) + jquery-rails (2.1.4) railties (>= 3.0, < 5.0) thor (>= 0.14, < 2.0) json (1.8.3) From b5d33fc4b586eb8a8314f66453de65dad4c82ec0 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Sep 2016 12:46:46 +1000 Subject: [PATCH 013/353] Add migrations from Spree --- .../20160916024535_add_state_to_spree_adjustments.spree.rb | 7 +++++++ db/schema.rb | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20160916024535_add_state_to_spree_adjustments.spree.rb diff --git a/db/migrate/20160916024535_add_state_to_spree_adjustments.spree.rb b/db/migrate/20160916024535_add_state_to_spree_adjustments.spree.rb new file mode 100644 index 0000000000..a4224678df --- /dev/null +++ b/db/migrate/20160916024535_add_state_to_spree_adjustments.spree.rb @@ -0,0 +1,7 @@ +# This migration comes from spree (originally 20121213162028) +class AddStateToSpreeAdjustments < ActiveRecord::Migration + def change + add_column :spree_adjustments, :state, :string + remove_column :spree_adjustments, :locked + end +end diff --git a/db/schema.rb b/db/schema.rb index 75c43b1d72..c0583e5f54 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -401,12 +401,12 @@ ActiveRecord::Schema.define(:version => 20161215230219) do t.datetime "created_at", :null => false t.datetime "updated_at", :null => false t.boolean "mandatory" - t.boolean "locked" t.integer "originator_id" t.string "originator_type" t.boolean "eligible", :default => true t.string "adjustable_type" t.decimal "included_tax", :precision => 10, :scale => 2, :default => 0.0, :null => false + t.string "state" end add_index "spree_adjustments", ["adjustable_id"], :name => "index_adjustments_on_order_id" @@ -586,9 +586,9 @@ ActiveRecord::Schema.define(:version => 20161215230219) do t.string "email" t.text "special_instructions" t.integer "distributor_id" - t.integer "order_cycle_id" t.string "currency" t.string "last_ip_address" + t.integer "order_cycle_id" t.integer "cart_id" t.integer "customer_id" end From 38da4c8e12aab943a23cc8c28b5607be9de1e22d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Sep 2016 15:02:55 +1000 Subject: [PATCH 014/353] Adjustments now have state instead of locked/unlocked --- app/models/billable_period.rb | 2 +- app/models/enterprise_fee.rb | 13 ------------- app/models/product_distribution.rb | 2 +- app/models/spree/line_item_decorator.rb | 2 +- lib/open_food_network/enterprise_fee_applicator.rb | 4 ++-- spec/models/enterprise_fee_spec.rb | 2 +- spec/models/product_distribution_spec.rb | 6 +++--- spec/models/spree/order_spec.rb | 4 ++-- 8 files changed, 11 insertions(+), 24 deletions(-) diff --git a/app/models/billable_period.rb b/app/models/billable_period.rb index d3a58e3745..d2ca41b0a9 100644 --- a/app/models/billable_period.rb +++ b/app/models/billable_period.rb @@ -71,7 +71,7 @@ class BillablePeriod < ActiveRecord::Base source: self, originator: nil, # enterprise.package mandatory: true, - locked: false + state: 'closed' } end end diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index ff118cf0aa..e7e8ee0513 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -54,19 +54,6 @@ class EnterpriseFee < ActiveRecord::Base order.adjustments.where(originator_type: 'EnterpriseFee').destroy_all end - # Create an adjustment that starts as locked. Preferable to making an adjustment and locking it since - # the unlocked adjustment tends to get hit by callbacks before we have a chance to lock it. - def create_locked_adjustment(label, target, calculable, mandatory=false) - amount = compute_amount(calculable) - return if amount == 0 && !mandatory - target.adjustments.create({ :amount => amount, - :source => calculable, - :originator => self, - :label => label, - :mandatory => mandatory, - :locked => true}, :without_protection => true) - end - private diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb index e866d3860b..a18d9d638c 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -17,7 +17,7 @@ class ProductDistribution < ActiveRecord::Base end def create_adjustment_for(line_item) - a = enterprise_fee.create_locked_adjustment(adjustment_label_for(line_item), line_item.order, line_item, true) + a = enterprise_fee.create_adjustment(adjustment_label_for(line_item), line_item.order, line_item, true) AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: 'distributor' end diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 9c80df8bb4..4985d059b0 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -60,7 +60,7 @@ Spree::LineItem.class_eval do end def price_with_adjustments - # EnterpriseFee#create_locked_adjustment applies adjustments on line items to their parent order, + # EnterpriseFee#create_adjustment applies adjustments on line items to their parent order, # so line_item.adjustments returns an empty array return 0 if quantity == 0 (price + order.adjustments.where(source_id: id).sum(&:amount) / quantity).round(2) diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index e6d52e1749..e213204047 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -1,7 +1,7 @@ module OpenFoodNetwork class EnterpriseFeeApplicator < Struct.new(:enterprise_fee, :variant, :role) def create_line_item_adjustment(line_item) - a = enterprise_fee.create_locked_adjustment(line_item_adjustment_label, line_item.order, line_item, true) + a = enterprise_fee.create_adjustment(line_item_adjustment_label, line_item.order, line_item, true) AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role @@ -9,7 +9,7 @@ module OpenFoodNetwork end def create_order_adjustment(order) - a = enterprise_fee.create_locked_adjustment(order_adjustment_label, order, order, true) + a = enterprise_fee.create_adjustment(order_adjustment_label, order, order, true) AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index e0df4ce5d0..6c4245c875 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -170,7 +170,7 @@ describe EnterpriseFee do order.adjustments.create({:amount => 12.34, :source => order, :originator => tax_rate, - :locked => true, + :state => 'closed', :label => 'hello' }, :without_protection => true) expect do diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb index 7e1a6933bc..2fa2bab427 100644 --- a/spec/models/product_distribution_spec.rb +++ b/spec/models/product_distribution_spec.rb @@ -78,7 +78,7 @@ describe ProductDistribution do it "returns the adjustment when present" do pd = create(:product_distribution) line_item = create(:line_item) - adjustment = pd.enterprise_fee.create_locked_adjustment('foo', line_item.order, line_item, true) + adjustment = pd.enterprise_fee.create_adjustment('foo', line_item.order, line_item, true) pd.send(:adjustment_for, line_item).should == adjustment end @@ -86,8 +86,8 @@ describe ProductDistribution do it "raises an error when there are multiple adjustments for this enterprise fee" do pd = create(:product_distribution) line_item = create(:line_item) - pd.enterprise_fee.create_locked_adjustment('one', line_item.order, line_item, true) - pd.enterprise_fee.create_locked_adjustment('two', line_item.order, line_item, true) + pd.enterprise_fee.create_adjustment('one', line_item.order, line_item, true) + pd.enterprise_fee.create_adjustment('two', line_item.order, line_item, true) expect do pd.send(:adjustment_for, line_item) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e7c46c27cb..392d9664fa 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -140,7 +140,7 @@ describe Spree::Order do it "returns the sum of eligible enterprise fee adjustments" do ef = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new ) ef.calculator.set_preference :amount, 123.45 - a = ef.create_locked_adjustment("adjustment", o, o, true) + a = ef.create_adjustment("adjustment", o, o, true) o.admin_and_handling_total.should == 123.45 end @@ -148,7 +148,7 @@ describe Spree::Order do it "does not include ineligible adjustments" do ef = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new ) ef.calculator.set_preference :amount, 123.45 - a = ef.create_locked_adjustment("adjustment", o, o, true) + a = ef.create_adjustment("adjustment", o, o, true) a.update_column :eligible, false From bfd54ef62159061edd7bba718c6ef228c5a6a08c Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 10:43:46 +1100 Subject: [PATCH 015/353] Update Spree: Fix admin authorization errors --- Gemfile.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f8a6b604c3..383caa2bab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,7 +30,7 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: ba444ace4de81626ef0a69a44123e25084874b4a + revision: 4a0013618dbd5bae254e921565bf51d3dd5ccd24 branch: spree-upgrade-step-2 specs: spree (1.3.99) @@ -208,7 +208,7 @@ GEM coffee-script-source execjs coffee-script-source (1.10.0) - colorize (0.7.7) + colorize (0.8.1) columnize (0.9.0) compass (1.0.3) chunky_png (~> 1.2) @@ -456,7 +456,7 @@ GEM treetop (~> 1.4.8) method_source (0.8.2) mime-types (1.25.1) - mini_portile2 (2.0.0) + mini_portile2 (2.1.0) momentjs-rails (2.5.1) railties (>= 3.1) money (5.0.0) @@ -465,8 +465,8 @@ GEM multi_json (1.12.1) multi_xml (0.5.5) newrelic_rpm (3.12.0.288) - nokogiri (1.6.7.2) - mini_portile2 (~> 2.0.0.rc2) + nokogiri (1.6.8.1) + mini_portile2 (~> 2.1.0) oj (2.1.2) orm_adapter (0.5.0) paper_trail (3.0.8) @@ -628,7 +628,7 @@ GEM sprockets (>= 2.0.0) turn (0.8.3) ansi - tzinfo (0.3.49) + tzinfo (0.3.52) uglifier (2.7.1) execjs (>= 0.3.0) json (>= 1.8.0) From efe0d3ab22e1cef3148a98bc7c9d250ab083befd Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 11:18:05 +1100 Subject: [PATCH 016/353] Update spree, upgrading jquery-rails to provide jquery 1.9. Fixes ng-tags-input. --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 383caa2bab..119ebe48f9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,7 +30,7 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: 4a0013618dbd5bae254e921565bf51d3dd5ccd24 + revision: f799d0fd62760e59ac097143dcac0ea35a76f001 branch: spree-upgrade-step-2 specs: spree (1.3.99) @@ -54,7 +54,7 @@ GIT deface (>= 0.9.0) ffaker (~> 1.15.0) highline (= 1.6.11) - jquery-rails (~> 2.1.4) + jquery-rails (~> 2.2.0) json (>= 1.5.5) kaminari (= 0.13.0) money (= 5.0.0) @@ -426,7 +426,7 @@ GEM ipaddress (0.8.0) journey (1.0.4) jquery-migrate-rails (1.2.1) - jquery-rails (2.1.4) + jquery-rails (2.2.2) railties (>= 3.0, < 5.0) thor (>= 0.14, < 2.0) json (1.8.3) From 017b63fa71fdf784f681f649b3bbc5b8d21da980 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 12:08:48 +1100 Subject: [PATCH 017/353] Update Spree: Fix further admin authorization errors --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 119ebe48f9..999596fd14 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,7 +30,7 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: f799d0fd62760e59ac097143dcac0ea35a76f001 + revision: 36326940a9c6edd6a76fc83a211c87f4869ae017 branch: spree-upgrade-step-2 specs: spree (1.3.99) From 8eca2602970b2387ce1c18a700188b39b385ab11 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 12:44:07 +1100 Subject: [PATCH 018/353] Update Spree: Prevent duplicate copies of images appearing --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 999596fd14..94175bc1d7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,7 +30,7 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: 36326940a9c6edd6a76fc83a211c87f4869ae017 + revision: 7e17ccea6aea10da3105ecc5d1efb0afe1acf8f2 branch: spree-upgrade-step-2 specs: spree (1.3.99) From 48e50540db68e67560490ce0a61bb6b038438278 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 14:06:07 +1100 Subject: [PATCH 019/353] Start Spree upgrade step 3 --- Gemfile | 2 +- Gemfile.lock | 55 +++++++++++++++++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index 31092561ec..2a1b1bcf05 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem 'i18n-js', '~> 3.0.0' gem 'nokogiri', '>= 1.6.7.1' gem 'pg' -gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-2' +gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-3' gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable' gem 'spree_auth_devise', github: 'spree/spree_auth_devise', branch: '1-3-stable' diff --git a/Gemfile.lock b/Gemfile.lock index 94175bc1d7..fd3f28abc2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,46 +30,65 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: 7e17ccea6aea10da3105ecc5d1efb0afe1acf8f2 - branch: spree-upgrade-step-2 + revision: ad9733e00935dab3f92aa37113a2472d38bd0574 + branch: spree-upgrade-step-3 specs: spree (1.3.99) spree_api (= 1.3.99) + spree_backend (= 1.3.99) spree_cmd (= 1.3.99) spree_core (= 1.3.99) spree_dash (= 1.3.99) - spree_promo (= 1.3.99) + spree_frontend (= 1.3.99) spree_sample (= 1.3.99) spree_api (1.3.99) + rabl (= 0.7.2) spree_core (= 1.3.99) versioncake (= 0.4.0) + spree_backend (1.3.99) + deface (>= 0.9.0) + jquery-rails (~> 2.0) + rails (~> 3.2.8) + select2-rails (~> 3.2) + spree_api (= 1.3.99) + spree_core (= 1.3.99) + stringex (~> 1.3.2) spree_cmd (1.3.99) thor (>= 0.14.6) spree_core (1.3.99) + actionmailer (~> 3.2.9) activemerchant (~> 1.50.0) + activerecord (~> 3.2.9) acts_as_list (= 0.1.4) - awesome_nested_set (= 2.1.5) + awesome_nested_set (= 2.1.4) aws-sdk (~> 1.11.1) cancan (= 1.6.8) deface (>= 0.9.0) ffaker (~> 1.15.0) highline (= 1.6.11) - jquery-rails (~> 2.2.0) + httparty (= 0.9.0) json (>= 1.5.5) kaminari (= 0.13.0) money (= 5.0.0) paperclip (~> 3.0) - rabl (= 0.7.2) rails (~> 3.2.13) + railties (~> 3.2.9) ransack (= 0.7.2) - select2-rails (~> 3.2) state_machine (= 1.2.0) stringex (~> 1.3.2) + truncate_html (= 0.9.2) spree_dash (1.3.99) - httparty (~> 0.8.1) - spree_core (= 1.3.99) - spree_promo (1.3.99) + httparty (~> 0.9.0) + spree_backend (= 1.3.99) + spree_frontend (= 1.3.99) + spree_frontend (1.3.99) + deface (>= 0.9.0) + jquery-rails (~> 2.2.0) + rails (~> 3.2.8) + select2-rails (~> 3.2) + spree_api (= 1.3.99) spree_core (= 1.3.99) + stringex (~> 1.3.2) spree_sample (1.3.99) spree_core (= 1.3.99) @@ -155,7 +174,7 @@ GEM acts-as-taggable-on (3.5.0) activerecord (>= 3.2, < 5) acts_as_list (0.1.4) - addressable (2.3.3) + addressable (2.4.0) andand (1.3.3) angular-rails-templates (0.2.0) railties (>= 3.1) @@ -167,7 +186,7 @@ GEM arel (3.0.3) ast (2.3.0) atomic (1.1.99) - awesome_nested_set (2.1.5) + awesome_nested_set (2.1.4) activerecord (>= 3.0.0) awesome_print (1.0.2) aws-sdk (1.11.1) @@ -226,8 +245,8 @@ GEM compass (~> 1.0.0) sass-rails (<= 5.0.1) sprockets (< 2.13) - crack (0.4.1) - safe_yaml (~> 0.9.0) + crack (0.4.3) + safe_yaml (~> 1.0.0) css_parser (1.3.5) addressable css_splitter (0.4.5) @@ -413,7 +432,7 @@ GEM highline (1.6.11) hike (1.2.3) http_parser.rb (0.5.3) - httparty (0.8.3) + httparty (0.9.0) multi_json (~> 1.0) multi_xml i18n (0.6.11) @@ -622,7 +641,7 @@ GEM treetop (1.4.15) polyglot polyglot (>= 0.3.1) - truncate_html (0.5.5) + truncate_html (0.9.2) turbo-sprockets-rails3 (0.3.6) railties (> 3.2.8, < 4.0.0) sprockets (>= 2.0.0) @@ -647,9 +666,9 @@ GEM railties (>= 3.0) warden (1.2.3) rack (>= 1.0) - webmock (1.13.0) + webmock (1.8.11) addressable (>= 2.2.7) - crack (>= 0.3.2) + crack (>= 0.1.7) websocket-driver (0.6.3) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) From 858576d870b14e0b270d55d94cf80ea002594ea9 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 15:30:22 +1100 Subject: [PATCH 020/353] CalculatedAdjustments requires include first --- app/models/enterprise_fee.rb | 5 ++-- app/models/spree/payment_method_decorator.rb | 9 ++++-- app/models/tag_rule/discount_order.rb | 2 +- .../core/calculated_adjustments_decorator.rb | 30 ++++++++++--------- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index e7e8ee0513..3b32ad0901 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -1,4 +1,6 @@ class EnterpriseFee < ActiveRecord::Base + include Spree::Core::CalculatedAdjustments + belongs_to :enterprise belongs_to :tax_category, class_name: 'Spree::TaxCategory', foreign_key: 'tax_category_id' @@ -14,9 +16,6 @@ class EnterpriseFee < ActiveRecord::Base # coordinator_fees and exchange_fees - calculated_adjustments - - attr_accessible :enterprise_id, :fee_type, :name, :tax_category_id, :calculator_type, :inherits_tax_category FEE_TYPES = %w(packing transport admin sales fundraising) diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb index bdbbc9a8fe..32819544ed 100644 --- a/app/models/spree/payment_method_decorator.rb +++ b/app/models/spree/payment_method_decorator.rb @@ -1,4 +1,6 @@ Spree::PaymentMethod.class_eval do + include Spree::Core::CalculatedAdjustments + Spree::PaymentMethod::DISPLAY = [:both, :front_end, :back_end] acts_as_taggable @@ -7,8 +9,6 @@ Spree::PaymentMethod.class_eval do attr_accessible :distributor_ids, :tag_list - calculated_adjustments - after_initialize :init validates_with DistributorsValidator @@ -39,7 +39,10 @@ Spree::PaymentMethod.class_eval do } def init - self.class.calculated_adjustments unless reflections.keys.include? :calculator + unless reflections.keys.include? :calculator + include Spree::Core::CalculatedAdjustments + end + self.calculator ||= Spree::Calculator::FlatRate.new(preferred_amount: 0) end diff --git a/app/models/tag_rule/discount_order.rb b/app/models/tag_rule/discount_order.rb index 5984814289..8b4a780cbd 100644 --- a/app/models/tag_rule/discount_order.rb +++ b/app/models/tag_rule/discount_order.rb @@ -1,5 +1,5 @@ class TagRule::DiscountOrder < TagRule - calculated_adjustments + include Spree::Core::CalculatedAdjustments private diff --git a/lib/spree/core/calculated_adjustments_decorator.rb b/lib/spree/core/calculated_adjustments_decorator.rb index 6a42eb68c5..6b12b4a64d 100644 --- a/lib/spree/core/calculated_adjustments_decorator.rb +++ b/lib/spree/core/calculated_adjustments_decorator.rb @@ -1,14 +1,16 @@ -module Spree - module Core - module CalculatedAdjustments - module ClassMethods - def calculated_adjustments_with_explicit_class_name - calculated_adjustments_without_explicit_class_name - # Class name is mis-inferred outside of Spree namespace - has_one :calculator, as: :calculable, dependent: :destroy, class_name: 'Spree::Calculator' - end - alias_method_chain :calculated_adjustments, :explicit_class_name - end - end - end -end +# Maybe we don't need this any more? Commenting out during upgrade process + +# module Spree +# module Core +# module CalculatedAdjustments +# module ClassMethods +# def calculated_adjustments_with_explicit_class_name +# calculated_adjustments_without_explicit_class_name +# # Class name is mis-inferred outside of Spree namespace +# has_one :calculator, as: :calculable, dependent: :destroy, class_name: 'Spree::Calculator' +# end +# alias_method_chain :calculated_adjustments, :explicit_class_name +# end +# end +# end +# end From 0446b8d72f88fc7817b3550a49e908768b87eb2d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 4 Nov 2016 15:30:46 +1100 Subject: [PATCH 021/353] TestingSupport moved out of Core namespace --- lib/open_food_network/enterprise_issue_validator.rb | 2 +- spec/factories.rb | 2 +- spec/spec_helper.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/open_food_network/enterprise_issue_validator.rb b/lib/open_food_network/enterprise_issue_validator.rb index f755a6faf1..eccc5a4a68 100644 --- a/lib/open_food_network/enterprise_issue_validator.rb +++ b/lib/open_food_network/enterprise_issue_validator.rb @@ -1,7 +1,7 @@ module OpenFoodNetwork class EnterpriseIssueValidator include Rails.application.routes.url_helpers - include Spree::Core::UrlHelpers + include Spree::TestingSupport::UrlHelpers def initialize(enterprise) @enterprise = enterprise diff --git a/spec/factories.rb b/spec/factories.rb index 3b2f67e531..1faf730017 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -1,5 +1,5 @@ require 'ffaker' -require 'spree/core/testing_support/factories' +require 'spree/testing_support/factories' # http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md # diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 00e0b152ba..22c4a026ff 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,8 +23,8 @@ WebMock.disable_net_connect!(:allow_localhost => true) # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each {|f| require f} -require 'spree/core/testing_support/controller_requests' -require 'spree/core/testing_support/capybara_ext' +require 'spree/testing_support/controller_requests' +require 'spree/testing_support/capybara_ext' require 'spree/api/testing_support/setup' require 'spree/api/testing_support/helpers' require 'spree/api/testing_support/helpers_decorator' @@ -98,7 +98,7 @@ RSpec.configure do |config| config.include Spree::UrlHelpers config.include Spree::CheckoutHelpers config.include Spree::MoneyHelper - config.include Spree::Core::TestingSupport::ControllerRequests, :type => :controller + config.include Spree::TestingSupport::ControllerRequests, :type => :controller config.include Devise::TestHelpers, :type => :controller config.extend Spree::Api::TestingSupport::Setup, :type => :controller config.include Spree::Api::TestingSupport::Helpers, :type => :controller From 3135ef6b7fa9ab38e441b4cfa58872b51da0c53b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 9 Nov 2016 12:47:57 +1100 Subject: [PATCH 022/353] Reinstate explicit class name Spree::Calculator on calculated_adjustments association --- .../core/calculated_adjustments_decorator.rb | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/spree/core/calculated_adjustments_decorator.rb b/lib/spree/core/calculated_adjustments_decorator.rb index 6b12b4a64d..592bf59854 100644 --- a/lib/spree/core/calculated_adjustments_decorator.rb +++ b/lib/spree/core/calculated_adjustments_decorator.rb @@ -1,16 +1,16 @@ -# Maybe we don't need this any more? Commenting out during upgrade process +module Spree + module Core + module CalculatedAdjustments + class << self + def included_with_explicit_class_name(klass) + included_without_explicit_class_name(klass) -# module Spree -# module Core -# module CalculatedAdjustments -# module ClassMethods -# def calculated_adjustments_with_explicit_class_name -# calculated_adjustments_without_explicit_class_name -# # Class name is mis-inferred outside of Spree namespace -# has_one :calculator, as: :calculable, dependent: :destroy, class_name: 'Spree::Calculator' -# end -# alias_method_chain :calculated_adjustments, :explicit_class_name -# end -# end -# end -# end + klass.class_eval do + has_one :calculator, as: :calculable, dependent: :destroy, class_name: 'Spree::Calculator' + end + end + alias_method_chain :included, :explicit_class_name + end + end + end +end From 5ad197278c71daf5b0fedf9633facfa7a8e9ac4d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 9 Nov 2016 14:04:09 +1100 Subject: [PATCH 023/353] Fix rendering of order form for JSON requests --- .../spree/admin/line_items_controller_decorator.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb index 5e95681f95..2d78fe6a1c 100644 --- a/app/controllers/spree/admin/line_items_controller_decorator.rb +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -37,6 +37,12 @@ Spree::Admin::LineItemsController.class_eval do private + def render_order_form + respond_to do |format| + format.html { render 'spree/admin/orders/form', order: @order.reload } + end + end + def load_order @order = Spree::Order.find_by_number!(params[:order_id]) authorize! :update, @order From 0219118e19727d7ea5bcf809c122f30fd2915a53 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 9 Nov 2016 14:37:04 +1100 Subject: [PATCH 024/353] Admin JS/CSS now in spree_backend --- app/assets/javascripts/admin/all.js | 3 +-- app/assets/stylesheets/admin/all.css | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/admin/all.js b/app/assets/javascripts/admin/all.js index 1524268900..34fab997a7 100644 --- a/app/assets/javascripts/admin/all.js +++ b/app/assets/javascripts/admin/all.js @@ -14,9 +14,8 @@ //= require angular-resource //= require angular-animate //= require angular-sanitize -//= require admin/spree_core +//= require admin/spree_backend //= require admin/spree_auth -//= require admin/spree_promo //= require admin/spree_paypal_express //= require ../shared/ng-infinite-scroll.min.js //= require ../shared/ng-tags-input.min.js diff --git a/app/assets/stylesheets/admin/all.css b/app/assets/stylesheets/admin/all.css index e913e9e710..47bf928477 100644 --- a/app/assets/stylesheets/admin/all.css +++ b/app/assets/stylesheets/admin/all.css @@ -4,9 +4,8 @@ * the top of the compiled file, but it's generally better to create a new file per style scope. * - *= require admin/spree_core + *= require admin/spree_backend *= require admin/spree_auth - *= require admin/spree_promo *= require shared/jquery-ui-timepicker-addon *= require shared/textAngular From 3605b610fe8a7e5af06289dd2cde2b502e5eee3d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 9 Nov 2016 14:37:49 +1100 Subject: [PATCH 025/353] Use spree_auth_devise 2-0-stable pretending to be 1.3 --- Gemfile | 2 +- Gemfile.lock | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Gemfile b/Gemfile index 2a1b1bcf05..b526c0f07e 100644 --- a/Gemfile +++ b/Gemfile @@ -12,7 +12,7 @@ gem 'nokogiri', '>= 1.6.7.1' gem 'pg' gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-3' gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable' -gem 'spree_auth_devise', github: 'spree/spree_auth_devise', branch: '1-3-stable' +gem 'spree_auth_devise', github: 'openfoodfoundation/spree_auth_devise', branch: 'spree-upgrade-intermediate' # Our branch contains two changes # - Pass customer email and phone number to PayPal (merged to upstream master) diff --git a/Gemfile.lock b/Gemfile.lock index fd3f28abc2..2b7e656337 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,6 +92,18 @@ GIT spree_sample (1.3.99) spree_core (= 1.3.99) +GIT + remote: git://github.com/openfoodfoundation/spree_auth_devise.git + revision: da9eecefc6fe13dedf4c6f3febec79caad839ec3 + branch: spree-upgrade-intermediate + specs: + spree_auth_devise (2.0.0) + devise (~> 2.2.5) + devise-encryptable (= 0.1.2) + spree_backend (~> 1.3.6) + spree_core (~> 1.3.6) + spree_frontend (~> 1.3.6) + GIT remote: git://github.com/spree/deface.git revision: 1110a1336252109bce7f98f9182042e0bc2930ae @@ -102,17 +114,6 @@ GIT nokogiri (~> 1.6.0) rails (>= 3.1) -GIT - remote: git://github.com/spree/spree_auth_devise.git - revision: ba95589a85368297c844f096c2a0c121e5b08138 - branch: 1-3-stable - specs: - spree_auth_devise (1.3.0) - cancan (~> 1.6.7) - devise (~> 2.2.3) - devise-encryptable (= 0.1.2) - spree_core - GIT remote: git://github.com/spree/spree_i18n.git revision: 752eb67204e9c5a4e22b62591a8fd55fe2285e43 @@ -193,7 +194,7 @@ GEM json (~> 1.4) nokogiri (>= 1.4.4) uuidtools (~> 2.1) - bcrypt (3.1.7) + bcrypt (3.1.11) bcrypt-ruby (3.1.5) bcrypt (>= 3.1.3) blockenspiel (0.4.5) @@ -664,7 +665,7 @@ GEM actionpack (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) - warden (1.2.3) + warden (1.2.6) rack (>= 1.0) webmock (1.8.11) addressable (>= 2.2.7) From 2a44b190fe2420b242627e38a206f1001df82f01 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 9 Nov 2016 14:46:34 +1100 Subject: [PATCH 026/353] Use spree_paypal_express 2-0-stable pretending to be 1.3 --- Gemfile | 2 +- Gemfile.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index b526c0f07e..9d951f292b 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,7 @@ gem 'spree_auth_devise', github: 'openfoodfoundation/spree_auth_devise', branch: # Our branch contains two changes # - Pass customer email and phone number to PayPal (merged to upstream master) # - Change type of password from string to password to hide it in the form -gem 'spree_paypal_express', :github => "openfoodfoundation/better_spree_paypal_express", :branch => "hide-password" +gem 'spree_paypal_express', :github => "openfoodfoundation/better_spree_paypal_express", :branch => "spree-upgrade-intermediate" #gem 'spree_paypal_express', :github => "spree-contrib/better_spree_paypal_express", :branch => "1-3-stable" gem 'delayed_job_active_record' diff --git a/Gemfile.lock b/Gemfile.lock index 2b7e656337..ca96139ae2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -14,12 +14,12 @@ GIT GIT remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git - revision: 840d973cd5bd3250b17674a624dad494aeb09eb3 - branch: hide-password + revision: 8d95f4544c682634812becaf50999fba0cd04df0 + branch: spree-upgrade-intermediate specs: spree_paypal_express (2.0.3) paypal-sdk-merchant (= 1.106.1) - spree_core (~> 1.3.4) + spree_core (~> 1.3.99) GIT remote: git://github.com/openfoodfoundation/ofn-qz.git @@ -678,7 +678,7 @@ GEM chronic (>= 0.6.3) wicked_pdf (1.1.0) wkhtmltopdf-binary (0.12.3.1) - xml-simple (1.1.4) + xml-simple (1.1.5) xpath (2.0.0) nokogiri (~> 1.3) From 2c374b448ae2dd17df1065aeefb8db4bd9a91727 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 18 Nov 2016 12:00:11 +1100 Subject: [PATCH 027/353] check_authorization removed in Spree. Add load_order before_filter to cover our custom actions. --- app/controllers/spree/admin/orders_controller_decorator.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 443eda0538..e84f7217bf 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -4,11 +4,7 @@ Spree::Admin::OrdersController.class_eval do include OpenFoodNetwork::SpreeApiKeyLoader helper CheckoutHelper before_filter :load_spree_api_key, :only => :bulk_management - - # We need to add expections for collection actions other than :index here - # because spree_auth_devise causes load_order to be called, which results - # in an auth failure as the @order object is nil for collection actions - before_filter :check_authorization, except: [:bulk_management, :managed] + before_filter :load_order, only: %i[show edit update fire resend invoice print] before_filter :load_distribution_choices, only: [:new, :edit, :update] From 0a043a6919bef46313bb0b4cc704c6bdfada44e3 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 18 Nov 2016 12:13:39 +1100 Subject: [PATCH 028/353] Add permissions to :line_item resource, now called by Spree --- app/models/spree/ability_decorator.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 6263c4ee51..44a8bc0961 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -1,7 +1,7 @@ class AbilityDecorator include CanCan::Ability - # All abilites are allocated from this initialiser, currently in 5 chunks. + # All abilites are allocated from this initialiser. # Spree also defines other abilities. def initialize(user) add_shopping_abilities user @@ -193,6 +193,7 @@ class AbilityDecorator end can [:admin, :bulk_management, :managed], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor) can [:admin , :for_line_items], Enterprise + can [:admin, :index, :create, :update, :destroy], :line_item can [:admin, :index, :create], Spree::LineItem can [:destroy, :update], Spree::LineItem do |item| order = item.order From 73abb93737bf19b9b8a3e6dc4d74c09c4aceaec5 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 18 Nov 2016 15:46:47 +1100 Subject: [PATCH 029/353] Update Spree: Bring forward simplified version of Spree.t for spree_auth_devise --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index ca96139ae2..79716ac250 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -30,7 +30,7 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: ad9733e00935dab3f92aa37113a2472d38bd0574 + revision: 9e40af3a3e2fe4d9a62b85e99b6fc4e2e1f67581 branch: spree-upgrade-step-3 specs: spree (1.3.99) From 1170897587980ca1a7dd08104bcacd7de2ef680f Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 25 Nov 2016 12:55:12 +1100 Subject: [PATCH 030/353] Add our own JSON handling actions for update and delete. Spree no longer does this for us. --- .../admin/line_items_controller_decorator.rb | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb index 2d78fe6a1c..039b95e3c2 100644 --- a/app/controllers/spree/admin/line_items_controller_decorator.rb +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -34,6 +34,27 @@ Spree::Admin::LineItemsController.class_eval do end end + def update + respond_to do |format| + format.html { render_order_form } + format.json { + if @line_item.update_attributes(params[:line_item]) + render nothing: true, status: 204 # No Content, does not trigger ng resource auto-update + else + render json: {errors: @line_item.errors}, status: 412 + end + } + end + end + + def destroy + @line_item.destroy + + respond_to do |format| + format.html { render_order_form } + format.json { render nothing: true, status: 204 } # No Content + end + end private From 0a8e8dfbbb5db0393524fb267a09b81f4c9ac37b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Dec 2016 11:10:19 +1100 Subject: [PATCH 031/353] Add specs and fix line items HTML response --- .../admin/line_items_controller_decorator.rb | 2 +- .../spree/admin/line_items_controller_spec.rb | 50 +++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb index 039b95e3c2..511ab1b4f6 100644 --- a/app/controllers/spree/admin/line_items_controller_decorator.rb +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -60,7 +60,7 @@ Spree::Admin::LineItemsController.class_eval do def render_order_form respond_to do |format| - format.html { render 'spree/admin/orders/form', order: @order.reload } + format.html { render partial: 'spree/admin/orders/form', locals: {order: @order.reload} } end end diff --git a/spec/controllers/spree/admin/line_items_controller_spec.rb b/spec/controllers/spree/admin/line_items_controller_spec.rb index 907bec2bbb..0838e83b64 100644 --- a/spec/controllers/spree/admin/line_items_controller_spec.rb +++ b/spec/controllers/spree/admin/line_items_controller_spec.rb @@ -165,17 +165,29 @@ describe Spree::Admin::LineItemsController do end context "coordinator enterprise" do + render_views + before do controller.stub spree_current_user: coordinator.owner - spree_put :update, params end - it "updates the line_item" do + it "updates the line item" do + spree_put :update, params line_item1.reload expect(line_item1.quantity).to eq 3 expect(line_item1.final_weight_volume).to eq 3000 expect(line_item1.price).to eq 3.00 end + + it "returns an empty JSON response" do + spree_put :update, params.merge(format: :json) + expect(response.body).to eq ' ' + end + + it "returns an HTML response with the order form" do + spree_put :update, params.merge(format: :html) + expect(response.body).to match /admin_order_form_fields/ + end end context "hub enterprise" do @@ -184,7 +196,7 @@ describe Spree::Admin::LineItemsController do spree_put :update, params end - it "retrieves a list of line_items" do + it "updates the line item" do line_item1.reload expect(line_item1.quantity).to eq 3 expect(line_item1.final_weight_volume).to eq 3000 @@ -193,4 +205,36 @@ describe Spree::Admin::LineItemsController do end end end + + describe "#destroy" do + render_views + + let(:supplier) { create(:supplier_enterprise) } + let(:distributor1) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } + let!(:order1) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let(:params) { { format: :json, id: line_item1.id, order_id: order1.number } } + + before do + controller.stub spree_current_user: coordinator.owner + end + + it "destroys the line item" do + expect { + spree_delete :destroy, params + }.to change { Spree::LineItem.where(id: line_item1).count }.from(1).to(0) + end + + it "returns an empty JSON response" do + spree_delete :destroy, params.merge(format: :json) + expect(response.body).to eq ' ' + end + + it "returns an HTML response with the order form" do + spree_delete :destroy, params.merge(format: :html) + expect(response.body).to match /admin_order_form_fields/ + end + end end From 3ff051f238989cc1f279b1c0c1af7eb33076d52c Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Dec 2016 12:59:53 +1100 Subject: [PATCH 032/353] Restore route for variant search. Fixes admin order product selection. --- app/views/spree/admin/shared/_routes.html.erb | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 app/views/spree/admin/shared/_routes.html.erb diff --git a/app/views/spree/admin/shared/_routes.html.erb b/app/views/spree/admin/shared/_routes.html.erb new file mode 100644 index 0000000000..b0e64466ad --- /dev/null +++ b/app/views/spree/admin/shared/_routes.html.erb @@ -0,0 +1,10 @@ + From c47af55bb966e2465499bb587212d4d46fcd9260 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Dec 2016 13:08:30 +1100 Subject: [PATCH 033/353] Fix: When a user fires an event (eg. capture payment), take them back to where they came from --- .../admin/payments_controller_decorator.rb | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb index 23db5eca09..c608f8930c 100644 --- a/app/controllers/spree/admin/payments_controller_decorator.rb +++ b/app/controllers/spree/admin/payments_controller_decorator.rb @@ -1,21 +1,31 @@ Spree::Admin::PaymentsController.class_eval do - # When a user fires an event, take them back to where they came from - # Responder: http://guides.spreecommerce.com/developer/logic.html#overriding-controller-action-responses - - # For some strange reason, adding PaymentsController.class_eval will cause gems/spree/app/controllers/spree/admin/payments_controller.rb:37 to error: - # payments_url not defined. - # This could be fixed by replacing line 37 with: - # respond_with(@payment, location: admin_order_payments_url) { |format| format.html { redirect_to admin_order_payments_path(@order) } } - respond_override :fire => { :html => { :success => lambda { - redirect_to request.referer # Keeps any filter and sort prefs - } } } - append_before_filter :filter_payment_methods + + # When a user fires an event, take them back to where they came from + # (we can't use respond_override because Spree no longer uses respond_with) + def fire + return unless event = params[:e] and @payment.payment_source + + # Because we have a transition method also called void, we do this to avoid conflicts. + event = "void_transaction" if event == "void" + if @payment.send("#{event}!") + flash[:success] = t(:payment_updated) + else + flash[:error] = t(:cannot_perform_operation) + end + rescue Spree::Core::GatewayError => ge + flash[:error] = "#{ge.message}" + ensure + redirect_to request.referer + end + + + private + # Only show payments for the order's distributor def filter_payment_methods @payment_methods = @payment_methods.select{ |pm| pm.has_distributor? @order.distributor} @payment_method ||= @payment_methods.first end - end From bfaaf16030dd52e37b279934d4108e693744db60 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 23 Dec 2016 15:08:12 +1100 Subject: [PATCH 034/353] Fix spree adjustments fix that maybe never worked? --- app/models/spree/payment_method_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb index 32819544ed..3b9eff2cab 100644 --- a/app/models/spree/payment_method_decorator.rb +++ b/app/models/spree/payment_method_decorator.rb @@ -40,7 +40,7 @@ Spree::PaymentMethod.class_eval do def init unless reflections.keys.include? :calculator - include Spree::Core::CalculatedAdjustments + self.class.include Spree::Core::CalculatedAdjustments end self.calculator ||= Spree::Calculator::FlatRate.new(preferred_amount: 0) From ac8cfe24e64e612feab2a4f13b40bd8b5093179d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 23 Dec 2016 15:10:36 +1100 Subject: [PATCH 035/353] Fix removal of producer properties - remove href attr so Spree doesn't attempt an XHR --- app/helpers/spree/admin/base_helper_decorator.rb | 11 +++++++++++ .../_producer_property_fields.html.haml | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/helpers/spree/admin/base_helper_decorator.rb b/app/helpers/spree/admin/base_helper_decorator.rb index 5398cee35f..c1fd1ab223 100644 --- a/app/helpers/spree/admin/base_helper_decorator.rb +++ b/app/helpers/spree/admin/base_helper_decorator.rb @@ -23,6 +23,17 @@ module Spree link_to_with_icon('icon-trash', name, '#', html_options) + f.hidden_field(:_destroy) end + def link_to_remove_fields_without_url(name, f, options = {}) + name = '' if options[:no_text] + options[:class] = '' unless options[:class] + options[:class] += 'no-text with-tip' if options[:no_text] + + html_options = {class: "remove_fields #{options[:class]}", data: {action: 'remove'}, title: t(:remove)} + html_options.merge!(options[:html]) if options.key? :html + + link_to_with_icon('icon-trash', name, '#', html_options).gsub('href="#" ', '').html_safe + + f.hidden_field(:_destroy) + end end end end diff --git a/app/views/admin/producer_properties/_producer_property_fields.html.haml b/app/views/admin/producer_properties/_producer_property_fields.html.haml index 6fe0b1d7ad..65e6f1a15b 100644 --- a/app/views/admin/producer_properties/_producer_property_fields.html.haml +++ b/app/views/admin/producer_properties/_producer_property_fields.html.haml @@ -12,4 +12,4 @@ = f.text_field :value, :class => 'autocomplete' %td.actions - unless @enterprise.producer_properties.empty? - = link_to_remove_fields t(:remove), f, no_text: true, url: (f.object.persisted? && main_app.admin_enterprise_producer_property_path(@enterprise, f.object)), html: {"onclick" => "if(typeof(enterprise_form) != 'undefined') { angular.element(enterprise_form).scope().setFormDirty() }".html_safe} + = link_to_remove_fields_without_url t(:remove), f, no_text: true, html: {"onclick" => "if(typeof(enterprise_form) != 'undefined') { angular.element(enterprise_form).scope().setFormDirty() }".html_safe} From 5eb42eac3c067e2c3e18801508a31c0d58135a36 Mon Sep 17 00:00:00 2001 From: Em-AK Date: Mon, 3 Apr 2017 18:06:29 +0200 Subject: [PATCH 036/353] Delete the override of a deleted field https://github.com/openfoodfoundation/spree/compare/spree-upgrade-step1c...spree-upgrade-step-3#diff-a63d9d7f4c1375ca141931651f799956 --- .../orders/edit/promo_cart_coupon_code_field.html.haml.deface | 1 - 1 file changed, 1 deletion(-) delete mode 100644 app/overrides/spree/orders/edit/promo_cart_coupon_code_field.html.haml.deface diff --git a/app/overrides/spree/orders/edit/promo_cart_coupon_code_field.html.haml.deface b/app/overrides/spree/orders/edit/promo_cart_coupon_code_field.html.haml.deface deleted file mode 100644 index dca5725b49..0000000000 --- a/app/overrides/spree/orders/edit/promo_cart_coupon_code_field.html.haml.deface +++ /dev/null @@ -1 +0,0 @@ -/ disabled From d68104cecb5ddf456a7e1f36158b6ffc28c9ebce Mon Sep 17 00:00:00 2001 From: Em-AK Date: Mon, 3 Apr 2017 20:16:29 +0200 Subject: [PATCH 037/353] Update a dependency to run the specs --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 79716ac250..49c003ee9b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -612,7 +612,7 @@ GEM unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.8.1) rubyzip (1.2.0) - safe_yaml (0.9.5) + safe_yaml (1.0.4) sass (3.3.14) sass-rails (3.2.6) railties (~> 3.2.0) From f22a12d6573b1c7959994abd0180c8ccd0b4cc8f Mon Sep 17 00:00:00 2001 From: Em-AK Date: Tue, 4 Apr 2017 11:41:23 +0200 Subject: [PATCH 038/353] Revert later: depend on our spree fork --- Gemfile | 2 +- Gemfile.lock | 50 +++++++++++++++++++++++++------------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/Gemfile b/Gemfile index 9d951f292b..16c51b37b7 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem 'i18n-js', '~> 3.0.0' gem 'nokogiri', '>= 1.6.7.1' gem 'pg' -gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-3' +gem 'spree', github: 'coopdevs/spree', branch: 'spree-upgrade-step-3' gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable' gem 'spree_auth_devise', github: 'openfoodfoundation/spree_auth_devise', branch: 'spree-upgrade-intermediate' diff --git a/Gemfile.lock b/Gemfile.lock index 49c003ee9b..b25de07c89 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,30 +7,8 @@ GIT activemodel (~> 3.0) GIT - remote: git://github.com/jeremydurham/custom-err-msg.git - revision: 3a8ec9dddc7a5b0aab7c69a6060596de300c68f4 - specs: - custom_error_message (1.1.1) - -GIT - remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git - revision: 8d95f4544c682634812becaf50999fba0cd04df0 - branch: spree-upgrade-intermediate - specs: - spree_paypal_express (2.0.3) - paypal-sdk-merchant (= 1.106.1) - spree_core (~> 1.3.99) - -GIT - remote: git://github.com/openfoodfoundation/ofn-qz.git - revision: 024680ccea429b2e5428d7b964fa67c52add34ec - specs: - ofn-qz (0.1.0) - railties (~> 3.1) - -GIT - remote: git://github.com/openfoodfoundation/spree.git - revision: 9e40af3a3e2fe4d9a62b85e99b6fc4e2e1f67581 + remote: git://github.com/coopdevs/spree.git + revision: 60bfa21ffecafa37f9ae0e27dbb005e0f20162a6 branch: spree-upgrade-step-3 specs: spree (1.3.99) @@ -92,6 +70,28 @@ GIT spree_sample (1.3.99) spree_core (= 1.3.99) +GIT + remote: git://github.com/jeremydurham/custom-err-msg.git + revision: 3a8ec9dddc7a5b0aab7c69a6060596de300c68f4 + specs: + custom_error_message (1.1.1) + +GIT + remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git + revision: 8d95f4544c682634812becaf50999fba0cd04df0 + branch: spree-upgrade-intermediate + specs: + spree_paypal_express (2.0.3) + paypal-sdk-merchant (= 1.106.1) + spree_core (~> 1.3.99) + +GIT + remote: git://github.com/openfoodfoundation/ofn-qz.git + revision: 024680ccea429b2e5428d7b964fa67c52add34ec + specs: + ofn-qz (0.1.0) + railties (~> 3.1) + GIT remote: git://github.com/openfoodfoundation/spree_auth_devise.git revision: da9eecefc6fe13dedf4c6f3febec79caad839ec3 @@ -483,7 +483,7 @@ GEM i18n (~> 0.4) json multi_json (1.12.1) - multi_xml (0.5.5) + multi_xml (0.6.0) newrelic_rpm (3.12.0.288) nokogiri (1.6.8.1) mini_portile2 (~> 2.1.0) From e73330d59793216f64068c8c84105ca6562dd1c8 Mon Sep 17 00:00:00 2001 From: Em-AK Date: Tue, 4 Apr 2017 16:55:36 +0200 Subject: [PATCH 039/353] Fix removal of product properties Prevent spree to make a post request on deletion of a property The data should be updated on the server by clicking on the Update button --- .../replace_link_to_remove_fields.html.haml.deface | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 app/overrides/spree/admin/product_properties/_product_property_fields/replace_link_to_remove_fields.html.haml.deface diff --git a/app/overrides/spree/admin/product_properties/_product_property_fields/replace_link_to_remove_fields.html.haml.deface b/app/overrides/spree/admin/product_properties/_product_property_fields/replace_link_to_remove_fields.html.haml.deface new file mode 100644 index 0000000000..b59f15ce72 --- /dev/null +++ b/app/overrides/spree/admin/product_properties/_product_property_fields/replace_link_to_remove_fields.html.haml.deface @@ -0,0 +1,4 @@ +/ replace_contents "[data-hook='product_property'] .actions" + +- unless @product.properties.empty? + = link_to_remove_fields_without_url t(:remove), f, no_text: true From 1136cb40a92cb288445d56c41e322395ae7b465a Mon Sep 17 00:00:00 2001 From: Em-AK Date: Wed, 5 Apr 2017 11:13:17 +0200 Subject: [PATCH 040/353] Revert to OFN spree fork This reverts commit 92f50d96803f9d0f302be51aad085c9a68ec4c32. And bundle update to the last commit of openfoodfoundation/spree, branch: spree-upgrade-step-3 --- Gemfile | 2 +- Gemfile.lock | 61 ++++++++++++++++++++++++++-------------------------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/Gemfile b/Gemfile index 16c51b37b7..9d951f292b 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem 'i18n-js', '~> 3.0.0' gem 'nokogiri', '>= 1.6.7.1' gem 'pg' -gem 'spree', github: 'coopdevs/spree', branch: 'spree-upgrade-step-3' +gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-3' gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable' gem 'spree_auth_devise', github: 'openfoodfoundation/spree_auth_devise', branch: 'spree-upgrade-intermediate' diff --git a/Gemfile.lock b/Gemfile.lock index b25de07c89..d95c210710 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,8 +7,30 @@ GIT activemodel (~> 3.0) GIT - remote: git://github.com/coopdevs/spree.git - revision: 60bfa21ffecafa37f9ae0e27dbb005e0f20162a6 + remote: git://github.com/jeremydurham/custom-err-msg.git + revision: 3a8ec9dddc7a5b0aab7c69a6060596de300c68f4 + specs: + custom_error_message (1.1.1) + +GIT + remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git + revision: 8d95f4544c682634812becaf50999fba0cd04df0 + branch: spree-upgrade-intermediate + specs: + spree_paypal_express (2.0.3) + paypal-sdk-merchant (= 1.106.1) + spree_core (~> 1.3.99) + +GIT + remote: git://github.com/openfoodfoundation/ofn-qz.git + revision: 024680ccea429b2e5428d7b964fa67c52add34ec + specs: + ofn-qz (0.1.0) + railties (~> 3.1) + +GIT + remote: git://github.com/openfoodfoundation/spree.git + revision: 20915c4dc9738392a236265f2e571b709e2d1c9f branch: spree-upgrade-step-3 specs: spree (1.3.99) @@ -70,28 +92,6 @@ GIT spree_sample (1.3.99) spree_core (= 1.3.99) -GIT - remote: git://github.com/jeremydurham/custom-err-msg.git - revision: 3a8ec9dddc7a5b0aab7c69a6060596de300c68f4 - specs: - custom_error_message (1.1.1) - -GIT - remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git - revision: 8d95f4544c682634812becaf50999fba0cd04df0 - branch: spree-upgrade-intermediate - specs: - spree_paypal_express (2.0.3) - paypal-sdk-merchant (= 1.106.1) - spree_core (~> 1.3.99) - -GIT - remote: git://github.com/openfoodfoundation/ofn-qz.git - revision: 024680ccea429b2e5428d7b964fa67c52add34ec - specs: - ofn-qz (0.1.0) - railties (~> 3.1) - GIT remote: git://github.com/openfoodfoundation/spree_auth_devise.git revision: da9eecefc6fe13dedf4c6f3febec79caad839ec3 @@ -215,8 +215,7 @@ GEM timers (~> 1.1.0) chronic (0.10.2) chunky_png (1.3.4) - climate_control (0.0.3) - activesupport (>= 3.0) + climate_control (0.1.0) cliver (0.3.2) cocaine (0.5.8) climate_control (>= 0.0.3, < 1.0) @@ -449,7 +448,7 @@ GEM jquery-rails (2.2.2) railties (>= 3.0, < 5.0) thor (>= 0.14, < 2.0) - json (1.8.3) + json (1.8.6) json_spec (1.1.1) multi_json (~> 1.0) rspec (~> 2.0) @@ -528,7 +527,7 @@ GEM activesupport (>= 2.3.14) multi_json (~> 1.0) rack (1.4.7) - rack-cache (1.6.1) + rack-cache (1.7.0) rack (>= 0.4) rack-livereload (0.3.15) rack @@ -557,7 +556,7 @@ GEM rainbow (2.2.2) rake raindrops (0.13.0) - rake (11.3.0) + rake (12.0.0) ransack (0.7.2) actionpack (~> 3.0) activerecord (~> 3.0) @@ -635,7 +634,7 @@ GEM therubyracer (0.12.0) libv8 (~> 3.16.14.0) ref - thor (0.19.1) + thor (0.19.4) tilt (1.4.1) timecop (0.8.1) timers (1.1.0) @@ -648,7 +647,7 @@ GEM sprockets (>= 2.0.0) turn (0.8.3) ansi - tzinfo (0.3.52) + tzinfo (0.3.53) uglifier (2.7.1) execjs (>= 0.3.0) json (>= 1.8.0) From 513330cfffc5fa6ce48c78961e8fed72b3519979 Mon Sep 17 00:00:00 2001 From: enricostano Date: Wed, 5 Apr 2017 14:23:18 +0200 Subject: [PATCH 041/353] Add JSON api for products clone --- .../admin/products_controller_decorator.rb | 4 -- .../api/products_controller_decorator.rb | 11 ++++++ config/routes.rb | 1 + .../spree/api/products_controller_spec.rb | 38 +++++++++++++++++++ 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 7929aa1df6..59c4cd3069 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -8,9 +8,6 @@ Spree::Admin::ProductsController.class_eval do before_filter :load_spree_api_key, :only => [:bulk_edit, :variant_overrides] before_filter :strip_new_properties, only: [:create, :update] - - respond_to :json, :only => :clone - respond_override create: { html: { success: lambda { if params[:button] == "add_another" @@ -22,7 +19,6 @@ Spree::Admin::ProductsController.class_eval do failure: lambda { render :new } } } - #respond_override :clone => { :json => {:success => lambda { redirect_to bulk_index_admin_products_url+"?q[id_eq]=#{@new.id}" } } } def product_distributions end diff --git a/app/controllers/spree/api/products_controller_decorator.rb b/app/controllers/spree/api/products_controller_decorator.rb index e809dbc23e..ee3cc9e70b 100644 --- a/app/controllers/spree/api/products_controller_decorator.rb +++ b/app/controllers/spree/api/products_controller_decorator.rb @@ -37,6 +37,17 @@ Spree::Api::ProductsController.class_eval do respond_with(@product, :status => 204) end + # POST /api/products/:product_id/clone + # + def clone + authorize! :create, Spree::Product + original_product = find_product(params[:product_id]) + authorize! :update, original_product + + @product = original_product.duplicate + + respond_with(@product, status: 201, default_template: :show) + end private diff --git a/config/routes.rb b/config/routes.rb index 52b23758ab..02c459cea0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -237,6 +237,7 @@ Spree::Core::Engine.routes.prepend do get :overridable end delete :soft_delete + post :clone resources :variants do delete :soft_delete diff --git a/spec/controllers/spree/api/products_controller_spec.rb b/spec/controllers/spree/api/products_controller_spec.rb index 312c6a29b4..1969a0abaf 100644 --- a/spec/controllers/spree/api/products_controller_spec.rb +++ b/spec/controllers/spree/api/products_controller_spec.rb @@ -107,5 +107,43 @@ module Spree product1.deleted_at.should_not be_nil end end + + describe '#clone' do + before do + spree_post :clone, product_id: product1.id, format: :json + end + + context 'as a normal user' do + sign_in_as_user! + + it 'denies access' do + assert_unauthorized! + end + end + + context 'as an enterprise user' do + sign_in_as_enterprise_user! [:supplier] + + it 'responds with a successful response' do + expect(response.status).to eq(201) + end + + it 'clones the product' do + expect(json_response['name']).to eq("COPY OF #{product1.name}") + end + end + + context 'as an administrator' do + sign_in_as_admin! + + it 'responds with a successful response' do + expect(response.status).to eq(201) + end + + it 'clones the product' do + expect(json_response['name']).to eq("COPY OF #{product1.name}") + end + end + end end end From c9920959597d02473ff6491967b39e82867aecb3 Mon Sep 17 00:00:00 2001 From: enricostano Date: Wed, 5 Apr 2017 14:23:47 +0200 Subject: [PATCH 042/353] Use new /clone JSON endpoint --- .../admin/services/bulk_products.js.coffee | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/admin/services/bulk_products.js.coffee b/app/assets/javascripts/admin/services/bulk_products.js.coffee index 4e06a87bd2..0d2de5ce77 100644 --- a/app/assets/javascripts/admin/services/bulk_products.js.coffee +++ b/app/assets/javascripts/admin/services/bulk_products.js.coffee @@ -1,4 +1,4 @@ -angular.module("ofn.admin").factory "BulkProducts", (PagedFetcher, dataFetcher) -> +angular.module("ofn.admin").factory "BulkProducts", (PagedFetcher, dataFetcher, $http) -> new class BulkProducts products: [] @@ -11,14 +11,8 @@ angular.module("ofn.admin").factory "BulkProducts", (PagedFetcher, dataFetcher) PagedFetcher.fetch url, (data) => @addProducts data.products cloneProduct: (product) -> - dataFetcher("/admin/products/" + product.permalink_live + "/clone.json").then (data) => - # Ideally we would use Spree's built in respond_override helper here to redirect the - # user after a successful clone with .json in the accept headers - # However, at the time of writing there appears to be an issue which causes the - # respond_with block in the destroy action of Spree::Admin::Product to break - # when a respond_overrride for the clone action is used. - id = data.product.id - dataFetcher("/api/products/" + id + "?template=bulk_show").then (newProduct) => + $http.post("/api/products/" + product.id + "/clone").success (data) => + dataFetcher("/api/products/" + data.id + "?template=bulk_show").then (newProduct) => @unpackProduct newProduct @insertProductAfter(product, newProduct) From 31f2551116d50c69304905ab05817a551bea5e82 Mon Sep 17 00:00:00 2001 From: Em-AK Date: Thu, 6 Apr 2017 12:23:14 +0200 Subject: [PATCH 043/353] Update the JS unit spec to the new endpoint --- Gemfile.lock | 2 +- .../admin/services/bulk_products_spec.js.coffee | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d95c210710..f9181e6a0a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -556,7 +556,7 @@ GEM rainbow (2.2.2) rake raindrops (0.13.0) - rake (12.0.0) + rake (10.5.0) ransack (0.7.2) actionpack (~> 3.0) activerecord (~> 3.0) diff --git a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee index 7d4c5d1d7c..fc46c2a30d 100644 --- a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee @@ -38,18 +38,14 @@ describe "BulkProducts service", -> describe "cloning products", -> - it "clones products using a http get request to /admin/products/(permalink)/clone.json", -> + it "clones products using a http post request to /api/products/(id)/clone", -> BulkProducts.products = [ id: 13 - permalink_live: "oranges" ] - $httpBackend.expectGET("/admin/products/oranges/clone.json").respond 200, - product: - id: 17 - name: "new_product" + $httpBackend.expectPOST("/api/products/13/clone").respond 201, + id: 17 $httpBackend.expectGET("/api/products/17?template=bulk_show").respond 200, [ id: 17 - name: "new_product" ] BulkProducts.cloneProduct BulkProducts.products[0] $httpBackend.flush() @@ -57,15 +53,13 @@ describe "BulkProducts service", -> it "adds the product", -> originalProduct = id: 16 - permalink_live: "oranges" clonedProduct = id: 17 spyOn(BulkProducts, "insertProductAfter") spyOn(BulkProducts, "unpackProduct") BulkProducts.products = [originalProduct] - $httpBackend.expectGET("/admin/products/oranges/clone.json").respond 200, - product: clonedProduct + $httpBackend.expectPOST("/api/products/16/clone").respond 201, clonedProduct $httpBackend.expectGET("/api/products/17?template=bulk_show").respond 200, clonedProduct BulkProducts.cloneProduct BulkProducts.products[0] $httpBackend.flush() From 8a42b606eb61b3c08235fcabdff84865afab101b Mon Sep 17 00:00:00 2001 From: Em-AK Date: Thu, 6 Apr 2017 10:48:03 +0200 Subject: [PATCH 044/353] Start spree upgrade step 4 --- Gemfile | 2 +- Gemfile.lock | 74 ++++++++++++++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/Gemfile b/Gemfile index 9d951f292b..0aa2a2fbe9 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem 'i18n-js', '~> 3.0.0' gem 'nokogiri', '>= 1.6.7.1' gem 'pg' -gem 'spree', github: 'openfoodfoundation/spree', branch: 'spree-upgrade-step-3' +gem 'spree', github: 'coopdevs/spree', branch: 'spree-upgrade-step-4' gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable' gem 'spree_auth_devise', github: 'openfoodfoundation/spree_auth_devise', branch: 'spree-upgrade-intermediate' diff --git a/Gemfile.lock b/Gemfile.lock index f9181e6a0a..674487c6c7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,31 +7,9 @@ GIT activemodel (~> 3.0) GIT - remote: git://github.com/jeremydurham/custom-err-msg.git - revision: 3a8ec9dddc7a5b0aab7c69a6060596de300c68f4 - specs: - custom_error_message (1.1.1) - -GIT - remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git - revision: 8d95f4544c682634812becaf50999fba0cd04df0 - branch: spree-upgrade-intermediate - specs: - spree_paypal_express (2.0.3) - paypal-sdk-merchant (= 1.106.1) - spree_core (~> 1.3.99) - -GIT - remote: git://github.com/openfoodfoundation/ofn-qz.git - revision: 024680ccea429b2e5428d7b964fa67c52add34ec - specs: - ofn-qz (0.1.0) - railties (~> 3.1) - -GIT - remote: git://github.com/openfoodfoundation/spree.git - revision: 20915c4dc9738392a236265f2e571b709e2d1c9f - branch: spree-upgrade-step-3 + remote: git://github.com/coopdevs/spree.git + revision: f60e40a5f148f669dedd63e17812290d38b45a48 + branch: spree-upgrade-step-4 specs: spree (1.3.99) spree_api (= 1.3.99) @@ -59,17 +37,17 @@ GIT actionmailer (~> 3.2.9) activemerchant (~> 1.50.0) activerecord (~> 3.2.9) - acts_as_list (= 0.1.4) - awesome_nested_set (= 2.1.4) + acts_as_list (= 0.1.9) + awesome_nested_set (= 2.1.5) aws-sdk (~> 1.11.1) cancan (= 1.6.8) deface (>= 0.9.0) ffaker (~> 1.15.0) - highline (= 1.6.11) + highline (= 1.6.15) httparty (= 0.9.0) json (>= 1.5.5) kaminari (= 0.13.0) - money (= 5.0.0) + money (= 5.1.0) paperclip (~> 3.0) rails (~> 3.2.13) railties (~> 3.2.9) @@ -83,7 +61,7 @@ GIT spree_frontend (= 1.3.99) spree_frontend (1.3.99) deface (>= 0.9.0) - jquery-rails (~> 2.2.0) + jquery-rails (~> 2.2.1) rails (~> 3.2.8) select2-rails (~> 3.2) spree_api (= 1.3.99) @@ -92,6 +70,28 @@ GIT spree_sample (1.3.99) spree_core (= 1.3.99) +GIT + remote: git://github.com/jeremydurham/custom-err-msg.git + revision: 3a8ec9dddc7a5b0aab7c69a6060596de300c68f4 + specs: + custom_error_message (1.1.1) + +GIT + remote: git://github.com/openfoodfoundation/better_spree_paypal_express.git + revision: 8d95f4544c682634812becaf50999fba0cd04df0 + branch: spree-upgrade-intermediate + specs: + spree_paypal_express (2.0.3) + paypal-sdk-merchant (= 1.106.1) + spree_core (~> 1.3.99) + +GIT + remote: git://github.com/openfoodfoundation/ofn-qz.git + revision: 024680ccea429b2e5428d7b964fa67c52add34ec + specs: + ofn-qz (0.1.0) + railties (~> 3.1) + GIT remote: git://github.com/openfoodfoundation/spree_auth_devise.git revision: da9eecefc6fe13dedf4c6f3febec79caad839ec3 @@ -174,7 +174,7 @@ GEM multi_json (~> 1.0) acts-as-taggable-on (3.5.0) activerecord (>= 3.2, < 5) - acts_as_list (0.1.4) + acts_as_list (0.1.9) addressable (2.4.0) andand (1.3.3) angular-rails-templates (0.2.0) @@ -187,7 +187,7 @@ GEM arel (3.0.3) ast (2.3.0) atomic (1.1.99) - awesome_nested_set (2.1.4) + awesome_nested_set (2.1.5) activerecord (>= 3.0.0) awesome_print (1.0.2) aws-sdk (1.11.1) @@ -429,7 +429,7 @@ GEM rspec (~> 2.14) haml (4.0.4) tilt - highline (1.6.11) + highline (1.6.15) hike (1.2.3) http_parser.rb (0.5.3) httparty (0.9.0) @@ -478,9 +478,8 @@ GEM mini_portile2 (2.1.0) momentjs-rails (2.5.1) railties (>= 3.1) - money (5.0.0) - i18n (~> 0.4) - json + money (5.1.0) + i18n (~> 0.6.0) multi_json (1.12.1) multi_xml (0.6.0) newrelic_rpm (3.12.0.288) @@ -593,8 +592,9 @@ GEM rspec-expectations (2.14.0) diff-lcs (>= 1.1.3, < 2.0) rspec-mocks (2.14.2) - rspec-rails (2.14.0) + rspec-rails (2.14.2) actionpack (>= 3.0) + activemodel (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) rspec-core (~> 2.14.0) From b5bac722cadd1cb66bf349656885ec650d6570a9 Mon Sep 17 00:00:00 2001 From: Em-AK Date: Thu, 6 Apr 2017 11:13:25 +0200 Subject: [PATCH 045/353] Fix: make rspec run --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 22c4a026ff..e24d2a7cf7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -28,7 +28,7 @@ require 'spree/testing_support/capybara_ext' require 'spree/api/testing_support/setup' require 'spree/api/testing_support/helpers' require 'spree/api/testing_support/helpers_decorator' -require 'spree/core/testing_support/authorization_helpers' +require 'spree/testing_support/authorization_helpers' # Capybara config require 'capybara/poltergeist' From a9966f48af28a0b859bf14c7d4272919c2068618 Mon Sep 17 00:00:00 2001 From: enricostano Date: Thu, 6 Apr 2017 11:48:39 +0200 Subject: [PATCH 046/353] Use new product_search method --- app/views/spree/admin/shared/_routes.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/shared/_routes.html.erb b/app/views/spree/admin/shared/_routes.html.erb index b0e64466ad..e4d1cf1502 100644 --- a/app/views/spree/admin/shared/_routes.html.erb +++ b/app/views/spree/admin/shared/_routes.html.erb @@ -3,7 +3,7 @@ :variants_search => admin_search_variants_path(:format => 'json'), :taxon_search => spree.api_taxons_path(:format => 'json'), :user_search => spree.admin_search_users_path(:format => 'json'), - :product_search => spree.search_admin_products_path(:format => 'json'), + :product_search => spree.api_products_path(:format => 'json'), :option_type_search => spree.api_option_types_path(:format => 'json'), :states_search => spree.api_states_path(:format => 'json') }.to_json %>; From 5ad0f8bb2e7cfc6e21e07ed50fbd36afdd9abaf5 Mon Sep 17 00:00:00 2001 From: enricostano Date: Thu, 6 Apr 2017 11:57:52 +0200 Subject: [PATCH 047/353] Fix module namespace --- spec/features/admin/overview_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/overview_spec.rb b/spec/features/admin/overview_spec.rb index 30f2cc0336..31591349c7 100644 --- a/spec/features/admin/overview_spec.rb +++ b/spec/features/admin/overview_spec.rb @@ -5,8 +5,8 @@ feature %q{ I want to be given information about the state of my enterprises, products and order cycles }, js: true do include AuthenticationWorkflow - include AuthorizationHelpers include WebHelper + include ::Spree::TestingSupport::AuthorizationHelpers context "as an enterprise user" do before do From d2b6a47ae48c45443956e9a1b8851927822da139 Mon Sep 17 00:00:00 2001 From: enricostano Date: Thu, 6 Apr 2017 12:13:54 +0200 Subject: [PATCH 048/353] Use new #available_to_order API --- app/models/spree/shipping_method_decorator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/shipping_method_decorator.rb b/app/models/spree/shipping_method_decorator.rb index 29e6e2c46a..453c1fadf2 100644 --- a/app/models/spree/shipping_method_decorator.rb +++ b/app/models/spree/shipping_method_decorator.rb @@ -42,8 +42,8 @@ Spree::ShippingMethod.class_eval do ] end - def available_to_order_with_distributor_check?(order, display_on=nil) - available_to_order_without_distributor_check?(order, display_on) && + def available_to_order_with_distributor_check?(order) + available_to_order_without_distributor_check?(order) && self.distributors.include?(order.distributor) end alias_method_chain :available_to_order?, :distributor_check From a80d75e7e2d33eef1f1ab788fdf196e95d3b2331 Mon Sep 17 00:00:00 2001 From: enricostano Date: Thu, 6 Apr 2017 13:09:54 +0200 Subject: [PATCH 049/353] Fix Spree taxons route reference --- app/views/spree/admin/shared/_routes.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/shared/_routes.html.erb b/app/views/spree/admin/shared/_routes.html.erb index e4d1cf1502..27957a3466 100644 --- a/app/views/spree/admin/shared/_routes.html.erb +++ b/app/views/spree/admin/shared/_routes.html.erb @@ -1,7 +1,7 @@ + + + From 10bbc5f9efd849f66273f941f808995ab64003db Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 Sep 2017 16:16:44 +1000 Subject: [PATCH 326/353] Hide the menu on embedded group pages --- app/controllers/groups_controller.rb | 1 + app/helpers/application_helper.rb | 6 ++++++ app/views/layouts/darkswarm.html.haml | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 0a3b46b342..ce72038e70 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -7,6 +7,7 @@ class GroupsController < BaseController def show enable_embedded_shopfront + @hide_menu = true if @shopfront_layout == 'embedded' @group = EnterpriseGroup.find_by_permalink(params[:id]) || EnterpriseGroup.find(params[:id]) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a9abf266dc..bec9d181c9 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -20,4 +20,10 @@ module ApplicationHelper super end end + + def body_classes + classes = [] + classes << "off-canvas" unless @hide_menu + classes << @shopfront_layout + end end diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index 5bd57ea8e6..c702848e89 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -22,7 +22,7 @@ = render "layouts/bugherd_script" = csrf_meta_tags - %body.off-canvas{class: @shopfront_layout, ng: {app: "Darkswarm"}} + %body{class: body_classes, ng: {app: "Darkswarm"}} / [if lte IE 8] = render partial: "shared/ie_warning" = javascript_include_tag "iehack" @@ -39,7 +39,7 @@ .off-canvas-wrap{offcanvas: true} .inner-wrap - = render "shared/menu/menu" + = render "shared/menu/menu" unless @hide_menu %section{ role: "main" } = yield From 6ad7c7b835856d4e4fa6d3a22a105037b3d5a90b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 22 Sep 2017 15:47:28 +1000 Subject: [PATCH 327/353] Tidy up HTML indent --- public/embedded-group-preview.html | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/public/embedded-group-preview.html b/public/embedded-group-preview.html index 70b28777e1..a15a5c6a17 100644 --- a/public/embedded-group-preview.html +++ b/public/embedded-group-preview.html @@ -1,20 +1,20 @@ Embedded Group - + -

- This is a preview page for embedded groups. - Choose a group to display by copying its permalink id into the URL after the question mark. - Example: embedded-group-preview.html?flavour-crusader -

+

+ This is a preview page for embedded groups. + Choose a group to display by copying its permalink id into the URL after the question mark. + Example: embedded-group-preview.html?flavour-crusader +

- - + + - + From 4dd71c12404554053da2d78b3a3a7e0cba6335fe Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 6 Oct 2017 15:48:44 +1100 Subject: [PATCH 328/353] Add CSS workaround to display repeated table head Fixes https://github.com/openfoodfoundation/openfoodnetwork/issues/1738 --- app/assets/stylesheets/mail/email.css.scss | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/assets/stylesheets/mail/email.css.scss b/app/assets/stylesheets/mail/email.css.scss index 0b2f641b4f..10a16941dd 100644 --- a/app/assets/stylesheets/mail/email.css.scss +++ b/app/assets/stylesheets/mail/email.css.scss @@ -408,3 +408,12 @@ ul { display: inline-block; margin: 0px; } + +/* + * Fix overlapping table header on second page of long invoices. + * Problem description: https://github.com/openfoodfoundation/openfoodnetwork/issues/1738 + * Solution: https://github.com/wkhtmltopdf/wkhtmltopdf/issues/1770#issuecomment-73530576 + */ +thead { display: table-header-group } +tfoot { display: table-row-group } +tr { page-break-inside: avoid } From d147d2035d2d4688a6b6965f2273255b600178d1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sat, 7 Oct 2017 11:50:59 +0100 Subject: [PATCH 329/353] Disable symbol array cop --- .rubocop.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 04839c512d..2e664e7c2b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -143,6 +143,10 @@ Style/WordArray: Enabled: false StyleGuide: http://relaxed.ruby.style/#stylewordarray +Style/SymbolArray: + Enabled: false + StyleGuide: https://rubocop.readthedocs.io/en/latest/cops_style/#stylesymbolarray + Lint/AmbiguousRegexpLiteral: Enabled: false StyleGuide: http://relaxed.ruby.style/#lintambiguousregexpliteral From c747bb5305bd64adf95388cb4f4dec4616ed7123 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 11 Oct 2017 15:51:16 +1100 Subject: [PATCH 330/353] Remove improper use of quick_login_as_admin from unit spec --- spec/models/spree/user_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index fb75264550..ea22d42ec0 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -1,6 +1,4 @@ describe Spree.user_class do - include AuthenticationWorkflow - describe "associations" do it { should have_many(:owned_enterprises) } @@ -95,7 +93,7 @@ describe Spree.user_class do end describe "as admin" do - let(:admin) { quick_login_as_admin } + let(:admin) { create(:admin_user) } it "returns all users" do expect(admin.known_users).to include u1, u2, u3 From 23d2b3a664423e3e6d45acf47f20a26023b72b61 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 11 Oct 2017 16:38:19 +1100 Subject: [PATCH 331/353] Move Stripe webhook logic into dedicated frontend controller --- .../admin/stripe_accounts_controller.rb | 13 ----- app/controllers/stripe_controller.rb | 14 ++++++ config/routes.rb | 5 +- .../admin/stripe_accounts_controller_spec.rb | 45 ----------------- spec/controllers/stripe_controller_spec.rb | 48 +++++++++++++++++++ 5 files changed, 66 insertions(+), 59 deletions(-) create mode 100644 app/controllers/stripe_controller.rb create mode 100644 spec/controllers/stripe_controller_spec.rb diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb index d862664b02..e23ab7ffb4 100644 --- a/app/controllers/admin/stripe_accounts_controller.rb +++ b/app/controllers/admin/stripe_accounts_controller.rb @@ -42,19 +42,6 @@ module Admin redirect_to spree.admin_path end - def deauthorize - # TODO is there a sensible way to confirm this webhook call is actually from Stripe? - event = Stripe::Event.construct_from(params) - return render nothing: true, status: 204 unless event.type == "account.application.deauthorized" - - destroyed = StripeAccount.where(stripe_user_id: event.account).destroy_all - if destroyed.any? - render text: "Account #{event.account} deauthorized", status: 200 - else - render nothing: true, status: 204 - end - end - def status return render json: { status: :stripe_disabled } unless Spree::Config.stripe_connect_enabled stripe_account = StripeAccount.find_by_enterprise_id(params[:enterprise_id]) diff --git a/app/controllers/stripe_controller.rb b/app/controllers/stripe_controller.rb new file mode 100644 index 0000000000..dd5b8e5639 --- /dev/null +++ b/app/controllers/stripe_controller.rb @@ -0,0 +1,14 @@ +class StripeController < BaseController + def deauthorize + # TODO is there a sensible way to confirm this webhook call is actually from Stripe? + event = Stripe::Event.construct_from(params) + return render nothing: true, status: 204 unless event.type == "account.application.deauthorized" + + destroyed = StripeAccount.where(stripe_user_id: event.account).destroy_all + if destroyed.any? + render text: "Account #{event.account} deauthorized", status: 200 + else + render nothing: true, status: 204 + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 9ed42772e5..64243e39a3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -53,6 +53,10 @@ Openfoodnetwork::Application.routes.draw do end end + resource :stripe, only: [] do + post :deauthorize + end + namespace :admin do resources :bulk_line_items end @@ -171,7 +175,6 @@ Openfoodnetwork::Application.routes.draw do get :connect, on: :collection get :connect_callback, on: :collection get :status, on: :collection - post :deauthorize, on: :collection end end diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb index 13ea627499..96a17e02d8 100644 --- a/spec/controllers/admin/stripe_accounts_controller_spec.rb +++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb @@ -90,51 +90,6 @@ describe Admin::StripeAccountsController, type: :controller do end end - describe "#deauthorize" do - let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } - let(:params) do - { - "format" => "json", - "id" => "evt_123", - "object" => "event", - "data" => { "object" => { "id" => "ca_9B" } }, - "type" => "account.application.deauthorized", - "account" => "webhook_id" - } - end - - it "deletes Stripe accounts in response to a webhook" do - post 'deauthorize', params - expect(response.status).to eq 200 - expect(response.body).to eq "Account webhook_id deauthorized" - expect(StripeAccount.all).not_to include stripe_account - end - - context "when the stripe_account id on the event does not match any known accounts" do - before do - params["account"] = "webhook_id1" - end - - it "does nothing" do - post 'deauthorize', params - expect(response.status).to eq 204 - expect(StripeAccount.all).to include stripe_account - end - end - - context "when the event is not a deauthorize event" do - before do - params["type"] = "account.application.authorized" - end - - it "does nothing" do - post 'deauthorize', params - expect(response.status).to eq 204 - expect(StripeAccount.all).to include stripe_account - end - end - end - describe "#destroy" do let(:params) { { format: :json, id: "some_id" } } diff --git a/spec/controllers/stripe_controller_spec.rb b/spec/controllers/stripe_controller_spec.rb new file mode 100644 index 0000000000..dc9f32f5c7 --- /dev/null +++ b/spec/controllers/stripe_controller_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe StripeController do + describe "#deauthorize" do + let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } + let(:params) do + { + "format" => "json", + "id" => "evt_123", + "object" => "event", + "data" => { "object" => { "id" => "ca_9B" } }, + "type" => "account.application.deauthorized", + "account" => "webhook_id" + } + end + + it "deletes Stripe accounts in response to a webhook" do + post 'deauthorize', params + expect(response.status).to eq 200 + expect(response.body).to eq "Account webhook_id deauthorized" + expect(StripeAccount.all).not_to include stripe_account + end + + context "when the stripe_account id on the event does not match any known accounts" do + before do + params["account"] = "webhook_id1" + end + + it "does nothing" do + post 'deauthorize', params + expect(response.status).to eq 204 + expect(StripeAccount.all).to include stripe_account + end + end + + context "when the event is not a deauthorize event" do + before do + params["type"] = "account.application.authorized" + end + + it "does nothing" do + post 'deauthorize', params + expect(response.status).to eq 204 + expect(StripeAccount.all).to include stripe_account + end + end + end +end From c54119f4824495c3d886902707ef593e8d1d4f6b Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 11 Oct 2017 17:11:02 +1100 Subject: [PATCH 332/353] Rename stripe controller action from 'deauthorize' to 'webhook' --- app/controllers/stripe_controller.rb | 2 +- config/routes.rb | 2 +- spec/controllers/stripe_controller_spec.rb | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/stripe_controller.rb b/app/controllers/stripe_controller.rb index dd5b8e5639..e81e951fb9 100644 --- a/app/controllers/stripe_controller.rb +++ b/app/controllers/stripe_controller.rb @@ -1,5 +1,5 @@ class StripeController < BaseController - def deauthorize + def webhook # TODO is there a sensible way to confirm this webhook call is actually from Stripe? event = Stripe::Event.construct_from(params) return render nothing: true, status: 204 unless event.type == "account.application.deauthorized" diff --git a/config/routes.rb b/config/routes.rb index 64243e39a3..1fcb2b6f7a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,7 +54,7 @@ Openfoodnetwork::Application.routes.draw do end resource :stripe, only: [] do - post :deauthorize + post :webhook end namespace :admin do diff --git a/spec/controllers/stripe_controller_spec.rb b/spec/controllers/stripe_controller_spec.rb index dc9f32f5c7..ac10d08432 100644 --- a/spec/controllers/stripe_controller_spec.rb +++ b/spec/controllers/stripe_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe StripeController do - describe "#deauthorize" do + describe "#webhook" do let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } let(:params) do { @@ -15,7 +15,7 @@ describe StripeController do end it "deletes Stripe accounts in response to a webhook" do - post 'deauthorize', params + post 'webhook', params expect(response.status).to eq 200 expect(response.body).to eq "Account webhook_id deauthorized" expect(StripeAccount.all).not_to include stripe_account @@ -27,7 +27,7 @@ describe StripeController do end it "does nothing" do - post 'deauthorize', params + post 'webhook', params expect(response.status).to eq 204 expect(StripeAccount.all).to include stripe_account end @@ -39,7 +39,7 @@ describe StripeController do end it "does nothing" do - post 'deauthorize', params + post 'webhook', params expect(response.status).to eq 204 expect(StripeAccount.all).to include stripe_account end From f22dd7513d6e2d62b9a30c093ba29abbf1329943 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 11 Oct 2017 17:50:46 +1100 Subject: [PATCH 333/353] Add a service object for handling Stripe webhooks --- app/controllers/stripe_controller.rb | 13 ++-- lib/stripe/webhook_handler.rb | 34 +++++++++ spec/controllers/stripe_controller_spec.rb | 1 - spec/lib/stripe/webhook_handler_spec.rb | 86 ++++++++++++++++++++++ 4 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 lib/stripe/webhook_handler.rb create mode 100644 spec/lib/stripe/webhook_handler_spec.rb diff --git a/app/controllers/stripe_controller.rb b/app/controllers/stripe_controller.rb index e81e951fb9..374f116cef 100644 --- a/app/controllers/stripe_controller.rb +++ b/app/controllers/stripe_controller.rb @@ -1,14 +1,11 @@ +require 'stripe/webhook_handler' + class StripeController < BaseController def webhook # TODO is there a sensible way to confirm this webhook call is actually from Stripe? - event = Stripe::Event.construct_from(params) - return render nothing: true, status: 204 unless event.type == "account.application.deauthorized" + handler = Stripe::WebhookHandler.new(params) + status = handler.handle ? 200 : 204 - destroyed = StripeAccount.where(stripe_user_id: event.account).destroy_all - if destroyed.any? - render text: "Account #{event.account} deauthorized", status: 200 - else - render nothing: true, status: 204 - end + render nothing: true, status: status end end diff --git a/lib/stripe/webhook_handler.rb b/lib/stripe/webhook_handler.rb new file mode 100644 index 0000000000..b40fa7a0a3 --- /dev/null +++ b/lib/stripe/webhook_handler.rb @@ -0,0 +1,34 @@ +module Stripe + class WebhookHandler + def initialize(params) + @event = Event.construct_from(params) + end + + def handle + return false unless known_event? + send(event_mappings[@event.type]) + end + + private + + def event_mappings + { + "account.application.deauthorized" => :deauthorize + } + end + + def known_event? + event_mappings.keys.include? @event.type + end + + def deauthorize + return false unless @event.respond_to?(:account) + destroyed = destroy_stripe_accounts_linked_to(@event.account) + destroyed.any? + end + + def destroy_stripe_accounts_linked_to(account) + StripeAccount.where(stripe_user_id: account).destroy_all + end + end +end diff --git a/spec/controllers/stripe_controller_spec.rb b/spec/controllers/stripe_controller_spec.rb index ac10d08432..7896cf1230 100644 --- a/spec/controllers/stripe_controller_spec.rb +++ b/spec/controllers/stripe_controller_spec.rb @@ -17,7 +17,6 @@ describe StripeController do it "deletes Stripe accounts in response to a webhook" do post 'webhook', params expect(response.status).to eq 200 - expect(response.body).to eq "Account webhook_id deauthorized" expect(StripeAccount.all).not_to include stripe_account end diff --git a/spec/lib/stripe/webhook_handler_spec.rb b/spec/lib/stripe/webhook_handler_spec.rb new file mode 100644 index 0000000000..2847efdab9 --- /dev/null +++ b/spec/lib/stripe/webhook_handler_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' +require 'stripe/webhook_handler' + +module Stripe + describe WebhookHandler do + let(:params) { { type: 'some.event' } } + let(:handler) { WebhookHandler.new(params) } + + describe "event_mappings" do + it { expect(handler.send(:event_mappings)).to be_a Hash } + end + + describe "known_event?" do + context "when event mappings know about the event type" do + before do + allow(handler).to receive(:event_mappings) { { 'some.event' => :something } } + end + + it { expect(handler.send(:known_event?)).to be true } + end + + context "when event mappings do not know about the event type" do + before do + allow(handler).to receive(:event_mappings) { { 'some.other.event' => :something } } + end + + it { expect(handler.send(:known_event?)).to be false } + end + end + + describe "handle" do + context "when the event is known" do + before do + allow(handler).to receive(:event_mappings) { { 'some.event' => :some_method } } + end + + it "calls the handler method, and returns the result" do + expect(handler).to receive(:some_method) { 'result' } + expect(handler.handle).to eq 'result' + end + end + + context "when the event is unknown" do + before do + allow(handler).to receive(:event_mappings) { { 'some.other.event' => :some_method } } + end + + it "does not call the handler method, and returns false" do + expect(handler).to_not receive(:some_method) + expect(handler.handle).to be false + end + end + end + + describe "deauthorize" do + context "when the event has no 'account' attribute" do + it "does destroy stripe accounts, returns false" do + expect(handler).to_not receive(:destroy_stripe_accounts_linked_to) + expect(handler.send(:deauthorize)).to be false + end + end + + context "when the event has an 'account' attribute" do + before do + params[:account] = 'some.account' + end + + context "when some stripe accounts are destroyed" do + before do + allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [double(:destroyed_stripe_account)] } + end + + it { expect(handler.send(:deauthorize)).to be true } + end + + context "when no stripe accounts are destroyed" do + before do + allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [] } + end + + it { expect(handler.send(:deauthorize)).to be false } + end + end + end + end +end From eb7cb02f33a7d66cf06aed891e06c00b0f7f6acd Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 13:58:53 +1100 Subject: [PATCH 334/353] Namespace stripe webhook controller in Stripe module --- app/controllers/stripe/webhooks_controller.rb | 16 ++++++++++++++++ app/controllers/stripe_controller.rb | 11 ----------- config/application.yml.example | 2 +- config/routes.rb | 4 ++-- .../webhooks_controller_spec.rb} | 10 +++++----- 5 files changed, 24 insertions(+), 19 deletions(-) create mode 100644 app/controllers/stripe/webhooks_controller.rb delete mode 100644 app/controllers/stripe_controller.rb rename spec/controllers/{stripe_controller_spec.rb => stripe/webhooks_controller_spec.rb} (88%) diff --git a/app/controllers/stripe/webhooks_controller.rb b/app/controllers/stripe/webhooks_controller.rb new file mode 100644 index 0000000000..8f778eee43 --- /dev/null +++ b/app/controllers/stripe/webhooks_controller.rb @@ -0,0 +1,16 @@ +require 'stripe/webhook_handler' + +module Stripe + class WebhooksController < BaseController + protect_from_forgery except: :create + + # POST /stripe/webhook + def create + # TODO is there a sensible way to confirm this webhook call is actually from Stripe? + handler = WebhookHandler.new(params) + status = handler.handle ? 200 : 204 + + render nothing: true, status: status + end + end +end diff --git a/app/controllers/stripe_controller.rb b/app/controllers/stripe_controller.rb deleted file mode 100644 index 374f116cef..0000000000 --- a/app/controllers/stripe_controller.rb +++ /dev/null @@ -1,11 +0,0 @@ -require 'stripe/webhook_handler' - -class StripeController < BaseController - def webhook - # TODO is there a sensible way to confirm this webhook call is actually from Stripe? - handler = Stripe::WebhookHandler.new(params) - status = handler.handle ? 200 : 204 - - render nothing: true, status: status - end -end diff --git a/config/application.yml.example b/config/application.yml.example index c3882edf4a..e10402fd79 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -30,7 +30,7 @@ CURRENCY: AUD # Stripe Connect details for instance account # Find these under 'API keys' and 'Connect' in your Stripe account dashboard -> Account Settings # Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/stripe/callback (e.g. https://openfoodnetwork.org.uk/stripe/connect) -# Under 'Webhooks', you should set up a Connect endpoint pointing to https://YOUR_SERVER_URL/stripe/webhook e.g. (https://openfoodnetwork.org.uk/stripe/webhook) +# Under 'Webhooks', you should set up a Connect endpoint pointing to https://YOUR_SERVER_URL/stripe/webhooks e.g. (https://openfoodnetwork.org.uk/stripe/webhooks) # STRIPE_INSTANCE_SECRET_KEY: "sk_test_xxxxxx" # This can be a test key or a live key # STRIPE_INSTANCE_PUBLISHABLE_KEY: "pk_test_xxxx" # This can be a test key or a live key # STRIPE_CLIENT_ID: "ca_xxxx" # This can be a development ID or a production ID diff --git a/config/routes.rb b/config/routes.rb index 1fcb2b6f7a..1f0a611fb9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -53,8 +53,8 @@ Openfoodnetwork::Application.routes.draw do end end - resource :stripe, only: [] do - post :webhook + namespace :stripe do + resources :webhooks, only: [:create] end namespace :admin do diff --git a/spec/controllers/stripe_controller_spec.rb b/spec/controllers/stripe/webhooks_controller_spec.rb similarity index 88% rename from spec/controllers/stripe_controller_spec.rb rename to spec/controllers/stripe/webhooks_controller_spec.rb index 7896cf1230..3d62585372 100644 --- a/spec/controllers/stripe_controller_spec.rb +++ b/spec/controllers/stripe/webhooks_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe StripeController do - describe "#webhook" do +describe Stripe::WebhooksController do + describe "#create" do let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } let(:params) do { @@ -15,7 +15,7 @@ describe StripeController do end it "deletes Stripe accounts in response to a webhook" do - post 'webhook', params + post 'create', params expect(response.status).to eq 200 expect(StripeAccount.all).not_to include stripe_account end @@ -26,7 +26,7 @@ describe StripeController do end it "does nothing" do - post 'webhook', params + post 'create', params expect(response.status).to eq 204 expect(StripeAccount.all).to include stripe_account end @@ -38,7 +38,7 @@ describe StripeController do end it "does nothing" do - post 'webhook', params + post 'create', params expect(response.status).to eq 204 expect(StripeAccount.all).to include stripe_account end From 434528516427a1a144f8e819f189d690ecec0050 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 14:48:19 +1100 Subject: [PATCH 335/353] Allow more granularity in Stripe WebhookHandler responses --- app/controllers/stripe/webhooks_controller.rb | 16 ++++++- lib/stripe/webhook_handler.rb | 6 +-- .../stripe/webhooks_controller_spec.rb | 47 ++++++++++--------- spec/lib/stripe/webhook_handler_spec.rb | 16 +++---- 4 files changed, 49 insertions(+), 36 deletions(-) diff --git a/app/controllers/stripe/webhooks_controller.rb b/app/controllers/stripe/webhooks_controller.rb index 8f778eee43..7ad7c1e507 100644 --- a/app/controllers/stripe/webhooks_controller.rb +++ b/app/controllers/stripe/webhooks_controller.rb @@ -8,9 +8,21 @@ module Stripe def create # TODO is there a sensible way to confirm this webhook call is actually from Stripe? handler = WebhookHandler.new(params) - status = handler.handle ? 200 : 204 + result = handler.handle - render nothing: true, status: status + render nothing: true, status: status_mappings[result] + end + + private + + # Stripe interprets a 4xx or 3xx response as a failure to receive the webhook, + # and will stop sending events if too many of these are returned. + def status_mappings + { + success: 200, + failure: 202, + silent_fail: 204 + } end end end diff --git a/lib/stripe/webhook_handler.rb b/lib/stripe/webhook_handler.rb index b40fa7a0a3..7a5332bacd 100644 --- a/lib/stripe/webhook_handler.rb +++ b/lib/stripe/webhook_handler.rb @@ -5,7 +5,7 @@ module Stripe end def handle - return false unless known_event? + return :failure unless known_event? send(event_mappings[@event.type]) end @@ -22,9 +22,9 @@ module Stripe end def deauthorize - return false unless @event.respond_to?(:account) + return :silent_fail unless @event.respond_to?(:account) destroyed = destroy_stripe_accounts_linked_to(@event.account) - destroyed.any? + destroyed.any? ? :success : :silent_fail end def destroy_stripe_accounts_linked_to(account) diff --git a/spec/controllers/stripe/webhooks_controller_spec.rb b/spec/controllers/stripe/webhooks_controller_spec.rb index 3d62585372..b415b6b865 100644 --- a/spec/controllers/stripe/webhooks_controller_spec.rb +++ b/spec/controllers/stripe/webhooks_controller_spec.rb @@ -2,45 +2,46 @@ require 'spec_helper' describe Stripe::WebhooksController do describe "#create" do - let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } let(:params) do { "format" => "json", "id" => "evt_123", "object" => "event", "data" => { "object" => { "id" => "ca_9B" } }, - "type" => "account.application.deauthorized", - "account" => "webhook_id" + "type" => "account.application.authorized", + "account" => "webhook_id1" } end - it "deletes Stripe accounts in response to a webhook" do - post 'create', params - expect(response.status).to eq 200 - expect(StripeAccount.all).not_to include stripe_account - end - - context "when the stripe_account id on the event does not match any known accounts" do - before do - params["account"] = "webhook_id1" - end - - it "does nothing" do + context "when an event with an unknown type is received" do + it "responds with a 202" do post 'create', params - expect(response.status).to eq 204 - expect(StripeAccount.all).to include stripe_account + expect(response.status).to eq 202 end end - context "when the event is not a deauthorize event" do + describe "when an account.application.deauthorized event is received" do + let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } before do - params["type"] = "account.application.authorized" + params["type"] = "account.application.deauthorized" end - it "does nothing" do - post 'create', params - expect(response.status).to eq 204 - expect(StripeAccount.all).to include stripe_account + context "when the stripe_account id on the event does not match any known accounts" do + it "doesn't delete any Stripe accounts, responds with 204" do + post 'create', params + expect(response.status).to eq 204 + expect(StripeAccount.all).to include stripe_account + end + end + + context "when the stripe_account id on the event matches a known account" do + before { params["account"] = "webhook_id" } + + it "deletes Stripe accounts in response to a webhook" do + post 'create', params + expect(response.status).to eq 200 + expect(StripeAccount.all).not_to include stripe_account + end end end end diff --git a/spec/lib/stripe/webhook_handler_spec.rb b/spec/lib/stripe/webhook_handler_spec.rb index 2847efdab9..fea36f21da 100644 --- a/spec/lib/stripe/webhook_handler_spec.rb +++ b/spec/lib/stripe/webhook_handler_spec.rb @@ -35,8 +35,8 @@ module Stripe end it "calls the handler method, and returns the result" do - expect(handler).to receive(:some_method) { 'result' } - expect(handler.handle).to eq 'result' + expect(handler).to receive(:some_method) { :result } + expect(handler.handle).to eq :result end end @@ -45,18 +45,18 @@ module Stripe allow(handler).to receive(:event_mappings) { { 'some.other.event' => :some_method } } end - it "does not call the handler method, and returns false" do + it "does not call the handler method, and returns :failure" do expect(handler).to_not receive(:some_method) - expect(handler.handle).to be false + expect(handler.handle).to be :failure end end end describe "deauthorize" do context "when the event has no 'account' attribute" do - it "does destroy stripe accounts, returns false" do + it "does destroy stripe accounts, returns :silent_fail" do expect(handler).to_not receive(:destroy_stripe_accounts_linked_to) - expect(handler.send(:deauthorize)).to be false + expect(handler.send(:deauthorize)).to be :silent_fail end end @@ -70,7 +70,7 @@ module Stripe allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [double(:destroyed_stripe_account)] } end - it { expect(handler.send(:deauthorize)).to be true } + it { expect(handler.send(:deauthorize)).to be :success } end context "when no stripe accounts are destroyed" do @@ -78,7 +78,7 @@ module Stripe allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [] } end - it { expect(handler.send(:deauthorize)).to be false } + it { expect(handler.send(:deauthorize)).to be :silent_fail } end end end From ed375a1e2c880c9673a278de6ee1e5c0e71b6d7e Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 16:31:17 +1100 Subject: [PATCH 336/353] Build Event object in controller instead of service object --- app/controllers/stripe/webhooks_controller.rb | 3 ++- lib/stripe/webhook_handler.rb | 4 ++-- spec/lib/stripe/webhook_handler_spec.rb | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/controllers/stripe/webhooks_controller.rb b/app/controllers/stripe/webhooks_controller.rb index 7ad7c1e507..c80dc500ae 100644 --- a/app/controllers/stripe/webhooks_controller.rb +++ b/app/controllers/stripe/webhooks_controller.rb @@ -7,7 +7,8 @@ module Stripe # POST /stripe/webhook def create # TODO is there a sensible way to confirm this webhook call is actually from Stripe? - handler = WebhookHandler.new(params) + event = Event.construct_from(params) + handler = WebhookHandler.new(event) result = handler.handle render nothing: true, status: status_mappings[result] diff --git a/lib/stripe/webhook_handler.rb b/lib/stripe/webhook_handler.rb index 7a5332bacd..da5482d723 100644 --- a/lib/stripe/webhook_handler.rb +++ b/lib/stripe/webhook_handler.rb @@ -1,7 +1,7 @@ module Stripe class WebhookHandler - def initialize(params) - @event = Event.construct_from(params) + def initialize(event) + @event = event end def handle diff --git a/spec/lib/stripe/webhook_handler_spec.rb b/spec/lib/stripe/webhook_handler_spec.rb index fea36f21da..1795430b9f 100644 --- a/spec/lib/stripe/webhook_handler_spec.rb +++ b/spec/lib/stripe/webhook_handler_spec.rb @@ -3,8 +3,8 @@ require 'stripe/webhook_handler' module Stripe describe WebhookHandler do - let(:params) { { type: 'some.event' } } - let(:handler) { WebhookHandler.new(params) } + let(:event) { double(:event, type: 'some.event') } + let(:handler) { WebhookHandler.new(event) } describe "event_mappings" do it { expect(handler.send(:event_mappings)).to be_a Hash } @@ -62,7 +62,7 @@ module Stripe context "when the event has an 'account' attribute" do before do - params[:account] = 'some.account' + allow(event).to receive(:account) { 'some.account' } end context "when some stripe accounts are destroyed" do From 068dbe5013913f0cb7ac661f29ed84445c794e12 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 17:14:09 +1100 Subject: [PATCH 337/353] Add verification to Stripe webhook endpoint --- app/controllers/stripe/webhooks_controller.rb | 25 +++++--- config/application.yml.example | 3 +- config/initializers/stripe.rb | 7 ++- .../stripe/webhooks_controller_spec.rb | 60 ++++++++++++++----- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/app/controllers/stripe/webhooks_controller.rb b/app/controllers/stripe/webhooks_controller.rb index c80dc500ae..8ada0361b6 100644 --- a/app/controllers/stripe/webhooks_controller.rb +++ b/app/controllers/stripe/webhooks_controller.rb @@ -3,26 +3,35 @@ require 'stripe/webhook_handler' module Stripe class WebhooksController < BaseController protect_from_forgery except: :create + before_filter :verify_webhook # POST /stripe/webhook def create - # TODO is there a sensible way to confirm this webhook call is actually from Stripe? - event = Event.construct_from(params) - handler = WebhookHandler.new(event) + handler = WebhookHandler.new(@event) result = handler.handle - render nothing: true, status: status_mappings[result] + render nothing: true, status: status_mappings[result] || 200 end private + def verify_webhook + payload = request.raw_post + signature = request.headers["HTTP_STRIPE_SIGNATURE"] + @event = Webhook.construct_event(payload, signature, Stripe.endpoint_secret) + rescue JSON::ParserError + render nothing: true, status: 400 + rescue Stripe::SignatureVerificationError + render nothing: true, status: 401 + end + # Stripe interprets a 4xx or 3xx response as a failure to receive the webhook, - # and will stop sending events if too many of these are returned. + # and will stop sending events if too many of either of these are returned. def status_mappings { - success: 200, - failure: 202, - silent_fail: 204 + success: 200, # The event was handled successfully + unknown: 202, # The event was of an unknown type + ignored: 204 # No action was taken in response to the event } end end diff --git a/config/application.yml.example b/config/application.yml.example index e10402fd79..1d8bf45f2d 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -29,8 +29,9 @@ CURRENCY: AUD # Stripe Connect details for instance account # Find these under 'API keys' and 'Connect' in your Stripe account dashboard -> Account Settings -# Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/stripe/callback (e.g. https://openfoodnetwork.org.uk/stripe/connect) +# Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/admin/stripe_accounts/connect_callback (e.g. https://openfoodnetwork.org.uk/admin/stripe_accounts/connect_callback) # Under 'Webhooks', you should set up a Connect endpoint pointing to https://YOUR_SERVER_URL/stripe/webhooks e.g. (https://openfoodnetwork.org.uk/stripe/webhooks) # STRIPE_INSTANCE_SECRET_KEY: "sk_test_xxxxxx" # This can be a test key or a live key # STRIPE_INSTANCE_PUBLISHABLE_KEY: "pk_test_xxxx" # This can be a test key or a live key # STRIPE_CLIENT_ID: "ca_xxxx" # This can be a development ID or a production ID +# STRIPE_ENDPOINT_SECRET: "whsec_xxxx" diff --git a/config/initializers/stripe.rb b/config/initializers/stripe.rb index c7ec6599d8..8237b0dcab 100644 --- a/config/initializers/stripe.rb +++ b/config/initializers/stripe.rb @@ -1,13 +1,14 @@ -# Add the :publishable_key property, to allow us to access this -# property from the object, rather than calling from ENV directly. +# Add some additional properties, to allow us to access these +# properties from the object, rather than calling from ENV directly. # This is mostly useful for stubbing when testing, but also feels # a bit cleaner than accessing keys in different ways. module Stripe class << self - attr_accessor :publishable_key + attr_accessor :publishable_key, :endpoint_secret end end Stripe.api_key = ENV['STRIPE_INSTANCE_SECRET_KEY'] Stripe.publishable_key = ENV['STRIPE_INSTANCE_PUBLISHABLE_KEY'] Stripe.client_id = ENV['STRIPE_CLIENT_ID'] +Stripe.endpoint_secret = ENV['STRIPE_ENDPOINT_SECRET'] diff --git a/spec/controllers/stripe/webhooks_controller_spec.rb b/spec/controllers/stripe/webhooks_controller_spec.rb index b415b6b865..40ec3c625b 100644 --- a/spec/controllers/stripe/webhooks_controller_spec.rb +++ b/spec/controllers/stripe/webhooks_controller_spec.rb @@ -13,34 +13,62 @@ describe Stripe::WebhooksController do } end - context "when an event with an unknown type is received" do - it "responds with a 202" do + context "when invalid json is provided" do + before do + allow(Stripe::Webhook).to receive(:construct_event).and_raise JSON::ParserError, "parsing failed" + end + + it "responds with a 400" do post 'create', params - expect(response.status).to eq 202 + expect(response.status).to eq 400 end end - describe "when an account.application.deauthorized event is received" do - let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } + context "when event signature verification fails" do before do - params["type"] = "account.application.deauthorized" + allow(Stripe::Webhook).to receive(:construct_event).and_raise Stripe::SignatureVerificationError.new("verfication failed", "header") end - context "when the stripe_account id on the event does not match any known accounts" do - it "doesn't delete any Stripe accounts, responds with 204" do + it "responds with a 401" do + post 'create', params + expect(response.status).to eq 401 + end + end + + context "when event signature verification succeeds" do + before do + allow(Stripe::Webhook).to receive(:construct_event) { Stripe::Event.construct_from(params) } + end + + context "when an event with an unknown type is received" do + it "responds with a 202" do post 'create', params - expect(response.status).to eq 204 - expect(StripeAccount.all).to include stripe_account + expect(response.status).to eq 202 end end - context "when the stripe_account id on the event matches a known account" do - before { params["account"] = "webhook_id" } + describe "when an account.application.deauthorized event is received" do + let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } + before do + params["type"] = "account.application.deauthorized" + end - it "deletes Stripe accounts in response to a webhook" do - post 'create', params - expect(response.status).to eq 200 - expect(StripeAccount.all).not_to include stripe_account + context "when the stripe_account id on the event does not match any known accounts" do + it "doesn't delete any Stripe accounts, responds with 204" do + post 'create', params + expect(response.status).to eq 204 + expect(StripeAccount.all).to include stripe_account + end + end + + context "when the stripe_account id on the event matches a known account" do + before { params["account"] = "webhook_id" } + + it "deletes Stripe accounts in response to a webhook" do + post 'create', params + expect(response.status).to eq 200 + expect(StripeAccount.all).not_to include stripe_account + end end end end From 99cac20725e09287be050dd668a7943c53109cd9 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 17:27:24 +1100 Subject: [PATCH 338/353] Fall back to 200 when handler returns an unknown result --- .../stripe/webhooks_controller_spec.rb | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/spec/controllers/stripe/webhooks_controller_spec.rb b/spec/controllers/stripe/webhooks_controller_spec.rb index 40ec3c625b..d413bbb08b 100644 --- a/spec/controllers/stripe/webhooks_controller_spec.rb +++ b/spec/controllers/stripe/webhooks_controller_spec.rb @@ -40,10 +40,26 @@ describe Stripe::WebhooksController do allow(Stripe::Webhook).to receive(:construct_event) { Stripe::Event.construct_from(params) } end - context "when an event with an unknown type is received" do - it "responds with a 202" do - post 'create', params - expect(response.status).to eq 202 + describe "setting the response status" do + let(:handler) { double(:handler) } + before { allow(Stripe::WebhookHandler).to receive(:new) { handler } } + + context "when an unknown result is returned by the handler" do + before { allow(handler).to receive(:handle) { :garbage } } + + it "falls back to 200" do + post 'create', params + expect(response.status).to eq 200 + end + end + + context "when the result returned by the handler is :failure" do + before { allow(handler).to receive(:handle) { :failure } } + + it "responds with 202" do + post 'create', params + expect(response.status).to eq 202 + end end end From 01f9fd323222b7b4f2e2925ecbd304491f9e81a9 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 17:38:20 +1100 Subject: [PATCH 339/353] Rename webhook handler status mappings --- lib/stripe/webhook_handler.rb | 6 +++--- spec/controllers/stripe/webhooks_controller_spec.rb | 4 ++-- spec/lib/stripe/webhook_handler_spec.rb | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/stripe/webhook_handler.rb b/lib/stripe/webhook_handler.rb index da5482d723..875dd9e52b 100644 --- a/lib/stripe/webhook_handler.rb +++ b/lib/stripe/webhook_handler.rb @@ -5,7 +5,7 @@ module Stripe end def handle - return :failure unless known_event? + return :unknown unless known_event? send(event_mappings[@event.type]) end @@ -22,9 +22,9 @@ module Stripe end def deauthorize - return :silent_fail unless @event.respond_to?(:account) + return :ignored unless @event.respond_to?(:account) destroyed = destroy_stripe_accounts_linked_to(@event.account) - destroyed.any? ? :success : :silent_fail + destroyed.any? ? :success : :ignored end def destroy_stripe_accounts_linked_to(account) diff --git a/spec/controllers/stripe/webhooks_controller_spec.rb b/spec/controllers/stripe/webhooks_controller_spec.rb index d413bbb08b..8c0bda6c87 100644 --- a/spec/controllers/stripe/webhooks_controller_spec.rb +++ b/spec/controllers/stripe/webhooks_controller_spec.rb @@ -53,8 +53,8 @@ describe Stripe::WebhooksController do end end - context "when the result returned by the handler is :failure" do - before { allow(handler).to receive(:handle) { :failure } } + context "when the result returned by the handler is :unknown" do + before { allow(handler).to receive(:handle) { :unknown } } it "responds with 202" do post 'create', params diff --git a/spec/lib/stripe/webhook_handler_spec.rb b/spec/lib/stripe/webhook_handler_spec.rb index 1795430b9f..e83cf01805 100644 --- a/spec/lib/stripe/webhook_handler_spec.rb +++ b/spec/lib/stripe/webhook_handler_spec.rb @@ -45,18 +45,18 @@ module Stripe allow(handler).to receive(:event_mappings) { { 'some.other.event' => :some_method } } end - it "does not call the handler method, and returns :failure" do + it "does not call the handler method, and returns :unknown" do expect(handler).to_not receive(:some_method) - expect(handler.handle).to be :failure + expect(handler.handle).to be :unknown end end end describe "deauthorize" do context "when the event has no 'account' attribute" do - it "does destroy stripe accounts, returns :silent_fail" do + it "does destroy stripe accounts, returns :ignored" do expect(handler).to_not receive(:destroy_stripe_accounts_linked_to) - expect(handler.send(:deauthorize)).to be :silent_fail + expect(handler.send(:deauthorize)).to be :ignored end end @@ -78,7 +78,7 @@ module Stripe allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [] } end - it { expect(handler.send(:deauthorize)).to be :silent_fail } + it { expect(handler.send(:deauthorize)).to be :ignored } end end end From 0b8b5e694e7669dc86cd06df1a77789e86462317 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 20:49:28 +1100 Subject: [PATCH 340/353] Move Stripe Connect callback action to dedicated controller --- .../admin/stripe_accounts_controller.rb | 15 ---- .../stripe/callbacks_controller.rb | 20 +++++ config/application.yml.example | 2 +- config/routes.rb | 2 +- .../admin/stripe_accounts_controller_spec.rb | 69 ----------------- .../stripe/callbacks_controller_spec.rb | 74 +++++++++++++++++++ 6 files changed, 96 insertions(+), 86 deletions(-) create mode 100644 app/controllers/stripe/callbacks_controller.rb create mode 100644 spec/controllers/stripe/callbacks_controller_spec.rb diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb index e23ab7ffb4..026db1840e 100644 --- a/app/controllers/admin/stripe_accounts_controller.rb +++ b/app/controllers/admin/stripe_accounts_controller.rb @@ -11,21 +11,6 @@ module Admin redirect_to Stripe::OAuth.authorize_url(url_params) end - def connect_callback - connector = Stripe::AccountConnector.new(spree_current_user, params) - - if connector.create_account - flash[:success] = t('admin.controllers.enterprises.stripe_connect_success') - elsif connector.connection_cancelled_by_user? - flash[:notice] = t('admin.controllers.enterprises.stripe_connect_cancelled') - else - flash[:error] = t('admin.controllers.enterprises.stripe_connect_fail') - end - redirect_to main_app.edit_admin_enterprise_path(connector.enterprise, anchor: 'payment_methods') - rescue Stripe::StripeError => e - render text: e.message, status: 500 - end - def destroy stripe_account = StripeAccount.find(params[:id]) authorize! :destroy, stripe_account diff --git a/app/controllers/stripe/callbacks_controller.rb b/app/controllers/stripe/callbacks_controller.rb new file mode 100644 index 0000000000..4db446144f --- /dev/null +++ b/app/controllers/stripe/callbacks_controller.rb @@ -0,0 +1,20 @@ +require 'stripe/account_connector' + +module Stripe + class CallbacksController < BaseController + def index + connector = Stripe::AccountConnector.new(spree_current_user, params) + + if connector.create_account + flash[:success] = t('admin.controllers.enterprises.stripe_connect_success') + elsif connector.connection_cancelled_by_user? + flash[:notice] = t('admin.controllers.enterprises.stripe_connect_cancelled') + else + flash[:error] = t('admin.controllers.enterprises.stripe_connect_fail') + end + redirect_to main_app.edit_admin_enterprise_path(connector.enterprise, anchor: 'payment_methods') + rescue Stripe::StripeError => e + render text: e.message, status: 500 + end + end +end diff --git a/config/application.yml.example b/config/application.yml.example index 1d8bf45f2d..310e248726 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -29,7 +29,7 @@ CURRENCY: AUD # Stripe Connect details for instance account # Find these under 'API keys' and 'Connect' in your Stripe account dashboard -> Account Settings -# Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/admin/stripe_accounts/connect_callback (e.g. https://openfoodnetwork.org.uk/admin/stripe_accounts/connect_callback) +# Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/stripe/callbacks (e.g. https://openfoodnetwork.org.uk/stripe/callbacks) # Under 'Webhooks', you should set up a Connect endpoint pointing to https://YOUR_SERVER_URL/stripe/webhooks e.g. (https://openfoodnetwork.org.uk/stripe/webhooks) # STRIPE_INSTANCE_SECRET_KEY: "sk_test_xxxxxx" # This can be a test key or a live key # STRIPE_INSTANCE_PUBLISHABLE_KEY: "pk_test_xxxx" # This can be a test key or a live key diff --git a/config/routes.rb b/config/routes.rb index 1f0a611fb9..0484754d39 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,6 +54,7 @@ Openfoodnetwork::Application.routes.draw do end namespace :stripe do + resources :callbacks, only: [:index] resources :webhooks, only: [:create] end @@ -173,7 +174,6 @@ Openfoodnetwork::Application.routes.draw do resources :stripe_accounts, only: [:destroy] do get :connect, on: :collection - get :connect_callback, on: :collection get :status, on: :collection end end diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb index 96a17e02d8..4db449d2df 100644 --- a/spec/controllers/admin/stripe_accounts_controller_spec.rb +++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb @@ -21,75 +21,6 @@ describe Admin::StripeAccountsController, type: :controller do end end - context "#connect_callback" do - let(:params) { { id: enterprise.permalink } } - let(:connector) { double(:connector) } - - before do - allow(controller).to receive(:spree_current_user) { enterprise.owner } - allow(Stripe::AccountConnector).to receive(:new) { connector } - end - - context "when the connector.create_account raises a StripeError" do - before do - allow(connector).to receive(:create_account).and_raise Stripe::StripeError, "some error" - end - - it "returns a 500 error" do - spree_get :connect_callback, params - expect(response.status).to be 500 - end - end - - context "when the connector.create_account raises an AccessDenied error" do - before do - allow(connector).to receive(:create_account).and_raise CanCan::AccessDenied, "some error" - end - - it "redirects to unauthorized" do - spree_get :connect_callback, params - expect(response).to redirect_to spree.unauthorized_path - end - end - - context "when the connector fails in creating a new stripe account record" do - before { allow(connector).to receive(:create_account) { false } } - - context "when the user cancelled the connection" do - before { allow(connector).to receive(:connection_cancelled_by_user?) { true } } - - it "renders a failure message" do - allow(connector).to receive(:enterprise) { enterprise } - spree_get :connect_callback, params - expect(flash[:notice]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_cancelled') - expect(response).to redirect_to edit_admin_enterprise_path(enterprise, anchor: 'payment_methods') - end - end - - context "when some other error caused the failure" do - before { allow(connector).to receive(:connection_cancelled_by_user?) { false } } - - it "renders a failure message" do - allow(connector).to receive(:enterprise) { enterprise } - spree_get :connect_callback, params - expect(flash[:error]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_fail') - expect(response).to redirect_to edit_admin_enterprise_path(enterprise, anchor: 'payment_methods') - end - end - end - - context "when the connector succeeds in creating a new stripe account record" do - before { allow(connector).to receive(:create_account) { true } } - - it "redirects to the enterprise edit path" do - allow(connector).to receive(:enterprise) { enterprise } - spree_get :connect_callback, params - expect(flash[:success]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_success') - expect(response).to redirect_to edit_admin_enterprise_path(enterprise, anchor: 'payment_methods') - end - end - end - describe "#destroy" do let(:params) { { format: :json, id: "some_id" } } diff --git a/spec/controllers/stripe/callbacks_controller_spec.rb b/spec/controllers/stripe/callbacks_controller_spec.rb new file mode 100644 index 0000000000..831dc5e348 --- /dev/null +++ b/spec/controllers/stripe/callbacks_controller_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe Stripe::CallbacksController do + let(:enterprise) { create(:distributor_enterprise) } + + context "#index" do + let(:params) { { id: enterprise.permalink } } + let(:connector) { double(:connector) } + + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } + allow(Stripe::AccountConnector).to receive(:new) { connector } + end + + context "when the connector.create_account raises a StripeError" do + before do + allow(connector).to receive(:create_account).and_raise Stripe::StripeError, "some error" + end + + it "returns a 500 error" do + spree_get :index, params + expect(response.status).to be 500 + end + end + + context "when the connector.create_account raises an AccessDenied error" do + before do + allow(connector).to receive(:create_account).and_raise CanCan::AccessDenied, "some error" + end + + it "redirects to unauthorized" do + spree_get :index, params + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "when the connector fails in creating a new stripe account record" do + before { allow(connector).to receive(:create_account) { false } } + + context "when the user cancelled the connection" do + before { allow(connector).to receive(:connection_cancelled_by_user?) { true } } + + it "renders a failure message" do + allow(connector).to receive(:enterprise) { enterprise } + spree_get :index, params + expect(flash[:notice]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_cancelled') + expect(response).to redirect_to edit_admin_enterprise_path(enterprise, anchor: 'payment_methods') + end + end + + context "when some other error caused the failure" do + before { allow(connector).to receive(:connection_cancelled_by_user?) { false } } + + it "renders a failure message" do + allow(connector).to receive(:enterprise) { enterprise } + spree_get :index, params + expect(flash[:error]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_fail') + expect(response).to redirect_to edit_admin_enterprise_path(enterprise, anchor: 'payment_methods') + end + end + end + + context "when the connector succeeds in creating a new stripe account record" do + before { allow(connector).to receive(:create_account) { true } } + + it "redirects to the enterprise edit path" do + allow(connector).to receive(:enterprise) { enterprise } + spree_get :index, params + expect(flash[:success]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_success') + expect(response).to redirect_to edit_admin_enterprise_path(enterprise, anchor: 'payment_methods') + end + end + end +end From f2ad087be5374f1192243c87e8b45d833ac211cd Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 12 Oct 2017 21:35:31 +1100 Subject: [PATCH 341/353] Change inheritance of StripeAccountsController --- .../admin/stripe_accounts_controller.rb | 10 ++-- app/models/spree/ability_decorator.rb | 2 +- .../admin/stripe_accounts_controller_spec.rb | 53 +++++++++---------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb index 026db1840e..e0a72c093c 100644 --- a/app/controllers/admin/stripe_accounts_controller.rb +++ b/app/controllers/admin/stripe_accounts_controller.rb @@ -1,9 +1,7 @@ require 'stripe/account_connector' module Admin - class StripeAccountsController < BaseController - protect_from_forgery except: :destroy_from_webhook - + class StripeAccountsController < Spree::Admin::BaseController def connect payload = params.slice(:enterprise_id) key = Openfoodnetwork::Application.config.secret_token @@ -41,5 +39,11 @@ module Admin render json: { status: :access_revoked } end end + + private + + def model_class + StripeAccount + end end end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 164e27d81a..662cc5d1d6 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -124,7 +124,7 @@ class AbilityDecorator column_preference.user == user end - can [:status, :destroy], StripeAccount do |stripe_account| + can [:admin, :connect, :status, :destroy], StripeAccount do |stripe_account| user.enterprises.include? stripe_account.enterprise end end diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb index 4db449d2df..9a6854ae85 100644 --- a/spec/controllers/admin/stripe_accounts_controller_spec.rb +++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb @@ -77,55 +77,52 @@ describe Admin::StripeAccountsController, type: :controller do end describe "#status" do - let(:params) { { format: :json } } + let(:params) { { format: :json, enterprise_id: enterprise.id } } before do allow(Stripe).to receive(:api_key) { "sk_test_12345" } Spree::Config.set(stripe_connect_enabled: false) end - context "when Stripe is not enabled" do - it "returns with a status of 'stripe_disabled'" do + context "when I don't manage the specified enterprise" do + let(:user) { create(:user) } + + before do + allow(controller).to receive(:spree_current_user) { user } + end + + it "redirects to unauthorized" do spree_get :status, params - json_response = JSON.parse(response.body) - expect(json_response["status"]).to eq "stripe_disabled" + expect(response).to redirect_to spree.unauthorized_path end end - context "when Stripe is enabled" do - before { Spree::Config.set(stripe_connect_enabled: true) } + context "when I manage the specified enterprise" do + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } + end - context "but no stripe account is associated with the specified enterprise" do - it "returns with a status of 'account_missing'" do + context "when Stripe is not enabled" do + it "returns with a status of 'stripe_disabled'" do spree_get :status, params json_response = JSON.parse(response.body) - expect(json_response["status"]).to eq "account_missing" + expect(json_response["status"]).to eq "stripe_disabled" end end - context "and a stripe account is associated with the specified enterprise" do - let!(:account) { create(:stripe_account, stripe_user_id: "acc_123", enterprise: enterprise) } + context "when Stripe is enabled" do + before { Spree::Config.set(stripe_connect_enabled: true) } - context "but I don't manage the enterprise" do - let(:user) { create(:user) } - let(:enterprise2) { create(:enterprise) } - before do - user.owned_enterprises << enterprise2 - params[:enterprise_id] = enterprise.id - allow(controller).to receive(:spree_current_user) { user } - end - - it "redirects to unauthorized" do + context "when no stripe account is associated with the specified enterprise" do + it "returns with a status of 'account_missing'" do spree_get :status, params - expect(response).to redirect_to spree.unauthorized_path + json_response = JSON.parse(response.body) + expect(json_response["status"]).to eq "account_missing" end end - context "and I manage the enterprise" do - before do - params[:enterprise_id] = enterprise.id - allow(controller).to receive(:spree_current_user) { enterprise.owner } - end + context "when a stripe account is associated with the specified enterprise" do + let!(:account) { create(:stripe_account, stripe_user_id: "acc_123", enterprise: enterprise) } context "but access has been revoked or does not exist on stripe's servers" do before do From aae8f1cbc485ae748fad4c2fc57c3c694a8749e8 Mon Sep 17 00:00:00 2001 From: Pierre de Lacroix Date: Tue, 26 Sep 2017 10:45:29 +0200 Subject: [PATCH 342/353] Change localizeCurrency filter to use I18n.toCurrency --- .../darkswarm/filters/localize_currency.js.coffee | 13 ++++++------- .../api/orders_by_distributor_serializer.rb | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee b/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee index 7087e09fc3..a194e78e7b 100644 --- a/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee +++ b/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee @@ -5,11 +5,10 @@ Darkswarm.filter "localizeCurrency", (currencyConfig)-> currency_code = if currencyConfig.display_currency then " " + currencyConfig.currency else "" # Set decimal points, 2 or 0 if hide_cents. decimals = if currencyConfig.hide_cents == "true" then 0 else 2 - # We need to use parseFloat before toFixed as the amount should come in as a string. - amount_fixed = parseFloat(amount).toFixed(decimals) + # Set wether the currency symbol appears first + sign_first = currencyConfig.symbol_position == 'before' + # We need to use parseFloat as the amount should come in as a string. + amount = parseFloat(amount) - # Build the final price string. TODO use spree decimal point and spacer character settings. - if currencyConfig.symbol_position == 'before' - currencyConfig.symbol + amount_fixed + currency_code - else - amount_fixed + " " + currencyConfig.symbol + currency_code + # Build the final price string. + I18n.toCurrency(amount, {precision: decimals, unit: currencyConfig.symbol, sign_first: sign_first}) + currency_code diff --git a/app/serializers/api/orders_by_distributor_serializer.rb b/app/serializers/api/orders_by_distributor_serializer.rb index b920272df2..df1cefffc5 100644 --- a/app/serializers/api/orders_by_distributor_serializer.rb +++ b/app/serializers/api/orders_by_distributor_serializer.rb @@ -4,7 +4,7 @@ module Api has_many :distributed_orders, serializer: Api::OrderSerializer def balance - object.distributed_orders.map(&:outstanding_balance).reduce(:+).to_money.to_s + object.distributed_orders.map(&:outstanding_balance).reduce(:+) end def hash From 089c754f62f8ec8c1e28f94adc2a3c9b090b3ad4 Mon Sep 17 00:00:00 2001 From: Pierre de Lacroix Date: Tue, 26 Sep 2017 19:48:54 +0200 Subject: [PATCH 343/353] Remove conversion of amounts to currency strings via Money in serializers as it's better done in JS --- app/serializers/api/order_serializer.rb | 4 ---- app/serializers/api/payment_serializer.rb | 4 ---- 2 files changed, 8 deletions(-) diff --git a/app/serializers/api/order_serializer.rb b/app/serializers/api/order_serializer.rb index 328c1dc11a..481d8ec213 100644 --- a/app/serializers/api/order_serializer.rb +++ b/app/serializers/api/order_serializer.rb @@ -28,10 +28,6 @@ module Api I18n.l(object.order_cycle.andand.orders_close_at, format: "%b %d, %Y %H:%M") end - def total - object.total.to_money.to_s - end - def shipment_state object.shipment_state ? object.shipment_state : nil end diff --git a/app/serializers/api/payment_serializer.rb b/app/serializers/api/payment_serializer.rb index f0d91cce72..8b956ecb1f 100644 --- a/app/serializers/api/payment_serializer.rb +++ b/app/serializers/api/payment_serializer.rb @@ -5,10 +5,6 @@ module Api object.payment_method.try(:name) end - def amount - object.amount.to_money.to_s - end - def updated_at I18n.l(object.updated_at, format: "%b %d, %Y %H:%M") end From 65d176f533746d463d125573e66d170f78516111 Mon Sep 17 00:00:00 2001 From: Pierre de Lacroix Date: Wed, 4 Oct 2017 00:33:42 +0200 Subject: [PATCH 344/353] Fix wrong way to force currency symbol after the amount --- .../darkswarm/filters/localize_currency.js.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee b/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee index a194e78e7b..37de2759cc 100644 --- a/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee +++ b/app/assets/javascripts/darkswarm/filters/localize_currency.js.coffee @@ -3,12 +3,12 @@ Darkswarm.filter "localizeCurrency", (currencyConfig)-> (amount) -> # Set country code (eg. "US"). currency_code = if currencyConfig.display_currency then " " + currencyConfig.currency else "" - # Set decimal points, 2 or 0 if hide_cents. + # Set decimal points, 2 or 0 if hide_cents. decimals = if currencyConfig.hide_cents == "true" then 0 else 2 - # Set wether the currency symbol appears first - sign_first = currencyConfig.symbol_position == 'before' + # Set format if the currency symbol should come after the number, otherwise (default) use the locale setting. + format = if currencyConfig.symbol_position == "after" then "%n %u" else undefined # We need to use parseFloat as the amount should come in as a string. amount = parseFloat(amount) # Build the final price string. - I18n.toCurrency(amount, {precision: decimals, unit: currencyConfig.symbol, sign_first: sign_first}) + currency_code + I18n.toCurrency(amount, {precision: decimals, unit: currencyConfig.symbol, format: format}) + currency_code From 508dfa4f23f3647ddb675a8abdb5fd965d85229c Mon Sep 17 00:00:00 2001 From: Pierre de Lacroix Date: Wed, 4 Oct 2017 14:54:53 +0200 Subject: [PATCH 345/353] Fix failing tests assuming localizeCurrency has no "delimiter" --- spec/support/spree/money_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/spree/money_helper.rb b/spec/support/spree/money_helper.rb index f0ca8f73d9..dc5a374c39 100644 --- a/spec/support/spree/money_helper.rb +++ b/spec/support/spree/money_helper.rb @@ -1,7 +1,7 @@ module Spree module MoneyHelper def with_currency(amount, options = {}) - Spree::Money.new(amount, {delimiter: ''}.merge(options)).to_s # Delimiter is to match js localizeCurrency + Spree::Money.new(amount, options).to_s end end end From e5fb8712d77d3a6e5de3dce483339c0774370aab Mon Sep 17 00:00:00 2001 From: Duende13 Date: Tue, 12 Sep 2017 07:09:39 +0100 Subject: [PATCH 346/353] Simplify Product Edit Screen removing fields and adding 2 new menu options for seo and group buy (#1741) --- app/models/spree/ability_decorator.rb | 2 +- .../add_group_buy_to_admin_product_edit.rb | 5 -- .../_form/add_notes_field.html.haml.deface | 6 -- .../add_primary_taxon_field.html.haml.deface | 2 +- .../_form/add_supplier.html.haml.deface | 5 +- .../products/_form/remove_available_on.deface | 2 + .../_form/remove_cost_currency.deface | 2 + .../products/_form/remove_cost_price.deface | 2 + .../_form/remove_meta_description.deface | 1 + .../remove_option_types_and_taxons.deface | 1 + ...eplace_master_price_label.html.haml.deface | 2 + .../_form/replace_taxons_div.html.haml.deface | 5 ++ .../add_group_buy.html.haml.deface | 5 ++ .../_product_tabs/add_seo.html.haml.deface | 5 ++ .../admin/products/_group_buy_form.html.haml | 14 ++--- .../spree/admin/products/_seo_form.html.haml | 15 +++++ .../products/group_buy_options.html.haml | 8 +++ app/views/spree/admin/products/seo.html.haml | 8 +++ config/routes.rb | 2 + spec/features/admin/products_spec.rb | 57 ++++++++++--------- 20 files changed, 99 insertions(+), 50 deletions(-) delete mode 100644 app/overrides/add_group_buy_to_admin_product_edit.rb delete mode 100644 app/overrides/spree/admin/products/_form/add_notes_field.html.haml.deface create mode 100644 app/overrides/spree/admin/products/_form/remove_available_on.deface create mode 100644 app/overrides/spree/admin/products/_form/remove_cost_currency.deface create mode 100644 app/overrides/spree/admin/products/_form/remove_cost_price.deface create mode 100644 app/overrides/spree/admin/products/_form/remove_meta_description.deface create mode 100644 app/overrides/spree/admin/products/_form/remove_option_types_and_taxons.deface create mode 100644 app/overrides/spree/admin/products/_form/replace_master_price_label.html.haml.deface create mode 100644 app/overrides/spree/admin/products/_form/replace_taxons_div.html.haml.deface create mode 100644 app/overrides/spree/admin/shared/_product_tabs/add_group_buy.html.haml.deface create mode 100644 app/overrides/spree/admin/shared/_product_tabs/add_seo.html.haml.deface create mode 100644 app/views/spree/admin/products/_seo_form.html.haml create mode 100644 app/views/spree/admin/products/group_buy_options.html.haml create mode 100644 app/views/spree/admin/products/seo.html.haml diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 662cc5d1d6..c08bd1f654 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -132,7 +132,7 @@ class AbilityDecorator def add_product_management_abilities(user) # Enterprise User can only access products that they are a supplier for can [:create], Spree::Product - can [:admin, :read, :update, :product_distributions, :bulk_edit, :bulk_update, :clone, :delete, :destroy], Spree::Product do |product| + can [:admin, :read, :update, :product_distributions, :seo, :group_buy_options, :bulk_edit, :bulk_update, :clone, :delete, :destroy], Spree::Product do |product| OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include? product.supplier end diff --git a/app/overrides/add_group_buy_to_admin_product_edit.rb b/app/overrides/add_group_buy_to_admin_product_edit.rb deleted file mode 100644 index eb44ca5c6c..0000000000 --- a/app/overrides/add_group_buy_to_admin_product_edit.rb +++ /dev/null @@ -1,5 +0,0 @@ -Deface::Override.new(:virtual_path => "spree/admin/products/_form", - :insert_top => "[data-hook='admin_product_form_right']", - :partial => "spree/admin/products/group_buy_form", - :name => "add_group_buy_to_admin_product_edit", - :original => '0c0e8d714989e48ee246a8253fb2b362f108621a') diff --git a/app/overrides/spree/admin/products/_form/add_notes_field.html.haml.deface b/app/overrides/spree/admin/products/_form/add_notes_field.html.haml.deface deleted file mode 100644 index dcc7937d6b..0000000000 --- a/app/overrides/spree/admin/products/_form/add_notes_field.html.haml.deface +++ /dev/null @@ -1,6 +0,0 @@ -/ insert_bottom "[data-hook='admin_product_form_additional_fields']" - -= f.field_container :notes do - = f.label :notes, t(:notes) - = f.text_area :notes, { :class => 'fullwidth', rows: 5 } - = f.error_message_on :notes diff --git a/app/overrides/spree/admin/products/_form/add_primary_taxon_field.html.haml.deface b/app/overrides/spree/admin/products/_form/add_primary_taxon_field.html.haml.deface index efd7ccbfaf..46afe1f94d 100644 --- a/app/overrides/spree/admin/products/_form/add_primary_taxon_field.html.haml.deface +++ b/app/overrides/spree/admin/products/_form/add_primary_taxon_field.html.haml.deface @@ -1,3 +1,3 @@ -/ insert_top "[data-hook='admin_product_form_right']" +/ insert_after "div[class='variant_units_form']" = render 'spree/admin/products/primary_taxon_form', f: f \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface b/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface index deba9ff633..a814c10b80 100644 --- a/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface +++ b/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface @@ -1,7 +1,6 @@ -/ insert_top "[data-hook='admin_product_form_right']" - +/ insert_before "code[erb-loud]:contains('f.field_container :price')" = f.field_container :supplier do = f.label :supplier, t(:spree_admin_supplier) %br = f.collection_select(:supplier_id, @producers, :id, :name, {:include_blank => true}, {:class => "select2"}) - = f.error_message_on :supplier + = f.error_message_on :supplier \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/remove_available_on.deface b/app/overrides/spree/admin/products/_form/remove_available_on.deface new file mode 100644 index 0000000000..645fb76bde --- /dev/null +++ b/app/overrides/spree/admin/products/_form/remove_available_on.deface @@ -0,0 +1,2 @@ +remove "code[erb-loud]:contains('f.label :available_on')" +closing_selector("code[erb-loud]:contains('f.text_field :available_on')") \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/remove_cost_currency.deface b/app/overrides/spree/admin/products/_form/remove_cost_currency.deface new file mode 100644 index 0000000000..a194a5e03a --- /dev/null +++ b/app/overrides/spree/admin/products/_form/remove_cost_currency.deface @@ -0,0 +1,2 @@ +remove "code[erb-loud]:contains('f.field_container :cost_currency')" +closing_selector("code[erb-silent]:contains('end')") \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/remove_cost_price.deface b/app/overrides/spree/admin/products/_form/remove_cost_price.deface new file mode 100644 index 0000000000..9f9cd8ec3f --- /dev/null +++ b/app/overrides/spree/admin/products/_form/remove_cost_price.deface @@ -0,0 +1,2 @@ +remove "code[erb-loud]:contains('f.field_container :cost_price')" +closing_selector("code[erb-silent]:contains('end')") \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/remove_meta_description.deface b/app/overrides/spree/admin/products/_form/remove_meta_description.deface new file mode 100644 index 0000000000..15a00a5df0 --- /dev/null +++ b/app/overrides/spree/admin/products/_form/remove_meta_description.deface @@ -0,0 +1 @@ +remove "div[data-hook='admin_product_form_meta']" \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/remove_option_types_and_taxons.deface b/app/overrides/spree/admin/products/_form/remove_option_types_and_taxons.deface new file mode 100644 index 0000000000..eeccddc25c --- /dev/null +++ b/app/overrides/spree/admin/products/_form/remove_option_types_and_taxons.deface @@ -0,0 +1 @@ +remove "div[class='twelve columns alpha omega']" \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/replace_master_price_label.html.haml.deface b/app/overrides/spree/admin/products/_form/replace_master_price_label.html.haml.deface new file mode 100644 index 0000000000..02152cb521 --- /dev/null +++ b/app/overrides/spree/admin/products/_form/replace_master_price_label.html.haml.deface @@ -0,0 +1,2 @@ +/ replace "[data-hook=admin_product_form_right] code[erb-loud]:contains('f.label :price')" += f.label :price, raw(t(:price) + content_tag(:span, ' *', :class => 'required')) \ No newline at end of file diff --git a/app/overrides/spree/admin/products/_form/replace_taxons_div.html.haml.deface b/app/overrides/spree/admin/products/_form/replace_taxons_div.html.haml.deface new file mode 100644 index 0000000000..697255ab70 --- /dev/null +++ b/app/overrides/spree/admin/products/_form/replace_taxons_div.html.haml.deface @@ -0,0 +1,5 @@ +/ insert_bottom "[data-hook=admin_product_form_left]" += f.field_container :taxons do + = f.label :taxon_ids, t(:taxons) + %br + = f.hidden_field :taxon_ids, :value => @product.taxon_ids.join(',') \ No newline at end of file diff --git a/app/overrides/spree/admin/shared/_product_tabs/add_group_buy.html.haml.deface b/app/overrides/spree/admin/shared/_product_tabs/add_group_buy.html.haml.deface new file mode 100644 index 0000000000..ab62e4725a --- /dev/null +++ b/app/overrides/spree/admin/shared/_product_tabs/add_group_buy.html.haml.deface @@ -0,0 +1,5 @@ +/ insert_bottom "[data-hook='admin_product_tabs']" + +- klass = current == 'Group Buy Options' ? 'active' : '' +%li{:class => klass} + = link_to_with_icon 'icon-tasks', 'Group Buy Options', group_buy_options_admin_product_url(@product) diff --git a/app/overrides/spree/admin/shared/_product_tabs/add_seo.html.haml.deface b/app/overrides/spree/admin/shared/_product_tabs/add_seo.html.haml.deface new file mode 100644 index 0000000000..98023bf7a1 --- /dev/null +++ b/app/overrides/spree/admin/shared/_product_tabs/add_seo.html.haml.deface @@ -0,0 +1,5 @@ +/ insert_bottom "[data-hook='admin_product_tabs']" + +- klass = current == 'SEO' ? 'active' : '' +%li{:class => klass} + = link_to_with_icon 'icon-tasks', 'SEO', seo_admin_product_url(@product) diff --git a/app/views/spree/admin/products/_group_buy_form.html.haml b/app/views/spree/admin/products/_group_buy_form.html.haml index f61fc7b38d..6ccf23a024 100644 --- a/app/views/spree/admin/products/_group_buy_form.html.haml +++ b/app/views/spree/admin/products/_group_buy_form.html.haml @@ -2,13 +2,13 @@ = f.label :group_buy, t('.group_buy') %br .alpha.two.columns - = f.radio_button :group_buy, '1', checked: f.object.group_buy = f.label :group_buy_1, t(:yes) + = f.radio_button :group_buy, '1', checked: f.object.group_buy .alpha.two.columns - = f.radio_button :group_buy, '0', checked: !f.object.group_buy = f.label :group_buy_0, t(:no) -%br.clear -= f.field_container :group_buy_unit_size do - = f.label :group_buy_unit_size, t('.bulk_unit_size') - %br - = f.text_field :group_buy_unit_size + = f.radio_button :group_buy, '0', checked: !f.object.group_buy + %br.clear + = f.field_container :group_buy_unit_size do + = f.label :group_buy_unit_size, t('.bulk_unit_size') + %br + = f.text_field :group_buy_unit_size diff --git a/app/views/spree/admin/products/_seo_form.html.haml b/app/views/spree/admin/products/_seo_form.html.haml new file mode 100644 index 0000000000..bbc5bdb4f0 --- /dev/null +++ b/app/views/spree/admin/products/_seo_form.html.haml @@ -0,0 +1,15 @@ +.row{"data-hook" => "admin_product_meta_form"} + .alpha.eleven.columns + = f.field_container :meta_description do + = f.label :meta_keywords, t(:meta_keywords) + %br/ + = f.text_field :meta_keywords, :class => 'fullwidth', :rows => 6 + = f.field_container :meta_description do + = f.label :meta_description, t(:meta_description) + %br/ + = f.text_field :meta_description, :class => 'fullwidth', :rows => 6 + .alpha.eleven.columns + = f.field_container :notes do + = f.label :notes, t(:notes) + = f.text_area :notes, { :class => 'fullwidth', rows: 5 } + = f.error_message_on :notes \ No newline at end of file diff --git a/app/views/spree/admin/products/group_buy_options.html.haml b/app/views/spree/admin/products/group_buy_options.html.haml new file mode 100644 index 0000000000..9261ddbfc6 --- /dev/null +++ b/app/views/spree/admin/products/group_buy_options.html.haml @@ -0,0 +1,8 @@ += render :partial => 'spree/admin/shared/product_sub_menu' += render :partial => 'spree/admin/shared/product_tabs', :locals => { :current => 'Group Buy Options' } += render :partial => 'spree/shared/error_messages', :locals => { :target => @product } + += form_for [:admin, @product], :method => :put, :html => { :multipart => true } do |f| + %fieldset.no-border-top + = render :partial => 'group_buy_form', :locals => { :f => f } + = render :partial => 'spree/admin/shared/edit_resource_links' diff --git a/app/views/spree/admin/products/seo.html.haml b/app/views/spree/admin/products/seo.html.haml new file mode 100644 index 0000000000..ddcd00402f --- /dev/null +++ b/app/views/spree/admin/products/seo.html.haml @@ -0,0 +1,8 @@ += render :partial => 'spree/admin/shared/product_sub_menu' += render :partial => 'spree/admin/shared/product_tabs', :locals => { :current => 'SEO' } += render :partial => 'spree/shared/error_messages', :locals => { :target => @product } + += form_for [:admin, @product], :method => :put, :html => { :multipart => true } do |f| + %fieldset.no-border-top + = render :partial => 'seo_form', :locals => { :f => f } + = render :partial => 'spree/admin/shared/edit_resource_links' diff --git a/config/routes.rb b/config/routes.rb index 0484754d39..ca8114ddb8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -275,6 +275,8 @@ Spree::Core::Engine.routes.prepend do resources :products do get :product_distributions, on: :member + get :group_buy_options, on: :member + get :seo, on: :member post :bulk_update, :on => :collection, :as => :bulk_update end diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index cc11e5853d..eb6e765b00 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -86,24 +86,6 @@ feature %q{ variant = product.variants.first variant.on_demand.should be_true end - - scenario "making a product into a group buy product" do - product = create(:simple_product, name: 'group buy product') - - login_to_admin_section - - visit spree.edit_admin_product_path(product) - - choose 'product_group_buy_1' - fill_in 'product_group_buy_unit_size', with: '10' - - click_button 'Update' - - flash_message.should == 'Product "group buy product" has been successfully updated!' - product.reload - product.group_buy.should be_true - product.group_buy_unit_size.should == 10.0 - end end context "as an enterprise user" do @@ -121,15 +103,6 @@ feature %q{ login_to_admin_as @new_user end - - context "additional fields" do - it "should have a notes field" do - product = create(:simple_product, supplier: @supplier2) - visit spree.edit_admin_product_path product - page.should have_content "Notes" - end - end - context "products do not require a tax category" do scenario "creating a new product", js: true do with_products_require_tax_category(false) do @@ -174,6 +147,22 @@ feature %q{ product.tax_category.should == tax_category end + scenario "editing product group buy options" do + product = product = create(:simple_product, supplier: @supplier2) + + visit spree.edit_admin_product_path product + within('#sidebar') { click_link 'Group Buy Options' } + choose('product_group_buy_1') + fill_in 'Bulk unit size', :with => '10' + + click_button 'Update' + + flash_message.should == "Product \"#{product.name}\" has been successfully updated!" + product.reload + product.group_buy.should be_true + product.group_buy_unit_size.should == 10.0 + end + scenario "editing product distributions" do product = create(:simple_product, supplier: @supplier2) @@ -195,6 +184,20 @@ feature %q{ product.distributors.should == [@distributors[0]] end + scenario "editing product SEO" do + product = product = create(:simple_product, supplier: @supplier2) + visit spree.edit_admin_product_path product + within('#sidebar') { click_link 'SEO' } + fill_in "product_meta_keywords", :with => 'Meta Keywords' + fill_in 'Meta Description', :with => 'Meta Description' + fill_in 'Notes', :with => 'Just testing Notes' + click_button 'Update' + flash_message.should == "Product \"#{product.name}\" has been successfully updated!" + product.reload + product.notes.should == 'Just testing Notes' + product.meta_keywords.should == 'Meta Keywords' + product.meta_description.should == 'Meta Description' + end scenario "deleting product properties", js: true do # Given a product with a property From 6006952603060714c838179ae98276867a706143 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 6 Oct 2017 08:45:33 +1100 Subject: [PATCH 347/353] Moving checkout request specs into their own folder --- .../{paypal_confirm_spec.rb => checkout/paypal_spec.rb} | 2 +- .../stripe_connect_spec.rb} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename spec/requests/{paypal_confirm_spec.rb => checkout/paypal_spec.rb} (96%) rename spec/requests/{stripe_connect_checkout_spec.rb => checkout/stripe_connect_spec.rb} (99%) diff --git a/spec/requests/paypal_confirm_spec.rb b/spec/requests/checkout/paypal_spec.rb similarity index 96% rename from spec/requests/paypal_confirm_spec.rb rename to spec/requests/checkout/paypal_spec.rb index 2f36d3a612..0537770474 100644 --- a/spec/requests/paypal_confirm_spec.rb +++ b/spec/requests/checkout/paypal_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe "confirming an order with paypal express payment method", type: :request do +describe "checking out an order with a paypal express payment method", type: :request do include ShopWorkflow let!(:address) { create(:address) } diff --git a/spec/requests/stripe_connect_checkout_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb similarity index 99% rename from spec/requests/stripe_connect_checkout_spec.rb rename to spec/requests/checkout/stripe_connect_spec.rb index b3d21afdd5..d78b315e34 100644 --- a/spec/requests/stripe_connect_checkout_spec.rb +++ b/spec/requests/checkout/stripe_connect_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe "Submitting Stripe Connect charge requests", type: :request do +describe "checking out an order with a Stripe Connect payment method", type: :request do include ShopWorkflow include AuthenticationWorkflow From c031b0e52b6d6133c120cee5793f342a544fd0ee Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 6 Oct 2017 12:16:13 +1100 Subject: [PATCH 348/353] Clear shipments and payments after failed payment at checkout --- app/controllers/checkout_controller.rb | 13 ++++ app/models/spree/order_decorator.rb | 7 +- .../requests/checkout/failed_checkout_spec.rb | 70 +++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 spec/requests/checkout/failed_checkout_spec.rb diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 62c91df4f4..817be578b0 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -15,6 +15,10 @@ class CheckoutController < Spree::CheckoutController include EnterprisesHelper def edit + # This is only required because of spree_paypal_express. If we implement + # a version of paypal that uses this controller, and more specifically + # the #update_failed method, then we can remove this call + restart_checkout end def update @@ -137,6 +141,7 @@ class CheckoutController < Spree::CheckoutController def update_failed clear_ship_address + restart_checkout respond_to do |format| format.html do render :edit @@ -155,6 +160,14 @@ class CheckoutController < Spree::CheckoutController end end + def restart_checkout + return if @order.state == 'cart' + @order.restart_checkout! # resets state to 'cart' + @order.shipments.with_state(:pending).destroy_all + @order.payments.with_state(:checkout).destroy_all + @order.reload + end + def skip_state_validation? true end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 0166fc10c1..31c14c0225 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -376,11 +376,14 @@ Spree::Order.class_eval do # object_params sets the payment amount to the order total, but it does this before # the shipping method is set. This results in the customer not being charged for their # order's shipping. To fix this, we refresh the payment amount here. - def charge_shipping! + def charge_shipping_and_payment_fees! update_totals return unless payments.any? payments.first.update_attribute :amount, total end end -Spree::Order.state_machine.after_transition to: :payment, do: :charge_shipping! +Spree::Order.state_machine.after_transition to: :payment, do: :charge_shipping_and_payment_fees! +Spree::Order.state_machine.event :restart_checkout do + transition :to => :cart, unless: :completed? +end diff --git a/spec/requests/checkout/failed_checkout_spec.rb b/spec/requests/checkout/failed_checkout_spec.rb new file mode 100644 index 0000000000..6246beadf0 --- /dev/null +++ b/spec/requests/checkout/failed_checkout_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe "checking out an order that initially fails", type: :request do + include ShopWorkflow + + let!(:shop) { create(:enterprise) } + let!(:order_cycle) { create(:simple_order_cycle) } + let!(:exchange) { create(:exchange, order_cycle: order_cycle, sender: order_cycle.coordinator, receiver: shop, incoming: false, pickup_time: "Monday") } + let!(:address) { create(:address) } + let!(:order) { create(:order, distributor: shop, order_cycle: order_cycle) } + let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) } + let!(:payment_method) { create(:bogus_payment_method, distributor_ids: [shop.id], environment: Rails.env) } + let!(:check_payment_method) { create(:payment_method, distributor_ids: [shop.id], environment: Rails.env) } + let!(:shipping_method) { create(:shipping_method, distributor_ids: [shop.id]) } + let!(:shipment) { create(:shipment, order: order, shipping_method: shipping_method) } + let(:params) do + { format: :json, order: { + shipping_method_id: shipping_method.id, + payments_attributes: [{payment_method_id: payment_method.id}], + bill_address_attributes: address.attributes.slice("firstname", "lastname", "address1", "address2", "phone", "city", "zipcode", "state_id", "country_id"), + ship_address_attributes: address.attributes.slice("firstname", "lastname", "address1", "address2", "phone", "city", "zipcode", "state_id", "country_id") + } } + end + + before do + order.reload.update_totals + set_order order + end + + context "when shipping and payment fees apply" do + let(:calculator) { Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10) } + + before do + payment_method.calculator = calculator.dup + payment_method.save! + check_payment_method.calculator = calculator.dup + check_payment_method.save! + shipping_method.calculator = calculator.dup + shipping_method.save! + end + + it "clears shipments and payments before rendering the checkout" do + put update_checkout_path, params + + # Checking out a BogusGateway without a source fails at :payment + # Shipments and payments should then be cleared before rendering checkout + expect(response.status).to be 400 + expect(flash[:error]).to eq I18n.t(:payment_processing_failed) + order.reload + expect(order.shipments.count).to be 0 + expect(order.payments.count).to be 0 + expect(order.adjustment_total).to eq 0 + + # Add another line item to change the fee totals + create(:line_item, order: order, quantity: 3, price: 5.00) + + # Use a check payment method, which should work + params[:order][:payments_attributes][0][:payment_method_id] = check_payment_method.id + put update_checkout_path, params + + expect(response.status).to be 200 + order.reload + expect(order.total).to eq 36 + expect(order.adjustment_total).to eq 6 + expect(order.item_total).to eq 30 + expect(order.shipments.count).to eq 1 + expect(order.payments.count).to eq 1 + end + end +end From f96502c369554e9a81d23b39724cc80f660c1727 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 11 Oct 2017 11:51:58 +1100 Subject: [PATCH 349/353] Add unit specs for CheckoutController#restart_checkout --- app/models/spree/payment_decorator.rb | 2 +- spec/controllers/checkout_controller_spec.rb | 65 ++++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb index a09e65c2fd..c9ae401541 100644 --- a/app/models/spree/payment_decorator.rb +++ b/app/models/spree/payment_decorator.rb @@ -105,7 +105,7 @@ module Spree # state is acheived through some trickery using an after_rollback callback on the # payment model. See Spree::Payment#persist_invalid def revoke_adjustment_eligibility - return unless adjustment.reload + return unless adjustment.try(:reload) return if adjustment.finalized? adjustment.update_attribute(:eligible, false) adjustment.finalize! diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 38d72d6442..51351797b1 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -190,16 +190,69 @@ describe CheckoutController do describe "Paypal routing" do let(:payment_method) { create(:payment_method, type: "Spree::Gateway::PayPalExpress") } before do - controller.stub(:current_distributor).and_return(distributor) - controller.stub(:current_order_cycle).and_return(order_cycle) - controller.stub(:current_order).and_return(order) + allow(controller).to receive(:current_distributor) { distributor } + allow(controller).to receive(:current_order_cycle) { order_cycle } + allow(controller).to receive(:current_order) { order } + allow(controller).to receive(:restart_checkout) end it "should check the payment method for Paypalness if we've selected one" do - Spree::PaymentMethod.should_receive(:find).with(payment_method.id.to_s).and_return payment_method - order.stub(:update_attributes).and_return true - order.stub(:state).and_return "payment" + expect(Spree::PaymentMethod).to receive(:find).with(payment_method.id.to_s) { payment_method } + allow(order).to receive(:update_attributes) { true } + allow(order).to receive(:state) { "payment" } spree_post :update, order: {payments_attributes: [{payment_method_id: payment_method.id}]} end end + + describe "#update_failed" do + before do + controller.instance_variable_set(:@order, order) + end + + it "clears the shipping address and restarts the checkout" do + expect(controller).to receive(:clear_ship_address) + expect(controller).to receive(:restart_checkout) + expect(controller).to receive(:respond_to) + controller.send(:update_failed) + end + end + + describe "#restart_checkout" do + let!(:shipment_pending) { create(:shipment, order: order, state: 'pending') } + let!(:payment_checkout) { create(:payment, order: order, state: 'checkout') } + let!(:payment_failed) { create(:payment, order: order, state: 'failed') } + + before do + controller.instance_variable_set(:@order, order.reload) + end + + context "when the order is already in the 'cart' state" do + it "does nothing" do + expect(order).to_not receive(:restart_checkout!) + controller.send(:restart_checkout) + end + end + + context "when the order is in a subsequent state" do + before do + order.update_attribute(:state, "payment") + end + + # NOTE: at the time of writing, it was not possible to create a shipment with a state other than + # 'pending' when the order has not been completed, so this is not a case that requires testing. + it "resets the order state, and clears incomplete shipments and payments" do + expect(order).to receive(:restart_checkout!).and_call_original + expect(order.shipments.count).to be 1 + expect(order.adjustments.shipping.count).to be 1 + expect(order.payments.count).to be 2 + expect(order.adjustments.payment_fee.count).to be 2 + controller.send(:restart_checkout) + expect(order.reload.state).to eq 'cart' + expect(order.shipments.count).to be 0 + expect(order.adjustments.shipping.count).to be 0 + expect(order.payments.count).to be 1 + expect(order.adjustments.payment_fee.count).to be 1 + end + end + end end From 1fcbf6b44dcc0981d287ffc412c968f1eed4f8d3 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 13 Oct 2017 12:45:02 +1100 Subject: [PATCH 350/353] Clear shipping_method_id from order when restarting checkout If the order is allowed to retain a shipping_method_id, then subsequent saves of the order will cause a new shipment to be initialised. Seems to only happen for delivery shipping methods. This is undesirable because fees for the new shipment will appear in the checkout summary, which is not smart enough to recognise existing shipment fees and adjust the order total accordingly. --- app/controllers/checkout_controller.rb | 1 + spec/controllers/checkout_controller_spec.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 817be578b0..ee0bb1f77d 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -163,6 +163,7 @@ class CheckoutController < Spree::CheckoutController def restart_checkout return if @order.state == 'cart' @order.restart_checkout! # resets state to 'cart' + @order.update_attributes!(shipping_method_id: nil) @order.shipments.with_state(:pending).destroy_all @order.payments.with_state(:checkout).destroy_all @order.reload diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 51351797b1..70332b5ea5 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -223,6 +223,7 @@ describe CheckoutController do let!(:payment_failed) { create(:payment, order: order, state: 'failed') } before do + order.update_attribute(:shipping_method_id, shipment_pending.shipping_method_id) controller.instance_variable_set(:@order, order.reload) end @@ -242,12 +243,14 @@ describe CheckoutController do # 'pending' when the order has not been completed, so this is not a case that requires testing. it "resets the order state, and clears incomplete shipments and payments" do expect(order).to receive(:restart_checkout!).and_call_original + expect(order.shipping_method_id).to_not be nil expect(order.shipments.count).to be 1 expect(order.adjustments.shipping.count).to be 1 expect(order.payments.count).to be 2 expect(order.adjustments.payment_fee.count).to be 2 controller.send(:restart_checkout) expect(order.reload.state).to eq 'cart' + expect(order.shipping_method_id).to be nil expect(order.shipments.count).to be 0 expect(order.adjustments.shipping.count).to be 0 expect(order.payments.count).to be 1 From 768378b1472b32ada4ec5b7e937ac3c1f37371b8 Mon Sep 17 00:00:00 2001 From: Pierre de Lacroix Date: Mon, 25 Sep 2017 14:04:54 +0200 Subject: [PATCH 351/353] Add pointer cursor to EXPAND ALL link in products bulk edit --- .../spree/admin/products/bulk_edit/_products_head.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml index 9a8f7ad86a..5bd4418ac2 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml @@ -19,7 +19,7 @@ %thead %tr{ ng: { controller: "ColumnsCtrl" } } %th.left-actions - %a{ 'ng-click' => 'toggleShowAllVariants()', :style => 'color: red' } + %a{ 'ng-click' => 'toggleShowAllVariants()', :style => 'color: red; cursor: pointer' } = t(:expand_all) %th.producer{ 'ng-show' => 'columns.producer.visible' }=t('admin.producer') %th.sku{ 'ng-show' => 'columns.sku.visible' }=t('admin.sku') From 1fe10b4b25da9f2551cbe6bf0a9dd5f38657c187 Mon Sep 17 00:00:00 2001 From: Pierre de Lacroix Date: Mon, 25 Sep 2017 16:17:35 +0200 Subject: [PATCH 352/353] Add pointer cursor to other links in products bulk edit --- app/assets/stylesheets/admin/products.css.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/assets/stylesheets/admin/products.css.scss b/app/assets/stylesheets/admin/products.css.scss index b14dadd866..79854e4c33 100644 --- a/app/assets/stylesheets/admin/products.css.scss +++ b/app/assets/stylesheets/admin/products.css.scss @@ -69,4 +69,10 @@ table#listing_products.bulk { margin-bottom: 0.5em; } } + + td.left-actions { + a.view-variants, a.add-variant { + cursor: pointer; + } + } } From bca409bfe43aeefdfd2a9bad8890d158efcc7dbe Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 6 Oct 2017 08:23:08 +1100 Subject: [PATCH 353/353] Bumping very outdated versions of pry and byebug --- Gemfile | 3 ++- Gemfile.lock | 26 +++++++++++--------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/Gemfile b/Gemfile index 36ca2fc59a..72c12fa7c5 100644 --- a/Gemfile +++ b/Gemfile @@ -126,7 +126,8 @@ group :test do end group :development do - gem 'pry-byebug' + gem 'byebug', '~> 9.0.0' # 9.1 requires ruby 2.2 + gem 'pry-byebug', '>= 3.4.3' gem 'debugger-linecache' gem 'guard' gem 'guard-livereload' diff --git a/Gemfile.lock b/Gemfile.lock index d41516fe5e..241c6298bb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -198,9 +198,7 @@ GEM blockenspiel (0.4.5) bugsnag (4.1.0) builder (3.0.4) - byebug (2.7.0) - columnize (~> 0.3) - debugger-linecache (~> 1.2) + byebug (9.0.6) cancan (1.6.8) capybara (2.7.1) addressable @@ -217,7 +215,7 @@ GEM cliver (0.3.2) cocaine (0.5.8) climate_control (>= 0.0.3, < 1.0) - coderay (1.0.9) + coderay (1.1.2) coffee-rails (3.2.2) coffee-script (>= 2.2.0) railties (~> 3.2.0) @@ -226,7 +224,6 @@ GEM execjs coffee-script-source (1.10.0) colorize (0.8.1) - columnize (0.9.0) compass (1.0.3) chunky_png (~> 1.2) compass-core (~> 1.0.2) @@ -474,7 +471,7 @@ GEM mail (2.5.4) mime-types (~> 1.16) treetop (~> 1.4.8) - method_source (0.8.2) + method_source (0.9.0) mime-types (1.25.1) mini_portile2 (2.1.0) momentjs-rails (2.5.1) @@ -523,13 +520,12 @@ GEM activerecord (~> 3.0) polyglot (0.3.5) powerpack (0.1.1) - pry (0.9.12.2) - coderay (~> 1.0.5) - method_source (~> 0.8) - slop (~> 3.4) - pry-byebug (1.3.2) - byebug (~> 2.7) - pry (~> 0.9.12) + pry (0.11.1) + coderay (~> 1.1.0) + method_source (~> 0.9.0) + pry-byebug (3.4.3) + byebug (>= 9.0, < 9.1) + pry (~> 0.10) rabl (0.7.2) activesupport (>= 2.3.14) multi_json (~> 1.0) @@ -629,7 +625,6 @@ GEM thor (~> 0.14) shoulda-matchers (1.1.0) activesupport (>= 3.0.0) - slop (3.4.5) spinjs-rails (1.3) rails (>= 3.1) sprockets (2.2.3) @@ -707,6 +702,7 @@ DEPENDENCIES aws-sdk blockenspiel bugsnag + byebug (~> 9.0.0) capybara coffee-rails (~> 3.2.1) compass-rails @@ -755,7 +751,7 @@ DEPENDENCIES parallel_tests pg poltergeist - pry-byebug + pry-byebug (>= 3.4.3) rabl rack-livereload rack-ssl