Merge branch 'master' into laura_and_will

Gemfile

@@ -40,6 +40,8 @@ gem 'custom_error_message', :github => 'jeremydurham/custom-err-msg'
 gem 'foreigner'
 gem 'immigrant'
 
+gem 'whenever', require: false
+
 # Gems used only for assets and not required
 # in production environments by default.
 group :assets do

Gemfile.lock

@@ -175,6 +175,7 @@ GEM
       xpath (~> 2.0)
     celluloid (0.15.2)
       timers (~> 1.1.0)
+    chronic (0.10.2)
     chunky_png (1.3.0)
     climate_control (0.0.3)
       activesupport (>= 3.0)
@@ -479,6 +480,9 @@ GEM
       addressable (>= 2.2.7)
       crack (>= 0.3.2)
     websocket-driver (0.3.2)
+    whenever (0.9.2)
+      activesupport (>= 2.3.4)
+      chronic (>= 0.6.3)
     xpath (2.0.0)
       nokogiri (~> 1.3)
     zeus (0.13.3)
@@ -552,3 +556,4 @@ DEPENDENCIES
   unicorn
   unicorn-rails
   webmock
+  whenever

app/assets/javascripts/admin/bulk_product_update.js.coffee

@@ -213,7 +213,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [
       if confirm("Are you sure?")
         $http(
           method: "DELETE"
-          url: "/api/products/" + product.id
+          url: "/api/products/" + product.id + "/soft_delete"
         ).success (data) ->
           $scope.products.splice $scope.products.indexOf(product), 1
           DirtyProducts.deleteProduct product.id

app/assets/javascripts/darkswarm/controllers/checkout/payment_controller.js.coffee

@@ -2,7 +2,21 @@ Darkswarm.controller "PaymentCtrl", ($scope, $timeout) ->
   angular.extend(this, new FieldsetMixin($scope))
   $scope.name = "payment"
 
-  $scope.months = {1: "January", 2: "February", 3: "March", 4: "April", 5: "May", 6: "June", 7: "July", 8: "August", 9: "September", 10: "October", 11: "November", 12: "December"}
+  $scope.months = [
+    {key: "January", value: "1"},
+    {key: "February", value: "2"},
+    {key: "March", value: "3"},
+    {key: "April", value: "4"}, 
+    {key: "May", value: "5"},
+    {key: "June", value: "6"},
+    {key: "July", value: "7"},
+    {key: "August", value: "8"},
+    {key: "September", value: "9"},
+    {key: "October", value: "10"},
+    {key: "November", value: "11"},
+    {key: "December", value: "12"},
+  ]
+
   $scope.years = [moment().year()..(moment().year()+15)]
   $scope.secrets.card_month = "1"
   $scope.secrets.card_year = moment().year()

app/controllers/application_controller.rb

@@ -1,6 +1,5 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
-  before_filter :require_certified_hostname
 
   include EnterprisesHelper
 
@@ -17,6 +16,7 @@ class ApplicationController < ActionController::Base
     end
   end
 
+
   private
 
   def require_distributor_chosen
@@ -42,17 +42,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  # There are several domains that point to the production server, but only one
-  # (vic.openfoodnetwork.org) that has the SSL certificate. Redirect all requests to this
-  # domain to avoid showing customers a scary invalid certificate error.
-  def require_certified_hostname
-    certified_host = "openfoodnetwork.org.au"
-
-    if Rails.env.production? && request.host != certified_host
-      redirect_to "http://#{certified_host}#{request.fullpath}"
-    end
-  end
-
 
   # All render calls within the block will be performed with the specified format
   # Useful for rendering html within a JSON response, particularly if the specified

app/controllers/spree/admin/variants_controller_decorator.rb

@@ -1,6 +1,17 @@
 Spree::Admin::VariantsController.class_eval do
   helper 'spree/products'
 
+  def destroy
+    @variant = Spree::Variant.find(params[:id])
+    @variant.delete # This line changed, as well as removal of following conditional
+    flash[:success] = I18n.t('notice_messages.variant_deleted')
+
+    respond_with(@variant) do |format|
+      format.html { redirect_to admin_product_variants_url(params[:product_id]) }
+      format.js   { render_js_for_destroy }
+    end
+  end
+
 
   protected
 

app/controllers/spree/api/products_controller_decorator.rb

@@ -8,6 +8,15 @@ Spree::Api::ProductsController.class_eval do
   end
 
 
+  def soft_delete
+    authorize! :delete, Spree::Product
+    @product = find_product(params[:product_id])
+    authorize! :delete, @product
+    @product.delete
+    respond_with(@product, :status => 204)
+  end
+
+
   private
 
   # Copied and modified from Spree::Api::BaseController to allow

app/controllers/spree/api/variants_controller_decorator.rb

@@ -3,11 +3,7 @@ Spree::Api::VariantsController.class_eval do
     @variant = scope.find(params[:variant_id])
     authorize! :delete, @variant
 
-    @variant.deleted_at = Time.now()
-    if @variant.save
-      respond_with(@variant, :status => 204)
-    else
-      invalid_resource!(@variant)
-    end
+    @variant.delete
+    respond_with @variant, status: 204
   end
 end

app/models/spree/product_decorator.rb

@@ -147,6 +147,15 @@ Spree::Product.class_eval do
     end
   end
 
+  def delete_with_delete_from_order_cycles
+    transaction do
+      delete_without_delete_from_order_cycles
+
+      ExchangeVariant.where('exchange_variants.variant_id IN (?)', self.variants_including_master_and_deleted).destroy_all
+    end
+  end
+  alias_method_chain :delete, :delete_from_order_cycles
+
 
   private
 
@@ -158,7 +167,7 @@ Spree::Product.class_eval do
     if variant_unit_changed?
       option_types.delete self.class.all_variant_unit_option_types
       option_types << variant_unit_option_type if variant_unit.present?
-      variants_including_master.each { |v| v.delete_unit_option_values }
+      variants_including_master.each { |v| v.update_units }
     end
   end
 

app/models/spree/variant_decorator.rb

@@ -54,12 +54,6 @@ Spree::Variant.class_eval do
   end
 
 
-  private
-
-  def update_weight_from_unit_value
-    self.weight = unit_value / 1000 if self.product.variant_unit == 'weight' && unit_value.present?
-  end
-
   def update_units
     delete_unit_option_values
 
@@ -71,6 +65,20 @@ Spree::Variant.class_eval do
     end
   end
 
+  def delete
+    transaction do
+      self.update_column(:deleted_at, Time.now)
+      ExchangeVariant.where(variant_id: self).destroy_all
+    end
+  end
+
+
+  private
+
+  def update_weight_from_unit_value
+    self.weight = unit_value / 1000 if self.product.variant_unit == 'weight' && unit_value.present?
+  end
+
   def option_value_name
     if display_as.present?
       display_as

app/views/admin/enterprises/_supplied_product.rabl

@@ -0,0 +1,10 @@
+object @product
+
+attributes :name
+node(:supplier_name) { |p| p.supplier.andand.name }
+node(:image_url) { |p| p.images.present? ? p.images.first.attachment.url(:mini) : nil }
+node(:master_id) { |p| p.master.id }
+child variants: :variants do |variant|
+  attributes :id
+  node(:label) { |v| v.options_text }
+end

app/views/admin/enterprises/index.rabl

@@ -2,13 +2,8 @@ collection @collection
 
 attributes :id, :name
 
-child supplied_products: :supplied_products do |product|
-  attributes :name
-  node(:supplier_name) { |p| p.supplier.andand.name }
-  node(:image_url) { |p| p.images.present? ? p.images.first.attachment.url(:mini) : nil }
-  node(:master_id) { |p| p.master.id }
-  child variants: :variants do |variant|
-    attributes :id
-    node(:label) { |v| v.options_text }
+node(:supplied_products) do |enterprise|
+  enterprise.supplied_products.not_deleted.map do |product|
+    partial 'admin/enterprises/supplied_product', object: product
   end
 end

app/views/json/_producer.rabl

@@ -8,7 +8,7 @@ node :logo do |producer|
 end
 
 node :path do |producer|
-  producer_path(producer) 
+  main_app.producer_path(producer) 
 end
 
 node :hash do |producer|

app/views/spree/checkout/payment/_gateway.html.haml

@@ -20,6 +20,6 @@
 
 .row
   .small-6.columns
-    %select{"ng-model" => "secrets.card_month", "ng-options" => "number as name for (number, name) in months", name: "secrets.card_month", required: true}
+    %select{"ng-model" => "secrets.card_month", "ng-options" => "currMonth.value as currMonth.key for currMonth in months", name: "secrets.card_month", required: true}
   .small-6.columns
     %select{"ng-model" => "secrets.card_year", "ng-options" => "year for year in years", name: "secrets.card_year", required: true}

config/routes.rb

@@ -119,6 +119,7 @@ Spree::Core::Engine.routes.prepend do
 
     resources :products do
       get :managed, on: :collection
+      delete :soft_delete
 
       resources :variants do
         delete :soft_delete

config/schedule.rb

@@ -0,0 +1,12 @@
+require 'whenever'
+
+# Learn more: http://github.com/javan/whenever
+
+env "MAILTO", "rohan@rohanmitchell.com"
+
+# If we use -e with a file containing specs, rspec interprets it and filters out our examples
+job_type :run_file, "cd :path; :environment_variable=:environment bundle exec script/rails runner :task :output"
+
+every 1.day, at: '12:05am' do
+  run_file "lib/open_food_network/integrity_checker.rb"
+end

db/migrate/20140612020206_remove_deleted_variants_from_order_cycles.rb

@@ -0,0 +1,10 @@
+class RemoveDeletedVariantsFromOrderCycles < ActiveRecord::Migration
+  def up
+    evs = ExchangeVariant.joins(:variant).where('spree_variants.deleted_at IS NOT NULL')
+    say "Removing #{evs.count} deleted variants from order cycles..."
+    evs.destroy_all
+  end
+
+  def down
+  end
+end

lib/open_food_network/integrity_checker.rb

@@ -0,0 +1,23 @@
+require 'rspec/rails'
+require 'rspec/autorun'
+
+# This spec file is one part of a two-part strategy to maintain data integrity. The first part
+# is to proactively protect data integrity using database constraints (not null, foreign keys,
+# etc) and ActiveRecord validations. As a backup to those two techniques, and particularly in
+# the cases where it's not possible to model an integrity concern with database constraints,
+# we can add a reactive integrity test here.
+
+# These tests are run nightly and the results are emailed to the MAILTO address in
+# config/schedule.rb if any failures occur.
+
+# Ref: http://pluralsight.com/training/Courses/TableOfContents/database-your-friend
+
+
+describe "data integrity" do
+  it "has no deleted variants in order cycles" do
+    # When a variant is soft deleted, it should be removed from all order cycles
+    # via Spree::Product#delete or Spree::Variant#delete.
+    evs = ExchangeVariant.joins(:variant).where('spree_variants.deleted_at IS NOT NULL')
+    evs.count.should == 0
+  end
+end

spec/controllers/spree/api/products_controller_spec.rb

@@ -7,9 +7,11 @@ module Spree
     render_views
 
     let(:supplier) { FactoryGirl.create(:supplier_enterprise) }
+    let(:supplier2) { FactoryGirl.create(:supplier_enterprise) }
     let!(:product1) { FactoryGirl.create(:product, supplier: supplier) }
     let!(:product2) { FactoryGirl.create(:product, supplier: supplier) }
     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] }
 
