From 12df6b45e70e1abe90bf5735932b3e43b6a02206 Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Sun, 27 Dec 2015 22:15:12 -0500 Subject: [PATCH 1/8] Presenter method to indicate an auction is over --- app/models/presenter/auction.rb | 4 ++++ spec/models/presenter/auction_spec.rb | 32 ++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/app/models/presenter/auction.rb b/app/models/presenter/auction.rb index 3e05c5eb..baa8c814 100644 --- a/app/models/presenter/auction.rb +++ b/app/models/presenter/auction.rb @@ -59,6 +59,10 @@ def available? ) end + def over? + model.end_datetime < Time.now + end + def user_is_winning_bidder?(user) return false if !current_bid? user.id == current_bid.bidder_id diff --git a/spec/models/presenter/auction_spec.rb b/spec/models/presenter/auction_spec.rb index e7239661..a01e1187 100644 --- a/spec/models/presenter/auction_spec.rb +++ b/spec/models/presenter/auction_spec.rb @@ -52,7 +52,7 @@ let(:ar_auction) { FactoryGirl.create(:auction, :closed) } it 'should be false' do - expect(auction.available?).to eq(false) + expect(auction).to_not be_available end end @@ -60,7 +60,7 @@ let(:ar_auction) { FactoryGirl.create(:auction, :future) } it 'should be false' do - expect(auction.available?).to eq(false) + expect(auction).to_not be_available end end @@ -68,7 +68,33 @@ let(:ar_auction) { FactoryGirl.create(:auction) } it 'should be false' do - expect(auction.available?).to eq(true) + expect(auction).to be_available + end + end + end + + describe '#over?' do + context 'when the auction has expired' do + let(:ar_auction) { FactoryGirl.create(:closed_auction) } + + it 'should be true' do + expect(auction).to be_over + end + end + + context 'when the auction is still running' do + let(:ar_auction) { FactoryGirl.create(:auction) } + + it 'should not be true' do + expect(auction).to_not be_over + end + end + + context 'when the auction has not started' do + let(:ar_auction) { FactoryGirl.create(:future_auction) } + + it 'should be false' do + expect(auction).to_not be_over end end end From ada28c43405cf0e5267033ab53fac964cc568aa8 Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Sun, 27 Dec 2015 22:53:04 -0500 Subject: [PATCH 2/8] Added a basic view for the bid winner This does not include any tests yet. Want to firm up that this is what we want first --- app/helpers/auction_helper.rb | 9 +++++++++ app/views/auctions/show.html.erb | 22 ++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 app/helpers/auction_helper.rb diff --git a/app/helpers/auction_helper.rb b/app/helpers/auction_helper.rb new file mode 100644 index 00000000..0b0ac44d --- /dev/null +++ b/app/helpers/auction_helper.rb @@ -0,0 +1,9 @@ +module AuctionHelper + def auction_status(auction) + if auction.available? + 'Open' + else + 'Closed' + end + end +end diff --git a/app/views/auctions/show.html.erb b/app/views/auctions/show.html.erb index 3817228f..c104f7e2 100644 --- a/app/views/auctions/show.html.erb +++ b/app/views/auctions/show.html.erb @@ -19,16 +19,22 @@

Status:

-

<% if @auction.available? %>Open<% else %>Closed<% end %>

+

<%= auction_status(@auction) %>

+ + <% if @auction.over? && @auction.bids? %> +

Winner:

+

<% if @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

+ + <% end %>

Current Bid:

<%= number_to_currency(@auction.current_bid_amount) %> - <%= - link_to( - pluralize(@auction.bids.length, 'bid'), - auction_bids_path(@auction.id) - ) - %> - + <%= + link_to( + pluralize(@auction.bids.length, 'bid'), + auction_bids_path(@auction.id) + ) + %> +

Bid deadline:

<%= @auction.end_datetime.strftime("%m/%d/%Y at %I:%M %Z") %>

