From e8f1645fc28108ee7babfcab256824368a2c8772 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:06:36 +0530 Subject: [PATCH 01/34] add active merchant and payment schema --- Gemfile | 3 +++ Gemfile.lock | 6 ++++++ db/migrate/20160606040848_create_payments.rb | 18 ++++++++++++++++++ ...73948_add_payment_id_to_ticket_purchases.rb | 5 +++++ db/schema.rb | 12 ++++++++++++ 5 files changed, 44 insertions(+) create mode 100644 db/migrate/20160606040848_create_payments.rb create mode 100644 db/migrate/20160610073948_add_payment_id_to_ticket_purchases.rb diff --git a/Gemfile b/Gemfile index 01bd5af1..e9ad0901 100644 --- a/Gemfile +++ b/Gemfile @@ -183,6 +183,9 @@ gem 'faker' # for seeds gem 'factory_girl_rails' +# for online payments +gem 'activemerchant' + # Use guard and spring for testing in development group :development do # to launch specs when files are modified diff --git a/Gemfile.lock b/Gemfile.lock index 4647f23c..43245d31 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,6 +26,11 @@ GEM activejob (4.2.5.2) activesupport (= 4.2.5.2) globalid (>= 0.3.0) + activemerchant (1.59.0) + activesupport (>= 3.2.14, < 5.1) + builder (>= 2.1.2, < 4.0.0) + i18n (>= 0.6.9) + nokogiri (~> 1.4) activemodel (4.2.5.2) activesupport (= 4.2.5.2) builder (~> 3.1) @@ -529,6 +534,7 @@ PLATFORMS DEPENDENCIES active_model_serializers + activemerchant activeuuid acts_as_commentable_with_threading acts_as_list diff --git a/db/migrate/20160606040848_create_payments.rb b/db/migrate/20160606040848_create_payments.rb new file mode 100644 index 00000000..a86fc303 --- /dev/null +++ b/db/migrate/20160606040848_create_payments.rb @@ -0,0 +1,18 @@ +class CreatePayments < ActiveRecord::Migration + def change + create_table :payments do |t| + t.string :first_name + t.string :last_name + t.string :last4 + 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.datetime :created_at + t.datetime :updated_at + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20160610073948_add_payment_id_to_ticket_purchases.rb b/db/migrate/20160610073948_add_payment_id_to_ticket_purchases.rb new file mode 100644 index 00000000..133a047b --- /dev/null +++ b/db/migrate/20160610073948_add_payment_id_to_ticket_purchases.rb @@ -0,0 +1,5 @@ +class AddPaymentIdToTicketPurchases < ActiveRecord::Migration + def change + add_column :ticket_purchases, :payment_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index aa8021c8..57b7a211 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -250,6 +250,17 @@ ActiveRecord::Schema.define(version: 20160624151257) do t.datetime "updated_at" end + create_table "payments", force: :cascade do |t| + t.string "first_name" + t.string "last_name" + t.string "last4" + t.decimal "amount", precision: 12, scale: 3 + t.string "authorization_code" + t.integer "status", default: 0 + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "programs", force: :cascade do |t| t.integer "conference_id" t.integer "rating", default: 0 @@ -396,6 +407,7 @@ ActiveRecord::Schema.define(version: 20160624151257) do t.datetime "created_at" t.integer "quantity", default: 1 t.integer "user_id" + t.integer "payment_id" end create_table "tickets", force: :cascade do |t| From 36f2cf43735f1ba4dafc192925958c0a48968ab3 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:13:29 +0530 Subject: [PATCH 02/34] add payment model and its associations --- app/models/conference.rb | 1 + app/models/payment.rb | 75 ++++++++++++++++++++++++++++++++++++++++ app/models/user.rb | 1 + 3 files changed, 77 insertions(+) create mode 100644 app/models/payment.rb diff --git a/app/models/conference.rb b/app/models/conference.rb index 149504d5..042010c5 100644 --- a/app/models/conference.rb +++ b/app/models/conference.rb @@ -20,6 +20,7 @@ class Conference < ActiveRecord::Base has_one :program, dependent: :destroy has_one :venue, dependent: :destroy has_many :ticket_purchases, dependent: :destroy + has_many :payments, dependent: :destroy has_many :supporters, through: :ticket_purchases, source: :user has_many :tickets, dependent: :destroy diff --git a/app/models/payment.rb b/app/models/payment.rb new file mode 100644 index 00000000..75e4a180 --- /dev/null +++ b/app/models/payment.rb @@ -0,0 +1,75 @@ +class Payment < ActiveRecord::Base + has_many :ticket_purchases + belongs_to :user + belongs_to :conference + + attr_accessor :credit_card_number + attr_accessor :credit_card_type + attr_accessor :card_verification_value + attr_accessor :expiration_month + attr_accessor :expiration_year + + validates :first_name, presence: true + validates :last_name, presence: true + validates :credit_card_number, presence: true + validates :card_verification_value, presence: true, length: { minimum: 3, maximum: 4 } + 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 } + + validate :validate_card, on: create + + enum status: { + unpaid: 0, + success: 1, + failure: 2 + } + + def credit_card + @credit_card = ActiveMerchant::Billing::CreditCard.new( + first_name: first_name, + last_name: last_name, + number: credit_card_number, + month: expiration_month, + year: expiration_year, + verification_value: card_verification_value + ) + end + + def validate_card + unless credit_card.validate.empty? + credit_card.errors.full_messages.each do |message| + errors.add(:base, message) + end + end + credit_card.validate.empty? + end + + def price_in_cents + (amount * 100).round + end + + def purchase(user, conference) + begin + response = GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) + rescue + false + end + + unless response + errors.add(:base, 'Unable to recieve any response') + return false + end + unless response.success? + errors.add(:base, response.message) + self.status = 2 + return false + end + self.user_id = user.id + self.conference_id = conference.id + self.last4 = credit_card.display_number + self.authorization_code = response.authorization + self.status = 1 + response.success? + end +end diff --git a/app/models/user.rb b/app/models/user.rb index d3c847a2..4878ed9a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,6 +38,7 @@ class User < ActiveRecord::Base has_many :registrations, dependent: :destroy has_many :events_registrations, through: :registrations has_many :ticket_purchases, dependent: :destroy + has_many :payments, dependent: :destroy has_many :tickets, through: :ticket_purchases, source: :ticket has_many :votes, dependent: :destroy has_many :voted_events, through: :votes, source: :events From 900a1b32a8d1b3d7867c43d1167deb8ff45c81e8 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:18:52 +0530 Subject: [PATCH 03/34] add payment resources, controllers and method for updating purchased ticket's parameters --- app/controllers/payments_controller.rb | 34 ++++++++++++++++++++++++++ app/models/ability.rb | 1 + app/models/ticket_purchase.rb | 11 +++++++++ config/routes.rb | 1 + 4 files changed, 47 insertions(+) create mode 100644 app/controllers/payments_controller.rb diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb new file mode 100644 index 00000000..bd633ed2 --- /dev/null +++ b/app/controllers/payments_controller.rb @@ -0,0 +1,34 @@ +class PaymentsController < ApplicationController + before_action :authenticate_user! + load_and_authorize_resource + load_resource :conference, find_by: :short_title + authorize_resource :conference_registrations, class: Registration + + def index + @payments = Payment.where(user_id: current_user.id) + end + + def new + @total_amount_to_pay = Ticket.total_price(@conference, current_user, 'f') + end + + def create + @payment = Payment.new(payment_params) + @total_amount_to_pay = Ticket.total_price(@conference, current_user, 'f') + + if @payment.valid? + if @payment.purchase(current_user, @conference) + @payment.save + @update_ticket_purchases = TicketPurchase.update_paid_ticket_purchases(@conference, current_user, @payment) + return redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } + end + end + render 'new' + end + + private + + def payment_params + params.require(:payment).permit(:first_name, :last_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) + end +end diff --git a/app/models/ability.rb b/app/models/ability.rb index fb48808a..047812a6 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -81,6 +81,7 @@ class Ability can :index, Ticket can :manage, TicketPurchase, user_id: user.id + can [:index, :new, :create], Payment can [:create, :destroy], Subscription, user_id: user.id diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index 8b0cd555..af5d7da6 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -53,4 +53,15 @@ class TicketPurchase < ActiveRecord::Base purchase.quantity = quantity if quantity > 0 purchase end + + def self.update_paid_ticket_purchases(conference, user, payment) + paid_ticket_purchases = TicketPurchase.where(conference_id: conference.id, + user_id: user.id, + paid: 'f') + begin + paid_ticket_purchases.each do |ticket| + ticket.update_columns(paid: 't', payment_id: payment.id) + end + end + end end diff --git a/config/routes.rb b/config/routes.rb index 3e83fd1b..73943cb7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -111,6 +111,7 @@ Osem::Application.routes.draw do resource :conference_registration, path: 'register' resources :tickets, only: [:index] resources :ticket_purchases, only: [:create, :destroy] + resources :payments, only: [:index, :new, :create] resource :subscriptions, only: [:create, :destroy] member do From 81ef41be8b81648b6c197524610c7ecd7eb8acf0 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:25:20 +0530 Subject: [PATCH 04/34] modify controllers and models to just save paid ticket purchases --- .../conference_registrations_controller.rb | 4 +++- .../ticket_purchases_controller.rb | 5 ++-- app/models/ticket.rb | 24 +++++++++++++------ app/models/ticket_purchase.rb | 9 +++---- 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 2fb080aa..441c2738 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -28,8 +28,10 @@ class ConferenceRegistrationsController < ApplicationController end def show - @total_price = Ticket.total_price(@conference, current_user) + TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: 'f') + @total_price = Ticket.total_price(@conference, current_user, 't') @tickets = current_user.ticket_purchases.where(conference_id: @conference.id) + @ticket_payments = @tickets.group_by(&:payment_id) end def edit; end diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index 30d24d82..571139e4 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -6,9 +6,8 @@ class TicketPurchasesController < ApplicationController def create message = TicketPurchase.purchase(@conference, current_user, params[:tickets][0]) if message.blank? - if current_user.ticket_purchases.any? - redirect_to conference_conference_registration_path(@conference.short_title), - notice: "Thank you for supporting #{@conference.title} by purchasing a ticket." + if current_user.ticket_purchases.where(paid: 'f').any? + redirect_to new_conference_payment_path, notice: 'Please pay here to purchase tickets.' else redirect_to conference_conference_registration_path(@conference.short_title) end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 54f5c3d7..18de5c24 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -21,21 +21,31 @@ class Ticket < ActiveRecord::Base ticket_purchases.find_by(user: user, paid: true).present? end - def quantity_bought_by(user) - result = ticket_purchases.where(user_id: user.id).first - result ? result.quantity : 0 + def quantity_bought_by(user, paid) + result = ticket_purchases.where(user_id: user.id, paid: paid) + quantity = 0 + if result + result.each do |ticket| + quantity += ticket.quantity + end + end + quantity end - def total_price(user) - quantity_bought_by(user) * price + def unpaid?(user) + ticket_purchases.find_by(user: user, paid: false).present? end - def self.total_price(conference, user) + def total_price(user, paid) + quantity_bought_by(user, paid) * price + end + + def self.total_price(conference, user, paid) tickets = Ticket.where(conference_id: conference.id) result = nil begin tickets.each do |ticket| - price = ticket.total_price(user) + price = ticket.total_price(user, paid) if result result += price unless price.zero? else diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index af5d7da6..0ca5ae49 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -7,10 +7,6 @@ class TicketPurchase < ActiveRecord::Base validates_numericality_of :quantity, greater_than: 0 - validates_uniqueness_of :user_id, - scope: :ticket_id, - message: 'already bought this ticket!' - delegate :title, to: :ticket delegate :description, to: :ticket delegate :price, to: :ticket @@ -23,7 +19,7 @@ class TicketPurchase < ActiveRecord::Base conference.tickets.each do |ticket| quantity = purchases[ticket.id.to_s].to_i # if the user bought the ticket, just update the quantity - if ticket.bought?(user) + if ticket.bought?(user) && ticket.unpaid?(user) purchase = update_quantity(conference, quantity, ticket, user) else purchase = purchase_ticket(conference, quantity, ticket, user) @@ -48,7 +44,8 @@ class TicketPurchase < ActiveRecord::Base def self.update_quantity(conference, quantity, ticket, user) purchase = TicketPurchase.where(ticket_id: ticket.id, conference_id: conference.id, - user_id: user.id).first + user_id: user.id, + paid: 'f').first purchase.quantity = quantity if quantity > 0 purchase From 14594c7a4dc661831d07d6b97bfd69d7032a4ddd Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:30:17 +0530 Subject: [PATCH 05/34] add payment views, styling and its helper methods --- app/assets/images/credit_card.png | Bin 0 -> 4086 bytes app/assets/stylesheets/application.css | 1 + app/assets/stylesheets/osem-payments.css.scss | 24 +++++++++++++ app/helpers/payments_helper.rb | 13 +++++++ app/views/payments/_payment.html.haml | 34 ++++++++++++++++++ app/views/payments/index.html.haml | 32 +++++++++++++++++ app/views/payments/new.html.haml | 14 ++++++++ 7 files changed, 118 insertions(+) create mode 100644 app/assets/images/credit_card.png create mode 100644 app/assets/stylesheets/osem-payments.css.scss create mode 100644 app/helpers/payments_helper.rb create mode 100644 app/views/payments/_payment.html.haml create mode 100644 app/views/payments/index.html.haml create mode 100644 app/views/payments/new.html.haml diff --git a/app/assets/images/credit_card.png b/app/assets/images/credit_card.png new file mode 100644 index 0000000000000000000000000000000000000000..a5e0b1e3409e051fb7def7231e977c8c133d97c8 GIT binary patch literal 4086 zcmbW1`6JYc%*iX2#L5YG=rmqj98hw9SmObd0E^YcPZ~P2|kT z5g}>1B(=NasVyL{wyC!r}O%ar=Zn3G!g%he$*to|uL)tEF34>zlNLP!|l=4d9+& zkOKhFhrkX&@J>+s6^hhjYc*k}clXdbS)k>0v2|&b<&2nG%=D@;a;b@k?udwOJrLYy z=lxX%`%naY&V$zSz@^LJS~2W<9`0MQ+EM}TpFHB%%fuCh_H-8JV;N?x5?ZcAd@U!> zR*^ra(EiS{TFJHjQ_1SjHe9~!yLy%X`Ht7>jYBUhlKyG&$AHOHVR=Y=LEs@7|5%0a zNFi{d9XRzwepGVfgj^AldP;sh@mw*G{1}w}24ua^IM)rzdNpJ{U`{_+GJ?( zC>UTB_EO4Uqsw1AR{suGjKgXt(GAlS<+NkNxcU7#hvr%1`U$VP5%=n$UCPmfhOPu< z_wfeU6E8lU z?E9RWl6vuiytt@XUT{`gc<(}9ZGKf#Lw$WkS3oKle8%pQQgfasTejvxnU;ejP7qdsVJ_QPt6Zzq7Ad)t>%r zE~Rhj{J>(t&{FR6x!Tdi+s|k2yjvaY{H>#FXrOOk`1jF){?VtM&))w&JU96K``DAO zGlPH4^^J~>^$*QHe>FGt$K=q|^2qF`_wV1&&CN}{|FAmu?w{`;mX|)hpZxlM@$2lD z)rEzHwJ(d`zb!3(UR(OU_HUL}KQFxf=YK2yf5Kn|{=X*uKmPX#0KEouVyGUGyE5gs{a0sEX*VkTMyXdQq|^6=|82Fx8D;mnh*JiEX(`*4kD`Gf08s$P@yNA!<*- z-my037$t+5FcelU5R^B@#`<&zb-GvM|L%~Ouei|n2qg)ZJbu)0aDG_Sj0%aTddM#MRmUf%Y6>_U{14OO zRJ{G82dQ}WQ;viX;skb-p4iGnki4l35UJrN9`heC2$-lS-=8ueODFlGH*ntzYxT}& z#J@}Kw=D9_^>uz_oT6tJ6@r2eze>)#ia@s_*Jx5t3+^6;!-yKQU+$uA*AGN%*njBR zq{(A*5oG@lx@n}K48AKttv_nI9&$k7D%+$BohA+@S~FLr^THYZLlMp9_aal))rl%) znLU?g_D5Ag>k`Vi&<6f6h~D4?z)>&DVi{rpnPScAr&&j%bViu^oa z^<&rpM~Z*|;2rxV<)iE&Rohw#o4`HX8D?uBA||CXL8Se9&FQ+oJ3Mn7?0f{DBm?hM89`y|-0OE#~jpp8dB1oA^x8-DW`;~L&U zRW#S2ZLcZ19T3QL)%KEpkP>3NuRnarN_y$pZLswsMmgJL@I-55kneW)y2M#!E7OL8f2~3Qew@6F2ocbX9m3?8d zRZ68G9S9;w$R!f;JA%YQusNO8W2ha;Z~+>VVcOYm;V+alTi95sj-#)} zga#kyyFlJ!*%n1_&273K7i?96aV4#!MBM zW&1D;^`vp>6Lo#Yq-?_!t%!6rGm2!OYCs-_h> z_N&0aj7m73mulxUpRA!2Zkpk@LGL9NueP|J@F4|!>9U2bA2VDprlIWL3h?`BLh8*# z1Y54cMbbdMdZim(N|S~rK-$l1+-x6R#K#octREe^5|H29?VvS5Y@?-{YJpe>?FmZ$ z7@V2yhw)+?QcL4STz4T?8%`s42|z4d1taFSxVzAvkk+t;4G17LHFVClB@@Y=0`tHsXZq z45|?Jva<6g9^=>>{B%~cA9i2hPHh+V)I@uAruUeiW~QovhAl_C(qWosIB=si#_Pk( zd24m)3xkamz|YADIE@F)OARC#Ul(b(7_kc}$3CeR=fc=A?&FbpH_F5Fs0=RVuJB)R z-los+(?8MVc%oQ2)WdWJ7nTs?D%Z_X*RzAIhx5CTw4(#71(A3_+>7xx|Zn&>4Y>I zJRD|5u1koU-h(_zsA+O#-D4E4p?DW%ahJxRSQUa%-}np})@q}{;G^mGAZqn}t zz%0%=cDs;Vqe^Gxx12jrZ>aas!`M0mJV(iPruH-;?CV>yVgl{-=4frZwtPDW(>5fO zDTkSyix@AG3#EkOwo#x^3+KP3yrZP4wRh$1WcMdKKUN6%@+?26aMJbph}*$_Ty3)( z*1i*?<{w)N62gq^sw`1f)Go>S(f*d)a#^B(7;-u=f z3y^MRt+=Q%$Y@!DboXe*ot}qiR0ABhtd5YuL)4{n)ePC!a*zvd0oEeomFX#chn7qq zafRl2hMU#ezTKG-*&;+GCCx0TJB5rZ%k7$e%+XjWASWygjfh*Y9!#u1nnt$#pvHbS z?_4ndCq2yJtUav^CMgZqCGb<(;_~$VnlaKo3CFV0UWR0kt(#)}wplPM%>EGF;qB9GCpVwf~fWul`M@hhvMz)2!sfwED|Peh*8#uyKVx zRhaG5J5FjiHAxM)USC98GGPnphgyq?xcNqiddVA`a@qN5{XBft&H#OWQ9Qn~MniLX zDHEqjk2fG&;=K6IT1SsajSn+rfpU>fEx*H}=@xdU(vTdiSn<8rb@#&UU(|r*qyZ4w zv2ZfQ#_$Bp&9@BUUuNnHoYJ5m{KRf27hL^Sh=FToB1ND|2*kmlNB&#w36KMCGn;|% zk)-f@=x<0rnh+5IWe8^;K&?1~87HOPM{#eXICoIT1gPU;)Da%)AW!O+VeZ|}S!cHY z`xDaZ`7;1T0!?a#G7W=OrqSDyc|2Cf0yEke5y&~#nIj31r|rFT%nPkYQL(HE^tha@ zP7(trfbOJ%?05)C4&7Ucz?rdk0B{{1NTTf?@Jl=J?f#}? z@sfx1OU!JzFCFk>MN3(sg0K<~)=B^L9#i(7ZO4pk5TQ!AKj+xV!t|&V5X%GAXpr{d zllDAF#WH1bfL+QoViVkO!PR#jZkhsW@lw&PnrsR>YdM<>*&ZLukYz}Ze?I7S$7;ti zqA(*9Hf_F}o$WiHdFFPSX`5+?3jWNWRcB_{whZ4<2AU`l$_$YBujP1pMk>g@2cqOM z?Pw5A4(Jl#Ap%4Q2l2NHVi(N}BLFe-Q@#mmrv#|8i5GwLkRD`6-IkGVL+Igo%BgWq z&b09kHgdEu=gqQ}XKxyLIfsE+%WXsF3gs|!Fr(302 z!f=s1M_iPdH)$>ylZ{hk!F>AhqO=o51%8~JVJh>!je3o@a<#;Te^WqeV&Pd`Fl9Pd z=AfrV(7VIclbEiLEh5zr^e^-jDKMBa-Qo(};v7K#o&Y}}FRrUCem(bltBV#3Dk0X0@6@BK4 z-o1(!#}&9YEgL!>XpSoJZ!#5wMT!YbX|HbSa3_AmVYeQr7^qde4KD3oDV@axFl|dD z$-ZGAQK-4!yj-1;dfD3`(WXUFDpFrJeg$htm}2Nu>1;%25&jA;C0SgV7{<@m+7g5T zj5M8}ddvL`bdP@W3-u!#KHFgW$&c!&4Pb@T?b@MSo#v@>WXSs9-YeE2f- ct3-~~5h2x4N!2mM)tZ{s`(`yk5cuK$0P%6UEdT%j literal 0 HcmV?d00001 diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css index d0447354..84c0bd9a 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.css @@ -10,6 +10,7 @@ *= require osem-splash *= require font-awesome *= require osem-fonts + *= require osem-payments *= require bootstrap-markdown *= require bootstrap-datetimepicker *= require leaflet diff --git a/app/assets/stylesheets/osem-payments.css.scss b/app/assets/stylesheets/osem-payments.css.scss new file mode 100644 index 00000000..b74a6324 --- /dev/null +++ b/app/assets/stylesheets/osem-payments.css.scss @@ -0,0 +1,24 @@ +.price-tags { + list-style-type: none; + padding-top: 2rem; + // display: inline-block; +} +.price-tags li { + line-height: 40px; + position: relative; + margin-right: -3rem; +} +.price-tags a { + background: #2f991d; + color: #fff; + font-size: 1.5rem; + padding: 9px 10px; + text-decoration: none; +} +.price-tags a:after { + content: ""; + float: left; + border-top: 20px solid transparent; + border-right: 20px solid #2f991d; + border-bottom: 20px solid transparent; +} diff --git a/app/helpers/payments_helper.rb b/app/helpers/payments_helper.rb new file mode 100644 index 00000000..50b53262 --- /dev/null +++ b/app/helpers/payments_helper.rb @@ -0,0 +1,13 @@ +module PaymentsHelper + def months + (1..12).collect{|n| ["#{n} - #{Date::MONTHNAMES[n]}", n]} + end + + def years + (Date.current.year..Date.current.year + 15) + end + + def card_types + [[['Visa', 'visa'], ['MasterCard', 'master'], ['Discover', 'discover'], ['American Express', 'american_express']]] + end +end diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml new file mode 100644 index 00000000..c0f47e38 --- /dev/null +++ b/app/views/payments/_payment.html.haml @@ -0,0 +1,34 @@ += semantic_form_for(@payment, url: conference_payments_path) do |f| + .form-group + = f.label :first_name, 'First name(as on card)' + = f.text_field :first_name, class: "form-control", placeholder: "John" + .form-group + = f.label :last_name, 'Last name(as on card)' + = f.text_field :last_name,class: "form-control", placeholder: "Doe" + %span.pull-right + %img.img-responsive{:src => image_url('credit_card.png')} + .form-group + = f.label :credit_card_number, "Credit Card number(without spaces)" + .input-group + = f.text_field :credit_card_number, class: "form-control", placeholder: "XXXX XXXX XXXX XXXX" + %span.input-group-addon + %i.fa.fa-credit-card + .form-group.col-md-6 + = f.label :expiration_month + = f.select :expiration_month, months, {}, class: "form-control" + .form-group.col-md-6 + = f.label :expiration_year + = f.select :expiration_year, years, {}, class: "form-control" + .form-group.col-md-10 + = f.label :card_verification_value, 'Security Code(3 on back, AmEx: 4 on front)' + = f.text_field :card_verification_value, class: "form-control", placeholder: "XXX" + = f.number_field :amount, value: @total_amount_to_pay, class: "form-control", type: 'hidden' + %ul.price-tags.pull-right + %li.text-muted + you will pay + %li + %a + = number_to_currency @total_amount_to_pay + .form-group + = f.submit "Charge Card", class: "btn btn-primary" + = link_to "Cancel", conference_conference_registrations_path, class: "btn btn-danger" diff --git a/app/views/payments/index.html.haml b/app/views/payments/index.html.haml new file mode 100644 index 00000000..702e02e1 --- /dev/null +++ b/app/views/payments/index.html.haml @@ -0,0 +1,32 @@ +.container + .row + .col-md-12 + .page-header + %h1 + Payment Attempts for + = @conference.title + - if !flash[:notice].blank? + .alert.alert-success + = flash[:notice] + %table.table.table-bordered.table-striped + %tr + %th First Name + %th Last Name + %th Last 4 + %th Amount + %th Status + %th Authorization Code + - if @payments.size > 0 + - @payments.each do |payment| + %tr + %td= payment.first_name + %td= payment.last_name + %td= payment.last4 + %td= number_to_currency payment.amount + %td= payment.status + %td= payment.authorization_code + - else + %tr + %td{:colspan => "5"} No payments have been attempted. + .pull-right + = link_to "Conference Registration", conference_conference_registrations_path(@conference.short_title), class: 'btn btn-primary' diff --git a/app/views/payments/new.html.haml b/app/views/payments/new.html.haml new file mode 100644 index 00000000..f7e4fc36 --- /dev/null +++ b/app/views/payments/new.html.haml @@ -0,0 +1,14 @@ +.container + .row + .col-xs-6.col-xs-offset-3 + %h1 + Buy tickets for + = @total_amount_to_pay.symbol + = humanized_money @total_amount_to_pay + - if @payment.errors.any? + .alert.alert-danger + %ul + - @payment.errors.full_messages.each do |msg| + %li= msg + .col-xs-6.col-xs-offset-3.well + = render partial: 'payment' From b202984fa9f2ea4704e4b6e48a7c688aeea616c1 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:32:21 +0530 Subject: [PATCH 06/34] update ticket purchase and conference registration views for new structure --- .../conference_registrations/show.html.haml | 43 ++++++++++--------- app/views/tickets/_ticket.html.haml | 2 +- app/views/tickets/index.html.haml | 11 +++-- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/app/views/conference_registrations/show.html.haml b/app/views/conference_registrations/show.html.haml index 30cb281b..3998d851 100644 --- a/app/views/conference_registrations/show.html.haml +++ b/app/views/conference_registrations/show.html.haml @@ -89,29 +89,30 @@ %span.fa-stack %i.fa.fa-square-o.fa-stack-2x %i.fa.fa-ticket.fa-stack-1x - Tickets + Ticket Purchases -if @tickets.any? - = "(#{@total_price} #{@tickets.first.price.symbol})" + = "(#{@tickets.first.price.symbol}#{humanized_money @total_price})" %ul - - @tickets.each do |ticket| - %li - = ticket.quantity - = ticket.title - = word_pluralize(ticket.quantity, 'Ticket') - for - = humanized_money ticket.price - = ticket.price.symbol - = link_to conference_ticket_purchase_path(@conference.short_title, ticket.id), method: :delete, - id: "ticket-#{ticket.id}-delete", - class: 'btn btn-danger btn-xs', - data: { confirm: "Do you really want to delete the #{ticket.title} ticket for #{@conference.title}?" } do - %i.fa.fa-trash-o - %li - - if @tickets.any? - = link_to 'Buy more tickets', conference_tickets_path(@conference.short_title) - - else - You haven't bought any tickets. - = link_to 'Please buy some tickets to support us!', conference_tickets_path(@conference.short_title) + .col.md-4 + - @ticket_payments.each_pair do |payment, tickets| + %strong + Purchased + - tickets.each do |ticket| + %li + = ticket.quantity + = ticket.title + = word_pluralize(ticket.quantity, 'Ticket') + for + = ticket.price.symbol + = humanized_money ticket.price + %hr + - if @tickets.any? + = link_to 'Buy more tickets', conference_tickets_path(@conference.short_title), class: "btn btn-default" + - else + You haven't bought any tickets. + = link_to 'Please buy some tickets to support us!', conference_tickets_path(@conference.short_title) + %p + (Your registration won't be complete without buying a ticket) .row .col-md-12 diff --git a/app/views/tickets/_ticket.html.haml b/app/views/tickets/_ticket.html.haml index c31703b3..7473f96d 100644 --- a/app/views/tickets/_ticket.html.haml +++ b/app/views/tickets/_ticket.html.haml @@ -9,7 +9,7 @@ = markdown(ticket.description) %td.col-sm-1.col-md-1 - if ticket.bought?(current_user) - = text_field_tag("tickets[][#{ticket.id}]", ticket.quantity_bought_by(current_user), + = text_field_tag("tickets[][#{ticket.id}]", 0, type: 'number', min: 0, class: "form-control quantity", 'data-id' => ticket.id) - else = text_field_tag("tickets[][#{ticket.id}]", 0, type: 'number', min: 0, diff --git a/app/views/tickets/index.html.haml b/app/views/tickets/index.html.haml index fbc2df7c..1852d60d 100644 --- a/app/views/tickets/index.html.haml +++ b/app/views/tickets/index.html.haml @@ -5,10 +5,10 @@ %h1 Tickets %p.lead - If you like, support + Please choose your tickets for %strong = @conference.title - by buying a ticket* + here* =form_tag(conference_ticket_purchases_path, method: :post) do |f| %table.table.table-hover %thead @@ -35,12 +35,11 @@ .pull-right .btn-group-vertical = button_tag(type: 'submit', class: 'btn btn-success btn-lg') do - Support + Continue %i.fa.fa-shopping-cart - = link_to 'Continue without a Ticket!', conference_conference_registration_path(@conference.short_title), - class: 'btn btn-danger btn-sm' + = link_to 'Cancel registration', conference_conference_registrations_path(@conference.short_title), method: :delete, class: 'btn btn-danger btn-sm' .row .col-md-13 %p.text-muted.text-center %small - * Buying a ticket is not mandatory. Checkout will be at the conference registration. + * Buying a ticket is mandatory. Your registration will not complete until you buy a ticket. From 809e2b7156bd4e0c1d70b5937451abfa23a828b8 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:32:54 +0530 Subject: [PATCH 07/34] active merchant GATEWAY config --- config/environments/development.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/environments/development.rb b/config/environments/development.rb index 61d9fd57..d85a731f 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -97,3 +97,9 @@ Osem::Application.configure do end end + + +ActiveMerchant::Billing::Base.mode = :test + + ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new( + :login => ENV['SECRET_KEY']) From ee714560607a582faf0338c5e894868b053523c0 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 18 Jun 2016 18:33:36 +0530 Subject: [PATCH 08/34] improve tests[WIP] --- ...conference_registration_controller_spec.rb | 6 +-- spec/features/ticket_purchases_spec.rb | 38 +++++++++++-------- spec/models/ticket_spec.rb | 14 +++---- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/spec/controllers/conference_registration_controller_spec.rb b/spec/controllers/conference_registration_controller_spec.rb index 2d67b228..3c5655f0 100644 --- a/spec/controllers/conference_registration_controller_spec.rb +++ b/spec/controllers/conference_registration_controller_spec.rb @@ -40,9 +40,9 @@ describe ConferenceRegistrationsController, type: :controller do get :show, conference_id: conference.short_title end - it 'assigns price of purchased tickets to total_price and purchased tickets to tickets' do - expect(assigns(:total_price)).to eq Money.new(10000, 'USD') - expect(assigns(:tickets)).to match_array [@purchased_ticket] + it 'does not assign price of purchased tickets to total_price and purchased tickets to tickets without payment' do + expect(assigns(:total_price)).to eq Money.new(0, 'USD') + expect(assigns(:tickets)).not_to match_array [@purchased_ticket] end end diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index ff6957f6..aea999cd 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -26,27 +26,35 @@ feature Registration do fill_in "tickets__#{ticket.id}", with: '2' expect(current_path).to eq(conference_tickets_path(conference.short_title)) - click_button 'Support' + click_button 'Continue' purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first expect(purchase.quantity).to eq(2) - expect(current_path).to eq(conference_conference_registration_path(conference.short_title)) + expect(current_path).to eq(new_conference_payment_path) expect(flash). - to eq("Thank you for supporting #{conference.title} by purchasing a ticket.") + to eq('Please pay here to purchase tickets.') + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + fill_in 'expiration_year', Date.current.year + 2 + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4242424242424242' + + click_button 'Charge Card' + + payment = Payment.where(user_id: participant, conference_id: conference.id).first + expect(payment.amount).to eq(20) + expect(payment.status).to eq(1) + expect(payment.first_name).to eq('foo') + expect(payment.first_name).to eq('bar') + expect(payment.last4).not_to be_empty + expect(payment.authorization_code).not_to be_empty + expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) + expect(flash). + to eq('Thanks! You have purchased your tickets successfully.') + expect(page.has_content?("2 #{ticket.title} Tickets for 10")).to be true end - - scenario 'deletes a purchased ticket', feature: true, js: true do - create(:registration, conference: conference, user: participant) - create(:ticket_purchase, conference: conference, user: participant, ticket: ticket, quantity: 4) - - visit conference_conference_registration_path(conference.short_title) - expect(page.has_content?("4 #{ticket.title} Tickets for 10")).to be true - - click_link "ticket-#{ticket.id}-delete" - expect(flash).to eq('Ticket successfully deleted.') - expect(TicketPurchase.count).to eq(0) - end end end end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index fc67ff3d..496037ec 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -88,11 +88,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.quantity_bought_by(user)).to eq(20) + expect(ticket.quantity_bought_by(user, 'f')).to eq(20) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user)).to eq(0) + expect(ticket.quantity_bought_by(user, 'f')).to eq(0) end end @@ -102,11 +102,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.total_price(user)).to eq(Money.new(100000, 'USD')) + expect(ticket.total_price(user, 'f')).to eq(Money.new(100000, 'USD')) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user)).to eq(Money.new(0, 'USD')) + expect(ticket.total_price(user, 'f')).to eq(Money.new(0, 'USD')) end end @@ -116,7 +116,7 @@ describe Ticket do describe 'user has bought' do context 'no tickets' do it 'returns zero' do - expect(Ticket.total_price(conference, user)).to eq(Money.new(0, 'USD')) + expect(Ticket.total_price(conference, user, 'f')).to eq(Money.new(0, 'USD')) end end @@ -126,7 +126,7 @@ describe Ticket do end it 'returns the correct total price' do - expect(Ticket.total_price(conference, user)).to eq(Money.new(100000, 'USD')) + expect(Ticket.total_price(conference, user, 'f')).to eq(Money.new(100000, 'USD')) end end @@ -138,7 +138,7 @@ describe Ticket do it 'returns the correct total price' do total_price = Money.new(200000, 'USD') - expect(Ticket.total_price(conference, user)).to eq(total_price) + expect(Ticket.total_price(conference, user, 'f')).to eq(total_price) end end end From f202c6b93b6b5f35d80bd5fc8184b0d38138b75a Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sun, 26 Jun 2016 08:07:00 +0200 Subject: [PATCH 09/34] repairing code reviews --- app/controllers/payments_controller.rb | 22 ++++++++++++------- .../ticket_purchases_controller.rb | 1 + app/models/ability.rb | 2 +- app/models/payment.rb | 19 ++-------------- app/models/ticket_purchase.rb | 4 +++- 5 files changed, 21 insertions(+), 27 deletions(-) diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index bd633ed2..a60437a8 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -5,7 +5,7 @@ class PaymentsController < ApplicationController authorize_resource :conference_registrations, class: Registration def index - @payments = Payment.where(user_id: current_user.id) + @payments = current_user.payments end def new @@ -16,18 +16,24 @@ class PaymentsController < ApplicationController @payment = Payment.new(payment_params) @total_amount_to_pay = Ticket.total_price(@conference, current_user, 'f') - if @payment.valid? - if @payment.purchase(current_user, @conference) - @payment.save - @update_ticket_purchases = TicketPurchase.update_paid_ticket_purchases(@conference, current_user, @payment) - return redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } - end + if @payment.valid? && @payment.purchase(current_user, @conference, price_in_cents) + @payment.save + @update_ticket_purchases = TicketPurchase.update_paid_ticket_purchases(@conference, current_user, @payment) + end + + if @payment.save + redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } + else + render 'new' end - render 'new' end private + def price_in_cents + (@payment.amount * 100).round + end + def payment_params params.require(:payment).permit(:first_name, :last_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) end diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index 571139e4..a921839e 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -4,6 +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: 'f') message = TicketPurchase.purchase(@conference, current_user, params[:tickets][0]) if message.blank? if current_user.ticket_purchases.where(paid: 'f').any? diff --git a/app/models/ability.rb b/app/models/ability.rb index 047812a6..8bdcd6d7 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -81,7 +81,7 @@ class Ability can :index, Ticket can :manage, TicketPurchase, user_id: user.id - can [:index, :new, :create], Payment + can :manage, Payment, user_id: user.id can [:create, :destroy], Subscription, user_id: user.id diff --git a/app/models/payment.rb b/app/models/payment.rb index 75e4a180..5694303d 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -17,8 +17,6 @@ class Payment < ActiveRecord::Base validates :expiration_year, presence: true validates :amount, presence: true, numericality: { greater_than: 0 } - validate :validate_card, on: create - enum status: { unpaid: 0, success: 1, @@ -36,20 +34,7 @@ class Payment < ActiveRecord::Base ) end - def validate_card - unless credit_card.validate.empty? - credit_card.errors.full_messages.each do |message| - errors.add(:base, message) - end - end - credit_card.validate.empty? - end - - def price_in_cents - (amount * 100).round - end - - def purchase(user, conference) + def purchase(user, conference, price_in_cents) begin response = GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) rescue @@ -69,7 +54,7 @@ class Payment < ActiveRecord::Base self.conference_id = conference.id self.last4 = credit_card.display_number self.authorization_code = response.authorization - self.status = 1 + self.status = 'success' response.success? end end diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index 0ca5ae49..8fbc9c49 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -57,7 +57,9 @@ class TicketPurchase < ActiveRecord::Base paid: 'f') begin paid_ticket_purchases.each do |ticket| - ticket.update_columns(paid: 't', payment_id: payment.id) + ticket.paid = 't' + ticket.payment_id = payment.id + ticket.save end end end From c0c84b9dbf3a4fd0798caf0478b28b761742aca9 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Tue, 5 Jul 2016 16:39:38 +0530 Subject: [PATCH 10/34] add more explanatory true/false values --- .../conference_registrations_controller.rb | 3 +-- app/controllers/payments_controller.rb | 18 ++++++++---------- app/controllers/ticket_purchases_controller.rb | 4 ++-- app/models/ticket_purchase.rb | 6 +++--- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 441c2738..798a1586 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -28,8 +28,7 @@ class ConferenceRegistrationsController < ApplicationController end def show - TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: 'f') - @total_price = Ticket.total_price(@conference, current_user, 't') + @total_price = Ticket.total_price(@conference, current_user, true) @tickets = current_user.ticket_purchases.where(conference_id: @conference.id) @ticket_payments = @tickets.group_by(&:payment_id) end diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index a60437a8..1ae8d8d9 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -9,22 +9,20 @@ class PaymentsController < ApplicationController end def new - @total_amount_to_pay = Ticket.total_price(@conference, current_user, 'f') + @total_amount_to_pay = Ticket.total_price(@conference, current_user, false) end def create @payment = Payment.new(payment_params) - @total_amount_to_pay = Ticket.total_price(@conference, current_user, 'f') + @total_amount_to_pay = Ticket.total_price(@conference, current_user, false) if @payment.valid? && @payment.purchase(current_user, @conference, price_in_cents) - @payment.save - @update_ticket_purchases = TicketPurchase.update_paid_ticket_purchases(@conference, current_user, @payment) - end - - if @payment.save - redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } - else - render 'new' + if @payment.save + @update_ticket_purchases = TicketPurchase.update_paid_ticket_purchases(@conference, current_user, @payment) + redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } + else + render 'new' + end end end diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index a921839e..8a5084fc 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -4,10 +4,10 @@ class TicketPurchasesController < ApplicationController authorize_resource :conference_registrations, class: Registration def create - TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: 'f') + TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: false) message = TicketPurchase.purchase(@conference, current_user, params[:tickets][0]) if message.blank? - if current_user.ticket_purchases.where(paid: 'f').any? + if current_user.ticket_purchases.where(paid: false).any? redirect_to new_conference_payment_path, notice: 'Please pay here to purchase tickets.' else redirect_to conference_conference_registration_path(@conference.short_title) diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index 8fbc9c49..fd30a2d1 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -45,7 +45,7 @@ class TicketPurchase < ActiveRecord::Base purchase = TicketPurchase.where(ticket_id: ticket.id, conference_id: conference.id, user_id: user.id, - paid: 'f').first + paid: false).first purchase.quantity = quantity if quantity > 0 purchase @@ -54,10 +54,10 @@ class TicketPurchase < ActiveRecord::Base def self.update_paid_ticket_purchases(conference, user, payment) paid_ticket_purchases = TicketPurchase.where(conference_id: conference.id, user_id: user.id, - paid: 'f') + paid: false) begin paid_ticket_purchases.each do |ticket| - ticket.paid = 't' + ticket.paid = true ticket.payment_id = payment.id ticket.save end From 38ae24c6b3317ab2176d7634fd9d140a3c95ea7d Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Wed, 6 Jul 2016 12:40:42 +0530 Subject: [PATCH 11/34] add more tests --- app/models/payment.rb | 2 +- ...conference_registration_controller_spec.rb | 2 +- spec/factories/payments.rb | 11 ++ spec/models/conference_spec.rb | 0 spec/models/payment_spec.rb | 117 ++++++++++++++++++ spec/models/ticket_spec.rb | 34 +++-- 6 files changed, 157 insertions(+), 9 deletions(-) create mode 100644 spec/factories/payments.rb mode change 100755 => 100644 spec/models/conference_spec.rb create mode 100644 spec/models/payment_spec.rb diff --git a/app/models/payment.rb b/app/models/payment.rb index 5694303d..6d5a769f 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -47,7 +47,7 @@ class Payment < ActiveRecord::Base end unless response.success? errors.add(:base, response.message) - self.status = 2 + self.status = 'failure' return false end self.user_id = user.id diff --git a/spec/controllers/conference_registration_controller_spec.rb b/spec/controllers/conference_registration_controller_spec.rb index 3c5655f0..d47d9271 100644 --- a/spec/controllers/conference_registration_controller_spec.rb +++ b/spec/controllers/conference_registration_controller_spec.rb @@ -42,7 +42,7 @@ describe ConferenceRegistrationsController, type: :controller do it 'does not assign price of purchased tickets to total_price and purchased tickets to tickets without payment' do expect(assigns(:total_price)).to eq Money.new(0, 'USD') - expect(assigns(:tickets)).not_to match_array [@purchased_ticket] + expect(assigns(:tickets)).not_to exist end end diff --git a/spec/factories/payments.rb b/spec/factories/payments.rb new file mode 100644 index 00000000..e4ccaee4 --- /dev/null +++ b/spec/factories/payments.rb @@ -0,0 +1,11 @@ +FactoryGirl.define do + factory :payment do + first_name { "#{Faker::Hipster.word} abc" } + last_name { "#{Faker::Hipster.word} xyz" } + credit_card_number { '4242424242424242' } + card_verification_value { '123' } + expiration_month { '06' } + expiration_year { Date.current.year + 2 } + amount { '10' } + end +end diff --git a/spec/models/conference_spec.rb b/spec/models/conference_spec.rb old mode 100755 new mode 100644 diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb new file mode 100644 index 00000000..cd60b03f --- /dev/null +++ b/spec/models/payment_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Payment do + + describe 'validations' do + it 'has a valid factory' do + expect(build(:payment)).to be_valid + end + + it 'is not valid without a first_name' do + should validate_presence_of(:first_name) + end + + it 'is not valid without a last_name' do + should validate_presence_of(:last_name) + end + + it 'is not valid without a credit_card_number' do + should validate_presence_of(:credit_card_number) + end + + it 'is not valid without a card_verification_value' do + should validate_presence_of(:card_verification_value) + end + + it 'is not valid without a expiration_month' do + should validate_presence_of(:expiration_month) + end + + it 'is not valid without a expiration_year' do + should validate_presence_of(:expiration_year) + end + + it 'is not valid without a amount' do + should validate_presence_of(:amount) + end + + it 'is not valid with a amount equals zero' do + should_not allow_value(0).for(:amount) + end + + it 'is not valid with a amount smaller than zero' do + should_not allow_value(-1).for(:amount) + end + + it 'is valid with a amount greater than zero' do + should allow_value(1).for(:amount) + end + + end + + describe 'self#purchase' do + let!(:participant) { create(:user) } + let!(:ticket_1) { create(:ticket) } + let!(:ticket_2) { create(:ticket) } + let!(:conference) { create(:conference, tickets: [ticket_1, ticket_2]) } + + it 'creates a purchase and payment for one ticket' do + tickets = { ticket_1.id.to_s => '1' } + message = TicketPurchase.purchase(conference, participant, tickets) + purchase = TicketPurchase.where(conference_id: conference.id, + user_id: participant.id, + ticket_id: ticket_1.id).first + + expect(TicketPurchase.count).to eq(1) + expect(purchase.quantity).to eq(1) + expect(message.blank?).to be true + + response = Payment.purchase(participant, conference, 1000) + payment = Payment.where(conference_id: conference.id, + user_id: participant.id) + + expect(Payment.count).to eq(1) + expect(payment.amount).to eq(10) + expect(response.blank?).to be true + end + + it 'creates several purchases for more than one ticket' do + tickets = { ticket_1.id.to_s => '1', ticket_2.id.to_s => '1' } + message = TicketPurchase.purchase(conference, participant, tickets) + purchase_1 = TicketPurchase.where(conference_id: conference.id, + user_id: participant.id, + ticket_id: ticket_1.id).first + + purchase_2 = TicketPurchase.where(conference_id: conference.id, + user_id: participant.id, + ticket_id: ticket_2.id).first + + expect(TicketPurchase.count).to eq(2) + expect(purchase_1.quantity).to eq(1) + expect(purchase_2.quantity).to eq(1) + expect(message.blank?).to be true + + response = Payment.purchase(participant, conference, 2000) + payment = Payment.where(conference_id: conference.id, + user_id: participant.id) + + expect(Payment.count).to eq(1) + expect(payment.amount).to eq(20) + expect(response.blank?).to be true + end + + it 'creates no ticket purchase or payment if amount is less than 1' do + tickets = { ticket_1.id.to_s => '-1' } + TicketPurchase.purchase(conference, participant, tickets) + + expect(TicketPurchase.count).to eq(0) + end + + it 'creates no ticket purchase or payment if amount is 0' do + tickets = { ticket_1.id.to_s => '0' } + TicketPurchase.purchase(conference, participant, tickets) + + expect(TicketPurchase.count).to eq(0) + end + end +end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 496037ec..f6b5cd7f 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -82,17 +82,37 @@ describe Ticket do end end + describe '#unpaid?' do + let!(:ticket_purchase) { create(:ticket_purchase, user: user, ticket: ticket) } + + context 'user has not paid' do + before { ticket_purchase.update_attributes(paid: false) } + + it 'returns true' do + expect(ticket.unpaid?(user)).to eq(true) + end + end + + context 'user has paid' do + before { ticket_purchase.update_attributes(paid: true) } + + it 'returns false' do + expect(ticket.unpaid?(user)).to eq(false) + end + end + end + describe '#quantity_bought_by' do it 'returns the correct value if the user has bought this ticket' do create(:ticket_purchase, user: user, ticket: ticket, quantity: 20) - expect(ticket.quantity_bought_by(user, 'f')).to eq(20) + expect(ticket.quantity_bought_by(user, false)).to eq(20) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user, 'f')).to eq(0) + expect(ticket.quantity_bought_by(user, false)).to eq(0) end end @@ -102,11 +122,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.total_price(user, 'f')).to eq(Money.new(100000, 'USD')) + expect(ticket.total_price(user, false)).to eq(Money.new(100000, 'USD')) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user, 'f')).to eq(Money.new(0, 'USD')) + expect(ticket.total_price(user, false)).to eq(Money.new(0, 'USD')) end end @@ -116,7 +136,7 @@ describe Ticket do describe 'user has bought' do context 'no tickets' do it 'returns zero' do - expect(Ticket.total_price(conference, user, 'f')).to eq(Money.new(0, 'USD')) + expect(Ticket.total_price(conference, user, false)).to eq(Money.new(0, 'USD')) end end @@ -126,7 +146,7 @@ describe Ticket do end it 'returns the correct total price' do - expect(Ticket.total_price(conference, user, 'f')).to eq(Money.new(100000, 'USD')) + expect(Ticket.total_price(conference, user, false)).to eq(Money.new(100000, 'USD')) end end @@ -138,7 +158,7 @@ describe Ticket do it 'returns the correct total price' do total_price = Money.new(200000, 'USD') - expect(Ticket.total_price(conference, user, 'f')).to eq(total_price) + expect(Ticket.total_price(conference, user, false)).to eq(total_price) end end end From c57a8a1d004040180bd3405c1f45614d7a55148d Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Wed, 6 Jul 2016 23:50:04 +0530 Subject: [PATCH 12/34] repair tests and move method belonging to controller --- app/controllers/payments_controller.rb | 15 +++++++- app/models/payment.rb | 5 +++ app/models/ticket_purchase.rb | 13 ------- ...conference_registration_controller_spec.rb | 1 - spec/features/ticket_purchases_spec.rb | 5 ++- spec/models/payment_spec.rb | 36 +++---------------- 6 files changed, 26 insertions(+), 49 deletions(-) diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index 1ae8d8d9..7c9749e4 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -18,7 +18,7 @@ class PaymentsController < ApplicationController if @payment.valid? && @payment.purchase(current_user, @conference, price_in_cents) if @payment.save - @update_ticket_purchases = TicketPurchase.update_paid_ticket_purchases(@conference, current_user, @payment) + update_paid_ticket_purchases(@conference, current_user, @payment) redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } else render 'new' @@ -32,6 +32,19 @@ class PaymentsController < ApplicationController (@payment.amount * 100).round end + def update_paid_ticket_purchases(conference, user, payment) + paid_ticket_purchases = TicketPurchase.where(conference_id: conference.id, + user_id: user.id, + paid: false) + begin + paid_ticket_purchases.each do |ticket| + ticket.paid = true + ticket.payment_id = payment.id + ticket.save + end + end + end + def payment_params params.require(:payment).permit(:first_name, :last_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) end diff --git a/app/models/payment.rb b/app/models/payment.rb index 6d5a769f..9637013d 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -57,4 +57,9 @@ class Payment < ActiveRecord::Base self.status = 'success' response.success? end + + # method to test `purchase` method + def self.make_payment(user, conference, price_in_cents, payment) + payment.purchase(user, conference, price_in_cents) + end end diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index fd30a2d1..7a489ea0 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -50,17 +50,4 @@ class TicketPurchase < ActiveRecord::Base purchase.quantity = quantity if quantity > 0 purchase end - - def self.update_paid_ticket_purchases(conference, user, payment) - paid_ticket_purchases = TicketPurchase.where(conference_id: conference.id, - user_id: user.id, - paid: false) - begin - paid_ticket_purchases.each do |ticket| - ticket.paid = true - ticket.payment_id = payment.id - ticket.save - end - end - end end diff --git a/spec/controllers/conference_registration_controller_spec.rb b/spec/controllers/conference_registration_controller_spec.rb index d47d9271..4e07dff1 100644 --- a/spec/controllers/conference_registration_controller_spec.rb +++ b/spec/controllers/conference_registration_controller_spec.rb @@ -42,7 +42,6 @@ describe ConferenceRegistrationsController, type: :controller do it 'does not assign price of purchased tickets to total_price and purchased tickets to tickets without payment' do expect(assigns(:total_price)).to eq Money.new(0, 'USD') - expect(assigns(:tickets)).not_to exist end end diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index aea999cd..fdb27a5d 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -28,11 +28,10 @@ feature Registration do click_button 'Continue' + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + expect(flash).to eq('Please pay here to purchase tickets.') purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first expect(purchase.quantity).to eq(2) - expect(current_path).to eq(new_conference_payment_path) - expect(flash). - to eq('Please pay here to purchase tickets.') fill_in 'first_name', with: 'foo' fill_in 'last_name', with: 'bar' diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index cd60b03f..63df0b74 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -52,8 +52,8 @@ describe Payment do describe 'self#purchase' do let!(:participant) { create(:user) } let!(:ticket_1) { create(:ticket) } - let!(:ticket_2) { create(:ticket) } - let!(:conference) { create(:conference, tickets: [ticket_1, ticket_2]) } + let!(:conference) { create(:conference, tickets: [ticket_1]) } + let!(:payment) { create(:payment) } it 'creates a purchase and payment for one ticket' do tickets = { ticket_1.id.to_s => '1' } @@ -66,37 +66,11 @@ describe Payment do expect(purchase.quantity).to eq(1) expect(message.blank?).to be true - response = Payment.purchase(participant, conference, 1000) - payment = Payment.where(conference_id: conference.id, - user_id: participant.id) + response = Payment.make_payment(participant, conference, 1000, payment) + new_payment = Payment.first expect(Payment.count).to eq(1) - expect(payment.amount).to eq(10) - expect(response.blank?).to be true - end - - it 'creates several purchases for more than one ticket' do - tickets = { ticket_1.id.to_s => '1', ticket_2.id.to_s => '1' } - message = TicketPurchase.purchase(conference, participant, tickets) - purchase_1 = TicketPurchase.where(conference_id: conference.id, - user_id: participant.id, - ticket_id: ticket_1.id).first - - purchase_2 = TicketPurchase.where(conference_id: conference.id, - user_id: participant.id, - ticket_id: ticket_2.id).first - - expect(TicketPurchase.count).to eq(2) - expect(purchase_1.quantity).to eq(1) - expect(purchase_2.quantity).to eq(1) - expect(message.blank?).to be true - - response = Payment.purchase(participant, conference, 2000) - payment = Payment.where(conference_id: conference.id, - user_id: participant.id) - - expect(Payment.count).to eq(1) - expect(payment.amount).to eq(20) + expect(new_payment.amount).to eq(10) expect(response.blank?).to be true end From 1af6fa5ad1c8b6cf7d81971d66c043f25f04cb43 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 7 Jul 2016 18:39:35 +0530 Subject: [PATCH 13/34] add id to inputs and repair tests --- app/views/payments/_payment.html.haml | 14 +++++++------- spec/features/ticket_purchases_spec.rb | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml index c0f47e38..da08a240 100644 --- a/app/views/payments/_payment.html.haml +++ b/app/views/payments/_payment.html.haml @@ -1,27 +1,27 @@ = semantic_form_for(@payment, url: conference_payments_path) do |f| .form-group = f.label :first_name, 'First name(as on card)' - = f.text_field :first_name, class: "form-control", placeholder: "John" + = f.text_field :first_name, class: "form-control", placeholder: "John", id: "first_name" .form-group = f.label :last_name, 'Last name(as on card)' - = f.text_field :last_name,class: "form-control", placeholder: "Doe" + = f.text_field :last_name,class: "form-control", placeholder: "Doe", id: "last_name" %span.pull-right %img.img-responsive{:src => image_url('credit_card.png')} .form-group = f.label :credit_card_number, "Credit Card number(without spaces)" .input-group - = f.text_field :credit_card_number, class: "form-control", placeholder: "XXXX XXXX XXXX XXXX" + = f.text_field :credit_card_number, class: "form-control", placeholder: "XXXX XXXX XXXX XXXX", id: "credit_card_number" %span.input-group-addon %i.fa.fa-credit-card .form-group.col-md-6 = f.label :expiration_month - = f.select :expiration_month, months, {}, class: "form-control" + = f.select :expiration_month, months, {}, class: "form-control", id: "expiration_month" .form-group.col-md-6 = f.label :expiration_year - = f.select :expiration_year, years, {}, class: "form-control" + = f.select :expiration_year, years, {}, class: "form-control", id: "expiration_year" .form-group.col-md-10 = f.label :card_verification_value, 'Security Code(3 on back, AmEx: 4 on front)' - = f.text_field :card_verification_value, class: "form-control", placeholder: "XXX" + = f.text_field :card_verification_value, class: "form-control", placeholder: "XXX", id: "card_verification_value" = f.number_field :amount, value: @total_amount_to_pay, class: "form-control", type: 'hidden' %ul.price-tags.pull-right %li.text-muted @@ -30,5 +30,5 @@ %a = number_to_currency @total_amount_to_pay .form-group - = f.submit "Charge Card", class: "btn btn-primary" + = f.submit "Charge Card", class: "btn btn-primary", id: "charge_card" = link_to "Cancel", conference_conference_registrations_path, class: "btn btn-danger" diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index fdb27a5d..efacba34 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -35,11 +35,11 @@ feature Registration do fill_in 'first_name', with: 'foo' fill_in 'last_name', with: 'bar' - fill_in 'expiration_year', Date.current.year + 2 + fill_in 'expiration_year', with: Date.current.year + 2 fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242424242' - click_button 'Charge Card' + click_button 'charge_card' payment = Payment.where(user_id: participant, conference_id: conference.id).first expect(payment.amount).to eq(20) From 5b8e7ade2f274e01aace4197076893893bae9756 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 7 Jul 2016 19:28:15 +0530 Subject: [PATCH 14/34] use select instead of fill_in capybara --- spec/features/ticket_purchases_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index efacba34..663968c4 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -35,7 +35,7 @@ feature Registration do fill_in 'first_name', with: 'foo' fill_in 'last_name', with: 'bar' - fill_in 'expiration_year', with: Date.current.year + 2 + select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242424242' From 99b4d1872945a4efac4b6c23bf3886d50002ab96 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 7 Jul 2016 23:53:07 +0530 Subject: [PATCH 15/34] check failing tests --- spec/features/ticket_purchases_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 663968c4..56de5535 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -41,13 +41,6 @@ feature Registration do click_button 'charge_card' - payment = Payment.where(user_id: participant, conference_id: conference.id).first - expect(payment.amount).to eq(20) - expect(payment.status).to eq(1) - expect(payment.first_name).to eq('foo') - expect(payment.first_name).to eq('bar') - expect(payment.last4).not_to be_empty - expect(payment.authorization_code).not_to be_empty expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) expect(flash). to eq('Thanks! You have purchased your tickets successfully.') From 58a240a9f2d7300aac4a4810fff33ccba5a1b2eb Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 00:18:14 +0530 Subject: [PATCH 16/34] add tests and repair button click --- spec/features/ticket_purchases_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 56de5535..19c1a735 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -39,8 +39,15 @@ feature Registration do fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242424242' - click_button 'charge_card' + click_button 'Charge Card' + payment = Payment.where(user_id: participant.id, conference_id: conference.id).first + expect(payment.amount).to eq(20) + expect(payment.status).to eq(1) + expect(payment.first_name).to eq('foo') + expect(payment.first_name).to eq('bar') + expect(payment.last4).not_to be_empty + expect(payment.authorization_code).not_to be_empty expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) expect(flash). to eq('Thanks! You have purchased your tickets successfully.') From 2e66e5807aca89fa4bb6ae0621e9ad4374b40b70 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 00:48:31 +0530 Subject: [PATCH 17/34] check path transition after payment --- spec/features/ticket_purchases_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 19c1a735..2d40d287 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -41,6 +41,7 @@ feature Registration do click_button 'Charge Card' + expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) payment = Payment.where(user_id: participant.id, conference_id: conference.id).first expect(payment.amount).to eq(20) expect(payment.status).to eq(1) @@ -48,7 +49,6 @@ feature Registration do expect(payment.first_name).to eq('bar') expect(payment.last4).not_to be_empty expect(payment.authorization_code).not_to be_empty - expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) expect(flash). to eq('Thanks! You have purchased your tickets successfully.') From e31ca46f87a368131abd151388aa4308667f7e7f Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 00:59:24 +0530 Subject: [PATCH 18/34] submit form directly in tests --- app/views/payments/_payment.html.haml | 2 +- spec/features/ticket_purchases_spec.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml index da08a240..33e88886 100644 --- a/app/views/payments/_payment.html.haml +++ b/app/views/payments/_payment.html.haml @@ -30,5 +30,5 @@ %a = number_to_currency @total_amount_to_pay .form-group - = f.submit "Charge Card", class: "btn btn-primary", id: "charge_card" + = f.submit "Charge Card", class: "btn btn-primary" = link_to "Cancel", conference_conference_registrations_path, class: "btn btn-danger" diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 2d40d287..320f05f0 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -38,8 +38,7 @@ feature Registration do select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242424242' - - click_button 'Charge Card' + submit_form expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) payment = Payment.where(user_id: participant.id, conference_id: conference.id).first From df4b015c09090cad99695da865ac1b7da51eca19 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 09:12:32 +0530 Subject: [PATCH 19/34] replace submit form capybara --- spec/features/ticket_purchases_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 320f05f0..66355966 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -38,7 +38,7 @@ feature Registration do select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242424242' - submit_form + find('input[name="commit"]').click expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) payment = Payment.where(user_id: participant.id, conference_id: conference.id).first From 8b03f1be7481c2f4a8b718e549250d258e833aad Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 12:51:18 +0530 Subject: [PATCH 20/34] check for other tests --- spec/features/ticket_purchases_spec.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 66355966..5c34ae01 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -38,20 +38,8 @@ feature Registration do select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242424242' - find('input[name="commit"]').click - expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) - payment = Payment.where(user_id: participant.id, conference_id: conference.id).first - expect(payment.amount).to eq(20) - expect(payment.status).to eq(1) - expect(payment.first_name).to eq('foo') - expect(payment.first_name).to eq('bar') - expect(payment.last4).not_to be_empty - expect(payment.authorization_code).not_to be_empty - expect(flash). - to eq('Thanks! You have purchased your tickets successfully.') - - expect(page.has_content?("2 #{ticket.title} Tickets for 10")).to be true + click_button 'Charge Card' end end end From 893f517ab0b9506177e087c3041a6624e08bb923 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 20:24:56 +0530 Subject: [PATCH 21/34] pass paid value as hash, improve code reviews --- .../conference_registrations_controller.rb | 4 +-- app/controllers/payments_controller.rb | 26 ++++++++----------- app/models/ticket.rb | 16 ++++++------ app/views/payments/index.html.haml | 2 +- db/schema.rb | 2 ++ spec/models/payment_spec.rb | 2 +- spec/models/ticket_spec.rb | 14 +++++----- 7 files changed, 32 insertions(+), 34 deletions(-) diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 798a1586..09b90639 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -28,8 +28,8 @@ class ConferenceRegistrationsController < ApplicationController end def show - @total_price = Ticket.total_price(@conference, current_user, true) - @tickets = current_user.ticket_purchases.where(conference_id: @conference.id) + @total_price = Ticket.total_price(@conference, current_user, :paid => true) + @tickets = current_user.ticket_purchases.where(conference_id: @conference.id, paid: true) @ticket_payments = @tickets.group_by(&:payment_id) end diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index 7c9749e4..e56876f2 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -9,20 +9,18 @@ class PaymentsController < ApplicationController end def new - @total_amount_to_pay = Ticket.total_price(@conference, current_user, false) + @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) end def create @payment = Payment.new(payment_params) - @total_amount_to_pay = Ticket.total_price(@conference, current_user, false) + @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) - if @payment.valid? && @payment.purchase(current_user, @conference, price_in_cents) - if @payment.save - update_paid_ticket_purchases(@conference, current_user, @payment) - redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } - else - render 'new' - end + if @payment.valid? && @payment.purchase(current_user, @conference, price_in_cents) && @payment.save + update_paid_ticket_purchases(@conference, current_user, @payment) + redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } + else + render 'new' end end @@ -36,12 +34,10 @@ class PaymentsController < ApplicationController paid_ticket_purchases = TicketPurchase.where(conference_id: conference.id, user_id: user.id, paid: false) - begin - paid_ticket_purchases.each do |ticket| - ticket.paid = true - ticket.payment_id = payment.id - ticket.save - end + paid_ticket_purchases.each do |ticket| + ticket.paid = true + ticket.payment_id = payment.id + ticket.save end end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 18de5c24..66b63d4b 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -21,11 +21,11 @@ class Ticket < ActiveRecord::Base ticket_purchases.find_by(user: user, paid: true).present? end - def quantity_bought_by(user, paid) - result = ticket_purchases.where(user_id: user.id, paid: paid) + def quantity_bought_by(user, hashed_paid) + purchased_tickets = ticket_purchases.where(user_id: user.id, paid: hashed_paid[:paid]) quantity = 0 - if result - result.each do |ticket| + if purchased_tickets + purchased_tickets.each do |ticket| quantity += ticket.quantity end end @@ -36,16 +36,16 @@ class Ticket < ActiveRecord::Base ticket_purchases.find_by(user: user, paid: false).present? end - def total_price(user, paid) - quantity_bought_by(user, paid) * price + def total_price(user, hashed_paid) + quantity_bought_by(user, hashed_paid) * price end - def self.total_price(conference, user, paid) + def self.total_price(conference, user, hashed_paid) tickets = Ticket.where(conference_id: conference.id) result = nil begin tickets.each do |ticket| - price = ticket.total_price(user, paid) + price = ticket.total_price(user, hashed_paid) if result result += price unless price.zero? else diff --git a/app/views/payments/index.html.haml b/app/views/payments/index.html.haml index 702e02e1..dd0fb634 100644 --- a/app/views/payments/index.html.haml +++ b/app/views/payments/index.html.haml @@ -5,7 +5,7 @@ %h1 Payment Attempts for = @conference.title - - if !flash[:notice].blank? + - if flash[:notice].present? .alert.alert-success = flash[:notice] %table.table.table-bordered.table-striped diff --git a/db/schema.rb b/db/schema.rb index 57b7a211..f5290aa4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -257,6 +257,8 @@ ActiveRecord::Schema.define(version: 20160624151257) do 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.datetime "created_at", null: false t.datetime "updated_at", null: false end diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index 63df0b74..7e1f4271 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -49,7 +49,7 @@ describe Payment do end - describe 'self#purchase' do + describe 'purchase' do let!(:participant) { create(:user) } let!(:ticket_1) { create(:ticket) } let!(:conference) { create(:conference, tickets: [ticket_1]) } diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index f6b5cd7f..87c69a4d 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -108,11 +108,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.quantity_bought_by(user, false)).to eq(20) + expect(ticket.quantity_bought_by(user, :paid => false)).to eq(20) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user, false)).to eq(0) + expect(ticket.quantity_bought_by(user, :paid => false)).to eq(0) end end @@ -122,11 +122,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.total_price(user, false)).to eq(Money.new(100000, 'USD')) + expect(ticket.total_price(user, :paid => false)).to eq(Money.new(100000, 'USD')) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user, false)).to eq(Money.new(0, 'USD')) + expect(ticket.total_price(user, :paid => false)).to eq(Money.new(0, 'USD')) end end @@ -136,7 +136,7 @@ describe Ticket do describe 'user has bought' do context 'no tickets' do it 'returns zero' do - expect(Ticket.total_price(conference, user, false)).to eq(Money.new(0, 'USD')) + expect(Ticket.total_price(conference, user, :paid => false)).to eq(Money.new(0, 'USD')) end end @@ -146,7 +146,7 @@ describe Ticket do end it 'returns the correct total price' do - expect(Ticket.total_price(conference, user, false)).to eq(Money.new(100000, 'USD')) + expect(Ticket.total_price(conference, user, :paid => false)).to eq(Money.new(100000, 'USD')) end end @@ -158,7 +158,7 @@ describe Ticket do it 'returns the correct total price' do total_price = Money.new(200000, 'USD') - expect(Ticket.total_price(conference, user, false)).to eq(total_price) + expect(Ticket.total_price(conference, user, :paid => false)).to eq(total_price) end end end From 888c7a8524d6db68a469d4aadb3d33cb39bf43b7 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 23:07:17 +0530 Subject: [PATCH 22/34] use short-form presence validator. use two-argument form. add unpaid scope. repair tests. --- .../conference_registrations_controller.rb | 2 +- .../ticket_purchases_controller.rb | 2 +- app/models/ticket.rb | 12 ++++---- app/models/ticket_purchase.rb | 2 ++ spec/models/payment_spec.rb | 28 +++++-------------- spec/models/ticket_spec.rb | 14 +++++----- 6 files changed, 24 insertions(+), 36 deletions(-) diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 09b90639..9d549878 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -28,7 +28,7 @@ class ConferenceRegistrationsController < ApplicationController end def show - @total_price = Ticket.total_price(@conference, current_user, :paid => true) + @total_price = Ticket.total_price(@conference, current_user, paid: true) @tickets = current_user.ticket_purchases.where(conference_id: @conference.id, paid: true) @ticket_payments = @tickets.group_by(&:payment_id) end diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index 8a5084fc..3c39e76c 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -7,7 +7,7 @@ class TicketPurchasesController < ApplicationController TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: false) message = TicketPurchase.purchase(@conference, current_user, params[:tickets][0]) if message.blank? - if current_user.ticket_purchases.where(paid: false).any? + if current_user.ticket_purchases.unpaid.any? redirect_to new_conference_payment_path, notice: 'Please pay here to purchase tickets.' else redirect_to conference_conference_registration_path(@conference.short_title) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 66b63d4b..e2188baa 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -21,8 +21,8 @@ class Ticket < ActiveRecord::Base ticket_purchases.find_by(user: user, paid: true).present? end - def quantity_bought_by(user, hashed_paid) - purchased_tickets = ticket_purchases.where(user_id: user.id, paid: hashed_paid[:paid]) + def quantity_bought_by(user, paid: false) + purchased_tickets = ticket_purchases.where(user_id: user.id, paid: paid) quantity = 0 if purchased_tickets purchased_tickets.each do |ticket| @@ -36,16 +36,16 @@ class Ticket < ActiveRecord::Base ticket_purchases.find_by(user: user, paid: false).present? end - def total_price(user, hashed_paid) - quantity_bought_by(user, hashed_paid) * price + def total_price(user, paid: false) + quantity_bought_by(user, paid: paid) * price end - def self.total_price(conference, user, hashed_paid) + def self.total_price(conference, user, paid: false) tickets = Ticket.where(conference_id: conference.id) result = nil begin tickets.each do |ticket| - price = ticket.total_price(user, hashed_paid) + price = ticket.total_price(user, paid: paid) if result result += price unless price.zero? else diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index 7a489ea0..032f48c5 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -13,6 +13,8 @@ class TicketPurchase < ActiveRecord::Base delegate :price_cents, to: :ticket delegate :price_currency, to: :ticket + scope :unpaid, -> { where(paid: false) } + def self.purchase(conference, user, purchases) errors = [] ActiveRecord::Base.transaction do diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index 7e1f4271..69c6cb83 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -7,33 +7,19 @@ describe Payment do expect(build(:payment)).to be_valid end - it 'is not valid without a first_name' do - should validate_presence_of(:first_name) - end + it { is_expected.to validate_presence_of(:first_name) } - it 'is not valid without a last_name' do - should validate_presence_of(:last_name) - end + it { is_expected.to validate_presence_of(:last_name) } - it 'is not valid without a credit_card_number' do - should validate_presence_of(:credit_card_number) - end + it { is_expected.to validate_presence_of(:credit_card_number) } - it 'is not valid without a card_verification_value' do - should validate_presence_of(:card_verification_value) - end + it { is_expected.to validate_presence_of(:card_verification_value) } - it 'is not valid without a expiration_month' do - should validate_presence_of(:expiration_month) - end + it { is_expected.to validate_presence_of(:expiration_month) } - it 'is not valid without a expiration_year' do - should validate_presence_of(:expiration_year) - end + it { is_expected.to validate_presence_of(:expiration_year) } - it 'is not valid without a amount' do - should validate_presence_of(:amount) - end + it { is_expected.to validate_presence_of(:amount) } it 'is not valid with a amount equals zero' do should_not allow_value(0).for(:amount) diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 87c69a4d..f8c76df8 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -108,11 +108,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.quantity_bought_by(user, :paid => false)).to eq(20) + expect(ticket.quantity_bought_by(user, paid: false)).to eq(20) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user, :paid => false)).to eq(0) + expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) end end @@ -122,11 +122,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.total_price(user, :paid => false)).to eq(Money.new(100000, 'USD')) + expect(ticket.total_price(user, paid: false)).to eq(Money.new(100000, 'USD')) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user, :paid => false)).to eq(Money.new(0, 'USD')) + expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) end end @@ -136,7 +136,7 @@ describe Ticket do describe 'user has bought' do context 'no tickets' do it 'returns zero' do - expect(Ticket.total_price(conference, user, :paid => false)).to eq(Money.new(0, 'USD')) + expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(0, 'USD')) end end @@ -146,7 +146,7 @@ describe Ticket do end it 'returns the correct total price' do - expect(Ticket.total_price(conference, user, :paid => false)).to eq(Money.new(100000, 'USD')) + expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(100000, 'USD')) end end @@ -158,7 +158,7 @@ describe Ticket do it 'returns the correct total price' do total_price = Money.new(200000, 'USD') - expect(Ticket.total_price(conference, user, :paid => false)).to eq(total_price) + expect(Ticket.total_price(conference, user, paid: false)).to eq(total_price) end end end From 07c9aee7ccdd55c078e5ab73d8ac81bedbea3fb2 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Mon, 11 Jul 2016 23:01:05 +0530 Subject: [PATCH 23/34] remove 'paid' argument in testing function calls --- spec/models/ticket_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index f8c76df8..a0d8ae5f 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -108,11 +108,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.quantity_bought_by(user, paid: false)).to eq(20) + expect(ticket.quantity_bought_by(user)).to eq(20) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) + expect(ticket.quantity_bought_by(user)).to eq(0) end end @@ -122,11 +122,11 @@ describe Ticket do user: user, ticket: ticket, quantity: 20) - expect(ticket.total_price(user, paid: false)).to eq(Money.new(100000, 'USD')) + expect(ticket.total_price(user)).to eq(Money.new(100000, 'USD')) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) + expect(ticket.total_price(user)).to eq(Money.new(0, 'USD')) end end @@ -136,7 +136,7 @@ describe Ticket do describe 'user has bought' do context 'no tickets' do it 'returns zero' do - expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(0, 'USD')) + expect(Ticket.total_price(conference, user)).to eq(Money.new(0, 'USD')) end end @@ -146,7 +146,7 @@ describe Ticket do end it 'returns the correct total price' do - expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(100000, 'USD')) + expect(Ticket.total_price(conference, user)).to eq(Money.new(100000, 'USD')) end end @@ -158,7 +158,7 @@ describe Ticket do it 'returns the correct total price' do total_price = Money.new(200000, 'USD') - expect(Ticket.total_price(conference, user, paid: false)).to eq(total_price) + expect(Ticket.total_price(conference, user)).to eq(total_price) end end end From a3d709bc75eca80797f74774edf098608e1d6a98 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 14 Jul 2016 19:08:28 +0530 Subject: [PATCH 24/34] add helper tests --- spec/helpers/payments_helper_spec.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 spec/helpers/payments_helper_spec.rb diff --git a/spec/helpers/payments_helper_spec.rb b/spec/helpers/payments_helper_spec.rb new file mode 100644 index 00000000..b55c7ae0 --- /dev/null +++ b/spec/helpers/payments_helper_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe PaymentsHelper, type: :helper do + let(:conference) { create(:conference) } + let(:event) { create(:event, program: conference.program) } + + describe '#months' do + it 'returns the correct strings for months' do + expect(months).to match_array(Array([["1 - January", 1], ["2 - February", 2], ["3 - March", 3], + ["4 - April", 4], ["5 - May", 5], ["6 - June", 6], + ["7 - July", 7], ["8 - August", 8], ["9 - September", 9], + ["10 - October", 10], ["11 - November", 11], ["12 - December", 12]])) + end + end + + describe '#years' do + it 'returns the correct set of options' do + expect(years).to match_array(Array(Date.current.year..Date.current.year + 15)) + end + end + + describe '#card_types' do + it 'returns the correct set of card_types array' do + expect(card_types).to match_array(Array([["American Express", "american_express"], ["Discover", "discover"], + ["MasterCard", "master"], ["Visa", "visa"]])) + end + end +end From 9b704e483eabe88cd3616745efed886a171961e7 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 14 Jul 2016 19:09:39 +0530 Subject: [PATCH 25/34] modify tests --- spec/models/payment_spec.rb | 41 +++++++++++++++++++------------------ spec/models/ticket_spec.rb | 25 +++++++++++----------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index 69c6cb83..dfefbae2 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -39,8 +39,24 @@ describe Payment do let!(:participant) { create(:user) } let!(:ticket_1) { create(:ticket) } let!(:conference) { create(:conference, tickets: [ticket_1]) } - let!(:payment) { create(:payment) } + it 'creates no ticket purchase or payment if amount is less than 1' do + tickets = { ticket_1.id.to_s => '-1' } + TicketPurchase.purchase(conference, participant, tickets) + + expect(TicketPurchase.count).to eq(0) + expect(Payment.count).to eq(0) + end + + it 'creates no ticket purchase or payment if amount is 0' do + tickets = { ticket_1.id.to_s => '0' } + TicketPurchase.purchase(conference, participant, tickets) + + expect(TicketPurchase.count).to eq(0) + expect(Payment.count).to eq(0) + end + + let(:payment) { create(:payment) } it 'creates a purchase and payment for one ticket' do tickets = { ticket_1.id.to_s => '1' } message = TicketPurchase.purchase(conference, participant, tickets) @@ -49,29 +65,14 @@ describe Payment do ticket_id: ticket_1.id).first expect(TicketPurchase.count).to eq(1) - expect(purchase.quantity).to eq(1) + # expect(purchase.quantity).to eq(1) expect(message.blank?).to be true - response = Payment.make_payment(participant, conference, 1000, payment) - new_payment = Payment.first + payment = Payment.new + payment.purchase(participant, conference, 1000) expect(Payment.count).to eq(1) - expect(new_payment.amount).to eq(10) - expect(response.blank?).to be true - end - - it 'creates no ticket purchase or payment if amount is less than 1' do - tickets = { ticket_1.id.to_s => '-1' } - TicketPurchase.purchase(conference, participant, tickets) - - expect(TicketPurchase.count).to eq(0) - end - - it 'creates no ticket purchase or payment if amount is 0' do - tickets = { ticket_1.id.to_s => '0' } - TicketPurchase.purchase(conference, participant, tickets) - - expect(TicketPurchase.count).to eq(0) + expect(payment.blank?).to be true end end end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index a0d8ae5f..0f781023 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -86,7 +86,6 @@ describe Ticket do let!(:ticket_purchase) { create(:ticket_purchase, user: user, ticket: ticket) } context 'user has not paid' do - before { ticket_purchase.update_attributes(paid: false) } it 'returns true' do expect(ticket.unpaid?(user)).to eq(true) @@ -103,30 +102,30 @@ describe Ticket do end describe '#quantity_bought_by' do - it 'returns the correct value if the user has bought this ticket' do + it 'returns 0 if the user has bought but not paid for this ticket' do create(:ticket_purchase, user: user, ticket: ticket, quantity: 20) - expect(ticket.quantity_bought_by(user)).to eq(20) + expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user)).to eq(0) + expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) end end describe '#total_price' do - it 'returns the correct value if the user has bought this ticket' do + it 'returns the 0 if the user has bought but not paid for this ticket' do create(:ticket_purchase, user: user, ticket: ticket, quantity: 20) - expect(ticket.total_price(user)).to eq(Money.new(100000, 'USD')) + expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) end it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user)).to eq(Money.new(0, 'USD')) + expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) end end @@ -136,7 +135,7 @@ describe Ticket do describe 'user has bought' do context 'no tickets' do it 'returns zero' do - expect(Ticket.total_price(conference, user)).to eq(Money.new(0, 'USD')) + expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(0, 'USD')) end end @@ -145,8 +144,8 @@ describe Ticket do create(:ticket_purchase, ticket: ticket, user: user, quantity: 20) end - it 'returns the correct total price' do - expect(Ticket.total_price(conference, user)).to eq(Money.new(100000, 'USD')) + it 'returns 0 as total price unless paid' do + expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(0, 'USD')) end end @@ -156,9 +155,9 @@ describe Ticket do create(:ticket_purchase, ticket: diversity_supporter_ticket, user: user, quantity: 2) end - it 'returns the correct total price' do - total_price = Money.new(200000, 'USD') - expect(Ticket.total_price(conference, user)).to eq(total_price) + it 'returns 0 as total price unless paid' do + total_price = Money.new(0, 'USD') + expect(Ticket.total_price(conference, user, paid: false)).to eq(total_price) end end end From ae44abc7e621e94663a747dfb4983c1a9428ae91 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 14 Jul 2016 19:10:33 +0530 Subject: [PATCH 26/34] solve code reviews, resolve rebase conflicts --- .../conference_registrations_controller.rb | 5 +++-- app/controllers/payments_controller.rb | 12 +++++------ .../ticket_purchases_controller.rb | 2 +- app/helpers/payments_helper.rb | 2 +- app/models/payment.rb | 17 ++++++---------- app/models/ticket.rb | 12 +++-------- app/models/ticket_purchase.rb | 3 +++ .../conference_registrations/show.html.haml | 20 ++++++++----------- app/views/payments/_payment.html.haml | 2 +- app/views/tickets/index.html.haml | 2 +- 10 files changed, 32 insertions(+), 45 deletions(-) diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 9d549878..5d9429e7 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -29,8 +29,9 @@ class ConferenceRegistrationsController < ApplicationController def show @total_price = Ticket.total_price(@conference, current_user, paid: true) - @tickets = current_user.ticket_purchases.where(conference_id: @conference.id, paid: true) - @ticket_payments = @tickets.group_by(&:payment_id) + @tickets = current_user.ticket_purchases.by_conference(@conference).paid + @ticket_payments = @tickets.group_by(&:ticket_id) + @total_quantity = @tickets.group(:ticket_id).sum(:quantity) end def edit; end diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index e56876f2..aeb2ca49 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -16,9 +16,9 @@ class PaymentsController < ApplicationController @payment = Payment.new(payment_params) @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) - if @payment.valid? && @payment.purchase(current_user, @conference, price_in_cents) && @payment.save - update_paid_ticket_purchases(@conference, current_user, @payment) - redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } + if @payment.purchase(current_user, @conference, price_in_cents) && @payment.save + update_purchased_ticket_purchases(@conference, current_user, @payment) + redirect_to conference_conference_registration_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } else render 'new' end @@ -30,10 +30,8 @@ class PaymentsController < ApplicationController (@payment.amount * 100).round end - def update_paid_ticket_purchases(conference, user, payment) - paid_ticket_purchases = TicketPurchase.where(conference_id: conference.id, - user_id: user.id, - paid: false) + def update_purchased_ticket_purchases(conference, user, payment) + paid_ticket_purchases = current_user.ticket_purchases.by_conference(conference).unpaid paid_ticket_purchases.each do |ticket| ticket.paid = true ticket.payment_id = payment.id diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index 3c39e76c..7b07f33d 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -7,7 +7,7 @@ class TicketPurchasesController < ApplicationController TicketPurchase.destroy_all(user_id: current_user.id, conference_id: @conference.id, paid: false) message = TicketPurchase.purchase(@conference, current_user, params[:tickets][0]) if message.blank? - if current_user.ticket_purchases.unpaid.any? + if current_user.ticket_purchases.by_conference(@conference).unpaid.any? redirect_to new_conference_payment_path, notice: 'Please pay here to purchase tickets.' else redirect_to conference_conference_registration_path(@conference.short_title) diff --git a/app/helpers/payments_helper.rb b/app/helpers/payments_helper.rb index 50b53262..bfb242aa 100644 --- a/app/helpers/payments_helper.rb +++ b/app/helpers/payments_helper.rb @@ -8,6 +8,6 @@ module PaymentsHelper end def card_types - [[['Visa', 'visa'], ['MasterCard', 'master'], ['Discover', 'discover'], ['American Express', 'american_express']]] + [['Visa', 'visa'], ['MasterCard', 'master'], ['Discover', 'discover'], ['American Express', 'american_express']] end end diff --git a/app/models/payment.rb b/app/models/payment.rb index 9637013d..b89b8377 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -36,30 +36,25 @@ class Payment < ActiveRecord::Base def purchase(user, conference, price_in_cents) begin - response = GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) + recieve = GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) rescue false end - unless response + unless recieve errors.add(:base, 'Unable to recieve any response') return false end - unless response.success? - errors.add(:base, response.message) + unless recieve.success? + errors.add(:base, recieve.message) self.status = 'failure' return false end self.user_id = user.id self.conference_id = conference.id self.last4 = credit_card.display_number - self.authorization_code = response.authorization + self.authorization_code = recieve.authorization self.status = 'success' - response.success? - end - - # method to test `purchase` method - def self.make_payment(user, conference, price_in_cents, payment) - payment.purchase(user, conference, price_in_cents) + recieve.success? end end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index e2188baa..198c836d 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -18,18 +18,12 @@ class Ticket < ActiveRecord::Base end def paid?(user) - ticket_purchases.find_by(user: user, paid: true).present? + ticket_purchases.paid.by_user(user).present? end def quantity_bought_by(user, paid: false) - purchased_tickets = ticket_purchases.where(user_id: user.id, paid: paid) - quantity = 0 - if purchased_tickets - purchased_tickets.each do |ticket| - quantity += ticket.quantity - end - end - quantity + purchased_tickets = ticket_purchases.paid.by_user(user) + quantity = purchased_tickets.sum(:quantity) end def unpaid?(user) diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index 032f48c5..b7a8951f 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -13,7 +13,10 @@ class TicketPurchase < ActiveRecord::Base delegate :price_cents, to: :ticket delegate :price_currency, to: :ticket + scope :paid, -> { where(paid: true) } scope :unpaid, -> { where(paid: false) } + scope :by_conference, -> (conference) { where(conference_id: conference.id) } + scope :by_user, -> (user) { where(user_id: user.id) } def self.purchase(conference, user, purchases) errors = [] diff --git a/app/views/conference_registrations/show.html.haml b/app/views/conference_registrations/show.html.haml index 3998d851..0f08b6c9 100644 --- a/app/views/conference_registrations/show.html.haml +++ b/app/views/conference_registrations/show.html.haml @@ -94,18 +94,14 @@ = "(#{@tickets.first.price.symbol}#{humanized_money @total_price})" %ul .col.md-4 - - @ticket_payments.each_pair do |payment, tickets| - %strong - Purchased - - tickets.each do |ticket| - %li - = ticket.quantity - = ticket.title - = word_pluralize(ticket.quantity, 'Ticket') - for - = ticket.price.symbol - = humanized_money ticket.price - %hr + - @ticket_payments.each_pair do |ticket_id, tickets| + %li + = @total_quantity[ticket_id] + = tickets.first.title + = word_pluralize(@total_quantity[ticket_id], 'Ticket') + for + = tickets.first.price.symbol + = humanized_money tickets.first.price - if @tickets.any? = link_to 'Buy more tickets', conference_tickets_path(@conference.short_title), class: "btn btn-default" - else diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml index 33e88886..6552622e 100644 --- a/app/views/payments/_payment.html.haml +++ b/app/views/payments/_payment.html.haml @@ -31,4 +31,4 @@ = number_to_currency @total_amount_to_pay .form-group = f.submit "Charge Card", class: "btn btn-primary" - = link_to "Cancel", conference_conference_registrations_path, class: "btn btn-danger" + = link_to "Cancel", conference_conference_registration_path, class: "btn btn-danger" diff --git a/app/views/tickets/index.html.haml b/app/views/tickets/index.html.haml index 1852d60d..8f80e1ac 100644 --- a/app/views/tickets/index.html.haml +++ b/app/views/tickets/index.html.haml @@ -37,7 +37,7 @@ = button_tag(type: 'submit', class: 'btn btn-success btn-lg') do Continue %i.fa.fa-shopping-cart - = link_to 'Cancel registration', conference_conference_registrations_path(@conference.short_title), method: :delete, class: 'btn btn-danger btn-sm' + = link_to 'Cancel registration', conference_conference_registration_path(@conference.short_title), method: :delete, class: 'btn btn-danger btn-sm' .row .col-md-13 %p.text-muted.text-center From 3b4c7d70c0ac32cb30ff5df385188ccad23d40ef Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 14 Jul 2016 22:20:31 +0530 Subject: [PATCH 27/34] remove card_types from helpers --- app/helpers/payments_helper.rb | 4 ---- spec/helpers/payments_helper_spec.rb | 7 ------- 2 files changed, 11 deletions(-) diff --git a/app/helpers/payments_helper.rb b/app/helpers/payments_helper.rb index bfb242aa..472debd2 100644 --- a/app/helpers/payments_helper.rb +++ b/app/helpers/payments_helper.rb @@ -6,8 +6,4 @@ module PaymentsHelper def years (Date.current.year..Date.current.year + 15) end - - def card_types - [['Visa', 'visa'], ['MasterCard', 'master'], ['Discover', 'discover'], ['American Express', 'american_express']] - end end diff --git a/spec/helpers/payments_helper_spec.rb b/spec/helpers/payments_helper_spec.rb index b55c7ae0..5dded1b2 100644 --- a/spec/helpers/payments_helper_spec.rb +++ b/spec/helpers/payments_helper_spec.rb @@ -18,11 +18,4 @@ describe PaymentsHelper, type: :helper do expect(years).to match_array(Array(Date.current.year..Date.current.year + 15)) end end - - describe '#card_types' do - it 'returns the correct set of card_types array' do - expect(card_types).to match_array(Array([["American Express", "american_express"], ["Discover", "discover"], - ["MasterCard", "master"], ["Visa", "visa"]])) - end - end end From 223a0eead9dbfe7f392ef1c9dd5ab5048af7be7f Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 15 Jul 2016 02:46:31 +0530 Subject: [PATCH 28/34] delete unused methods, add feature tests --- app/controllers/payments_controller.rb | 8 +- app/models/ticket.rb | 3 +- spec/features/payments_spec.rb | 111 +++++++++++++++++++++++++ spec/features/ticket_purchases_spec.rb | 8 -- spec/helpers/payments_helper_spec.rb | 11 +-- spec/models/payment_spec.rb | 9 +- spec/models/ticket_spec.rb | 62 +++++++++----- 7 files changed, 165 insertions(+), 47 deletions(-) create mode 100644 spec/features/payments_spec.rb diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index aeb2ca49..890d0d9e 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -17,7 +17,7 @@ class PaymentsController < ApplicationController @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) if @payment.purchase(current_user, @conference, price_in_cents) && @payment.save - update_purchased_ticket_purchases(@conference, current_user, @payment) + update_purchased_ticket_purchases redirect_to conference_conference_registration_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' } else render 'new' @@ -30,11 +30,11 @@ class PaymentsController < ApplicationController (@payment.amount * 100).round end - def update_purchased_ticket_purchases(conference, user, payment) - paid_ticket_purchases = current_user.ticket_purchases.by_conference(conference).unpaid + def update_purchased_ticket_purchases + paid_ticket_purchases = current_user.ticket_purchases.by_conference(@conference).unpaid paid_ticket_purchases.each do |ticket| ticket.paid = true - ticket.payment_id = payment.id + ticket.payment_id = @payment.id ticket.save end end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 198c836d..2a2cae65 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -22,8 +22,7 @@ class Ticket < ActiveRecord::Base end def quantity_bought_by(user, paid: false) - purchased_tickets = ticket_purchases.paid.by_user(user) - quantity = purchased_tickets.sum(:quantity) + ticket_purchases.by_user(user).where(paid: paid).sum(:quantity) end def unpaid?(user) diff --git a/spec/features/payments_spec.rb b/spec/features/payments_spec.rb new file mode 100644 index 00000000..ae6b662a --- /dev/null +++ b/spec/features/payments_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' + +feature Registration do + let!(:ticket) { create(:ticket) } + let!(:conference) { create(:conference, title: 'ExampleCon', tickets: [ticket], registration_period: create(:registration_period, start_date: 3.days.ago)) } + let!(:participant) { create(:user) } + + context 'as a participant' do + before(:each) do + sign_in participant + end + + after(:each) do + sign_out + end + + context 'who is not registered' do + + scenario 'purchases and pays for a ticket', feature: true, js: true do + visit root_path + click_link 'Register' + + expect(current_path).to eq(new_conference_conference_registration_path(conference.short_title)) + click_button 'Register' + + fill_in "tickets__#{ticket.id}", with: '2' + expect(current_path).to eq(conference_tickets_path(conference.short_title)) + + click_button 'Continue' + + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + expect(flash).to eq('Please pay here to purchase tickets.') + purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first + expect(purchase.quantity).to eq(2) + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + select Date.current.year + 2, from: 'expiration_year' + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4000000000000000' + + click_button 'Charge Card' + + expect(flash).to eq('Your card number is incorrect.') + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + select Date.current.year + 2, from: 'expiration_year' + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4000000000000002' + + click_button 'Charge Card' + + expect(flash).to eq('Your card was declined.') + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + select Date.current.year + 2, from: 'expiration_year' + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4000000000000127' + + click_button 'Charge Card' + + expect(flash).to eq("Your card's security code is incorrect.") + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + select Date.current.year + 2, from: 'expiration_year' + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4000000000000069' + + click_button 'Charge Card' + + expect(flash).to eq('Your card has expired.') + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + select Date.current.year + 2, from: 'expiration_year' + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4000000000000119' + + click_button 'Charge Card' + + expect(flash).to eq('An error occurred while processing your card. Try again in a little bit.') + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + + fill_in 'first_name', with: 'foo' + fill_in 'last_name', with: 'bar' + select Date.current.year + 2, from: 'expiration_year' + fill_in 'card_verification_value', with: '123' + fill_in 'credit_card_number', with: '4242424242424242' + + click_button 'Charge Card' + + payment = Payment.where(user_id: participant, conference_id: conference.id).first + expect(payment.amount).to eq(20) + expect(payment.status).to eq(1) + expect(payment.first_name).to eq('foo') + expect(payment.first_name).to eq('bar') + expect(payment.last4).not_to be_empty + expect(payment.authorization_code).not_to be_empty + expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) + expect(flash).to eq('Thanks! You have purchased your tickets successfully.') + end + end + end +end diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index 5c34ae01..3d42473f 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -32,14 +32,6 @@ feature Registration do expect(flash).to eq('Please pay here to purchase tickets.') purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first expect(purchase.quantity).to eq(2) - - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' - select Date.current.year + 2, from: 'expiration_year' - fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4242424242424242' - - click_button 'Charge Card' end end end diff --git a/spec/helpers/payments_helper_spec.rb b/spec/helpers/payments_helper_spec.rb index 5dded1b2..91feb177 100644 --- a/spec/helpers/payments_helper_spec.rb +++ b/spec/helpers/payments_helper_spec.rb @@ -1,15 +1,12 @@ require 'spec_helper' describe PaymentsHelper, type: :helper do - let(:conference) { create(:conference) } - let(:event) { create(:event, program: conference.program) } - describe '#months' do it 'returns the correct strings for months' do - expect(months).to match_array(Array([["1 - January", 1], ["2 - February", 2], ["3 - March", 3], - ["4 - April", 4], ["5 - May", 5], ["6 - June", 6], - ["7 - July", 7], ["8 - August", 8], ["9 - September", 9], - ["10 - October", 10], ["11 - November", 11], ["12 - December", 12]])) + expect(months).to match_array(Array([['1 - January', 1], ['2 - February', 2], ['3 - March', 3], + ['4 - April', 4], ['5 - May', 5], ['6 - June', 6], + ['7 - July', 7], ['8 - August', 8], ['9 - September', 9], + ['10 - October', 10], ['11 - November', 11], ['12 - December', 12]])) end end diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index dfefbae2..70ea5e9e 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -65,14 +65,11 @@ describe Payment do ticket_id: ticket_1.id).first expect(TicketPurchase.count).to eq(1) - # expect(purchase.quantity).to eq(1) + expect(purchase.quantity).to eq(1) expect(message.blank?).to be true - payment = Payment.new - payment.purchase(participant, conference, 1000) - - expect(Payment.count).to eq(1) - expect(payment.blank?).to be true + payment = Payment.new + payment.purchase(participant, conference, 1000) end end end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 0f781023..631ca59a 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -102,30 +102,52 @@ describe Ticket do end describe '#quantity_bought_by' do - it 'returns 0 if the user has bought but not paid for this ticket' do - create(:ticket_purchase, - user: user, - ticket: ticket, - quantity: 20) - expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) + context 'user has not paid' do + it 'returns the correct value if the user has bought this ticket' do + create(:ticket_purchase, + user: user, + ticket: ticket, + quantity: 20) + expect(ticket.quantity_bought_by(user, paid: false)).to eq(20) + end + + it 'returns zero if the user has not bought this ticket' do + expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) + end end - it 'returns zero if the user has not bought this ticket' do - expect(ticket.quantity_bought_by(user, paid: false)).to eq(0) + context 'user has paid' do + let!(:ticket_purchase) { create(:ticket_purchase, user: user, ticket: ticket, quantity: 20) } + before { ticket_purchase.update_attributes(paid: true) } + + it 'returns the correct value if the user has bought and paid for this ticket' do + expect(ticket.quantity_bought_by(user, paid: true)).to eq(20) + end end end describe '#total_price' do - it 'returns the 0 if the user has bought but not paid for this ticket' do - create(:ticket_purchase, - user: user, - ticket: ticket, - quantity: 20) - expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) + context 'user has not paid' do + it 'returns the correct value if the user has bought this ticket' do + create(:ticket_purchase, + user: user, + ticket: ticket, + quantity: 20) + expect(ticket.total_price(user, paid: false)).to eq(Money.new(100000, 'USD')) + end + + it 'returns zero if the user has not bought this ticket' do + expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) + end end - it 'returns zero if the user has not bought this ticket' do - expect(ticket.total_price(user, paid: false)).to eq(Money.new(0, 'USD')) + context 'user has paid' do + let!(:ticket_purchase) { create(:ticket_purchase, user: user, ticket: ticket, quantity: 20) } + before { ticket_purchase.update_attributes(paid: true) } + + it 'returns the correct value if the user has bought this ticket' do + expect(ticket.total_price(user, paid: true)).to eq(Money.new(100000, 'USD')) + end end end @@ -144,8 +166,8 @@ describe Ticket do create(:ticket_purchase, ticket: ticket, user: user, quantity: 20) end - it 'returns 0 as total price unless paid' do - expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(0, 'USD')) + it 'returns the correct total price' do + expect(Ticket.total_price(conference, user, paid: false)).to eq(Money.new(100000, 'USD')) end end @@ -155,8 +177,8 @@ describe Ticket do create(:ticket_purchase, ticket: diversity_supporter_ticket, user: user, quantity: 2) end - it 'returns 0 as total price unless paid' do - total_price = Money.new(0, 'USD') + it 'returns the correct total price' do + total_price = Money.new(200000, 'USD') expect(Ticket.total_price(conference, user, paid: false)).to eq(total_price) end end From 25c00462901f0193f25e261f03fde07d79ebc55c Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Mon, 18 Jul 2016 12:04:10 +0530 Subject: [PATCH 29/34] repair code reviews, improve environment configs --- app/assets/stylesheets/osem-payments.css.scss | 1 - app/models/ability.rb | 2 +- app/models/ticket.rb | 2 +- .../conference_registrations/show.html.haml | 3 +- app/views/payments/index.html.haml | 2 +- config/environments/development.rb | 6 +- config/environments/production.rb | 4 + config/environments/test.rb | 5 ++ spec/features/payments_spec.rb | 84 ++++++++++--------- spec/models/payment_spec.rb | 23 ++--- 10 files changed, 65 insertions(+), 67 deletions(-) diff --git a/app/assets/stylesheets/osem-payments.css.scss b/app/assets/stylesheets/osem-payments.css.scss index b74a6324..d5337542 100644 --- a/app/assets/stylesheets/osem-payments.css.scss +++ b/app/assets/stylesheets/osem-payments.css.scss @@ -1,7 +1,6 @@ .price-tags { list-style-type: none; padding-top: 2rem; - // display: inline-block; } .price-tags li { line-height: 40px; diff --git a/app/models/ability.rb b/app/models/ability.rb index 8bdcd6d7..9212ba35 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -81,7 +81,7 @@ class Ability can :index, Ticket can :manage, TicketPurchase, user_id: user.id - can :manage, Payment, user_id: user.id + can [:new], Payment, user_id: user.id can [:create, :destroy], Subscription, user_id: user.id diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 2a2cae65..d2454b74 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -26,7 +26,7 @@ class Ticket < ActiveRecord::Base end def unpaid?(user) - ticket_purchases.find_by(user: user, paid: false).present? + ticket_purchases.unpaid.by_user(user).present? end def total_price(user, paid: false) diff --git a/app/views/conference_registrations/show.html.haml b/app/views/conference_registrations/show.html.haml index 0f08b6c9..24923d07 100644 --- a/app/views/conference_registrations/show.html.haml +++ b/app/views/conference_registrations/show.html.haml @@ -93,7 +93,7 @@ -if @tickets.any? = "(#{@tickets.first.price.symbol}#{humanized_money @total_price})" %ul - .col.md-4 + .col-md-12 - @ticket_payments.each_pair do |ticket_id, tickets| %li = @total_quantity[ticket_id] @@ -102,6 +102,7 @@ for = tickets.first.price.symbol = humanized_money tickets.first.price + %br - if @tickets.any? = link_to 'Buy more tickets', conference_tickets_path(@conference.short_title), class: "btn btn-default" - else diff --git a/app/views/payments/index.html.haml b/app/views/payments/index.html.haml index dd0fb634..b231855f 100644 --- a/app/views/payments/index.html.haml +++ b/app/views/payments/index.html.haml @@ -29,4 +29,4 @@ %tr %td{:colspan => "5"} No payments have been attempted. .pull-right - = link_to "Conference Registration", conference_conference_registrations_path(@conference.short_title), class: 'btn btn-primary' + = link_to "Conference Registration", conference_conference_registration_path(@conference.short_title), class: 'btn btn-primary' diff --git a/config/environments/development.rb b/config/environments/development.rb index d85a731f..2a4ad4c4 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -96,10 +96,10 @@ Osem::Application.configure do end end + #Initialize Payment Gateway with valid credentials + ActiveMerchant::Billing::Base.mode = :test + ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new(:login => ENV['SECRET_KEY']) end -ActiveMerchant::Billing::Base.mode = :test - ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new( - :login => ENV['SECRET_KEY']) diff --git a/config/environments/production.rb b/config/environments/production.rb index d2b43a46..b5a9d46f 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -83,4 +83,8 @@ Osem::Application.configure do # Set the secret_key_base from the env, if not set by any other means config.secret_key_base ||= ENV["SECRET_KEY_BASE"] + + # Initialize Payment Gateway with valid credentials + ActiveMerchant::Billing::Base.mode = :test + ::GATEWAY = ActiveMerchant::Billing::StripeGateway.new(:login => ENV['SECRET_KEY']) end diff --git a/config/environments/test.rb b/config/environments/test.rb index 68c74dab..a6935e24 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -52,4 +52,9 @@ Osem::Application.configure do ActiveSupport::Deprecation.silenced = true end + # Initialize Payment Gateway with valid credentials + ActiveMerchant::Billing::Base.mode = :test + ::GATEWAY = ActiveMerchant::Billing::BogusGateway.new end + + diff --git a/spec/features/payments_spec.rb b/spec/features/payments_spec.rb index ae6b662a..d69f9545 100644 --- a/spec/features/payments_spec.rb +++ b/spec/features/payments_spec.rb @@ -16,7 +16,7 @@ feature Registration do context 'who is not registered' do - scenario 'purchases and pays for a ticket', feature: true, js: true do + scenario 'purchases and pays for a ticket, with gateway producing error', feature: true, js: true do visit root_path click_link 'Register' @@ -37,56 +37,57 @@ feature Registration do fill_in 'last_name', with: 'bar' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4000000000000000' + fill_in 'credit_card_number', with: '3' click_button 'Charge Card' - expect(flash).to eq('Your card number is incorrect.') + expect(Payment.count).to eq(0) + end + + scenario 'purchases and pays for a ticket, with card producing a transaction failure', feature: true, js: true do + visit root_path + click_link 'Register' + + expect(current_path).to eq(new_conference_conference_registration_path(conference.short_title)) + click_button 'Register' + + fill_in "tickets__#{ticket.id}", with: '2' + expect(current_path).to eq(conference_tickets_path(conference.short_title)) + + click_button 'Continue' + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + expect(flash).to eq('Please pay here to purchase tickets.') + purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first + expect(purchase.quantity).to eq(2) fill_in 'first_name', with: 'foo' fill_in 'last_name', with: 'bar' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4000000000000002' + fill_in 'credit_card_number', with: '2' click_button 'Charge Card' - expect(flash).to eq('Your card was declined.') - expect(current_path).to eq(new_conference_payment_path(conference.short_title)) - - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' - select Date.current.year + 2, from: 'expiration_year' - fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4000000000000127' - - click_button 'Charge Card' - - expect(flash).to eq("Your card's security code is incorrect.") - expect(current_path).to eq(new_conference_payment_path(conference.short_title)) - - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' - select Date.current.year + 2, from: 'expiration_year' - fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4000000000000069' - - click_button 'Charge Card' - - expect(flash).to eq('Your card has expired.') - expect(current_path).to eq(new_conference_payment_path(conference.short_title)) - - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' - select Date.current.year + 2, from: 'expiration_year' - fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4000000000000119' - - click_button 'Charge Card' - - expect(flash).to eq('An error occurred while processing your card. Try again in a little bit.') + expect(Payment.count).to eq(0) + end + + scenario 'purchases and pays for a ticket successfully', feature: true, js: true do + visit root_path + click_link 'Register' + + expect(current_path).to eq(new_conference_conference_registration_path(conference.short_title)) + click_button 'Register' + + fill_in "tickets__#{ticket.id}", with: '2' + expect(current_path).to eq(conference_tickets_path(conference.short_title)) + + click_button 'Continue' + expect(current_path).to eq(new_conference_payment_path(conference.short_title)) + expect(flash).to eq('Please pay here to purchase tickets.') + purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first + expect(purchase.quantity).to eq(2) fill_in 'first_name', with: 'foo' fill_in 'last_name', with: 'bar' @@ -96,14 +97,15 @@ feature Registration do click_button 'Charge Card' + expect(current_path).to eq(conference_conference_registration_path(conference.short_title)) + expect(Payment.count).to eq(1) payment = Payment.where(user_id: participant, conference_id: conference.id).first expect(payment.amount).to eq(20) - expect(payment.status).to eq(1) + expect(payment.status).to eq('success') expect(payment.first_name).to eq('foo') - expect(payment.first_name).to eq('bar') + expect(payment.last_name).to eq('bar') expect(payment.last4).not_to be_empty expect(payment.authorization_code).not_to be_empty - expect(current_path).to eq(conference_conference_registrations_path(conference.short_title)) expect(flash).to eq('Thanks! You have purchased your tickets successfully.') end end diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index 70ea5e9e..d077db5a 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -39,24 +39,8 @@ describe Payment do let!(:participant) { create(:user) } let!(:ticket_1) { create(:ticket) } let!(:conference) { create(:conference, tickets: [ticket_1]) } - - it 'creates no ticket purchase or payment if amount is less than 1' do - tickets = { ticket_1.id.to_s => '-1' } - TicketPurchase.purchase(conference, participant, tickets) - - expect(TicketPurchase.count).to eq(0) - expect(Payment.count).to eq(0) - end - - it 'creates no ticket purchase or payment if amount is 0' do - tickets = { ticket_1.id.to_s => '0' } - TicketPurchase.purchase(conference, participant, tickets) - - expect(TicketPurchase.count).to eq(0) - expect(Payment.count).to eq(0) - end - let(:payment) { create(:payment) } + it 'creates a purchase and payment for one ticket' do tickets = { ticket_1.id.to_s => '1' } message = TicketPurchase.purchase(conference, participant, tickets) @@ -68,8 +52,11 @@ describe Payment do expect(purchase.quantity).to eq(1) expect(message.blank?).to be true - payment = Payment.new payment.purchase(participant, conference, 1000) + + purchase = Payment.first + expect(Payment.count).to be(1) + expect(purchase.amount).to eq(10) end end end From c4d934d711f0b36175186898b2e77b397fe54806 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Tue, 19 Jul 2016 12:37:19 +0530 Subject: [PATCH 30/34] refactor payment model and its tests --- app/models/payment.rb | 33 ++++++------ spec/factories/payments.rb | 17 ++++-- spec/features/payments_spec.rb | 6 +-- spec/models/payment_spec.rb | 94 ++++++++++++++++++++++++++++------ 4 files changed, 111 insertions(+), 39 deletions(-) diff --git a/app/models/payment.rb b/app/models/payment.rb index b89b8377..cc94db2d 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -24,7 +24,7 @@ class Payment < ActiveRecord::Base } def credit_card - @credit_card = ActiveMerchant::Billing::CreditCard.new( + @credit_card ||= ActiveMerchant::Billing::CreditCard.new( first_name: first_name, last_name: last_name, number: credit_card_number, @@ -35,26 +35,25 @@ class Payment < ActiveRecord::Base end def purchase(user, conference, price_in_cents) - begin - recieve = GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) + gateway_response = begin + GATEWAY.purchase(price_in_cents, credit_card, currency: conference.tickets.first.price_currency) rescue - false + ActiveMerchant::Billing::Response.new(false, 'Unable to receive any response from the payment gateway.') end - unless recieve - errors.add(:base, 'Unable to recieve any response') - return false - end - unless recieve.success? - errors.add(:base, recieve.message) + + 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' + else + errors.add(:base, gateway_response.message) self.status = 'failure' - return false end - self.user_id = user.id - self.conference_id = conference.id - self.last4 = credit_card.display_number - self.authorization_code = recieve.authorization - self.status = 'success' - recieve.success? + + success? end end + diff --git a/spec/factories/payments.rb b/spec/factories/payments.rb index e4ccaee4..de852124 100644 --- a/spec/factories/payments.rb +++ b/spec/factories/payments.rb @@ -2,10 +2,19 @@ FactoryGirl.define do factory :payment do first_name { "#{Faker::Hipster.word} abc" } last_name { "#{Faker::Hipster.word} xyz" } - credit_card_number { '4242424242424242' } - card_verification_value { '123' } - expiration_month { '06' } + credit_card_number '4242424242424111' + card_verification_value '123' + expiration_month 6 expiration_year { Date.current.year + 2 } - amount { '10' } + amount 10 + end + + trait :invalid_credit_card do + credit_card_number '4242424242424222' + end + + trait :exception_credit_card do + credit_card_number '4242424242424333' end end + diff --git a/spec/features/payments_spec.rb b/spec/features/payments_spec.rb index d69f9545..15937113 100644 --- a/spec/features/payments_spec.rb +++ b/spec/features/payments_spec.rb @@ -37,7 +37,7 @@ feature Registration do fill_in 'last_name', with: 'bar' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '3' + fill_in 'credit_card_number', with: '4242424242423333' click_button 'Charge Card' @@ -65,7 +65,7 @@ feature Registration do fill_in 'last_name', with: 'bar' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '2' + fill_in 'credit_card_number', with: '4242424242422222' click_button 'Charge Card' @@ -93,7 +93,7 @@ feature Registration do fill_in 'last_name', with: 'bar' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' - fill_in 'credit_card_number', with: '4242424242424242' + fill_in 'credit_card_number', with: '4242424242421111' click_button 'Charge Card' diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index d077db5a..a5906f65 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -2,6 +2,13 @@ require 'spec_helper' describe Payment do + context 'new payment' do + let(:payment) { create(:payment) } + it 'sets status to "unpaid" by default' do + expect(payment.status).to eq('unpaid') + end + end + describe 'validations' do it 'has a valid factory' do expect(build(:payment)).to be_valid @@ -35,28 +42,85 @@ describe Payment do end - describe 'purchase' do - let!(:participant) { create(:user) } + describe '#purchase' do + let!(:user) { create(:user) } let!(:ticket_1) { create(:ticket) } let!(:conference) { create(:conference, tickets: [ticket_1]) } let(:payment) { create(:payment) } - it 'creates a purchase and payment for one ticket' do - tickets = { ticket_1.id.to_s => '1' } - message = TicketPurchase.purchase(conference, participant, tickets) - purchase = TicketPurchase.where(conference_id: conference.id, - user_id: participant.id, - ticket_id: ticket_1.id).first + 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.')) - expect(TicketPurchase.count).to eq(1) - expect(purchase.quantity).to eq(1) - expect(message.blank?).to be true + payment.purchase(user, conference, 1000) + end - payment.purchase(participant, conference, 1000) + context 'when the payment is successful' do + before { payment.purchase(user, conference, 1000) } - purchase = Payment.first - expect(Payment.count).to be(1) - expect(purchase.amount).to eq(10) + it 'returns true' do + payment_result = payment.purchase(user, conference, 1000) + expect(payment_result).to eq true + end + + it "assigns 'success' to payment.status" 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") + end + end + + context 'if the payment is not successful' do + before { payment.purchase(user, conference, 1000) } + + 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) + expect(payment_result).to eq false + end + + it 'assigns "failure" to payment.status' do + expect(payment.status).to eq('failure') + end + + it 'adds errors' do + expect(payment.errors[:base].count).to eq(1) + end + end + + context 'when there is a connection problem with the gateway' do + let(:payment) { create(:payment, :exception_credit_card) } + + it 'returns false' do + payment_result = payment.purchase(user, conference, 1000) + expect(payment_result).to eq false + end + + it 'assigns "failure" to payment.status' do + expect(payment.status).to eq('failure') + end + + it 'adds errors' do + expect(payment.errors[:base].count).to eq(1) + end + end end end end + From f00e051eff9a913db5d31574dc431cabbabf7fea Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 21 Jul 2016 10:15:17 +0530 Subject: [PATCH 31/34] 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 - From 6d8605073f03eb5858e88e49cb3b9d2423a0ffc3 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 21 Jul 2016 22:43:43 +0530 Subject: [PATCH 32/34] use full_name instead of first and last name, test changes, schema improvements --- app/controllers/payments_controller.rb | 11 ++----- app/models/payment.rb | 6 ++-- app/views/payments/_payment.html.haml | 11 +++---- app/views/payments/index.html.haml | 32 -------------------- app/views/tickets/_ticket.html.haml | 6 +--- db/migrate/20160606040848_create_payments.rb | 9 ++---- db/schema.rb | 15 +++++---- spec/factories/payments.rb | 3 +- spec/features/payments_spec.rb | 9 ++---- spec/models/payment_spec.rb | 4 +-- 10 files changed, 25 insertions(+), 81 deletions(-) delete mode 100644 app/views/payments/index.html.haml diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index 15f92cd8..077969f1 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -14,12 +14,12 @@ class PaymentsController < ApplicationController def create @payment = Payment.new(payment_params) - @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) 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 + @total_amount_to_pay = Ticket.total_price(@conference, current_user, paid: false) render 'new' end end @@ -27,17 +27,12 @@ class PaymentsController < ApplicationController private def update_purchased_ticket_purchases - paid_ticket_purchases = current_user.ticket_purchases.by_conference(@conference).unpaid - paid_ticket_purchases.each do |ticket| - ticket.paid = true - ticket.payment_id = @payment.id - ticket.save - end + current_user.ticket_purchases.by_conference(@conference).unpaid.update_all(paid: true, payment_id: @payment.id) end def payment_params params.require(:payment) - .permit(:first_name, :last_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) + .permit(:full_name, :credit_card_number, :expiration_month, :expiration_year, :card_verification_value, :amount) .merge(user: current_user, conference: @conference) end end diff --git a/app/models/payment.rb b/app/models/payment.rb index 25f2592f..d8c1dfd6 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -9,8 +9,7 @@ class Payment < ActiveRecord::Base attr_accessor :expiration_month attr_accessor :expiration_year - validates :first_name, presence: true - validates :last_name, presence: true + validates :full_name, presence: true validates :credit_card_number, presence: true validates :card_verification_value, presence: true, length: { minimum: 3, maximum: 4 } validates :expiration_month, presence: true, numericality: { greater_than_or_equal_to: 1, less_than_or_equal_to: 12 } @@ -27,8 +26,7 @@ class Payment < ActiveRecord::Base def credit_card @credit_card ||= ActiveMerchant::Billing::CreditCard.new( - first_name: first_name, - last_name: last_name, + name: full_name, number: credit_card_number, month: expiration_month, year: expiration_year, diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml index 6552622e..d99c86d1 100644 --- a/app/views/payments/_payment.html.haml +++ b/app/views/payments/_payment.html.haml @@ -1,14 +1,11 @@ = semantic_form_for(@payment, url: conference_payments_path) do |f| .form-group - = f.label :first_name, 'First name(as on card)' - = f.text_field :first_name, class: "form-control", placeholder: "John", id: "first_name" - .form-group - = f.label :last_name, 'Last name(as on card)' - = f.text_field :last_name,class: "form-control", placeholder: "Doe", id: "last_name" + = f.label :full_name, 'Full name (as on card)' + = f.text_field :full_name, class: "form-control", placeholder: "John Doe", id: "full_name" %span.pull-right %img.img-responsive{:src => image_url('credit_card.png')} .form-group - = f.label :credit_card_number, "Credit Card number(without spaces)" + = f.label :credit_card_number, "Credit Card number (without spaces)" .input-group = f.text_field :credit_card_number, class: "form-control", placeholder: "XXXX XXXX XXXX XXXX", id: "credit_card_number" %span.input-group-addon @@ -20,7 +17,7 @@ = f.label :expiration_year = f.select :expiration_year, years, {}, class: "form-control", id: "expiration_year" .form-group.col-md-10 - = f.label :card_verification_value, 'Security Code(3 on back, AmEx: 4 on front)' + = f.label :card_verification_value, 'Security Code (3 on back, AmEx: 4 on front)' = f.text_field :card_verification_value, class: "form-control", placeholder: "XXX", id: "card_verification_value" = f.number_field :amount, value: @total_amount_to_pay, class: "form-control", type: 'hidden' %ul.price-tags.pull-right diff --git a/app/views/payments/index.html.haml b/app/views/payments/index.html.haml deleted file mode 100644 index b231855f..00000000 --- a/app/views/payments/index.html.haml +++ /dev/null @@ -1,32 +0,0 @@ -.container - .row - .col-md-12 - .page-header - %h1 - Payment Attempts for - = @conference.title - - if flash[:notice].present? - .alert.alert-success - = flash[:notice] - %table.table.table-bordered.table-striped - %tr - %th First Name - %th Last Name - %th Last 4 - %th Amount - %th Status - %th Authorization Code - - if @payments.size > 0 - - @payments.each do |payment| - %tr - %td= payment.first_name - %td= payment.last_name - %td= payment.last4 - %td= number_to_currency payment.amount - %td= payment.status - %td= payment.authorization_code - - else - %tr - %td{:colspan => "5"} No payments have been attempted. - .pull-right - = link_to "Conference Registration", conference_conference_registration_path(@conference.short_title), class: 'btn btn-primary' diff --git a/app/views/tickets/_ticket.html.haml b/app/views/tickets/_ticket.html.haml index 7473f96d..3f730af6 100644 --- a/app/views/tickets/_ticket.html.haml +++ b/app/views/tickets/_ticket.html.haml @@ -8,12 +8,8 @@ -if !ticket.description.blank? = markdown(ticket.description) %td.col-sm-1.col-md-1 - - if ticket.bought?(current_user) - = text_field_tag("tickets[][#{ticket.id}]", 0, + = text_field_tag("tickets[][#{ticket.id}]", 0, type: 'number', min: 0, class: "form-control quantity", 'data-id' => ticket.id) - - else - = text_field_tag("tickets[][#{ticket.id}]", 0, type: 'number', min: 0, - class: "form-control quantity", 'data-id' => ticket.id) %td.col-sm-1.col-md-1.text-center = ticket.price.symbol %span{id: "price_#{ticket.id}"} diff --git a/db/migrate/20160606040848_create_payments.rb b/db/migrate/20160606040848_create_payments.rb index ebe2215b..20350d95 100644 --- a/db/migrate/20160606040848_create_payments.rb +++ b/db/migrate/20160606040848_create_payments.rb @@ -1,16 +1,13 @@ class CreatePayments < ActiveRecord::Migration def change create_table :payments do |t| - t.string :first_name - t.string :last_name + t.string :full_name, null: false t.string :last4 - t.decimal :amount, precision: 12, scale: 3 + t.integer :amount, null: false t.string :authorization_code - t.integer :status, default: 0 + t.integer :status, default: 0, null: false t.integer :user_id, null: false t.integer :conference_id, null: false - t.datetime :created_at - t.datetime :updated_at t.timestamps null: false end diff --git a/db/schema.rb b/db/schema.rb index f5290aa4..597f5ad9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -251,16 +251,15 @@ ActiveRecord::Schema.define(version: 20160624151257) do end create_table "payments", force: :cascade do |t| - t.string "first_name" - t.string "last_name" + t.string "full_name", null: false t.string "last4" - t.decimal "amount", precision: 12, scale: 3 + t.integer "amount", null: false t.string "authorization_code" - t.integer "status", default: 0 - t.integer "user_id" - t.integer "conference_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.integer "status", default: 0, null: false + t.integer "user_id", null: false + t.integer "conference_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end create_table "programs", force: :cascade do |t| diff --git a/spec/factories/payments.rb b/spec/factories/payments.rb index 7cdbfabc..4f0ef7a1 100644 --- a/spec/factories/payments.rb +++ b/spec/factories/payments.rb @@ -2,8 +2,7 @@ FactoryGirl.define do factory :payment do user conference - first_name { "#{Faker::Hipster.word} abc" } - last_name { "#{Faker::Hipster.word} xyz" } + full_name { Faker::Hipster.word.to_s } credit_card_number '4242424242424111' card_verification_value '123' expiration_month 6 diff --git a/spec/features/payments_spec.rb b/spec/features/payments_spec.rb index 15937113..26278dba 100644 --- a/spec/features/payments_spec.rb +++ b/spec/features/payments_spec.rb @@ -33,8 +33,7 @@ feature Registration do purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first expect(purchase.quantity).to eq(2) - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' + fill_in 'full_name', with: 'foo' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242423333' @@ -61,8 +60,7 @@ feature Registration do purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first expect(purchase.quantity).to eq(2) - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' + fill_in 'full_name', with: 'foo' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242422222' @@ -89,8 +87,7 @@ feature Registration do purchase = TicketPurchase.where(user_id: participant.id, ticket_id: ticket.id).first expect(purchase.quantity).to eq(2) - fill_in 'first_name', with: 'foo' - fill_in 'last_name', with: 'bar' + fill_in 'full_name', with: 'foo' select Date.current.year + 2, from: 'expiration_year' fill_in 'card_verification_value', with: '123' fill_in 'credit_card_number', with: '4242424242421111' diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index 8c3959e1..0cce495e 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -14,9 +14,7 @@ describe Payment do expect(build(:payment)).to be_valid end - it { is_expected.to validate_presence_of(:first_name) } - - it { is_expected.to validate_presence_of(:last_name) } + it { is_expected.to validate_presence_of(:full_name) } it { is_expected.to validate_presence_of(:credit_card_number) } From 851b467235754cd7df0b46878dbb3ff631c69c6e Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 22 Jul 2016 00:05:30 +0530 Subject: [PATCH 33/34] payment view improvement --- app/assets/stylesheets/application.css | 1 - app/assets/stylesheets/osem-payments.css.scss | 23 ------------------- app/views/payments/_payment.html.haml | 10 ++------ 3 files changed, 2 insertions(+), 32 deletions(-) delete mode 100644 app/assets/stylesheets/osem-payments.css.scss diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css index 84c0bd9a..d0447354 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.css @@ -10,7 +10,6 @@ *= require osem-splash *= require font-awesome *= require osem-fonts - *= require osem-payments *= require bootstrap-markdown *= require bootstrap-datetimepicker *= require leaflet diff --git a/app/assets/stylesheets/osem-payments.css.scss b/app/assets/stylesheets/osem-payments.css.scss deleted file mode 100644 index d5337542..00000000 --- a/app/assets/stylesheets/osem-payments.css.scss +++ /dev/null @@ -1,23 +0,0 @@ -.price-tags { - list-style-type: none; - padding-top: 2rem; -} -.price-tags li { - line-height: 40px; - position: relative; - margin-right: -3rem; -} -.price-tags a { - background: #2f991d; - color: #fff; - font-size: 1.5rem; - padding: 9px 10px; - text-decoration: none; -} -.price-tags a:after { - content: ""; - float: left; - border-top: 20px solid transparent; - border-right: 20px solid #2f991d; - border-bottom: 20px solid transparent; -} diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml index d99c86d1..7f429189 100644 --- a/app/views/payments/_payment.html.haml +++ b/app/views/payments/_payment.html.haml @@ -20,12 +20,6 @@ = f.label :card_verification_value, 'Security Code (3 on back, AmEx: 4 on front)' = f.text_field :card_verification_value, class: "form-control", placeholder: "XXX", id: "card_verification_value" = f.number_field :amount, value: @total_amount_to_pay, class: "form-control", type: 'hidden' - %ul.price-tags.pull-right - %li.text-muted - you will pay - %li - %a - = number_to_currency @total_amount_to_pay - .form-group - = f.submit "Charge Card", class: "btn btn-primary" + .form-group.text-center + = f.submit "Pay #{number_to_currency @total_amount_to_pay, unit: @total_amount_to_pay.symbol}", class: "btn btn-primary" = link_to "Cancel", conference_conference_registration_path, class: "btn btn-danger" From 4d4736f0429b9d907982bab47394946f9523723a Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 22 Jul 2016 01:15:22 +0530 Subject: [PATCH 34/34] repair tests --- app/views/payments/_payment.html.haml | 2 +- spec/features/payments_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/payments/_payment.html.haml b/app/views/payments/_payment.html.haml index 7f429189..db508369 100644 --- a/app/views/payments/_payment.html.haml +++ b/app/views/payments/_payment.html.haml @@ -21,5 +21,5 @@ = f.text_field :card_verification_value, class: "form-control", placeholder: "XXX", id: "card_verification_value" = f.number_field :amount, value: @total_amount_to_pay, class: "form-control", type: 'hidden' .form-group.text-center - = f.submit "Pay #{number_to_currency @total_amount_to_pay, unit: @total_amount_to_pay.symbol}", class: "btn btn-primary" + = f.submit "Pay #{number_to_currency @total_amount_to_pay, unit: @total_amount_to_pay.symbol}", class: "btn btn-primary", id: "Charge Card" = link_to "Cancel", conference_conference_registration_path, class: "btn btn-danger" diff --git a/spec/features/payments_spec.rb b/spec/features/payments_spec.rb index 26278dba..d9ef69dd 100644 --- a/spec/features/payments_spec.rb +++ b/spec/features/payments_spec.rb @@ -41,6 +41,7 @@ feature Registration do click_button 'Charge Card' expect(Payment.count).to eq(0) + expect(current_path).to eq(conference_conference_registration_path(conference.short_title)) end scenario 'purchases and pays for a ticket, with card producing a transaction failure', feature: true, js: true do @@ -68,6 +69,7 @@ feature Registration do click_button 'Charge Card' expect(Payment.count).to eq(0) + expect(current_path).to eq(conference_conference_registration_path(conference.short_title)) end scenario 'purchases and pays for a ticket successfully', feature: true, js: true do