@@ -39,6 +41,20 @@ module Spree
         keys = json_response.first.keys.map{ |key| key.to_sym }
         attributes.all?{ |attr| keys.include? attr }.should == true
       end
+
+      it "soft deletes my products" do
+        spree_delete :soft_delete, {product_id: product1.to_param, format: :json}
+        response.status.should == 204
+        lambda { product1.reload }.should_not raise_error
+        product1.deleted_at.should_not be_nil
+      end
+
+      it "is denied access to soft deleting another enterprises' product" do
+        spree_delete :soft_delete, {product_id: product_other_supplier.to_param, format: :json}
+        assert_unauthorized!
+        lambda { product_other_supplier.reload }.should_not raise_error
+        product_other_supplier.deleted_at.should be_nil
+      end
     end
 
     context "as an administrator" do
@@ -80,17 +96,23 @@ module Spree
       end
 
       it "should allow available_on to be nil" do
-
         spree_get :index, { :template => 'bulk_index', :format => :json }
         json_response.size.should == 3
 
-        product4 = FactoryGirl.create(:product)
-        product4.available_on = nil
-        product4.save!
+        product5 = FactoryGirl.create(:product)
+        product5.available_on = nil
+        product5.save!
 
         spree_get :index, { :template => 'bulk_index', :format => :json }
         json_response.size.should == 4
       end
