From 7b33712f7ac51bcf2021b82c7df8182d7ef6da87 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:27:25 +0100 Subject: [PATCH 01/27] Add an API endpoint for EnterpriseFeesController#destroy --- .../directives/delete_resource.js.coffee | 2 +- .../api/enterprise_fees_controller.rb | 21 +++++++++++++++++++ config/routes.rb | 2 ++ config/routes/admin.rb | 2 +- 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 app/controllers/api/enterprise_fees_controller.rb diff --git a/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee b/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee index c5936393db..ba37f0518f 100644 --- a/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee +++ b/app/assets/javascripts/admin/enterprise_fees/directives/delete_resource.js.coffee @@ -1,7 +1,7 @@ angular.module('admin.enterpriseFees').directive 'spreeDeleteResource', -> (scope, element, attrs) -> if scope.enterprise_fee.id - url = '/admin/enterprise_fees/' + scope.enterprise_fee.id + url = '/api/enterprise_fees/' + scope.enterprise_fee.id html = '' #var html = 'Delete Delete'; element.append html diff --git a/app/controllers/api/enterprise_fees_controller.rb b/app/controllers/api/enterprise_fees_controller.rb new file mode 100644 index 0000000000..3d03b28515 --- /dev/null +++ b/app/controllers/api/enterprise_fees_controller.rb @@ -0,0 +1,21 @@ +module Api + class EnterpriseFeesController < BaseController + respond_to :json + + def destroy + authorize! :destroy, enterprise_fee + + if enterprise_fee.destroy + render text: I18n.t(:successfully_removed), status: 204 + else + render text: enterprise_fee.errors.full_messages.first, status: 403 + end + end + + private + + def enterprise_fee + @enterprise_fee ||= EnterpriseFee.find_by_id params[:id] + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 1196e950ee..4804e4cae2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -111,6 +111,8 @@ Openfoodnetwork::Application.routes.draw do resources :customers, only: [:index, :update] + resources :enterprise_fees, only: [:destroy] + post '/product_images/:product_id', to: 'product_images#update_product_image' end diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c191322d74..1f55f8d154 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -35,7 +35,7 @@ Openfoodnetwork::Application.routes.draw do resources :enterprise_relationships resources :enterprise_roles - resources :enterprise_fees do + resources :enterprise_fees, except: :destroy do collection do get :for_order_cycle post :bulk_update, :as => :bulk_update From a162a2c50b14ce13521f62293fdcad690051d7bc Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:31:22 +0100 Subject: [PATCH 02/27] Move product distributions check as a before_destroy in EnterpriseFee model --- app/models/enterprise_fee.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 05276cf3d7..60c8bb9726 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -26,6 +26,7 @@ class EnterpriseFee < ActiveRecord::Base validates_presence_of :name before_save :ensure_valid_tax_category_settings + before_destroy :ensure_no_product_distributions scope :for_enterprise, lambda { |enterprise| where(enterprise_id: enterprise) } scope :for_enterprises, lambda { |enterprises| where(enterprise_id: enterprises) } @@ -68,6 +69,15 @@ class EnterpriseFee < ActiveRecord::Base return true end + def ensure_no_product_distributions + dependent_distribution = ProductDistribution.where(enterprise_fee_id: self).first + return unless dependent_distribution + product = dependent_distribution.product + error = I18n.t(:enterprise_fees_destroy_error, id: product.id, name: product.name) + errors.add(:base, error) + false + end + def refresh_products_cache OpenFoodNetwork::ProductsCache.enterprise_fee_changed self end From 1309d80f65827a9db4b3d0cb78a1f1313b76585d Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Thu, 15 Nov 2018 16:22:30 +0100 Subject: [PATCH 03/27] Import products#edit and form partial in our codebase and apply overrides --- .../add_description_wysiwyg.html.haml.deface | 3 - .../add_primary_taxon_field.html.haml.deface | 3 - .../_form/add_supplier.html.haml.deface | 6 - .../_form/add_units_form.html.haml.deface | 17 --- .../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 - .../admin/products/edit/add_angular.deface | 2 - .../edit/nojs_new_product.html.haml.deface | 3 - .../edit/return_to_products.html.haml.deface | 3 - .../spree/admin/products/_form.html.haml | 106 ++++++++++++++++++ app/views/spree/admin/products/edit.html.haml | 14 +++ 16 files changed, 120 insertions(+), 52 deletions(-) delete mode 100644 app/overrides/spree/admin/products/_form/add_description_wysiwyg.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/_form/add_primary_taxon_field.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/_form/add_units_form.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/_form/remove_available_on.deface delete mode 100644 app/overrides/spree/admin/products/_form/remove_cost_currency.deface delete mode 100644 app/overrides/spree/admin/products/_form/remove_cost_price.deface delete mode 100644 app/overrides/spree/admin/products/_form/remove_meta_description.deface delete mode 100644 app/overrides/spree/admin/products/_form/remove_option_types_and_taxons.deface delete mode 100644 app/overrides/spree/admin/products/_form/replace_master_price_label.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/_form/replace_taxons_div.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/edit/add_angular.deface delete mode 100644 app/overrides/spree/admin/products/edit/nojs_new_product.html.haml.deface delete mode 100644 app/overrides/spree/admin/products/edit/return_to_products.html.haml.deface create mode 100644 app/views/spree/admin/products/_form.html.haml create mode 100644 app/views/spree/admin/products/edit.html.haml diff --git a/app/overrides/spree/admin/products/_form/add_description_wysiwyg.html.haml.deface b/app/overrides/spree/admin/products/_form/add_description_wysiwyg.html.haml.deface deleted file mode 100644 index d04e787482..0000000000 --- a/app/overrides/spree/admin/products/_form/add_description_wysiwyg.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace "[data-hook=admin_product_form_left] code[erb-loud]:contains('f.text_area :description')" -%text-angular{'id' => 'product_description', 'name' => 'product[description]', 'class' => 'text-angular', 'textangular-strip' => true, 'ta-paste' => "stripFormatting($html)", 'ta-toolbar' => "[['bold','italics','clear']]"} - = sanitize(@product.description) 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 deleted file mode 100644 index 46afe1f94d..0000000000 --- a/app/overrides/spree/admin/products/_form/add_primary_taxon_field.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ 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 deleted file mode 100644 index a814c10b80..0000000000 --- a/app/overrides/spree/admin/products/_form/add_supplier.html.haml.deface +++ /dev/null @@ -1,6 +0,0 @@ -/ 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 \ No newline at end of file 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 deleted file mode 100644 index 6571fb3ab6..0000000000 --- a/app/overrides/spree/admin/products/_form/add_units_form.html.haml.deface +++ /dev/null @@ -1,17 +0,0 @@ -/ insert_top "[data-hook='admin_product_form_right']" - -.variant_units_form{ 'ng-app' => 'admin.products', 'ng-controller' => 'editUnitsCtrl' } - - = f.field_container :units do - = 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' => ''} - - = f.text_field :variant_unit, {'id' => 'variant_unit', 'ng-value' => 'product.variant_unit', 'hidden' => true} - = f.text_field :variant_unit_scale, {'id' => 'variant_unit_scale', 'ng-value' => 'product.variant_unit_scale', 'hidden' => true} - - .variant_unit_name{'ng-show' => 'product.variant_unit == "items"'} - = f.field_container :variant_unit_name do - = f.label :variant_unit_name, t(:spree_admin_variant_unit_name) - = f.text_field :variant_unit_name, {placeholder: t('admin.products.unit_name_placeholder')} - = f.error_message_on :variant_unit_name diff --git a/app/overrides/spree/admin/products/_form/remove_available_on.deface b/app/overrides/spree/admin/products/_form/remove_available_on.deface deleted file mode 100644 index 645fb76bde..0000000000 --- a/app/overrides/spree/admin/products/_form/remove_available_on.deface +++ /dev/null @@ -1,2 +0,0 @@ -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 deleted file mode 100644 index a194a5e03a..0000000000 --- a/app/overrides/spree/admin/products/_form/remove_cost_currency.deface +++ /dev/null @@ -1,2 +0,0 @@ -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 deleted file mode 100644 index 9f9cd8ec3f..0000000000 --- a/app/overrides/spree/admin/products/_form/remove_cost_price.deface +++ /dev/null @@ -1,2 +0,0 @@ -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 deleted file mode 100644 index 15a00a5df0..0000000000 --- a/app/overrides/spree/admin/products/_form/remove_meta_description.deface +++ /dev/null @@ -1 +0,0 @@ -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 deleted file mode 100644 index eeccddc25c..0000000000 --- a/app/overrides/spree/admin/products/_form/remove_option_types_and_taxons.deface +++ /dev/null @@ -1 +0,0 @@ -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 deleted file mode 100644 index 02152cb521..0000000000 --- a/app/overrides/spree/admin/products/_form/replace_master_price_label.html.haml.deface +++ /dev/null @@ -1,2 +0,0 @@ -/ 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 deleted file mode 100644 index 697255ab70..0000000000 --- a/app/overrides/spree/admin/products/_form/replace_taxons_div.html.haml.deface +++ /dev/null @@ -1,5 +0,0 @@ -/ 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/products/edit/add_angular.deface b/app/overrides/spree/admin/products/edit/add_angular.deface deleted file mode 100644 index 926c47c0ec..0000000000 --- a/app/overrides/spree/admin/products/edit/add_angular.deface +++ /dev/null @@ -1,2 +0,0 @@ -add_to_attributes 'fieldset.no-border-top' -attributes 'ng-app' => 'admin.products' \ No newline at end of file diff --git a/app/overrides/spree/admin/products/edit/nojs_new_product.html.haml.deface b/app/overrides/spree/admin/products/edit/nojs_new_product.html.haml.deface deleted file mode 100644 index ff1bad39cd..0000000000 --- a/app/overrides/spree/admin/products/edit/nojs_new_product.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace_contents "#new_product_link" - -= button_link_to t(:new_product), new_object_url, {:icon => 'icon-plus', :id => 'admin_new_product' } diff --git a/app/overrides/spree/admin/products/edit/return_to_products.html.haml.deface b/app/overrides/spree/admin/products/edit/return_to_products.html.haml.deface deleted file mode 100644 index 1865179b4c..0000000000 --- a/app/overrides/spree/admin/products/edit/return_to_products.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace "code[erb-loud]:contains('button_link_to t(:back_to_products_list)')" - -= button_link_to t('admin.products.back_to_products_list'), admin_products_path, :icon => 'icon-arrow-left' diff --git a/app/views/spree/admin/products/_form.html.haml b/app/views/spree/admin/products/_form.html.haml new file mode 100644 index 0000000000..4e97d209cd --- /dev/null +++ b/app/views/spree/admin/products/_form.html.haml @@ -0,0 +1,106 @@ +%div{"data-hook" => "admin_product_form_fields"} + .left.eight.columns.alpha + = f.field_container :name do + = f.label :name, raw(t(:name) + content_tag(:span, ' *', :class => 'required')) + = f.text_field :name, :class => 'fullwidth title' + = f.error_message_on :name + + = f.field_container :permalink do + = f.label :permalink, raw(t(:permalink) + content_tag(:span, ' *', :class => "required")) + = f.text_field :permalink, :class => 'fullwidth title' + = f.error_message_on :permalink + + = f.field_container :description do + = f.label :description, t(:description) + %text-angular{'id' => 'product_description', 'name' => 'product[description]', 'class' => 'text-angular', 'textangular-strip' => true, 'ta-paste' => "stripFormatting($html)", 'ta-toolbar' => "[['bold','italics','clear']]"} + = sanitize(@product.description) + = f.error_message_on :description + + = f.field_container :taxons do + = f.label :taxon_ids, t(:taxons) + %br + = f.hidden_field :taxon_ids, :value => @product.taxon_ids.join(',') + + .right.four.columns.omega + .variant_units_form{ 'ng-app' => 'admin.products', 'ng-controller' => 'editUnitsCtrl' } + + = f.field_container :units do + = 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' => ''} + + = f.text_field :variant_unit, {'id' => 'variant_unit', 'ng-value' => 'product.variant_unit', 'hidden' => true} + = f.text_field :variant_unit_scale, {'id' => 'variant_unit_scale', 'ng-value' => 'product.variant_unit_scale', 'hidden' => true} + + .variant_unit_name{'ng-show' => 'product.variant_unit == "items"'} + = f.field_container :variant_unit_name do + = f.label :variant_unit_name, t(:spree_admin_variant_unit_name) + = f.text_field :variant_unit_name, {placeholder: t('admin.products.unit_name_placeholder')} + = f.error_message_on :variant_unit_name + + = render 'spree/admin/products/primary_taxon_form', f: f + + = 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.field_container :price do + = f.label :price, raw(t(:price) + content_tag(:span, ' *', :class => 'required')) + = f.text_field :price, :value => number_to_currency(@product.price, :unit => '') + = f.error_message_on :price + + .clear + + - unless @product.has_variants? + = f.field_container :sku do + = f.label :sku, t(:sku) + = f.text_field :sku, :size => 16 + + - if Spree::Config[:track_inventory_levels] + .alpha.two.columns + = f.field_container :on_hand do + = f.label :on_hand, t(:on_hand) + = f.number_field :on_hand, :min => 0 + .omega.two.columns + = f.field_container :on_demand, :class => ['checkbox'] do + %label + = f.check_box :on_demand + = t(:on_demand) + + .clear + + %ul#shipping_specs + %li#shipping_specs_weight_field.field.alpha.two.columns + = f.label :weight, t(:weight) + = f.text_field :weight, :size => 4 + %li#shipping_specs_height_field.field.omega.two.columns + = f.label :height, t(:height) + = f.text_field :height, :size => 4 + %li#shipping_specs_width_field.field.alpha.two.columns + = f.label :width, t(:width) + = f.text_field :width, :size => 4 + %li#shipping_specs_depth_field.field.omega.two.columns + = f.label :depth, t(:depth) + = f.text_field :depth, :size => 4 + + = f.field_container :shipping_categories do + = f.label :shipping_category_id, t(:shipping_categories) + = f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, { :include_blank => 'None' }, { :class => 'select2' }) + = f.error_message_on :shipping_category + + = f.field_container :tax_category do + = f.label :tax_category_id, t(:tax_category) + = f.collection_select(:tax_category_id, @tax_categories, :id, :name, { :include_blank => 'None' }, { :class => 'select2' }) + = f.error_message_on :tax_category + + .clear + + %div + + .clear + +- unless Rails.env.test? + :javascript + $('.select2-container').css({width: '20em'}) diff --git a/app/views/spree/admin/products/edit.html.haml b/app/views/spree/admin/products/edit.html.haml new file mode 100644 index 0000000000..95ef4c061c --- /dev/null +++ b/app/views/spree/admin/products/edit.html.haml @@ -0,0 +1,14 @@ +- content_for :page_actions do + %li= button_link_to t('admin.products.back_to_products_list'), admin_products_path, :icon => 'icon-arrow-left' + %li#new_product_link + = button_link_to t(:new_product), new_object_url, { :icon => 'icon-plus', :id => 'admin_new_product' } + += render :partial => 'spree/admin/shared/product_sub_menu' + += render :partial => 'spree/admin/shared/product_tabs', :locals => { :current => 'Product Details' } += render :partial => 'spree/shared/error_messages', :locals => { :target => @product } + += form_for [:admin, @product], :method => :put, :html => { :multipart => true } do |f| + %fieldset.no-border-top{'ng-app' => 'admin.products'} + = render :partial => 'form', :locals => { :f => f } + = render :partial => 'spree/admin/shared/edit_resource_links' From 0868404e986b5c3e859b38c87718ee642397d172 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:32:03 +0100 Subject: [PATCH 04/27] Add specs for new Api::EnterpriseFeesController --- .../api/enterprise_fees_controller_spec.rb | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 spec/controllers/api/enterprise_fees_controller_spec.rb diff --git a/spec/controllers/api/enterprise_fees_controller_spec.rb b/spec/controllers/api/enterprise_fees_controller_spec.rb new file mode 100644 index 0000000000..fb938d43c5 --- /dev/null +++ b/spec/controllers/api/enterprise_fees_controller_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +module Api + describe EnterpriseFeesController, type: :controller do + include AuthenticationWorkflow + + let!(:unreferenced_fee) { create(:enterprise_fee) } + let!(:referenced_fee) { create(:enterprise_fee) } + let(:product) { create(:product) } + let(:distributor) { create(:distributor_enterprise) } + let!(:product_distribution) { create(:product_distribution, product: product, distributor: distributor, enterprise_fee: referenced_fee) } + let(:current_user) { create(:admin_user) } + + before do + allow(controller).to receive(:spree_current_user) { current_user } + end + + describe "destroy" do + it "removes the fee" do + expect { spree_delete :destroy, id: unreferenced_fee.id, format: :json } + .to change { EnterpriseFee.count }.by -1 + end + + context "when the fee is referenced by a product distribution" do + it "does not remove the fee" do + spree_delete :destroy, id: referenced_fee.id, format: :json + expect(response.status).to eq 403 + expect(response.body).to match(/That enterprise fee cannot be deleted/) + expect(referenced_fee.reload).to eq(referenced_fee) + end + end + end + end +end From a50786be3470faa272ba216796e0492f288ca97d Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Fri, 9 Nov 2018 17:34:22 +0100 Subject: [PATCH 05/27] Remove old do_not_remove_referenced_fees method --- .../admin/enterprise_fees_controller.rb | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/app/controllers/admin/enterprise_fees_controller.rb b/app/controllers/admin/enterprise_fees_controller.rb index 528284c4e4..46e2b535fb 100644 --- a/app/controllers/admin/enterprise_fees_controller.rb +++ b/app/controllers/admin/enterprise_fees_controller.rb @@ -2,7 +2,6 @@ module Admin class EnterpriseFeesController < ResourceController before_filter :load_enterprise_fee_set, :only => :index before_filter :load_data - before_filter :do_not_destroy_referenced_fees, :only => :destroy def index @@ -45,22 +44,6 @@ module Admin private - def do_not_destroy_referenced_fees - product_distribution = ProductDistribution.where(:enterprise_fee_id => @object).first - if product_distribution - p = product_distribution.product - error = I18n.t(:enterprise_fees_destroy_error, id: p.id, name: p.name) - - respond_with(@object) do |format| - format.html do - flash[:error] = error - redirect_to collection_url - end - format.js { render text: error, status: 403 } - end - end - end - def load_enterprise_fee_set @enterprise_fee_set = EnterpriseFeeSet.new :collection => collection end From 3301b5850a5dffacb0b308efad5e010f73038fd5 Mon Sep 17 00:00:00 2001 From: Maxim Colls Date: Sat, 17 Nov 2018 18:48:10 +0100 Subject: [PATCH 06/27] Improved UX in the accordion steps in the checkout page --- .../checkout/accordion_controller.js.coffee | 25 ++++++++----------- .../checkout/billing_controller.js.coffee | 4 +-- .../checkout/details_controller.js.coffee | 6 ++--- .../darkswarm/mixins/fieldset_mixin.js.coffee | 4 +-- app/views/checkout/edit.html.haml | 2 +- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee index e85f9e5cee..d61ff6ceb7 100644 --- a/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee @@ -1,25 +1,22 @@ Darkswarm.controller "AccordionCtrl", ($scope, localStorageService, $timeout, $document, CurrentHub) -> - key = "accordion_#{$scope.order.id}#{CurrentHub.hub.id}#{$scope.order.user_id}" - value = if localStorageService.get(key) then {} else { details: true, billing: false, shipping: false, payment: false } - localStorageService.bind $scope, "accordion", value, key $scope.accordionSections = ["details", "billing", "shipping", "payment"] - # Scrolling is confused by our position:fixed top bar - add an offset to scroll - # to the correct location, plus 5px buffer - offset_height = $("nav.top-bar").height() + 5 + $scope.accordion = { details: true, billing: true, shipping: true, payment: true } - $scope.show = (section)-> + $scope.show = (section) -> $scope.accordion[section] = true - # If we call scrollTo() directly after show(), when one of the accordions above the - # scroll location is closed by show(), scrollTo() will scroll to the old location of - # the element. Putting this in a 50 ms timeout is enough delay for the DOM to - # have updated. - $timeout -> - $document.scrollTo($("##{section}"), offset_height, 500) - , 50 + + $scope.scrollTo = (section) -> + # Scrolling is confused by our position:fixed top bar - add an offset to scroll + # to the correct location, plus 5px buffer + offset_height = $("nav.top-bar").height() + 5 + $document.scrollTo($("##{section}"), offset_height, 400) $scope.$on 'purchaseFormInvalid', (event, form) -> # Scroll to first invalid section for section in $scope.accordionSections if not form[section].$valid $scope.show section + $timeout -> + $scope.scrollTo(section) + , 50 break diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee index b8dec8b190..b7246fddd9 100644 --- a/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee @@ -5,7 +5,7 @@ Darkswarm.controller "BillingCtrl", ($scope, $timeout) -> $scope.summary = -> [$scope.order.bill_address.address1, - $scope.order.bill_address.city, + $scope.order.bill_address.city, $scope.order.bill_address.zipcode] - $timeout $scope.onTimeout + $timeout $scope.onTimeout diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee index e8a6b1ff23..30c243ed7c 100644 --- a/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee @@ -13,11 +13,11 @@ Darkswarm.controller "DetailsCtrl", ($scope, $timeout, $http, CurrentUser, Authe $scope.summary = -> [$scope.fullName(), - $scope.order.email, + $scope.order.email, $scope.order.bill_address.phone] $scope.fullName = -> - [$scope.order.bill_address.firstname ? null, + [$scope.order.bill_address.firstname ? null, $scope.order.bill_address.lastname ? null].join(" ").trim() - $timeout $scope.onTimeout + $timeout $scope.onTimeout diff --git a/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee b/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee index 739d2de9bd..d8bb1b262e 100644 --- a/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee +++ b/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee @@ -2,11 +2,11 @@ window.FieldsetMixin = ($scope)-> $scope.next = (event = false)-> event.preventDefault() if event return unless $scope.nextPanel + $scope.accordion[$scope.name] = false $scope.show $scope.nextPanel $scope.onTimeout = -> - if $scope[$scope.name].$valid - $scope.next() + $scope.accordion[$scope.name] = !$scope[$scope.name].$valid $scope.valid = -> $scope.form().$valid diff --git a/app/views/checkout/edit.html.haml b/app/views/checkout/edit.html.haml index 75c9eff08f..ab5253538b 100644 --- a/app/views/checkout/edit.html.haml +++ b/app/views/checkout/edit.html.haml @@ -16,7 +16,7 @@ = render partial: "shopping_shared/details" - %accordion{"close-others" => "true"} + %accordion{"close-others" => "false"} %checkout.row{"ng-controller" => "CheckoutCtrl"} .small-12.medium-8.large-9.columns - unless spree_current_user From 884d4d0122d1627287055310d1115ddfb8b26b0b Mon Sep 17 00:00:00 2001 From: Maxim Colls Date: Sat, 17 Nov 2018 19:46:09 +0100 Subject: [PATCH 07/27] Fixed specs --- .../consumer/shopping/checkout_spec.rb | 34 ++++--------------- .../shopping/embedded_shopfronts_spec.rb | 3 -- .../shopping/variant_overrides_spec.rb | 4 --- 3 files changed, 6 insertions(+), 35 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index e4775cbdd3..ec4b86b698 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -64,19 +64,18 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do let(:user) { create(:user) } def fill_out_form - toggle_shipping choose sm1.name - toggle_payment choose pm1.name - toggle_details + within "#details" do fill_in "First Name", with: "Will" fill_in "Last Name", with: "Marshall" fill_in "Email", with: "test@test.com" fill_in "Phone", with: "0468363090" end - toggle_billing + check "Save as default billing address" + within "#billing" do fill_in "City", with: "Melbourne" fill_in "Postcode", with: "3066" @@ -85,7 +84,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do select "Victoria", from: "State" end - toggle_shipping check "Shipping address same as billing address?" check "Save as default shipping address" end @@ -149,7 +147,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do it "checks out successfully" do visit checkout_path choose sm2.name - toggle_payment choose pm1.name expect do @@ -192,7 +189,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do visit checkout_path fill_out_form - toggle_payment choose stripe_pm.name end @@ -228,7 +224,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do end it "shows a breakdown of the order price" do - toggle_shipping choose sm2.name page.should have_selector 'orderdetails .cart-total', text: with_currency(11.23) @@ -242,7 +237,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do end it "shows all shipping methods in order by name" do - toggle_shipping within '#shipping' do expect(page).to have_selector "label", count: 4 # Three shipping methods + instructions label labels = page.all('label').map(&:text) @@ -254,7 +248,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do context "when shipping method requires an address" do before do - toggle_shipping choose sm1.name end it "shows ship address forms when 'same as billing address' is unchecked" do @@ -269,7 +262,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do it "shows shipping methods allowed by the rule" do # No rules in effect - toggle_shipping page.should have_content "Frogs" page.should have_content "Donkeys" page.should have_content "Local" @@ -315,7 +307,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do before do visit checkout_path checkout_as_guest - toggle_payment end it "shows all available payment methods" do @@ -326,8 +317,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do describe "purchasing" do it "takes us to the order confirmation page when we submit a complete form" do - toggle_details - within "#details" do fill_in "First Name", with: "Will" fill_in "Last Name", with: "Marshall" @@ -335,8 +324,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do fill_in "Phone", with: "0468363090" end - toggle_billing - within "#billing" do fill_in "Address", with: "123 Your Face" select "Australia", from: "Country" @@ -345,15 +332,11 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do fill_in "Postcode", with: "3066" end - toggle_shipping - within "#shipping" do choose sm2.name fill_in 'Any comments or special instructions?', with: "SpEcIaL NoTeS" end - toggle_payment - within "#payment" do choose pm1.name end @@ -382,18 +365,16 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do context "with basic details filled" do before do - toggle_shipping choose sm1.name - toggle_payment choose pm1.name - toggle_details + within "#details" do fill_in "First Name", with: "Will" fill_in "Last Name", with: "Marshall" fill_in "Email", with: "test@test.com" fill_in "Phone", with: "0468363090" end - toggle_billing + within "#billing" do fill_in "City", with: "Melbourne" fill_in "Postcode", with: "3066" @@ -401,7 +382,7 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do select "Australia", from: "Country" select "Victoria", from: "State" end - toggle_shipping + check "Shipping address same as billing address?" end @@ -441,7 +422,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do expect(page).to have_selector ".transaction-fee td", text: with_currency(0.00) expect(page).to have_selector ".total", text: with_currency(11.23) - toggle_payment choose "#{pm2.name} (#{with_currency(5.67)})" expect(page).to have_selector ".transaction-fee td", text: with_currency(5.67) @@ -463,7 +443,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do let!(:pm1) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: gateway_type) } it "takes us to the order confirmation page when submitted with a valid credit card" do - toggle_payment fill_in 'Card Number', with: "4111111111111111" select 'February', from: 'secrets.card_month' select (Date.current.year+1).to_s, from: 'secrets.card_year' @@ -478,7 +457,6 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do end it "shows the payment processing failed message when submitted with an invalid credit card" do - toggle_payment fill_in 'Card Number', with: "9999999988887777" select 'February', from: 'secrets.card_month' select (Date.current.year+1).to_s, from: 'secrets.card_year' diff --git a/spec/features/consumer/shopping/embedded_shopfronts_spec.rb b/spec/features/consumer/shopping/embedded_shopfronts_spec.rb index 39a79aba2e..80f7e52eee 100644 --- a/spec/features/consumer/shopping/embedded_shopfronts_spec.rb +++ b/spec/features/consumer/shopping/embedded_shopfronts_spec.rb @@ -67,7 +67,6 @@ feature "Using embedded shopfront functionality", js: true do fill_in "Phone", with: "0456789012" end - toggle_billing within "#billing" do fill_in "Address", with: "123 Street" select "Australia", from: "Country" @@ -76,12 +75,10 @@ feature "Using embedded shopfront functionality", js: true do fill_in "Postcode", with: "3066" end - toggle_shipping within "#shipping" do find('input[type="radio"]').trigger 'click' end - toggle_payment within "#payment" do find('input[type="radio"]').trigger 'click' end diff --git a/spec/features/consumer/shopping/variant_overrides_spec.rb b/spec/features/consumer/shopping/variant_overrides_spec.rb index 49d1b0b2e4..ff101fd5a0 100644 --- a/spec/features/consumer/shopping/variant_overrides_spec.rb +++ b/spec/features/consumer/shopping/variant_overrides_spec.rb @@ -173,7 +173,6 @@ feature "shopping with variant overrides defined", js: true, retry: 3 do fill_in "Phone", with: "0456789012" end - toggle_billing within "#billing" do fill_in "Address", with: "123 Street" select "Australia", from: "Country" @@ -182,12 +181,10 @@ feature "shopping with variant overrides defined", js: true, retry: 3 do fill_in "Postcode", with: "3066" end - toggle_shipping within "#shipping" do choose sm.name end - toggle_payment within "#payment" do choose pm.name end @@ -201,5 +198,4 @@ feature "shopping with variant overrides defined", js: true, retry: 3 do wait_until_enabled 'li.cart a.button' first(:link, 'Checkout now').click end - end From 783e0eed3c31cf2d62432d88d961abe8c0017886 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 20 Nov 2018 22:19:49 +0000 Subject: [PATCH 08/27] In admin/users, convert Spree.t calls to t calls using lazy lookup and move translations to ofn's en.yml --- app/views/spree/admin/users/_form.html.haml | 10 +++++----- app/views/spree/admin/users/edit.html.haml | 6 +++--- app/views/spree/admin/users/index.html.haml | 14 +++++++------- config/locales/en.yml | 19 +++++++++++++++++-- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index e495f29632..47bf167151 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -1,11 +1,11 @@ .row .alpha.five.columns = f.field_container :email do - = f.label :email, Spree.t(:email) + = f.label :email, t(".email") = f.email_field :email, class: "fullwidth" = error_message_on :user, :email .field - = label_tag nil, Spree.t(:roles) + = label_tag nil, t(".roles") %ul - @roles.each do |role| %li @@ -13,14 +13,14 @@ = label_tag role.name = hidden_field_tag "user[spree_role_ids][]", "" = f.field_container :enterprise_limit do - = f.label :enterprise_limit, t(:enterprise_limit) + = f.label :enterprise_limit, t(".enterprise_limit") = f.text_field :enterprise_limit, class: "fullwidth" .omega.five.columns = f.field_container :password do - = f.label :password, Spree.t(:password) + = f.label :password, t(".password") = f.password_field :password, class: "fullwidth" = f.error_message_on :password = f.field_container :password do - = f.label :password_confirmation, Spree.t(:confirm_password) + = f.label :password_confirmation, t(".confirm_password") = f.password_field :password_confirmation, class: "fullwidth" = f.error_message_on :password_confirmation \ No newline at end of file diff --git a/app/views/spree/admin/users/edit.html.haml b/app/views/spree/admin/users/edit.html.haml index 1457f1065a..b192342306 100644 --- a/app/views/spree/admin/users/edit.html.haml +++ b/app/views/spree/admin/users/edit.html.haml @@ -1,10 +1,10 @@ - content_for :page_title do - = Spree.t(:editing_user) + = t(".editing_user") - content_for :page_actions do %li - = button_link_to Spree.t(:back_to_users_list), spree.admin_users_path, icon: "icon-arrow-left" + = button_link_to t(".back_to_users_list"), spree.admin_users_path, icon: "icon-arrow-left" %fieldset.alpha.ten.columns{"data-hook" => "admin_user_edit_general_settings"} - %legend= Spree.t(:general_settings) + %legend= t(".general_settings") %div{"data-hook" => "admin_user_edit_form_header"} = render partial: "spree/shared/error_messages", locals: { target: @user } %div{"data-hook" => "admin_user_edit_form"} diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml index e7ef05d711..9672637e6c 100644 --- a/app/views/spree/admin/users/index.html.haml +++ b/app/views/spree/admin/users/index.html.haml @@ -1,8 +1,8 @@ - content_for :page_title do - = Spree.t(:listing_users) + = t(".listing_users") - content_for :page_actions do %li - = button_link_to Spree.t(:new_user), new_object_url, icon: "icon-plus", id: "admin_new_user_link" + = button_link_to t(".new_user"), new_object_url, icon: "icon-plus", id: "admin_new_user_link" = render "admin/shared/users_sub_menu" @@ -13,8 +13,8 @@ %col{ style: "width: 15%" } %thead %tr - %th= sort_link @search,:email, Spree.t(:user), {}, {title: "users_email_title"} - %th= sort_link @search,:enterprise_limit, t(:enterprise_limit) + %th= sort_link @search,:email, t(".user"), {}, {title: "users_email_title"} + %th= sort_link @search,:enterprise_limit, t(".enterprise_limit") %th.actions %tbody - @users.each do |user| @@ -28,13 +28,13 @@ = link_to_delete user, no_text: true = paginate @users - content_for :sidebar_title do - = Spree.t(:search) + = t(".search") - content_for :sidebar do .box.align-center = search_form_for [:admin, @search] do |f| .field - = f.label Spree.t(:email) + = f.label t(".email") %br = f.text_field :email_cont, class: "fullwidth" %div - = button Spree.t(:search), "icon-search" + = button t(".search"), "icon-search" diff --git a/config/locales/en.yml b/config/locales/en.yml index e203fe0039..433a171063 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -209,7 +209,6 @@ en: distributors: Distributors distribution: Distribution order_cycles: Order Cycles - enterprise_limit: Enterprise Limit bulk_order_management: Bulk Order Management enterprises: Enterprises enterprise_groups: Groups @@ -1939,7 +1938,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using enterprise_long_desc: "Long Description" enterprise_long_desc_placeholder: "This is your opportunity to tell the story of your enterprise - what makes you different and wonderful? We'd suggest keeping your description to under 600 characters or 150 words." enterprise_long_desc_length: "%{num} characters / up to 600 recommended" - enterprise_limit: Enterprise Limit enterprise_abn: "ABN" enterprise_abn_placeholder: "eg. 99 123 456 789" enterprise_acn: "ACN" @@ -2755,6 +2753,23 @@ See the %{link} to find out more about %{sitename}'s features and to start using bulk_coop_packing_sheets: 'Bulk Co-op - Packing Sheets' bulk_coop_customer_payments: 'Bulk Co-op - Customer Payments' users: + index: + listing_users: "Listing Users" + new_user: "New User" + user: "User" + enterprise_limit: "Enterprise Limit" + search: "Search" + email: "Email" + edit: + editing_user: "Editing User" + back_to_users_list: "Back To Users List" + general_settings: "General Settings" + form: + email: "Email" + roles: "Roles" + enterprise_limit: "Enterprise Limit" + confirm_password: "Confirm Password" + password: "Password" email_confirmation: confirmation_pending: "Email confirmation is pending. We've sent a confirmation email to %{address}." variants: From a5407d780c6a60d80a09724dcd6369cdddbde8e9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 18 Nov 2018 20:41:07 +0000 Subject: [PATCH 09/27] Include unexpected validation errors --- app/models/product_import/product_importer.rb | 3 ++- app/views/admin/product_import/_errors_list.html.haml | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/product_import/product_importer.rb b/app/models/product_import/product_importer.rb index 3bc66fa815..3b8671290f 100644 --- a/app/models/product_import/product_importer.rb +++ b/app/models/product_import/product_importer.rb @@ -100,7 +100,8 @@ module ProductImport entries[entry.line_number] = { attributes: entry.displayable_attributes, validates_as: entry.validates_as, - errors: entry.invalid_attributes + errors: entry.invalid_attributes, + product_validations: entry.product_validations } end entries.to_json diff --git a/app/views/admin/product_import/_errors_list.html.haml b/app/views/admin/product_import/_errors_list.html.haml index 787aabe76b..8397c7013b 100644 --- a/app/views/admin/product_import/_errors_list.html.haml +++ b/app/views/admin/product_import/_errors_list.html.haml @@ -7,3 +7,5 @@ ( {{entry.attributes.display_name}} ) %p.error{ng: {repeat: "(attribute, error) in entry.errors", show: "ignore_fields.indexOf(attribute) < 0" }}  -  {{error}} + %p.error{ng: {repeat: "(attribute, error) in entry.product_validations"}} +  -  {{error}} From b3a9b502fa6e2c0db91670089b3e874365705e91 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 18 Nov 2018 21:16:17 +0000 Subject: [PATCH 10/27] Add missing translation key --- config/locales/en.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index 34d4584439..41a1157759 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2395,6 +2395,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using validation_msg_relationship_already_established: "^That relationship is already established." validation_msg_at_least_one_hub: "^At least one hub must be selected" validation_msg_product_category_cant_be_blank: "^Product Category cant be blank" + validation_msg_tax: "^Tax Category is required" validation_msg_tax_category_cant_be_blank: "^Tax Category can't be blank" validation_msg_is_associated_with_an_exising_customer: "is associated with an existing customer" content_configuration_pricing_table: "(TODO: Pricing table)" From d4b49a662f0a03df92b510d62708eaa1a04a9da7 Mon Sep 17 00:00:00 2001 From: haseleyi Date: Wed, 21 Nov 2018 15:31:33 -0800 Subject: [PATCH 11/27] Add translation for 'Back to Payments List' button in new payment form --- config/locales/en.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index e203fe0039..a28451fd4d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -251,6 +251,7 @@ en: password_confirmation: Password Confirmation reset_password_token: Reset password token expired: has expired, please request a new one + back_to_payments_list: "Back to Payments List" actions: create_and_add_another: "Create and Add Another" From 95dbfae7573f35197e338fee0ad0566538fd9ff1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 22 Nov 2018 15:42:56 +1100 Subject: [PATCH 12/27] Use expect syntax --- spec/support/request/shop_workflow.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/request/shop_workflow.rb b/spec/support/request/shop_workflow.rb index 876f184644..9df7f72126 100644 --- a/spec/support/request/shop_workflow.rb +++ b/spec/support/request/shop_workflow.rb @@ -13,7 +13,7 @@ module ShopWorkflow end def set_order(order) - ApplicationController.any_instance.stub(:session).and_return({order_id: order.id, access_token: order.token}) + allow_any_instance_of(ApplicationController).to receive(:session).and_return({order_id: order.id, access_token: order.token}) end def add_product_to_cart(order, product, quantity: 1) From f4d5727fb455618822f4bbd8415dea35d686434e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 23 Nov 2018 15:38:35 +1100 Subject: [PATCH 13/27] Associate customers with their user records When we introduced the Customer model, we didn't associate any existing customers with users that have the same email address. Later we decided to create that association when users sign up. But we didn't update all the existing customers. We do that now for data consistency and to solve several bugs. --- ...1123012635_associate_customers_to_users.rb | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 db/migrate/20181123012635_associate_customers_to_users.rb diff --git a/db/migrate/20181123012635_associate_customers_to_users.rb b/db/migrate/20181123012635_associate_customers_to_users.rb new file mode 100644 index 0000000000..b42739ff68 --- /dev/null +++ b/db/migrate/20181123012635_associate_customers_to_users.rb @@ -0,0 +1,42 @@ +# When we introduced the Customer model, we didn't associate any existing +# customers with users that have the same email address. +# Later we decided to create that association when users sign up. But we didn't +# update all the existing customers. We do that now for data consistency and to +# solve several bugs. +# +# - https://github.com/openfoodfoundation/openfoodnetwork/pull/2084 +# - https://github.com/openfoodfoundation/openfoodnetwork/issues/2841 +class AssociateCustomersToUsers < ActiveRecord::Migration + class Customer < ActiveRecord::Base + end + + def up + save_customers + execute "UPDATE customers + SET user_id = spree_users.id + FROM spree_users + WHERE customers.email = spree_users.email + AND customers.user_id IS NULL;" + end + + def down + customers = backed_up_customers + Customer.where(id: customers).update_all(user_id: nil) + end + + def save_customers + customers = Customer. + joins("INNER JOIN spree_users ON customers.email = spree_users.email"). + where(user_id: nil).all + + File.write(backup_file, Marshal.dump(customers)) + end + + def backed_up_customers + Marshal.load(File.read(backup_file)) + end + + def backup_file + File.join("log", "customers_without_user_association.log") + end +end From 2abb316bccfe6c1a08bac8ec407ef35f56038586 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 23 Nov 2018 17:28:59 +0100 Subject: [PATCH 14/27] Update Skylight from 1.6.1 to 1.7.1 This doesn't bring too many improvements but it's the latest version can work on our Rails 3.2 version. You can read the changelog at https://github.com/skylightio/skylight-ruby/blob/master/CHANGELOG.md#170-april-24-2018. --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 17399ca6b1..f762df9268 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -688,7 +688,7 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - skylight (1.6.1) + skylight (1.7.1) activesupport (>= 3.0.0) spinjs-rails (1.3) rails (>= 3.1) From 4b588cbfb0d141af9ad913ac38fc6f3845d1f200 Mon Sep 17 00:00:00 2001 From: Maxim Colls Date: Fri, 23 Nov 2018 16:47:55 +0000 Subject: [PATCH 15/27] Removed unused toggle helpers in spec/support --- spec/support/request/checkout_workflow.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/spec/support/request/checkout_workflow.rb b/spec/support/request/checkout_workflow.rb index a38edaa6de..11a13194bc 100644 --- a/spec/support/request/checkout_workflow.rb +++ b/spec/support/request/checkout_workflow.rb @@ -18,16 +18,4 @@ module CheckoutWorkflow def toggle_details toggle_accordion :details end - - def toggle_billing - toggle_accordion :billing - end - - def toggle_shipping - toggle_accordion :shipping - end - - def toggle_payment - toggle_accordion :payment - end end From 4499c38cfe92f1d3275e707d67c173da6fdc7540 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Fri, 23 Nov 2018 19:14:51 +0000 Subject: [PATCH 16/27] Bump unicorn-rails from 1.1.0 to 2.2.1 Bumps [unicorn-rails](https://github.com/samuelkadolph/unicorn-rails) from 1.1.0 to 2.2.1. - [Release notes](https://github.com/samuelkadolph/unicorn-rails/releases) - [Commits](https://github.com/samuelkadolph/unicorn-rails/compare/v1.1.0...v2.2.1) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 17399ca6b1..7eba5d9ac6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -721,7 +721,7 @@ GEM unicorn (5.4.1) kgio (~> 2.6) raindrops (~> 0.7) - unicorn-rails (1.1.0) + unicorn-rails (2.2.1) rack unicorn uuidtools (2.1.5) From 691de9199fc4edbcb0285c951e182cbc5f868d1a Mon Sep 17 00:00:00 2001 From: haseleyi Date: Fri, 23 Nov 2018 11:22:42 -0800 Subject: [PATCH 17/27] Add missing translation for 'Has Shopfront' in single-enterprise dashboard --- app/helpers/enterprises_helper.rb | 2 +- config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/helpers/enterprises_helper.rb b/app/helpers/enterprises_helper.rb index 8cef0281b1..e1b85fc14b 100644 --- a/app/helpers/enterprises_helper.rb +++ b/app/helpers/enterprises_helper.rb @@ -55,7 +55,7 @@ module EnterprisesHelper if enterprise.sells == 'none' enterprise.producer_profile_only ? I18n.t(:profile) : I18n.t(:supplier_only) else - "Has Shopfront" + I18n.t(:has_shopfront) end end diff --git a/config/locales/en.yml b/config/locales/en.yml index e203fe0039..133557d046 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -225,6 +225,7 @@ en: admin_and_handling: Admin & Handling profile: Profile supplier_only: Supplier Only + has_shopfront: Has Shopfront weight: Weight volume: Volume items: Items From 5c6ec50dea8407ba3cdef80b7670d0d00639cf62 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Mon, 26 Nov 2018 13:51:16 +0800 Subject: [PATCH 18/27] Ignore describe and context in Rubocop BlockLength --- .rubocop_styleguide.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop_styleguide.yml b/.rubocop_styleguide.yml index ddf8e236b8..790240470b 100644 --- a/.rubocop_styleguide.yml +++ b/.rubocop_styleguide.yml @@ -186,6 +186,9 @@ Lint/AssignmentInCondition: Metrics/AbcSize: Max: 15 +Metrics/BlockLength: + ExcludedMethods: ["describe", "context"] + Metrics/BlockNesting: Max: 3 From b365488653715ccc55914eb8fff2bee3528c894d Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Sun, 25 Nov 2018 12:05:34 +0100 Subject: [PATCH 19/27] Enable rack-mini-profiler in development This will run it always in development mode. --- Gemfile | 2 ++ Gemfile.lock | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Gemfile b/Gemfile index 4bc5c01091..7943910142 100644 --- a/Gemfile +++ b/Gemfile @@ -150,4 +150,6 @@ group :development do # While we don't require this gem directly, no dependents forced the upgrade to a version # greater than 1.0.9, so we just required the latest available version here. gem 'eventmachine', '>= 1.2.3' + + gem 'rack-mini-profiler', '< 1.0.0' end diff --git a/Gemfile.lock b/Gemfile.lock index 17399ca6b1..89f22e3c3b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -584,6 +584,8 @@ GEM rack (1.4.7) rack-cache (1.8.0) rack (>= 0.4) + rack-mini-profiler (0.10.7) + rack (>= 1.2.0) rack-rewrite (1.5.1) rack-ssl (1.3.4) rack @@ -814,6 +816,7 @@ DEPENDENCIES poltergeist (>= 1.16.0) pry-byebug (>= 3.4.3) rabl + rack-mini-profiler (< 1.0.0) rack-rewrite rack-ssl rails (~> 3.2.22) From 9efa45663c3f0c14ad1dd7f30182e67afd4dcb43 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 27 Nov 2018 14:29:52 +1100 Subject: [PATCH 20/27] Encode records with special chars as well Using Marshal.dump on the French production database raised an error: Encoding::UndefinedConversionError: "\xC3" from ASCII-8BIT to UTF-8 Replacing Marshal with YAML solves the problem. It is also more reliable and human readable. This code was run against the French, Australian and UK production data successfully. --- db/migrate/20181123012635_associate_customers_to_users.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20181123012635_associate_customers_to_users.rb b/db/migrate/20181123012635_associate_customers_to_users.rb index b42739ff68..95fab93677 100644 --- a/db/migrate/20181123012635_associate_customers_to_users.rb +++ b/db/migrate/20181123012635_associate_customers_to_users.rb @@ -29,11 +29,11 @@ class AssociateCustomersToUsers < ActiveRecord::Migration joins("INNER JOIN spree_users ON customers.email = spree_users.email"). where(user_id: nil).all - File.write(backup_file, Marshal.dump(customers)) + File.write(backup_file, YAML.dump(customers)) end def backed_up_customers - Marshal.load(File.read(backup_file)) + YAML.load(File.read(backup_file)) end def backup_file From c233ea38cfc9ad769fe8184baf97a4174ec6043a Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Tue, 30 Oct 2018 11:27:39 +0100 Subject: [PATCH 21/27] Import Spree views for orders#edit in our codebase and integrate deface modifications --- .../add_distribution_fields.html.haml.deface | 3 - .../_form/hide_form_until_distribution.deface | 2 - .../relabel_update_button.html.haml.deface | 3 - .../replace_variant_label.html.haml.deface | 3 - .../replace_variant_price.html.haml.deface | 3 - .../edit/add_action_dropdown.html.haml.deface | 6 -- .../edit/suppress_errors.html.haml.deface | 6 -- .../spree/admin/orders/_add_product.html.haml | 17 +++++ app/views/spree/admin/orders/_form.html.haml | 68 +++++++++++++++++++ .../spree/admin/orders/_line_item.html.haml | 12 ++++ app/views/spree/admin/orders/edit.html.haml | 8 ++- .../spree/admin/shared/_order_tabs.html.haml | 65 ++++++++++++++++++ 12 files changed, 169 insertions(+), 27 deletions(-) delete mode 100644 app/overrides/spree/admin/orders/_form/add_distribution_fields.html.haml.deface delete mode 100644 app/overrides/spree/admin/orders/_form/hide_form_until_distribution.deface delete mode 100644 app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface delete mode 100644 app/overrides/spree/admin/orders/_line_item/replace_variant_label.html.haml.deface delete mode 100644 app/overrides/spree/admin/orders/_line_item/replace_variant_price.html.haml.deface delete mode 100644 app/overrides/spree/admin/orders/edit/add_action_dropdown.html.haml.deface delete mode 100644 app/overrides/spree/admin/orders/edit/suppress_errors.html.haml.deface create mode 100644 app/views/spree/admin/orders/_add_product.html.haml create mode 100644 app/views/spree/admin/orders/_form.html.haml create mode 100644 app/views/spree/admin/orders/_line_item.html.haml create mode 100644 app/views/spree/admin/shared/_order_tabs.html.haml diff --git a/app/overrides/spree/admin/orders/_form/add_distribution_fields.html.haml.deface b/app/overrides/spree/admin/orders/_form/add_distribution_fields.html.haml.deface deleted file mode 100644 index af07be1d23..0000000000 --- a/app/overrides/spree/admin/orders/_form/add_distribution_fields.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ insert_before "[data-hook='admin_order_form_buttons']" - -= render partial: 'spree/admin/orders/_form/distribution_fields' diff --git a/app/overrides/spree/admin/orders/_form/hide_form_until_distribution.deface b/app/overrides/spree/admin/orders/_form/hide_form_until_distribution.deface deleted file mode 100644 index 7fe5652aae..0000000000 --- a/app/overrides/spree/admin/orders/_form/hide_form_until_distribution.deface +++ /dev/null @@ -1,2 +0,0 @@ -add_to_attributes "table.index, [data-hook='admin_order_form_buttons']" -attributes "ng-show" => "distributionChosen()" diff --git a/app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface b/app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface deleted file mode 100644 index d4d3aefc8d..0000000000 --- a/app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace "code[erb-loud]:contains('button t(:update)')" - -= button t(:update_and_recalculate_fees), 'icon-refresh' diff --git a/app/overrides/spree/admin/orders/_line_item/replace_variant_label.html.haml.deface b/app/overrides/spree/admin/orders/_line_item/replace_variant_label.html.haml.deface deleted file mode 100644 index 6481791112..0000000000 --- a/app/overrides/spree/admin/orders/_line_item/replace_variant_label.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace 'code[erb-loud]:contains(\'"(#{f.object.variant.options_text})"\')' - -= "(#{f.object.full_name})" diff --git a/app/overrides/spree/admin/orders/_line_item/replace_variant_price.html.haml.deface b/app/overrides/spree/admin/orders/_line_item/replace_variant_price.html.haml.deface deleted file mode 100644 index 605f645996..0000000000 --- a/app/overrides/spree/admin/orders/_line_item/replace_variant_price.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace 'code[erb-loud]:contains(\'f.object.variant.display_amount\')' - -= f.object.single_money diff --git a/app/overrides/spree/admin/orders/edit/add_action_dropdown.html.haml.deface b/app/overrides/spree/admin/orders/edit/add_action_dropdown.html.haml.deface deleted file mode 100644 index 58b13a3b32..0000000000 --- a/app/overrides/spree/admin/orders/edit/add_action_dropdown.html.haml.deface +++ /dev/null @@ -1,6 +0,0 @@ -/ insert_after "code[erb-loud]:contains('button_link_to t(:resend)')" - -%li.links-dropdown#links-dropdown{ links: order_links(@order).to_json } - -:coffee - angular.bootstrap(document.getElementById("links-dropdown"),['admin.dropdown']) diff --git a/app/overrides/spree/admin/orders/edit/suppress_errors.html.haml.deface b/app/overrides/spree/admin/orders/edit/suppress_errors.html.haml.deface deleted file mode 100644 index 31a8c961e8..0000000000 --- a/app/overrides/spree/admin/orders/edit/suppress_errors.html.haml.deface +++ /dev/null @@ -1,6 +0,0 @@ -/ replace "code[erb-loud]:contains(\'error_messages\')" - --# Suppress errors when manually creating a new order - needs to proceed to edit page --# without having line items (which otherwise gives a validation error) -- unless params["suppress_error_msg"] - = render partial: "spree/shared/error_messages", :locals => { :target => @order } diff --git a/app/views/spree/admin/orders/_add_product.html.haml b/app/views/spree/admin/orders/_add_product.html.haml new file mode 100644 index 0000000000..ab807961f7 --- /dev/null +++ b/app/views/spree/admin/orders/_add_product.html.haml @@ -0,0 +1,17 @@ += render partial: "spree/admin/variants/autocomplete", formats: :js +#add-line-item{"data-hook" => ""} + %fieldset + %legend{align: "center"} + = t(:add_product) + .field.eight.columns.alpha{"data-hook" => "add_product_name"} + = label_tag :add_product_name, t(:name_or_sku) + = hidden_field_tag :add_variant_id, "", class: "variant_autocomplete fullwidth" + .field.two.columns{"data-hook" => "add_quantity"} + = label_tag :add_quantity, t(:qty) + = number_field_tag :add_quantity, 1, min: 0 + .actions.two.columns.omega{"data-hook" => "add_button"} + = link_to_with_icon 'icon-plus', t(:add), admin_order_line_items_url(@order), + method: :post, + id: 'add_line_item_to_order', + class: 'button fullwidth', + 'data-update' => 'order-form-wrapper' diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml new file mode 100644 index 0000000000..07a41cb7e4 --- /dev/null +++ b/app/views/spree/admin/orders/_form.html.haml @@ -0,0 +1,68 @@ +%div{"data-hook" => "admin_order_form_fields"} + - if @line_item.try(:errors).present? + = render partial: 'spree/shared/error_messages', locals: { target: @line_item } + = form_for @order, url: admin_order_url(@order), method: :put do |f| + %fieldset.no-border-top + = f.hidden_field :number + %table.index{"ng-show" => "distributionChosen()"} + %colgroup + %col{style: "width: 49%;"}/ + %col{style: "width: 14%;"}/ + %col{style: "width: 10%;"}/ + %col{style: "width: 14%;"}/ + %col{style: "width: 8%;"}/ + %thead#line-items + %tr{"data-hook" => "admin_order_form_line_items_headers"} + %th + = t(:item_description) + %th.price + = t(:price) + %th.qty + = t(:qty) + %th.total + %span + = t(:total) + %th.orders-actions.actions{"data-hook" => "admin_order_form_line_items_header_actions"} + %tbody{"data-hook" => "admin_order_form_line_items"} + = f.fields_for :line_items do |li_form| + = render partial: 'spree/admin/orders/line_item', locals: { f: li_form } + %tbody#subtotal.no-border-top{"data-hook" => "admin_order_form_subtotal"} + %tr#subtotal-row + %td{colspan: "3"} + %b + = t(:subtotal) + \: + %td.total.align-center + %span + = @order.display_item_total.to_html + %td.actions + %tbody#order-charges.no-border-top{"data-hook" => "admin_order_form_adjustments"} + - @order.adjustments.eligible.each do |adjustment| + %tr + %td{colspan: "3"} + %strong + = adjustment.label + \: + %td.total.align-center + %span= adjustment.display_amount.to_html + %td.actions + %tbody#order-total.grand-total.no-border-top{"data-hook" => "admin_order_form_total"} + %tr + %td{colspan: "3"} + %b + = t(:order_total) + \: + %td.total.align-center + %span#order_total + = @order.display_total.to_html + %td.actions + + = render partial: 'spree/admin/orders/_form/distribution_fields' + + .filter-actions.actions{"data-hook" => "admin_order_form_buttons", "ng-show" => "distributionChosen()"} + = button t(:update_and_recalculate_fees), 'icon-refresh' + %span.or + = t(:or) + = link_to_with_icon 'button icon-arrow-left', t(:back), admin_orders_url + = javascript_tag do + = render partial: 'spree/admin/shared/update_order_state', handlers: [:js] diff --git a/app/views/spree/admin/orders/_line_item.html.haml b/app/views/spree/admin/orders/_line_item.html.haml new file mode 100644 index 0000000000..4bcc30fae0 --- /dev/null +++ b/app/views/spree/admin/orders/_line_item.html.haml @@ -0,0 +1,12 @@ +%tr{"data-hook" => "admin_order_form_line_item_row", id: "#{spree_dom_id(f.object)}", class: "#{cycle('odd', 'even')}"} + %td + = f.object.variant.product.name + = "(#{f.object.full_name})" unless f.object.variant.option_values.empty? + %td.price.align-center + = f.object.variant.display_amount + %td.qty + = f.number_field :quantity, min: 0, class: "qty" + %td.total.align-center + = f.object.single_money + %td.actions{"data-hook" => "admin_order_form_line_item_actions"} + = link_to_delete f.object, {url: admin_order_line_item_url(@order.number, f.object), no_text: true} diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml index f897065224..82d5507ec1 100644 --- a/app/views/spree/admin/orders/edit.html.haml +++ b/app/views/spree/admin/orders/edit.html.haml @@ -3,6 +3,8 @@ - content_for :page_actions do %li= event_links %li= button_link_to t(:resend), resend_admin_order_url(@order), method: :post, icon: 'icon-email' + %li.links-dropdown#links-dropdown{ links: order_links(@order).to_json } + %li= button_link_to t(:back_to_orders_list), admin_orders_path, icon: 'icon-arrow-left' = admin_inject_shops(module: 'admin.orders') @@ -11,7 +13,8 @@ = render 'spree/admin/shared/order_tabs', current: 'Order Details' %div{"data-hook" => "admin_order_edit_header"} - = render 'spree/shared/error_messages', target: @order + - unless params["suppress_error_msg"] + = render partial: "spree/shared/error_messages", :locals => { :target => @order } %div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl", "ofn-distributor-id" => @order.distributor_id, "ofn-order-cycle-id" => @order.order_cycle_id} = render 'add_product' @@ -22,3 +25,6 @@ - content_for :head do = javascript_tag 'var expand_variants = true;' + +:coffee + angular.bootstrap(document.getElementById("links-dropdown"),['admin.dropdown']) diff --git a/app/views/spree/admin/shared/_order_tabs.html.haml b/app/views/spree/admin/shared/_order_tabs.html.haml new file mode 100644 index 0000000000..a2f346bf27 --- /dev/null +++ b/app/views/spree/admin/shared/_order_tabs.html.haml @@ -0,0 +1,65 @@ +- content_for :page_title do + = t(:order) + \# + = @order.number + +- if @order.bill_address.present? + = @order.bill_address.firstname + = @order.bill_address.lastname + \- + +- content_for :sidebar_title do + = t(:order_information) + +- content_for :sidebar do + %header#order_tab_summary + %dl.additional-info + %dt#order_status + = t(:status) + %dd + %span{:class => "state #{@order.state}"} + = t(@order.state, scope: :order_state) + %dt + = t(:total) + \: + %dd#order_total + = @order.display_total.to_html + + - if @order.completed? + %dt + = t(:shipment) + \: + %dd#shipment_status + %span.state + = @order.shipment_state + = t(@order.shipment_state, scope: :shipment_states, default: [:missing, none]) + %dt + = t(:payment) + \: + %dd#payment_status + %span.state + = @order.payment_state + = t(@order.payment_state, scope: :payment_states, default: [:missing, none]) + %dt + = t(:date_completed) + \: + %dd#date_complete + = pretty_time(@order.completed_at) + + %nav.menu + %ul{"data-hook" => "admin_order_tabs"} + %li{ class: "#{'active' if true}" } + = link_to_with_icon 'icon-edit', t(:order_details), edit_admin_order_url(@order) + %li{"#{'class=active' if current == 'Customer Details'}"} + = link_to_with_icon 'icon-user', t(:customer_details), admin_order_customer_url(@order) + %li{"#{'class=active' if current == 'Adjustments Details'}"} + = link_to_with_icon 'icon-cogs', t(:adjustments), admin_order_adjustments_url(@order) + + %li{"#{'class=active' if current == 'Payments'}"} + = link_to_with_icon 'icon-credit-card', t(:payments), admin_order_payments_url(@order) + + %li{"#{'class=active' if current == 'Shipments'}"} + = link_to_with_icon 'icon-road', t(:shipments), admin_order_shipments_url(@order) + - if @order.completed? + %li{"#{'class=active' if current == 'Return Authorizations'}"} + = link_to_with_icon 'icon-share-alt', t(:return_authorizations), admin_order_return_authorizations_url(@order) From 652191a4c52e5d4b941a951e0b0478c3db4ec8c9 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Tue, 30 Oct 2018 12:11:35 +0100 Subject: [PATCH 22/27] Fixing missing variable none --- app/views/spree/admin/shared/_order_tabs.html.haml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/views/spree/admin/shared/_order_tabs.html.haml b/app/views/spree/admin/shared/_order_tabs.html.haml index a2f346bf27..17ebf435bb 100644 --- a/app/views/spree/admin/shared/_order_tabs.html.haml +++ b/app/views/spree/admin/shared/_order_tabs.html.haml @@ -30,16 +30,14 @@ = t(:shipment) \: %dd#shipment_status - %span.state - = @order.shipment_state - = t(@order.shipment_state, scope: :shipment_states, default: [:missing, none]) + %span{class: "state #{@order.shipment_state}"} + = t(@order.shipment_state, scope: :shipment_states, default: [:missing, "none"]) %dt = t(:payment) \: %dd#payment_status - %span.state - = @order.payment_state - = t(@order.payment_state, scope: :payment_states, default: [:missing, none]) + %span{class: "state #{@order.payment_state}"} + = t(@order.payment_state, scope: :payment_states, default: [:missing, "none"]) %dt = t(:date_completed) \: From 422a68630f1f47ba7a680237b29b80c2b7b361e9 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Tue, 30 Oct 2018 14:03:51 +0100 Subject: [PATCH 23/27] Remove data-hook tags --- .../spree/admin/orders/_add_product.html.haml | 8 ++++---- app/views/spree/admin/orders/_form.html.haml | 16 ++++++++-------- .../spree/admin/orders/_line_item.html.haml | 4 ++-- app/views/spree/admin/orders/edit.html.haml | 6 +++--- .../spree/admin/shared/_order_tabs.html.haml | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/views/spree/admin/orders/_add_product.html.haml b/app/views/spree/admin/orders/_add_product.html.haml index ab807961f7..eecc678521 100644 --- a/app/views/spree/admin/orders/_add_product.html.haml +++ b/app/views/spree/admin/orders/_add_product.html.haml @@ -1,15 +1,15 @@ = render partial: "spree/admin/variants/autocomplete", formats: :js -#add-line-item{"data-hook" => ""} +#add-line-item %fieldset %legend{align: "center"} = t(:add_product) - .field.eight.columns.alpha{"data-hook" => "add_product_name"} + .field.eight.columns.alpha = label_tag :add_product_name, t(:name_or_sku) = hidden_field_tag :add_variant_id, "", class: "variant_autocomplete fullwidth" - .field.two.columns{"data-hook" => "add_quantity"} + .field.two.columns = label_tag :add_quantity, t(:qty) = number_field_tag :add_quantity, 1, min: 0 - .actions.two.columns.omega{"data-hook" => "add_button"} + .actions.two.columns.omega = link_to_with_icon 'icon-plus', t(:add), admin_order_line_items_url(@order), method: :post, id: 'add_line_item_to_order', diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml index 07a41cb7e4..8564ae04f5 100644 --- a/app/views/spree/admin/orders/_form.html.haml +++ b/app/views/spree/admin/orders/_form.html.haml @@ -1,4 +1,4 @@ -%div{"data-hook" => "admin_order_form_fields"} +%div - if @line_item.try(:errors).present? = render partial: 'spree/shared/error_messages', locals: { target: @line_item } = form_for @order, url: admin_order_url(@order), method: :put do |f| @@ -12,7 +12,7 @@ %col{style: "width: 14%;"}/ %col{style: "width: 8%;"}/ %thead#line-items - %tr{"data-hook" => "admin_order_form_line_items_headers"} + %tr %th = t(:item_description) %th.price @@ -22,11 +22,11 @@ %th.total %span = t(:total) - %th.orders-actions.actions{"data-hook" => "admin_order_form_line_items_header_actions"} - %tbody{"data-hook" => "admin_order_form_line_items"} + %th.orders-actions.actions + %tbody = f.fields_for :line_items do |li_form| = render partial: 'spree/admin/orders/line_item', locals: { f: li_form } - %tbody#subtotal.no-border-top{"data-hook" => "admin_order_form_subtotal"} + %tbody#subtotal.no-border-top %tr#subtotal-row %td{colspan: "3"} %b @@ -36,7 +36,7 @@ %span = @order.display_item_total.to_html %td.actions - %tbody#order-charges.no-border-top{"data-hook" => "admin_order_form_adjustments"} + %tbody#order-charges.no-border-top - @order.adjustments.eligible.each do |adjustment| %tr %td{colspan: "3"} @@ -46,7 +46,7 @@ %td.total.align-center %span= adjustment.display_amount.to_html %td.actions - %tbody#order-total.grand-total.no-border-top{"data-hook" => "admin_order_form_total"} + %tbody#order-total.grand-total.no-border-top %tr %td{colspan: "3"} %b @@ -59,7 +59,7 @@ = render partial: 'spree/admin/orders/_form/distribution_fields' - .filter-actions.actions{"data-hook" => "admin_order_form_buttons", "ng-show" => "distributionChosen()"} + .filter-actions.actions{"ng-show" => "distributionChosen()"} = button t(:update_and_recalculate_fees), 'icon-refresh' %span.or = t(:or) diff --git a/app/views/spree/admin/orders/_line_item.html.haml b/app/views/spree/admin/orders/_line_item.html.haml index 4bcc30fae0..9afed0e2ec 100644 --- a/app/views/spree/admin/orders/_line_item.html.haml +++ b/app/views/spree/admin/orders/_line_item.html.haml @@ -1,4 +1,4 @@ -%tr{"data-hook" => "admin_order_form_line_item_row", id: "#{spree_dom_id(f.object)}", class: "#{cycle('odd', 'even')}"} +%tr{id: "#{spree_dom_id(f.object)}", class: "#{cycle('odd', 'even')}"} %td = f.object.variant.product.name = "(#{f.object.full_name})" unless f.object.variant.option_values.empty? @@ -8,5 +8,5 @@ = f.number_field :quantity, min: 0, class: "qty" %td.total.align-center = f.object.single_money - %td.actions{"data-hook" => "admin_order_form_line_item_actions"} + %td.actions = link_to_delete f.object, {url: admin_order_line_item_url(@order.number, f.object), no_text: true} diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml index 82d5507ec1..015e9234e8 100644 --- a/app/views/spree/admin/orders/edit.html.haml +++ b/app/views/spree/admin/orders/edit.html.haml @@ -12,19 +12,19 @@ = render 'spree/admin/shared/order_tabs', current: 'Order Details' -%div{"data-hook" => "admin_order_edit_header"} +%div - unless params["suppress_error_msg"] = render partial: "spree/shared/error_messages", :locals => { :target => @order } %div{"ng-app" => "admin.orders", "ng-controller" => "orderCtrl", "ofn-distributor-id" => @order.distributor_id, "ofn-order-cycle-id" => @order.order_cycle_id} = render 'add_product' - %div{"data-hook" => "admin_order_edit_form"} + %div #order-form-wrapper = render 'form', order: @order - content_for :head do = javascript_tag 'var expand_variants = true;' - + :coffee angular.bootstrap(document.getElementById("links-dropdown"),['admin.dropdown']) diff --git a/app/views/spree/admin/shared/_order_tabs.html.haml b/app/views/spree/admin/shared/_order_tabs.html.haml index 17ebf435bb..af04fe676e 100644 --- a/app/views/spree/admin/shared/_order_tabs.html.haml +++ b/app/views/spree/admin/shared/_order_tabs.html.haml @@ -45,7 +45,7 @@ = pretty_time(@order.completed_at) %nav.menu - %ul{"data-hook" => "admin_order_tabs"} + %ul %li{ class: "#{'active' if true}" } = link_to_with_icon 'icon-edit', t(:order_details), edit_admin_order_url(@order) %li{"#{'class=active' if current == 'Customer Details'}"} From 60214b9a90bc0d0c03f81c8f4c9278e153223e39 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Wed, 31 Oct 2018 10:54:25 +0100 Subject: [PATCH 24/27] Fix failing specs --- app/views/spree/admin/orders/_form.html.haml | 2 +- spec/features/admin/variant_overrides_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml index 8564ae04f5..449a86ecf8 100644 --- a/app/views/spree/admin/orders/_form.html.haml +++ b/app/views/spree/admin/orders/_form.html.haml @@ -1,4 +1,4 @@ -%div +%div.admin_order_form_fields - if @line_item.try(:errors).present? = render partial: 'spree/shared/error_messages', locals: { target: @line_item } = form_for @order, url: admin_order_url(@order), method: :put do |f| diff --git a/spec/features/admin/variant_overrides_spec.rb b/spec/features/admin/variant_overrides_spec.rb index 853104711c..60130ddc6d 100644 --- a/spec/features/admin/variant_overrides_spec.rb +++ b/spec/features/admin/variant_overrides_spec.rb @@ -16,10 +16,10 @@ feature %q{ let!(:producer_related) { create(:supplier_enterprise) } let!(:producer_unrelated) { create(:supplier_enterprise) } let!(:er1) { create(:enterprise_relationship, parent: producer, child: hub, - permissions_list: [:create_variant_overrides]) + permissions_list: [:create_variant_overrides]) } let!(:er2) { create(:enterprise_relationship, parent: producer_related, child: hub, - permissions_list: [:create_variant_overrides]) + permissions_list: [:create_variant_overrides]) } context "as an enterprise user" do @@ -28,7 +28,7 @@ feature %q{ describe "selecting a hub" do let!(:er1) { create(:enterprise_relationship, parent: hub2, child: producer_managed, - permissions_list: [:add_to_order_cycle]) + permissions_list: [:add_to_order_cycle]) } # This er should not confer ability to create VOs for hub2 it "displays a list of hub choices (ie. only those managed by the user)" do @@ -338,7 +338,7 @@ feature %q{ it "shows the overridden price" do targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' click_link 'Add' - expect(page).to have_selector("table.index tbody[data-hook='admin_order_form_line_items'] tr") # Wait for JS + expect(page).to have_selector("table.index tbody tr") # Wait for JS expect(page).to have_content(product.variants.first.variant_overrides.first.price) end end From 65dd3eb5b9eb4ad7d7553c55fbe8520bbb01755f Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Mon, 5 Nov 2018 18:35:07 +0100 Subject: [PATCH 25/27] Change line items controller HTML response spec to look for form order_edit class --- app/views/spree/admin/orders/_form.html.haml | 2 +- spec/controllers/spree/admin/line_items_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml index 449a86ecf8..8564ae04f5 100644 --- a/app/views/spree/admin/orders/_form.html.haml +++ b/app/views/spree/admin/orders/_form.html.haml @@ -1,4 +1,4 @@ -%div.admin_order_form_fields +%div - if @line_item.try(:errors).present? = render partial: 'spree/shared/error_messages', locals: { target: @line_item } = form_for @order, url: admin_order_url(@order), method: :put do |f| diff --git a/spec/controllers/spree/admin/line_items_controller_spec.rb b/spec/controllers/spree/admin/line_items_controller_spec.rb index 5b7575e7e5..874600c321 100644 --- a/spec/controllers/spree/admin/line_items_controller_spec.rb +++ b/spec/controllers/spree/admin/line_items_controller_spec.rb @@ -90,7 +90,7 @@ describe Spree::Admin::LineItemsController, type: :controller do it 'returns an HTML response with the order form' do spree_put :update, params - expect(response.body).to match(/admin_order_form_fields/) + expect(response.body).to match(/edit_order/) end end end From 69e186f482ebe0ccc13a2d3ba1c94cfd67b7c180 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Tue, 20 Nov 2018 10:13:18 +0100 Subject: [PATCH 26/27] Define dynamic classes in variables as a turnaround to HAML bug --- .../spree/admin/shared/_order_tabs.html.haml | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/app/views/spree/admin/shared/_order_tabs.html.haml b/app/views/spree/admin/shared/_order_tabs.html.haml index af04fe676e..d250507193 100644 --- a/app/views/spree/admin/shared/_order_tabs.html.haml +++ b/app/views/spree/admin/shared/_order_tabs.html.haml @@ -17,8 +17,9 @@ %dt#order_status = t(:status) %dd - %span{:class => "state #{@order.state}"} - = t(@order.state, scope: :order_state) + - order_state_classes = "state #{@order.state}" + %span{ class: order_state_classes } + = t(@order.state, scope: :order_state) %dt = t(:total) \: @@ -30,13 +31,15 @@ = t(:shipment) \: %dd#shipment_status - %span{class: "state #{@order.shipment_state}"} + - shipment_state_classes = "state #{@order.shipment_state}" + %span{ class: shipment_state_classes } = t(@order.shipment_state, scope: :shipment_states, default: [:missing, "none"]) %dt = t(:payment) \: %dd#payment_status - %span{class: "state #{@order.payment_state}"} + - payment_state_classes = "state #{@order.payment_state}" + %span{ class: payment_state_classes } = t(@order.payment_state, scope: :payment_states, default: [:missing, "none"]) %dt = t(:date_completed) @@ -46,18 +49,27 @@ %nav.menu %ul - %li{ class: "#{'active' if true}" } + - order_details_classes = "active" if current == "Order Details" + %li{ class: order_details_classes } = link_to_with_icon 'icon-edit', t(:order_details), edit_admin_order_url(@order) - %li{"#{'class=active' if current == 'Customer Details'}"} + + - customer_details_classes = "active" if current == "Customer Details" + %li{ class: customer_details_classes } = link_to_with_icon 'icon-user', t(:customer_details), admin_order_customer_url(@order) - %li{"#{'class=active' if current == 'Adjustments Details'}"} + + - adjustments_classes = "active" if current == "Adjustments" + %li{ class: adjustments_classes } = link_to_with_icon 'icon-cogs', t(:adjustments), admin_order_adjustments_url(@order) - %li{"#{'class=active' if current == 'Payments'}"} + - payments_classes = "active" if current == "Payments" + %li{ class: payments_classes } = link_to_with_icon 'icon-credit-card', t(:payments), admin_order_payments_url(@order) - %li{"#{'class=active' if current == 'Shipments'}"} + - shipments_classes = "active" if current == "Shipments" + %li{ class: shipments_classes } = link_to_with_icon 'icon-road', t(:shipments), admin_order_shipments_url(@order) + - if @order.completed? - %li{"#{'class=active' if current == 'Return Authorizations'}"} + - authorizations_classes = "active" if current == "Return Authorizations" + %li{ class: authorizations_classes } = link_to_with_icon 'icon-share-alt', t(:return_authorizations), admin_order_return_authorizations_url(@order) From 285346fcdac71c32566449a3ab1008af02670ce9 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 29 Nov 2018 10:25:44 +1100 Subject: [PATCH 27/27] Update schema for latest migration I feel embarrased that this is the second follow up of my last migration: https://github.com/openfoodfoundation/openfoodnetwork/pull/3126 The last migration didn't change any database structure, but the schema still needs the latest migration version. Otherwise it will display pending migrations when setting up the database. This commit allows to run `bundle exec rake db:reset` in development without the following message: Run `rake db:migrate` to update your database then try again. You have 1 pending migrations: 20181123012635 AssociateCustomersToUsers The next pull request with a migration would have solved this problem as well. --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 49dae1fa6e..7f3b49ca3c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20181106162211) do +ActiveRecord::Schema.define(:version => 20181123012635) do create_table "account_invoices", :force => true do |t| t.integer "user_id", :null => false