pass paid value as hash, improve code reviews

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)
+    @total_price = Ticket.total_price(@conference, current_user, :paid => true)
-    @tickets = current_user.ticket_purchases.where(conference_id: @conference.id)
+    @tickets = current_user.ticket_purchases.where(conference_id: @conference.id, paid: true)
     @ticket_payments = @tickets.group_by(&:payment_id)
   end
 

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.valid? && @payment.purchase(current_user, @conference, price_in_cents) && @payment.save
-      if @payment.save
+      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.' }
-        redirect_to conference_conference_registrations_path(@conference.short_title), flash: { success: 'Thanks! You have purchased your tickets successfully.' }
+    else
-      else
+      render 'new'
-        render 'new'
-      end
     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|
-      paid_ticket_purchases.each do |ticket|
+      ticket.paid = true
-        ticket.paid = true
+      ticket.payment_id = payment.id
-        ticket.payment_id = payment.id
+      ticket.save
-        ticket.save
-      end
     end
   end
 

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)
+  def quantity_bought_by(user, hashed_paid)
-    result = ticket_purchases.where(user_id: user.id, paid: paid)
+   purchased_tickets = ticket_purchases.where(user_id: user.id, paid: hashed_paid[:paid])
     quantity = 0
-    if result
+    if purchased_tickets
-      result.each do |ticket|
+      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)
+  def total_price(user, hashed_paid)
-    quantity_bought_by(user, paid) * price
+    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

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

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

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]) }

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