From 8fc18bb8e3e0c65a8853da4e9a72283e731f3f80 Mon Sep 17 00:00:00 2001 From: Fatty Date: Fri, 12 Dec 2014 15:30:35 +1000 Subject: [PATCH 1/6] asset files moved to backend and frontend instead of old admin and store paths - installer writes these to the vendor directory --- app/assets/javascripts/admin/spree_subscribe.js | 1 - .../{admin => backend}/intervals_autocomplete.js | 0 app/assets/javascripts/backend/spree_subscribe.js | 1 + .../{store => frontend}/spree_subscribe.js.coffee | 0 .../stylesheets/{admin => backend}/spree_subscribe.css | 0 .../stylesheets/{store => frontend}/spree_subscribe.scss | 0 .../spree_subscribe/install/install_generator.rb | 8 ++++---- 7 files changed, 5 insertions(+), 5 deletions(-) delete mode 100644 app/assets/javascripts/admin/spree_subscribe.js rename app/assets/javascripts/{admin => backend}/intervals_autocomplete.js (100%) create mode 100644 app/assets/javascripts/backend/spree_subscribe.js rename app/assets/javascripts/{store => frontend}/spree_subscribe.js.coffee (100%) rename app/assets/stylesheets/{admin => backend}/spree_subscribe.css (100%) rename app/assets/stylesheets/{store => frontend}/spree_subscribe.scss (100%) diff --git a/app/assets/javascripts/admin/spree_subscribe.js b/app/assets/javascripts/admin/spree_subscribe.js deleted file mode 100644 index 60bd69a..0000000 --- a/app/assets/javascripts/admin/spree_subscribe.js +++ /dev/null @@ -1 +0,0 @@ -//= require admin/intervals_autocomplete \ No newline at end of file diff --git a/app/assets/javascripts/admin/intervals_autocomplete.js b/app/assets/javascripts/backend/intervals_autocomplete.js similarity index 100% rename from app/assets/javascripts/admin/intervals_autocomplete.js rename to app/assets/javascripts/backend/intervals_autocomplete.js diff --git a/app/assets/javascripts/backend/spree_subscribe.js b/app/assets/javascripts/backend/spree_subscribe.js new file mode 100644 index 0000000..b423caf --- /dev/null +++ b/app/assets/javascripts/backend/spree_subscribe.js @@ -0,0 +1 @@ +//= require backend/intervals_autocomplete \ No newline at end of file diff --git a/app/assets/javascripts/store/spree_subscribe.js.coffee b/app/assets/javascripts/frontend/spree_subscribe.js.coffee similarity index 100% rename from app/assets/javascripts/store/spree_subscribe.js.coffee rename to app/assets/javascripts/frontend/spree_subscribe.js.coffee diff --git a/app/assets/stylesheets/admin/spree_subscribe.css b/app/assets/stylesheets/backend/spree_subscribe.css similarity index 100% rename from app/assets/stylesheets/admin/spree_subscribe.css rename to app/assets/stylesheets/backend/spree_subscribe.css diff --git a/app/assets/stylesheets/store/spree_subscribe.scss b/app/assets/stylesheets/frontend/spree_subscribe.scss similarity index 100% rename from app/assets/stylesheets/store/spree_subscribe.scss rename to app/assets/stylesheets/frontend/spree_subscribe.scss diff --git a/lib/generators/spree_subscribe/install/install_generator.rb b/lib/generators/spree_subscribe/install/install_generator.rb index 91eee4d..e612e66 100644 --- a/lib/generators/spree_subscribe/install/install_generator.rb +++ b/lib/generators/spree_subscribe/install/install_generator.rb @@ -3,13 +3,13 @@ module Generators class InstallGenerator < Rails::Generators::Base def add_javascripts - append_file 'app/assets/javascripts/store/all.js', "//= require store/spree_subscribe\n" - append_file 'app/assets/javascripts/admin/all.js', "//= require admin/spree_subscribe\n" + append_file 'vendor/assets/javascripts/spree/frontend/all.js', "//= require frontend/spree_subscribe\n" + append_file 'vendor/assets/javascripts/spree/backend/all.js', "//= require backend/spree_subscribe\n" end def add_stylesheets - inject_into_file 'app/assets/stylesheets/store/all.css', " *= require store/spree_subscribe\n", :before => /\*\//, :verbose => true - inject_into_file 'app/assets/stylesheets/admin/all.css', " *= require admin/spree_subscribe\n", :before => /\*\//, :verbose => true + inject_into_file 'vendor/assets/stylesheets/spree/frontend/all.css', " *= require frontend/spree_subscribe\n", :before => /\*\//, :verbose => true + inject_into_file 'vendor/assets/stylesheets/spree/backend/all.css', " *= require backend/spree_subscribe\n", :before => /\*\//, :verbose => true end def add_migrations From 42c15f0720c121f2bc08532541d2ba86dbd05ad9 Mon Sep 17 00:00:00 2001 From: Fatty Date: Fri, 12 Dec 2014 15:31:00 +1000 Subject: [PATCH 2/6] spree dependency bump --- spree_subscribe.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spree_subscribe.gemspec b/spree_subscribe.gemspec index 7e26429..0bd9354 100644 --- a/spree_subscribe.gemspec +++ b/spree_subscribe.gemspec @@ -16,7 +16,7 @@ Gem::Specification.new do |s| s.require_path = 'lib' s.requirements << 'none' - s.add_dependency 'spree_core', '~> 2.0.3' + s.add_dependency 'spree_core', '~> 2.3.0' s.add_development_dependency 'capybara', '~> 2.1' s.add_development_dependency 'coffee-rails' From 55daa6edb87392b2e09081465a8ee6306449a1ab Mon Sep 17 00:00:00 2001 From: Fatty Date: Fri, 12 Dec 2014 15:31:19 +1000 Subject: [PATCH 3/6] rails 4 --- app/models/concerns/intervalable.rb | 2 -- app/models/spree/order_decorator.rb | 2 -- app/models/spree/product_decorator.rb | 2 -- app/models/spree/subscription.rb | 5 +---- app/models/spree/subscription_interval.rb | 2 -- app/models/spree/user_decorator.rb | 2 +- app/models/spree/variant_decorator.rb | 2 -- 7 files changed, 2 insertions(+), 15 deletions(-) diff --git a/app/models/concerns/intervalable.rb b/app/models/concerns/intervalable.rb index 5df2747..af0848a 100644 --- a/app/models/concerns/intervalable.rb +++ b/app/models/concerns/intervalable.rb @@ -10,8 +10,6 @@ module Intervalable included do - attr_accessible :times, :time_unit - validates :times, :time_unit, :presence => true validates_inclusion_of :time_unit, :in => UNITS.keys diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 8b67c79..4123e88 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -1,6 +1,4 @@ Spree::Order.class_eval do - attr_accessible :subscription_id - belongs_to :subscription, :class_name => "Spree::Subscription" state_machine :initial => :cart do diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 8016bb6..a42b341 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -1,6 +1,4 @@ Spree::Product.class_eval do - attr_accessible :subscribable, :subscription_interval_ids, :subscribed_price - delegate_belongs_to :master, :subscribed_price has_and_belongs_to_many :subscription_intervals, :foreign_key => "product_id", :join_table => 'spree_subscription_intervals_products' diff --git a/app/models/spree/subscription.rb b/app/models/spree/subscription.rb index e3af894..7d6df9f 100644 --- a/app/models/spree/subscription.rb +++ b/app/models/spree/subscription.rb @@ -1,9 +1,6 @@ require 'concerns/intervalable' class Spree::Subscription < ActiveRecord::Base - attr_accessible :reorder_on, :user_id, :times, :time_unit, :line_item_id, :billing_address_id, :state, - :shipping_address_id, :shipping_method_id, :payment_method_id, :source_id, :source_type - include Intervalable attr_accessor :new_order @@ -19,7 +16,7 @@ class Spree::Subscription < ActiveRecord::Base has_many :reorders, :class_name => "Spree::Order" - scope :active, where(:state => 'active') + scope :active, -> { where(state: 'active') } state_machine :state, :initial => 'cart' do event :suspend do diff --git a/app/models/spree/subscription_interval.rb b/app/models/spree/subscription_interval.rb index 074deb9..68a811c 100644 --- a/app/models/spree/subscription_interval.rb +++ b/app/models/spree/subscription_interval.rb @@ -1,8 +1,6 @@ require 'concerns/intervalable' class Spree::SubscriptionInterval < ActiveRecord::Base - attr_accessible :name - include Intervalable has_and_belongs_to_many :products, :class_name => "Spree::Product", :join_table => 'spree_subscription_intervals_products' diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 48da242..d055d11 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -1,3 +1,3 @@ Spree.user_class.class_eval do - has_many :subscriptions, :order => "updated_at DESC", :class_name => 'Spree::Subscription' + has_many :subscriptions, -> { order(updated_at: :desc) }, :class_name => 'Spree::Subscription' end diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index a6cdb48..bb0ee21 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -1,5 +1,3 @@ Spree::Variant.class_eval do delegate :subscribable?, :to => :product - - attr_accessible :subscribed_price end \ No newline at end of file From 342b0f5337dab5344a083eeaeb1868e73c784678 Mon Sep 17 00:00:00 2001 From: Fatty Date: Fri, 12 Dec 2014 17:43:45 +1000 Subject: [PATCH 4/6] added ability to add single variants --- .../spree/orders_controller_decorator.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 814d62f..1e7b822 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -8,16 +8,13 @@ def check_subscriptions return unless params[:subscriptions] && params[:subscriptions][:active].to_s == "1" - params[:products].each do |product_id,variant_id| - add_subscription variant_id, params[:subscriptions][:interval_id] - end if params[:products] + interval_id = params[:subscriptions][:interval_id] - params[:variants].each do |variant_id, quantity| - add_subscription variant_id, params[:subscriptions][:interval_id] - end if params[:variants] - end + params[:products].each { |product_id, variant_id| add_subscription(variant_id, interval_id) } if params[:products] + params[:variants].each { |variant_id, quantity| add_subscription(variant_id, interval_id) } if params[:variants] - protected + add_subscription(params[:variant_id], params[:subscriptions][:interval_id]) if params[:variant_id] + end # DD: TODO write test for this method def add_subscription(variant_id, interval_id) From 7b64761d35d1dab5488d84db8d90e7f8d9fe8260 Mon Sep 17 00:00:00 2001 From: Fatty Date: Fri, 12 Dec 2014 17:44:28 +1000 Subject: [PATCH 5/6] added status scopes to filter results in index page - this was causing a NullRefernce exception when viewing subscriptions in 'cart' status --- app/models/spree/subscription.rb | 5 +++++ app/views/spree/admin/subscriptions/index.html.erb | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/spree/subscription.rb b/app/models/spree/subscription.rb index 7d6df9f..c49e83d 100644 --- a/app/models/spree/subscription.rb +++ b/app/models/spree/subscription.rb @@ -16,7 +16,12 @@ class Spree::Subscription < ActiveRecord::Base has_many :reorders, :class_name => "Spree::Order" + scope :cart, -> { where(state: 'cart') } scope :active, -> { where(state: 'active') } + scope :inactive, -> { where(state: 'inactive') } + scope :cancelled, -> { where(state: 'cancelled') } + + scope :current, -> { where(state: ['active', 'inactive']) } state_machine :state, :initial => 'cart' do event :suspend do diff --git a/app/views/spree/admin/subscriptions/index.html.erb b/app/views/spree/admin/subscriptions/index.html.erb index 088f6f6..0de7bb0 100644 --- a/app/views/spree/admin/subscriptions/index.html.erb +++ b/app/views/spree/admin/subscriptions/index.html.erb @@ -35,14 +35,14 @@ - <% @subscriptions.each do |sub| %> + <% @subscriptions.current.each do |sub| %> <%= sub.line_item.variant.name %>
<%= sub.line_item.variant.options_text %> <%= sub.user.id %> - <%= money sub.line_item.price %> + <%= Spree::Money.new(sub.line_item.price).to_html %> <%= sub.time_title %> <% if sub.active? %> From e4d2ab9be663f5e075e323057dcad90068a7d9fd Mon Sep 17 00:00:00 2001 From: Fatty Date: Fri, 12 Dec 2014 22:24:39 +1000 Subject: [PATCH 6/6] added due scope to subscription so that we have an easy way of finding which subscriptions are ready to be reordered. moved the processing inside of the model instead of in the rake task so that the two dont need to share logic. changed the order of some of the steps to work with spree-2-3. added a simple progress sanity checker - this should ideally use the state machine helper methods --- app/models/spree/subscription.rb | 82 ++++++++++++++++++++++---------- lib/tasks/spree_subscribe.rake | 4 +- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/app/models/spree/subscription.rb b/app/models/spree/subscription.rb index c49e83d..a296989 100644 --- a/app/models/spree/subscription.rb +++ b/app/models/spree/subscription.rb @@ -22,6 +22,8 @@ class Spree::Subscription < ActiveRecord::Base scope :cancelled, -> { where(state: 'cancelled') } scope :current, -> { where(state: ['active', 'inactive']) } + + scope :due, -> { active.where("reorder_on <= ?", Date.today) } state_machine :state, :initial => 'cart' do event :suspend do @@ -37,35 +39,43 @@ class Spree::Subscription < ActiveRecord::Base after_transition :on => :start, :do => :set_checkout_requirements after_transition :on => :resume, :do => :check_reorder_date end + + def self.reorder_due! + due.each(&:reorder) + end # DD: TODO pull out into a ReorderBuilding someday def reorder - raise false unless self.state == 'active' - - create_reorder && - add_subscribed_line_item && - select_shipping && - add_payment && - confirm_reorder && - complete_reorder && - calculate_reorder_date! + raise false unless active? + + result = create_reorder && + select_shipping && + add_payment && + confirm_reorder && + complete_reorder && + calculate_reorder_date! + + puts result ? " -> Next reorder date: #{self.reorder_on}" : " -> FAILED" + + result end def create_reorder + puts "[SPREE::SUBSCRIPTION] Reordering subscription: #{id}" + puts " -> creating order..." + self.new_order = Spree::Order.create( - bill_address: self.billing_address.clone, - ship_address: self.shipping_address.clone, - subscription_id: self.id, - email: self.user.email - ) - self.new_order.user_id = self.user_id + bill_address: self.billing_address.clone, + ship_address: self.shipping_address.clone, + subscription_id: self.id, + email: self.user.email, + user_id: self.user_id + ) # DD: make it work with spree_multi_domain - if self.new_order.respond_to?(:store_id) - self.new_order.store_id = self.line_item.order.store_id - end - - self.new_order.next # -> address + self.new_order.store_id = self.line_item.order.store_id if self.new_order.respond_to?(:store_id) + + add_subscribed_line_item && progress # -> delivery end def add_subscribed_line_item @@ -75,39 +85,45 @@ def add_subscribed_line_item line_item.price = self.line_item.price line_item.save! - self.new_order.next # -> delivery + result = progress # -> delivery end def select_shipping + puts " -> selecting shipping rate..." + # DD: shipments are created when order state goes to "delivery" shipment = self.new_order.shipments.first # DD: there should be only one shipment rate = shipment.shipping_rates.first{|r| r.shipping_method.id == self.shipping_method.id } raise "No rate was found. TODO: Implement logic to select the cheapest rate." unless rate + shipment.selected_shipping_rate_id = rate.id shipment.save end def add_payment - payment = self.new_order.payments.build( :amount => self.new_order.item_total ) + puts " -> adding payment..." + + payment = self.new_order.payments.build(amount: self.new_order.outstanding_balance) payment.source = self.source payment.payment_method = self.payment_method payment.save! - self.new_order.next # -> payment + progress # -> payment end def confirm_reorder - self.new_order.next # -> confirm + progress # -> confirm end def complete_reorder self.new_order.update! - self.new_order.next && self.new_order.save # -> complete + progress && self.new_order.save # -> complete end def calculate_reorder_date! self.reorder_on ||= Date.today self.reorder_on += self.time + save end @@ -136,9 +152,23 @@ def set_checkout_requirements :user_id => order.user_id ) end + + def new_order_state + self.new_order.state + end + def progress + current_state = new_order_state + result = self.new_order.next + + success = !!result && current_state != new_order_state + + puts " !! Order progression failed. Status still '#{new_order_state}'" unless success + + success + end def self.reorder_states @reorder_states ||= state_machine.states.map(&:name) - ["cart"] end - + end diff --git a/lib/tasks/spree_subscribe.rake b/lib/tasks/spree_subscribe.rake index 24c5b0a..882d255 100644 --- a/lib/tasks/spree_subscribe.rake +++ b/lib/tasks/spree_subscribe.rake @@ -1,9 +1,9 @@ namespace :spree_subscribe do namespace :reorders do - desc "Find all subscriptions that are due today and reorder their products" + desc "Find all subscriptions that are due on or before today and reorder their products" task :create => :environment do - Spree::Subscription.where(:reorder_on => Date.today).each{|s| s.reorder } + Spree::Subscription.reorder_due! end end