From 728a27f2ad0212f189a6faaabc7457139c0f0953 Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Mon, 28 Dec 2015 08:22:00 -0500 Subject: [PATCH 3/8] Check a user is logged in before You Are Winner Even if the logged in user is not the winner of the auction, the check will crash if there is no logged in user. The check could also be fixed to return false if nobody is logged in, but will just make this change for now instead since I don't want to modify that method yet. --- app/views/auctions/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/auctions/show.html.erb b/app/views/auctions/show.html.erb index c104f7e2..d58294eb 100644 --- a/app/views/auctions/show.html.erb +++ b/app/views/auctions/show.html.erb @@ -23,7 +23,7 @@ <% if @auction.over? && @auction.bids? %>

Winner:

-

<% if @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

+

<% if current_user && @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

<% end %>

Current Bid:

From 85ae4558893cc5b3700e8098c558bea2f4f364b7 Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Sun, 27 Dec 2015 22:53:04 -0500 Subject: [PATCH 4/8] Added a basic view for the bid winner This does not include any tests yet. Want to firm up that this is what we want first --- app/views/auctions/show.html.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/auctions/show.html.erb b/app/views/auctions/show.html.erb index d58294eb..83240bef 100644 --- a/app/views/auctions/show.html.erb +++ b/app/views/auctions/show.html.erb @@ -23,8 +23,7 @@ <% if @auction.over? && @auction.bids? %>

Winner:

-

<% if current_user && @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

- +

<% if @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

<% end %>

Current Bid:

<%= number_to_currency(@auction.current_bid_amount) %> From 213ef39c3b1b69c6ea9f88dedac8e3f63fc97f08 Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Mon, 28 Dec 2015 08:22:00 -0500 Subject: [PATCH 5/8] Check a user is logged in before You Are Winner Even if the logged in user is not the winner of the auction, the check will crash if there is no logged in user. The check could also be fixed to return false if nobody is logged in, but will just make this change for now instead since I don't want to modify that method yet. --- app/views/auctions/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/auctions/show.html.erb b/app/views/auctions/show.html.erb index 83240bef..0dfddc38 100644 --- a/app/views/auctions/show.html.erb +++ b/app/views/auctions/show.html.erb @@ -23,7 +23,7 @@ <% if @auction.over? && @auction.bids? %>

Winner:

-

<% if @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

+

<% if current_user && @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

<% end %>

Current Bid:

