Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 4 spree 2 3 #12

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/assets/javascripts/admin/spree_subscribe.js

This file was deleted.

1 change: 1 addition & 0 deletions app/assets/javascripts/backend/spree_subscribe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
//= require backend/intervals_autocomplete
13 changes: 5 additions & 8 deletions app/controllers/spree/orders_controller_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused block argument - product_id. If it's necessary, use _ or _product_id as an argument name to indicate that it won't be used.
Line is too long. [118/80]

params[:variants].each { |variant_id, quantity| add_subscription(variant_id, interval_id) } if params[:variants]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused block argument - quantity. If it's necessary, use _ or _quantity as an argument name to indicate that it won't be used.
Line is too long. [116/80]


protected
add_subscription(params[:variant_id], params[:subscriptions][:interval_id]) if params[:variant_id]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [102/80]

end

# DD: TODO write test for this method
def add_subscription(variant_id, interval_id)
Expand Down
2 changes: 0 additions & 2 deletions app/models/concerns/intervalable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 0 additions & 2 deletions app/models/spree/order_decorator.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 0 additions & 2 deletions app/models/spree/product_decorator.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
92 changes: 62 additions & 30 deletions app/models/spree/subscription.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -19,7 +16,14 @@ class Spree::Subscription < ActiveRecord::Base

has_many :reorders, :class_name => "Spree::Order"

scope :active, where(:state => 'active')
scope :cart, -> { where(state: 'cart') }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

scope :active, -> { where(state: 'active') }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

scope :inactive, -> { where(state: 'inactive') }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silly robot - prefer single-quotes unless I want to use inline variables "#{foo}"

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
Expand All @@ -35,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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.


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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

ship_address: self.shipping_address.clone,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

subscription_id: self.id,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

email: self.user.email,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

user_id: self.user_id
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

)

# 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [100/80]
Redundant self detected.


add_subscribed_line_item && progress # -> delivery
end

def add_subscribed_line_item
Expand All @@ -73,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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useless assignment to variable - result.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [87/80]
Redundant self detected.

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

end

def calculate_reorder_date!
self.reorder_on ||= Date.today
self.reorder_on += self.time

save
end

Expand Down Expand Up @@ -134,9 +152,23 @@ def set_checkout_requirements
:user_id => order.user_id
)
end

def new_order_state
self.new_order.state
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your face is a redundant self

end
def progress
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use empty lines between defs.

current_state = new_order_state
result = self.new_order.next
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.


success = !!result && current_state != new_order_state

puts " !! Order progression failed. Status still '#{new_order_state}'" unless success
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [89/80]


success
end

def self.reorder_states
@reorder_states ||= state_machine.states.map(&:name) - ["cart"]
end

end
2 changes: 0 additions & 2 deletions app/models/spree/subscription_interval.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
2 changes: 1 addition & 1 deletion app/models/spree/user_decorator.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 0 additions & 2 deletions app/models/spree/variant_decorator.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
Spree::Variant.class_eval do
delegate :subscribable?, :to => :product

attr_accessible :subscribed_price
end
4 changes: 2 additions & 2 deletions app/views/spree/admin/subscriptions/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@
</tr>
</thead>
<tbody>
<% @subscriptions.each do |sub| %>
<% @subscriptions.current.each do |sub| %>
<tr data-hook="admin_subscriptions_index_rows" class="state-<%= sub.state.downcase %> <%= cycle('odd', 'even') %>">
<td class="subscription-product">
<span class="product-name"><%= sub.line_item.variant.name %></span><br/>
<%= sub.line_item.variant.options_text %>
</td>
<td class="subscription-user"><%= sub.user.id %></td>
<td class="subscription-price"><%= money sub.line_item.price %></td>
<td class="subscription-price"><%= Spree::Money.new(sub.line_item.price).to_html %></td>
<td class="subscription-interval"><%= sub.time_title %></td>
<td class="subscription-reorder-date">
<% if sub.active? %>
Expand Down
8 changes: 4 additions & 4 deletions lib/generators/spree_subscribe/install/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/tasks/spree_subscribe.rake
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion spree_subscribe.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down