From 888c7a8524d6db68a469d4aadb3d33cb39bf43b7 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 23:07:17 +0530 Subject: [PATCH] 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