Skip to content

Fix new subscriptions #291

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

Merged
merged 2 commits into from
Jan 11, 2015
Merged
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
8 changes: 6 additions & 2 deletions app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def new
def create
redirect_to teamname_path(slug: @team.slug) if @plan.free?

@account = @team.build_account(params[:account])
@account = @team.build_account(account_params)
@account.admin_id = current_user.id
# TODO: (whatupdave) this doesn't look like it's being used any more. Remove if possible
# @account.trial_end = Date.new(2013, 1, 1).to_time.to_i if session[:discount] == ENV['DISCOUNT_TOKEN']
Expand All @@ -38,7 +38,7 @@ def create
end

def update
if @account.update_attributes(params[:account]) && @account.save_with_payment(@plan)
if @account.update_attributes(account_params) && @account.save_with_payment(@plan)
redirect_to new_team_opportunity_path(@team), notice: "You are subscribed to #{@plan.name}." + plan_capability(@plan, @team)
else
flash[:error] = @account.errors.full_messages.join("\n")
Expand Down Expand Up @@ -107,4 +107,8 @@ def paying_user_context
# Honeybadger.context(user_email: current_user.try(:email)) if current_user
end

def account_params
params.require(:teams_account).permit(:stripe_card_token)
end

end
48 changes: 22 additions & 26 deletions app/models/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
class Plan < ActiveRecord::Base
has_many :subscriptions , class_name: 'Teams::AccountPlan'

before_create :generate_public_id
after_create :register_on_stripe
after_destroy :unregister_from_stripe

before_create :generate_public_id

CURRENCIES = %w(usd)
MONTHLY = 'month'

Expand All @@ -41,6 +40,7 @@ class Plan < ActiveRecord::Base
scope :free, -> { where(amount: 0) }
scope :with_analytics, -> { where(analytics: true) }
scope :without_analytics, -> { where(analytics: false) }

class << self
def enhanced_team_page_analytics
monthly.paid.with_analytics.first
Expand All @@ -59,8 +59,26 @@ def enhanced_team_page_free
end
end


alias_attribute :stripe_plan_id, :public_id
alias_attribute :has_analytics?, :analytics

def price
amount / 100
end

def subscription?
!one_time?
end

def free?
amount.zero?
end

# TODO refactor
# We should avoid nil.
def one_time?
self.interval.nil?
end

#copy to sidekiq worker
def stripe_plan
Expand All @@ -69,7 +87,6 @@ def stripe_plan
nil
end


#sidekiq it
def register_on_stripe
if subscription?
Expand All @@ -95,29 +112,8 @@ def unregister_from_stripe
end
end

def price
amount / 100
end


def subscription?
!one_time?
end

def free?
amount.zero?
end

# TODO refactor
# We should avoid nil.
def one_time?
self.interval.nil?
end

alias_attribute :has_analytics?, :analytics

#TODO CHANGE with default in rails 4
def generate_public_id
self.public_id = SecureRandom.urlsafe_base64(4).downcase
self.public_id ||= SecureRandom.urlsafe_base64(4).downcase
end
end
16 changes: 3 additions & 13 deletions app/models/teams/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,12 @@ class Teams::Account < ActiveRecord::Base
has_many :plans, through: :account_plans
belongs_to :admin, class_name: 'User'

validates :team_id, presence: true, uniqueness: true
validates_presence_of :stripe_card_token
validates_presence_of :stripe_customer_token
validates :team_id, presence: true, uniqueness: true

attr_protected :stripe_customer_token, :admin_id

validate :stripe_customer_token, presence: true
validate :stripe_card_token, presence: true
validate :admin_id, :payer_is_team_admin

def payer_is_team_admin
if admin_id.nil? #or !team.admin?(admin)
errors.add(:admin_id, "must be team admin to create an account")
end
end

def subscribe_to!(plan, force=false)
Copy link
Contributor

Choose a reason for hiding this comment

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

what about
validate :stripe_customer_token, presence: true , if: ->(r){r.state == 'active'}
we need a migration to add add this field :string, default: 'pending'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't that unnecessary? As far as I understand - Teams who haven't paid, don't have Accounts/Plans. We only create Accounts/Plans for teams who are paying customers, so a vanilla presence validation makes sense.

The only way I see a team going to an inactive state, is if they decided to cancel their subscription, in which case they should be moved to the free plan. In that case, they'll still have the old stripe_customer_token, so our validation still holds.

self.plan_ids = [plan.id]
if force || update_on_stripe(plan)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can drop admin_id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a change that should be made in #463. It'll involve modifying the admin method too.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Expand All @@ -46,10 +36,10 @@ def subscribe_to!(plan, force=false)
end

def save_with_payment(plan=nil)
if valid?
if stripe_card_token
create_customer unless plan.try(:one_time?)
subscribe_to!(plan) unless plan.nil?
team.save!
save!
return true
else
return false
Expand Down
10 changes: 7 additions & 3 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ def self.create_network_for(name)
end
end

puts '---- NETWORKS ----'

S.create_network_for('Ruby')
S.create_network_for('JavaScript')

puts '---- PLANS ----'

Plan.find_or_create_by_id(1) do |s|
s.amount = 0
s.interval = 'month'
Expand Down Expand Up @@ -116,19 +120,19 @@ def self.create_network_for(name)
S.create_protip_for(bryce) do |p|
p.title = 'Suspendisse potenti'
p.body = '<p>Suspendisse potenti. Nunc iaculis risus vel &#8216;Orci Ornare&#8217; dignissim sed vitae nulla. Nulla lobortis tempus commodo. Suspendisse <em>potenti</em>. Duis sagittis, est sit amet gravida tristique, purus lectus venenatis urna, id &#8216;molestie&#8217; magna risus ut nunc. Donec tempus tempus tellus, ac <abbr title="Hypertext Markup Language">HTML</abbr> lacinia turpis mattis ac. Fusce ac sodales magna. Fusce ac sodales <abbr title="Cascading Style Sheets">CSS</abbr> magna.</p>'
p.topics = %w{suspendisse potenti}
p.topic_list = %w{suspendisse potenti}
end

S.create_protip_for(bryce) do |p|
p.title = 'Vinyl Blue Bottle four loko wayfarers'
p.body = 'Austin try-hard artisan, bicycle rights salvia squid dreamcatcher hoodie before they sold out Carles scenester ennui. Organic mumblecore Tumblr, gentrify retro 90\'s fanny pack flexitarian raw denim roof party cornhole. Hella direct trade mixtape +1 cliche, slow-carb Neutra craft beer tousled fap DIY.'
p.topics = %w{etsy hipster}
p.topic_list = %w{etsy hipster}
end

S.create_protip_for(lisa) do |p|
p.title = 'Cras molestie risus a enim convallis vitae luctus libero lacinia'
p.body = '<p>Cras molestie risus a enim convallis vitae luctus libero lacinia. Maecenas sit <q cite="http://www.heydonworks.com">amet tellus nec mi gravida posuere</q> non pretium magna. Nulla vel magna sit amet dui <a href="#">lobortis</a> commodo vitae vel nulla. </p>'
p.topics = %w{cras molestie}
p.topic_list = %w{cras molestie}
end

puts '---- TEAMS ----'
Expand Down