From bf44a1c8627257f02cff86afc9f1262e2312c052 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 4 Apr 2015 17:06:26 +0100 Subject: [PATCH 01/41] Update DB schema to store updatable weight on items sold --- .../api/line_items_controller_decorator.rb | 17 +++++++++++++++++ .../20150305004846_add_weight_to_line_items.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 app/controllers/spree/api/line_items_controller_decorator.rb create mode 100644 db/migrate/20150305004846_add_weight_to_line_items.rb diff --git a/app/controllers/spree/api/line_items_controller_decorator.rb b/app/controllers/spree/api/line_items_controller_decorator.rb new file mode 100644 index 0000000000..f9a025873d --- /dev/null +++ b/app/controllers/spree/api/line_items_controller_decorator.rb @@ -0,0 +1,17 @@ +Spree::Api::LineItemsController.class_eval do + after_filter :apply_enterprise_fees, :only => :update + + def apply_enterprise_fees + authorize! :read, order + order.update_distribution_charge! + end +end + + +#when we update a line item the .update_distribution_charge! is called +# order.should_receive .update_distribution_charge! +# check fails when absent + +# in order model check that .update_distribution_charge! is properly tested. +# think through use cases - existing completed order +# currently likely just used to complete orders so add test case that works on a completed order diff --git a/db/migrate/20150305004846_add_weight_to_line_items.rb b/db/migrate/20150305004846_add_weight_to_line_items.rb new file mode 100644 index 0000000000..a4df1a12f0 --- /dev/null +++ b/db/migrate/20150305004846_add_weight_to_line_items.rb @@ -0,0 +1,5 @@ +class AddWeightToLineItems < ActiveRecord::Migration + def change + add_column :spree_line_items, :unit_value, :decimal, :precision => 8, :scale => 2 + end +end diff --git a/db/schema.rb b/db/schema.rb index 7435fa4e30..bec31e61fc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20150225232938) do +ActiveRecord::Schema.define(:version => 20150305004846) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -532,6 +532,7 @@ ActiveRecord::Schema.define(:version => 20150225232938) do t.string "currency" t.decimal "distribution_fee", :precision => 10, :scale => 2 t.string "shipping_method_name" + t.decimal "unit_value", :precision => 8, :scale => 2 end add_index "spree_line_items", ["order_id"], :name => "index_line_items_on_order_id" From ffd850c761f16e14b7f3049c8cab8a3dca18db71 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 4 Apr 2015 17:44:02 +0100 Subject: [PATCH 02/41] Adding specs for variable weight adjustment via builk order management --- .../spree/api/line_items_controller_spec.rb | 31 +++++++++++++++++++ .../admin/bulk_order_management_spec.rb | 18 +++++++++-- .../unit/bulk_order_management_spec.js.coffee | 31 +++++++++++++++++-- 3 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 spec/controllers/spree/api/line_items_controller_spec.rb diff --git a/spec/controllers/spree/api/line_items_controller_spec.rb b/spec/controllers/spree/api/line_items_controller_spec.rb new file mode 100644 index 0000000000..e3c9a262af --- /dev/null +++ b/spec/controllers/spree/api/line_items_controller_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +module Spree + describe Spree::Api::LineItemsController do + render_views + + before do + stub_authentication! + Spree.user_class.stub :find_by_spree_api_key => current_api_user + end + + def self.make_simple_data! + let!(:order) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now) } + let!(:line_item) { FactoryGirl.create(:line_item, order: order, unit_value: 500) } + end + + #test that when a line item is updated, an order's fees are updated too + context "as an admin user" do + sign_in_as_admin! + make_simple_data! + + context "as a line item is updated" do + it "apply enterprise fees on the item" do + line_item_params = { order_id: order.number, id: line_item.id, line_item: { id: line_item.id, unit_value: 520 }, format: :json} + controller.should_receive(:apply_enterprise_fees).and_return(true) + spree_post :update, line_item_params + end + end + end + end +end diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index c8ad230a00..7aa6449c1e 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -57,7 +57,7 @@ feature %q{ end it "displays a column for order date" do - page.should have_selector "th,date", text: "ORDER DATE", :visible => true + page.should have_selector "th.date", text: "ORDER DATE", :visible => true page.should have_selector "td.date", text: o1.completed_at.strftime("%F %T"), :visible => true page.should have_selector "td.date", text: o2.completed_at.strftime("%F %T"), :visible => true end @@ -141,8 +141,22 @@ feature %q{ admin_user = quick_login_as_admin end + let!(:p1) { FactoryGirl.create(:product_with_option_types, group_buy: true, group_buy_unit_size: 5000, variant_unit: "weight", variants: [FactoryGirl.create(:variant, unit_value: 1000)] ) } + let!(:v1) { p1.variants.first } let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:li1) { FactoryGirl.create(:line_item, order: o1, :quantity => 5 ) } + let!(:li1) { FactoryGirl.create(:line_item, order: o1, variant: v1, :quantity => 5, :unit_value => 1000 ) } + + context "modifying the weight/volume of a line item" do + it "update-pending is added to variable 'price'" do + visit '/admin/orders/bulk_management' + first("div#columns_dropdown", :text => "COLUMNS").click + first("div#columns_dropdown div.menu div.menu_item", text: "Weight/Volume").click + page.should_not have_css "input[name='price'].update-pending" + li1_unit_value_column = find("tr#li_#{li1.id} td.unit_value") + li1_unit_value_column.fill_in "unit_value", :with => 1200 + page.should have_css "input[name='price'].update-pending", :visible => false + end + end context "using column display toggle" do it "shows a column display toggle button, which shows a list of columns when clicked" do diff --git a/spec/javascripts/unit/bulk_order_management_spec.js.coffee b/spec/javascripts/unit/bulk_order_management_spec.js.coffee index 35b9e13d61..76ae925785 100644 --- a/spec/javascripts/unit/bulk_order_management_spec.js.coffee +++ b/spec/javascripts/unit/bulk_order_management_spec.js.coffee @@ -33,8 +33,8 @@ describe "AdminOrderMgmtCtrl", -> httpBackend.flush() expect(scope.suppliers).toEqual [{ id : '0', name : 'All' }, 'list of suppliers'] - expect(scope.distributors).toEqual [ { id : '0', name : 'All' }, 'list of distributors' ] - expect(scope.orderCycles).toEqual [ { id : '0', name : 'All' }, 'oc1', 'oc2', 'oc3' ] + expect(scope.distributors).toEqual [ { id : '0', name : 'All' }, 'list of distributors' ] + expect(scope.orderCycles).toEqual [ { id : '0', name : 'All' }, 'oc1', 'oc2', 'oc3' ] expect(scope.initialiseVariables.calls.length).toBe 1 expect(scope.fetchOrders.calls.length).toBe 1 @@ -350,6 +350,33 @@ describe "AdminOrderMgmtCtrl", -> spyOn(VariantUnitManager, "getUnitName").andReturn "kg" expect(scope.formattedValueWithUnitName(2000,unitsVariant)).toEqual "2 kg" + describe "updating the price upon updating the weight of a line item", -> + + it "resets the weight if the weight is set to zero", -> + scope.filteredLineItems = [ + { units_variant: { unit_value: 100 }, price: 2, unit_value: 0 } + ] + expect(scope.weightAdjustedPrice(scope.filteredLineItems[0], 100)).toEqual scope.filteredLineItems[0].price + + it "updates the price if the weight is changed", -> + scope.filteredLineItems = [ + { units_variant: { unit_value: 100 }, price: 2, unit_value: 200 } + ] + old_value = scope.filteredLineItems[0].units_variant.unit_value + new_value = scope.filteredLineItems[0].unit_value + sp = scope.filteredLineItems[0].price * new_value / old_value + expect(scope.weightAdjustedPrice(scope.filteredLineItems[0], old_value)).toEqual sp + + it "doesn't update the price if the weight is not changed", -> + scope.filteredLineItems = [ + { units_variant: { unit_value: 100 }, price: 2, unit_value: 100 } + ] + old_value = scope.filteredLineItems[0].unit_value + new_value = scope.filteredLineItems[0].unit_value + sp = scope.filteredLineItems[0].price + expect(scope.weightAdjustedPrice(scope.filteredLineItems[0], old_value)).toEqual sp + + describe "managing pending changes", -> dataSubmitter = pendingChangesService = null From ff935af18bd4a2a459e945dafa1e35642ac5d198 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 4 Apr 2015 17:52:31 +0100 Subject: [PATCH 03/41] Variable Weights: Adding ability to update the weight/volume of a line_item after checkout. The price of the line_item is automatically updated to reflect the value of the new weight. --- .../admin/bulk_order_management.js.coffee | 9 +- .../directives/line_item_upd_attr.js.coffee | 6 +- app/models/spree/line_item_decorator.rb | 3 +- .../admin/orders/bulk_management.html.haml | 107 ++++++++++-------- .../spree/api/line_items/bulk_show.v1.rabl | 6 +- 5 files changed, 75 insertions(+), 56 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 4c1a319c1a..a20cdd27e7 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -32,7 +32,8 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ variant: { name: "Variant", visible: true } quantity: { name: "Quantity", visible: true } max: { name: "Max", visible: true } - + unit_value: { name: "Weight/Volume", visible: false } + price: { name: "Price", visible: false } $scope.initialise = -> $scope.initialiseVariables() authorise_api_reponse = "" @@ -162,6 +163,12 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.supplierFilter = $scope.suppliers[0].id $scope.orderCycleFilter = $scope.orderCycles[0].id $scope.quickSearch = "" + + $scope.weightAdjustedPrice = (lineItem, oldValue) -> + if lineItem.unit_value <= 0 + lineItem.unit_value = lineItem.units_variant.unit_value + lineItem.price = lineItem.price * lineItem.unit_value / oldValue + #$scope.bulk_order_form.line_item.price.$setViewValue($scope.bulk_order_form.line_item.price.$viewValue) ] daysFromToday = (days) -> diff --git a/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee b/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee index c83d7fdc0f..c5afce07a5 100644 --- a/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee +++ b/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee @@ -8,7 +8,9 @@ angular.module("ofn.admin").directive "ofnLineItemUpdAttr", [ scope.$watch -> scope.$eval(attrs.ngModel) , (value) -> - if ngModel.$dirty + #if ngModel.$dirty + # i think i can take this out, this directive is still only called + # on a change and only an updated value will create a db call. if value == element.dbValue pendingChanges.remove(scope.line_item.id, attrName) switchClass( element, "", ["update-pending", "update-error", "update-success"], false ) @@ -20,4 +22,4 @@ angular.module("ofn.admin").directive "ofnLineItemUpdAttr", [ url: "/api/orders/#{scope.line_item.order.number}/line_items/#{scope.line_item.id}?line_item[#{attrName}]=#{value}" pendingChanges.add(scope.line_item.id, attrName, changeObj) switchClass( element, "update-pending", ["update-error", "update-success"], false ) -] \ No newline at end of file +] diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index b2c0a583fc..4eec0bcd2b 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -1,5 +1,6 @@ Spree::LineItem.class_eval do - attr_accessible :max_quantity + attr_accessible :max_quantity, :unit_value + attr_accessible :unit_value, :price, :as => :api # -- Scopes scope :managed_by, lambda { |user| diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index 61bf9fbb7e..a1f53826e1 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -100,53 +100,60 @@ %div{ :class => "sixteen columns alpha", 'ng-show' => '!loading && filteredLineItems.length == 0'} %h1#no_results No orders found. %div{ 'ng-hide' => 'loading || filteredLineItems.length == 0' } - %table.index#listing_orders.bulk{ :class => "sixteen columns alpha" } - %thead - %tr - %th.bulk - %input{ :type => "checkbox", :name => 'toggle_bulk', 'ng-click' => 'toggleAllCheckboxes()', 'ng-checked' => "allBoxesChecked()" } - %th.order_no{ 'ng-show' => 'columns.order_no.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.number'; reverse = !reverse" } Order No. - %th.full_name{ 'ng-show' => 'columns.full_name.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.full_name'; reverse = !reverse" } Name - %th.email{ 'ng-show' => 'columns.email.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.email'; reverse = !reverse" } Email - %th.phone{ 'ng-show' => 'columns.phone.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.phone'; reverse = !reverse" } Phone - %th.date{ 'ng-show' => 'columns.order_date.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.completed_at'; reverse = !reverse" } Order Date - %th.producer{ 'ng-show' => 'columns.producer.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'supplier.name'; reverse = !reverse" } Producer - %th.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.order_cycle.name'; reverse = !reverse" } Order Cycle - %th.hub{ 'ng-show' => 'columns.hub.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.distributor.name'; reverse = !reverse" } Hub - %th.variant{ 'ng-show' => 'columns.variant.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'units_variant.unit_text'; reverse = !reverse" } Product: Unit - %th.quantity{ 'ng-show' => 'columns.quantity.visible' } Quantity - %th.max{ 'ng-show' => 'columns.max.visible' } Max - %th.actions - %th.actions - Ask?  - %input{ :type => 'checkbox', 'ng-model' => "confirmDelete" } - %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:predicate:reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", :id => "li_{{line_item.id}}" } - %td.bulk - %input{ :type => "checkbox", :name => 'bulk', 'ng-model' => 'line_item.checked' } - %td.order_no{ 'ng-show' => 'columns.order_no.visible' } {{ line_item.order.number }} - %td.full_name{ 'ng-show' => 'columns.full_name.visible' } {{ line_item.order.full_name }} - %td.email{ 'ng-show' => 'columns.email.visible' } {{ line_item.order.email }} - %td.phone{ 'ng-show' => 'columns.phone.visible' } {{ line_item.order.phone }} - %td.date{ 'ng-show' => 'columns.order_date.visible' } {{ line_item.order.completed_at }} - %td.producer{ 'ng-show' => 'columns.producer.visible' } {{ line_item.supplier.name }} - %td.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } {{ line_item.order.order_cycle.name }} - %td.hub{ 'ng-show' => 'columns.hub.visible' } {{ line_item.order.distributor.name }} - %td.variant{ 'ng-show' => 'columns.variant.visible' } - %a{ :href => '#', 'ng-click' => "setSelectedUnitsVariant(line_item.units_product,line_item.units_variant)" } {{ line_item.units_variant.unit_text }} - %td.quantity{ 'ng-show' => 'columns.quantity.visible' } - %input{ :type => 'number', :name => 'quantity', 'ng-model' => "line_item.quantity", 'ofn-line-item-upd-attr' => "quantity" } - %td.max{ 'ng-show' => 'columns.max.visible' } {{ line_item.max_quantity }} - %td.actions - %a{ :class => "edit-order icon-edit no-text", 'ofn-confirm-link-path' => "/admin/orders/{{line_item.order.number}}/edit" } - %td.actions - %a{ 'ng-click' => "deleteLineItem(line_item)", :class => "delete-line-item icon-trash no-text" } - %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'pendingChanges.submitAll()' } \ No newline at end of file + %form{ 'ng-model' => "bulk_order_form" } + %table.index#listing_orders.bulk{ :class => "sixteen columns alpha" } + %thead + %tr + %th.bulk + %input{ :type => "checkbox", :name => 'toggle_bulk', 'ng-click' => 'toggleAllCheckboxes()', 'ng-checked' => "allBoxesChecked()" } + %th.order_no{ 'ng-show' => 'columns.order_no.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.number'; reverse = !reverse" } Order No. + %th.full_name{ 'ng-show' => 'columns.full_name.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.full_name'; reverse = !reverse" } Name + %th.email{ 'ng-show' => 'columns.email.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.email'; reverse = !reverse" } Email + %th.phone{ 'ng-show' => 'columns.phone.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.phone'; reverse = !reverse" } Phone + %th.date{ 'ng-show' => 'columns.order_date.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.completed_at'; reverse = !reverse" } Order Date + %th.producer{ 'ng-show' => 'columns.producer.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'supplier.name'; reverse = !reverse" } Producer + %th.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.order_cycle.name'; reverse = !reverse" } Order Cycle + %th.hub{ 'ng-show' => 'columns.hub.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.distributor.name'; reverse = !reverse" } Hub + %th.variant{ 'ng-show' => 'columns.variant.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'units_variant.unit_text'; reverse = !reverse" } Product: Unit + %th.quantity{ 'ng-show' => 'columns.quantity.visible' } Quantity + %th.max{ 'ng-show' => 'columns.max.visible' } Max + %th.unit_value{ 'ng-show' => 'columns.unit_value.visible' } Weight/Volume + %th.price{ 'ng-show' => 'columns.price.visible' } Price + %th.actions + %th.actions + Ask?  + %input{ :type => 'checkbox', 'ng-model' => "confirmDelete" } + %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:predicate:reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", :id => "li_{{line_item.id}}" } + %td.bulk + %input{ :type => "checkbox", :name => 'bulk', 'ng-model' => 'line_item.checked' } + %td.order_no{ 'ng-show' => 'columns.order_no.visible' } {{ line_item.order.number }} + %td.full_name{ 'ng-show' => 'columns.full_name.visible' } {{ line_item.order.full_name }} + %td.email{ 'ng-show' => 'columns.email.visible' } {{ line_item.order.email }} + %td.phone{ 'ng-show' => 'columns.phone.visible' } {{ line_item.order.phone }} + %td.date{ 'ng-show' => 'columns.order_date.visible' } {{ line_item.order.completed_at }} + %td.producer{ 'ng-show' => 'columns.producer.visible' } {{ line_item.supplier.name }} + %td.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } {{ line_item.order.order_cycle.name }} + %td.hub{ 'ng-show' => 'columns.hub.visible' } {{ line_item.order.distributor.name }} + %td.variant{ 'ng-show' => 'columns.variant.visible' } + %a{ :href => '#', 'ng-click' => "setSelectedUnitsVariant(line_item.units_product,line_item.units_variant)" } {{ line_item.units_variant.unit_text }} + %td.quantity{ 'ng-show' => 'columns.quantity.visible' } + %input{ :type => 'number', :name => 'quantity', 'ng-model' => "line_item.quantity", 'ofn-line-item-upd-attr' => "quantity" } + %td.max{ 'ng-show' => 'columns.max.visible' } {{ line_item.max_quantity }} + %td.unit_value{ 'ng-show' => 'columns.unit_value.visible' } + %input{ :type => 'number', :name => 'unit_value', :id => 'unit_value', 'ng-model' => "line_item.unit_value", 'ng-change' => "weightAdjustedPrice(line_item, {{ line_item.unit_value }})", 'ofn-line-item-upd-attr' => "unit_value" } + %td.price{ 'ng-show' => 'columns.price.visible' } + %input{ :type => 'text', :name => 'price', :id => 'price', :value => '{{ line_item.price | currency }}', 'ng-model' => "line_item.price", 'ng-readonly' => "true", 'ofn-line-item-upd-attr' => "price" } + %td.actions + %a{ :class => "edit-order icon-edit no-text", 'ofn-confirm-link-path' => "/admin/orders/{{line_item.order.number}}/edit" } + %td.actions + %a{ 'ng-click' => "deleteLineItem(line_item)", :class => "delete-line-item icon-trash no-text" } + %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'pendingChanges.submitAll()' } diff --git a/app/views/spree/api/line_items/bulk_show.v1.rabl b/app/views/spree/api/line_items/bulk_show.v1.rabl index 8b53c42086..6b24ef4bf2 100644 --- a/app/views/spree/api/line_items/bulk_show.v1.rabl +++ b/app/views/spree/api/line_items/bulk_show.v1.rabl @@ -1,5 +1,7 @@ object @line_item -attributes :id, :quantity, :max_quantity +attributes :id, :quantity, :max_quantity, :price node( :supplier ) { |li| partial 'api/enterprises/bulk_show', :object => li.product.supplier } node( :units_product ) { |li| partial 'spree/api/products/units_show', :object => li.product } -node( :units_variant ) { |li| partial 'spree/api/variants/units_show', :object => li.variant } \ No newline at end of file +node( :units_variant ) { |li| partial 'spree/api/variants/units_show', :object => li.variant } +node( :unit_value ) { |li| li.unit_value.to_f } +node( :price ) { |li| li.price } From 83981fbb15e254cacf5c9cacefe0cec4d0f9f787 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Tue, 14 Apr 2015 17:29:56 +0100 Subject: [PATCH 04/41] Adding additional logic for if the line_item unit_value is nil --- app/assets/javascripts/admin/bulk_order_management.js.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index a20cdd27e7..a52b770831 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -165,6 +165,8 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.quickSearch = "" $scope.weightAdjustedPrice = (lineItem, oldValue) -> + if oldValue <= 0 + oldValue = lineItem.units_variant.unit_value if lineItem.unit_value <= 0 lineItem.unit_value = lineItem.units_variant.unit_value lineItem.price = lineItem.price * lineItem.unit_value / oldValue From 162a5651401d2a8288123e4a807e3d576218017a Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Fri, 24 Apr 2015 16:14:24 +0100 Subject: [PATCH 05/41] Removing notes to myself from this file --- .../spree/api/line_items_controller_decorator.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/controllers/spree/api/line_items_controller_decorator.rb b/app/controllers/spree/api/line_items_controller_decorator.rb index f9a025873d..35fca864f4 100644 --- a/app/controllers/spree/api/line_items_controller_decorator.rb +++ b/app/controllers/spree/api/line_items_controller_decorator.rb @@ -6,12 +6,3 @@ Spree::Api::LineItemsController.class_eval do order.update_distribution_charge! end end - - -#when we update a line item the .update_distribution_charge! is called -# order.should_receive .update_distribution_charge! -# check fails when absent - -# in order model check that .update_distribution_charge! is properly tested. -# think through use cases - existing completed order -# currently likely just used to complete orders so add test case that works on a completed order From 60452835497ee52b36c1264e4b214837f8390ffb Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Fri, 24 Apr 2015 16:17:00 +0100 Subject: [PATCH 06/41] Populate the line item unit value, when line_item created and update old data in migration --- app/models/spree/order_decorator.rb | 1 + .../20150424151117_populate_line_item_unit_value.rb | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 db/migrate/20150424151117_populate_line_item_unit_value.rb diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 9d213bd633..a7158096dd 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -127,6 +127,7 @@ Spree::Order.class_eval do else current_item = Spree::LineItem.new(:quantity => quantity, max_quantity: max_quantity) current_item.variant = variant + current_item.unit_value = variant.unit_value if currency current_item.currency = currency unless currency.nil? current_item.price = variant.price_in(currency).amount diff --git a/db/migrate/20150424151117_populate_line_item_unit_value.rb b/db/migrate/20150424151117_populate_line_item_unit_value.rb new file mode 100644 index 0000000000..2122b84472 --- /dev/null +++ b/db/migrate/20150424151117_populate_line_item_unit_value.rb @@ -0,0 +1,9 @@ +class PopulateLineItemUnitValue < ActiveRecord::Migration + def up + execute "UPDATE spree_line_items SET unit_value = spree_variants.unit_value FROM spree_variants WHERE spree_line_items.variant_id = spree_variants.id" + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end From d344c3dec8da637abe28753a61509077e6884ed7 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Fri, 24 Apr 2015 16:22:17 +0100 Subject: [PATCH 07/41] Updating the spec based on @Robs suggestions, hoping for his insights. Still doesn't work. --- spec/controllers/spree/api/line_items_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/spree/api/line_items_controller_spec.rb b/spec/controllers/spree/api/line_items_controller_spec.rb index e3c9a262af..7d0b215ae9 100644 --- a/spec/controllers/spree/api/line_items_controller_spec.rb +++ b/spec/controllers/spree/api/line_items_controller_spec.rb @@ -20,9 +20,9 @@ module Spree make_simple_data! context "as a line item is updated" do - it "apply enterprise fees on the item" do + it "update distribution charge on the order" do line_item_params = { order_id: order.number, id: line_item.id, line_item: { id: line_item.id, unit_value: 520 }, format: :json} - controller.should_receive(:apply_enterprise_fees).and_return(true) + order.should_receive(:update_distribution_charge!).and_call_original spree_post :update, line_item_params end end From a253b8852517d327c9b6fa00c64edb041b619c47 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 29 Apr 2015 10:42:38 +1000 Subject: [PATCH 08/41] Fixing line item controller spec --- spec/controllers/spree/api/line_items_controller_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/controllers/spree/api/line_items_controller_spec.rb b/spec/controllers/spree/api/line_items_controller_spec.rb index 7d0b215ae9..37ec50eb7e 100644 --- a/spec/controllers/spree/api/line_items_controller_spec.rb +++ b/spec/controllers/spree/api/line_items_controller_spec.rb @@ -22,7 +22,8 @@ module Spree context "as a line item is updated" do it "update distribution charge on the order" do line_item_params = { order_id: order.number, id: line_item.id, line_item: { id: line_item.id, unit_value: 520 }, format: :json} - order.should_receive(:update_distribution_charge!).and_call_original + allow(controller).to receive(:order) { order } + expect(order).to receive(:update_distribution_charge!) spree_post :update, line_item_params end end From f7ade48e8655c282d765f06ed47dc865b142bbe0 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 4 Apr 2015 17:06:26 +0100 Subject: [PATCH 09/41] Update DB schema to store updatable weight on items sold --- .../api/line_items_controller_decorator.rb | 17 +++++++++++++++++ .../20150305004846_add_weight_to_line_items.rb | 5 +++++ db/schema.rb | 1 + 3 files changed, 23 insertions(+) create mode 100644 app/controllers/spree/api/line_items_controller_decorator.rb create mode 100644 db/migrate/20150305004846_add_weight_to_line_items.rb diff --git a/app/controllers/spree/api/line_items_controller_decorator.rb b/app/controllers/spree/api/line_items_controller_decorator.rb new file mode 100644 index 0000000000..f9a025873d --- /dev/null +++ b/app/controllers/spree/api/line_items_controller_decorator.rb @@ -0,0 +1,17 @@ +Spree::Api::LineItemsController.class_eval do + after_filter :apply_enterprise_fees, :only => :update + + def apply_enterprise_fees + authorize! :read, order + order.update_distribution_charge! + end +end + + +#when we update a line item the .update_distribution_charge! is called +# order.should_receive .update_distribution_charge! +# check fails when absent + +# in order model check that .update_distribution_charge! is properly tested. +# think through use cases - existing completed order +# currently likely just used to complete orders so add test case that works on a completed order diff --git a/db/migrate/20150305004846_add_weight_to_line_items.rb b/db/migrate/20150305004846_add_weight_to_line_items.rb new file mode 100644 index 0000000000..a4df1a12f0 --- /dev/null +++ b/db/migrate/20150305004846_add_weight_to_line_items.rb @@ -0,0 +1,5 @@ +class AddWeightToLineItems < ActiveRecord::Migration + def change + add_column :spree_line_items, :unit_value, :decimal, :precision => 8, :scale => 2 + end +end diff --git a/db/schema.rb b/db/schema.rb index cb8b503583..96f5896c79 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -549,6 +549,7 @@ ActiveRecord::Schema.define(:version => 20150424025907) do t.string "currency" t.decimal "distribution_fee", :precision => 10, :scale => 2 t.string "shipping_method_name" + t.decimal "unit_value", :precision => 8, :scale => 2 end add_index "spree_line_items", ["order_id"], :name => "index_line_items_on_order_id" From 2f463474fba9f1ce9c2b01e1bb7940369482a8d9 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 4 Apr 2015 17:44:02 +0100 Subject: [PATCH 10/41] Adding specs for variable weight adjustment via builk order management --- .../spree/api/line_items_controller_spec.rb | 31 +++++++++++++++++++ .../admin/bulk_order_management_spec.rb | 18 +++++++++-- .../unit/bulk_order_management_spec.js.coffee | 31 +++++++++++++++++-- 3 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 spec/controllers/spree/api/line_items_controller_spec.rb diff --git a/spec/controllers/spree/api/line_items_controller_spec.rb b/spec/controllers/spree/api/line_items_controller_spec.rb new file mode 100644 index 0000000000..e3c9a262af --- /dev/null +++ b/spec/controllers/spree/api/line_items_controller_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +module Spree + describe Spree::Api::LineItemsController do + render_views + + before do + stub_authentication! + Spree.user_class.stub :find_by_spree_api_key => current_api_user + end + + def self.make_simple_data! + let!(:order) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now) } + let!(:line_item) { FactoryGirl.create(:line_item, order: order, unit_value: 500) } + end + + #test that when a line item is updated, an order's fees are updated too + context "as an admin user" do + sign_in_as_admin! + make_simple_data! + + context "as a line item is updated" do + it "apply enterprise fees on the item" do + line_item_params = { order_id: order.number, id: line_item.id, line_item: { id: line_item.id, unit_value: 520 }, format: :json} + controller.should_receive(:apply_enterprise_fees).and_return(true) + spree_post :update, line_item_params + end + end + end + end +end diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index c8ad230a00..7aa6449c1e 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -57,7 +57,7 @@ feature %q{ end it "displays a column for order date" do - page.should have_selector "th,date", text: "ORDER DATE", :visible => true + page.should have_selector "th.date", text: "ORDER DATE", :visible => true page.should have_selector "td.date", text: o1.completed_at.strftime("%F %T"), :visible => true page.should have_selector "td.date", text: o2.completed_at.strftime("%F %T"), :visible => true end @@ -141,8 +141,22 @@ feature %q{ admin_user = quick_login_as_admin end + let!(:p1) { FactoryGirl.create(:product_with_option_types, group_buy: true, group_buy_unit_size: 5000, variant_unit: "weight", variants: [FactoryGirl.create(:variant, unit_value: 1000)] ) } + let!(:v1) { p1.variants.first } let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:li1) { FactoryGirl.create(:line_item, order: o1, :quantity => 5 ) } + let!(:li1) { FactoryGirl.create(:line_item, order: o1, variant: v1, :quantity => 5, :unit_value => 1000 ) } + + context "modifying the weight/volume of a line item" do + it "update-pending is added to variable 'price'" do + visit '/admin/orders/bulk_management' + first("div#columns_dropdown", :text => "COLUMNS").click + first("div#columns_dropdown div.menu div.menu_item", text: "Weight/Volume").click + page.should_not have_css "input[name='price'].update-pending" + li1_unit_value_column = find("tr#li_#{li1.id} td.unit_value") + li1_unit_value_column.fill_in "unit_value", :with => 1200 + page.should have_css "input[name='price'].update-pending", :visible => false + end + end context "using column display toggle" do it "shows a column display toggle button, which shows a list of columns when clicked" do diff --git a/spec/javascripts/unit/bulk_order_management_spec.js.coffee b/spec/javascripts/unit/bulk_order_management_spec.js.coffee index 35b9e13d61..76ae925785 100644 --- a/spec/javascripts/unit/bulk_order_management_spec.js.coffee +++ b/spec/javascripts/unit/bulk_order_management_spec.js.coffee @@ -33,8 +33,8 @@ describe "AdminOrderMgmtCtrl", -> httpBackend.flush() expect(scope.suppliers).toEqual [{ id : '0', name : 'All' }, 'list of suppliers'] - expect(scope.distributors).toEqual [ { id : '0', name : 'All' }, 'list of distributors' ] - expect(scope.orderCycles).toEqual [ { id : '0', name : 'All' }, 'oc1', 'oc2', 'oc3' ] + expect(scope.distributors).toEqual [ { id : '0', name : 'All' }, 'list of distributors' ] + expect(scope.orderCycles).toEqual [ { id : '0', name : 'All' }, 'oc1', 'oc2', 'oc3' ] expect(scope.initialiseVariables.calls.length).toBe 1 expect(scope.fetchOrders.calls.length).toBe 1 @@ -350,6 +350,33 @@ describe "AdminOrderMgmtCtrl", -> spyOn(VariantUnitManager, "getUnitName").andReturn "kg" expect(scope.formattedValueWithUnitName(2000,unitsVariant)).toEqual "2 kg" + describe "updating the price upon updating the weight of a line item", -> + + it "resets the weight if the weight is set to zero", -> + scope.filteredLineItems = [ + { units_variant: { unit_value: 100 }, price: 2, unit_value: 0 } + ] + expect(scope.weightAdjustedPrice(scope.filteredLineItems[0], 100)).toEqual scope.filteredLineItems[0].price + + it "updates the price if the weight is changed", -> + scope.filteredLineItems = [ + { units_variant: { unit_value: 100 }, price: 2, unit_value: 200 } + ] + old_value = scope.filteredLineItems[0].units_variant.unit_value + new_value = scope.filteredLineItems[0].unit_value + sp = scope.filteredLineItems[0].price * new_value / old_value + expect(scope.weightAdjustedPrice(scope.filteredLineItems[0], old_value)).toEqual sp + + it "doesn't update the price if the weight is not changed", -> + scope.filteredLineItems = [ + { units_variant: { unit_value: 100 }, price: 2, unit_value: 100 } + ] + old_value = scope.filteredLineItems[0].unit_value + new_value = scope.filteredLineItems[0].unit_value + sp = scope.filteredLineItems[0].price + expect(scope.weightAdjustedPrice(scope.filteredLineItems[0], old_value)).toEqual sp + + describe "managing pending changes", -> dataSubmitter = pendingChangesService = null From 2a991ad1300fda8dcf17fca2b9d6277f5d44c1ea Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 4 Apr 2015 17:52:31 +0100 Subject: [PATCH 11/41] Variable Weights: Adding ability to update the weight/volume of a line_item after checkout. The price of the line_item is automatically updated to reflect the value of the new weight. --- .../admin/bulk_order_management.js.coffee | 9 +- .../directives/line_item_upd_attr.js.coffee | 6 +- app/models/spree/line_item_decorator.rb | 3 +- .../admin/orders/bulk_management.html.haml | 107 ++++++++++-------- .../spree/api/line_items/bulk_show.v1.rabl | 6 +- 5 files changed, 75 insertions(+), 56 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 4c1a319c1a..a20cdd27e7 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -32,7 +32,8 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ variant: { name: "Variant", visible: true } quantity: { name: "Quantity", visible: true } max: { name: "Max", visible: true } - + unit_value: { name: "Weight/Volume", visible: false } + price: { name: "Price", visible: false } $scope.initialise = -> $scope.initialiseVariables() authorise_api_reponse = "" @@ -162,6 +163,12 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.supplierFilter = $scope.suppliers[0].id $scope.orderCycleFilter = $scope.orderCycles[0].id $scope.quickSearch = "" + + $scope.weightAdjustedPrice = (lineItem, oldValue) -> + if lineItem.unit_value <= 0 + lineItem.unit_value = lineItem.units_variant.unit_value + lineItem.price = lineItem.price * lineItem.unit_value / oldValue + #$scope.bulk_order_form.line_item.price.$setViewValue($scope.bulk_order_form.line_item.price.$viewValue) ] daysFromToday = (days) -> diff --git a/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee b/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee index c83d7fdc0f..c5afce07a5 100644 --- a/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee +++ b/app/assets/javascripts/admin/directives/line_item_upd_attr.js.coffee @@ -8,7 +8,9 @@ angular.module("ofn.admin").directive "ofnLineItemUpdAttr", [ scope.$watch -> scope.$eval(attrs.ngModel) , (value) -> - if ngModel.$dirty + #if ngModel.$dirty + # i think i can take this out, this directive is still only called + # on a change and only an updated value will create a db call. if value == element.dbValue pendingChanges.remove(scope.line_item.id, attrName) switchClass( element, "", ["update-pending", "update-error", "update-success"], false ) @@ -20,4 +22,4 @@ angular.module("ofn.admin").directive "ofnLineItemUpdAttr", [ url: "/api/orders/#{scope.line_item.order.number}/line_items/#{scope.line_item.id}?line_item[#{attrName}]=#{value}" pendingChanges.add(scope.line_item.id, attrName, changeObj) switchClass( element, "update-pending", ["update-error", "update-success"], false ) -] \ No newline at end of file +] diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index b2c0a583fc..4eec0bcd2b 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -1,5 +1,6 @@ Spree::LineItem.class_eval do - attr_accessible :max_quantity + attr_accessible :max_quantity, :unit_value + attr_accessible :unit_value, :price, :as => :api # -- Scopes scope :managed_by, lambda { |user| diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index 61bf9fbb7e..a1f53826e1 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -100,53 +100,60 @@ %div{ :class => "sixteen columns alpha", 'ng-show' => '!loading && filteredLineItems.length == 0'} %h1#no_results No orders found. %div{ 'ng-hide' => 'loading || filteredLineItems.length == 0' } - %table.index#listing_orders.bulk{ :class => "sixteen columns alpha" } - %thead - %tr - %th.bulk - %input{ :type => "checkbox", :name => 'toggle_bulk', 'ng-click' => 'toggleAllCheckboxes()', 'ng-checked' => "allBoxesChecked()" } - %th.order_no{ 'ng-show' => 'columns.order_no.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.number'; reverse = !reverse" } Order No. - %th.full_name{ 'ng-show' => 'columns.full_name.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.full_name'; reverse = !reverse" } Name - %th.email{ 'ng-show' => 'columns.email.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.email'; reverse = !reverse" } Email - %th.phone{ 'ng-show' => 'columns.phone.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.phone'; reverse = !reverse" } Phone - %th.date{ 'ng-show' => 'columns.order_date.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.completed_at'; reverse = !reverse" } Order Date - %th.producer{ 'ng-show' => 'columns.producer.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'supplier.name'; reverse = !reverse" } Producer - %th.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.order_cycle.name'; reverse = !reverse" } Order Cycle - %th.hub{ 'ng-show' => 'columns.hub.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'order.distributor.name'; reverse = !reverse" } Hub - %th.variant{ 'ng-show' => 'columns.variant.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'units_variant.unit_text'; reverse = !reverse" } Product: Unit - %th.quantity{ 'ng-show' => 'columns.quantity.visible' } Quantity - %th.max{ 'ng-show' => 'columns.max.visible' } Max - %th.actions - %th.actions - Ask?  - %input{ :type => 'checkbox', 'ng-model' => "confirmDelete" } - %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:predicate:reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", :id => "li_{{line_item.id}}" } - %td.bulk - %input{ :type => "checkbox", :name => 'bulk', 'ng-model' => 'line_item.checked' } - %td.order_no{ 'ng-show' => 'columns.order_no.visible' } {{ line_item.order.number }} - %td.full_name{ 'ng-show' => 'columns.full_name.visible' } {{ line_item.order.full_name }} - %td.email{ 'ng-show' => 'columns.email.visible' } {{ line_item.order.email }} - %td.phone{ 'ng-show' => 'columns.phone.visible' } {{ line_item.order.phone }} - %td.date{ 'ng-show' => 'columns.order_date.visible' } {{ line_item.order.completed_at }} - %td.producer{ 'ng-show' => 'columns.producer.visible' } {{ line_item.supplier.name }} - %td.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } {{ line_item.order.order_cycle.name }} - %td.hub{ 'ng-show' => 'columns.hub.visible' } {{ line_item.order.distributor.name }} - %td.variant{ 'ng-show' => 'columns.variant.visible' } - %a{ :href => '#', 'ng-click' => "setSelectedUnitsVariant(line_item.units_product,line_item.units_variant)" } {{ line_item.units_variant.unit_text }} - %td.quantity{ 'ng-show' => 'columns.quantity.visible' } - %input{ :type => 'number', :name => 'quantity', 'ng-model' => "line_item.quantity", 'ofn-line-item-upd-attr' => "quantity" } - %td.max{ 'ng-show' => 'columns.max.visible' } {{ line_item.max_quantity }} - %td.actions - %a{ :class => "edit-order icon-edit no-text", 'ofn-confirm-link-path' => "/admin/orders/{{line_item.order.number}}/edit" } - %td.actions - %a{ 'ng-click' => "deleteLineItem(line_item)", :class => "delete-line-item icon-trash no-text" } - %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'pendingChanges.submitAll()' } \ No newline at end of file + %form{ 'ng-model' => "bulk_order_form" } + %table.index#listing_orders.bulk{ :class => "sixteen columns alpha" } + %thead + %tr + %th.bulk + %input{ :type => "checkbox", :name => 'toggle_bulk', 'ng-click' => 'toggleAllCheckboxes()', 'ng-checked' => "allBoxesChecked()" } + %th.order_no{ 'ng-show' => 'columns.order_no.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.number'; reverse = !reverse" } Order No. + %th.full_name{ 'ng-show' => 'columns.full_name.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.full_name'; reverse = !reverse" } Name + %th.email{ 'ng-show' => 'columns.email.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.email'; reverse = !reverse" } Email + %th.phone{ 'ng-show' => 'columns.phone.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.phone'; reverse = !reverse" } Phone + %th.date{ 'ng-show' => 'columns.order_date.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.completed_at'; reverse = !reverse" } Order Date + %th.producer{ 'ng-show' => 'columns.producer.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'supplier.name'; reverse = !reverse" } Producer + %th.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.order_cycle.name'; reverse = !reverse" } Order Cycle + %th.hub{ 'ng-show' => 'columns.hub.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'order.distributor.name'; reverse = !reverse" } Hub + %th.variant{ 'ng-show' => 'columns.variant.visible' } + %a{ :href => '', 'ng-click' => "predicate = 'units_variant.unit_text'; reverse = !reverse" } Product: Unit + %th.quantity{ 'ng-show' => 'columns.quantity.visible' } Quantity + %th.max{ 'ng-show' => 'columns.max.visible' } Max + %th.unit_value{ 'ng-show' => 'columns.unit_value.visible' } Weight/Volume + %th.price{ 'ng-show' => 'columns.price.visible' } Price + %th.actions + %th.actions + Ask?  + %input{ :type => 'checkbox', 'ng-model' => "confirmDelete" } + %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:predicate:reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", :id => "li_{{line_item.id}}" } + %td.bulk + %input{ :type => "checkbox", :name => 'bulk', 'ng-model' => 'line_item.checked' } + %td.order_no{ 'ng-show' => 'columns.order_no.visible' } {{ line_item.order.number }} + %td.full_name{ 'ng-show' => 'columns.full_name.visible' } {{ line_item.order.full_name }} + %td.email{ 'ng-show' => 'columns.email.visible' } {{ line_item.order.email }} + %td.phone{ 'ng-show' => 'columns.phone.visible' } {{ line_item.order.phone }} + %td.date{ 'ng-show' => 'columns.order_date.visible' } {{ line_item.order.completed_at }} + %td.producer{ 'ng-show' => 'columns.producer.visible' } {{ line_item.supplier.name }} + %td.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } {{ line_item.order.order_cycle.name }} + %td.hub{ 'ng-show' => 'columns.hub.visible' } {{ line_item.order.distributor.name }} + %td.variant{ 'ng-show' => 'columns.variant.visible' } + %a{ :href => '#', 'ng-click' => "setSelectedUnitsVariant(line_item.units_product,line_item.units_variant)" } {{ line_item.units_variant.unit_text }} + %td.quantity{ 'ng-show' => 'columns.quantity.visible' } + %input{ :type => 'number', :name => 'quantity', 'ng-model' => "line_item.quantity", 'ofn-line-item-upd-attr' => "quantity" } + %td.max{ 'ng-show' => 'columns.max.visible' } {{ line_item.max_quantity }} + %td.unit_value{ 'ng-show' => 'columns.unit_value.visible' } + %input{ :type => 'number', :name => 'unit_value', :id => 'unit_value', 'ng-model' => "line_item.unit_value", 'ng-change' => "weightAdjustedPrice(line_item, {{ line_item.unit_value }})", 'ofn-line-item-upd-attr' => "unit_value" } + %td.price{ 'ng-show' => 'columns.price.visible' } + %input{ :type => 'text', :name => 'price', :id => 'price', :value => '{{ line_item.price | currency }}', 'ng-model' => "line_item.price", 'ng-readonly' => "true", 'ofn-line-item-upd-attr' => "price" } + %td.actions + %a{ :class => "edit-order icon-edit no-text", 'ofn-confirm-link-path' => "/admin/orders/{{line_item.order.number}}/edit" } + %td.actions + %a{ 'ng-click' => "deleteLineItem(line_item)", :class => "delete-line-item icon-trash no-text" } + %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'pendingChanges.submitAll()' } diff --git a/app/views/spree/api/line_items/bulk_show.v1.rabl b/app/views/spree/api/line_items/bulk_show.v1.rabl index 8b53c42086..6b24ef4bf2 100644 --- a/app/views/spree/api/line_items/bulk_show.v1.rabl +++ b/app/views/spree/api/line_items/bulk_show.v1.rabl @@ -1,5 +1,7 @@ object @line_item -attributes :id, :quantity, :max_quantity +attributes :id, :quantity, :max_quantity, :price node( :supplier ) { |li| partial 'api/enterprises/bulk_show', :object => li.product.supplier } node( :units_product ) { |li| partial 'spree/api/products/units_show', :object => li.product } -node( :units_variant ) { |li| partial 'spree/api/variants/units_show', :object => li.variant } \ No newline at end of file +node( :units_variant ) { |li| partial 'spree/api/variants/units_show', :object => li.variant } +node( :unit_value ) { |li| li.unit_value.to_f } +node( :price ) { |li| li.price } From 54da7ae241262a7e79666037828c02108fd0170e Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Tue, 14 Apr 2015 17:29:56 +0100 Subject: [PATCH 12/41] Adding additional logic for if the line_item unit_value is nil --- app/assets/javascripts/admin/bulk_order_management.js.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index a20cdd27e7..a52b770831 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -165,6 +165,8 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.quickSearch = "" $scope.weightAdjustedPrice = (lineItem, oldValue) -> + if oldValue <= 0 + oldValue = lineItem.units_variant.unit_value if lineItem.unit_value <= 0 lineItem.unit_value = lineItem.units_variant.unit_value lineItem.price = lineItem.price * lineItem.unit_value / oldValue From 662c7fe3681a9b454823607c8654213a147af251 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Fri, 24 Apr 2015 16:14:24 +0100 Subject: [PATCH 13/41] Removing notes to myself from this file --- .../spree/api/line_items_controller_decorator.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/controllers/spree/api/line_items_controller_decorator.rb b/app/controllers/spree/api/line_items_controller_decorator.rb index f9a025873d..35fca864f4 100644 --- a/app/controllers/spree/api/line_items_controller_decorator.rb +++ b/app/controllers/spree/api/line_items_controller_decorator.rb @@ -6,12 +6,3 @@ Spree::Api::LineItemsController.class_eval do order.update_distribution_charge! end end - - -#when we update a line item the .update_distribution_charge! is called -# order.should_receive .update_distribution_charge! -# check fails when absent - -# in order model check that .update_distribution_charge! is properly tested. -# think through use cases - existing completed order -# currently likely just used to complete orders so add test case that works on a completed order From 521834bd76d8a22b6279c49262821fa97528abec Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Fri, 24 Apr 2015 16:17:00 +0100 Subject: [PATCH 14/41] Populate the line item unit value, when line_item created and update old data in migration --- app/models/spree/order_decorator.rb | 1 + .../20150424151117_populate_line_item_unit_value.rb | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 db/migrate/20150424151117_populate_line_item_unit_value.rb diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 0dc6977965..cbc5c0086d 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -127,6 +127,7 @@ Spree::Order.class_eval do else current_item = Spree::LineItem.new(:quantity => quantity, max_quantity: max_quantity) current_item.variant = variant + current_item.unit_value = variant.unit_value if currency current_item.currency = currency unless currency.nil? current_item.price = variant.price_in(currency).amount diff --git a/db/migrate/20150424151117_populate_line_item_unit_value.rb b/db/migrate/20150424151117_populate_line_item_unit_value.rb new file mode 100644 index 0000000000..2122b84472 --- /dev/null +++ b/db/migrate/20150424151117_populate_line_item_unit_value.rb @@ -0,0 +1,9 @@ +class PopulateLineItemUnitValue < ActiveRecord::Migration + def up + execute "UPDATE spree_line_items SET unit_value = spree_variants.unit_value FROM spree_variants WHERE spree_line_items.variant_id = spree_variants.id" + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end From 4d025ee7a923060a8e09d5977f8136094132b726 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Fri, 24 Apr 2015 16:22:17 +0100 Subject: [PATCH 15/41] Updating the spec based on @Robs suggestions, hoping for his insights. Still doesn't work. --- spec/controllers/spree/api/line_items_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/spree/api/line_items_controller_spec.rb b/spec/controllers/spree/api/line_items_controller_spec.rb index e3c9a262af..7d0b215ae9 100644 --- a/spec/controllers/spree/api/line_items_controller_spec.rb +++ b/spec/controllers/spree/api/line_items_controller_spec.rb @@ -20,9 +20,9 @@ module Spree make_simple_data! context "as a line item is updated" do - it "apply enterprise fees on the item" do + it "update distribution charge on the order" do line_item_params = { order_id: order.number, id: line_item.id, line_item: { id: line_item.id, unit_value: 520 }, format: :json} - controller.should_receive(:apply_enterprise_fees).and_return(true) + order.should_receive(:update_distribution_charge!).and_call_original spree_post :update, line_item_params end end From 3179887842309f497ffb8685cca3ac726d64dcf9 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sat, 9 May 2015 19:03:06 +0100 Subject: [PATCH 16/41] Do not allow line_item.unit_value to be updated if the variant.unit_value is zero --- .../javascripts/admin/bulk_order_management.js.coffee | 6 ++++++ app/views/spree/admin/orders/bulk_management.html.haml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index a52b770831..bf80b9f353 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -171,6 +171,12 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ lineItem.unit_value = lineItem.units_variant.unit_value lineItem.price = lineItem.price * lineItem.unit_value / oldValue #$scope.bulk_order_form.line_item.price.$setViewValue($scope.bulk_order_form.line_item.price.$viewValue) + + $scope.unitValueLessThanZero = (lineItem) -> + if lineItem.units_variant.unit_value <= 0 + true + else + false ] daysFromToday = (days) -> diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index a1f53826e1..f6680aefae 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -149,7 +149,7 @@ %input{ :type => 'number', :name => 'quantity', 'ng-model' => "line_item.quantity", 'ofn-line-item-upd-attr' => "quantity" } %td.max{ 'ng-show' => 'columns.max.visible' } {{ line_item.max_quantity }} %td.unit_value{ 'ng-show' => 'columns.unit_value.visible' } - %input{ :type => 'number', :name => 'unit_value', :id => 'unit_value', 'ng-model' => "line_item.unit_value", 'ng-change' => "weightAdjustedPrice(line_item, {{ line_item.unit_value }})", 'ofn-line-item-upd-attr' => "unit_value" } + %input{ :type => 'number', :name => 'unit_value', :id => 'unit_value', 'ng-model' => "line_item.unit_value", 'ng-readonly' => "unitValueLessThanZero(line_item)", 'ng-change' => "weightAdjustedPrice(line_item, {{ line_item.unit_value }})", 'ofn-line-item-upd-attr' => "unit_value" } %td.price{ 'ng-show' => 'columns.price.visible' } %input{ :type => 'text', :name => 'price', :id => 'price', :value => '{{ line_item.price | currency }}', 'ng-model' => "line_item.price", 'ng-readonly' => "true", 'ofn-line-item-upd-attr' => "price" } %td.actions From 62ae38372ef4f47ae1b03881a7375e0dfffc1b87 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Mon, 11 May 2015 20:31:24 +1000 Subject: [PATCH 17/41] #541 add "show more" link to producers lists in hub on home page --- .../controllers/hub_node_controller.js.coffee | 4 ++-- .../stylesheets/darkswarm/hub_node.css.sass | 17 +++++++++++++++++ app/views/home/_fat.html.haml | 18 +++++++++++++++--- app/views/home/_skinny.html.haml | 2 +- app/views/producers/_fat.html.haml | 2 +- app/views/producers/_skinny.html.haml | 2 +- 6 files changed, 37 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee index e73ca5fa27..922dc8abb4 100644 --- a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee @@ -1,6 +1,6 @@ Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $location, $templateCache, CurrentHub) -> - $scope.toggle = -> - HashNavigation.toggle $scope.hub.hash + $scope.toggle = (e) -> + HashNavigation.toggle $scope.hub.hash if !angular.element(e.target).inheritedData('is-link') $scope.open = -> HashNavigation.active $scope.hub.hash diff --git a/app/assets/stylesheets/darkswarm/hub_node.css.sass b/app/assets/stylesheets/darkswarm/hub_node.css.sass index d02f9b5299..dfe4d42074 100644 --- a/app/assets/stylesheets/darkswarm/hub_node.css.sass +++ b/app/assets/stylesheets/darkswarm/hub_node.css.sass @@ -83,6 +83,23 @@ .active_table_row:first-child .skinny-head background-color: rgba(255,255,255,0.85) + .producers-list + li.more-producers-link + .less + display: none + a:hover + text-decoration: underline + li.additional-producer + display: none + &.show-more-producers + li.additional-producer + display: block + li.more-producers-link + .more + display: none + .less + display: block + //INACTIVE - closed hub &.inactive &.closed, &.open diff --git a/app/views/home/_fat.html.haml b/app/views/home/_fat.html.haml index 6796601637..092d9e898d 100644 --- a/app/views/home/_fat.html.haml +++ b/app/views/home/_fat.html.haml @@ -1,4 +1,4 @@ -.row.active_table_row{"ng-show" => "open()", "ng-click" => "toggle()", "ng-class" => "{'open' : !ofn-i_032-closed-sign()}"} +.row.active_table_row{"ng-show" => "open()", "ng-click" => "toggle($event)", "ng-class" => "{'open' : !ofn-i_032-closed-sign()}"} .columns.small-12.medium-6.large-5.fat %div{"bo-if" => "hub.taxons"} %label Shop for @@ -21,8 +21,20 @@ .columns.small-12.medium-3.large-5.fat %div{"bo-if" => "hub.producers"} %label Our producers - %ul.small-block-grid-2.medium-block-grid-1.large-block-grid-2 - %li{"ng-repeat" => "enterprise in hub.producers"} + %ul.small-block-grid-2.medium-block-grid-1.large-block-grid-2{"ng-class" => "{'show-more-producers' : toggleMoreProducers}", "class" => "producers-list"} + %li{"ng-repeat" => "enterprise in hub.producers | limitTo:7"} + %enterprise-modal + %i.ofn-i_036-producers + %span{"bo-text" => "enterprise.name"} + %li{"data-is-link" => "true", "class" => "more-producers-link"} + %a{"ng-click" => "toggleMoreProducers=!toggleMoreProducers"} + .more + + + %span{"bo-text" => "hub.producers.length-7"} + More + .less + Show less + %li{"ng-repeat" => "enterprise in hub.producers.slice(7,hub.producers.length)", "class" => "additional-producer"} %enterprise-modal %i.ofn-i_036-producers %span{"bo-text" => "enterprise.name"} diff --git a/app/views/home/_skinny.html.haml b/app/views/home/_skinny.html.haml index cf001371f7..9cf7a01c77 100644 --- a/app/views/home/_skinny.html.haml +++ b/app/views/home/_skinny.html.haml @@ -1,4 +1,4 @@ -.row.active_table_row{"ng-if" => "hub.is_distributor", "ng-click" => "toggle()", "ng-class" => "{'closed' : !open(), 'is_distributor' : producer.is_distributor}", bindonce: true} +.row.active_table_row{"ng-if" => "hub.is_distributor", "ng-click" => "toggle($event)", "ng-class" => "{'closed' : !open(), 'is_distributor' : producer.is_distributor}", bindonce: true} .columns.small-12.medium-6.large-5.skinny-head %a.hub{"bo-href" => "hub.path", "ng-class" => "{primary: hub.active, secondary: !hub.active}", "ofn-empties-cart" => "hub"} diff --git a/app/views/producers/_fat.html.haml b/app/views/producers/_fat.html.haml index a6d6661501..99099a31f8 100644 --- a/app/views/producers/_fat.html.haml +++ b/app/views/producers/_fat.html.haml @@ -1,4 +1,4 @@ -.row.active_table_row{"ng-if" => "open()", "ng-click" => "toggle()", "ng-class" => "{'open' : !ofn-i_032-closed-sign()}"} +.row.active_table_row{"ng-if" => "open()", "ng-click" => "toggle($event)", "ng-class" => "{'open' : !ofn-i_032-closed-sign()}"} .columns.small-12.medium-7.large-7.fat / Will add in long description available once clean up HTML formatting producer.long_description diff --git a/app/views/producers/_skinny.html.haml b/app/views/producers/_skinny.html.haml index 49ab0a35be..11860d35d6 100644 --- a/app/views/producers/_skinny.html.haml +++ b/app/views/producers/_skinny.html.haml @@ -1,4 +1,4 @@ -.row.active_table_row{"ng-click" => "toggle()", "ng-class" => "{'closed' : !open(), 'is_distributor' : producer.is_distributor}"} +.row.active_table_row{"ng-click" => "toggle($event)", "ng-class" => "{'closed' : !open(), 'is_distributor' : producer.is_distributor}"} .columns.small-12.medium-4.large-4.skinny-head %span{"bo-if" => "producer.is_distributor" } %a.is_distributor{"bo-href" => "producer.path" } From a473d0ed11cdce163d37fac574b31df35fa4b6d5 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 13 May 2015 15:58:03 +1000 Subject: [PATCH 18/41] Checking in db version change --- db/schema.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 96f5896c79..f9f32af7d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20150424025907) do +ActiveRecord::Schema.define(:version => 20150424151117) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -619,9 +619,9 @@ ActiveRecord::Schema.define(:version => 20150424025907) 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 From 7f80c02c0ea0736cca16117823dc48c55b9e4240 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 11:45:05 +1000 Subject: [PATCH 19/41] Adding route for managed route for admin orders --- config/routes.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index 4621ee4a35..f8b70ba4d6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -169,6 +169,10 @@ Spree::Core::Engine.routes.prepend do post :bulk_update, :on => :collection, :as => :bulk_update end + + resources :orders do + get :managed, on: :collection + end end resources :orders do From 823adf32721a1cd0d44ea18497f79b5d43b03f5b Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 11:45:48 +1000 Subject: [PATCH 20/41] Translating existing order-related rabl templates accross to AMS --- .../api/admin/basic_order_cycle_serializer.rb | 14 +++++++++ .../api/admin/line_item_serializer.rb | 15 +++++++++ app/serializers/api/admin/order_serializer.rb | 31 +++++++++++++++++++ .../api/admin/units_product_serializer.rb | 3 ++ .../api/admin/units_variant_serializer.rb | 8 +++++ 5 files changed, 71 insertions(+) create mode 100644 app/serializers/api/admin/basic_order_cycle_serializer.rb create mode 100644 app/serializers/api/admin/line_item_serializer.rb create mode 100644 app/serializers/api/admin/order_serializer.rb create mode 100644 app/serializers/api/admin/units_product_serializer.rb create mode 100644 app/serializers/api/admin/units_variant_serializer.rb diff --git a/app/serializers/api/admin/basic_order_cycle_serializer.rb b/app/serializers/api/admin/basic_order_cycle_serializer.rb new file mode 100644 index 0000000000..e94795821a --- /dev/null +++ b/app/serializers/api/admin/basic_order_cycle_serializer.rb @@ -0,0 +1,14 @@ +class Api::Admin::BasicOrderCycleSerializer < ActiveModel::Serializer + attributes :id, :name, :first_order, :last_order + + has_many :suppliers, serializer: Api::Admin::IdNameSerializer + has_many :distributors, serializer: Api::Admin::IdNameSerializer + + def first_order + object.orders_open_at.strftime("%F") + end + + def last_order + (object.orders_close_at + 1.day).strftime("%F") + end +end diff --git a/app/serializers/api/admin/line_item_serializer.rb b/app/serializers/api/admin/line_item_serializer.rb new file mode 100644 index 0000000000..784e1e8be3 --- /dev/null +++ b/app/serializers/api/admin/line_item_serializer.rb @@ -0,0 +1,15 @@ +class Api::Admin::LineItemSerializer < ActiveModel::Serializer + attributes :id, :quantity, :max_quantity, :supplier, :units_product, :units_variant + + def supplier + Api::Admin::IdNameSerializer.new(object.product.supplier).serializable_hash + end + + def units_product + Api::Admin::UnitsProductSerializer.new(object.product).serializable_hash + end + + def units_variant + Api::Admin::UnitsVariantSerializer.new(object.variant).serializable_hash + end +end diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb new file mode 100644 index 0000000000..2277551989 --- /dev/null +++ b/app/serializers/api/admin/order_serializer.rb @@ -0,0 +1,31 @@ +class Api::Admin::OrderSerializer < ActiveModel::Serializer + attributes :id, :number, :full_name, :email, :phone, :completed_at, :line_items + + has_one :distributor, serializer: Api::Admin::IdNameSerializer + has_one :order_cycle, serializer: Api::Admin::BasicOrderCycleSerializer + + def full_name + object.billing_address.nil? ? "" : ( object.billing_address.full_name || "" ) + end + + def email + object.email || "" + end + + def phone + object.billing_address.nil? ? "a" : ( object.billing_address.phone || "" ) + end + + def completed_at + object.completed_at.blank? ? "" : object.completed_at.strftime("%F %T") + end + + def line_items + # we used to have a scope here, but we are at the point where a user which can edit an order + # should be able to edit all of the line_items as well, making the scope redundant + ActiveModel::ArraySerializer.new( + object.line_items.order('id ASC'), + {each_serializer: Api::Admin::LineItemSerializer} + ) + end +end diff --git a/app/serializers/api/admin/units_product_serializer.rb b/app/serializers/api/admin/units_product_serializer.rb new file mode 100644 index 0000000000..37521b2bb9 --- /dev/null +++ b/app/serializers/api/admin/units_product_serializer.rb @@ -0,0 +1,3 @@ +class Api::Admin::UnitsProductSerializer < ActiveModel::Serializer + attributes :id, :name, :group_buy_unit_size, :variant_unit +end diff --git a/app/serializers/api/admin/units_variant_serializer.rb b/app/serializers/api/admin/units_variant_serializer.rb new file mode 100644 index 0000000000..e96f9299f6 --- /dev/null +++ b/app/serializers/api/admin/units_variant_serializer.rb @@ -0,0 +1,8 @@ +class Api::Admin::UnitsVariantSerializer < ActiveModel::Serializer + attributes :id, :unit_text, :unit_value + + def unit_text + options_text = object.options_text + object.product.name + (options_text.empty? ? "" : ": #{options_text}") + end +end From fc55a000b87e73cee57129cbf689edccbd620b5b Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 11:48:13 +1000 Subject: [PATCH 21/41] Adding managed controller action which uses new order serializer to render json --- .../admin/orders_controller_decorator.rb | 10 +- app/models/spree/ability_decorator.rb | 3 +- .../spree/admin/orders_controller_spec.rb | 139 +++++++++++++++++- 3 files changed, 147 insertions(+), 5 deletions(-) diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 42e7068afc..247e780deb 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -7,7 +7,7 @@ Spree::Admin::OrdersController.class_eval do # We need to add expections for collection actions other than :index here # because spree_auth_devise causes load_order to be called, which results # in an auth failure as the @order object is nil for collection actions - before_filter :check_authorization, :except => :bulk_management + before_filter :check_authorization, except: [:bulk_management, :managed] # After updating an order, the fees should be updated as well # Currently, adding or deleting line items does not trigger updating the @@ -17,7 +17,7 @@ Spree::Admin::OrdersController.class_eval do after_filter :update_distribution_charge, :only => :update respond_override :index => { :html => - { :success => lambda { + { :success => lambda { # Filter orders to only show those distributed by current user (or all for admin user) @orders = @search.result.includes([:user, :shipments, :payments]). distributed_by_user(spree_current_user). @@ -37,4 +37,10 @@ Spree::Admin::OrdersController.class_eval do def update_distribution_charge @order.update_distribution_charge! end + + def managed + permissions = OpenFoodNetwork::Permissions.new(spree_current_user) + @orders = permissions.editable_orders.ransack(params[:q]).result.page(params[:page]).per(params[:per_page]) + render json: @orders, each_serializer: Api::Admin::OrderSerializer + end end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 7bc6fb6535..13fe55a852 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -142,7 +142,7 @@ class AbilityDecorator # during the order creation process from the admin backend order.distributor.nil? || user.enterprises.include?(order.distributor) end - can [:admin, :bulk_management], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor) + can [:admin, :bulk_management, :managed], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor) can [:admin, :create], Spree::LineItem can [:destroy], Spree::LineItem do |item| user.admin? || user.enterprises.include?(order.distributor) || user == order.order_cycle.manager @@ -154,7 +154,6 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::ReturnAuthorization can [:destroy], Spree::Adjustment do |adjustment| # Sharing code with destroying a line item. This should be unified and probably applied for other actions as well. - binding.pry if user.admin? true elsif adjustment.adjustable.instance_of? Spree::Order diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index 5724732c50..ac6a5532b2 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -1,9 +1,10 @@ require 'spec_helper' describe Spree::Admin::OrdersController do - let!(:order) { create(:order) } + include AuthenticationWorkflow context "updating an order with line items" do + let!(:order) { create(:order) } let(:line_item) { create(:line_item) } before { login_as_admin } @@ -27,4 +28,140 @@ describe Spree::Admin::OrdersController do } end end + + describe "managed" do + render_views + + let(:order_attributes) { [:id, :full_name, :email, :phone, :completed_at, :line_items, :distributor, :order_cycle, :number] } + + def self.make_simple_data! + let!(:dist1) { FactoryGirl.create(:distributor_enterprise) } + let!(:order1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:order2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:order3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1) } + let!(:line_item2) { FactoryGirl.create(:line_item, order: order2) } + let!(:line_item3) { FactoryGirl.create(:line_item, order: order2) } + let!(:line_item4) { FactoryGirl.create(:line_item, order: order3) } + let(:line_item_attributes) { [:id, :quantity, :max_quantity, :supplier, :units_product, :units_variant] } + end + + context "as a normal user" do + before { controller.stub spree_current_user: create_enterprise_user } + + make_simple_data! + + it "should deny me access to managed orders" do + spree_get :managed, { :template => 'bulk_index', :format => :json } + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "as an administrator" do + make_simple_data! + + before do + controller.stub spree_current_user: quick_login_as_admin + spree_get :managed, { :template => 'bulk_index', :format => :json } + end + + it "retrieves a list of orders with appropriate attributes, including line items with appropriate attributes" do + keys = json_response.first.keys.map{ |key| key.to_sym } + order_attributes.all?{ |attr| keys.include? attr }.should == true + end + + it "retrieves a list of line items with appropriate attributes" do + li_keys = json_response.first['line_items'].first.keys.map{ |key| key.to_sym } + line_item_attributes.all?{ |attr| li_keys.include? attr }.should == true + end + + it "sorts orders in ascending id order" do + ids = json_response.map{ |order| order['id'] } + ids[0].should < ids[1] + ids[1].should < ids[2] + end + + it "formats completed_at to 'yyyy-mm-dd hh:mm'" do + json_response.map{ |order| order['completed_at'] }.all?{ |a| a.match("^\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}$") }.should == true + end + + it "returns an array for line_items" do + json_response.map{ |order| order['line_items'] }.all?{ |a| a.is_a? Array }.should == true + end + + it "returns quantity and max quantity at integers" do + json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['quantity'] }.all?{ |q| q.is_a? Fixnum }.should == true + json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['max_quantity'] }.all?{ |mq| mq.nil? || mq.is_a?( Fixnum ) }.should == true + end + + it "returns supplier object with id and name keys" do + json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['supplier'] }.all?{ |s| s.has_key?('id') && s.has_key?('name') }.should == true + end + + it "returns distributor object with id and name keys" do + json_response.map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') && d.has_key?('name') }.should == true + end + + it "retrieves the order number" do + json_response.map{ |order| order['number'] }.all?{ |number| number.match("^R\\d{5,10}$") }.should == true + end + end + + context "as an enterprise user" do + let(:supplier) { create(:supplier_enterprise) } + let(:distributor1) { create(:distributor_enterprise) } + let(:distributor2) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } + let!(:order1) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let!(:line_item2) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let!(:order2) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.now, distributor: distributor2, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item3) { FactoryGirl.create(:line_item, order: order2, product: FactoryGirl.create(:product, supplier: supplier)) } + + context "producer enterprise" do + + before do + controller.stub spree_current_user: supplier.owner + spree_get :managed, { :format => :json } + end + + it "does not display line items for which my enterprise is a supplier" do + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "coordinator enterprise" do + before do + controller.stub spree_current_user: coordinator.owner + spree_get :managed, { :format => :json } + end + + it "retrieves a list of orders" do + keys = json_response.first.keys.map{ |key| key.to_sym } + order_attributes.all?{ |attr| keys.include? attr }.should == true + end + + it "only displays line items from orders for which my enterprise is the order_cycle coorinator" do + json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.sort.should == [line_item1.id, line_item2.id, line_item3.id].sort + end + end + + context "hub enterprise" do + before do + controller.stub spree_current_user: distributor1.owner + spree_get :managed, { :format => :json } + end + + it "retrieves a list of orders" do + keys = json_response.first.keys.map{ |key| key.to_sym } + order_attributes.all?{ |attr| keys.include? attr }.should == true + end + + it "only displays line items from orders for which my enterprise is a distributor" do + json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.sort.should == [line_item1.id, line_item2.id].sort + end + end + end + end end From 0ad2978926f17e663b96bafffe4f8365e660af50 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 11:48:48 +1000 Subject: [PATCH 22/41] Removing old managed route from api orders controller and switching BOM over to use new controller action --- .../admin/bulk_order_management.js.coffee | 2 +- .../spree/api/orders_controller_decorator.rb | 8 -- .../spree/api/orders_controller_spec.rb | 121 ------------------ .../unit/bulk_order_management_spec.js.coffee | 2 +- 4 files changed, 2 insertions(+), 131 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 20ddc177ba..076dd55e3a 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -62,7 +62,7 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.fetchOrders = -> $scope.loading = true - dataFetcher("/api/orders/managed?template=bulk_index;page=1;per_page=500;q[state_not_eq]=canceled;q[completed_at_not_null]=true;q[completed_at_gt]=#{$scope.startDate};q[completed_at_lt]=#{$scope.endDate}").then (data) -> + dataFetcher("/admin/orders/managed?template=bulk_index;page=1;per_page=500;q[state_not_eq]=canceled;q[completed_at_not_null]=true;q[completed_at_gt]=#{$scope.startDate};q[completed_at_lt]=#{$scope.endDate}").then (data) -> $scope.resetOrders data $scope.loading = false diff --git a/app/controllers/spree/api/orders_controller_decorator.rb b/app/controllers/spree/api/orders_controller_decorator.rb index ca1fc4a570..21f64f51d8 100644 --- a/app/controllers/spree/api/orders_controller_decorator.rb +++ b/app/controllers/spree/api/orders_controller_decorator.rb @@ -4,12 +4,4 @@ Spree::Api::OrdersController.class_eval do # because Spree's API controller causes authorize_read! to be called, which # results in an ActiveRecord::NotFound Exception as the order object is not # defined for collection actions - before_filter :authorize_read!, :except => [:managed] - - def managed - authorize! :admin, Spree::Order - authorize! :read, Spree::Order - @orders = Spree::Order.ransack(params[:q]).result.distributed_by_user(current_api_user).page(params[:page]).per(params[:per_page]) - respond_with(@orders, default_template: :index) - end end diff --git a/spec/controllers/spree/api/orders_controller_spec.rb b/spec/controllers/spree/api/orders_controller_spec.rb index ec87169f76..5b651016f3 100644 --- a/spec/controllers/spree/api/orders_controller_spec.rb +++ b/spec/controllers/spree/api/orders_controller_spec.rb @@ -3,127 +3,6 @@ require 'spree/api/testing_support/helpers' module Spree describe Spree::Api::OrdersController do - include Spree::Api::TestingSupport::Helpers - render_views - before do - stub_authentication! - Spree.user_class.stub :find_by_spree_api_key => current_api_user - end - - let(:order_attributes) { [:id, :full_name, :email, :phone, :completed_at, :line_items, :distributor, :order_cycle, :number] } - - def self.make_simple_data! - let!(:dist1) { FactoryGirl.create(:distributor_enterprise) } - let!(:order1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } - let!(:order2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } - let!(:order3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } - let!(:line_item1) { FactoryGirl.create(:line_item, order: order1) } - let!(:line_item2) { FactoryGirl.create(:line_item, order: order2) } - let!(:line_item3) { FactoryGirl.create(:line_item, order: order2) } - let!(:line_item4) { FactoryGirl.create(:line_item, order: order3) } - let(:line_item_attributes) { [:id, :quantity, :max_quantity, :supplier, :units_product, :units_variant] } - end - - - context "as a normal user" do - sign_in_as_user! - make_simple_data! - - it "should deny me access to managed orders" do - spree_get :managed, { :template => 'bulk_index', :format => :json } - assert_unauthorized! - end - end - - context "as an administrator" do - sign_in_as_admin! - make_simple_data! - - before :each do - spree_get :managed, { :template => 'bulk_index', :format => :json } - end - - it "retrieves a list of orders with appropriate attributes, including line items with appropriate attributes" do - keys = json_response.first.keys.map{ |key| key.to_sym } - order_attributes.all?{ |attr| keys.include? attr }.should == true - end - - it "retrieves a list of line items with appropriate attributes" do - li_keys = json_response.first['line_items'].first.keys.map{ |key| key.to_sym } - line_item_attributes.all?{ |attr| li_keys.include? attr }.should == true - end - - it "sorts orders in ascending id order" do - ids = json_response.map{ |order| order['id'] } - ids[0].should < ids[1] - ids[1].should < ids[2] - end - - it "formats completed_at to 'yyyy-mm-dd hh:mm'" do - json_response.map{ |order| order['completed_at'] }.all?{ |a| a.match("^\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}$") }.should == true - end - - it "returns an array for line_items" do - json_response.map{ |order| order['line_items'] }.all?{ |a| a.is_a? Array }.should == true - end - - it "returns quantity and max quantity at integers" do - json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['quantity'] }.all?{ |q| q.is_a? Fixnum }.should == true - json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['max_quantity'] }.all?{ |mq| mq.nil? || mq.is_a?( Fixnum ) }.should == true - end - - it "returns supplier object with id and name keys" do - json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['supplier'] }.all?{ |s| s.has_key?('id') && s.has_key?('name') }.should == true - end - - it "returns distributor object with id and name keys" do - json_response.map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') && d.has_key?('name') }.should == true - end - - it "retrieves the order number" do - json_response.map{ |order| order['number'] }.all?{ |number| number.match("^R\\d{5,10}$") }.should == true - end - end - - context "as an enterprise user" do - let(:supplier) { create(:supplier_enterprise) } - let(:distributor1) { create(:distributor_enterprise) } - let(:distributor2) { create(:distributor_enterprise) } - let!(:order1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } - let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } - let!(:line_item2) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } - let!(:order2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: distributor2, billing_address: FactoryGirl.create(:address) ) } - let!(:line_item3) { FactoryGirl.create(:line_item, order: order2, product: FactoryGirl.create(:product, supplier: supplier)) } - - context "producer enterprise" do - sign_in_as_enterprise_user! [:supplier] - - before :each do - spree_get :managed, { :template => 'bulk_index', :format => :json } - end - - it "does not display line item for which my enteprise is a supplier" do - response.status.should == 401 - end - end - - context "hub enterprise" do - sign_in_as_enterprise_user! [:distributor1] - - before :each do - spree_get :managed, { :template => 'bulk_index', :format => :json } - end - - it "retrieves a list of orders" do - keys = json_response.first.keys.map{ |key| key.to_sym } - order_attributes.all?{ |attr| keys.include? attr }.should == true - end - - it "only displays line items from orders for which my enterprise is a distributor" do - json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.should == [line_item1.id, line_item2.id] - end - end - end end end diff --git a/spec/javascripts/unit/bulk_order_management_spec.js.coffee b/spec/javascripts/unit/bulk_order_management_spec.js.coffee index 3377c799db..71ebb9f3b9 100644 --- a/spec/javascripts/unit/bulk_order_management_spec.js.coffee +++ b/spec/javascripts/unit/bulk_order_management_spec.js.coffee @@ -43,7 +43,7 @@ describe "AdminOrderMgmtCtrl", -> describe "fetching orders", -> beforeEach -> scope.initialiseVariables() - httpBackend.expectGET("/api/orders/managed?template=bulk_index;page=1;per_page=500;q[state_not_eq]=canceled;q[completed_at_not_null]=true;q[completed_at_gt]=SomeDate;q[completed_at_lt]=SomeDate").respond "list of orders" + httpBackend.expectGET("/admin/orders/managed?template=bulk_index;page=1;per_page=500;q[state_not_eq]=canceled;q[completed_at_not_null]=true;q[completed_at_gt]=SomeDate;q[completed_at_lt]=SomeDate").respond "list of orders" it "makes a call to dataFetcher, with current start and end date parameters", -> scope.fetchOrders() From c56efabfbecb39bb15bb12c8b9e02556815f5537 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 11:52:49 +1000 Subject: [PATCH 23/41] Removing obsolete rabl templates --- app/views/spree/api/line_items/bulk_show.v1.rabl | 5 ----- app/views/spree/api/orders/bulk_index.v1.rabl | 2 -- app/views/spree/api/orders/bulk_show.v1.rabl | 14 -------------- app/views/spree/api/products/units_show.v1.rabl | 2 -- app/views/spree/api/variants/units_show.v1.rabl | 9 --------- 5 files changed, 32 deletions(-) delete mode 100644 app/views/spree/api/line_items/bulk_show.v1.rabl delete mode 100644 app/views/spree/api/orders/bulk_index.v1.rabl delete mode 100644 app/views/spree/api/orders/bulk_show.v1.rabl delete mode 100644 app/views/spree/api/products/units_show.v1.rabl delete mode 100644 app/views/spree/api/variants/units_show.v1.rabl diff --git a/app/views/spree/api/line_items/bulk_show.v1.rabl b/app/views/spree/api/line_items/bulk_show.v1.rabl deleted file mode 100644 index 8b53c42086..0000000000 --- a/app/views/spree/api/line_items/bulk_show.v1.rabl +++ /dev/null @@ -1,5 +0,0 @@ -object @line_item -attributes :id, :quantity, :max_quantity -node( :supplier ) { |li| partial 'api/enterprises/bulk_show', :object => li.product.supplier } -node( :units_product ) { |li| partial 'spree/api/products/units_show', :object => li.product } -node( :units_variant ) { |li| partial 'spree/api/variants/units_show', :object => li.variant } \ No newline at end of file diff --git a/app/views/spree/api/orders/bulk_index.v1.rabl b/app/views/spree/api/orders/bulk_index.v1.rabl deleted file mode 100644 index 384f518fcf..0000000000 --- a/app/views/spree/api/orders/bulk_index.v1.rabl +++ /dev/null @@ -1,2 +0,0 @@ -collection @orders.order('id ASC') -extends "spree/api/orders/bulk_show" \ No newline at end of file diff --git a/app/views/spree/api/orders/bulk_show.v1.rabl b/app/views/spree/api/orders/bulk_show.v1.rabl deleted file mode 100644 index 9addbbec6f..0000000000 --- a/app/views/spree/api/orders/bulk_show.v1.rabl +++ /dev/null @@ -1,14 +0,0 @@ -object @order -attributes :id, :number - -node( :full_name ) { |order| order.billing_address.nil? ? "" : ( order.billing_address.full_name || "" ) } -node( :email ) { |order| order.email || "" } -node( :phone ) { |order| order.billing_address.nil? ? "a" : ( order.billing_address.phone || "" ) } -node( :completed_at ) { |order| order.completed_at.blank? ? "" : order.completed_at.strftime("%F %T") } -node( :distributor ) { |order| partial 'api/enterprises/bulk_show', :object => order.distributor } -node( :order_cycle ) { |order| partial 'api/order_cycles/bulk_show', :object => order.order_cycle } -node( :line_items ) do |order| - order.line_items.managed_by(@current_api_user).order('id ASC').map do |line_item| - partial 'spree/api/line_items/bulk_show', :object => line_item - end -end \ No newline at end of file diff --git a/app/views/spree/api/products/units_show.v1.rabl b/app/views/spree/api/products/units_show.v1.rabl deleted file mode 100644 index 2108609a33..0000000000 --- a/app/views/spree/api/products/units_show.v1.rabl +++ /dev/null @@ -1,2 +0,0 @@ -object @product -attributes :id, :name, :group_buy_unit_size, :variant_unit \ No newline at end of file diff --git a/app/views/spree/api/variants/units_show.v1.rabl b/app/views/spree/api/variants/units_show.v1.rabl deleted file mode 100644 index 634213e53f..0000000000 --- a/app/views/spree/api/variants/units_show.v1.rabl +++ /dev/null @@ -1,9 +0,0 @@ -object @variant -attributes :id - -node( :unit_text ) do |v| - options_text = v.options_text - v.product.name + (options_text.empty? ? "" : ": #{options_text}") -end - -node( :unit_value ) { |v| v.unit_value } From 8d73b2f53258a00e67c784cd74a2ab423945d39d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 13:47:01 +1000 Subject: [PATCH 24/41] involving... order cycle scopes return distinct OCs --- app/models/order_cycle.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index d16f1d7b8b..88e4953edc 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -70,7 +70,8 @@ class OrderCycle < ActiveRecord::Base # Order cycles where I managed an enterprise at either end of an outgoing exchange # ie. coordinator or distibutor joins(:exchanges).merge(Exchange.outgoing). - where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises) + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises). + select('DISTINCT order_cycles.*') } scope :involving_managed_producers_of, lambda { |user| @@ -79,7 +80,8 @@ class OrderCycle < ActiveRecord::Base # Order cycles where I managed an enterprise at either end of an incoming exchange # ie. coordinator or producer joins(:exchanges).merge(Exchange.incoming). - where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises) + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises). + select('DISTINCT order_cycles.*') } def self.first_opening_for(distributor) From 05131de1ad65897098f4c5f6125de2daa1af0aef Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 14:11:31 +1000 Subject: [PATCH 25/41] Use full_name on BOM instead of options_text --- app/serializers/api/admin/units_variant_serializer.rb | 8 ++++---- app/views/spree/admin/orders/bulk_management.html.haml | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/serializers/api/admin/units_variant_serializer.rb b/app/serializers/api/admin/units_variant_serializer.rb index e96f9299f6..dc27e3adc3 100644 --- a/app/serializers/api/admin/units_variant_serializer.rb +++ b/app/serializers/api/admin/units_variant_serializer.rb @@ -1,8 +1,8 @@ class Api::Admin::UnitsVariantSerializer < ActiveModel::Serializer - attributes :id, :unit_text, :unit_value + attributes :id, :full_name, :unit_value - def unit_text - options_text = object.options_text - object.product.name + (options_text.empty? ? "" : ": #{options_text}") + def full_name + full_name = object.full_name + object.product.name + (full_name.empty? ? "" : ": #{full_name}") end end diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index 61bf9fbb7e..0f9bd05d04 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -43,7 +43,7 @@ Shared Resource? %div{ :class => "eight columns" } %h6{ :class => "eight columns alpha", 'ng-show' => 'sharedResource', style: 'text-align: center;' } {{ selectedUnitsProduct.name + ": ALL" }} - %h6{ :class => "eight columns alpha", 'ng-hide' => 'sharedResource', style: 'text-align: center;' } {{ selectedUnitsVariant.unit_text }} + %h6{ :class => "eight columns alpha", 'ng-hide' => 'sharedResource', style: 'text-align: center;' } {{ selectedUnitsVariant.full_name }} %div{ :class => "four columns omega" } %h6{ :class => "four columns alpha", :style => 'text-align: right;' } %a{ :href => '#', 'ng-click' => 'selectedUnitsVariant = {};selectedUnitsProduct = {};sharedResource=false;' } Clear @@ -122,7 +122,7 @@ %th.hub{ 'ng-show' => 'columns.hub.visible' } %a{ :href => '', 'ng-click' => "predicate = 'order.distributor.name'; reverse = !reverse" } Hub %th.variant{ 'ng-show' => 'columns.variant.visible' } - %a{ :href => '', 'ng-click' => "predicate = 'units_variant.unit_text'; reverse = !reverse" } Product: Unit + %a{ :href => '', 'ng-click' => "predicate = 'units_variant.full_name'; reverse = !reverse" } Product: Unit %th.quantity{ 'ng-show' => 'columns.quantity.visible' } Quantity %th.max{ 'ng-show' => 'columns.max.visible' } Max %th.actions @@ -141,7 +141,7 @@ %td.order_cycle{ 'ng-show' => 'columns.order_cycle.visible' } {{ line_item.order.order_cycle.name }} %td.hub{ 'ng-show' => 'columns.hub.visible' } {{ line_item.order.distributor.name }} %td.variant{ 'ng-show' => 'columns.variant.visible' } - %a{ :href => '#', 'ng-click' => "setSelectedUnitsVariant(line_item.units_product,line_item.units_variant)" } {{ line_item.units_variant.unit_text }} + %a{ :href => '#', 'ng-click' => "setSelectedUnitsVariant(line_item.units_product,line_item.units_variant)" } {{ line_item.units_variant.full_name }} %td.quantity{ 'ng-show' => 'columns.quantity.visible' } %input{ :type => 'number', :name => 'quantity', 'ng-model' => "line_item.quantity", 'ofn-line-item-upd-attr' => "quantity" } %td.max{ 'ng-show' => 'columns.max.visible' } {{ line_item.max_quantity }} @@ -149,4 +149,4 @@ %a{ :class => "edit-order icon-edit no-text", 'ofn-confirm-link-path' => "/admin/orders/{{line_item.order.number}}/edit" } %td.actions %a{ 'ng-click' => "deleteLineItem(line_item)", :class => "delete-line-item icon-trash no-text" } - %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'pendingChanges.submitAll()' } \ No newline at end of file + %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'pendingChanges.submitAll()' } From 01d4cf6ecf7dc607bd568ee7c51370a1e6115b3a Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 15:02:36 +1000 Subject: [PATCH 26/41] Renaming managed_products permissions method to editable_products --- lib/open_food_network/permissions.rb | 2 +- spec/lib/open_food_network/permissions_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 4da645582d..e207eea797 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -106,7 +106,7 @@ module OpenFoodNetwork Spree::LineItem.where(order_id: editable_orders) end - def managed_products + def editable_products managed_enterprise_products_ids = managed_enterprise_products.pluck :id permitted_enterprise_products_ids = related_enterprise_products.pluck :id Spree::Product.where('id IN (?)', managed_enterprise_products_ids + permitted_enterprise_products_ids) diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 10552572f0..2f19a7a404 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -164,7 +164,7 @@ module OpenFoodNetwork end end - describe "finding managed products" do + describe "finding editable products" do let!(:p1) { create(:simple_product) } let!(:p2) { create(:simple_product) } @@ -175,12 +175,12 @@ module OpenFoodNetwork it "returns products produced by managed enterprises" do permissions.stub(:managed_enterprise_products) { Spree::Product.where(id: p1) } - permissions.managed_products.should == [p1] + permissions.editable_products.should == [p1] end it "returns products produced by permitted enterprises" do permissions.stub(:related_enterprise_products) { Spree::Product.where(id: p2) } - permissions.managed_products.should == [p2] + permissions.editable_products.should == [p2] end end From 8132f07d8819d41704ceb7286f6aa1d09d2e44fb Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 15:53:10 +1000 Subject: [PATCH 27/41] Adding visible products method to permissions --- lib/open_food_network/permissions.rb | 18 +++++-- .../lib/open_food_network/permissions_spec.rb | 49 +++++++++++++------ 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index e207eea797..87baa533ad 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -108,8 +108,18 @@ module OpenFoodNetwork def editable_products managed_enterprise_products_ids = managed_enterprise_products.pluck :id - permitted_enterprise_products_ids = related_enterprise_products.pluck :id - Spree::Product.where('id IN (?)', managed_enterprise_products_ids + permitted_enterprise_products_ids) + permitted_enterprise_products_ids = products_supplied_by( + related_enterprises_granting(:manage_products).pluck(:id) + ).pluck :id + Spree::Product.where('spree_products.id IN (?)', managed_enterprise_products_ids | permitted_enterprise_products_ids) + end + + def visible_products + managed_enterprise_products_ids = managed_enterprise_products.pluck :id + permitted_enterprise_products_ids = products_supplied_by( + related_enterprises_granting(:manage_products).pluck(:id) | related_enterprises_granting(:add_to_order_cycle).pluck(:id) + ).pluck :id + Spree::Product.where('spree_products.id IN (?)', managed_enterprise_products_ids | permitted_enterprise_products_ids) end def managed_product_enterprises @@ -181,8 +191,8 @@ module OpenFoodNetwork Spree::Product.managed_by(@user) end - def related_enterprise_products - Spree::Product.where('supplier_id IN (?)', related_enterprises_granting(:manage_products)) + def products_supplied_by(suppliers) + Spree::Product.where('supplier_id IN (?)', suppliers) end end end diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 2f19a7a404..cf2aa1a217 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -165,12 +165,12 @@ module OpenFoodNetwork end describe "finding editable products" do - let!(:p1) { create(:simple_product) } - let!(:p2) { create(:simple_product) } + let!(:p1) { create(:simple_product, supplier: create(:supplier_enterprise) ) } + let!(:p2) { create(:simple_product, supplier: create(:supplier_enterprise) ) } before do permissions.stub(:managed_enterprise_products) { Spree::Product.where('1=0') } - permissions.stub(:related_enterprise_products) { Spree::Product.where('1=0') } + allow(permissions).to receive(:related_enterprises_granting).with(:manage_products) { Enterprise.where("1=0") } end it "returns products produced by managed enterprises" do @@ -179,11 +179,41 @@ module OpenFoodNetwork end it "returns products produced by permitted enterprises" do - permissions.stub(:related_enterprise_products) { Spree::Product.where(id: p2) } + allow(permissions).to receive(:related_enterprises_granting). + with(:manage_products) { Enterprise.where(id: p2.supplier) } permissions.editable_products.should == [p2] end end + describe "finding visible products" do + let!(:p1) { create(:simple_product, supplier: create(:supplier_enterprise) ) } + let!(:p2) { create(:simple_product, supplier: create(:supplier_enterprise) ) } + let!(:p3) { create(:simple_product, supplier: create(:supplier_enterprise) ) } + + before do + permissions.stub(:managed_enterprise_products) { Spree::Product.where("1=0") } + allow(permissions).to receive(:related_enterprises_granting).with(:manage_products) { Enterprise.where("1=0") } + allow(permissions).to receive(:related_enterprises_granting).with(:add_to_order_cycle) { Enterprise.where("1=0") } + end + + it "returns products produced by managed enterprises" do + permissions.stub(:managed_enterprise_products) { Spree::Product.where(id: p1) } + permissions.visible_products.should == [p1] + end + + it "returns products produced by enterprises that have granted manage products" do + allow(permissions).to receive(:related_enterprises_granting). + with(:manage_products) { Enterprise.where(id: p2.supplier) } + permissions.visible_products.should == [p2] + end + + it "returns products produced by enterprises that have granted P-OC" do + allow(permissions).to receive(:related_enterprises_granting). + with(:add_to_order_cycle) { Enterprise.where(id: p3.supplier) } + permissions.visible_products.should == [p3] + end + end + describe "finding enterprises that we manage products for" do let(:e) { double(:enterprise) } @@ -232,17 +262,6 @@ module OpenFoodNetwork end end - describe "finding the supplied products of related enterprises" do - let!(:e) { create(:enterprise) } - let!(:p) { create(:simple_product, supplier: e) } - - it "returns supplied products" do - permissions.should_receive(:related_enterprises_granting).with(:manage_products) { [e] } - - permissions.send(:related_enterprise_products).should == [p] - end - end - describe "finding orders that are visible in reports" do let(:distributor) { create(:distributor_enterprise) } let(:coordinator) { create(:distributor_enterprise) } From 65a6329132739e99634ed261f64bf2a81e4b5e9e Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 16:19:08 +1000 Subject: [PATCH 28/41] Products and inventory reports scopes products to visible in permissions --- .../products_and_inventory_report.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/open_food_network/products_and_inventory_report.rb b/lib/open_food_network/products_and_inventory_report.rb index 1cb485efee..163f9a4f50 100644 --- a/lib/open_food_network/products_and_inventory_report.rb +++ b/lib/open_food_network/products_and_inventory_report.rb @@ -9,7 +9,7 @@ module OpenFoodNetwork def header [ - "Supplier", + "Supplier", "Producer Suburb", "Product", "Product Properties", @@ -36,6 +36,16 @@ module OpenFoodNetwork end end + def permissions + return @permissions unless @permissions.nil? + @permissions = OpenFoodNetwork::Permissions.new(@user) + end + + def visible_products + return @visible_products unless @visible_products.nil? + @visible_products = permissions.visible_products + end + def variants filter(child_variants) + filter(master_variants) end @@ -43,7 +53,7 @@ module OpenFoodNetwork def child_variants Spree::Variant.where(:is_master => false) .joins(:product) - .merge(Spree::Product.managed_by(@user)) + .merge(visible_products) .order("spree_products.name") end @@ -53,7 +63,7 @@ module OpenFoodNetwork .where("(select spree_variants.id from spree_variants as other_spree_variants WHERE other_spree_variants.product_id = spree_variants.product_id AND other_spree_variants.is_master = 'f' LIMIT 1) IS NULL") - .merge(Spree::Product.managed_by(@user)) + .merge(visible_products) .order("spree_products.name") end From 3431c687b85aa8f56f73a6908db3c5c7924c3b3a Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 20:54:27 +1000 Subject: [PATCH 29/41] Making sure every created by factories has a distributor --- spec/factories.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/factories.rb b/spec/factories.rb index c798999134..6cfddd85a5 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -238,6 +238,10 @@ FactoryGirl.modify do unit_description '' end + factory :order do + distributor { create(:distributor_enterprise) } + end + factory :shipping_method do distributors { [Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise)] } display_on '' From 1aca4657d9a7e1b7ba1ce9db0c78eb7081cfc095 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 20:59:06 +1000 Subject: [PATCH 30/41] Oops, switch managed_products to editable_products for Api::ProductsController --- app/controllers/spree/api/products_controller_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/api/products_controller_decorator.rb b/app/controllers/spree/api/products_controller_decorator.rb index 6e9799c86d..e809dbc23e 100644 --- a/app/controllers/spree/api/products_controller_decorator.rb +++ b/app/controllers/spree/api/products_controller_decorator.rb @@ -11,7 +11,7 @@ Spree::Api::ProductsController.class_eval do # TODO: This should be named 'managed'. Is the action above used? Maybe we should remove it. def bulk_products - @products = OpenFoodNetwork::Permissions.new(current_api_user).managed_products. + @products = OpenFoodNetwork::Permissions.new(current_api_user).editable_products. merge(product_scope). order('created_at DESC'). ransack(params[:q]).result. From 0b2877136460871cf17a6240a360361e6439d074 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 20 May 2015 21:06:20 +1000 Subject: [PATCH 31/41] Removing unrequired specs for variant/product 'units_show' --- spec/controllers/spree/api/products_controller_spec.rb | 7 ------- spec/controllers/spree/api/variants_controller_spec.rb | 7 ------- 2 files changed, 14 deletions(-) diff --git a/spec/controllers/spree/api/products_controller_spec.rb b/spec/controllers/spree/api/products_controller_spec.rb index eb56859365..312c6a29b4 100644 --- a/spec/controllers/spree/api/products_controller_spec.rb +++ b/spec/controllers/spree/api/products_controller_spec.rb @@ -13,7 +13,6 @@ module Spree let!(:product3) { FactoryGirl.create(:product, supplier: supplier) } let(:product_other_supplier) { FactoryGirl.create(:product, supplier: supplier2) } let(:attributes) { [:id, :name, :supplier, :price, :on_hand, :available_on, :permalink_live] } - let(:unit_attributes) { [:id, :name, :group_buy_unit_size, :variant_unit] } before do stub_authentication! @@ -72,12 +71,6 @@ module Spree attributes.all?{ |attr| keys.include? attr }.should == true end - it "retrieves a list of products with attributes relating to units" do - spree_get :show, { :id => product1.id, :template => "units_show", :format => :json } - keys = json_response.keys.map{ |key| key.to_sym } - unit_attributes.all?{ |attr| keys.include? attr }.should == true - end - it "sorts products in ascending id order" do spree_get :index, { :template => 'bulk_index', :format => :json } ids = json_response.map{ |product| product['id'] } diff --git a/spec/controllers/spree/api/variants_controller_spec.rb b/spec/controllers/spree/api/variants_controller_spec.rb index 5df1a40dde..5fb9f2f2a0 100644 --- a/spec/controllers/spree/api/variants_controller_spec.rb +++ b/spec/controllers/spree/api/variants_controller_spec.rb @@ -9,7 +9,6 @@ module Spree let!(:variant2) { FactoryGirl.create(:variant) } let!(:variant3) { FactoryGirl.create(:variant) } let(:attributes) { [:id, :options_text, :price, :on_hand, :unit_value, :unit_description, :on_demand, :display_as, :display_name] } - let(:unit_attributes) { [:id, :unit_text, :unit_value] } before do stub_authentication! @@ -25,12 +24,6 @@ module Spree attributes.all?{ |attr| keys.include? attr }.should == true end - it "retrieves a list of variants with attributes relating to units" do - spree_get :show, { :id => variant1.id, :template => "units_show", :format => :json } - keys = json_response.keys.map{ |key| key.to_sym } - unit_attributes.all?{ |attr| keys.include? attr }.should == true - end - it "is denied access when trying to delete a variant" do product = create(:product) variant = product.master From 79a59e2e812fe6d126bff783fde38620f6cd269c Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 21 May 2015 13:34:32 +1000 Subject: [PATCH 32/41] Add order_with_distributor factory --- spec/factories.rb | 8 +-- .../admin/bulk_order_management_spec.rb | 64 +++++++++---------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 6cfddd85a5..cb4d06b35e 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -183,6 +183,10 @@ FactoryGirl.define do end end + factory :order_with_distributor, :parent => :order do + distributor { create(:distributor_enterprise) } + end + factory :zone_with_member, :parent => :zone do default_tax true @@ -238,10 +242,6 @@ FactoryGirl.modify do unit_description '' end - factory :order do - distributor { create(:distributor_enterprise) } - end - factory :shipping_method do distributors { [Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise)] } display_on '' diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index 1058bebfc0..032acaca4e 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -18,9 +18,9 @@ feature %q{ end context "displaying the list of line items" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o3) { FactoryGirl.create(:order, state: 'address', completed_at: nil ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o3) { FactoryGirl.create(:order_with_distributor, state: 'address', completed_at: nil ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } let!(:li3) { FactoryGirl.create(:line_item, order: o3 ) } @@ -41,8 +41,8 @@ feature %q{ end context "displaying individual columns" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, bill_address: FactoryGirl.create(:address) ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, bill_address: nil ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, bill_address: FactoryGirl.create(:address) ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, bill_address: nil ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2, product: FactoryGirl.create(:product_with_option_types) ) } @@ -94,7 +94,7 @@ feature %q{ end context "tracking changes" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1, :quantity => 5 ) } before :each do @@ -117,7 +117,7 @@ feature %q{ end context "submitting data to the server" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1, :quantity => 5 ) } before :each do @@ -141,7 +141,7 @@ feature %q{ admin_user = quick_login_as_admin end - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1, :quantity => 5 ) } context "using column display toggle" do @@ -171,7 +171,7 @@ feature %q{ context "supplier filter" do let!(:s1) { create(:supplier_enterprise) } let!(:s2) { create(:supplier_enterprise) } - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1, product: create(:product, supplier: s1) ) } let!(:li2) { FactoryGirl.create(:line_item, order: o1, product: create(:product, supplier: s2) ) } @@ -205,8 +205,8 @@ feature %q{ context "distributor filter" do let!(:d1) { create(:distributor_enterprise) } let!(:d2) { create(:distributor_enterprise) } - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: d1 ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: d2 ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, distributor: d1 ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, distributor: d2 ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } @@ -241,8 +241,8 @@ feature %q{ let!(:distributor) { create(:distributor_enterprise) } let!(:oc1) { FactoryGirl.create(:simple_order_cycle, distributors: [distributor]) } let!(:oc2) { FactoryGirl.create(:simple_order_cycle, distributors: [distributor]) } - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, order_cycle: oc1 ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, order_cycle: oc2 ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, order_cycle: oc1 ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, order_cycle: oc2 ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } @@ -284,8 +284,8 @@ feature %q{ let!(:oc2) { FactoryGirl.create(:simple_order_cycle, suppliers: [s2], distributors: [d2] ) } let!(:p1) { FactoryGirl.create(:product, supplier: s1) } let!(:p2) { FactoryGirl.create(:product, supplier: s2) } - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: d1, order_cycle: oc1 ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: d2, order_cycle: oc2 ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, distributor: d1, order_cycle: oc1 ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, distributor: d2, order_cycle: oc2 ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1, product: p1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2, product: p2 ) } @@ -328,9 +328,9 @@ feature %q{ end context "using quick search" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o3) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } let!(:li3) { FactoryGirl.create(:line_item, order: o3 ) } @@ -355,9 +355,9 @@ feature %q{ end context "using date restriction controls" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: (Date.today - 8).strftime("%F %T") ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o3) { FactoryGirl.create(:order, state: 'complete', completed_at: (Date.today + 2).strftime("%F %T") ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: (Date.today - 8).strftime("%F %T") ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o3) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: (Date.today + 2).strftime("%F %T") ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1, :quantity => 1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2, :quantity => 2 ) } let!(:li3) { FactoryGirl.create(:line_item, order: o3, :quantity => 3 ) } @@ -429,8 +429,8 @@ feature %q{ end context "bulk action controls" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } @@ -496,8 +496,8 @@ feature %q{ context "using action buttons" do context "using edit buttons" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } @@ -515,8 +515,8 @@ feature %q{ end context "using delete buttons" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } @@ -539,13 +539,13 @@ feature %q{ end context "clicking the link on variant name" do - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } let!(:li2) { FactoryGirl.create(:line_item, order: o2 ) } let!(:p3) { FactoryGirl.create(:product_with_option_types, group_buy: true, group_buy_unit_size: 5000, variant_unit: "weight", variants: [FactoryGirl.create(:variant, unit_value: 1000)] ) } let!(:v3) { p3.variants.first } - let!(:o3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now ) } + let!(:o3) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now ) } let!(:li3) { FactoryGirl.create(:line_item, order: o3, variant: v3, quantity: 3, max_quantity: 6 ) } let!(:li4) { FactoryGirl.create(:line_item, order: o2, variant: v3, quantity: 1, max_quantity: 3 ) } @@ -605,8 +605,8 @@ feature %q{ let(:s1) { create(:supplier_enterprise, name: 'First Supplier') } let(:d1) { create(:distributor_enterprise, name: 'First Distributor') } let(:d2) { create(:distributor_enterprise, name: 'Another Distributor') } - let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: d1 ) } - let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: d2 ) } + let!(:o1) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, distributor: d1 ) } + let!(:o2) { FactoryGirl.create(:order_with_distributor, state: 'complete', completed_at: Time.now, distributor: d2 ) } let!(:line_item_distributed) { FactoryGirl.create(:line_item, order: o1, product: create(:product, supplier: s1) ) } let!(:line_item_not_distributed) { FactoryGirl.create(:line_item, order: o2, product: create(:product, supplier: s1) ) } From cd44d43b3ebcea97c2b17387810c094f275f184d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 21 May 2015 17:48:35 +1000 Subject: [PATCH 33/41] Adding price to line_item serializer (oops, forgot to include when merging in master) --- app/serializers/api/admin/line_item_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/api/admin/line_item_serializer.rb b/app/serializers/api/admin/line_item_serializer.rb index 7e2917acdd..21fde91145 100644 --- a/app/serializers/api/admin/line_item_serializer.rb +++ b/app/serializers/api/admin/line_item_serializer.rb @@ -1,5 +1,5 @@ class Api::Admin::LineItemSerializer < ActiveModel::Serializer - attributes :id, :quantity, :max_quantity, :supplier, :unit_value, :units_product, :units_variant + attributes :id, :quantity, :max_quantity, :supplier, :price, :unit_value, :units_product, :units_variant def supplier Api::Admin::IdNameSerializer.new(object.product.supplier).serializable_hash From a5482c269f6ca18ead3bf5fe0c510b7f2f465671 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 24 May 2015 09:52:34 +1000 Subject: [PATCH 34/41] #541 Fixed issue of showing negative 'more' numbers, and only working for current Hub --- .../stylesheets/darkswarm/hub_node.css.sass | 34 ++++++++++--------- app/views/home/_fat.html.haml | 2 +- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/hub_node.css.sass b/app/assets/stylesheets/darkswarm/hub_node.css.sass index dfe4d42074..94950fa22c 100644 --- a/app/assets/stylesheets/darkswarm/hub_node.css.sass +++ b/app/assets/stylesheets/darkswarm/hub_node.css.sass @@ -65,6 +65,24 @@ .active_table_row:nth-child(2) padding-bottom: 0.75rem + + .producers-list + li.more-producers-link + .less + display: none + a:hover + text-decoration: underline + li.additional-producer + display: none + &.show-more-producers + li.additional-producer + display: block + li.more-producers-link + .more + display: none + .less + display: block + //CURRENT hub (shows selected hub) &.current //overwrites active_table @@ -83,22 +101,6 @@ .active_table_row:first-child .skinny-head background-color: rgba(255,255,255,0.85) - .producers-list - li.more-producers-link - .less - display: none - a:hover - text-decoration: underline - li.additional-producer - display: none - &.show-more-producers - li.additional-producer - display: block - li.more-producers-link - .more - display: none - .less - display: block //INACTIVE - closed hub &.inactive diff --git a/app/views/home/_fat.html.haml b/app/views/home/_fat.html.haml index 092d9e898d..a5543fa2a3 100644 --- a/app/views/home/_fat.html.haml +++ b/app/views/home/_fat.html.haml @@ -26,7 +26,7 @@ %enterprise-modal %i.ofn-i_036-producers %span{"bo-text" => "enterprise.name"} - %li{"data-is-link" => "true", "class" => "more-producers-link"} + %li{"data-is-link" => "true", "class" => "more-producers-link", "bo-show" => "hub.producers.length>7"} %a{"ng-click" => "toggleMoreProducers=!toggleMoreProducers"} .more + From ccf1e2951cce17cd4c4ff9f348a1e690976cc466 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 28 May 2015 10:45:47 +1000 Subject: [PATCH 35/41] Fix intermittent failure in permissions spec --- spec/lib/open_food_network/permissions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index cf2aa1a217..646a219a5b 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -47,7 +47,7 @@ module OpenFoodNetwork expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) } expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) } expect(permissions).to receive(:related_enterprises_granted).with(:some_permission) { Enterprise.where(id: e4) } - expect(permissions.send(:managed_and_related_enterprises_with, :some_permission)).to eq [e1, e3, e4] + expect(permissions.send(:managed_and_related_enterprises_with, :some_permission)).to match_array [e1, e3, e4] end end end From 97e49c2bdb4c3173ec357ce8a498bb54e7235123 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 28 May 2015 10:46:08 +1000 Subject: [PATCH 36/41] Replace 'array.sort.should == expected.sort' pattern with match_array --- .../spree/admin/orders_controller_spec.rb | 4 ++-- .../spree/admin/reports_controller_spec.rb | 12 +++++----- .../spree/admin/variants_controller_spec.rb | 2 +- spec/features/admin/enterprise_groups_spec.rb | 2 +- .../admin/enterprise_relationships_spec.rb | 2 +- spec/features/admin/order_cycles_spec.rb | 20 ++++++++-------- spec/features/admin/products_spec.rb | 4 ++-- spec/features/admin/shipping_methods_spec.rb | 2 +- .../customers_report_spec.rb | 18 +++++++------- .../order_cycle_form_applicator_spec.rb | 16 ++++++------- .../lib/open_food_network/permissions_spec.rb | 2 +- .../products_and_inventory_report_spec.rb | 22 ++++++++--------- spec/models/enterprise_fee_spec.rb | 4 ++-- spec/models/enterprise_relationship_spec.rb | 8 +++---- spec/models/enterprise_spec.rb | 22 ++++++++--------- spec/models/exchange_spec.rb | 6 ++--- spec/models/order_cycle_spec.rb | 24 +++++++++---------- spec/models/spree/line_item_spec.rb | 2 +- spec/models/spree/payment_spec.rb | 4 ++-- spec/models/spree/product_spec.rb | 2 +- spec/models/spree/shipping_method_spec.rb | 2 +- spec/models/spree/variant_spec.rb | 2 +- spec/models/variant_override_spec.rb | 2 +- 23 files changed, 92 insertions(+), 92 deletions(-) diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb index ac6a5532b2..dae1edcf29 100644 --- a/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/spec/controllers/spree/admin/orders_controller_spec.rb @@ -143,7 +143,7 @@ describe Spree::Admin::OrdersController do end it "only displays line items from orders for which my enterprise is the order_cycle coorinator" do - json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.sort.should == [line_item1.id, line_item2.id, line_item3.id].sort + json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.should match_array [line_item1.id, line_item2.id, line_item3.id] end end @@ -159,7 +159,7 @@ describe Spree::Admin::OrdersController do end it "only displays line items from orders for which my enterprise is a distributor" do - json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.sort.should == [line_item1.id, line_item2.id].sort + json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.should match_array [line_item1.id, line_item2.id] end end end diff --git a/spec/controllers/spree/admin/reports_controller_spec.rb b/spec/controllers/spree/admin/reports_controller_spec.rb index 58d406519b..e930c1e339 100644 --- a/spec/controllers/spree/admin/reports_controller_spec.rb +++ b/spec/controllers/spree/admin/reports_controller_spec.rb @@ -170,17 +170,17 @@ describe Spree::Admin::ReportsController do it "should build distributors for the current user" do spree_get :products_and_inventory - assigns(:distributors).sort.should == [c1, c2, d1, d2, d3].sort + assigns(:distributors).should match_array [c1, c2, d1, d2, d3] end it "builds suppliers for the current user" do spree_get :products_and_inventory - assigns(:suppliers).sort.should == [s1, s2, s3].sort + assigns(:suppliers).should match_array [s1, s2, s3] end it "builds order cycles for the current user" do spree_get :products_and_inventory - assigns(:order_cycles).sort.should == [ocB, ocA].sort + assigns(:order_cycles).should match_array [ocB, ocA] end it "assigns report types" do @@ -211,17 +211,17 @@ describe Spree::Admin::ReportsController do it "should build distributors for the current user" do spree_get :customers - assigns(:distributors).sort.should == [c1, c2, d1, d2, d3].sort + assigns(:distributors).should match_array [c1, c2, d1, d2, d3] end it "builds suppliers for the current user" do spree_get :customers - assigns(:suppliers).sort.should == [s1, s2, s3].sort + assigns(:suppliers).should match_array [s1, s2, s3] end it "builds order cycles for the current user" do spree_get :customers - assigns(:order_cycles).sort.should == [ocB, ocA].sort + assigns(:order_cycles).should match_array [ocB, ocA] end it "assigns report types" do diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index 22972a7301..965ff8d7ce 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -30,7 +30,7 @@ module Spree it "does not filter when no distributor or order cycle is specified" do spree_get :search, q: 'Prod' - assigns(:variants).sort.should == [p1.master, p2.master].sort + assigns(:variants).should match_array [p1.master, p2.master] end end end diff --git a/spec/features/admin/enterprise_groups_spec.rb b/spec/features/admin/enterprise_groups_spec.rb index 59712f3bfe..2adb96c0a5 100644 --- a/spec/features/admin/enterprise_groups_spec.rb +++ b/spec/features/admin/enterprise_groups_spec.rb @@ -50,7 +50,7 @@ feature %q{ eg.name.should == 'EGEGEG' eg.description.should == 'This is a description' eg.on_front_page.should be_true - eg.enterprises.sort.should == [e1, e2].sort + eg.enterprises.should match_array [e1, e2] end scenario "editing an enterprise group" do diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 3a2d9692de..ef0f1e5537 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -50,7 +50,7 @@ feature %q{ page.should have_relationship e1, e2, ['to add to order cycle', 'to override variant details', 'to edit profile'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first er.should be_present - er.permissions.map(&:name).sort.should == ['add_to_order_cycle', 'edit_profile', 'create_variant_overrides'].sort + er.permissions.map(&:name).should match_array ['add_to_order_cycle', 'edit_profile', 'create_variant_overrides'] end diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index cb96799fbb..eed6551bfa 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -354,7 +354,7 @@ feature %q{ page.should have_selector 'td.distributors', text: 'My distributor' # And my coordinator fees should have been configured - OrderCycle.last.coordinator_fee_ids.sort.should == [coordinator_fee1.id, coordinator_fee2.id].sort + OrderCycle.last.coordinator_fee_ids.should match_array [coordinator_fee1.id, coordinator_fee2.id] # And my supplier fees should have been configured OrderCycle.last.exchanges.incoming.last.enterprise_fee_ids.should == [supplier_fee2.id] @@ -364,7 +364,7 @@ feature %q{ # And it should have some variants selected selected_initial_variants = initial_variants.take initial_variants.size - 1 - OrderCycle.last.variants.map(&:id).sort.should == (selected_initial_variants.map(&:id) + [v1.id, v2.id]).sort + OrderCycle.last.variants.map(&:id).should match_array (selected_initial_variants.map(&:id) + [v1.id, v2.id]) # And the collection details should have been updated OrderCycle.last.exchanges.where(pickup_time: 'New time 0', pickup_instructions: 'New instructions 0').should be_present @@ -568,9 +568,9 @@ feature %q{ flash_message.should == "Your order cycle has been created." order_cycle = OrderCycle.find_by_name('My order cycle') - order_cycle.suppliers.sort.should == [supplier_managed, supplier_permitted].sort + order_cycle.suppliers.should match_array [supplier_managed, supplier_permitted] order_cycle.coordinator.should == distributor_managed - order_cycle.distributors.sort.should == [distributor_managed, distributor_permitted].sort + order_cycle.distributors.should match_array [distributor_managed, distributor_permitted] end scenario "editing an order cycle we can see (and for now, edit) all exchanges in the order cycle" do @@ -592,9 +592,9 @@ feature %q{ page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort + oc.suppliers.should match_array [supplier_managed, supplier_permitted, supplier_unmanaged] oc.coordinator.should == distributor_managed - oc.distributors.sort.should == [distributor_managed, distributor_permitted, distributor_unmanaged].sort + oc.distributors.should match_array [distributor_managed, distributor_permitted, distributor_unmanaged] end scenario "editing an order cycle" do @@ -683,9 +683,9 @@ feature %q{ page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort + oc.suppliers.should match_array [supplier_managed, supplier_permitted, supplier_unmanaged] oc.coordinator.should == distributor_managed - oc.distributors.sort.should == [distributor_managed, distributor_permitted, distributor_unmanaged].sort + oc.distributors.should match_array [distributor_managed, distributor_permitted, distributor_unmanaged] end end @@ -736,9 +736,9 @@ feature %q{ page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort + oc.suppliers.should match_array [supplier_managed, supplier_permitted, supplier_unmanaged] oc.coordinator.should == distributor_managed - oc.distributors.sort.should == [my_distributor, distributor_managed, distributor_permitted, distributor_unmanaged].sort + oc.distributors.should match_array [my_distributor, distributor_managed, distributor_permitted, distributor_unmanaged] end end end diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index 740de12045..95569799b4 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -68,10 +68,10 @@ feature %q{ click_button 'Update' product.reload - product.distributors.sort.should == [@distributors[0], @distributors[2]].sort + product.distributors.should match_array [@distributors[0], @distributors[2]] - product.product_distributions.map { |pd| pd.enterprise_fee }.sort.should == [@enterprise_fees[0], @enterprise_fees[2]].sort + product.product_distributions.map { |pd| pd.enterprise_fee }.should match_array [@enterprise_fees[0], @enterprise_fees[2]] end scenario "making a product into a group buy product" do diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index 2aeeb1db1d..2857fa6502 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -36,7 +36,7 @@ feature 'shipping methods' do sm = Spree::ShippingMethod.last sm.name.should == 'Carrier Pidgeon' - sm.distributors.sort.should == [d1, d2].sort + sm.distributors.should match_array [d1, d2] end it "at checkout, user can only see shipping methods for their current distributor (checkout spec)" diff --git a/spec/lib/open_food_network/customers_report_spec.rb b/spec/lib/open_food_network/customers_report_spec.rb index c655fa626e..289a32929e 100644 --- a/spec/lib/open_food_network/customers_report_spec.rb +++ b/spec/lib/open_food_network/customers_report_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' module OpenFoodNetwork describe CustomersReport do context "as a site admin" do - let(:user) do + let(:user) do user = create(:user) user.spree_roles << Spree::Role.find_or_create_by_name!("admin") user @@ -44,14 +44,14 @@ module OpenFoodNetwork it "builds a table from a list of variants" do a = create(:address) d = create(:distributor_enterprise) - o = create(:order, distributor: d, bill_address: a) + o = create(:order, distributor: d, bill_address: a) o.shipping_method = create(:shipping_method) subject.stub(:orders).and_return [o] subject.table.should == [[ - a.firstname, a.lastname, - [a.address1, a.address2, a.city].join(" "), - o.email, a.phone, d.name, + a.firstname, a.lastname, + [a.address1, a.address2, a.city].join(" "), + o.email, a.phone, d.name, [d.address.address1, d.address.address2, d.address.city].join(" "), o.shipping_method.name ]] @@ -74,7 +74,7 @@ module OpenFoodNetwork end context "as an enterprise user" do - let(:user) do + let(:user) do user = create(:user) user.spree_roles = [] user.save! @@ -131,7 +131,7 @@ module OpenFoodNetwork order2.line_items << create(:line_item, product: product2) subject.stub(:params).and_return(supplier_id: supplier.id) - subject.filter(orders).sort.should == [order1] + subject.filter(orders).should == [order1] end it "filters to a specific distributor" do @@ -141,7 +141,7 @@ module OpenFoodNetwork order2 = create(:order, distributor: d2) subject.stub(:params).and_return(distributor_id: d1.id) - subject.filter(orders).sort.should == [order1] + subject.filter(orders).should == [order1] end it "filters to a specific cycle" do @@ -151,7 +151,7 @@ module OpenFoodNetwork order2 = create(:order, order_cycle: oc2) subject.stub(:params).and_return(order_cycle_id: oc1.id) - subject.filter(orders).sort.should == [order1] + subject.filter(orders).should == [order1] end end end diff --git a/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb b/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb index 6a03964039..5314c4376c 100644 --- a/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb +++ b/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb @@ -300,8 +300,8 @@ module OpenFoodNetwork expect(exchange.sender).to eq sender expect(exchange.receiver).to eq receiver expect(exchange.incoming).to eq incoming - expect(exchange.variants.sort).to eq [variant1, variant2].sort - expect(exchange.enterprise_fees.sort).to eq [enterprise_fee1, enterprise_fee2].sort + expect(exchange.variants).to match_array [variant1, variant2] + expect(exchange.enterprise_fees).to match_array [enterprise_fee1, enterprise_fee2] applicator.send(:touched_exchanges).should == [exchange] end @@ -345,8 +345,8 @@ module OpenFoodNetwork it "updates the variants, enterprise fees and pickup information of the exchange" do exchange.reload - expect(exchange.variants.sort).to eq [variant1, variant3].sort - expect(exchange.enterprise_fees.sort).to eq [enterprise_fee2, enterprise_fee3] + expect(exchange.variants).to match_array [variant1, variant3] + expect(exchange.enterprise_fees).to match_array [enterprise_fee2, enterprise_fee3] expect(exchange.pickup_time).to eq 'New Pickup Time' expect(exchange.pickup_instructions).to eq 'New Pickup Instructions' expect(applicator.send(:touched_exchanges)).to eq [exchange] @@ -364,8 +364,8 @@ module OpenFoodNetwork it "updates the variants, enterprise fees and pickup information of the exchange" do exchange.reload - expect(exchange.variants.sort).to eq [variant1, variant3].sort - expect(exchange.enterprise_fees.sort).to eq [enterprise_fee2, enterprise_fee3] + expect(exchange.variants).to match_array [variant1, variant3] + expect(exchange.enterprise_fees).to match_array [enterprise_fee2, enterprise_fee3] expect(exchange.pickup_time).to eq 'New Pickup Time' expect(exchange.pickup_instructions).to eq 'New Pickup Instructions' expect(applicator.send(:touched_exchanges)).to eq [exchange] @@ -383,8 +383,8 @@ module OpenFoodNetwork it "updates the variants in the exchange, but not the fees or pickup information" do exchange.reload - expect(exchange.variants.sort).to eq [variant1, variant3].sort - expect(exchange.enterprise_fees.sort).to eq [enterprise_fee1, enterprise_fee2] + expect(exchange.variants).to match_array [variant1, variant3] + expect(exchange.enterprise_fees).to match_array [enterprise_fee1, enterprise_fee2] expect(exchange.pickup_time).to_not eq 'New Pickup Time' expect(exchange.pickup_instructions).to_not eq 'New Pickup Instructions' expect(applicator.send(:touched_exchanges)).to eq [exchange] diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 646a219a5b..4224a00d89 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -98,7 +98,7 @@ module OpenFoodNetwork {1 => [e1.id], 2 => [e1.id, e2.id]} end - permissions.variant_override_producers.sort.should == [e1, e2].sort + permissions.variant_override_producers.should match_array [e1, e2] end end diff --git a/spec/lib/open_food_network/products_and_inventory_report_spec.rb b/spec/lib/open_food_network/products_and_inventory_report_spec.rb index 2e23919fe6..4ae93e7e64 100644 --- a/spec/lib/open_food_network/products_and_inventory_report_spec.rb +++ b/spec/lib/open_food_network/products_and_inventory_report_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' module OpenFoodNetwork describe ProductsAndInventoryReport do context "As a site admin" do - let(:user) do + let(:user) do user = create(:user) user.spree_roles << Spree::Role.find_or_create_by_name!("admin") user @@ -14,7 +14,7 @@ module OpenFoodNetwork it "Should return headers" do subject.header.should == [ - "Supplier", + "Supplier", "Producer Suburb", "Product", "Product Properties", @@ -63,7 +63,7 @@ module OpenFoodNetwork context "As an enterprise user" do let(:supplier) { create(:supplier_enterprise) } - let(:enterprise_user) do + let(:enterprise_user) do user = create(:user) user.enterprise_roles.create(enterprise: supplier) user.spree_roles = [] @@ -79,7 +79,7 @@ module OpenFoodNetwork variant_1 = create(:variant, product: product1) variant_2 = create(:variant, product: product1) - subject.child_variants.sort.should == [variant_1, variant_2].sort + subject.child_variants.should match_array [variant_1, variant_2] end it "should only return variants managed by the user" do @@ -87,7 +87,7 @@ module OpenFoodNetwork product2 = create(:simple_product, supplier: supplier) variant_1 = create(:variant, product: product1) variant_2 = create(:variant, product: product2) - + subject.child_variants.should == [variant_2] end end @@ -96,15 +96,15 @@ module OpenFoodNetwork it "should only return variants managed by the user" do product1 = create(:simple_product, supplier: create(:supplier_enterprise)) product2 = create(:simple_product, supplier: supplier) - + subject.master_variants.should == [product2.master] end it "doesn't return master variants with siblings" do product = create(:simple_product, supplier: supplier) - create(:variant, product: product) - - subject.master_variants.should be_empty + create(:variant, product: product) + + subject.master_variants.should be_empty end end @@ -113,13 +113,13 @@ module OpenFoodNetwork it "should return unfiltered variants sans-params" do product1 = create(:simple_product, supplier: supplier) product2 = create(:simple_product, supplier: supplier) - subject.filter(Spree::Variant.scoped).sort.should == [product1.master, product2.master].sort + subject.filter(Spree::Variant.scoped).should match_array [product1.master, product2.master] end it "should filter deleted products" do product1 = create(:simple_product, supplier: supplier) product2 = create(:simple_product, supplier: supplier) product2.delete - subject.filter(Spree::Variant.scoped).sort.should == [product1.master].sort + subject.filter(Spree::Variant.scoped).should match_array [product1.master] end describe "based on report type" do it "returns only variants on hand" do diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index fc72f383d2..f0f9df1f0b 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -43,7 +43,7 @@ describe EnterpriseFee do ef3 = create(:enterprise_fee, calculator: Spree::Calculator::PerItem.new) ef4 = create(:enterprise_fee, calculator: Spree::Calculator::PriceSack.new) - EnterpriseFee.per_item.sort.should == [ef1, ef2, ef3, ef4].sort + EnterpriseFee.per_item.should match_array [ef1, ef2, ef3, ef4] end end @@ -52,7 +52,7 @@ describe EnterpriseFee do ef1 = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new) ef2 = create(:enterprise_fee, calculator: Spree::Calculator::FlexiRate.new) - EnterpriseFee.per_order.sort.should == [ef1, ef2].sort + EnterpriseFee.per_order.should match_array [ef1, ef2] end it "does not return fees with any other calculator" do diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index e87c204037..cbbdd00504 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -34,7 +34,7 @@ describe EnterpriseRelationship do it "creates permissions with a list" do er = EnterpriseRelationship.create! parent: e1, child: e2, permissions_list: ['one', 'two'] er.reload - er.permissions.map(&:name).sort.should == ['one', 'two'].sort + er.permissions.map(&:name).should match_array ['one', 'two'] end it "does nothing when the list is nil" do @@ -50,11 +50,11 @@ describe EnterpriseRelationship do let!(:er3) { create(:enterprise_relationship, parent: e1, child: e3) } it "finds relationships that grant permissions to some enterprises" do - EnterpriseRelationship.permitting([e1, e2]).sort.should == [er1, er2].sort + EnterpriseRelationship.permitting([e1, e2]).should match_array [er1, er2] end it "finds relationships that are granted by particular enterprises" do - EnterpriseRelationship.permitted_by([e1, e2]).sort.should == [er1, er3].sort + EnterpriseRelationship.permitted_by([e1, e2]).should match_array [er1, er3] end end @@ -66,7 +66,7 @@ describe EnterpriseRelationship do er3 = create(:enterprise_relationship, parent: e3, child: e1, permissions_list: ['three', 'four']) - EnterpriseRelationship.with_permission('two').sort.should == [er1, er2].sort + EnterpriseRelationship.with_permission('two').should match_array [er1, er2] end end end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index df1b58aca6..ed4669c0c2 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -118,7 +118,7 @@ describe Enterprise do let!(:er2) { create(:enterprise_relationship, parent_id: e.id, child_id: c.id) } it "finds relatives" do - e.relatives.sort.should == [p, c].sort + e.relatives.should match_array [p, c] end it "scopes relatives to visible distributors" do @@ -422,7 +422,7 @@ describe Enterprise do create(:product, :distributors => [d1], :on_hand => 5) create(:product, :distributors => [d3], :on_hand => 0) - Enterprise.with_distributed_active_products_on_hand.sort.should == [d1, d2] + Enterprise.with_distributed_active_products_on_hand.should match_array [d1, d2] end it "returns distributors with available products in stock" do @@ -438,7 +438,7 @@ describe Enterprise do create(:product, :distributors => [d4], :on_hand => 0) create(:product, :distributors => [d5]).delete - Enterprise.with_distributed_active_products_on_hand.sort.should == [d1, d2] + Enterprise.with_distributed_active_products_on_hand.should match_array [d1, d2] Enterprise.with_distributed_active_products_on_hand.distinct_count.should == 2 end end @@ -458,7 +458,7 @@ describe Enterprise do create(:product, :supplier => d4, :on_hand => 0) create(:product, :supplier => d5).delete - Enterprise.with_supplied_active_products_on_hand.sort.should == [d1, d2] + Enterprise.with_supplied_active_products_on_hand.should match_array [d1, d2] Enterprise.with_supplied_active_products_on_hand.distinct_count.should == 2 end end @@ -485,7 +485,7 @@ describe Enterprise do p1 = create(:simple_product, supplier: s1) p2 = create(:simple_product, supplier: s2) - Enterprise.supplying_variant_in([p1.master, p2.master]).sort.should == [s1, s2].sort + Enterprise.supplying_variant_in([p1.master, p2.master]).should match_array [s1, s2] end it "does not return duplicates" do @@ -625,9 +625,9 @@ describe Enterprise do er = EnterpriseRelationship.where(parent_id: opts[:from], child_id: opts[:to]).last er.should_not be_nil if opts[:with] == :all_permissions - er.permissions.map(&:name).sort.should == ['add_to_order_cycle', 'manage_products', 'edit_profile', 'create_variant_overrides'].sort + er.permissions.map(&:name).should match_array ['add_to_order_cycle', 'manage_products', 'edit_profile', 'create_variant_overrides'] elsif opts.key? :with - er.permissions.map(&:name).sort.should == opts[:with].map(&:to_s).sort + er.permissions.map(&:name).should match_array opts[:with].map(&:to_s) end end end @@ -674,7 +674,7 @@ describe Enterprise do d = create(:distributor_enterprise) p = create(:product, distributors: [d]) v = create(:variant, product: p) - d.distributed_variants.sort.should == [p.master, v].sort + d.distributed_variants.should match_array [p.master, v] end it "finds variants distributed by order cycle" do @@ -696,7 +696,7 @@ describe Enterprise do d = create(:distributor_enterprise) p = create(:product, distributors: [d]) v = create(:variant, product: p) - d.product_distribution_variants.sort.should == [p.master, v].sort + d.product_distribution_variants.should match_array [p.master, v] end it "does not find variants distributed by order cycle" do @@ -752,12 +752,12 @@ describe Enterprise do it "gets all taxons of all distributed products" do Spree::Product.stub(:in_distributor).and_return [product1, product2] - distributor.distributed_taxons.sort.should == [taxon1, taxon2].sort + distributor.distributed_taxons.should match_array [taxon1, taxon2] end it "gets all taxons of all supplied products" do Spree::Product.stub(:in_supplier).and_return [product1, product2] - supplier.supplied_taxons.sort.should == [taxon1, taxon2].sort + supplier.supplied_taxons.should match_array [taxon1, taxon2] end end diff --git a/spec/models/exchange_spec.rb b/spec/models/exchange_spec.rb index 82f360aa16..0df7aeafab 100644 --- a/spec/models/exchange_spec.rb +++ b/spec/models/exchange_spec.rb @@ -170,17 +170,17 @@ describe Exchange do it "finds exchanges coming from any of a number of enterprises" do Exchange.from_enterprises([coordinator]).should == [outgoing_exchange] - Exchange.from_enterprises([supplier, coordinator]).sort.should == [incoming_exchange, outgoing_exchange].sort + Exchange.from_enterprises([supplier, coordinator]).should match_array [incoming_exchange, outgoing_exchange] end it "finds exchanges going to any of a number of enterprises" do Exchange.to_enterprises([coordinator]).should == [incoming_exchange] - Exchange.to_enterprises([coordinator, distributor]).sort.should == [incoming_exchange, outgoing_exchange].sort + Exchange.to_enterprises([coordinator, distributor]).should match_array [incoming_exchange, outgoing_exchange] end it "finds exchanges involving any of a number of enterprises" do Exchange.involving([supplier]).should == [incoming_exchange] - Exchange.involving([coordinator]).sort.should == [incoming_exchange, outgoing_exchange].sort + Exchange.involving([coordinator]).should match_array [incoming_exchange, outgoing_exchange] Exchange.involving([distributor]).should == [outgoing_exchange] end end diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 817d2e8e93..46cefd0600 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -37,7 +37,7 @@ describe OrderCycle do oc_undated = create(:simple_order_cycle, orders_open_at: nil, orders_close_at: nil) OrderCycle.active.should == [oc_active] - OrderCycle.inactive.sort.should == [oc_not_yet_open, oc_already_closed].sort + OrderCycle.inactive.should match_array [oc_not_yet_open, oc_already_closed] OrderCycle.upcoming.should == [oc_not_yet_open] OrderCycle.closed.should == [oc_already_closed] OrderCycle.undated.should == [oc_undated] @@ -153,7 +153,7 @@ describe OrderCycle do e2 = create(:exchange, incoming: true, order_cycle: oc, receiver: oc.coordinator, sender: create(:enterprise)) - oc.suppliers.sort.should == [e1.sender, e2.sender].sort + oc.suppliers.should match_array [e1.sender, e2.sender] end it "reports its distributors" do @@ -164,7 +164,7 @@ describe OrderCycle do e2 = create(:exchange, incoming: false, order_cycle: oc, sender: oc.coordinator, receiver: create(:enterprise)) - oc.distributors.sort.should == [e1.receiver, e2.receiver].sort + oc.distributors.should match_array [e1.receiver, e2.receiver] end it "checks for existance of distributors" do @@ -215,11 +215,11 @@ describe OrderCycle do end it "reports on the variants exchanged" do - @oc.variants.sort.should == [@p0.master, @p1.master, @p2.master, @p2_v].sort + @oc.variants.should match_array [@p0.master, @p1.master, @p2.master, @p2_v] end it "reports on the variants distributed" do - @oc.distributed_variants.sort.should == [@p1.master, @p2.master, @p2_v].sort + @oc.distributed_variants.should match_array [@p1.master, @p2.master, @p2_v] end it "reports on the variants distributed by a particular distributor" do @@ -231,7 +231,7 @@ describe OrderCycle do end it "reports on the products exchanged" do - @oc.products.sort.should == [@p0, @p1, @p2] + @oc.products.should match_array [@p0, @p1, @p2] end end @@ -313,7 +313,7 @@ describe OrderCycle do @oc.pickup_time_for(@d2).should == '2-8pm Friday' end end - + describe "finding pickup instructions for a distributor" do it "returns the pickup instructions" do @oc.pickup_instructions_for(@d1).should == "Come get it!" @@ -375,7 +375,7 @@ describe OrderCycle do occ.coordinator_fee_ids.should_not be_empty occ.coordinator_fee_ids.should == oc.coordinator_fee_ids - + # to_h gives us a unique hash for each exchange # check that the clone has no additional exchanges occ.exchanges.map(&:to_h).all? do |ex| @@ -402,7 +402,7 @@ describe OrderCycle do describe "finding order cycles opening in the future" do it "should give the soonest opening order cycle for a distributor" do distributor = create(:distributor_enterprise) - oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 10.days.from_now, orders_close_at: 11.days.from_now) + oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 10.days.from_now, orders_close_at: 11.days.from_now) OrderCycle.first_opening_for(distributor).should == oc end @@ -411,12 +411,12 @@ describe OrderCycle do OrderCycle.first_opening_for(distributor).should == nil end end - + describe "finding open order cycles" do it "should give the soonest closing order cycle for a distributor" do distributor = create(:distributor_enterprise) - oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 1.days.ago, orders_close_at: 11.days.from_now) - oc2 = create(:simple_order_cycle, name: 'oc 2', distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 12.days.from_now) + oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 1.days.ago, orders_close_at: 11.days.from_now) + oc2 = create(:simple_order_cycle, name: 'oc 2', distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 12.days.from_now) OrderCycle.first_closing_for(distributor).should == oc end end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 6166732b76..4058ba30e1 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -22,7 +22,7 @@ module Spree it "finds line items for products supplied by one of a number of enterprises" do LineItem.supplied_by_any([s1]).should == [li1] LineItem.supplied_by_any([s2]).should == [li2] - LineItem.supplied_by_any([s1, s2]).sort.should == [li1, li2].sort + LineItem.supplied_by_any([s1, s2]).should match_array [li1, li2] end end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 8a781e7700..0877959cbb 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -7,7 +7,7 @@ module Spree let(:payment) { create(:payment, source: create(:credit_card)) } it "can capture and void" do - payment.actions.sort.should == %w(capture void).sort + payment.actions.should match_array %w(capture void) end describe "when a payment has been taken" do @@ -17,7 +17,7 @@ module Spree end it "can void and credit" do - payment.actions.sort.should == %w(void credit).sort + payment.actions.should match_array %w(void credit) end end end diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 275d8b1970..287f60b400 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -545,7 +545,7 @@ module Spree ot3 = create(:option_type, name: 'unit_items', presentation: 'Items') ot4 = create(:option_type, name: 'foo_unit_bar', presentation: 'Foo') - Spree::Product.all_variant_unit_option_types.sort.should == [ot1, ot2, ot3].sort + Spree::Product.all_variant_unit_option_types.should match_array [ot1, ot2, ot3] end end diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index d6b821e890..baea2ac56a 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -15,7 +15,7 @@ module Spree sm.distributors << d1 sm.distributors << d2 - sm.reload.distributors.sort.should == [d1, d2].sort + sm.reload.distributors.should match_array [d1, d2] end it "finds shipping methods for a particular distributor" do diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 5a82f04ba7..866f17ff1d 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -25,7 +25,7 @@ module Spree end it "returns variants in stock or on demand, but not those that are neither" do - Variant.where(is_master: false).in_stock.sort.should == [@v_in_stock, @v_on_demand].sort + Variant.where(is_master: false).in_stock.should match_array [@v_in_stock, @v_on_demand] end end diff --git a/spec/models/variant_override_spec.rb b/spec/models/variant_override_spec.rb index ee06bd20ec..3a6bd9b1df 100644 --- a/spec/models/variant_override_spec.rb +++ b/spec/models/variant_override_spec.rb @@ -12,7 +12,7 @@ describe VariantOverride do let!(:vo2) { create(:variant_override, hub: hub2, variant: v) } it "finds variant overrides for a set of hubs" do - VariantOverride.for_hubs([hub1, hub2]).sort.should == [vo1, vo2].sort + VariantOverride.for_hubs([hub1, hub2]).should match_array [vo1, vo2] end end From 7f43dbf9bb23b237de19e6cd9b564cca4b394f37 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 28 May 2015 10:58:12 +1000 Subject: [PATCH 37/41] Fix further intermittent failures in permissions spec --- spec/lib/open_food_network/permissions_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 4224a00d89..1b6e23eeb0 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -13,7 +13,7 @@ module OpenFoodNetwork before { allow(user).to receive(:admin?) { true } } it "returns all enterprises" do - expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to eq [e1, e2] + expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to match_array [e1, e2] end end @@ -24,7 +24,7 @@ module OpenFoodNetwork it "returns only my managed enterprises any that have granting them P-OC" do expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) } expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) } - expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to eq [e1, e3] + expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to match_array [e1, e3] end end end @@ -34,7 +34,7 @@ module OpenFoodNetwork before { allow(user).to receive(:admin?) { true } } it "returns all enterprises" do - expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to eq [e1, e2] + expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to match_array [e1, e2] end end From 19448a182ecade379bc1efaba59181f7e766f851 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 27 May 2015 11:54:13 +1000 Subject: [PATCH 38/41] Add permalink field to enterprise groups --- app/models/enterprise_group.rb | 1 + .../20150527004427_add_permalink_to_groups.rb | 26 +++++++++++++++++++ db/schema.rb | 6 +++-- spec/factories.rb | 1 + 4 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20150527004427_add_permalink_to_groups.rb diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 9d510d8f2b..aae2af4b17 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -15,6 +15,7 @@ class EnterpriseGroup < ActiveRecord::Base validates :name, presence: true validates :description, presence: true + validates :permalink, uniqueness: true, presence: true attr_accessible :name, :description, :long_description, :on_front_page, :enterprise_ids attr_accessible :owner_id diff --git a/db/migrate/20150527004427_add_permalink_to_groups.rb b/db/migrate/20150527004427_add_permalink_to_groups.rb new file mode 100644 index 0000000000..ab6886cff4 --- /dev/null +++ b/db/migrate/20150527004427_add_permalink_to_groups.rb @@ -0,0 +1,26 @@ +class AddPermalinkToGroups < ActiveRecord::Migration + def up + add_column :enterprise_groups, :permalink, :string + + EnterpriseGroup.reset_column_information + + EnterpriseGroup.all.each do |group| + counter = 1 + permalink = group.name.parameterize + permalink = "my-group-name" if permalink == "" + while EnterpriseGroup.find_by_permalink(permalink) do + permalink = group.name.parameterize + counter.to_s + counter += 1 + end + + group.update_column :permalink, permalink + end + + change_column :enterprise_groups, :permalink, :string, null: false + add_index :enterprise_groups, :permalink, :unique => true + end + + def down + remove_column :enterprise_groups, :permalink + end +end diff --git a/db/schema.rb b/db/schema.rb index f9f32af7d1..6d9de020f1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20150424151117) do +ActiveRecord::Schema.define(:version => 20150527004427) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -236,10 +236,12 @@ ActiveRecord::Schema.define(:version => 20150424151117) do t.string "linkedin", :default => "", :null => false t.string "twitter", :default => "", :null => false t.integer "owner_id" + t.string "permalink", :null => false end add_index "enterprise_groups", ["address_id"], :name => "index_enterprise_groups_on_address_id" add_index "enterprise_groups", ["owner_id"], :name => "index_enterprise_groups_on_owner_id" + add_index "enterprise_groups", ["permalink"], :name => "index_enterprise_groups_on_permalink", :unique => true create_table "enterprise_groups_enterprises", :id => false, :force => true do |t| t.integer "enterprise_group_id" @@ -619,9 +621,9 @@ ActiveRecord::Schema.define(:version => 20150424151117) 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/factories.rb b/spec/factories.rb index cb4d06b35e..f61172dc2a 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -135,6 +135,7 @@ FactoryGirl.define do factory :enterprise_group, :class => EnterpriseGroup do name 'Enterprise group' + sequence(:permalink) { |n| "group#{n}" } description 'this is a group' on_front_page false address { FactoryGirl.build(:address) } From aef128f2c9282793ad55d7b9fa09a8413d06cdfc Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 27 May 2015 12:20:54 +1000 Subject: [PATCH 39/41] permalink editable --- app/models/enterprise_group.rb | 1 + .../admin/enterprise_groups/_form_primary_details.html.haml | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index aae2af4b17..1524a80b53 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -19,6 +19,7 @@ class EnterpriseGroup < ActiveRecord::Base attr_accessible :name, :description, :long_description, :on_front_page, :enterprise_ids attr_accessible :owner_id + attr_accessible :permalink attr_accessible :logo, :promo_image attr_accessible :address_attributes attr_accessible :email, :website, :facebook, :instagram, :linkedin, :twitter diff --git a/app/views/admin/enterprise_groups/_form_primary_details.html.haml b/app/views/admin/enterprise_groups/_form_primary_details.html.haml index 6d326e33fa..d57333626f 100644 --- a/app/views/admin/enterprise_groups/_form_primary_details.html.haml +++ b/app/views/admin/enterprise_groups/_form_primary_details.html.haml @@ -19,3 +19,8 @@ = f.label :enterprise_ids, 'Enterprises' %br/ = f.collection_select :enterprise_ids, @enterprises, :id, :name, {}, {class: "select2 fullwidth", multiple: true} + + = f.field_container :permalink do + = f.label :permalink, "Permalink (unique, no spaces)" + %br/ + = f.text_field :permalink From e4f93863fd5ce50cb089af4d43a8008a8bb24ca2 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 28 May 2015 10:17:09 +1000 Subject: [PATCH 40/41] Finding unique permalink before validation. --- app/models/enterprise.rb | 2 +- app/models/enterprise_group.rb | 25 +++++++++++++ spec/factories.rb | 1 - spec/models/enterprise_group_spec.rb | 55 ++++++++++++++++++++++++++++ spec/models/enterprise_spec.rb | 5 +++ 5 files changed, 86 insertions(+), 2 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index e921167cf0..067e800e3c 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -299,7 +299,7 @@ class Enterprise < ActiveRecord::Base test_permalink = test_permalink.parameterize test_permalink = "my-enterprise" if test_permalink.blank? existing = Enterprise.select(:permalink).order(:permalink).where("permalink LIKE ?", "#{test_permalink}%").map(&:permalink) - if existing.empty? + unless existing.include?(test_permalink) test_permalink else used_indices = existing.map do |p| diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 1524a80b53..a59b641d7c 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -15,6 +15,8 @@ class EnterpriseGroup < ActiveRecord::Base validates :name, presence: true validates :description, presence: true + + before_validation :sanitize_permalink validates :permalink, uniqueness: true, presence: true attr_accessible :name, :description, :long_description, :on_front_page, :enterprise_ids @@ -73,4 +75,27 @@ class EnterpriseGroup < ActiveRecord::Base address.zipcode.sub!(/^undefined$/, '') end + private + + def self.find_available_value(existing, requested) + return requested unless existing.include?(requested) + used_indices = existing.map do |p| + p.slice!(/^#{requested}/) + p.match(/^\d+$/).to_s.to_i + end + options = (1..used_indices.length + 1).to_a - used_indices + requested + options.first.to_s + end + + def find_available_permalink(requested) + existing = self.class.where(id: !id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink) + self.class.find_available_value(existing, requested) + end + + def sanitize_permalink + if permalink.blank? || permalink_changed? + requested = permalink.presence || permalink_was.presence || name.presence || 'group' + self.permalink = find_available_permalink(requested.parameterize) + end + end end diff --git a/spec/factories.rb b/spec/factories.rb index f61172dc2a..abaecffa47 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -97,7 +97,6 @@ FactoryGirl.define do factory :enterprise, :class => Enterprise do owner { FactoryGirl.create :user } sequence(:name) { |n| "Enterprise #{n}" } - sequence(:permalink) { |n| "enterprise#{n}" } sells 'any' description 'enterprise' long_description '

