From 4e1eb33ff5a6a2a6174c563f15481ff7e7d98220 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Thu, 5 Mar 2015 11:25:35 +1100 Subject: [PATCH 01/13] Redirect users to proper login page when they type /login --- config/routes.rb | 2 ++ spec/features/consumer/authentication_spec.rb | 29 ++++++------------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index e7f9f2bba0..33a31023b8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,7 +1,9 @@ Openfoodnetwork::Application.routes.draw do root :to => 'home#index' + get "/#/login", to: "home#index", as: :spree_login + get "/login", to: redirect("/#/login") get "/map", to: "map#index", as: :map diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index f8b405374e..2aefb42db5 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/spec/features/consumer/authentication_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' feature "Authentication", js: true do include UIComponentHelper + describe "login" do let(:user) { create(:user, password: "password", password_confirmation: "password") } @@ -32,7 +33,7 @@ feature "Authentication", js: true do scenario "failing to login" do fill_in "Email", with: user.email click_login_button - page.should have_content "Invalid email or password" + page.should have_content "Invalid email or password" end scenario "logging in successfully" do @@ -70,7 +71,7 @@ feature "Authentication", js: true do ActionMailer::Base.deliveries.clear select_login_tab "Forgot Password?" end - + scenario "failing to reset password" do fill_in "Your email", with: "notanemail@myemail.com" click_reset_password_button @@ -78,7 +79,7 @@ feature "Authentication", js: true do end scenario "resetting password" do - fill_in "Your email", with: user.email + fill_in "Your email", with: user.email click_reset_password_button page.should have_reset_password ActionMailer::Base.deliveries.last.subject.should =~ /Password Reset/ @@ -90,29 +91,17 @@ feature "Authentication", js: true do browse_as_medium end scenario "showing login" do - open_off_canvas + open_off_canvas open_login_modal page.should have_login_modal end end end - describe "oldskool" do - scenario "with valid credentials" do - visit "/login" - fill_in "Email", with: user.email - fill_in "Password", with: "password" - click_button "Login" - current_path.should == "/" - end - - scenario "with invalid credentials" do - visit "/login" - fill_in "Email", with: user.email - fill_in "Password", with: "this isn't my password" - click_button "Login" - page.should have_content "Invalid email or password" - end + scenario "Loggin by typing login/ redirects to /#/login" do + visit "/login" + uri = URI.parse(current_url) + (uri.path + "#" + uri.fragment).should == '/#/login' end end end From 1b709a3e0370bb7db9ff8e6e3818d026fab65add Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Thu, 5 Mar 2015 16:03:48 +1100 Subject: [PATCH 02/13] Do not load Order Cycles that closed more than a month a go --- Gemfile.lock | 4 ++-- app/controllers/admin/order_cycles_controller.rb | 2 +- app/models/order_cycle.rb | 2 ++ db/schema.rb | 2 +- spec/models/order_cycle_spec.rb | 10 ++++++++++ 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 57e0877ad2..320e680184 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -325,7 +325,7 @@ GEM mail (2.5.4) mime-types (~> 1.16) treetop (~> 1.4.8) - method_source (0.8.1) + method_source (0.8.2) mime-types (1.25.1) mini_portile (0.6.2) momentjs-rails (2.5.1) @@ -509,7 +509,7 @@ GEM xml-simple (1.1.4) xpath (2.0.0) nokogiri (~> 1.3) - zeus (0.13.3) + zeus (0.15.4) method_source (>= 0.6.7) PLATFORMS diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 04a6d234c6..22a7aaa3cc 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -79,7 +79,7 @@ module Admin ocs.undated + ocs.soonest_closing + ocs.soonest_opening + - ocs.most_recently_closed + ocs.recently_closed end private diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 134af04c91..bf52d5c129 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -19,7 +19,9 @@ class OrderCycle < ActiveRecord::Base scope :undated, where(orders_open_at: nil, orders_close_at: nil) scope :soonest_closing, lambda { active.order('order_cycles.orders_close_at ASC') } + # TODO This method returns all the closed orders. So maybe we can replace it with :recently_closed. scope :most_recently_closed, lambda { closed.order('order_cycles.orders_close_at DESC') } + scope :recently_closed, lambda { where("order_cycles.orders_close_at >= ?", 31.days.ago).order("order_cycles.orders_close_at DESC") } scope :soonest_opening, lambda { upcoming.order('order_cycles.orders_open_at ASC') } scope :distributing_product, lambda { |product| diff --git a/db/schema.rb b/db/schema.rb index a8ac91e6d8..6ef4fe2ee9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -585,9 +585,9 @@ ActiveRecord::Schema.define(:version => 20150220035501) do t.string "email" t.text "special_instructions" t.integer "distributor_id" - t.integer "order_cycle_id" t.string "currency" t.string "last_ip_address" + t.integer "order_cycle_id" t.integer "cart_id" end diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 7ba2b66589..cc6035caa8 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -90,6 +90,16 @@ describe OrderCycle do end end + describe "#recently_closed" do + it "finds the orders closed in the last 30 days sorted in descending order" do + oc1 = create(:simple_order_cycle, orders_close_at: 1.day.ago) + oc2 = create(:simple_order_cycle, orders_close_at: 30.days.ago) + create(:simple_order_cycle, orders_close_at: 31.days.ago) + + OrderCycle.recently_closed.should == [oc1 , oc2] + end + end + it "finds the most recently closed order cycles" do oc1 = create(:simple_order_cycle, orders_close_at: 2.hours.ago) oc2 = create(:simple_order_cycle, orders_close_at: 1.hour.ago) From a21bfc909aa24e0290fd2077a7bca7a6a2e5aef6 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Thu, 5 Mar 2015 16:12:31 +1100 Subject: [PATCH 03/13] Remove the suppliers column on admin order cycles index page --- app/views/admin/order_cycles/index.html.haml | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/admin/order_cycles/index.html.haml b/app/views/admin/order_cycles/index.html.haml index 995cfdd2c3..afe18bc230 100644 --- a/app/views/admin/order_cycles/index.html.haml +++ b/app/views/admin/order_cycles/index.html.haml @@ -25,7 +25,6 @@ %th Open %th Close - unless order_cycles_simple_index - %th Suppliers %th Coordinator %th Distributors %th Products From d6c30ae1ef8b7c2cfa201a71c3e5c6f70908d46b Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Thu, 5 Mar 2015 16:46:51 +1100 Subject: [PATCH 04/13] OrderCycle#recently_closed doesn't return orders that are open --- app/models/order_cycle.rb | 7 ++++++- spec/models/order_cycle_spec.rb | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index bf52d5c129..adcd596a8c 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -21,7 +21,12 @@ class OrderCycle < ActiveRecord::Base scope :soonest_closing, lambda { active.order('order_cycles.orders_close_at ASC') } # TODO This method returns all the closed orders. So maybe we can replace it with :recently_closed. scope :most_recently_closed, lambda { closed.order('order_cycles.orders_close_at DESC') } - scope :recently_closed, lambda { where("order_cycles.orders_close_at >= ?", 31.days.ago).order("order_cycles.orders_close_at DESC") } + + scope :recently_closed, -> { + closed. + where("order_cycles.orders_close_at >= ?", 31.days.ago). + order("order_cycles.orders_close_at DESC") } + scope :soonest_opening, lambda { upcoming.order('order_cycles.orders_open_at ASC') } scope :distributing_product, lambda { |product| diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index cc6035caa8..817d2e8e93 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -92,6 +92,7 @@ describe OrderCycle do describe "#recently_closed" do it "finds the orders closed in the last 30 days sorted in descending order" do + create(:simple_order_cycle, orders_close_at: 3.days.from_now) oc1 = create(:simple_order_cycle, orders_close_at: 1.day.ago) oc2 = create(:simple_order_cycle, orders_close_at: 30.days.ago) create(:simple_order_cycle, orders_close_at: 31.days.ago) From 3fe1fc3f67c2df1a9975c622c03375d80f5c8ff7 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Thu, 5 Mar 2015 17:32:55 +1100 Subject: [PATCH 05/13] Use dates way into the future so that test that depend on OrderSycles#recently_closed don't break. --- spec/features/admin/order_cycles_spec.rb | 48 ++++++++++++------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index c84a6d65c7..2597417b75 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -83,8 +83,8 @@ feature %q{ # And I fill in the basic fields fill_in 'order_cycle_name', with: 'Plums & Avos' - fill_in 'order_cycle_orders_open_at', with: '2012-11-06 06:00:00' - fill_in 'order_cycle_orders_close_at', with: '2012-11-13 17:00:00' + fill_in 'order_cycle_orders_open_at', with: '2040-11-06 06:00:00' + fill_in 'order_cycle_orders_close_at', with: '2040-11-13 17:00:00' # And I add a coordinator fee click_button 'Add coordinator fee' @@ -126,8 +126,8 @@ feature %q{ page.should have_selector 'a', text: 'Plums & Avos' - page.should have_selector "input[value='2012-11-06 06:00:00 +1100']" - page.should have_selector "input[value='2012-11-13 17:00:00 +1100']" + page.should have_selector "input[value='2040-11-06 06:00:00 +1100']" + page.should have_selector "input[value='2040-11-13 17:00:00 +1100']" page.should have_content 'My coordinator' page.should have_selector 'td.suppliers', text: 'My supplier' @@ -271,8 +271,8 @@ feature %q{ # And I update it fill_in 'order_cycle_name', with: 'Plums & Avos' - fill_in 'order_cycle_orders_open_at', with: '2012-11-06 06:00:00' - fill_in 'order_cycle_orders_close_at', with: '2012-11-13 17:00:00' + fill_in 'order_cycle_orders_open_at', with: '2040-11-06 06:00:00' + fill_in 'order_cycle_orders_close_at', with: '2040-11-13 17:00:00' select 'My coordinator', from: 'order_cycle_coordinator_id' # And I configure some coordinator fees @@ -335,8 +335,8 @@ feature %q{ page.should have_selector 'a', text: 'Plums & Avos' - page.should have_selector "input[value='2012-11-06 06:00:00 +1100']" - page.should have_selector "input[value='2012-11-13 17:00:00 +1100']" + page.should have_selector "input[value='2040-11-06 06:00:00 +1100']" + page.should have_selector "input[value='2040-11-13 17:00:00 +1100']" page.should have_content 'My coordinator' page.should have_selector 'td.suppliers', text: 'My supplier' @@ -373,18 +373,18 @@ feature %q{ # And I fill in some new opening/closing times and save them within("tr.order-cycle-#{oc1.id}") do - all('input').first.set '2012-12-01 12:00:00' - all('input').last.set '2012-12-01 12:00:01' + all('input').first.set '2040-12-01 12:00:00' + all('input').last.set '2040-12-01 12:00:01' end within("tr.order-cycle-#{oc2.id}") do - all('input').first.set '2012-12-01 12:00:02' - all('input').last.set '2012-12-01 12:00:03' + all('input').first.set '2040-12-01 12:00:02' + all('input').last.set '2040-12-01 12:00:03' end within("tr.order-cycle-#{oc3.id}") do - all('input').first.set '2012-12-01 12:00:04' - all('input').last.set '2012-12-01 12:00:05' + all('input').first.set '2040-12-01 12:00:04' + all('input').last.set '2040-12-01 12:00:05' end click_button 'Update' @@ -528,8 +528,8 @@ feature %q{ save_screenshot '/Users/rob/Desktop/ss1.png' fill_in 'order_cycle_name', with: 'My order cycle' - fill_in 'order_cycle_orders_open_at', with: '2012-11-06 06:00:00' - fill_in 'order_cycle_orders_close_at', with: '2012-11-13 17:00:00' + fill_in 'order_cycle_orders_open_at', with: '2040-11-06 06:00:00' + fill_in 'order_cycle_orders_close_at', with: '2040-11-13 17:00:00' select 'Managed supplier', from: 'new_supplier_id' click_button 'Add supplier' @@ -650,8 +650,8 @@ feature %q{ # And I fill in the basic fields fill_in 'order_cycle_name', with: 'Plums & Avos' - fill_in 'order_cycle_orders_open_at', with: '2014-10-17 06:00:00' - fill_in 'order_cycle_orders_close_at', with: '2014-10-24 17:00:00' + fill_in 'order_cycle_orders_open_at', with: '2040-10-17 06:00:00' + fill_in 'order_cycle_orders_close_at', with: '2040-10-24 17:00:00' fill_in 'order_cycle_outgoing_exchange_0_pickup_time', with: 'pickup time' fill_in 'order_cycle_outgoing_exchange_0_pickup_instructions', with: 'pickup instructions' @@ -676,8 +676,8 @@ feature %q{ # Then my order cycle should have been created page.should have_content 'Your order cycle has been created.' page.should have_selector 'a', text: 'Plums & Avos' - page.should have_selector "input[value='2014-10-17 06:00:00 +1100']" - page.should have_selector "input[value='2014-10-24 17:00:00 +1100']" + page.should have_selector "input[value='2040-10-17 06:00:00 +1100']" + page.should have_selector "input[value='2040-10-24 17:00:00 +1100']" # And it should have some variants selected oc = OrderCycle.last @@ -737,8 +737,8 @@ feature %q{ # And I fill in the basic fields fill_in 'order_cycle_name', with: 'Plums & Avos' - fill_in 'order_cycle_orders_open_at', with: '2014-10-17 06:00:00' - fill_in 'order_cycle_orders_close_at', with: '2014-10-24 17:00:00' + fill_in 'order_cycle_orders_open_at', with: '2040-10-17 06:00:00' + fill_in 'order_cycle_orders_close_at', with: '2040-10-24 17:00:00' fill_in 'order_cycle_outgoing_exchange_0_pickup_time', with: 'xy' fill_in 'order_cycle_outgoing_exchange_0_pickup_instructions', with: 'zzy' @@ -759,8 +759,8 @@ feature %q{ # Then my order cycle should have been updated page.should have_content 'Your order cycle has been updated.' page.should have_selector 'a', text: 'Plums & Avos' - page.should have_selector "input[value='2014-10-17 06:00:00 +1100']" - page.should have_selector "input[value='2014-10-24 17:00:00 +1100']" + page.should have_selector "input[value='2040-10-17 06:00:00 +1100']" + page.should have_selector "input[value='2040-10-24 17:00:00 +1100']" # And it should have a variant selected oc = OrderCycle.last From 3e2142c3cf56441d595d60a7282405895c7ba822 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Fri, 6 Mar 2015 11:33:03 +1100 Subject: [PATCH 06/13] Remove the hover / pop-up over the # variants on admin order cycles index page --- app/views/admin/order_cycles/_row.html.haml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/views/admin/order_cycles/_row.html.haml b/app/views/admin/order_cycles/_row.html.haml index de18679cf6..6f1b389fd8 100644 --- a/app/views/admin/order_cycles/_row.html.haml +++ b/app/views/admin/order_cycles/_row.html.haml @@ -17,11 +17,7 @@ %br/ %td.products - - variant_images = capture do - - order_cycle.variants.each do |v| - = image_tag(v.images.first.attachment.url(:mini)) if v.images.present? - %br/ - %span.with-tip{'data-powertip' => variant_images}= "#{order_cycle.variants.count} variants" + %span= "#{order_cycle.variants.count} variants" %td.actions = link_to '', main_app.edit_admin_order_cycle_path(order_cycle), class: 'edit-order-cycle icon-edit no-text' From 2c89573441f96d8426ef788e708564294438d178 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Wed, 11 Mar 2015 11:59:25 +1100 Subject: [PATCH 07/13] Shave off a few seconds when showing order_cycles#index by loading order_cycle_enterprises only once. --- app/controllers/admin/order_cycles_controller.rb | 1 + app/views/admin/order_cycles/_row.html.haml | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 22a7aaa3cc..b629a2fd3b 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -84,6 +84,7 @@ module Admin private def load_order_cycle_set + @order_cycle_enterprises = OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises @order_cycle_set = OrderCycleSet.new :collection => collection end diff --git a/app/views/admin/order_cycles/_row.html.haml b/app/views/admin/order_cycles/_row.html.haml index 6f1b389fd8..0c66fafa1e 100644 --- a/app/views/admin/order_cycles/_row.html.haml +++ b/app/views/admin/order_cycles/_row.html.haml @@ -1,5 +1,6 @@ - order_cycle = order_cycle_form.object - klass = "order-cycle-#{order_cycle.id} #{order_cycle_status_class order_cycle}" + %tr{class: klass} %td= link_to order_cycle.name, main_app.edit_admin_order_cycle_path(order_cycle) %td= order_cycle_form.text_field :orders_open_at, :class => 'datetimepicker', :value => order_cycle.orders_open_at @@ -7,12 +8,12 @@ - unless order_cycles_simple_index %td.suppliers - - order_cycle.suppliers.merge(OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises).each do |s| + - order_cycle.suppliers.merge(@order_cycle_enterprises).each do |s| = s.name %br/ %td= order_cycle.coordinator.name %td.distributors - - order_cycle.distributors.merge(OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises).each do |d| + - order_cycle.distributors.merge(@order_cycle_enterprises).each do |d| = d.name %br/ From 208fa02ec01e49b18dcea520493b1428e70b2612 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Wed, 11 Mar 2015 13:14:45 +1100 Subject: [PATCH 08/13] Create a 'Show More' button at the top of the page, next to New Order on on admin order cycles index page --- app/controllers/admin/order_cycles_controller.rb | 11 ++++++----- app/views/admin/order_cycles/index.html.haml | 6 ++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index b629a2fd3b..435be5b38e 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -5,7 +5,7 @@ module Admin class OrderCyclesController < ResourceController include OrderCyclesHelper - before_filter :load_order_cycle_set, :only => :index + before_filter :load_data_for_index, :only => :index before_filter :require_coordinator, only: :new def show @@ -73,19 +73,20 @@ module Admin protected - def collection + def collection(show_more=false) ocs = OrderCycle.managed_by(spree_current_user) ocs.undated + ocs.soonest_closing + ocs.soonest_opening + - ocs.recently_closed + (show_more ? ocs.closed : ocs.recently_closed) end private - def load_order_cycle_set + def load_data_for_index + @show_more = !!params[:show_more] @order_cycle_enterprises = OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises - @order_cycle_set = OrderCycleSet.new :collection => collection + @order_cycle_set = OrderCycleSet.new :collection => collection(@show_more) end def require_coordinator diff --git a/app/views/admin/order_cycles/index.html.haml b/app/views/admin/order_cycles/index.html.haml index afe18bc230..2afed8ed4c 100644 --- a/app/views/admin/order_cycles/index.html.haml +++ b/app/views/admin/order_cycles/index.html.haml @@ -4,6 +4,12 @@ = content_for :page_actions do %li#new_order_cycle_link = button_link_to "New Order Cycle", main_app.new_admin_order_cycle_path, :icon => 'icon-plus', :id => 'admin_new_order_cycle_link' + - if @show_more + %li + = button_link_to "Show less", main_app.admin_order_cycles_path + - else + %li + = button_link_to "Show more", main_app.admin_order_cycles_path(params: { show_more: true }) = form_for @order_cycle_set, :url => main_app.bulk_update_admin_order_cycles_path do |f| %table.index#listing_order_cycles From f90f7565fb70754211fc12625e93f61e50911250 Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Wed, 11 Mar 2015 18:08:09 +1100 Subject: [PATCH 09/13] Allow admins to delete Order Cycles --- app/helpers/order_cycles_helper.rb | 4 ++++ app/views/admin/order_cycles/_row.html.haml | 5 ++++- spec/features/admin/order_cycles_spec.rb | 9 +++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index 6e401af0a6..fb523f2c98 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -75,6 +75,10 @@ module OrderCyclesHelper order_cycle.exchanges.to_enterprises(current_distributor).outgoing.first.pickup_time end + def can_delete?(order_cycle) + Spree::Order.where(order_cycle_id: order_cycle).none? + end + private def validated_enterprise_options(enterprises, options={}) diff --git a/app/views/admin/order_cycles/_row.html.haml b/app/views/admin/order_cycles/_row.html.haml index 0c66fafa1e..f5c8ebafbe 100644 --- a/app/views/admin/order_cycles/_row.html.haml +++ b/app/views/admin/order_cycles/_row.html.haml @@ -23,4 +23,7 @@ %td.actions = link_to '', main_app.edit_admin_order_cycle_path(order_cycle), class: 'edit-order-cycle icon-edit no-text' %td.actions - = link_to '', main_app.clone_admin_order_cycle_path(order_cycle), class: 'clone-order-cycle icon-copy no-text' + = link_to '', main_app.clone_admin_order_cycle_path(order_cycle), class: 'clone-order-cycle icon-copy no-text' + - if can_delete?(order_cycle) + %td.actions + = link_to '', main_app.admin_order_cycle_path(order_cycle), class: 'delete-product icon-trash no-text', :method => :delete, data: { confirm: "Are you sure?" } diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 2597417b75..b0a63e5b5e 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -777,6 +777,15 @@ feature %q{ end end + scenario "deleting an order cycle" do + create(:simple_order_cycle, name: "Translusent Berries") + login_to_admin_section + click_link 'Order Cycles' + page.should have_content("Translusent Berries") + first('a.delete-product').click + page.should_not have_content("Translusent Berries") + end + private From e125bcf45185877e6cb801fb376f19c754d958d7 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 16 Mar 2015 12:15:43 +1100 Subject: [PATCH 10/13] Add column header for extra action column --- app/views/admin/order_cycles/index.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/admin/order_cycles/index.html.haml b/app/views/admin/order_cycles/index.html.haml index a4ae7172a7..e39fdde12a 100644 --- a/app/views/admin/order_cycles/index.html.haml +++ b/app/views/admin/order_cycles/index.html.haml @@ -24,6 +24,7 @@ %col %col %col + %col %thead %tr @@ -37,6 +38,7 @@ %th Products %th.actions %th.actions + %th.actions %tbody = f.fields_for :collection do |order_cycle_form| From e38772ada052a134a83f763df281b8aa21f0e5a6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 16 Mar 2015 12:41:06 +1100 Subject: [PATCH 11/13] Enterprise user can delete unreferenced order cycles --- app/models/spree/ability_decorator.rb | 2 +- spec/models/spree/ability_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 145d8310de..5866a50c3f 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -132,7 +132,7 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::ReturnAuthorization can [:create], OrderCycle - can [:admin, :index, :read, :edit, :update, :bulk_update, :clone], OrderCycle do |order_cycle| + can [:admin, :index, :read, :edit, :update, :bulk_update, :clone, :destroy], OrderCycle do |order_cycle| user.enterprises.include? order_cycle.coordinator end can [:for_order_cycle], Enterprise diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 19006cbcd1..4023961eef 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -371,11 +371,11 @@ module Spree let(:oc2) { create(:simple_order_cycle) } it "should be able to read/write OrderCycles they are the co-ordinator of" do - should have_ability([:admin, :index, :read, :edit, :update, :clone], for: oc1) + should have_ability([:admin, :index, :read, :edit, :update, :clone, :destroy], for: oc1) end it "should not be able to read/write OrderCycles they are not the co-ordinator of" do - should_not have_ability([:admin, :index, :read, :create, :edit, :update, :clone], for: oc2) + should_not have_ability([:admin, :index, :read, :create, :edit, :update, :clone, :destroy], for: oc2) end it "should be able to create OrderCycles" do From 95c09315f50ef324849275d443f6e0a294cba96f Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 16 Mar 2015 12:42:05 +1100 Subject: [PATCH 12/13] Change class delete-product -> delete-order-cycle --- app/views/admin/order_cycles/_row.html.haml | 2 +- spec/features/admin/order_cycles_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/order_cycles/_row.html.haml b/app/views/admin/order_cycles/_row.html.haml index 659dc91e77..588ebdd745 100644 --- a/app/views/admin/order_cycles/_row.html.haml +++ b/app/views/admin/order_cycles/_row.html.haml @@ -36,4 +36,4 @@ = link_to '', main_app.clone_admin_order_cycle_path(order_cycle), class: 'clone-order-cycle icon-copy no-text' - if can_delete?(order_cycle) %td.actions - = link_to '', main_app.admin_order_cycle_path(order_cycle), class: 'delete-product icon-trash no-text', :method => :delete, data: { confirm: "Are you sure?" } + = link_to '', main_app.admin_order_cycle_path(order_cycle), class: 'delete-order-cycle icon-trash no-text', :method => :delete, data: { confirm: "Are you sure?" } diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 5a4036879f..4bfd0b683c 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -783,7 +783,7 @@ feature %q{ login_to_admin_section click_link 'Order Cycles' page.should have_content("Translusent Berries") - first('a.delete-product').click + first('a.delete-order-cycle').click page.should_not have_content("Translusent Berries") end From a6f0d8f69a0ec574a2bf643dc629f043a31705a5 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 16 Mar 2015 12:42:35 +1100 Subject: [PATCH 13/13] Show a nice error message instead of 500 error when deleting a referenced order cycle --- app/controllers/admin/order_cycles_controller.rb | 11 +++++++++++ .../admin/order_cycles_controller_spec.rb | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 435be5b38e..d705cc720c 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -7,6 +7,8 @@ module Admin before_filter :load_data_for_index, :only => :index before_filter :require_coordinator, only: :new + around_filter :protect_invalid_destroy, only: :destroy + def show respond_to do |format| @@ -106,5 +108,14 @@ module Admin render :set_coordinator end end + + def protect_invalid_destroy + begin + yield + rescue ActiveRecord::InvalidForeignKey + redirect_to main_app.admin_order_cycles_url + flash[:error] = "That order cycle has been selected by a customer and cannot be deleted. To prevent customers from accessing it, please close it instead." + end + end end end diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index 598059b5f4..b58884086c 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -56,5 +56,21 @@ module Admin end end end + + describe "destroy" do + let!(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } + + describe "when an order cycle becomes non-deletable, and we attempt to delete it" do + let!(:oc) { create(:simple_order_cycle, coordinator: distributor) } + let!(:order) { create(:order, order_cycle: oc) } + + before { spree_get :destroy, id: oc.id } + + it "displays an error message" do + expect(response).to redirect_to admin_order_cycles_path + expect(flash[:error]).to eq "That order cycle has been selected by a customer and cannot be deleted. To prevent customers from accessing it, please close it instead." + end + end + end end end