<%= number_to_currency(@auction.current_bid_amount) %> From 3189a2327fce148d53c2457fbff7f3511aa8837e Mon Sep 17 00:00:00 2001 From: Alan deLevie Date: Thu, 17 Dec 2015 13:20:49 -0500 Subject: [PATCH 6/8] Failing spec for who won features on auction page. Added failing spec for closed, bidless auction. --- .../bidder_interacts_with_auction_spec.rb | 25 +++++++++++++++++++ spec/support/feature_helpers.rb | 6 +++++ 2 files changed, 31 insertions(+) diff --git a/spec/features/bidder_interacts_with_auction_spec.rb b/spec/features/bidder_interacts_with_auction_spec.rb index 350c7bc4..cd30eec3 100644 --- a/spec/features/bidder_interacts_with_auction_spec.rb +++ b/spec/features/bidder_interacts_with_auction_spec.rb @@ -194,6 +194,31 @@ end end + scenario "Viewing auction page for a closed auction" do + create_closed_auction + auction = Presenter::Auction.new(@auction) + visit auction_bids_path(auction.id) + + expect(page).to have_content("Winning Bid (#{auction.current_bidder.name}):") + expect(page).not_to have_content("Current Bid:") + + expect(page).to have_content("Auction ended at:") + expect(page).not_to have_content("Bid deadline:") + end + + scenario "Viewing auction page for a closed auction with no bidders" do + create_closed_bidless_auction + auction = Presenter::Auction.new(@auction) + visit auction_bids_path(auction.id) + + expect(page).to have_content("Auction ended with no bids.") + expect(page).not_to have_content("Current Bid:") + + expect(page).to have_content("Auction ended at:") + expect(page).not_to have_content("Bid deadline:") + end + + scenario "Viewing bid history for a closed auction" do Timecop.scale(36000) do create_closed_auction diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index 58cfecb3..ac7388cc 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -9,6 +9,12 @@ def create_bidless_auction(end_datetime: Time.now + 3.days) return @auction, @bidders end +def create_closed_bidless_auction + create_bidless_auction + @auction.end_datetime = Time.now - 1.day + @auction.save +end + def create_current_auction @auction = FactoryGirl.create(:auction, :with_bidders) @bidders = @auction.bids From 30a9c442f52a300fd505ece484fbe2496734b912 Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Tue, 29 Dec 2015 16:57:56 -0500 Subject: [PATCH 7/8] Add a user_is_bidder method to Presenter::Auction Also, there were no tests for user_is_winning_bidder? method. Ooops. --- app/models/presenter/auction.rb | 4 +++ spec/models/presenter/auction_spec.rb | 44 +++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/app/models/presenter/auction.rb b/app/models/presenter/auction.rb index baa8c814..e42db273 100644 --- a/app/models/presenter/auction.rb +++ b/app/models/presenter/auction.rb @@ -68,6 +68,10 @@ def user_is_winning_bidder?(user) user.id == current_bid.bidder_id end + def user_is_bidder?(user) + bids.detect {|b| user.id == b.bidder_id } != nil + end + def html_description return '' if description.blank? markdown.render(description) diff --git a/spec/models/presenter/auction_spec.rb b/spec/models/presenter/auction_spec.rb index a01e1187..a2e50003 100644 --- a/spec/models/presenter/auction_spec.rb +++ b/spec/models/presenter/auction_spec.rb @@ -47,6 +47,46 @@ end end + describe '#user_is_winning_bidder?' do + let(:ar_auction) { FactoryGirl.create(:auction, :with_bidders) } + + context 'when the user is currently the winner' do + let(:bidder) { auction.bids.first.bidder } + + it 'should return true' do + expect(auction.user_is_winning_bidder?(bidder)).to be_truthy + end + end + + context 'when the user is not the current winner' do + let(:bidder) { auction.bids.last.bidder } + + it 'should return false' do + expect(auction.user_is_winning_bidder?(bidder)).to be_falsey + end + end + end + + describe '#user_is_bidder?' do + let(:ar_auction) { FactoryGirl.create(:auction, :with_bidders) } + + context 'when the user has placed a bid on the project' do + let(:bidder) { auction.bids.last.bidder } + + it 'should return true' do + expect(auction.user_is_bidder?(bidder)).to be_truthy + end + end + + context 'when the user has not placed a bid on the project' do + let(:bidder) { FactoryGirl.create(:user) } + + it 'should return false' do + expect(auction.user_is_bidder?(bidder)).to be_falsey + end + end + end + describe '#available?' do context 'when the auction has expired' do let(:ar_auction) { FactoryGirl.create(:auction, :closed) } @@ -75,7 +115,7 @@ describe '#over?' do context 'when the auction has expired' do - let(:ar_auction) { FactoryGirl.create(:closed_auction) } + let(:ar_auction) { FactoryGirl.create(:auction, :closed) } it 'should be true' do expect(auction).to be_over @@ -91,7 +131,7 @@ end context 'when the auction has not started' do - let(:ar_auction) { FactoryGirl.create(:future_auction) } + let(:ar_auction) { FactoryGirl.create(:auction, :future) } it 'should be false' do expect(auction).to_not be_over From 4c028684cc0b3888db84095c8e038c314b5ed5bb Mon Sep 17 00:00:00 2001 From: Jacob Harris Date: Wed, 30 Dec 2015 13:45:41 -0500 Subject: [PATCH 8/8] Fixed tests, views to match mockups The idea is to use the header to indicate the status of auctions --- app/views/auctions/_win_header.html.erb | 54 +++++++++++ app/views/auctions/show.html.erb | 17 ++-- spec/factories/auctions.rb | 11 ++- .../bidder_interacts_with_auction_spec.rb | 90 +++++++++++++++++-- spec/support/feature_helpers.rb | 2 +- 5 files changed, 148 insertions(+), 26 deletions(-) create mode 100644 app/views/auctions/_win_header.html.erb diff --git a/app/views/auctions/_win_header.html.erb b/app/views/auctions/_win_header.html.erb new file mode 100644 index 00000000..624999c2 --- /dev/null +++ b/app/views/auctions/_win_header.html.erb @@ -0,0 +1,54 @@ +<% if auction.over? %> + <% if auction.bids? %> + <% if current_user %> + <% if auction.user_is_winning_bidder?(current_user) %> +

