From f00e051eff9a913db5d31574dc431cabbabf7fea Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 21 Jul 2016 10:15:17 +0530 Subject: [PATCH] refactor payment#purchase method, improve environment variable names --- app/controllers/payments_controller.rb | 10 ++- .../ticket_purchases_controller.rb | 2 +- app/models/payment.rb | 14 ++-- config/environments/development.rb | 2 +- config/environments/production.rb | 2 +- db/migrate/20160606040848_create_payments.rb | 4 +- spec/factories/payments.rb | 3 +- spec/models/payment_spec.rb | 66 +++++++++++++------ 8 files changed, 64 insertions(+), 39 deletions(-) diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index 890d0d9e..15f92cd8 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -16,7 +16,7 @@ class PaymentsController < ApplicationController @payment = Payment.new(payment_params) @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) - if @payment.purchase(current_user, @conference, price_in_cents) && @payment.save + if @payment.purchase && @payment.save update_purchased_ticket_purchases redirect_to conference_conference_registration_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } else @@ -26,10 +26,6 @@ class PaymentsController < ApplicationController private - def price_in_cents - (@payment.amount * 100).round - end - def update_purchased_ticket_purchases paid_ticket_purchases = current_user.ticket_purchases.by_conference(@conference).unpaid paid_ticket_purchases.each do |ticket| @@ -40,6 +36,8 @@ class PaymentsController < ApplicationController end def payment_params - params.require(:payment).permit(:first_name, :last_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) + params.require(:payment) + .permit(:first_name, :last_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) + .merge(user: current_user, conference: @conference) end end diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index 7b07f33d..ff52021a 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -4,7 +4,7 @@ class TicketPurchasesController < ApplicationController authorize_resource :conference_registrations, class: Registration def create - TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: false) + TicketPurchase.by_conference(@conference).unpaid.by_user(current_user).destroy_all message = TicketPurchase.purchase(@conference, current_user, params[:tickets][0]) if message.blank? if current_user.ticket_purchases.by_conference(@conference).unpaid.any? diff --git a/app/models/payment.rb b/app/models/payment.rb index cc94db2d..25f2592f 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -16,6 +16,8 @@ class Payment < ActiveRecord::Base validates :expiration_month, presence: true, numericality: { greater_than_or_equal_to: 1, less_than_or_equal_to: 12 } validates :expiration_year, presence: true validates :amount, presence: true, numericality: { greater_than: 0 } + validates :user_id, presence: true + validates :conference_id, presence: true enum status: { unpaid: 0, @@ -34,17 +36,18 @@ class Payment < ActiveRecord::Base ) end - def purchase(user, conference, price_in_cents) + def amount_to_pay + Ticket.total_price(conference, user, paid: false).cents + end + + def purchase gateway_response = begin - GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) + GATEWAY.purchase(amount_to_pay, credit_card, currency: conference.tickets.first.price_currency) rescue ActiveMerchant::Billing::Response.new(false, 'Unable to receive any response from the payment gateway.') end - if gateway_response.success? - self.user_id = user.id - self.conference_id = conference.id self.last4 = credit_card.display_number self.authorization_code = gateway_response.authorization self.status = 'success' @@ -56,4 +59,3 @@ class Payment < ActiveRecord::Base success? end end - diff --git a/config/environments/development.rb b/config/environments/development.rb index 2a4ad4c4..126a7ea1 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -98,7 +98,7 @@ Osem::Application.configure do #Initialize Payment Gateway with valid credentials ActiveMerchant::Billing::Base.mode = :test - ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new(:login => ENV['SECRET_KEY']) + ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new(:login => ENV['OSEM_GATEWAY_TEST_SECRET_KEY']) end diff --git a/config/environments/production.rb b/config/environments/production.rb index b5a9d46f..92090758 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -86,5 +86,5 @@ Osem::Application.configure do # Initialize Payment Gateway with valid credentials ActiveMerchant::Billing::Base.mode = :test - ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new(:login => ENV['SECRET_KEY']) + ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new(:login => ENV['OSEM_GATEWAY_LIVE_SECRET_KEY']) end diff --git a/db/migrate/20160606040848_create_payments.rb b/db/migrate/20160606040848_create_payments.rb index a86fc303..ebe2215b 100644 --- a/db/migrate/20160606040848_create_payments.rb +++ b/db/migrate/20160606040848_create_payments.rb @@ -7,8 +7,8 @@ class CreatePayments < ActiveRecord::Migration t.decimal :amount, precision: 12, scale: 3 t.string :authorization_code t.integer :status, default: 0 - t.integer :user_id - t.integer :conference_id + t.integer :user_id, null: false + t.integer :conference_id, null: false t.datetime :created_at t.datetime :updated_at diff --git a/spec/factories/payments.rb b/spec/factories/payments.rb index de852124..7cdbfabc 100644 --- a/spec/factories/payments.rb +++ b/spec/factories/payments.rb @@ -1,5 +1,7 @@ FactoryGirl.define do factory :payment do + user + conference first_name { "#{Faker::Hipster.word} abc" } last_name { "#{Faker::Hipster.word} xyz" } credit_card_number '4242424242424111' @@ -17,4 +19,3 @@ FactoryGirl.define do credit_card_number '4242424242424333' end end - diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index a5906f65..8c3959e1 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -28,6 +28,10 @@ describe Payment do it { is_expected.to validate_presence_of(:amount) } + it { is_expected.to validate_presence_of(:user_id) } + + it { is_expected.to validate_presence_of(:conference_id) } + it 'is not valid with a amount equals zero' do should_not allow_value(0).for(:amount) end @@ -42,24 +46,53 @@ describe Payment do end + describe '#credit_card' do + let(:payment) { create(:payment) } + + it 'assigns correct "month"' do + expect(payment.credit_card.month).to eq(6) + end + + it 'assigns correct "year"' do + expect(payment.credit_card.year).to eq(Date.current.year + 2) + end + + it 'assigns correct "verification_value"' do + expect(payment.credit_card.verification_value).to eq('123') + end + + it 'assigns correct "card_number"' do + expect(payment.credit_card.display_number).to eq('XXXX-XXXX-XXXX-4111') + end + end + + describe '#amount_to_pay' do + let!(:user) { create(:user) } + let!(:conference) { create(:conference) } + let(:ticket_1) { create(:ticket, price: 10, price_currency: 'USD', conference: conference) } + let(:payment) { create(:payment, user: user, conference: conference) } + + it ' returns correct unpaid amount' do + create(:ticket_purchase, ticket: ticket_1, user: user, quantity: 8) + expect(payment.amount_to_pay).to eq(8000) + end + end + describe '#purchase' do let!(:user) { create(:user) } let!(:ticket_1) { create(:ticket) } let!(:conference) { create(:conference, tickets: [ticket_1]) } - let(:payment) { create(:payment) } + let!(:payment) { create(:payment, user: user, conference: conference) } - it 'calls the payment gateway with the correct parameters' do - expect(GATEWAY).to receive(:purchase).with(1000, payment.credit_card, currency: 'USD') - .and_return(ActiveMerchant::Billing::Response.new(true, 'Success.')) + let!(:tickets) { {ticket_1.id.to_s => '1'} } - payment.purchase(user, conference, 1000) - end + before { TicketPurchase.purchase(conference, user, tickets) } context 'when the payment is successful' do - before { payment.purchase(user, conference, 1000) } + before { payment.purchase } it 'returns true' do - payment_result = payment.purchase(user, conference, 1000) + payment_result = payment.purchase expect(payment_result).to eq true end @@ -67,31 +100,23 @@ describe Payment do expect(payment.status).to eq('success') end - it 'assigns user_id' do - expect(payment.user_id).to eq(user.id) - end - - it 'assigns conference_id' do - expect(payment.conference_id).to eq(conference.id) - end - it 'assigns last4' do expect(payment.last4).to eq('XXXX-XXXX-XXXX-4111') end it 'assigns authorization_code' do - expect(payment.authorization_code).to eq("53433") + expect(payment.authorization_code).to eq('53433') end end context 'if the payment is not successful' do - before { payment.purchase(user, conference, 1000) } + before { payment.purchase } let(:payment) { create(:payment, :invalid_credit_card) } context 'when the card is invalid' do it 'returns false' do - payment_result = payment.purchase(user, conference, 1000) + payment_result = payment.purchase expect(payment_result).to eq false end @@ -108,7 +133,7 @@ describe Payment do let(:payment) { create(:payment, :exception_credit_card) } it 'returns false' do - payment_result = payment.purchase(user, conference, 1000) + payment_result = payment.purchase expect(payment_result).to eq false end @@ -123,4 +148,3 @@ describe Payment do end end end -