From 893f517ab0b9506177e087c3041a6624e08bb923 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 8 Jul 2016 20:24:56 +0530 Subject: [PATCH] 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