+

You are the winner

+

Congratulations! We will contact you with further instructions.

+
+ <% elsif auction.user_is_bidder?(current_user) %> +
+

You are not the winner

+

Someone else placed a lower bid than you.

+
+ <% end %> + <% else %> +
+
+

Auction Now Closed

+
+
+ <% end %> + <% else %> +
+
+

Auction Now Closed

+

This auction ended with no bids.

+
+
+ <% end %> +<% elsif auction.available? && auction.bids? && current_user && auction.user_is_bidder?(current_user) %> + <% if flash["bid"] %> +
+
+

Bid Submitted! You currently have the winning bid.

+

If your bid is selected as the winner, we will contact you with further instructions. View your bids »

+
+
+ <% elsif auction.user_is_winning_bidder?(current_user) %> +
+
+

You currently have the winning bid.

+

If your bid is selected as the winner, we will contact you with further instructions.

+
+
+ <% else %> +
+
+

You currently do not have the winning bid.

+

You must enter a new lower bid to win this auction.

+
+
+ <% end %> +<%# display nothing if in the future %> +<% end %> diff --git a/app/views/auctions/show.html.erb b/app/views/auctions/show.html.erb index 0dfddc38..19b98c36 100644 --- a/app/views/auctions/show.html.erb +++ b/app/views/auctions/show.html.erb @@ -1,11 +1,4 @@ -<% if flash["bid"] %> -
-
-

Bid Submitted! You currently have the winning bid.

-

If your bid is selected as the winner, we will contact you with further instructions. View your bids »

-
-
-<% end %> +<%= render partial: 'win_header', locals: {auction: @auction} %>

« Back to open projects

@@ -22,10 +15,10 @@

<%= auction_status(@auction) %>

<% if @auction.over? && @auction.bids? %> -

Winner:

-

<% if current_user && @auction.user_is_winning_bidder?(current_user) %>You are the winner!<% else %><%= @auction.current_bid.bidder_name %><% end %>

+

Winning bid (<%= @auction.current_bidder_name %>):

+ <% else %> +

Current bid:

<% end %> -

Current Bid:

