-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/auto bid #15
base: master
Are you sure you want to change the base?
Feature/auto bid #15
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,13 +32,45 @@ defmodule RebayApiWeb.BidController do | |
|> Map.put("uuid", Ecto.UUID.generate()) | ||
|
||
with {:ok, %Bid{} = bid} <- UserItem.create_bid(bid_params) do | ||
handle_auto_bids(item.id) | ||
|
||
conn | ||
|> put_status(:created) | ||
|> put_resp_header("location", Routes.item_bid_path(conn, :show, item.uuid, bid)) | ||
|> render("show.json", bid: bid) | ||
end | ||
end | ||
|
||
defp handle_auto_bids(item_id) do | ||
item_id | ||
|> UserItem.get_auto_bids_by_item | ||
|> run_auto_bids(item_id) | ||
|
||
case length(item_id |> UserItem.get_auto_bids_by_item) do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you reduce duplication and improve readability by assigning the collection of bids to a variable? |
||
0 -> nil | ||
1 -> nil | ||
_ -> handle_auto_bids(item_id) | ||
end | ||
end | ||
|
||
defp run_auto_bids(bids, item_id) do | ||
bids | ||
|> Enum.each(fn bid -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe Elixir pattern-matching can be leveraged recursively to the same effect, if you want to do this more idiomatically than with the use of |
||
last_bidders_id = UserItem.get_highest_bidder_user_id(item_id) | ||
price_increment = 100 | ||
next_bid_price = UserItem.get_highest_bid_price(item_id) + price_increment | ||
if bid.max_bid_price >= next_bid_price && bid.user_id != last_bidders_id do | ||
new_bid = %{ | ||
user_id: bid.user_id, | ||
item_id: item_id, | ||
bid_price: next_bid_price, | ||
uuid: Ecto.UUID.generate() | ||
} | ||
UserItem.create_bid(new_bid) | ||
end | ||
end) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The auto-bidding functionality may be worth extracting to a separate module. The controller should really just be concerned with handling requests. |
||
|
||
def show(conn, %{"uuid" => uuid}) do | ||
bid = UserItem.get_bid!(uuid) | ||
render(conn, "show.json", bid: bid) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,12 @@ defmodule RebayApiWeb.SessionControllerTest do | |
alias RebayApi.Repo | ||
alias RebayApi.Accounts.User | ||
alias RebayApi.TestHelpers | ||
|
||
test "redirects user to Google for authentication", %{conn: conn} do | ||
conn = get conn, "/auth/google?scope=email%20profile" | ||
assert redirected_to(conn, 302) | ||
end | ||
|
||
test "creates user from Google information", %{conn: conn} do | ||
users = User |> Repo.all | ||
assert Enum.count(users) == 0 | ||
|
@@ -17,8 +17,8 @@ defmodule RebayApiWeb.SessionControllerTest do | |
credentials: %{token: "fdsnoafhnoofh08h38h"}, | ||
info: %{ | ||
image: "some-image-url.foo", | ||
email: "[email protected]", | ||
first_name: "Travis", | ||
email: "[email protected]", | ||
first_name: "Travis", | ||
provider: "google", | ||
token: "foo", | ||
}, | ||
|
@@ -30,7 +30,7 @@ defmodule RebayApiWeb.SessionControllerTest do | |
|
||
users = User |> Repo.all | ||
[ user | _ ] = users | ||
|
||
assert Enum.count(users) == 1 | ||
assert get_resp_header(conn, "location") == ["#{Application.get_env(:rebay_api, :client_host)}/login/#{user.uuid}"] | ||
assert fetch_cookies(conn).cookies["session_id"] == "fdsnoafhnoofh08h38h" | ||
|
@@ -53,8 +53,8 @@ defmodule RebayApiWeb.SessionControllerTest do | |
credentials: %{token: "fdsnoafhnoofh08h38h"}, | ||
info: %{ | ||
image: invalid_image, | ||
email: "[email protected]", | ||
first_name: "Travis", | ||
email: "[email protected]", | ||
first_name: "Travis", | ||
provider: "google", | ||
token: "foo", | ||
}, | ||
|
@@ -67,4 +67,4 @@ defmodule RebayApiWeb.SessionControllerTest do | |
users = User |> Repo.all | ||
assert Enum.count(users) == 0 | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be nitpicky, but I think it makes for better readability to implement pipelines when the same message is being passed through. So passing in
item_id
to a function that returns a collection of bids that get passed torun_auto_bids
doesn't read as well to me as something more likeUserItem.get_auto_bids_by_item(item_id) |> run_auto_bids(item_id)
. Such a minor thing here, but I think it becomes more helpful with more complicated pipelines.