From 4dcfc55418d0f000132a8356dee4ae76869b96cd Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Tue, 2 Aug 2016 01:37:55 +0530 Subject: [PATCH 01/11] add ability to create and purchase free tickets --- app/models/ticket.rb | 2 +- app/models/ticket_purchase.rb | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index ec9c3436..8d8864ae 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -13,7 +13,7 @@ class Ticket < ActiveRecord::Base validates :price_cents, :price_currency, :title, presence: true - validates_numericality_of :price_cents, greater_than: 0 + validates_numericality_of :price_cents, greater_than_or_equal_to: 0 def bought?(user) buyers.include?(user) diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index b7a8951f..7df0dee7 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -23,7 +23,7 @@ class TicketPurchase < ActiveRecord::Base ActiveRecord::Base.transaction do conference.tickets.each do |ticket| quantity = purchases[ticket.id.to_s].to_i - # if the user bought the ticket, just update the quantity + # if the user bought the ticket and is still unpaid, just update the quantity if ticket.bought?(user) && ticket.unpaid?(user) purchase = update_quantity(conference, quantity, ticket, user) else @@ -39,10 +39,20 @@ class TicketPurchase < ActiveRecord::Base end def self.purchase_ticket(conference, quantity, ticket, user) - purchase = new(ticket_id: ticket.id, - conference_id: conference.id, - user_id: user.id, - quantity: quantity) if quantity > 0 + if quantity > 0 + if ticket.price_cents.zero? + purchase = new(ticket_id: ticket.id, + conference_id: conference.id, + user_id: user.id, + quantity: quantity, + paid: true) + else + purchase = new(ticket_id: ticket.id, + conference_id: conference.id, + user_id: user.id, + quantity: quantity) + end + end purchase end From b2f75dd907d46c2f5bac7d212eb9c0eee999605c Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 18 Aug 2016 23:22:46 +0530 Subject: [PATCH 02/11] spec/tickets: add tests for free ticket creation --- spec/features/tickets_spec.rb | 4 ++-- spec/models/ticket_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/features/tickets_spec.rb b/spec/features/tickets_spec.rb index 3a40f537..ab872196 100644 --- a/spec/features/tickets_spec.rb +++ b/spec/features/tickets_spec.rb @@ -35,7 +35,7 @@ feature Ticket do fill_in 'ticket_price', with: '-1' click_button 'Create Ticket' - expect(flash).to eq("Creating Ticket failed: Title can't be blank. Price cents must be greater than 0.") + expect(flash).to eq("Creating Ticket failed: Title can't be blank. Price cents must be greater than or equal to 0.") expect(Ticket.count).to eq(0) end @@ -72,7 +72,7 @@ feature Ticket do # It's necessary to multiply by 100 because the price is in cents expect(ticket.price).to eq(Money.new(100 * 100, 'USD')) expect(ticket.title).to eq('Business Ticket') - expect(flash).to eq("Ticket update failed: Title can't be blank. Price cents must be greater than 0.") + expect(flash).to eq("Ticket update failed: Title can't be blank. Price cents must be greater than or equal to 0.") expect(Ticket.count).to eq(1) end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 631ca59a..70aa80d1 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -22,14 +22,14 @@ describe Ticket do should validate_presence_of(:price_currency) end - it 'is not valid with a price_cents equals zero' do - should_not allow_value(0).for(:price_cents) - end - it 'is not valid with a price_cents smaller than zero' do should_not allow_value(-1).for(:price_cents) end + it 'is valid with a price_cents equals zero' do + should allow_value(0).for(:price_cents) + end + it 'is valid with a price_cents greater than zero' do should allow_value(1).for(:price_cents) end From b28be23284acbe47ed5e611710ca83ba7d9ce9ce Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Thu, 18 Aug 2016 23:23:38 +0530 Subject: [PATCH 03/11] spec/ticket: add tests for purchasing free tickets --- spec/features/ticket_purchases_spec.rb | 23 ++++++++++++++++++++++- spec/models/ticket_purchase_spec.rb | 16 +++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index fc8e5174..ad8bca54 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -2,7 +2,8 @@ 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!(:free_ticket) { create(:ticket, price_cents: 0) } + let!(:conference) { create(:conference, title: 'ExampleCon', tickets: [ticket, free_ticket], registration_period: create(:registration_period, start_date: 3.days.ago)) } let!(:participant) { create(:user) } context 'as a participant' do @@ -87,6 +88,26 @@ feature Registration do expect(flash).to eq('Your card was declined. Please try again with correct credentials.') end end + + scenario 'purchases free tickets' 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__#{free_ticket.id}", with: '5' + expect(current_path).to eq(conference_tickets_path(conference.short_title)) + + click_button 'Continue' + + expect(current_path).to eq(new_conference_conference_registration_path(conference.short_title)) + purchase = TicketPurchase.where(user_id: participant.id, ticket_id: free_ticket.id).first + expect(purchase.quantity).to eq(5) + expect(purchase.paid).to eq(true) + + expect(page.has_content?("5 #{free_ticket.title} Tickets for $ 0")).to be true + end end context 'who is registered' do diff --git a/spec/models/ticket_purchase_spec.rb b/spec/models/ticket_purchase_spec.rb index 9248b52f..a31d5291 100644 --- a/spec/models/ticket_purchase_spec.rb +++ b/spec/models/ticket_purchase_spec.rb @@ -41,7 +41,21 @@ describe TicketPurchase 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!(:free_ticket) { create(:ticket) } + let!(:conference) { create(:conference, tickets: [ticket_1, ticket_2, free_ticket]) } + + it 'creates purchase to free ticket' do + tickets = { free_ticket.id.to_s => '10' } + message = TicketPurchase.purchase(conference, participant, tickets) + purchase = TicketPurchase.where(conference_id: conference.id, + user_id: participant.id, + ticket_id: free_ticket.id).first + + expect(TicketPurchase.count).to eq(1) + expect(purchase.quantity).to eq(10) + expect(message.blank?).to be true + + end it 'creates a purchase for one ticket' do tickets = { ticket_1.id.to_s => '1' } From b36d4e98e2e9fa0d0ce5c99b78686d1833b144c4 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Fri, 19 Aug 2016 00:51:49 +0530 Subject: [PATCH 04/11] ticket_purchase_controller: free ticket routing --- app/controllers/ticket_purchases_controller.rb | 9 +++++++-- spec/features/ticket_purchases_spec.rb | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index a8749eed..a1bf8826 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -11,8 +11,13 @@ class TicketPurchasesController < ApplicationController redirect_to new_conference_payment_path, notice: 'Please pay here to get tickets.' else - redirect_to conference_tickets_path(@conference.short_title), - error: 'Please get at least one ticket to continue.' + if current_user.ticket_purchases.by_conference(@conference).paid.any? + redirect_to conference_conference_registration_path(@conference.short_title), + notice: 'You have free tickets for the conference.' + else + redirect_to conference_tickets_path(@conference.short_title), + error: 'Please get at least one ticket to continue.' + end end else redirect_to conference_conference_registration_path(@conference.short_title), diff --git a/spec/features/ticket_purchases_spec.rb b/spec/features/ticket_purchases_spec.rb index ad8bca54..f8c755a3 100644 --- a/spec/features/ticket_purchases_spec.rb +++ b/spec/features/ticket_purchases_spec.rb @@ -101,10 +101,10 @@ feature Registration do click_button 'Continue' - expect(current_path).to eq(new_conference_conference_registration_path(conference.short_title)) + expect(current_path).to eq(conference_conference_registration_path(conference.short_title)) purchase = TicketPurchase.where(user_id: participant.id, ticket_id: free_ticket.id).first expect(purchase.quantity).to eq(5) - expect(purchase.paid).to eq(true) + expect(purchase.paid).to be true expect(page.has_content?("5 #{free_ticket.title} Tickets for $ 0")).to be true end From a6f7516eaee64774f0b8d7c77d73baea0dde8e57 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Mon, 22 Aug 2016 13:09:02 +0530 Subject: [PATCH 05/11] controller/ticket_purchase: rubocop fix --- app/controllers/ticket_purchases_controller.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/controllers/ticket_purchases_controller.rb b/app/controllers/ticket_purchases_controller.rb index a1bf8826..1c672d69 100644 --- a/app/controllers/ticket_purchases_controller.rb +++ b/app/controllers/ticket_purchases_controller.rb @@ -10,14 +10,12 @@ class TicketPurchasesController < ApplicationController if current_user.ticket_purchases.by_conference(@conference).unpaid.any? redirect_to new_conference_payment_path, notice: 'Please pay here to get tickets.' + elsif current_user.ticket_purchases.by_conference(@conference).paid.any? + redirect_to conference_conference_registration_path(@conference.short_title), + notice: 'You have free tickets for the conference.' else - if current_user.ticket_purchases.by_conference(@conference).paid.any? - redirect_to conference_conference_registration_path(@conference.short_title), - notice: 'You have free tickets for the conference.' - else - redirect_to conference_tickets_path(@conference.short_title), - error: 'Please get at least one ticket to continue.' - end + redirect_to conference_tickets_path(@conference.short_title), + error: 'Please get at least one ticket to continue.' end else redirect_to conference_conference_registration_path(@conference.short_title), From 53291e25b5fe4495602a23771a49c7c07bd55f10 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 27 Aug 2016 20:38:31 +0530 Subject: [PATCH 06/11] automatically create free ticket with new conf --- app/models/conference.rb | 10 ++++++++++ spec/models/conference_spec.rb | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/models/conference.rb b/app/models/conference.rb index 8bf16a09..6fa13a4e 100644 --- a/app/models/conference.rb +++ b/app/models/conference.rb @@ -64,6 +64,8 @@ class Conference < ActiveRecord::Base before_create :add_color before_create :create_email_settings + after_create :create_free_ticket + def date_range_string startstr = 'Unknown - ' endstr = 'Unknown' @@ -647,6 +649,14 @@ class Conference < ActiveRecord::Base create_roles end + ## + # Creates free ticket for the conference + # after the conference has been successfully created + # Will create 1 new record for 'free' ticket + def create_free_ticket + ticket = Ticket.where(conference: self, title: 'Free Access', price_cents: 0).first_or_create!(description: 'Get free access tickets for the conference.') + end + ## # Creates the roles of the conference # after the conference has been successfully created diff --git a/spec/models/conference_spec.rb b/spec/models/conference_spec.rb index 8aebc0cd..f8a5db2b 100755 --- a/spec/models/conference_spec.rb +++ b/spec/models/conference_spec.rb @@ -1583,4 +1583,14 @@ describe Conference do end end end + + describe 'after_create' do + let!(:conference) { create(:conference) } + + it 'creates free tickets' do + free_ticket = Ticket.find_by(conference: conference) + expect(free_ticket).not_to be_nil + expect(free_ticket.price_cents).to eq(0) + end + end end From fe828d24db31f38b1c6c2d6cf0d59302e31f7bf8 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Mon, 29 Aug 2016 19:49:25 +0530 Subject: [PATCH 07/11] conference: refactor code and tests --- app/models/conference.rb | 2 +- spec/models/conference_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/conference.rb b/app/models/conference.rb index 6fa13a4e..801ea040 100644 --- a/app/models/conference.rb +++ b/app/models/conference.rb @@ -654,7 +654,7 @@ class Conference < ActiveRecord::Base # after the conference has been successfully created # Will create 1 new record for 'free' ticket def create_free_ticket - ticket = Ticket.where(conference: self, title: 'Free Access', price_cents: 0).first_or_create!(description: 'Get free access tickets for the conference.') + tickets.where(title: 'Free Access', price_cents: 0).first_or_create!(description: 'Get free access tickets for the conference.') end ## diff --git a/spec/models/conference_spec.rb b/spec/models/conference_spec.rb index f8a5db2b..ad0720ad 100755 --- a/spec/models/conference_spec.rb +++ b/spec/models/conference_spec.rb @@ -1588,7 +1588,7 @@ describe Conference do let!(:conference) { create(:conference) } it 'creates free tickets' do - free_ticket = Ticket.find_by(conference: conference) + free_ticket = conference.tickets.first expect(free_ticket).not_to be_nil expect(free_ticket.price_cents).to eq(0) end From b9b7a4d97d56597fed0f262748be603a71a80ed3 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sat, 3 Sep 2016 17:26:42 +0530 Subject: [PATCH 08/11] remove tickets testing ambiguity --- spec/features/tickets_spec.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/features/tickets_spec.rb b/spec/features/tickets_spec.rb index ab872196..ab513cd1 100644 --- a/spec/features/tickets_spec.rb +++ b/spec/features/tickets_spec.rb @@ -24,7 +24,7 @@ feature Ticket do click_button 'Create Ticket' expect(flash).to eq('Ticket successfully created.') - expect(Ticket.count).to eq(1) + expect(Ticket.count).to eq(2) end scenario 'add a invalid ticket', feature: true, js: true do @@ -36,7 +36,7 @@ feature Ticket do click_button 'Create Ticket' expect(flash).to eq("Creating Ticket failed: Title can't be blank. Price cents must be greater than or equal to 0.") - expect(Ticket.count).to eq(0) + expect(Ticket.count).to eq(1) end context 'Ticket already created' do @@ -44,9 +44,9 @@ feature Ticket do scenario 'edit valid ticket', feature: true, js: true do visit admin_conference_tickets_path(conference.short_title) - click_link 'Edit' + click_link('Edit', href: edit_admin_conference_ticket_path(conference.short_title, ticket.id)) - fill_in 'ticket_title', with: 'Free Ticket' + fill_in 'ticket_title', with: 'Event Ticket' fill_in 'ticket_price', with: '50' click_button 'Update Ticket' @@ -54,14 +54,14 @@ feature Ticket do ticket.reload # It's necessary to multiply by 100 because the price is in cents expect(ticket.price).to eq(Money.new(50 * 100, 'USD')) - expect(ticket.title).to eq('Free Ticket') + expect(ticket.title).to eq('Event Ticket') expect(flash).to eq('Ticket successfully updated.') - expect(Ticket.count).to eq(1) + expect(Ticket.count).to eq(2) end scenario 'edit invalid ticket', feature: true, js: true do visit admin_conference_tickets_path(conference.short_title) - click_link 'Edit' + click_link('Edit', href: edit_admin_conference_ticket_path(conference.short_title, ticket.id)) fill_in 'ticket_title', with: '' fill_in 'ticket_price', with: '-5' @@ -73,15 +73,15 @@ feature Ticket do expect(ticket.price).to eq(Money.new(100 * 100, 'USD')) expect(ticket.title).to eq('Business Ticket') expect(flash).to eq("Ticket update failed: Title can't be blank. Price cents must be greater than or equal to 0.") - expect(Ticket.count).to eq(1) + expect(Ticket.count).to eq(2) end scenario 'delete ticket', feature: true, js: true do visit admin_conference_tickets_path(conference.short_title) - click_link 'Delete' + click_link('Delete', href: admin_conference_ticket_path(conference.short_title, ticket.id)) expect(flash).to eq('Ticket successfully destroyed.') - expect(Ticket.count).to eq(0) + expect(Ticket.count).to eq(1) end end end From b1826a1abcc4d94f113589a0f67c05e762240094 Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sun, 4 Sep 2016 10:23:25 +0530 Subject: [PATCH 09/11] rubocop: ignore conference class length warning --- .rubocop.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 7da4d8d5..b7c4b92e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -96,6 +96,8 @@ Metrics/BlockNesting: Metrics/ClassLength: Max: 575 + Exclude: + - 'app/models/conference.rb' # avoid redundunt curly braces when it is obvious that hash is used Style/BracesAroundHashParameters: From e0d5b461b37fc399c2ec3eb4a793e8c8a32d280e Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Sun, 20 Nov 2016 14:21:01 +0530 Subject: [PATCH 10/11] rubocop: annotate exceptions --- .rubocop.yml | 2 -- app/models/conference.rb | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index b7c4b92e..7da4d8d5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -96,8 +96,6 @@ Metrics/BlockNesting: Metrics/ClassLength: Max: 575 - Exclude: - - 'app/models/conference.rb' # avoid redundunt curly braces when it is obvious that hash is used Style/BracesAroundHashParameters: diff --git a/app/models/conference.rb b/app/models/conference.rb index 801ea040..31c61d73 100644 --- a/app/models/conference.rb +++ b/app/models/conference.rb @@ -1,3 +1,4 @@ +# rubocop:disable Metrics/ClassLength ## # This class represents a conference class Conference < ActiveRecord::Base @@ -1063,3 +1064,4 @@ class Conference < ActiveRecord::Base result end end +# rubocop:enable Metrics/ClassLength From 241bc89293b1bb9240f32bc5d81ddc6701ededcb Mon Sep 17 00:00:00 2001 From: Rishabh Saxena Date: Tue, 22 Nov 2016 14:43:59 +0530 Subject: [PATCH 11/11] repair code reviews --- app/models/ticket_purchase.rb | 17 +++++------------ spec/models/conference_spec.rb | 7 ++++--- spec/models/ticket_purchase_spec.rb | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/app/models/ticket_purchase.rb b/app/models/ticket_purchase.rb index 7df0dee7..793f6926 100644 --- a/app/models/ticket_purchase.rb +++ b/app/models/ticket_purchase.rb @@ -40,18 +40,11 @@ class TicketPurchase < ActiveRecord::Base def self.purchase_ticket(conference, quantity, ticket, user) if quantity > 0 - if ticket.price_cents.zero? - purchase = new(ticket_id: ticket.id, - conference_id: conference.id, - user_id: user.id, - quantity: quantity, - paid: true) - else - purchase = new(ticket_id: ticket.id, - conference_id: conference.id, - user_id: user.id, - quantity: quantity) - end + purchase = new(ticket_id: ticket.id, + conference_id: conference.id, + user_id: user.id, + quantity: quantity, + paid: ticket.price_cents.zero?) end purchase end diff --git a/spec/models/conference_spec.rb b/spec/models/conference_spec.rb index ad0720ad..e16373bd 100755 --- a/spec/models/conference_spec.rb +++ b/spec/models/conference_spec.rb @@ -1585,11 +1585,12 @@ describe Conference do end describe 'after_create' do - let!(:conference) { create(:conference) } + let(:conference) { Conference.new(title: 'ABC', short_title: 'XYZ', start_date: Date.today, end_date: Date.today + 10, timezone: 'GMT') } - it 'creates free tickets' do + it 'calls back to create free ticket' do + conference.save + conference.run_callbacks :create free_ticket = conference.tickets.first - expect(free_ticket).not_to be_nil expect(free_ticket.price_cents).to eq(0) end end diff --git a/spec/models/ticket_purchase_spec.rb b/spec/models/ticket_purchase_spec.rb index a31d5291..0bb33f8e 100644 --- a/spec/models/ticket_purchase_spec.rb +++ b/spec/models/ticket_purchase_spec.rb @@ -41,7 +41,7 @@ describe TicketPurchase do let!(:participant) { create(:user) } let!(:ticket_1) { create(:ticket) } let!(:ticket_2) { create(:ticket) } - let!(:free_ticket) { create(:ticket) } + let!(:free_ticket) { create(:ticket, price_cents: 0) } let!(:conference) { create(:conference, tickets: [ticket_1, ticket_2, free_ticket]) } it 'creates purchase to free ticket' do