From 004548a4fe514246679842c65c1778aec473447a Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 18 Jun 2014 10:21:21 +1000 Subject: [PATCH] Rearrange shipping method edit page --- .../shipping_methods_controller_decorator.rb | 2 +- app/models/spree/shipping_method_decorator.rb | 2 +- .../add_require_ship_address.html.haml.deface | 5 -- .../remove_form_availability_fields.deface | 1 + .../remove_form_calculator_fields.deface | 1 + .../replace_form_fields.html.haml.deface | 57 +++++++++++++++++++ .../admin/shared/_hubs_sidebar.html.haml | 2 +- spec/features/admin/shipping_methods_spec.rb | 32 +++++++---- 8 files changed, 84 insertions(+), 18 deletions(-) delete mode 100644 app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface create mode 100644 app/overrides/spree/admin/shipping_methods/_form/remove_form_availability_fields.deface create mode 100644 app/overrides/spree/admin/shipping_methods/_form/remove_form_calculator_fields.deface create mode 100644 app/overrides/spree/admin/shipping_methods/_form/replace_form_fields.html.haml.deface diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index d6c64425ef4..192d0442984 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -2,7 +2,7 @@ module Spree module Admin ShippingMethodsController.class_eval do before_filter :do_not_destroy_referenced_shipping_methods, :only => :destroy - before_filter :load_hubs, only: [:new, :edit] + before_filter :load_hubs, only: [:new, :edit, :create, :update] # Sort shipping methods by distributor name # ! Code copied from Spree::Admin::ResourceController with two added lines diff --git a/app/models/spree/shipping_method_decorator.rb b/app/models/spree/shipping_method_decorator.rb index 05a975ecd18..303ab7b96f2 100644 --- a/app/models/spree/shipping_method_decorator.rb +++ b/app/models/spree/shipping_method_decorator.rb @@ -1,6 +1,6 @@ Spree::ShippingMethod.class_eval do has_and_belongs_to_many :distributors, join_table: 'distributors_shipping_methods', :class_name => 'Enterprise', association_foreign_key: 'distributor_id' - attr_accessible :distributor_ids + attr_accessible :distributor_ids, :description attr_accessible :require_ship_address scope :managed_by, lambda { |user| diff --git a/app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface b/app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface deleted file mode 100644 index b7bffd4c840..00000000000 --- a/app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface +++ /dev/null @@ -1,5 +0,0 @@ -/ insert_bottom "[data-hook='admin_shipping_method_form_availability_fields'] > fieldset" - -= f.field_container :shipping_requirements do - = f.label :require_ship_address, "Requires shipping address?" - = f.check_box :require_ship_address diff --git a/app/overrides/spree/admin/shipping_methods/_form/remove_form_availability_fields.deface b/app/overrides/spree/admin/shipping_methods/_form/remove_form_availability_fields.deface new file mode 100644 index 00000000000..7ab45439c40 --- /dev/null +++ b/app/overrides/spree/admin/shipping_methods/_form/remove_form_availability_fields.deface @@ -0,0 +1 @@ +remove "div[data-hook='admin_shipping_method_form_availability_fields']" \ No newline at end of file diff --git a/app/overrides/spree/admin/shipping_methods/_form/remove_form_calculator_fields.deface b/app/overrides/spree/admin/shipping_methods/_form/remove_form_calculator_fields.deface new file mode 100644 index 00000000000..5063888fed3 --- /dev/null +++ b/app/overrides/spree/admin/shipping_methods/_form/remove_form_calculator_fields.deface @@ -0,0 +1 @@ +remove "div[data-hook='admin_shipping_method_form_calculator_fields']" \ No newline at end of file diff --git a/app/overrides/spree/admin/shipping_methods/_form/replace_form_fields.html.haml.deface b/app/overrides/spree/admin/shipping_methods/_form/replace_form_fields.html.haml.deface new file mode 100644 index 00000000000..4dcc3519810 --- /dev/null +++ b/app/overrides/spree/admin/shipping_methods/_form/replace_form_fields.html.haml.deface @@ -0,0 +1,57 @@ +/ replace "div[data-hook='admin_shipping_method_form_fields']" + +.alpha.twelve.columns{"data-hook" => "admin_shipping_method_form_fields"} + .row + .alpha.three.columns + = f.label :name, t(:name) + .omega.eight.columns + = f.text_field :name, :class => 'fullwidth', placeholder: "eg. 'Pick-up from Primary School'" + = error_message_on :shipping_method, :name + .row + .alpha.three.columns + = f.label :description, t(:description) + .omega.eight.columns + = f.text_area :description, class: 'fullwidth', rows: 2, placeholder: "eg. 'Please collect your order from 123 Imaginary St, Northcote, 3070'" + = error_message_on :shipping_method, :description + - if @available_zones.length == 1 + = f.hidden_field :zone_id, value: @available_zones.first.id + - else + .row + .alpha.three.columns + = f.label :zone_id, t(:zone) + .omega.eight.columns + = f.collection_select(:zone_id, @available_zones, :id, :name, {}, {:class => 'select2 fullwidth'}) + = error_message_on :shipping_method, :zone_id + - if spree_current_user.admin? + .row + .alpha.three.columns + = f.label :display_on, t(:display) + .omega.eight.columns + = select(:shipping_method, :display_on, Spree::ShippingMethod::DISPLAY.collect { |display| [t(display), display == :both ? nil : display.to_s] }, {}, {:class => 'select2 fullwidth'}) + = error_message_on :shipping_method, :display_on + .row + -# Shipping Category used to be a select field, but we cut it down to two options to simplify the interface and because we thought 'Collection' and + -# 'Delivery' pretty much covered it. If we need more categories in the future, suggest reimplementing the select box from spree's code. + .three.columns.alpha + %label Category + = f.hidden_field :shipping_category_id, value: nil + - Spree::ShippingCategory.where(name: ['Delivery', 'Collection']).limit(2).each do |shipping_category| + .two.columns + = f.radio_button :shipping_category_id, shipping_category.id +   + = f.label "shipping_category_id_#{shipping_category.id}", t(shipping_category.name) + .row + .alpha.three.columns + = f.label :require_ship_address, "Requires shipping address?" + .two.columns + = f.radio_button :require_ship_address, true +   + = f.label :yes, t(:yes) + .six.columns + = f.radio_button :require_ship_address, false +   + = f.label :no, t(:no) + + .row + .alpha.eleven.columns + = render :partial => 'spree/admin/shared/calculator_fields', :locals => { :f => f } \ No newline at end of file diff --git a/app/views/spree/admin/shared/_hubs_sidebar.html.haml b/app/views/spree/admin/shared/_hubs_sidebar.html.haml index ea32f4d7840..8998e6d5930 100644 --- a/app/views/spree/admin/shared/_hubs_sidebar.html.haml +++ b/app/views/spree/admin/shared/_hubs_sidebar.html.haml @@ -1,5 +1,5 @@ - hubs_color = @hubs.count > 0 ? "blue" : "red" -.sidebar_item.four.columns#hubs +.sidebar_item.omega.four.columns#hubs .four.columns.alpha.header{ class: "#{hubs_color}" } %span.four.columns.alpha.centered Distributors .four.columns.alpha.list{ class: "#{hubs_color}" } diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index 00573761488..94efc2aacfd 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -17,9 +17,19 @@ # Given some distributors d1 = create(:distributor_enterprise, name: 'Aeronautical Adventures') d2 = create(:distributor_enterprise, name: 'Nautical Travels') + sc1 = create(:shipping_category, name: 'Delivery') + sc2 = create(:shipping_category, name: 'Collection') - # When I create a shipping method and set the distributors + # Shows appropriate fields when logged in as admin visit spree.new_admin_shipping_method_path + page.should have_field 'shipping_method_name' + page.should have_field 'shipping_method_description' + page.should have_select 'shipping_method_display_on' + page.should have_field "shipping_method_shipping_category_id_#{sc1.id}" + page.should have_field "shipping_method_shipping_category_id_#{sc2.id}" + page.should have_field 'shipping_method_require_ship_address_true', checked: true + + # When I create a shipping method and set the distributors fill_in 'shipping_method_name', with: 'Carrier Pidgeon' check "shipping_method_distributor_ids_#{d1.id}" check "shipping_method_distributor_ids_#{d2.id}" @@ -70,19 +80,21 @@ login_to_admin_as enterprise_user end - it "lets me choose whether a shipping address is required" do - click_link "Enterprises" - within(".enterprise-#{distributor1.id}") { click_link 'Shipping Methods' } - click_link 'New Shipping Method' - - page.should have_content "Requires shipping address?" - end - - it "creates shipping methods" do + it "creating a shipping method" do + sc1 = create(:shipping_category, name: 'Delivery') + sc2 = create(:shipping_category, name: 'Collection') click_link 'Enterprises' within(".enterprise-#{distributor1.id}") { click_link 'Shipping Methods' } click_link 'New Shipping Method' + # Show the correct fields + page.should have_field 'shipping_method_name' + page.should have_field 'shipping_method_description' + page.should_not have_select 'shipping_method_display_on' + page.should have_field "shipping_method_shipping_category_id_#{sc1.id}" + page.should have_field "shipping_method_shipping_category_id_#{sc2.id}" + page.should have_field 'shipping_method_require_ship_address_true', checked: true + fill_in 'shipping_method_name', :with => 'Teleport' check "shipping_method_distributor_ids_#{distributor1.id}"