Hello, world!

This is a paragraph.

' diff --git a/spec/models/enterprise_group_spec.rb b/spec/models/enterprise_group_spec.rb index ceca13b13b..45e7b0692c 100644 --- a/spec/models/enterprise_group_spec.rb +++ b/spec/models/enterprise_group_spec.rb @@ -2,11 +2,32 @@ require 'spec_helper' describe EnterpriseGroup do describe "validations" do + it "pass with name, description and address" do + e = EnterpriseGroup.new + e.name = 'Test Group' + e.description = 'A valid test group.' + e.address = build(:address) + e.should be_valid + end + it "is valid when built from factory" do e = build(:enterprise_group) e.should be_valid end + it "replace empty permalink and pass" do + e = build(:enterprise_group, permalink: '') + e.should be_valid + e.permalink.should == e.name.parameterize + end + + it "restores permalink and pass" do + e = create(:enterprise_group, permalink: 'p') + e.permalink = '' + e.should be_valid + e.permalink.should == 'p' + end + it "requires a name" do e = build(:enterprise_group, name: '') e.should_not be_valid @@ -60,5 +81,39 @@ describe EnterpriseGroup do EnterpriseGroup.managed_by(user).should == [eg1] end + + describe "finding a permalink" do + it "finds available permalink" do + existing = [] + expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink" + end + + it "finds available permalink similar to existing" do + existing = ["permalink1"] + expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink" + end + + it "adds unique number to existing permalinks" do + existing = ["permalink"] + expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink1" + existing = ["permalink", "permalink1"] + expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink2" + end + + it "ignores permalinks with characters after the index value" do + existing = ["permalink", "permalink1", "permalink2xxx"] + expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink2" + end + + it "finds gaps in the indices of existing permalinks" do + existing = ["permalink", "permalink1", "permalink3"] + expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink2" + end + + it "finds available indexed permalink" do + existing = ["permalink", "permalink1"] + expect(EnterpriseGroup.find_available_value(existing, "permalink1")).to eq "permalink11" + end + end end end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index ed4669c0c2..c81c3818df 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -845,6 +845,11 @@ describe Enterprise do expect(Enterprise.find_available_permalink("permalink")).to eq "permalink2" end + it "finds available permalink similar to existing" do + create(:enterprise, permalink: "permalink2xxx") + expect(Enterprise.find_available_permalink("permalink2")).to eq "permalink2" + end + it "finds gaps in the indices of existing permalinks" do create(:enterprise, permalink: "permalink3") expect(Enterprise.find_available_permalink("permalink")).to eq "permalink2" From ff2eed77601d0813605e703ef77f3a224faabc6d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 28 May 2015 11:27:06 +1000 Subject: [PATCH 41/41] Using permalink in URLs pointing to groups --- app/controllers/admin/enterprise_groups_controller.rb | 10 ++++++++-- app/controllers/groups_controller.rb | 2 +- app/models/enterprise_group.rb | 4 ++++ app/views/groups/index.html.haml | 2 +- app/views/json/_groups.rabl | 2 +- app/views/shopping_shared/_groups.html.haml | 2 +- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/enterprise_groups_controller.rb b/app/controllers/admin/enterprise_groups_controller.rb index 3f8888edfb..cb3ac80935 100644 --- a/app/controllers/admin/enterprise_groups_controller.rb +++ b/app/controllers/admin/enterprise_groups_controller.rb @@ -9,7 +9,7 @@ module Admin def move_up EnterpriseGroup.with_isolation_level_serializable do - @enterprise_group = EnterpriseGroup.find params[:enterprise_group_id] + @enterprise_group = EnterpriseGroup.find_by_permalink params[:enterprise_group_id] @enterprise_group.move_higher end redirect_to main_app.admin_enterprise_groups_path @@ -17,7 +17,7 @@ module Admin def move_down EnterpriseGroup.with_isolation_level_serializable do - @enterprise_group = EnterpriseGroup.find params[:enterprise_group_id] + @enterprise_group = EnterpriseGroup.find_by_permalink params[:enterprise_group_id] @enterprise_group.move_lower end redirect_to main_app.admin_enterprise_groups_path @@ -33,6 +33,12 @@ module Admin end alias_method_chain :build_resource, :address + # Overriding method on Spree's resource controller, + # so that resources are found using permalink + def find_resource + EnterpriseGroup.find_by_permalink(params[:id]) + end + private def load_data diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 8653131b5a..fe43f0a0fa 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -7,6 +7,6 @@ class GroupsController < BaseController end def show - @group = EnterpriseGroup.find params[:id] + @group = EnterpriseGroup.find_by_permalink(params[:id]) || EnterpriseGroup.find(params[:id]) end end diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index a59b641d7c..986dc07d22 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -75,6 +75,10 @@ class EnterpriseGroup < ActiveRecord::Base address.zipcode.sub!(/^undefined$/, '') end + def to_param + permalink + end + private def self.find_available_value(existing, requested) diff --git a/app/views/groups/index.html.haml b/app/views/groups/index.html.haml index 901cb17ad9..710039c7fb 100644 --- a/app/views/groups/index.html.haml +++ b/app/views/groups/index.html.haml @@ -20,7 +20,7 @@ .row.pad-top{bindonce: true} .small-12.medium-6.columns .groups-header - %a{"bo-href-i" => "/groups/{{group.id}}"} + %a{"bo-href-i" => "/groups/{{group.permalink}}"} %i.ofn-i_035-groups %span.group-name{"bo-text" => "group.name"} .small-3.medium-2.columns diff --git a/app/views/json/_groups.rabl b/app/views/json/_groups.rabl index a079290e23..bc50586a62 100644 --- a/app/views/json/_groups.rabl +++ b/app/views/json/_groups.rabl @@ -1,5 +1,5 @@ collection @groups -attributes :id, :name, :position, :description, :long_description, :email, :website, :facebook, :instagram, :linkedin, :twitter +attributes :id, :permalink, :name, :position, :description, :long_description, :email, :website, :facebook, :instagram, :linkedin, :twitter child enterprises: :enterprises do attributes :id diff --git a/app/views/shopping_shared/_groups.html.haml b/app/views/shopping_shared/_groups.html.haml index 68c68882fb..d94d61359c 100644 --- a/app/views/shopping_shared/_groups.html.haml +++ b/app/views/shopping_shared/_groups.html.haml @@ -9,5 +9,5 @@ %ul.bullet-list - for group in current_distributor.groups %li - %a{href: main_app.groups_path + "/#/#group#{group.id}"} + %a{href: main_app.groups_path + "/#{group.permalink}"} = group.name