From cbfb896ca41b0e9b101f1d9edc173c287db3098d Mon Sep 17 00:00:00 2001 From: Rob H Date: Sat, 17 Nov 2012 11:46:43 +1100 Subject: [PATCH 01/16] Change 'Delivery Fees' to 'Distribution Costs' in cart --- app/views/spree/orders/_distributor_fees.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/orders/_distributor_fees.html.haml b/app/views/spree/orders/_distributor_fees.html.haml index 1b3633eb6a..a6b43a41d6 100644 --- a/app/views/spree/orders/_distributor_fees.html.haml +++ b/app/views/spree/orders/_distributor_fees.html.haml @@ -1,6 +1,6 @@ #delivery-fees = cms_page_content(:content, Cms::Page.find_by_full_path('/cart-delivery-fees')) - %h2 Delivery Fees + %h2 Distribution Costs %table#delivery %thead From b34d9da8e5343e967cb9a29f8dcf9b243491a2e3 Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 21 Nov 2012 16:00:51 +1100 Subject: [PATCH 02/16] Add cms field to cart page to explain cart, located at cms path: /cart --- .rvmrc | 2 +- app/overrides/add_cms_checkout_distribution.rb | 2 +- app/overrides/add_cms_to_cart.rb | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 app/overrides/add_cms_to_cart.rb diff --git a/.rvmrc b/.rvmrc index b8cc2e1574..93d9b6d38a 100644 --- a/.rvmrc +++ b/.rvmrc @@ -1 +1 @@ -rvm ruby-1.9.3-p125-perf@eaterprises --create +rvm ruby-1.9.3-p194@eaterprises --create diff --git a/app/overrides/add_cms_checkout_distribution.rb b/app/overrides/add_cms_checkout_distribution.rb index 28e45b6290..e6cf1774dd 100644 --- a/app/overrides/add_cms_checkout_distribution.rb +++ b/app/overrides/add_cms_checkout_distribution.rb @@ -1,4 +1,4 @@ Deface::Override.new(:virtual_path => "spree/checkout/_delivery", :insert_before => "fieldset#shipping_method", :text => "<%= cms_page_content(:content, Cms::Page.find_by_full_path('/delivery')) %>", - :name => "process_my_order_button") \ No newline at end of file + :name => "cms_checkout_distribution") \ No newline at end of file diff --git a/app/overrides/add_cms_to_cart.rb b/app/overrides/add_cms_to_cart.rb new file mode 100644 index 0000000000..3b3b79897c --- /dev/null +++ b/app/overrides/add_cms_to_cart.rb @@ -0,0 +1,4 @@ +Deface::Override.new(:virtual_path => "spree/orders/edit", + :insert_after => "h1", + :text => "<%= cms_page_content(:content, Cms::Page.find_by_full_path('/cart')) %>", + :name => "cms_to_cart") \ No newline at end of file From 2786791c39bbc07ccf9d96f99ee9cb2c26a958e8 Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 21 Nov 2012 16:14:56 +1100 Subject: [PATCH 03/16] Change location of checkout delivery page cms from /delivery to /distribution --- app/overrides/add_cms_checkout_distribution.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/overrides/add_cms_checkout_distribution.rb b/app/overrides/add_cms_checkout_distribution.rb index e6cf1774dd..7e58596461 100644 --- a/app/overrides/add_cms_checkout_distribution.rb +++ b/app/overrides/add_cms_checkout_distribution.rb @@ -1,4 +1,4 @@ Deface::Override.new(:virtual_path => "spree/checkout/_delivery", :insert_before => "fieldset#shipping_method", - :text => "<%= cms_page_content(:content, Cms::Page.find_by_full_path('/delivery')) %>", + :text => "<%= cms_page_content(:content, Cms::Page.find_by_full_path('/distribution')) %>", :name => "cms_checkout_distribution") \ No newline at end of file From 3d025bb709bf9f91a8eaf8f950d79312970a0c0c Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 23 Nov 2012 09:26:27 +1100 Subject: [PATCH 04/16] WIP Replace order information display for Confirm and Complete stages of checkout --- .../replace_order_details_steps_data.rb | 4 ++ .../shared/_order_details_steps_data.html.erb | 52 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 app/overrides/replace_order_details_steps_data.rb create mode 100644 app/views/spree/shared/_order_details_steps_data.html.erb diff --git a/app/overrides/replace_order_details_steps_data.rb b/app/overrides/replace_order_details_steps_data.rb new file mode 100644 index 0000000000..10b307cd1c --- /dev/null +++ b/app/overrides/replace_order_details_steps_data.rb @@ -0,0 +1,4 @@ +Deface::Override.new(:virtual_path => "spree/shared/_order_details", + :replace => "div.row.steps-data", + :partial => "spree/shared/order_details_steps_data", + :name => "order_details_steps_data") \ No newline at end of file diff --git a/app/views/spree/shared/_order_details_steps_data.html.erb b/app/views/spree/shared/_order_details_steps_data.html.erb new file mode 100644 index 0000000000..04e8d4d65a --- /dev/null +++ b/app/views/spree/shared/_order_details_steps_data.html.erb @@ -0,0 +1,52 @@ +
+ +
+
<%= "Customer Details" %> <%= link_to "(#{t(:edit)})", checkout_state_path(:address) unless @order.completed? %>
+
+ Name: <%= order.bill_address.full_name %>
+ Address: <%= order.bill_address.address1 + ", " + order.bill_address.city %> +
+
+ +
+
<%= "Distributor Details" %> <%=# link_to "(#{t(:edit)})", checkout_state_path(:address) unless @order.completed? %>
+
+ Distributor: <%= order.distributor.name %>
+ Address: <%= order.distributor.address.address1 + ", " + order.distributor.address.city %> +
+
+ + +
+
+
+
<%= t(:payment_information) %> <%= link_to "(#{t(:edit)})", checkout_state_path(:payment) unless @order.completed? %>
+
+ <% if order.payment_method.name.include? "PayPal" %> +
Your payment via Paypal has been processed successfully.
+ <% elsif order.payment_method.name.include? "EFT" %> + Please pay by direct debit:

+ Eaterprises Australia
+ BSB: XXX-XXX
+ Acct: XXXXXXX + <% elsif order.creditcards.empty? == false %> + + <%= image_tag "creditcards/icons/#{order.creditcards.first.cc_type}.png" %> + <%= t(:ending_in)%> <%= order.creditcards.first.last_digits %> + +
+ + <%= order.creditcards.first.first_name %> + <%= order.creditcards.first.last_name %> + + <% elsif order.payment_method.type == "Spree::PaymentMethod::Check" %> + <%= order.payment_method.description %> + <% end %> +
+
+
\ No newline at end of file From ee482d520c9d0f4215516869e0fa3339f8648598 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 23 Nov 2012 10:51:14 +1100 Subject: [PATCH 05/16] Replace order information display for Confirm and Complete stages of checkout --- app/assets/stylesheets/store/openfoodweb.css.scss | 5 +++++ .../spree/shared/_order_details_steps_data.html.erb | 11 ++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/store/openfoodweb.css.scss b/app/assets/stylesheets/store/openfoodweb.css.scss index 24c0b1fd5a..5af6ca1b33 100644 --- a/app/assets/stylesheets/store/openfoodweb.css.scss +++ b/app/assets/stylesheets/store/openfoodweb.css.scss @@ -225,3 +225,8 @@ fieldset#product-distributor-details { } } } + +/* Alert for EFT Payment during checkout process */ +div#eft-payment-alert { + border: 2px solid red; +} diff --git a/app/views/spree/shared/_order_details_steps_data.html.erb b/app/views/spree/shared/_order_details_steps_data.html.erb index 04e8d4d65a..96214287f8 100644 --- a/app/views/spree/shared/_order_details_steps_data.html.erb +++ b/app/views/spree/shared/_order_details_steps_data.html.erb @@ -9,7 +9,7 @@
-
<%= "Distributor Details" %> <%=# link_to "(#{t(:edit)})", checkout_state_path(:address) unless @order.completed? %>
+
<%= "Distributor Details" %> <%# link_to "(#{t(:edit)})", checkout_state_path(:address) unless @order.completed? %>
Distributor: <%= order.distributor.name %>
Address: <%= order.distributor.address.address1 + ", " + order.distributor.address.city %> @@ -24,16 +24,13 @@
-->
-
+
id="eft-payment-alert"<% end %>>
<%= t(:payment_information) %> <%= link_to "(#{t(:edit)})", checkout_state_path(:payment) unless @order.completed? %>
<% if order.payment_method.name.include? "PayPal" %> -
Your payment via Paypal has been processed successfully.
+
Your payment via PayPal has been processed successfully.
<% elsif order.payment_method.name.include? "EFT" %> - Please pay by direct debit:

- Eaterprises Australia
- BSB: XXX-XXX
- Acct: XXXXXXX + <%= order.payment_method.description.html_safe %> <% elsif order.creditcards.empty? == false %> <%= image_tag "creditcards/icons/#{order.creditcards.first.cc_type}.png" %> From 43248aee9957000226ec375989af3dc467d20a71 Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 28 Nov 2012 09:17:05 +1100 Subject: [PATCH 06/16] Add can_change_distributor validation to the Order model --- app/models/spree/order_decorator.rb | 2 + db/schema.rb | 148 ++++++++++++++-------------- 2 files changed, 76 insertions(+), 74 deletions(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 449640a4b6..36f552c0df 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -2,6 +2,8 @@ Spree::Order.class_eval do belongs_to :distributor, :class_name => 'Enterprise' before_validation :shipping_address_from_distributor + validate :can_change_distributor?, :if => :distributor_id_changed? + after_create :set_default_shipping_method def can_change_distributor? diff --git a/db/schema.rb b/db/schema.rb index 4bbccb5c05..9f744d4956 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -186,8 +186,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "alternative_phone" t.integer "state_id" t.integer "country_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "company" end @@ -196,12 +196,12 @@ ActiveRecord::Schema.define(:version => 20121031222403) do create_table "spree_adjustments", :force => true do |t| t.integer "source_id" - t.decimal "amount", :precision => 8, :scale => 2, :default => 0.0 + t.decimal "amount", :precision => 8, :scale => 2 t.string "label" t.string "source_type" t.integer "adjustable_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "mandatory" t.boolean "locked" t.integer "originator_id" @@ -233,15 +233,15 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "type" t.integer "calculable_id", :null => false t.string "calculable_type", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_configurations", :force => true do |t| t.string "name" t.string "type", :limit => 50 - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "spree_configurations", ["name", "type"], :name => "index_configurations_on_name_and_type" @@ -279,8 +279,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "environment", :default => "development" t.string "server", :default => "test" t.boolean "test_mode", :default => true - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_inventory_units", :force => true do |t| @@ -288,8 +288,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "state" t.integer "variant_id" t.integer "order_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "shipment_id" t.integer "return_authorization_id" end @@ -303,8 +303,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "variant_id" t.integer "quantity", :null => false t.decimal "price", :precision => 8, :scale => 2, :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "max_quantity" t.integer "shipping_method_id" end @@ -316,22 +316,22 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "source_id" t.string "source_type" t.text "details" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_mail_methods", :force => true do |t| t.string "environment" t.boolean "active", :default => true - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_option_types", :force => true do |t| t.string "name", :limit => 100 t.string "presentation", :limit => 100 - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "position", :default => 0, :null => false end @@ -345,8 +345,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "name" t.string "presentation" t.integer "option_type_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_option_values_variants", :id => false, :force => true do |t| @@ -365,8 +365,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.decimal "adjustment_total", :precision => 8, :scale => 2, :default => 0.0, :null => false t.decimal "credit_total", :precision => 8, :scale => 2, :default => 0.0, :null => false t.integer "user_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.datetime "completed_at" t.integer "bill_address_id" t.integer "ship_address_id" @@ -387,8 +387,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.text "description" t.boolean "active", :default => true t.string "environment", :default => "development" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.datetime "deleted_at" t.string "display_on" end @@ -396,8 +396,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do create_table "spree_payments", :force => true do |t| t.decimal "amount", :precision => 8, :scale => 2, :default => 0.0, :null => false t.integer "order_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "source_id" t.string "source_type" t.integer "payment_method_id" @@ -426,8 +426,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "owner_id" t.string "owner_type" t.text "value" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "key" t.string "value_type" end @@ -452,16 +452,16 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "position" t.integer "product_id" t.integer "option_type_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_product_properties", :force => true do |t| t.string "value" t.integer "product_id" t.integer "property_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "spree_product_properties", ["product_id"], :name => "index_product_properties_on_product_id" @@ -485,8 +485,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "meta_keywords" t.integer "tax_category_id" t.integer "shipping_category_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "count_on_hand", :default => 0, :null => false t.integer "supplier_id" t.boolean "group_buy" @@ -531,8 +531,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "user_id" t.integer "product_group_id" t.string "type" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "spree_promotion_rules", ["product_group_id"], :name => "index_promotion_rules_on_product_group_id" @@ -549,8 +549,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do create_table "spree_properties", :force => true do |t| t.string "name" t.string "presentation", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_properties_prototypes", :id => false, :force => true do |t| @@ -560,8 +560,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do create_table "spree_prototypes", :force => true do |t| t.string "name" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_return_authorizations", :force => true do |t| @@ -570,8 +570,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.decimal "amount", :precision => 8, :scale => 2, :default => 0.0, :null => false t.integer "order_id" t.text "reason" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_roles", :force => true do |t| @@ -594,8 +594,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "order_id" t.integer "shipping_method_id" t.integer "address_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "state" end @@ -603,15 +603,15 @@ ActiveRecord::Schema.define(:version => 20121031222403) do create_table "spree_shipping_categories", :force => true do |t| t.string "name" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_shipping_methods", :force => true do |t| t.string "name" t.integer "zone_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "display_on" t.integer "shipping_category_id" t.boolean "match_none" @@ -627,8 +627,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "transaction_id" t.integer "customer_id" t.string "payment_type" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_state_changes", :force => true do |t| @@ -636,8 +636,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "previous_state" t.integer "stateful_id" t.integer "user_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "stateful_type" t.string "next_state" end @@ -651,8 +651,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do create_table "spree_tax_categories", :force => true do |t| t.string "name" t.string "description" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "is_default", :default => false t.datetime "deleted_at" end @@ -661,15 +661,15 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.decimal "amount", :precision => 8, :scale => 5 t.integer "zone_id" t.integer "tax_category_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "included_in_price", :default => false end create_table "spree_taxonomies", :force => true do |t| t.string "name", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_taxons", :force => true do |t| @@ -678,8 +678,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "name", :null => false t.string "permalink" t.integer "taxonomy_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "lft" t.integer "rgt" t.string "icon_file_name" @@ -697,8 +697,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "permissable_id" t.string "permissable_type" t.string "token" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "spree_tokenized_permissions", ["permissable_id", "permissable_type"], :name => "index_tokenized_name_and_type" @@ -707,8 +707,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "environment" t.string "analytics_id" t.boolean "active", :default => true - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_users", :force => true do |t| @@ -729,8 +729,8 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.string "login" t.integer "ship_address_id" t.integer "bill_address_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "authentication_token" t.string "unlock_token" t.datetime "locked_at" @@ -753,7 +753,7 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.boolean "is_master", :default => false t.integer "product_id" t.integer "count_on_hand", :default => 0, :null => false - t.decimal "cost_price", :precision => 8, :scale => 2, :default => 0.0 + t.decimal "cost_price", :precision => 8, :scale => 2 t.integer "position" end @@ -763,15 +763,15 @@ ActiveRecord::Schema.define(:version => 20121031222403) do t.integer "zoneable_id" t.string "zoneable_type" t.integer "zone_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "spree_zones", :force => true do |t| t.string "name" t.string "description" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "default_tax", :default => false t.integer "zone_members_count", :default => 0 end From ef2216834931a98d13f587ddb3fc36ee9c47186f Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 28 Nov 2012 10:54:11 +1100 Subject: [PATCH 07/16] Move select_distributor and deselect_distributor actions from enterprises controller to orders controller --- app/controllers/enterprises_controller.rb | 24 ------------------- .../spree/orders_controller_decorator.rb | 23 ++++++++++++++++++ .../spree/products/_source_sidebar.html.haml | 4 ++-- config/routes.rb | 7 ++++-- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index 1b05eaf1c3..1290530ddd 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -26,28 +26,4 @@ class EnterprisesController < BaseController @searcher = Spree::Config.searcher_class.new(options) @products = @searcher.retrieve_products end - - def select_distributor - distributor = Enterprise.is_distributor.find params[:id] - - order = current_order(true) - - if order.can_change_distributor? - order.distributor = distributor - order.save! - end - - redirect_to distributor - end - - def deselect_distributor - order = current_order(true) - - if order.can_change_distributor? - order.distributor = nil - order.save! - end - - redirect_to root_path - end end diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 90b851fbaf..798eedd60a 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -30,6 +30,29 @@ Spree::OrdersController.class_eval do end end + def select_distributor + distributor = Enterprise.is_distributor.find params[:id] + + order = current_order(true) + + if order.can_change_distributor? + order.distributor = distributor + order.save! + end + + redirect_to main_app.enterprise_path(distributor) + end + + def deselect_distributor + order = current_order(true) + + if order.can_change_distributor? + order.distributor = nil + order.save! + end + + redirect_to root_path + end private diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index bd25db53a0..41da860f88 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -12,10 +12,10 @@ - @distributors.each do |distributor| %li.nowrap - if order.nil? || order.can_change_distributor? - = link_to distributor.name, main_app.select_distributor_enterprise_path(distributor) + = link_to distributor.name, select_distributor_order_path(distributor) - elsif order.distributor == distributor = link_to distributor.name, [main_app, distributor] - else %span.inactive= distributor.name - if current_distributor && order.can_change_distributor? - = button_to 'Browse All Distributors', main_app.deselect_distributor_enterprises_path, :method => :get + = button_to 'Browse All Distributors', deselect_distributor_orders_path, :method => :get diff --git a/config/routes.rb b/config/routes.rb index 60454a3ae1..1c7abf560c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,8 +4,6 @@ Openfoodweb::Application.routes.draw do resources :enterprises do get :suppliers, :on => :collection get :distributors, :on => :collection - get :select_distributor, :on => :member - get :deselect_distributor, :on => :collection end namespace :admin do @@ -25,4 +23,9 @@ Spree::Core::Engine.routes.prepend do match '/admin/reports/bulk_coop' => 'admin/reports#bulk_coop', :as => "bulk_coop_admin_reports", :via => [:get, :post] match '/admin/reports/payments' => 'admin/reports#payments', :as => "payments_admin_reports", :via => [:get, :post] match '/admin/reports/order_cycles' => 'admin/reports#order_cycles', :as => "order_cycles_admin_reports", :via => [:get, :post] + + resources :orders do + get :select_distributor, :on => :member + get :deselect_distributor, :on => :collection + end end From 46abc2fc42771aedbe4d04d5aac9169aee6ab919 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 7 Dec 2012 11:56:40 +1100 Subject: [PATCH 08/16] Add ability to change distributor based on whether products in the cart are available --- .../spree/orders_controller_decorator.rb | 2 +- app/models/spree/order_decorator.rb | 34 ++++++++-- app/overrides/replace_delivery_address.rb | 11 +++- .../_other_available_distributors.html.erb | 16 +++++ .../spree/products/_source_sidebar.html.haml | 2 +- spec/models/order_spec.rb | 62 ++++++++++++++++++- 6 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 app/views/spree/checkout/_other_available_distributors.html.erb diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 798eedd60a..2342665975 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -35,7 +35,7 @@ Spree::OrdersController.class_eval do order = current_order(true) - if order.can_change_distributor? + if order.can_change_to_distributor?(distributor) order.distributor = distributor order.save! end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 36f552c0df..14b626a189 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -2,17 +2,43 @@ Spree::Order.class_eval do belongs_to :distributor, :class_name => 'Enterprise' before_validation :shipping_address_from_distributor - validate :can_change_distributor?, :if => :distributor_id_changed? + validate :change_distributor_validation, :if => :distributor_id_changed? + attr_accessible :distributor_id after_create :set_default_shipping_method + def change_distributor_validation + # Check that the line_items in the current order are available from a newly selected distributor + errors.add(:distributor_id, "The products in your cart are not available from '" + distributor.name + "'") unless can_change_to_distributor? distributor + end + + def can_change_to_distributor? distributor + # Distributor may not be changed once an item has been added to the cart/order, unless all items are available from the specified distributor + line_items.empty? || (available_distributors || []).include?(distributor) + end + def can_change_distributor? # Distributor may not be changed once an item has been added to the cart/order line_items.empty? end + def available_distributors + # Find all other enterprises which offer all product variants contained within the current order + distributors_with_all_variants = get_distributors_with_all_variants(Enterprise.all) + end + + def get_distributors_with_all_variants(enterprises) + variants_in_current_order = line_items.map{ |li| li.variant } + distributors_with_all_variants = [] + enterprises.each do |e| + variants_available_from_enterprise = ProductDistribution.find_all_by_distributor_id( e.id ).map{ |pd| pd.product.variants }.flatten + distributors_with_all_variants << e if ( variants_in_current_order - variants_available_from_enterprise ).empty? + end + distributors_with_all_variants + end + def distributor=(distributor) - raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || can_change_distributor? + raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || can_change_to_distributor?(distributor) super(distributor) end @@ -21,9 +47,9 @@ Spree::Order.class_eval do save! end - def can_add_product_to_cart?(product) - can_change_distributor? || product.distributors.include?(distributor) + # Products may be added if no line items are currently in the cart or if the product is available from the current distributor + line_items.empty? || product.distributors.include?(distributor) end def set_variant_attributes(variant, attributes) diff --git a/app/overrides/replace_delivery_address.rb b/app/overrides/replace_delivery_address.rb index 2b13785b1c..60b9883d5f 100644 --- a/app/overrides/replace_delivery_address.rb +++ b/app/overrides/replace_delivery_address.rb @@ -1,4 +1,9 @@ Deface::Override.new(:virtual_path => "spree/checkout/_address", - :replace => "[data-hook='shipping_fieldset_wrapper']", - :partial => "spree/checkout/distributor", - :name => "delivery_address") + :replace => "[data-hook='shipping_fieldset_wrapper']", + :partial => "spree/checkout/distributor", + :name => "replace_shipping_form") + +Deface::Override.new(:virtual_path => "spree/checkout/edit", + :insert_after => "[data-hook='checkout_summary_box']", + :partial => "spree/checkout/other_available_distributors", + :name => "other_available_distributors") diff --git a/app/views/spree/checkout/_other_available_distributors.html.erb b/app/views/spree/checkout/_other_available_distributors.html.erb new file mode 100644 index 0000000000..51ed8bce0e --- /dev/null +++ b/app/views/spree/checkout/_other_available_distributors.html.erb @@ -0,0 +1,16 @@ +
+ <% + other_available_distributors = @order.available_distributors + other_available_distributors.delete(@order.distributor) + unless other_available_distributors.empty? + %> + <%= form_for(@order) do |f| %> + <%= f.label :distributor_label, "Alternative distributors for this order:" %> + <% available_distributors_array = other_available_distributors.map { |distributor| [distributor.name + ": " + distributor.address.address1 + ", " + distributor.address.city, distributor.id.to_i] } %> + <%= f.select :distributor_id, options_for_select( available_distributors_array ) %> + <%= f.submit "Change Distributor" %> + <% end %> + <% else %> + No alternative distributors available. + <% end %> +
\ No newline at end of file diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index 41da860f88..e5a03d2896 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -11,7 +11,7 @@ - order = current_order(false) - @distributors.each do |distributor| %li.nowrap - - if order.nil? || order.can_change_distributor? + - if order.nil? || order.can_change_to_distributor?(distributor) = link_to distributor.name, select_distributor_order_path(distributor) - elsif order.distributor == distributor = link_to distributor.name, [main_app, distributor] diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 335c67a574..f80c1d1fa9 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -26,7 +26,7 @@ describe Spree::Order do subject.can_change_distributor?.should be_false end - it "raises an exception if distributor is changed without permission" do + it "checks that distributor is available when changing, and raises an exception if distributor is changed without permission" do d = create(:distributor_enterprise) p = create(:product, :distributors => [d]) subject.distributor = d @@ -34,6 +34,7 @@ describe Spree::Order do subject.add_variant(p.master, 1) subject.can_change_distributor?.should be_false + subject.should_receive(:available_distributors) expect do subject.distributor = nil @@ -79,4 +80,63 @@ describe Spree::Order do li = Spree::LineItem.last li.max_quantity.should == 3 end + + context "finding alternative distributors" do + it "checks that variants are available" do + distributors_with_all_variants = double(:distributors_with_all_variants) + subject.should_receive(:get_distributors_with_all_variants).with(Enterprise.all) + subject.available_distributors + end + + context "finding distributors which have the same variants" do + before(:each) do + @enterprise1 = FactoryGirl.create(:enterprise, id: 1) + subject.distributor = @enterprise1 + @product1 = FactoryGirl.create(:product) + @product2 = FactoryGirl.create(:product) + @product3 = FactoryGirl.create(:product) + variant11 = FactoryGirl.create(:variant, product: @product1) + variant12 = FactoryGirl.create(:variant, product: @product1) + variant21 = FactoryGirl.create(:variant, product: @product2) + variant31 = FactoryGirl.create(:variant, product: @product3) + variant32 = FactoryGirl.create(:variant, product: @product3) + + # Product Distributions + # Enterprise 1 sells product 1 and product 3 + FactoryGirl.create(:product_distribution, product: @product1, distributor: @enterprise1) + FactoryGirl.create(:product_distribution, product: @product3, distributor: @enterprise1) + + # Build the current order + line_item1 = FactoryGirl.create(:line_item, order: subject, variant: variant11) + line_item2 = FactoryGirl.create(:line_item, order: subject, variant: variant12) + line_item3 = FactoryGirl.create(:line_item, order: subject, variant: variant31) + subject.line_items = [line_item1,line_item2,line_item3] + end + + it "matches the distributor enterprise of the current order" do + subject.get_distributors_with_all_variants([@enterprise1]).should == [@enterprise1] + end + + it "does not match enterprises with no products available" do + test_enterprise = FactoryGirl.create(:enterprise, id: 2) + subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should_not include test_enterprise + end + + it "does not match enterprises with only some of the same variants in the current order available" do + test_enterprise = FactoryGirl.create(:enterprise, id: 2) + # Test Enterprise sells only product 1 + FactoryGirl.create(:product_distribution, product: @product1, distributor: test_enterprise) + subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should_not include test_enterprise + end + + it "matches enteprises which offer all products in the current order" do + test_enterprise = FactoryGirl.create(:enterprise, id: 2) + # Enterprise 3 Sells Products 1, 2 and 3 + FactoryGirl.create(:product_distribution, product: @product1, distributor: test_enterprise) + FactoryGirl.create(:product_distribution, product: @product2, distributor: test_enterprise) + FactoryGirl.create(:product_distribution, product: @product3, distributor: test_enterprise) + subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should include test_enterprise + end + end + end end From 9cf6124df317569ab7aba5638e2aa021eee8e3c2 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 7 Dec 2012 12:24:28 +1100 Subject: [PATCH 09/16] Change checkout explanations from CMS pages to CMS snippets --- app/overrides/add_cms_checkout_distribution.rb | 2 +- app/overrides/add_cms_to_cart.rb | 2 +- app/views/spree/orders/_distributor_fees.html.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/overrides/add_cms_checkout_distribution.rb b/app/overrides/add_cms_checkout_distribution.rb index 7e58596461..e1c1d5b7b5 100644 --- a/app/overrides/add_cms_checkout_distribution.rb +++ b/app/overrides/add_cms_checkout_distribution.rb @@ -1,4 +1,4 @@ Deface::Override.new(:virtual_path => "spree/checkout/_delivery", :insert_before => "fieldset#shipping_method", - :text => "<%= cms_page_content(:content, Cms::Page.find_by_full_path('/distribution')) %>", + :text => "<%= cms_snippet_content(Cms::Snippet.find_by_identifier('distribution')) %>", :name => "cms_checkout_distribution") \ No newline at end of file diff --git a/app/overrides/add_cms_to_cart.rb b/app/overrides/add_cms_to_cart.rb index 3b3b79897c..9be4e4a65d 100644 --- a/app/overrides/add_cms_to_cart.rb +++ b/app/overrides/add_cms_to_cart.rb @@ -1,4 +1,4 @@ Deface::Override.new(:virtual_path => "spree/orders/edit", :insert_after => "h1", - :text => "<%= cms_page_content(:content, Cms::Page.find_by_full_path('/cart')) %>", + :text => "<%= cms_snippet_content(Cms::Snippet.find_by_identifier('cart')) %>", :name => "cms_to_cart") \ No newline at end of file diff --git a/app/views/spree/orders/_distributor_fees.html.haml b/app/views/spree/orders/_distributor_fees.html.haml index a6b43a41d6..ff3e640c2d 100644 --- a/app/views/spree/orders/_distributor_fees.html.haml +++ b/app/views/spree/orders/_distributor_fees.html.haml @@ -1,6 +1,6 @@ #delivery-fees - = cms_page_content(:content, Cms::Page.find_by_full_path('/cart-delivery-fees')) %h2 Distribution Costs + = cms_snippet_content(Cms::Snippet.find_by_identifier('cart_distribution_costs')) %table#delivery %thead From b5a95e73dc45be263ad3d65f8b5a4b0cdb5cbf94 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 7 Dec 2012 12:29:44 +1100 Subject: [PATCH 10/16] Only show alternative distributors on 'address' page of checkout process --- .../spree/checkout/_other_available_distributors.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/spree/checkout/_other_available_distributors.html.erb b/app/views/spree/checkout/_other_available_distributors.html.erb index 51ed8bce0e..026b320b90 100644 --- a/app/views/spree/checkout/_other_available_distributors.html.erb +++ b/app/views/spree/checkout/_other_available_distributors.html.erb @@ -1,3 +1,4 @@ +<% unless @order.state != 'address' %>
<% other_available_distributors = @order.available_distributors @@ -13,4 +14,5 @@ <% else %> No alternative distributors available. <% end %> -
\ No newline at end of file +
+<% end %> \ No newline at end of file From 8a37b4e9184317932cb711270bf4a95d797191b7 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 14 Dec 2012 11:02:01 +1100 Subject: [PATCH 11/16] Push validation logic for changing distributor down into lib class --- .../spree/orders_controller_decorator.rb | 15 +- app/models/enterprise.rb | 4 + app/models/spree/order_decorator.rb | 49 ++---- .../_other_available_distributors.html.erb | 2 +- .../spree/products/_add_to_cart.html.haml | 6 +- .../spree/products/_source_sidebar.html.haml | 5 +- .../distributor_change_validator.rb | 22 +++ .../distributor_change_validator_spec.rb | 80 +++++++++ spec/models/order_spec.rb | 156 ++++++------------ 9 files changed, 188 insertions(+), 151 deletions(-) create mode 100644 lib/open_food_web/distributor_change_validator.rb create mode 100644 spec/lib/open_food_web/distributor_change_validator_spec.rb diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 2342665975..455d2c4ef2 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -34,11 +34,8 @@ Spree::OrdersController.class_eval do distributor = Enterprise.is_distributor.find params[:id] order = current_order(true) - - if order.can_change_to_distributor?(distributor) - order.distributor = distributor - order.save! - end + order.distributor = distributor + order.save! redirect_to main_app.enterprise_path(distributor) end @@ -46,10 +43,8 @@ Spree::OrdersController.class_eval do def deselect_distributor order = current_order(true) - if order.can_change_distributor? - order.distributor = nil - order.save! - end + order.distributor = nil + order.save! redirect_to root_path end @@ -73,7 +68,7 @@ Spree::OrdersController.class_eval do # -- If products in cart, distributor can't be changed order = current_order(false) - if !order.nil? && !order.can_change_distributor? && order.distributor != distributor + if !order.nil? && !DistributorChangeValidator.new(order).can_change_to_distributor?(distributor) return false end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index d4679f6424..9afcaaff58 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -27,6 +27,10 @@ class Enterprise < ActiveRecord::Base def to_param "#{id}-#{name.parameterize}" end + + def available_variants + ProductDistribution.find_all_by_distributor_id( self.id ).map{ |pd| pd.product.variants }.flatten + end private diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 14b626a189..f588ab1353 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -1,44 +1,22 @@ +require 'open_food_web/distributor_change_validator' + Spree::Order.class_eval do belongs_to :distributor, :class_name => 'Enterprise' before_validation :shipping_address_from_distributor - validate :change_distributor_validation, :if => :distributor_id_changed? + validate :products_available_from_new_distributor, :if => :distributor_id_changed? attr_accessible :distributor_id after_create :set_default_shipping_method - def change_distributor_validation + + def products_available_from_new_distributor # Check that the line_items in the current order are available from a newly selected distributor - errors.add(:distributor_id, "The products in your cart are not available from '" + distributor.name + "'") unless can_change_to_distributor? distributor - end - - def can_change_to_distributor? distributor - # Distributor may not be changed once an item has been added to the cart/order, unless all items are available from the specified distributor - line_items.empty? || (available_distributors || []).include?(distributor) - end - - def can_change_distributor? - # Distributor may not be changed once an item has been added to the cart/order - line_items.empty? - end - - def available_distributors - # Find all other enterprises which offer all product variants contained within the current order - distributors_with_all_variants = get_distributors_with_all_variants(Enterprise.all) - end - - def get_distributors_with_all_variants(enterprises) - variants_in_current_order = line_items.map{ |li| li.variant } - distributors_with_all_variants = [] - enterprises.each do |e| - variants_available_from_enterprise = ProductDistribution.find_all_by_distributor_id( e.id ).map{ |pd| pd.product.variants }.flatten - distributors_with_all_variants << e if ( variants_in_current_order - variants_available_from_enterprise ).empty? - end - distributors_with_all_variants + errors.add(:distributor_id, "cannot supply the products in your cart") unless DistributorChangeValidator.new(self).can_change_to_distributor?(distributor) end def distributor=(distributor) - raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || can_change_to_distributor?(distributor) + #raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || DistributorChangeValidator.new(self).can_change_to_distributor?(distributor) super(distributor) end @@ -47,11 +25,6 @@ Spree::Order.class_eval do save! end - def can_add_product_to_cart?(product) - # Products may be added if no line items are currently in the cart or if the product is available from the current distributor - line_items.empty? || product.distributors.include?(distributor) - end - def set_variant_attributes(variant, attributes) line_item = contains?(variant) @@ -62,6 +35,14 @@ Spree::Order.class_eval do line_item.assign_attributes(attributes) line_item.save! end + + def can_add_product_to_cart? product + DistributorChangeValidator.new(self).can_change_distributor? || product.distributors.include?(distributor) + end + + def line_item_variants + line_items.map{ |li| li.variant } + end private diff --git a/app/views/spree/checkout/_other_available_distributors.html.erb b/app/views/spree/checkout/_other_available_distributors.html.erb index 026b320b90..2b84986dd4 100644 --- a/app/views/spree/checkout/_other_available_distributors.html.erb +++ b/app/views/spree/checkout/_other_available_distributors.html.erb @@ -1,7 +1,7 @@ <% unless @order.state != 'address' %>
<% - other_available_distributors = @order.available_distributors + other_available_distributors = DistributorChangeValidator.new(@order).available_distributors(Enterprise.all) other_available_distributors.delete(@order.distributor) unless other_available_distributors.empty? %> diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index e934cebe21..3f48211f6c 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -1,8 +1,9 @@ .add-to-cart + - order = current_order(false) - if !@product.has_stock? && !Spree::Config[:allow_backorders] = content_tag('strong', t(:out_of_stock)) - - elsif current_order(false) && !current_order(false).can_add_product_to_cart?(@product) + - elsif current_order(false) && !order.can_add_product_to_cart?(@product) .error-distributor Please complete your order at = link_to current_distributor.name, root_path @@ -14,8 +15,7 @@ - if @product.group_buy %p Max quantity = number_field_tag (@product.has_variants? ? :max_quantity : "variant_attributes[#{@product.master.id}][max_quantity]"), 1, :class => 'title max_quantity', :in => 1..@product.on_hand - - order = current_order(false) - - if order.nil? || order.can_change_distributor? + - if order.nil? || DistributorChangeValidator.new(order).can_change_distributor? %p Distributor = select_tag "distributor_id", options_from_collection_for_select([Enterprise.new]+@product.distributors, "id", "name", current_distributor.andand.id) - else diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index e5a03d2896..c91627596e 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -9,13 +9,14 @@ %h6.filter_name Shop by Distributor %ul.filter_choices - order = current_order(false) + - validator = DistributorChangeValidator.new(order) - @distributors.each do |distributor| %li.nowrap - - if order.nil? || order.can_change_to_distributor?(distributor) + - if order.nil? || validator.can_change_to_distributor?(distributor) = link_to distributor.name, select_distributor_order_path(distributor) - elsif order.distributor == distributor = link_to distributor.name, [main_app, distributor] - else %span.inactive= distributor.name - - if current_distributor && order.can_change_distributor? + - if current_distributor && validator.can_change_distributor? = button_to 'Browse All Distributors', deselect_distributor_orders_path, :method => :get diff --git a/lib/open_food_web/distributor_change_validator.rb b/lib/open_food_web/distributor_change_validator.rb new file mode 100644 index 0000000000..ca7545eefc --- /dev/null +++ b/lib/open_food_web/distributor_change_validator.rb @@ -0,0 +1,22 @@ +class DistributorChangeValidator + + def initialize order + @order = order + end + + def can_change_distributor? + # Distributor may not be changed once an item has been added to the cart/order + @order.line_items.empty? + end + + def can_change_to_distributor? distributor + # Distributor may not be changed once an item has been added to the cart/order, unless all items are available from the specified distributor + @order.line_items.empty? || (available_distributors(Enterprise.all) || []).include?(distributor) + end + + def available_distributors enterprises + enterprises.select do |e| + (@order.line_item_variants - e.available_variants).empty? + end + end +end \ No newline at end of file diff --git a/spec/lib/open_food_web/distributor_change_validator_spec.rb b/spec/lib/open_food_web/distributor_change_validator_spec.rb new file mode 100644 index 0000000000..d8f8967000 --- /dev/null +++ b/spec/lib/open_food_web/distributor_change_validator_spec.rb @@ -0,0 +1,80 @@ +require 'open_food_web/distributor_change_validator' + +describe DistributorChangeValidator do + let(:order) { double(:order) } + let(:subject) { DistributorChangeValidator.new(order) } + + context "permissions for changing distributor" do + it "allows distributor to be changed if line_items is empty" do + order.stub(:line_items) { [] } + subject.can_change_distributor?.should be_true + end + + it "does not allow distributor to be changed if line_items is not empty" do + order.stub(:line_items) { [1, 2, 3] } + subject.can_change_distributor?.should be_false + end + end + + context "finding distributors which have the same variants" do + it "matches enterprises which offer all products within the order" do + variant1 = double(:variant) + variant2 = double(:variant) + variant3 = double(:variant) + variant5 = double(:variant) + line_item_variants = [variant1, variant3, variant5] + order.stub(:line_item_variants){ line_item_variants } + enterprise = double(:enterprise) + enterprise.stub(:available_variants){ line_item_variants } # Exactly the same variants as the order + + subject.available_distributors([enterprise]).should == [enterprise] + end + + it "does not match enterprises with no products available" do + variant1 = double(:variant) + variant3 = double(:variant) + variant5 = double(:variant) + line_item_variants = [variant1, variant3, variant5] + order.stub(:line_item_variants){ line_item_variants } + enterprise = double(:enterprise) + enterprise.stub(:available_variants){ [] } # No variants + + subject.available_distributors([enterprise]).should_not include enterprise + end + + it "does not match enterprises with only some of the same variants in the order available" do + variant1 = double(:variant) + variant2 = double(:variant) + variant3 = double(:variant) + variant4 = double(:variant) + variant5 = double(:variant) + line_item_variants = [variant1, variant3, variant5] + order.stub(:line_item_variants){ line_item_variants } + enterprise_with_some_variants = double(:enterprise) + enterprise_with_some_variants.stub(:available_variants){ [variant1, variant3] } # Only some variants + enterprise_with_some_plus_extras = double(:enterprise) + enterprise_with_some_plus_extras.stub(:available_variants){ [variant1, variant2, variant3, variant4] } # Only some variants, plus extras + + subject.available_distributors([enterprise_with_some_variants]).should_not include enterprise_with_some_variants + subject.available_distributors([enterprise_with_some_plus_extras]).should_not include enterprise_with_some_plus_extras + end + + it "matches enteprises which offer all products in the order, plus additional products" do + variant1 = double(:variant) + variant2 = double(:variant) + variant3 = double(:variant) + variant4 = double(:variant) + variant5 = double(:variant) + line_item_variants = [variant1, variant3, variant5] + order.stub(:line_item_variants){ line_item_variants } + enterprise = double(:enterprise) + enterprise.stub(:available_variants){ [variant1, variant2, variant3, variant4, variant5] } # Excess variants + + subject.available_distributors([enterprise]).should == [enterprise] + end + + it "matches no enterprises when none are provided" do + subject.available_distributors([]).should == [] + end + end +end \ No newline at end of file diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index f80c1d1fa9..171258579c 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -14,59 +14,6 @@ describe Spree::Order do subject.adjustments.where(:label => "Shipping").should be_present end - it "reveals permission for changing distributor" do - d = create(:distributor_enterprise) - p = create(:product, :distributors => [d]) - - subject.distributor = d - subject.save! - - subject.can_change_distributor?.should be_true - subject.add_variant(p.master, 1) - subject.can_change_distributor?.should be_false - end - - it "checks that distributor is available when changing, and raises an exception if distributor is changed without permission" do - d = create(:distributor_enterprise) - p = create(:product, :distributors => [d]) - subject.distributor = d - subject.save! - - subject.add_variant(p.master, 1) - subject.can_change_distributor?.should be_false - subject.should_receive(:available_distributors) - - expect do - subject.distributor = nil - end.to raise_error "You cannot change the distributor of an order with products" - end - - it "reveals permission for adding products to the cart" do - d1 = create(:distributor_enterprise) - d2 = create(:distributor_enterprise) - - p_first = create(:product, :distributors => [d1]) - p_subsequent_same_dist = create(:product, :distributors => [d1]) - p_subsequent_other_dist = create(:product, :distributors => [d2]) - - # We need to set distributor, since order.add_variant does not, and - # we also need to save the order so that line items can be added to - # the association. - subject.distributor = d1 - subject.save! - - # The first product in the cart can be added - subject.can_add_product_to_cart?(p_first).should be_true - subject.add_variant(p_first.master, 1) - - # A subsequent product can be added if the distributor matches - subject.can_add_product_to_cart?(p_subsequent_same_dist).should be_true - subject.add_variant(p_subsequent_same_dist.master, 1) - - # And cannot be added if it does not match - subject.can_add_product_to_cart?(p_subsequent_other_dist).should be_false - end - it "sets attributes on line items for variants" do d = create(:distributor_enterprise) p = create(:product, :distributors => [d]) @@ -81,62 +28,69 @@ describe Spree::Order do li.max_quantity.should == 3 end - context "finding alternative distributors" do - it "checks that variants are available" do - distributors_with_all_variants = double(:distributors_with_all_variants) - subject.should_receive(:get_distributors_with_all_variants).with(Enterprise.all) - subject.available_distributors + context "permissions for adding products to the cart" do + it "allows products to be added to cart when cart is empty" do + p_first = double(:product) + p_first.stub(:distributors) { [d1] } + subject.stub(:line_items) { [] } + subject.can_add_product_to_cart?(p_first).should be_true end - context "finding distributors which have the same variants" do - before(:each) do - @enterprise1 = FactoryGirl.create(:enterprise, id: 1) - subject.distributor = @enterprise1 - @product1 = FactoryGirl.create(:product) - @product2 = FactoryGirl.create(:product) - @product3 = FactoryGirl.create(:product) - variant11 = FactoryGirl.create(:variant, product: @product1) - variant12 = FactoryGirl.create(:variant, product: @product1) - variant21 = FactoryGirl.create(:variant, product: @product2) - variant31 = FactoryGirl.create(:variant, product: @product3) - variant32 = FactoryGirl.create(:variant, product: @product3) + it "allows products to be added to cart when they are available from the current distributor" do + d1 = double(:distributor) + p_first = double(:product) + p_first.stub(:distributors) { [d1] } + p_subsequent_same_dist = double(:product) + p_subsequent_same_dist.stub(:distributors) { [d1] } + subject.stub(:line_items) { [1, 2, 3] } + subject.stub(:distributor) { d1 } + subject.can_add_product_to_cart?(p_subsequent_same_dist).should be_true + end - # Product Distributions - # Enterprise 1 sells product 1 and product 3 - FactoryGirl.create(:product_distribution, product: @product1, distributor: @enterprise1) - FactoryGirl.create(:product_distribution, product: @product3, distributor: @enterprise1) + it "does not allow products to be added to cart when they are not available from the current distributor" do + d1 = double(:distributor) + d2 = double(:distributor) + p_first = double(:product) + p_first.stub(:distributors) { [d1] } + p_subsequent_other_dist = double(:product) + p_subsequent_other_dist.stub(:distributors) { [d2] } + subject.stub(:line_items) { [1, 2, 3] } + subject.stub(:distributor) { d1 } + subject.can_add_product_to_cart?(p_subsequent_other_dist).should be_false + end + end - # Build the current order - line_item1 = FactoryGirl.create(:line_item, order: subject, variant: variant11) - line_item2 = FactoryGirl.create(:line_item, order: subject, variant: variant12) - line_item3 = FactoryGirl.create(:line_item, order: subject, variant: variant31) - subject.line_items = [line_item1,line_item2,line_item3] - end + context "validating distributor changes" do + it "checks that a distributor is available when changing" do + order_enterprise = FactoryGirl.create(:enterprise, id: 1, :name => "Order Enterprise") + subject.distributor = order_enterprise + product1 = FactoryGirl.create(:product) + product2 = FactoryGirl.create(:product) + product3 = FactoryGirl.create(:product) + variant11 = FactoryGirl.create(:variant, product: product1) + variant12 = FactoryGirl.create(:variant, product: product1) + variant21 = FactoryGirl.create(:variant, product: product2) + variant31 = FactoryGirl.create(:variant, product: product3) + variant32 = FactoryGirl.create(:variant, product: product3) - it "matches the distributor enterprise of the current order" do - subject.get_distributors_with_all_variants([@enterprise1]).should == [@enterprise1] - end + # Product Distributions + # Order Enterprise sells product 1 and product 3 + FactoryGirl.create(:product_distribution, product: product1, distributor: order_enterprise) + FactoryGirl.create(:product_distribution, product: product3, distributor: order_enterprise) - it "does not match enterprises with no products available" do - test_enterprise = FactoryGirl.create(:enterprise, id: 2) - subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should_not include test_enterprise - end + # Build the current order + line_item1 = FactoryGirl.create(:line_item, order: subject, variant: variant11) + line_item2 = FactoryGirl.create(:line_item, order: subject, variant: variant12) + line_item3 = FactoryGirl.create(:line_item, order: subject, variant: variant31) + subject.line_items = [line_item1,line_item2,line_item3] - it "does not match enterprises with only some of the same variants in the current order available" do - test_enterprise = FactoryGirl.create(:enterprise, id: 2) - # Test Enterprise sells only product 1 - FactoryGirl.create(:product_distribution, product: @product1, distributor: test_enterprise) - subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should_not include test_enterprise - end + test_enterprise = FactoryGirl.create(:enterprise, id: 2, :name => "Test Enterprise") + # Test Enterprise sells only product 1 + FactoryGirl.create(:product_distribution, product: product1, distributor: test_enterprise) - it "matches enteprises which offer all products in the current order" do - test_enterprise = FactoryGirl.create(:enterprise, id: 2) - # Enterprise 3 Sells Products 1, 2 and 3 - FactoryGirl.create(:product_distribution, product: @product1, distributor: test_enterprise) - FactoryGirl.create(:product_distribution, product: @product2, distributor: test_enterprise) - FactoryGirl.create(:product_distribution, product: @product3, distributor: test_enterprise) - subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should include test_enterprise - end + subject.distributor = test_enterprise + subject.should_not be_valid + subject.errors.should include :distributor_id end end end From c7ac0f7d91cf8eaa8f872a3cbc216d6890e13ad6 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 14 Dec 2012 11:57:34 +1100 Subject: [PATCH 12/16] Allow distributor to be changed from product page --- app/views/spree/products/_source.html.haml | 29 ++++++++++++++----- .../spree/products/_source_sidebar.html.haml | 4 +-- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/views/spree/products/_source.html.haml b/app/views/spree/products/_source.html.haml index dc7f033b91..1fd3598ef4 100644 --- a/app/views/spree/products/_source.html.haml +++ b/app/views/spree/products/_source.html.haml @@ -1,17 +1,30 @@ %div{:data-hook => "product_source"} - %h6.product-section-title Source + %h6.product-section-title SUPPLIER %table#product-source.table-display{:width => "100%"} %tbody - if @product.supplier %tr.odd - %td - %strong Supplier - %td= @product.supplier.name + %td= link_to @product.supplier.name, [main_app, @product.supplier] %br/ + %h6.product-section-title DISTRIBUTORS %table#product-source.table-display{:width => "100%"} %tbody + - order = current_order(false) + - validator = DistributorChangeValidator.new(order) - @product.distributors.each do |distributor| - %tr.even - %td - %strong Distributor - %td= distributor.name + - if distributor == order.distributor + %tr.odd + %td + %b= link_to(distributor.name, [main_app, distributor]) + %td + %b Current + - elsif order.nil? || validator.can_change_to_distributor?(distributor) + %tr.even + %td= link_to distributor.name, [main_app, distributor] + %td= link_to "Change to distributor", select_distributor_order_path(distributor) + - else + %tr.even + %td= link_to distributor.name, [main_app, distributor] + %td + %abbr(title="One or more of the products in your cart is not available from this distributor") Unavailable + \ No newline at end of file diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index c91627596e..03fc78ac84 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -13,10 +13,10 @@ - @distributors.each do |distributor| %li.nowrap - if order.nil? || validator.can_change_to_distributor?(distributor) - = link_to distributor.name, select_distributor_order_path(distributor) + = link_to distributor.name, [main_app, distributor] - elsif order.distributor == distributor = link_to distributor.name, [main_app, distributor] - else - %span.inactive= distributor.name + %abbr(title="One or more of the products in your cart is not available from this distributor")= distributor.name - if current_distributor && validator.can_change_distributor? = button_to 'Browse All Distributors', deselect_distributor_orders_path, :method => :get From 1f8b4d185b6c95798bf57d8b534f36dbaf1a94b4 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 14 Dec 2012 14:01:13 +1100 Subject: [PATCH 13/16] Rearrange 'Add To Cart' section of product page --- .../stylesheets/store/openfoodweb.css.scss | 5 +++ .../spree/products/_add_to_cart.html.haml | 33 ++++++++++++------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/assets/stylesheets/store/openfoodweb.css.scss b/app/assets/stylesheets/store/openfoodweb.css.scss index 5af6ca1b33..3e91985fe9 100644 --- a/app/assets/stylesheets/store/openfoodweb.css.scss +++ b/app/assets/stylesheets/store/openfoodweb.css.scss @@ -230,3 +230,8 @@ fieldset#product-distributor-details { div#eft-payment-alert { border: 2px solid red; } + +/* Cleared div for clearing previous floating elements */ +div.cleared { + clear: both; +} diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index 3f48211f6c..b60d0173fc 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -10,17 +10,26 @@ before shopping with another distributor. - else - %p Quantity - = number_field_tag (@product.has_variants? ? :quantity : "variants[#{@product.master.id}]"), 1, :class => 'title', :in => 1..@product.on_hand + %div(class = "columns alpha two") + %div Quantity + = number_field_tag (@product.has_variants? ? :quantity : "variants[#{@product.master.id}]"), 1, :class => 'title', :in => 1..@product.on_hand - if @product.group_buy - %p Max quantity - = number_field_tag (@product.has_variants? ? :max_quantity : "variant_attributes[#{@product.master.id}][max_quantity]"), 1, :class => 'title max_quantity', :in => 1..@product.on_hand - - if order.nil? || DistributorChangeValidator.new(order).can_change_distributor? - %p Distributor - = select_tag "distributor_id", options_from_collection_for_select([Enterprise.new]+@product.distributors, "id", "name", current_distributor.andand.id) - - else - = hidden_field_tag "distributor_id", order.distributor.id - .distributor-fixed= "Your distributor for this order is #{order.distributor.name}" - %br/ + %div(class = "columns alpha three") + %div Max quantity + = number_field_tag (@product.has_variants? ? :max_quantity : "variant_attributes[#{@product.master.id}][max_quantity]"), 1, :class => 'title max_quantity', :in => 1..@product.on_hand + %div.cleared + %br + - if order.nil? + %div Distributor for your order: + = select_tag "distributor_id", options_from_collection_for_select([Enterprise.new]+@product.distributors, "id", "name", current_distributor.andand.id) + - else + - available_distributors = DistributorChangeValidator.new(order).available_distributors(@product.distributors) + - if available_distributors.length > 0 + %div Distributor for your order: + = select_tag "distributor_id", options_from_collection_for_select(available_distributors, "id", "name", current_distributor.andand.id) + - else + = hidden_field_tag "distributor_id", order.distributor.id + .distributor-fixed= "Your distributor for this order is #{order.distributor.name}" + %br = button_tag :class => 'large primary', :id => 'add-to-cart-button', :type => :submit do - = t(:add_to_cart) + = t(:add_to_cart) \ No newline at end of file From 63cf128f863dc311a82dbda47144deda4e76066b Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 14 Dec 2012 15:08:35 +1100 Subject: [PATCH 14/16] Remove product descriptions from line item listings in cart and order --- app/overrides/order_item_description.rb | 4 ++++ app/views/spree/orders/_cart_item_description.html.haml | 3 +-- app/views/spree/orders/_order_item_description.html.haml | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 app/overrides/order_item_description.rb create mode 100644 app/views/spree/orders/_order_item_description.html.haml diff --git a/app/overrides/order_item_description.rb b/app/overrides/order_item_description.rb new file mode 100644 index 0000000000..ae1a28ecd2 --- /dev/null +++ b/app/overrides/order_item_description.rb @@ -0,0 +1,4 @@ +Deface::Override.new(:virtual_path => "spree/shared/_order_details", + :replace => "[data-hook='order_item_description']", + :partial => "spree/orders/order_item_description", + :name => "order_item_description") \ No newline at end of file diff --git a/app/views/spree/orders/_cart_item_description.html.haml b/app/views/spree/orders/_cart_item_description.html.haml index ff04782b44..9a516168bc 100644 --- a/app/views/spree/orders/_cart_item_description.html.haml +++ b/app/views/spree/orders/_cart_item_description.html.haml @@ -4,5 +4,4 @@ - if @order.insufficient_stock_lines.include? line_item %span.out-of-stock = variant.in_stock? ? t(:insufficient_stock, :on_hand => variant.on_hand) : t(:out_of_stock) - %br/ - = truncate_html(variant.product.description, :length => 100, :omission => "...") + %br/ \ No newline at end of file diff --git a/app/views/spree/orders/_order_item_description.html.haml b/app/views/spree/orders/_order_item_description.html.haml new file mode 100644 index 0000000000..d7a9184736 --- /dev/null +++ b/app/views/spree/orders/_order_item_description.html.haml @@ -0,0 +1,3 @@ +%td(data-hook = "order_item_description") + %h4= item.variant.product.name + = "(" + variant_options(item.variant) + ")" unless item.variant .option_values.empty? \ No newline at end of file From 6a45e8be3bcca253e5c65e79faf0c56097a78909 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 18 Jan 2013 15:29:36 +1100 Subject: [PATCH 15/16] Make tests pass after changes to checkout broke them all --- app/models/enterprise.rb | 2 +- .../spree/products/_add_to_cart.html.haml | 9 ++- app/views/spree/products/_source.html.haml | 8 +-- .../enterprises_controller_spec.rb | 61 ------------------- spec/controllers/orders_controller_spec.rb | 25 ++++++++ spec/requests/consumer/checkout_spec.rb | 2 +- spec/requests/consumer/distributors_spec.rb | 23 ++++--- spec/requests/consumer/product_spec.rb | 3 +- spec/requests/consumer/taxonomy_spec.rb | 2 +- 9 files changed, 48 insertions(+), 87 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 9afcaaff58..17c9380097 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -29,7 +29,7 @@ class Enterprise < ActiveRecord::Base end def available_variants - ProductDistribution.find_all_by_distributor_id( self.id ).map{ |pd| pd.product.variants }.flatten + ProductDistribution.find_all_by_distributor_id( self.id ).map{ |pd| pd.product.variants + [pd.product.master] }.flatten end diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index b60d0173fc..495ea77bbb 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -3,12 +3,11 @@ - if !@product.has_stock? && !Spree::Config[:allow_backorders] = content_tag('strong', t(:out_of_stock)) - - elsif current_order(false) && !order.can_add_product_to_cart?(@product) + - elsif !order.nil? && !order.can_add_product_to_cart?(@product) .error-distributor Please complete your order at = link_to current_distributor.name, root_path before shopping with another distributor. - - else %div(class = "columns alpha two") %div Quantity @@ -19,12 +18,12 @@ = number_field_tag (@product.has_variants? ? :max_quantity : "variant_attributes[#{@product.master.id}][max_quantity]"), 1, :class => 'title max_quantity', :in => 1..@product.on_hand %div.cleared %br - - if order.nil? + - if order.nil? || order.distributor.nil? %div Distributor for your order: - = select_tag "distributor_id", options_from_collection_for_select([Enterprise.new]+@product.distributors, "id", "name", current_distributor.andand.id) + = select_tag "distributor_id", options_from_collection_for_select([Enterprise.new]+@product.distributors, "id", "name", :include_blank => '') - else - available_distributors = DistributorChangeValidator.new(order).available_distributors(@product.distributors) - - if available_distributors.length > 0 + - if available_distributors.length > 1 %div Distributor for your order: = select_tag "distributor_id", options_from_collection_for_select(available_distributors, "id", "name", current_distributor.andand.id) - else diff --git a/app/views/spree/products/_source.html.haml b/app/views/spree/products/_source.html.haml index 1fd3598ef4..0757658c0e 100644 --- a/app/views/spree/products/_source.html.haml +++ b/app/views/spree/products/_source.html.haml @@ -12,7 +12,7 @@ - order = current_order(false) - validator = DistributorChangeValidator.new(order) - @product.distributors.each do |distributor| - - if distributor == order.distributor + - if !order.nil? && distributor == order.distributor %tr.odd %td %b= link_to(distributor.name, [main_app, distributor]) @@ -21,10 +21,10 @@ - elsif order.nil? || validator.can_change_to_distributor?(distributor) %tr.even %td= link_to distributor.name, [main_app, distributor] - %td= link_to "Change to distributor", select_distributor_order_path(distributor) + %td Available + -#%td= link_to "Change to distributor", select_distributor_order_path(distributor) - else %tr.even %td= link_to distributor.name, [main_app, distributor] - %td - %abbr(title="One or more of the products in your cart is not available from this distributor") Unavailable + %td \ No newline at end of file diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index 1554984dfb..a5709d3a74 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/spec/controllers/enterprises_controller_spec.rb @@ -19,65 +19,4 @@ describe EnterprisesController do assigns(:suppliers).should == [s] end - - it "selects distributors" do - d = create(:distributor_enterprise) - - spree_get :select_distributor, :id => d.id - response.should be_redirect - - order = current_order(false) - order.distributor.should == d - end - - it "deselects distributors" do - d = create(:distributor_enterprise) - order = current_order(true) - order.distributor = d - order.save! - - spree_get :deselect_distributor - response.should be_redirect - - order.reload - order.distributor.should be_nil - end - - context "when a product has been added to the cart" do - it "does not allow selecting another distributor" do - # Given some distributors and an order with a product - d1 = create(:distributor_enterprise) - d2 = create(:distributor_enterprise) - p = create(:product, :distributors => [d1]) - o = current_order(true) - - o.distributor = d1 - o.save! - o.add_variant(p.master, 1) - - # When I attempt to select a distributor - spree_get :select_distributor, :id => d2.id - - # Then my distributor should remain unchanged - o.reload - o.distributor.should == d1 - end - - it "does not allow deselecting distributors" do - # Given a distributor and an order with a product - d = create(:distributor_enterprise) - p = create(:product, :distributors => [d]) - o = current_order(true) - o.distributor = d - o.save! - o.add_variant(p.master, 1) - - # When I attempt to deselect the distributor - spree_get :deselect_distributor - - # Then my distributor should remain unchanged - o.reload - o.distributor.should == d - end - end end diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb index ded96678dd..440bf8fcbf 100644 --- a/spec/controllers/orders_controller_spec.rb +++ b/spec/controllers/orders_controller_spec.rb @@ -7,7 +7,32 @@ describe Spree::OrdersController do def current_user controller.current_user end + + it "selects distributors" do + d = create(:distributor_enterprise) + p = create(:product, :distributors => [d]) + spree_get :select_distributor, :id => d.id + response.should be_redirect + + order = current_order(false) + order.distributor.should == d + end + + it "deselects distributors" do + d = create(:distributor_enterprise) + p = create(:product, :distributors => [d]) + + order = current_order(true) + order.distributor = d + order.save! + + spree_get :deselect_distributor + response.should be_redirect + + order.reload + order.distributor.should be_nil + end context "adding the first product to the cart" do it "does not add the product if the user does not specify a distributor" do diff --git a/spec/requests/consumer/checkout_spec.rb b/spec/requests/consumer/checkout_spec.rb index 2a0bce8c5d..320dfcf281 100644 --- a/spec/requests/consumer/checkout_spec.rb +++ b/spec/requests/consumer/checkout_spec.rb @@ -118,7 +118,7 @@ feature %q{ click_button 'Save and Continue' # -- Checkout: Payment - click_button 'Save and Continue' + click_button 'Process My Order' # -- Checkout: Order complete page.should have_content('Your order has been processed successfully') diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index 32e642edf5..cd03a67def 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -67,11 +67,10 @@ feature %q{ it "displays the distributor's name on the home page" do # Given a distributor with a product d = create(:distributor_enterprise, :name => 'Melb Uni Co-op', :description => '

Hello, world!

') - create(:product, :distributors => [d]) + p1 = create(:product, :distributors => [d]) # When I select the distributor - visit spree.root_path - click_link d.name + visit spree.select_distributor_order_path(d) visit spree.root_path # Then I should see the name of the distributor that I've selected @@ -90,8 +89,7 @@ feature %q{ p2 = create(:product, :distributors => [d2], :taxons => [taxon]) # When I select the first distributor - visit spree.root_path - click_link d1.name + visit spree.select_distributor_order_path(d1) visit spree.root_path # Then I should see products split by local/remote distributor @@ -112,11 +110,11 @@ feature %q{ it "allows the user to leave the distributor" do # Given a distributor with a product d = create(:distributor_enterprise, :name => 'Melb Uni Co-op') - create(:product, :distributors => [d]) + p1 = create(:product, :distributors => [d]) # When I select the distributor and then leave it + visit spree.select_distributor_order_path(d) visit spree.root_path - click_link d.name click_button 'Browse All Distributors' # Then I should have left the distributor @@ -139,16 +137,17 @@ feature %q{ it "displays the local distributor as the default choice when available for the current product" do # Given a distributor and a product under it - distributor = create(:distributor_enterprise) - product = create(:product, :distributors => [distributor]) + distributor1 = create(:distributor_enterprise) + distributor2 = create(:distributor_enterprise) + product = create(:product, :distributors => [distributor1,distributor2]) # When we select the distributor and view the product - visit spree.root_path - click_link distributor.name + visit spree.select_distributor_order_path(distributor1) visit spree.product_path(product) + binding.pry # Then we should see our distributor as the default option when adding the item to our cart - page.should have_selector "select#distributor_id option[value='#{distributor.id}'][selected='selected']" + page.should have_selector "select#distributor_id option[value='#{distributor1.id}'][selected='selected']" end it "works when viewing a product from a remote distributor" do diff --git a/spec/requests/consumer/product_spec.rb b/spec/requests/consumer/product_spec.rb index e8741548e2..1cd5f81574 100644 --- a/spec/requests/consumer/product_spec.rb +++ b/spec/requests/consumer/product_spec.rb @@ -36,8 +36,7 @@ feature %q{ d = create(:distributor_enterprise) p = create(:product, :distributors => [d]) - visit spree.root_path - click_link d.name + visit spree.select_distributor_order_path(d) visit spree.product_path p within '#product-distributor-details' do diff --git a/spec/requests/consumer/taxonomy_spec.rb b/spec/requests/consumer/taxonomy_spec.rb index 9a2190c1a0..47021bb848 100644 --- a/spec/requests/consumer/taxonomy_spec.rb +++ b/spec/requests/consumer/taxonomy_spec.rb @@ -49,7 +49,7 @@ feature %q{ 2.times { create(:product, :taxons => [taxon_three], :distributors => [my_distributor]) } # When I visit the home page and select my distributor - visit spree.root_path + visit spree.select_distributor_order_path(my_distributor) click_link my_distributor.name page.should have_content 'You are shopping at My Distributor' From f7890bd94df1f565877a2ef70c34764bd901aca9 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 18 Jan 2013 19:17:55 +1100 Subject: [PATCH 16/16] Fixes following code review for checkout changes --- app/helpers/application_helper.rb | 1 - app/helpers/enterprises_helper.rb | 4 +++ app/helpers/spree/orders_helper.rb | 4 +++ app/models/spree/order_decorator.rb | 9 ------ .../_other_available_distributors.html.erb | 9 ++---- .../orders/_order_item_description.html.haml | 2 +- .../spree/products/_add_to_cart.html.haml | 2 +- spec/models/order_spec.rb | 32 ------------------- spec/requests/consumer/checkout_spec.rb | 16 +++++++++- spec/requests/consumer/distributors_spec.rb | 1 - 10 files changed, 27 insertions(+), 53 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4f41a1a0ed..620dd8bb29 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -5,7 +5,6 @@ module ApplicationHelper end end - # Pass URL helper calls on to spree where applicable so that we don't need to use # spree.foo_path in any view rendered from non-spree-namespaced controllers. def method_missing(method, *args, &block) diff --git a/app/helpers/enterprises_helper.rb b/app/helpers/enterprises_helper.rb index fb8a9a13bc..a1012dbe1c 100644 --- a/app/helpers/enterprises_helper.rb +++ b/app/helpers/enterprises_helper.rb @@ -2,4 +2,8 @@ module EnterprisesHelper def current_distributor @current_distributor ||= current_order(false).andand.distributor end + + def enterprises_options enterprises + enterprises.map { |enterprise| [enterprise.name + ": " + enterprise.address.address1 + ", " + enterprise.address.city, enterprise.id.to_i] } + end end diff --git a/app/helpers/spree/orders_helper.rb b/app/helpers/spree/orders_helper.rb index 7c89d20bf8..08d5e5492f 100644 --- a/app/helpers/spree/orders_helper.rb +++ b/app/helpers/spree/orders_helper.rb @@ -5,5 +5,9 @@ module Spree amount = order.line_items.map { |li| li.itemwise_shipping_cost }.sum options.delete(:format_as_currency) ? number_to_currency(amount) : amount end + + def alternative_available_distributors(order) + DistributorChangeValidator.new(order).available_distributors(Enterprise.all) - [order.distributor] + end end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index f588ab1353..17dbe09405 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -15,11 +15,6 @@ Spree::Order.class_eval do errors.add(:distributor_id, "cannot supply the products in your cart") unless DistributorChangeValidator.new(self).can_change_to_distributor?(distributor) end - def distributor=(distributor) - #raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || DistributorChangeValidator.new(self).can_change_to_distributor?(distributor) - super(distributor) - end - def set_distributor!(distributor) self.distributor = distributor save! @@ -36,10 +31,6 @@ Spree::Order.class_eval do line_item.save! end - def can_add_product_to_cart? product - DistributorChangeValidator.new(self).can_change_distributor? || product.distributors.include?(distributor) - end - def line_item_variants line_items.map{ |li| li.variant } end diff --git a/app/views/spree/checkout/_other_available_distributors.html.erb b/app/views/spree/checkout/_other_available_distributors.html.erb index 2b84986dd4..d9f4560cb7 100644 --- a/app/views/spree/checkout/_other_available_distributors.html.erb +++ b/app/views/spree/checkout/_other_available_distributors.html.erb @@ -1,14 +1,9 @@ <% unless @order.state != 'address' %>
- <% - other_available_distributors = DistributorChangeValidator.new(@order).available_distributors(Enterprise.all) - other_available_distributors.delete(@order.distributor) - unless other_available_distributors.empty? - %> + <% unless alternative_available_distributors(@order).empty? %> <%= form_for(@order) do |f| %> <%= f.label :distributor_label, "Alternative distributors for this order:" %> - <% available_distributors_array = other_available_distributors.map { |distributor| [distributor.name + ": " + distributor.address.address1 + ", " + distributor.address.city, distributor.id.to_i] } %> - <%= f.select :distributor_id, options_for_select( available_distributors_array ) %> + <%= f.select :distributor_id, options_for_select( enterprises_options(alternative_available_distributors(@order)) ) %> <%= f.submit "Change Distributor" %> <% end %> <% else %> diff --git a/app/views/spree/orders/_order_item_description.html.haml b/app/views/spree/orders/_order_item_description.html.haml index d7a9184736..d4531e8976 100644 --- a/app/views/spree/orders/_order_item_description.html.haml +++ b/app/views/spree/orders/_order_item_description.html.haml @@ -1,3 +1,3 @@ %td(data-hook = "order_item_description") %h4= item.variant.product.name - = "(" + variant_options(item.variant) + ")" unless item.variant .option_values.empty? \ No newline at end of file + = "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? \ No newline at end of file diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index 495ea77bbb..0b0e118e54 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -3,7 +3,7 @@ - if !@product.has_stock? && !Spree::Config[:allow_backorders] = content_tag('strong', t(:out_of_stock)) - - elsif !order.nil? && !order.can_add_product_to_cart?(@product) + - elsif !order.nil? && !DistributorChangeValidator.new(order).can_change_distributor? && !@product.distributors.include?(order.distributor) .error-distributor Please complete your order at = link_to current_distributor.name, root_path diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 171258579c..e3d52db8bb 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -28,38 +28,6 @@ describe Spree::Order do li.max_quantity.should == 3 end - context "permissions for adding products to the cart" do - it "allows products to be added to cart when cart is empty" do - p_first = double(:product) - p_first.stub(:distributors) { [d1] } - subject.stub(:line_items) { [] } - subject.can_add_product_to_cart?(p_first).should be_true - end - - it "allows products to be added to cart when they are available from the current distributor" do - d1 = double(:distributor) - p_first = double(:product) - p_first.stub(:distributors) { [d1] } - p_subsequent_same_dist = double(:product) - p_subsequent_same_dist.stub(:distributors) { [d1] } - subject.stub(:line_items) { [1, 2, 3] } - subject.stub(:distributor) { d1 } - subject.can_add_product_to_cart?(p_subsequent_same_dist).should be_true - end - - it "does not allow products to be added to cart when they are not available from the current distributor" do - d1 = double(:distributor) - d2 = double(:distributor) - p_first = double(:product) - p_first.stub(:distributors) { [d1] } - p_subsequent_other_dist = double(:product) - p_subsequent_other_dist.stub(:distributors) { [d2] } - subject.stub(:line_items) { [1, 2, 3] } - subject.stub(:distributor) { d1 } - subject.can_add_product_to_cart?(p_subsequent_other_dist).should be_false - end - end - context "validating distributor changes" do it "checks that a distributor is available when changing" do order_enterprise = FactoryGirl.create(:enterprise, id: 1, :name => "Order Enterprise") diff --git a/spec/requests/consumer/checkout_spec.rb b/spec/requests/consumer/checkout_spec.rb index 320dfcf281..c88f0da8ca 100644 --- a/spec/requests/consumer/checkout_spec.rb +++ b/spec/requests/consumer/checkout_spec.rb @@ -17,6 +17,16 @@ feature %q{ :state => Spree::State.find_by_name('Victoria'), :country => Spree::Country.find_by_name('Australia')), :pickup_times => 'Tuesday, 4 PM') + + + @distributor_alternative = create(:distributor_enterprise, :name => 'Alternative Distributor', + :address => create(:address, + :address1 => '1600 Rathdowne St', + :city => 'Carlton North', + :zipcode => 3054, + :state => Spree::State.find_by_name('Victoria'), + :country => Spree::Country.find_by_name('Australia')), + :pickup_times => 'Tuesday, 4 PM') @shipping_method_1 = create(:shipping_method, :name => 'Shipping Method One') @shipping_method_1.calculator.set_preference :amount, 1 @@ -28,9 +38,11 @@ feature %q{ @product_1 = create(:product, :name => 'Fuji apples') @product_1.product_distributions.create(:distributor => @distributor, :shipping_method => @shipping_method_1) + @product_1.product_distributions.create(:distributor => @distributor_alternative, :shipping_method => @shipping_method_1) @product_2 = create(:product, :name => 'Garlic') @product_2.product_distributions.create(:distributor => @distributor, :shipping_method => @shipping_method_2) + @product_2.product_distributions.create(:distributor => @distributor_alternative, :shipping_method => @shipping_method_2) @zone = create(:zone) c = Spree::Country.find_by_name('Australia') @@ -110,7 +122,9 @@ feature %q{ page.should have_content value end end - + + page.should have_selector "select#order_distributor_id option[value='#{@distributor_alternative.id}']" + click_button 'Save and Continue' # -- Checkout: Delivery diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index cd03a67def..4a747e5c0f 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -144,7 +144,6 @@ feature %q{ # When we select the distributor and view the product visit spree.select_distributor_order_path(distributor1) visit spree.product_path(product) - binding.pry # Then we should see our distributor as the default option when adding the item to our cart page.should have_selector "select#distributor_id option[value='#{distributor1.id}'][selected='selected']"