+
+      it "soft deletes a product" do
+        spree_delete :soft_delete, {product_id: product1.to_param, format: :json}
+        response.status.should == 204
+        lambda { product1.reload }.should_not raise_error
+        product1.deleted_at.should_not be_nil
+      end
     end
   end
 end

spec/features/admin/products_spec.rb

@@ -45,6 +45,8 @@ feature %q{
       product.on_hand.should == 5
       product.description.should == "A description..."
       product.group_buy.should be_false
+      product.master.option_values.map(&:name).should == ['5kg']
+      product.master.options_text.should == "5kg"
 
       # Distributors
       visit spree.product_distributions_admin_product_path(product)

spec/features/admin/variants_spec.rb

@@ -83,4 +83,18 @@ feature %q{
     page.should_not have_field "variant_unit_value"
     page.should_not have_field "variant_unit_description"
   end
+
+  it "soft-deletes variants", js: true do
+    p = create(:simple_product)
+    v = create(:variant, product: p)
+
+    login_to_admin_section
+    visit spree.admin_product_variants_path p
+
+    page.find('a.delete-resource').click
+    page.should_not have_content v.options_text
+
+    v.reload
+    v.deleted_at.should_not be_nil
+  end
 end

spec/javascripts/unit/bulk_product_update_spec.js.coffee

@@ -973,7 +973,7 @@ describe "AdminProductEditCtrl", ->
 
 
   describe "deleting products", ->
-    it "deletes products with a http delete request to /api/products/id", ->
+    it "deletes products with a http delete request to /api/products/id/soft_delete", ->
       spyOn(window, "confirm").andReturn true
       $scope.products = [
         {
@@ -986,7 +986,7 @@ describe "AdminProductEditCtrl", ->
         }
       ]
       $scope.dirtyProducts = {}
-      $httpBackend.expectDELETE("/api/products/13").respond 200, "data"
+      $httpBackend.expectDELETE("/api/products/13/soft_delete").respond 200, "data"
       $scope.deleteProduct $scope.products[1]
       $httpBackend.flush()
 
@@ -1005,7 +1005,7 @@ describe "AdminProductEditCtrl", ->
       DirtyProducts.addProductProperty 9, "someProperty", "something"
       DirtyProducts.addProductProperty 13, "name", "P1"
 
-      $httpBackend.expectDELETE("/api/products/13").respond 200, "data"
+      $httpBackend.expectDELETE("/api/products/13/soft_delete").respond 200, "data"
       $scope.deleteProduct $scope.products[1]
       $httpBackend.flush()
       expect($scope.products).toEqual [
@@ -1036,7 +1036,7 @@ describe "AdminProductEditCtrl", ->
 
   
     describe "when the variant has been saved", ->
-      it "deletes variants with a http delete request to /api/products/product_permalink/variants/(variant_id)", ->
+      it "deletes variants with a http delete request to /api/products/product_permalink/variants/(variant_id)/soft_delete", ->
         spyOn(window, "confirm").andReturn true
         $scope.products = [
           {

spec/models/spree/product_spec.rb

@@ -442,24 +442,34 @@ module Spree
           p.update_attributes!(name: 'foo')
         end
 
-        it "removes the related option values from all its variants" do
+        it "removes the related option values from all its variants and replaces them" do
           ot = Spree::OptionType.find_by_name 'unit_weight'
-          v = create(:variant, product: p)
+          v = create(:variant, unit_value: 1, product: p)
           p.reload
 
-          expect {
+          v.option_values.map(&:name).include?("1L").should == false
+          v.option_values.map(&:name).include?("1g").should == true
+                    expect {
             p.update_attributes!(variant_unit: 'volume', variant_unit_scale: 0.001)
-          }.to change(v.option_values(true), :count).by(-1)
+          }.to change(p.master.option_values(true), :count).by(0)
+          v.reload
+          v.option_values.map(&:name).include?("1L").should == true
+          v.option_values.map(&:name).include?("1g").should == false
         end
 
-        it "removes the related option values from its master variant" do
+        it "removes the related option values from its master variant and replaces them" do
           ot = Spree::OptionType.find_by_name 'unit_weight'
           p.master.update_attributes!(unit_value: 1)
           p.reload
 
-          expect {
+          p.master.option_values.map(&:name).include?("1L").should == false
+          p.master.option_values.map(&:name).include?("1g").should == true
+                    expect {
             p.update_attributes!(variant_unit: 'volume', variant_unit_scale: 0.001)
-          }.to change(p.master.option_values(true), :count).by(-1)
+          }.to change(p.master.option_values(true), :count).by(0)
+          p.reload
+          p.master.option_values.map(&:name).include?("1L").should == true
+          p.master.option_values.map(&:name).include?("1g").should == false
         end
       end
 
@@ -570,7 +580,7 @@ module Spree
       end
     end
 
-    describe "Taxons" do
+    describe "taxons" do
       let(:taxon1) { create(:taxon) }
       let(:taxon2) { create(:taxon) }
       let(:product) { create(:simple_product) }
@@ -580,5 +590,24 @@ module Spree
       end
     end
 
+    describe "deletion" do
+      let(:p)  { create(:simple_product) }
+      let(:v)  { create(:variant, product: p) }
+      let(:oc) { create(:simple_order_cycle) }
+      let(:s)  { create(:supplier_enterprise) }
+      let(:e)  { create(:exchange, order_cycle: oc, incoming: true, sender: s, receiver: oc.coordinator) }
+
+      it "removes the master variant from all order cycles" do
+        e.variants << p.master
+        p.delete
+        e.variants(true).should be_empty
+      end
+
+      it "removes all other variants from order cycles" do
+        e.variants << v
+        p.delete
+        e.variants(true).should be_empty
+      end
+    end
   end
 end

spec/views/admin/enterprises/index.rabl_spec.rb

@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+describe "admin/enterprises/index.rabl" do
+  let(:enterprise)       { create(:distributor_enterprise) }
+  let!(:product)         { create(:simple_product, supplier: enterprise) }
+  let!(:deleted_product) { create(:simple_product, supplier: enterprise, deleted_at: 1.day.ago) }
+  let(:render)           { Rabl.render([enterprise], 'admin/enterprises/index', view_path: 'app/views', scope: RablHelper::FakeContext.instance) }
+
+  describe "supplied products" do
+    it "does not render deleted products" do
+      render.should have_json_size(1).at_path '0/supplied_products'
+      render.should be_json_eql(product.master.id).at_path '0/supplied_products/0/master_id'
+    end
+  end
+end