<%= number_to_currency(@auction.current_bid_amount) %> <%= link_to( @@ -35,7 +28,7 @@ %>

-

Bid deadline:

+

<% if @auction.over? %>Auction ended at:<% else %>Bid deadline:<% end %>

<%= @auction.end_datetime.strftime("%m/%d/%Y at %I:%M %Z") %>

BID » diff --git a/spec/factories/auctions.rb b/spec/factories/auctions.rb index e5d49aa7..69eca678 100644 --- a/spec/factories/auctions.rb +++ b/spec/factories/auctions.rb @@ -9,13 +9,16 @@ trait :with_bidders do after(:build) do |instance| - (1..rand(10)+1).each do |i| - amount = 3499 - (100 * i) - rand(30) - instance.bids << FactoryGirl.create(:bid, auction: instance, amount: amount) + Timecop.freeze(instance.start_datetime) do + Timecop.scale(3600) + (1..4).each do |i| + amount = 3499 - (20 * i) - rand(10) + instance.bids << FactoryGirl.create(:bid, auction: instance, amount: amount) + end end end end - + trait :closed do end_datetime { Time.now - 1.day } end diff --git a/spec/features/bidder_interacts_with_auction_spec.rb b/spec/features/bidder_interacts_with_auction_spec.rb index cd30eec3..310cae9e 100644 --- a/spec/features/bidder_interacts_with_auction_spec.rb +++ b/spec/features/bidder_interacts_with_auction_spec.rb @@ -85,7 +85,7 @@ click_on("Submit") # returns us back to the bid page - expect(page).to have_content("Current Bid:") + expect(page).to have_content("Current bid:") expect(page).to have_content("$800.00") end @@ -103,7 +103,7 @@ click_on("Submit") # returns us back to the bid page - expect(page).to have_content("Current Bid:") + expect(page).to have_content("Current bid:") expect(page).to have_content("$999.00") expect(page).to have_content("You currently have the winning bid.") end @@ -194,25 +194,97 @@ end end - scenario "Viewing auction page for a closed auction" do + scenario "Viewing auction page for a closed auction where a user is not authenticated" do + create_closed_auction + auction = Presenter::Auction.new(@auction) + visit auction_path(auction.id) + + expect(page).to have_css('.usa-alert-info') + expect(page).to have_css('.auction-alert') + expect(page).to have_content("Winning bid (#{auction.current_bidder_name}):") + expect(page).not_to have_content("Current bid:") + + expect(page).to have_content("Auction ended at:") + expect(page).not_to have_content("Bid deadline:") + end + + scenario "Viewing auction page for a closed auction where authenticated user is the winner" do create_closed_auction + + visit "/" + sign_in_bidder + + bid = nil + Timecop.freeze(@auction.end_datetime) do + bid = @auction.bids.create(bidder_id: @bidder.id, amount: 1) + end + expect(bid).to be_valid + @auction.reload + auction = Presenter::Auction.new(@auction) - visit auction_bids_path(auction.id) - expect(page).to have_content("Winning Bid (#{auction.current_bidder.name}):") - expect(page).not_to have_content("Current Bid:") + expect(auction.current_bidder_name).to eq(@bidder.name) + visit auction_path(auction.id) + + expect(page).to have_css('.usa-alert-success') + expect(page).to have_content("You are the winner") + expect(page).to have_content("Winning bid (#{auction.current_bidder_name}):") + expect(page).not_to have_content("Current bid:") expect(page).to have_content("Auction ended at:") expect(page).not_to have_content("Bid deadline:") end + scenario "Viewing auction page for a closed auction where authenticated user is not the winner" do + create_closed_auction + + visit '/' + sign_in_bidder + + auction = Presenter::Auction.new(@auction) + + bid = nil + Timecop.freeze(@auction.start_datetime) do + bid = @auction.bids.create(bidder_id: @bidder.id, amount: 3498) + end + expect(bid).to be_valid + + visit auction_path(auction.id) + + expect(page).to have_css('.usa-alert-error') + expect(page).to have_content("You are not the winner") + expect(page).to have_content("Winning bid (#{auction.current_bidder_name}):") + expect(page).not_to have_content("Current bid:") + + expect(page).to have_content("Auction ended at:") + expect(page).not_to have_content("Bid deadline:") + end + +scenario "Viewing auction page for a closed auction where authenticated user has not placed bids" do + create_closed_auction + auction = Presenter::Auction.new(@auction) + + visit '/' + sign_in_bidder + + visit auction_path(auction.id) + + expect(page).to_not have_css('.usa-alert-error') + expect(page).to_not have_content("You are not the winner") + expect(page).to have_content("Winning bid (#{auction.current_bidder_name}):") + expect(page).not_to have_content("Current bid:") + + expect(page).to have_content("Auction ended at:") + expect(page).not_to have_content("Bid deadline:") + end + scenario "Viewing auction page for a closed auction with no bidders" do create_closed_bidless_auction auction = Presenter::Auction.new(@auction) - visit auction_bids_path(auction.id) + visit auction_path(auction.id) - expect(page).to have_content("Auction ended with no bids.") - expect(page).not_to have_content("Current Bid:") + expect(page).to have_content("This auction ended with no bids.") + expect(page).to have_content("Current bid:") expect(page).to have_content("Auction ended at:") expect(page).not_to have_content("Bid deadline:") diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index ac7388cc..0ad67171 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -43,7 +43,7 @@ def sign_in_bidder end def create_authed_bidder - @bidder = FactoryGirl.create(:user, github_id: current_user_uid) + @bidder = FactoryGirl.create(:user, github_id: current_user_uid, name: 'Doris Doogooder') end def show_page