Benjamin Roth
@apneadiving
ruby & javascript freelancer
We all faced or still face issues with our code.
Lets see some I came across.
def paypal_event
uri = URI.parse(ENV['PAYPAL_IPN_URL'])
http = Net::HTTP.new(uri.host, uri.port)
http.open_timeout = 60
http.read_timeout = 60
http.verify_mode = OpenSSL::SSL::VERIFY_NONE
http.use_ssl = true
response = http.post(uri.request_uri, request.raw_post,
'Content-Length' => "#{request.raw_post.size}",
'User-Agent' => "My custom user agent"
).body
case response
when "VERIFIED"
# The receiver email has to equal the PayPal receiving email set as environment variable
# The period type, the payment cycle and the recurring payment are set
if params[:receiver_email] === ENV['PAYPAL_EMAIL'] && params[:period_type] && params[:payment_cycle] && params[:recurring_payment_id]
# Find the user subscription
user_subscription = UserSubscription.find_by(pay_id: params[:recurring_payment_id])
render nothing: true and return unless user_subscription.present?
# Find the user and the subscription
user = User.find(user_subscription.user_id)
# If the transaction type is set
if params[:txn_type].present? && params[:txn_type] === 'recurring_payment_suspended_due_to_max_failed_payment'
# Setup PayPal request
request = Paypal::Express::Request.new(
username: ENV['PAYPAL_USER'],
password: ENV['PAYPAL_PWD'],
signature: ENV['PAYPAL_SIGNATURE']
)
# Cancel the PayPal subscription
request.renew!(user_subscription.pay_id, :Cancel)
if user_subscription.active?
# Cancel the subscription
user_subscription.deactivate
user_subscription.save
end
end
# If payment status and transaction id are set and the transaction id has not been previously processed
if !params[:payment_status].nil? && !params[:txn_id].nil? && (params[:txn_id] != session[:txn_id])
# Set the current transaction id
session[:txn_id] = params[:txn_id]
# If the payment status is completed
if params[:payment_status] === 'Completed'
if user.present? && user_subscription.present?
# Create a new receipt
receipt = Receipt.create(
user_id: user.id,
charge_id: user_subscription.pay_id,
data: params,
totalUSD: params[:mc_gross],
status: 'paid',
paid_at: Time.now
)
user_subscription.receipts << receipt
end
# Set the duration depending on the payment cycle
subscription_duration = 1.month
subscription_duration = 1.year if params[:payment_cycle] === 'Yearly'
# If the subscription was cancelled (because a payment failed in the past), make sure to reactivate it.
user_subscription.reactivate if(user_subscription.cancelled? || user_subscription.set_for_cancellation?)
# Set the expiration of the subscription further
user_subscription.expiration = Time.now + subscription_duration + 2.weeks
user_subscription.delay(run_at: (user_subscription.expiration + 1.minute)).deactivate_expiration_check
user_subscription.save
# If the payment status is refunded or partially refunded
elsif (params[:payment_status] === 'Refunded') || (params[:payment_status] === 'Partially_Refunded')
# Find the receipt
charge_id = params[:parent_txn_id]
receipt = Receipt.find_by_charge_id(charge_id)
if receipt.present?
# params[:mc_gross] is negative so add it to the current total
new_amount = receipt.totalUSD + params[:mc_gross].to_f
receipt.totalUSD = new_amount
receipt.save
end
# If the payment status is denied, expired, failed or reversed
elsif (params[:payment_status] === 'Denied') || (params[:payment_status] === 'Expired') || (params[:payment_status] === 'Failed') || (params[:payment_status] === 'Reversed')
# Setup PayPal request
request = Paypal::Express::Request.new(
username: ENV['PAYPAL_USER'],
password: ENV['PAYPAL_PWD'],
signature: ENV['PAYPAL_SIGNATURE']
)
# Cancel the PayPal subscription
request.renew!(user_subscription.pay_id, :Cancel)
# Cancel the subscription
if user_subscription.active?
user_subscription.deactivate
user_subscription.save
end
end
end
end
end
render nothing: true
end
(yes it handles payment)
before_action :set_user_subscription
def pause
months = params[:months] || 1
# If "forever" then cancel subscription
if months == "forever"
deactivate_if_active(@user_subscription)
cancel_subscription_tracking(@user_subscription)
flash[:notice] = "Your subscription has been cancelled."
redirect_to account_settings_users_path(anchor: "tab_subscriptions") and return
end
if months == "now"
if @user_subscription.active?
flash[:notice] = "Your subscription is already active."
redirect_to account_settings_users_path(anchor: "tab_subscriptions") and return
end
if @user_subscription.inactive?
begin
@user_subscription.reactivate_with_old_payment_info
rescue => e
puts "ERROR: in reactivate_with_old_payment_info"
puts "#{e.message}"
notify_airbrake(e)
flash[:error] = "There was an error reactivating your subscription. Please contact support@datacamp.com"
redirect_to account_settings_users_path(anchor: "tab_subscriptions") and return
end
end
reactivate_tracking
flash[:notice] = "Your subscription has been successfully reactivated."
redirect_to account_settings_users_path(anchor: "tab_subscriptions") and return
end
if @user_subscription.active? || @user_subscription.set_for_cancellation?
@user_subscription.pause
pause_tracking(@user_subscription, months)
flash[:notice] = "Your subscription has been successfully paused."
elsif @user_subscription.set_for_pause? || @user_subscription.paused?
extend_pause_tracking(@user_subscription, months)
flash[:notice] = "Your subscription pause has been successfully updated."
end
next_billing_date = next_billing_date(@user_subscription)
if next_billing_date.blank?
flash[:error] = "We encountered an error processing your subscription. Please contact support@datacamp.com."
redirect_to account_settings_users_path(anchor: "tab_subscriptions") and return
end
@user_subscription.set_reactivation_check(next_billing_date.to_date + months.to_i.months)
redirect_to account_settings_users_path(anchor: "tab_subscriptions")
end
private
def set_user_subscription
@user_subscription = current_user.current_subscription
if @user_subscription.blank?
flash[:error] = "We couldn't find a subscription linked to your account. Please contact support@datacamp.com"
redirect_to account_settings_users_path(anchor: "tab_subscriptions") and return
end
end
def pause_tracking(user_subscription, months)
Analytics.track_event(current_user, cookies, 'Subscription Paused')
track_event("Subscription Paused", "main", "subscription", properties: { months: months }, uid: current_user.id)
end
def extend_pause_tracking(user_subscription, months)
Analytics.track_event(current_user, cookies, 'Subscription Pause Extended')
track_event("Subscription Pause Extended", "main", "subscription", properties: { months: months }, uid: current_user.id)
end
def cancel_subscription_tracking(user_subscription)
Analytics.track_event(current_user, cookies, 'Subscription Cancelled')
track_event("Subscription Cancelled", "main", "subscription", uid: current_user.id)
end
def reactivate_tracking
Analytics.track_event(current_user, cookies, 'Subscription Reactivated Manually')
track_event("Subscription Reactivated Manually", "main", "subscription", uid: current_user.id)
end
def deactivate_if_active(user_subscription)
if user_subscription.active? || user_subscription.set_for_pause? || user_subscription.paused?
# Cancel subscription
user_subscription.deactivate
user_subscription.save
end
end
def next_billing_date(user_subscription)
return nil if user_subscription.pay_method != "braintree"
subscription = Braintree::Subscription.find(user_subscription.pay_id)
if subscription.nil? || subscription.next_billing_date.nil?
return 'NA'
else
return subscription.next_billing_date
end
end
(in disguise)
class ApplicationController < ActionController::Base
include FlashMessagesHelper
include NumbersHelper
include ProfilingHelper
include SessionHelper
include AppconfigsHelper
include Trackable
include Lovable::Controller
include Pundit
protect_from_forgery
helper_method :mixpanel_distinct_id
helper_method :default_after_sign_in_redirect_path
helper_method :xeditable?
helper :all
before_filter :log_ram_before
before_filter do
if current_user.blank? && Tenant.app_config('sso_configured') == true
redirect_to saml_init_path
else
authenticate_user!
end
end
before_filter :set_paper_trail_whodunnit
before_filter :assign_bunch_of_stuff, if: lambda { !!params.fetch('sign_in_via_token', false) }
before_filter :prepare_filter_data, except: [:after_sign_in_path_for, :set_user_variables]
before_filter :config_base_currency_id
before_filter :set_locale
before_filter :set_global_scope
before_filter :detect_ie_users
before_filter do
NewRelic::Agent.add_custom_parameters({ tenant: Apartment::Tenant.current, user_id: session[:user_id] })
end
after_filter :flash_to_headers
after_filter :log_ram_after
after_filter :count_objects
around_filter :profile
Sometimes its just a matter of context
class ApplicationController
before_filter :authenticate_user!
end
class PublicController < ApplicationController
skip_before_action :authenticate_user!
end
class PrivateController < ApplicationController
end
class ApplicationController
end
class LoggedAreaController < ApplicationController
before_filter :authenticate_user!
end
class PublicController < ApplicationController
end
class PrivateController < LoggedAreaController
end
Would avoid
Would do
Or leaked logic...
<% if user.reputation > 500 %>
...
<% end %>
<% if user.with_high_reputation? %>
...
<% end %>
# in some controller
user.active = true
# in some controller
user.set_active
Would avoid
Would do
Or bad practices...
def with_high_reputation?
reputation > 500
end
def with_high_reputation?
reputation > HIGH_REPUTATION_SCORE
end
Would avoid
Would do
or maybe useless callbacks
before_save :downcase_email
def downcase_email
self.email = email.respond_to?(:downcase) ? email.downcase : email
end
def email=(value)
super value.respond_to?(:downcase) ? value.downcase : value
end
Would avoid
Would do
Callbacks ain't right
~ Me
Personal quest
Use Callbacks &
I won't call you back
~ Bob Cat
Personal quest
Don't you dare...
~ Posh Dog
Personal quest
What we'll focus on today
Extract contexts!
def accept
if invitation.accepted?
render json: { errors: ['Invitation already accepted'] }, status: 422
else
user = User.create_from_invitation(invitation)
invitation.accepted = true
invitation.save
render json: { user: user }
end
end
Create user
from invitation
Change invitation status
What do you think?
In case you didn't notice
(because you can't really)
Create user
from invitation
Change invitation status
Pay referrer
In case you didn't notice
(because you can't really)
Create user
from invitation
Change invitation status
Pay referrer
1- warm up: signup
2- warm up: create invitation
4- credit user
6- Are we good?
xcontext 'errors happen'
7- use boolean services
They should respond to:
- success?
- issues
Warning! if and even nested if are coming
A solution: github
Another approach
Chaining
@thread = Thread.new(thread_params)
if @thread.save
@message = thread.messages.build(msg_params)
if @message.save
render json: { message: @message }
else
render_errors(@message.errors)
end
else
render_errors(@thread.errors)
end
Example
chain do
@thread = Thread.new(thread_params)
dam(@thread.errors) unless @thread.save
end
chain do
@message = @thread.messages.build(msg_params)
dam(@message.errors) unless @message.save
end
chain { render json: { message: @message } }
on_dam {|error_pool| render_errors(error_pool) }
8- use waterfall