From 20bfcd6e48adeaa6de0cac2c6f968c0fcc1859e4 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 5 Sep 2014 17:11:57 +1000 Subject: [PATCH 1/8] Switching enterprise relationships form around --- .../enterprise_relationships.js.coffee | 4 ++-- .../_enterprise_relationship.html.haml | 18 ++++++++++-------- .../enterprise_relationships/_form.html.haml | 7 +++++-- .../enterprise_relationships/index.html.haml | 3 +-- .../admin/enterprise_relationships_spec.rb | 18 +++++++++--------- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index cad556efd8..16c9c38fea 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -24,5 +24,5 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris permission_presentation: (permission) -> switch permission - when "add_to_order_cycle" then "can add to order cycle" - when "manage_products" then "can manage the products of" + when "add_to_order_cycle" then "to add to order cycle" + when "manage_products" then "to manage products" diff --git a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml index 6a7dcdc7f4..45e79b1877 100644 --- a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml +++ b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml @@ -1,8 +1,10 @@ -%td {{ enterprise_relationship.child_name }} -%td - %ul - %li{"ng-repeat" => "permission in enterprise_relationship.permissions"} - {{ EnterpriseRelationships.permission_presentation(permission.name) }} -%td {{ enterprise_relationship.parent_name }} -%td.actions - %a.delete-enterprise-relationship.icon-trash.no-text{'ng-click' => 'delete(enterprise_relationship)'} +%tr{"ng-repeat" => "enterprise_relationship in EnterpriseRelationships.enterprise_relationships | filter:query"} + %td {{ enterprise_relationship.parent_name }} + %td permits + %td {{ enterprise_relationship.child_name }} + %td + %ul + %li{"ng-repeat" => "permission in enterprise_relationship.permissions"} + {{ EnterpriseRelationships.permission_presentation(permission.name) }} + %td.actions + %a.delete-enterprise-relationship.icon-trash.no-text{'ng-click' => 'delete(enterprise_relationship)'} diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index 86086c8781..9d031a57c5 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -1,4 +1,9 @@ %tr + %td + %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} + + %td + permits %td %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} %td @@ -6,8 +11,6 @@ %label %input{type: "checkbox", "ng-model" => "permissions[permission]"} {{ EnterpriseRelationships.permission_presentation(permission) }} - %td - %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} %td.actions %input{type: "button", value: "Create", "ng-click" => "create()"} .errors {{ EnterpriseRelationships.create_errors }} diff --git a/app/views/admin/enterprise_relationships/index.html.haml b/app/views/admin/enterprise_relationships/index.html.haml index e0e289efcf..0807a37825 100644 --- a/app/views/admin/enterprise_relationships/index.html.haml +++ b/app/views/admin/enterprise_relationships/index.html.haml @@ -11,5 +11,4 @@ %table#enterprise-relationships %tbody = render 'form' - %tr{"ng-repeat" => "enterprise_relationship in EnterpriseRelationships.enterprise_relationships | filter:query"} - = render 'enterprise_relationship' + = render 'enterprise_relationship' diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 7317f44066..007f663db0 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -24,10 +24,10 @@ feature %q{ # Then I should see the relationships within('table#enterprise-relationships') do - page.should have_relationship e1, e2, ['can add to order cycle'] - page.should have_relationship e2, e3, ['can manage the products of'] + page.should have_relationship e1, e2, ['to add to order cycle'] + page.should have_relationship e2, e3, ['to manage products'] page.should have_relationship e3, e4, - ['can add to order cycle', 'can manage the products of'] + ['to add to order cycle', 'to manage products'] end end @@ -39,13 +39,13 @@ feature %q{ visit admin_enterprise_relationships_path select 'One', from: 'enterprise_relationship_parent_id' - check 'can add to order cycle' - check 'can manage the products of' - uncheck 'can manage the products of' + check 'to add to order cycle' + check 'to manage products' + uncheck 'to manage products' select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' - page.should have_relationship e1, e2, ['can add to order cycle'] + page.should have_relationship e1, e2, ['to add to order cycle'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first er.should be_present er.permissions.map(&:name).should == ['add_to_order_cycle'] @@ -118,8 +118,8 @@ feature %q{ private def have_relationship(parent, child, perms=[]) - perms = perms.join(' ') || 'permits' + perms = perms.join(' ') - have_table_row [child.name, perms, parent.name, ''] + have_table_row [parent.name, 'permits', child.name, perms, ''] end end From 58dcdbd9c40abadbb0ede928e65b1ce216185619 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 5 Sep 2014 18:30:15 +1000 Subject: [PATCH 2/8] Restricting ability to change enterprise type at the controller level --- .../admin/enterprises_controller.rb | 6 ++++ .../admin/enterprises_controller_spec.rb | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 85fe6831fb..594ccc751c 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -4,6 +4,7 @@ module Admin before_filter :load_countries, :except => :index before_filter :load_methods_and_fees, :only => [:new, :edit, :update, :create] create.after :grant_management + before_filter :override_type, only: :update helper 'spree/products' include OrderCyclesHelper @@ -67,6 +68,11 @@ module Admin @enterprise_fees = EnterpriseFee.managed_by(spree_current_user).for_enterprise(@enterprise).order(:fee_type, :name).all end + def override_type + # TODO this should be done using CanCan, but our current version does not allow this level of fine grained control + params[:enterprise].delete :type unless spree_current_user.admin? + end + # Overriding method on Spree's resource controller def location_after_save if params[:enterprise].key? :producer_properties_attributes diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index 20482bef09..e52f09a5a4 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/spec/controllers/admin/enterprises_controller_spec.rb @@ -36,5 +36,34 @@ module Admin admin_user.enterprise_roles.where(enterprise_id: enterprise).should be_empty end end + + describe "updating an enterprise" do + let(:profile_enterprise) { create(:enterprise, type: 'profile') } + + context "as manager" do + it "does not allow 'type' to be changed" do + # TODO should be implemented and tested using cancan abilities, but can't do this using our current version + profile_enterprise.enterprise_roles.build(user: user).save + controller.stub spree_current_user: user + enterprise_params = { id: profile_enterprise.id, enterprise: { type: 'full' } } + + spree_put :update, enterprise_params + profile_enterprise.reload + expect(profile_enterprise.type).to eq 'profile' + end + end + + context "as super admin" do + it "allows 'type' to be changed" do + # TODO should be implemented and tested using cancan abilities, but can't do this using our current version + controller.stub spree_current_user: admin_user + enterprise_params = { id: profile_enterprise.id, enterprise: { type: 'full' } } + + spree_put :update, enterprise_params + profile_enterprise.reload + expect(profile_enterprise.type).to eq 'full' + end + end + end end end From 780df6bfe061f1deaad43c793fe48dc9ca56e8c4 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 5 Sep 2014 18:38:43 +1000 Subject: [PATCH 3/8] Hide 'profile type' form element for non super-admin users --- app/views/admin/enterprises/_form.html.haml | 37 +++++++++++---------- spec/features/admin/enterprises_spec.rb | 2 +- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/app/views/admin/enterprises/_form.html.haml b/app/views/admin/enterprises/_form.html.haml index 07be41dd6e..dbf986c7aa 100644 --- a/app/views/admin/enterprises/_form.html.haml +++ b/app/views/admin/enterprises/_form.html.haml @@ -36,24 +36,25 @@ = f.check_box :is_primary_producer, 'ng-model' => 'Enterprise.is_primary_producer'   = f.label :is_primary_producer, 'Producer' - .row - .alpha.eleven.columns - .three.columns.alpha - = f.label :type, 'Profile type' - .with-tip{'data-powertip' => "Full - enterprise may have products and relationships.
Single - enterprise may have products but no relationships.
Profile - enterprise has a profile but no products or relationships.
"} - %a What's this? - .two.columns - = f.radio_button :type, "full" -   - = f.label :type, "Full", value: "full" - .two.columns - = f.radio_button :type, "single" -   - = f.label :type, "Single", value: "single" - .four.columns.omega - = f.radio_button :type, "profile" -   - = f.label :type, "Profile", value: "profile" + - if spree_current_user.admin? + .row + .alpha.eleven.columns + .three.columns.alpha + = f.label :type, 'Profile type' + .with-tip{'data-powertip' => "Full - enterprise may have products and relationships.
Single - enterprise may have products but no relationships.
Profile - enterprise has a profile but no products or relationships.
"} + %a What's this? + .two.columns + = f.radio_button :type, "full" +   + = f.label :type, "Full", value: "full" + .two.columns + = f.radio_button :type, "single" +   + = f.label :type, "Single", value: "single" + .four.columns.omega + = f.radio_button :type, "profile" +   + = f.label :type, "Profile", value: "profile" .row .three.columns.alpha %label Visible in search? diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index da32f92681..08bae3aec5 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -127,7 +127,7 @@ feature %q{ choose 'Single' fill_in 'enterprise_description', :with => 'Connecting farmers and eaters' fill_in 'enterprise_long_description', :with => 'Zombie ipsum reversus ab viral inferno, nam rick grimes malum cerebro.' - + # Check Angularjs switching of sidebar elements uncheck 'enterprise_is_primary_producer' uncheck 'enterprise_is_distributor' From ee4a1925fe91c904138670eecbd06ce8e26da537 Mon Sep 17 00:00:00 2001 From: Rob H Date: Sat, 6 Sep 2014 00:34:27 +1000 Subject: [PATCH 4/8] Bulk Order Management works with navigation helper override --- .../spree/admin/navigation_helper_decorator.rb | 1 + spec/features/admin/bulk_order_management_spec.rb | 14 +++++++------- spec/helpers/navigation_helper_spec.rb | 4 ++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/helpers/spree/admin/navigation_helper_decorator.rb b/app/helpers/spree/admin/navigation_helper_decorator.rb index 024f467154..eb210ef482 100644 --- a/app/helpers/spree/admin/navigation_helper_decorator.rb +++ b/app/helpers/spree/admin/navigation_helper_decorator.rb @@ -8,6 +8,7 @@ module Spree klass = klass_for_without_sym_fallback(name) klass ||= name.singularize.to_sym klass = :overview if klass == :dashboard + klass = Spree::Order if klass == :bulk_order_management klass end alias_method_chain :klass_for, :sym_fallback diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index a8fe6cbda7..fe18f1a9bf 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -15,13 +15,6 @@ feature %q{ admin_user = quick_login_as_admin end - it "displays a Bulk Management Tab under the Orders item" do - visit '/admin/orders' - page.should have_link "Bulk Order Management" - click_link "Bulk Order Management" - page.should have_selector "h1.page-title", text: "Bulk Order Management" - end - it "displays a message when number of line items is zero" do visit '/admin/orders/bulk_management' page.should have_text "No orders found." @@ -586,6 +579,13 @@ feature %q{ quick_login_as @enterprise_user end + it "displays a Bulk Management Tab under the Orders item" do + visit '/admin/orders' + page.should have_link "Bulk Order Management" + click_link "Bulk Order Management" + page.should have_selector "h1.page-title", text: "Bulk Order Management" + end + it "shows only line item from orders that I distribute, and not those that I supply" do visit '/admin/orders/bulk_management' diff --git a/spec/helpers/navigation_helper_spec.rb b/spec/helpers/navigation_helper_spec.rb index 41decd6e5a..76fe9573f2 100644 --- a/spec/helpers/navigation_helper_spec.rb +++ b/spec/helpers/navigation_helper_spec.rb @@ -15,6 +15,10 @@ module Spree it "returns :overview for the dashboard" do helper.klass_for('dashboard').should == :overview end + + it "returns Spree::Order for bulk_order_management" do + helper.klass_for('bulk_order_management').should == Spree::Order + end end end end From 9dc2b248c7ef40482eccc2bf7dc609d5d113a295 Mon Sep 17 00:00:00 2001 From: Rob H Date: Sat, 6 Sep 2014 09:37:34 +1000 Subject: [PATCH 5/8] Bulk management permissions make more sense --- app/models/spree/ability_decorator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 9f9c96cb15..79019f3e8a 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -59,12 +59,12 @@ class AbilityDecorator # Enterprise User can only access orders that they are a distributor for can [:index, :create], Spree::Order - can [:read, :update, :bulk_management, :fire, :resend], Spree::Order do |order| + can [:read, :update, :fire, :resend], Spree::Order do |order| # We allow editing orders with a nil distributor as this state occurs # during the order creation process from the admin backend order.distributor.nil? || user.enterprises.include?(order.distributor) end - can [:admin], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor?) + can [:admin, :bulk_management], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor?) can [:admin, :create], Spree::LineItem can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Payment From b8fadb50ae6cbbae7178dd15654f0fc6e5d9a5b6 Mon Sep 17 00:00:00 2001 From: Rob H Date: Sat, 6 Sep 2014 12:00:27 +1000 Subject: [PATCH 6/8] Special Instructions in checkout are actually wired up --- .../checkout/checkout_controller.js.coffee | 4 ++-- .../darkswarm/services/checkout.js.coffee | 6 +++--- app/controllers/checkout_controller.rb | 6 +++--- app/views/checkout/_form.html.haml | 5 ++--- .../archive/features/consumer/checkout_spec.rb | 18 +++++++++--------- .../consumer/shopping/checkout_spec.rb | 18 +++++++++++++----- 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/checkout_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/checkout_controller.js.coffee index e77712eae6..9c953c4f4a 100644 --- a/app/assets/javascripts/darkswarm/controllers/checkout/checkout_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/checkout/checkout_controller.js.coffee @@ -6,9 +6,9 @@ Darkswarm.controller "CheckoutCtrl", ($scope, storage, Checkout, CurrentUser, Cu prefix = "order_#{Checkout.order.id}#{CurrentUser?.id}#{CurrentHub.hub.id}" for field in $scope.fieldsToBind - storage.bind $scope, "Checkout.order.#{field}", + storage.bind $scope, "Checkout.order.#{field}", storeName: "#{prefix}_#{field}" - storage.bind $scope, "Checkout.ship_address_same_as_billing", + storage.bind $scope, "Checkout.ship_address_same_as_billing", storeName: "#{prefix}_sameasbilling" defaultValue: true diff --git a/app/assets/javascripts/darkswarm/services/checkout.js.coffee b/app/assets/javascripts/darkswarm/services/checkout.js.coffee index cafd4d6718..a12ed1ae45 100644 --- a/app/assets/javascripts/darkswarm/services/checkout.js.coffee +++ b/app/assets/javascripts/darkswarm/services/checkout.js.coffee @@ -13,7 +13,7 @@ Darkswarm.factory 'Checkout', (CurrentOrder, ShippingMethods, PaymentMethods, $h Loading.clear() @errors = response.errors RailsFlashLoader.loadFlash(response.flash) - + # Rails wants our Spree::Address data to be provided with _attributes preprocess: -> munged_order = {} @@ -25,7 +25,7 @@ Darkswarm.factory 'Checkout', (CurrentOrder, ShippingMethods, PaymentMethods, $h munged_order["ship_address_attributes"] = value when "payment_method_id" munged_order["payments_attributes"] = [{payment_method_id: value}] - when "shipping_method_id", "payment_method_id", "email" + when "shipping_method_id", "payment_method_id", "email", "special_instructions" munged_order[name] = value else # Ignore everything else @@ -58,7 +58,7 @@ Darkswarm.factory 'Checkout', (CurrentOrder, ShippingMethods, PaymentMethods, $h shippingPrice: -> @shippingMethod()?.price || 0.0 - + paymentMethod: -> PaymentMethods.payment_methods_by_id[@order.payment_method_id] diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 8e2a33bd56..8e249e5a1f 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -9,7 +9,7 @@ class CheckoutController < Spree::CheckoutController include OrderCyclesHelper include EnterprisesHelper - + def edit # Because this controller doesn't inherit from our BaseController # We need to duplicate the code here @@ -56,7 +56,7 @@ class CheckoutController < Spree::CheckoutController private - + # Copied and modified from spree. Remove check for order state, since the state machine is # progressed all the way in one go with the one page checkout. def object_params @@ -94,7 +94,7 @@ class CheckoutController < Spree::CheckoutController def skip_state_validation? true end - + def load_order @order = current_order redirect_to main_app.shop_path and return unless @order and @order.checkout_allowed? diff --git a/app/views/checkout/_form.html.haml b/app/views/checkout/_form.html.haml index a64be0ac44..52f6f983cc 100644 --- a/app/views/checkout/_form.html.haml +++ b/app/views/checkout/_form.html.haml @@ -1,8 +1,8 @@ -= f_form_for current_order, url: main_app.update_checkout_path, += f_form_for current_order, html: {name: "checkout", id: "checkout_form", novalidate: true, - name: "checkout"} do |f| + "ng-submit" => "purchase($event)"} do |f| = inject_available_shipping_methods = inject_available_payment_methods @@ -20,7 +20,6 @@ = render partial: "checkout/payment", locals: {f: f} %p %button.button.primary{type: :submit, - "ng-click" => "purchase($event)", "ng-disabled" => "checkout.$invalid"} Place order now / {{ checkout.$valid }} diff --git a/spec/archive/features/consumer/checkout_spec.rb b/spec/archive/features/consumer/checkout_spec.rb index 9641085c39..74a2443b36 100644 --- a/spec/archive/features/consumer/checkout_spec.rb +++ b/spec/archive/features/consumer/checkout_spec.rb @@ -7,7 +7,7 @@ feature %q{ }, skip: true do include AuthenticationWorkflow include WebHelper - + background do set_feature_toggle :order_cycles, true @@ -24,8 +24,8 @@ 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', @@ -33,7 +33,7 @@ feature %q{ :zipcode => 3054, :state => Spree::State.find_by_name('Victoria'), :country => Spree::Country.find_by_name('Australia')), - :pickup_times => 'Tuesday, 4 PM') + :pickup_times => 'Tuesday, 4 PM') @enterprise_fee_1 = create(:enterprise_fee, :name => 'Enterprise Fee One', :calculator => Spree::Calculator::PerItem.new) @enterprise_fee_1.calculator.set_preference :amount, 1 @@ -347,16 +347,16 @@ feature %q{ login_to_consumer_section click_link 'FruitAndVeg' - visit enterprise_path @distributor1 + visit enterprise_path @distributor1 click_link 'Bananas' click_button 'Add To Cart' - visit enterprise_path @distributor1 + visit enterprise_path @distributor1 click_link 'Zucchini' click_button 'Add To Cart' find('#checkout-link').click - + # And manually visit the old checkout visit "/checkout" @@ -389,7 +389,7 @@ feature %q{ # -- Checkout: Delivery page.should have_content "DELIVERY METHOD" order_charges = page.all("tbody#summary-order-charges tr").map {|row| row.all('td').map(&:text)}.take(2) - order_charges.should == [["Distribution:", "$51.00"]] + order_charges.should == [["Distribution:", "$51.00"]] click_checkout_continue_button @@ -403,7 +403,7 @@ feature %q{ # -- Checkout: Order complete page.should have_content 'Your order has been processed successfully' page.should have_content @payment_method_distributor_oc.description - page.should have_content @distributor_oc.name + page.should have_content @distributor_oc.name page.should have_selector 'tfoot#order-charges tr.total td', text: 'Distribution' page.should have_selector 'tfoot#order-charges tr.total td', text: '51.00' diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index b26e924495..56eee61e4f 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -94,10 +94,6 @@ feature "As a consumer I want to check out my cart", js: true do describe "purchasing" do it "takes us to the order confirmation page when we submit a complete form" do - toggle_shipping - choose sm2.name - toggle_payment - choose pm1.name toggle_details within "#details" do fill_in "First Name", with: "Will" @@ -112,14 +108,25 @@ feature "As a consumer I want to check out my cart", js: true do select "Victoria", from: "State" fill_in "City", with: "Melbourne" fill_in "Postcode", with: "3066" - end + toggle_shipping + within "#shipping" do + choose sm2.name + fill_in 'Any notes or custom delivery instructions?', with: "SpEcIaL NoTeS" + end + toggle_payment + within "#payment" do + choose pm1.name + end + place_order page.should have_content "Your order has been processed successfully" ActionMailer::Base.deliveries.length.should == 2 email = ActionMailer::Base.deliveries.last site_name = Spree::Config[:site_name] email.subject.should include "#{site_name} Order Confirmation" + o = Spree::Order.complete.first + expect(o.special_instructions).to eq "SpEcIaL NoTeS" end context "with basic details filled" do @@ -157,6 +164,7 @@ feature "As a consumer I want to check out my cart", js: true do it "takes us to the order confirmation page when submitted with a valid credit card" do toggle_payment + save_screenshot '/Users/rob/Desktop/ss.png' fill_in 'Card Number', with: "4111111111111111" select 'February', from: 'secrets.card_month' select (Date.today.year+1).to_s, from: 'secrets.card_year' From 6540bb8efc84e62e32e7a2b5290274d3b1fdd5a0 Mon Sep 17 00:00:00 2001 From: Rob H Date: Sun, 7 Sep 2014 19:51:14 +1000 Subject: [PATCH 7/8] Adding select field for enterprise type to index when super admin --- .../admin/enterprises_controller.rb | 14 ++++-- app/views/admin/enterprises/index.html.haml | 8 +-- .../admin/enterprises_controller_spec.rb | 37 +++++++++++++- spec/features/admin/enterprises_spec.rb | 50 ++++++++++++------- 4 files changed, 81 insertions(+), 28 deletions(-) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 594ccc751c..ceeb26abfc 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -4,7 +4,8 @@ module Admin before_filter :load_countries, :except => :index before_filter :load_methods_and_fees, :only => [:new, :edit, :update, :create] create.after :grant_management - before_filter :override_type, only: :update + before_filter :check_type, only: :update + before_filter :check_bulk_type, only: :bulk_update helper 'spree/products' include OrderCyclesHelper @@ -68,8 +69,15 @@ module Admin @enterprise_fees = EnterpriseFee.managed_by(spree_current_user).for_enterprise(@enterprise).order(:fee_type, :name).all end - def override_type - # TODO this should be done using CanCan, but our current version does not allow this level of fine grained control + def check_bulk_type + unless spree_current_user.admin? + params[:enterprise_set][:collection_attributes].each do |i, enterprise_params| + enterprise_params.delete :type + end + end + end + + def check_type params[:enterprise].delete :type unless spree_current_user.admin? end diff --git a/app/views/admin/enterprises/index.html.haml b/app/views/admin/enterprises/index.html.haml index 7ab6ea4d0e..30dfc7a8f2 100644 --- a/app/views/admin/enterprises/index.html.haml +++ b/app/views/admin/enterprises/index.html.haml @@ -10,17 +10,17 @@ = form_for @enterprise_set, :url => main_app.bulk_update_admin_enterprises_path do |f| %table#listing_enterprises.index %colgroup - %col{style: "width: 20%;"}/ + %col{style: "width: 25%;"}/ %col{style: "width: 10%;"}/ %col{style: "width: 5%;"}/ - %col/ + %col{style: "width: 10%;"}/ %col{style: "width: 20%;"}/ %thead %tr{"data-hook" => "enterprises_header"} %th Name %th Role %th Visible? - %th Description + %th Type %th %tbody = f.fields_for :collection do |enterprise_form| @@ -37,7 +37,7 @@ - else %h1.icon-exclamation-sign.with-tip{"data-powertip" => "This enterprise does not have any roles", style: "text-align: center;color: #DA5354"} %td= enterprise_form.check_box :visible - %td= enterprise.description + %td= enterprise_form.select :type, Enterprise::TYPES, {}, class: 'select2 fullwidth' %td{"data-hook" => "admin_users_index_row_actions"} = render 'actions', enterprise: enterprise - if @enterprises.empty? diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index e52f09a5a4..14faf26287 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/spec/controllers/admin/enterprises_controller_spec.rb @@ -42,7 +42,6 @@ module Admin context "as manager" do it "does not allow 'type' to be changed" do - # TODO should be implemented and tested using cancan abilities, but can't do this using our current version profile_enterprise.enterprise_roles.build(user: user).save controller.stub spree_current_user: user enterprise_params = { id: profile_enterprise.id, enterprise: { type: 'full' } } @@ -55,7 +54,6 @@ module Admin context "as super admin" do it "allows 'type' to be changed" do - # TODO should be implemented and tested using cancan abilities, but can't do this using our current version controller.stub spree_current_user: admin_user enterprise_params = { id: profile_enterprise.id, enterprise: { type: 'full' } } @@ -65,5 +63,40 @@ module Admin end end end + + describe "bulk updating enterprises" do + let(:profile_enterprise1) { create(:enterprise, type: 'profile') } + let(:profile_enterprise2) { create(:enterprise, type: 'profile') } + + context "as manager" do + it "does not allow 'type' to be changed" do + profile_enterprise1.enterprise_roles.build(user: user).save + profile_enterprise2.enterprise_roles.build(user: user).save + controller.stub spree_current_user: user + bulk_enterprise_params = { enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, type: 'full' }, '1' => { id: profile_enterprise2.id, type: 'full' } } } } + + spree_put :bulk_update, bulk_enterprise_params + profile_enterprise1.reload + profile_enterprise2.reload + expect(profile_enterprise1.type).to eq 'profile' + expect(profile_enterprise2.type).to eq 'profile' + end + end + + context "as super admin" do + it "allows 'type' to be changed" do + profile_enterprise1.enterprise_roles.build(user: user).save + profile_enterprise2.enterprise_roles.build(user: user).save + controller.stub spree_current_user: admin_user + bulk_enterprise_params = { enterprise_set: { collection_attributes: { '0' => { id: profile_enterprise1.id, type: 'full' }, '1' => { id: profile_enterprise2.id, type: 'full' } } } } + + spree_put :bulk_update, bulk_enterprise_params + profile_enterprise1.reload + profile_enterprise2.reload + expect(profile_enterprise1.type).to eq 'full' + expect(profile_enterprise2.type).to eq 'full' + end + end + end end end diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index 08bae3aec5..00c15160f0 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -15,39 +15,43 @@ feature %q{ click_link 'Enterprises' within("tr.enterprise-#{s.id}") do - page.should have_content s.name - page.should have_content "Edit Profile" - page.should have_content "Delete" - page.should_not have_content "Payment Methods" - page.should_not have_content "Shipping Methods" - page.should have_content "Enterprise Fees" + expect(page).to have_content s.name + expect(page).to have_select "enterprise_set_collection_attributes_1_type" + expect(page).to have_content "Edit Profile" + expect(page).to have_content "Delete" + expect(page).to_not have_content "Payment Methods" + expect(page).to_not have_content "Shipping Methods" + expect(page).to have_content "Enterprise Fees" end within("tr.enterprise-#{d.id}") do - page.should have_content d.name - page.should have_content "Edit Profile" - page.should have_content "Delete" - page.should have_content "Payment Methods" - page.should have_content "Shipping Methods" - page.should have_content "Enterprise Fees" + expect(page).to have_content d.name + expect(page).to have_select "enterprise_set_collection_attributes_0_type" + expect(page).to have_content "Edit Profile" + expect(page).to have_content "Delete" + expect(page).to have_content "Payment Methods" + expect(page).to have_content "Shipping Methods" + expect(page).to have_content "Enterprise Fees" end end scenario "editing enterprises in bulk" do s = create(:supplier_enterprise) - d = create(:distributor_enterprise) + d = create(:distributor_enterprise, type: 'profile') login_to_admin_section click_link 'Enterprises' within("tr.enterprise-#{d.id}") do - page.should have_checked_field "enterprise_set_collection_attributes_0_visible" + expect(page).to have_checked_field "enterprise_set_collection_attributes_0_visible" uncheck "enterprise_set_collection_attributes_0_visible" + select 'full', from: "enterprise_set_collection_attributes_0_type" end click_button "Update" flash_message.should == 'Enterprises updated successfully' distributor = Enterprise.find(d.id) - distributor.visible.should == false + expect(distributor.visible).to eq false + expect(distributor.type).to eq 'full' end scenario "viewing an enterprise" do @@ -259,10 +263,18 @@ feature %q{ click_link "Enterprises" - page.should have_content supplier1.name - page.should have_content distributor1.name - page.should_not have_content supplier2.name - page.should_not have_content distributor2.name + within("tr.enterprise-#{supplier1.id}") do + expect(page).to have_content supplier1.name + expect(page).to have_select "enterprise_set_collection_attributes_1_type" + end + + within("tr.enterprise-#{distributor1.id}") do + expect(page).to have_content distributor1.name + expect(page).to have_select "enterprise_set_collection_attributes_0_type" + end + + expect(page).to_not have_content "supplier2.name" + expect(page).to_not have_content "distributor2.name" end scenario "creating an enterprise" do From 5fb4110328b62ff166b92a7ca7ddced9283ab943 Mon Sep 17 00:00:00 2001 From: Rob H Date: Sun, 7 Sep 2014 21:04:53 +1000 Subject: [PATCH 8/8] Adding distributor and producer checkboxes to enterprise index --- app/views/admin/enterprises/index.html.haml | 13 +++++-------- spec/features/admin/enterprises_spec.rb | 14 +++++++++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/views/admin/enterprises/index.html.haml b/app/views/admin/enterprises/index.html.haml index 30dfc7a8f2..c8b3cce3d4 100644 --- a/app/views/admin/enterprises/index.html.haml +++ b/app/views/admin/enterprises/index.html.haml @@ -28,14 +28,11 @@ %tr{class: "enterprise-#{enterprise.id}"} %td= link_to enterprise.name, main_app.edit_admin_enterprise_path(enterprise) %td - - if enterprise.is_primary_producer && enterprise.is_distributor - Producer & Distributor - - elsif enterprise.is_distributor - Distributor - - elsif enterprise.is_primary_producer - Producer - - else - %h1.icon-exclamation-sign.with-tip{"data-powertip" => "This enterprise does not have any roles", style: "text-align: center;color: #DA5354"} + = enterprise_form.check_box :is_primary_producer + Producer + %br/ + = enterprise_form.check_box :is_distributor + Hub %td= enterprise_form.check_box :visible %td= enterprise_form.select :type, Enterprise::TYPES, {}, class: 'select2 fullwidth' %td{"data-hook" => "admin_users_index_row_actions"} diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index 00c15160f0..a824716d95 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -263,16 +263,20 @@ feature %q{ click_link "Enterprises" - within("tr.enterprise-#{supplier1.id}") do - expect(page).to have_content supplier1.name - expect(page).to have_select "enterprise_set_collection_attributes_1_type" - end - within("tr.enterprise-#{distributor1.id}") do expect(page).to have_content distributor1.name + expect(page).to have_checked_field "enterprise_set_collection_attributes_0_is_distributor" + expect(page).to have_unchecked_field "enterprise_set_collection_attributes_0_is_primary_producer" expect(page).to have_select "enterprise_set_collection_attributes_0_type" end + within("tr.enterprise-#{supplier1.id}") do + expect(page).to have_content supplier1.name + expect(page).to have_unchecked_field "enterprise_set_collection_attributes_1_is_distributor" + expect(page).to have_checked_field "enterprise_set_collection_attributes_1_is_primary_producer" + expect(page).to have_select "enterprise_set_collection_attributes_1_type" + end + expect(page).to_not have_content "supplier2.name" expect(page).to_not have_content "distributor2.name" end