Adoptsy #27
Adoptsy #27
Conversation
bEtsyWhat We're Looking For
Only the person who submitted the PR will get an email about this feedback. Please let the rest of your team know about it. |
| end | ||
|
|
||
| describe "create" do | ||
| it "can create a product" do |
There was a problem hiding this comment.
It doesn't look like you've included any authorization testing, which I would say is a problem! Remember, you need to test to see if people who aren't logged in get bounced from these actions
There was a problem hiding this comment.
Hi Devin,
The Oauth works fine for me! Just tested it in Chrome and Safari. As far as I know it worked for my teammates too. Did you try clearing your cookies, using incognito, or logging in via Safari?
There was a problem hiding this comment.
I will certainly give it another try, but yeah, I did try clearing cookies and using incognito. I'll see if Safari does any favors for me.
Side note, Oauth login is different from the authorization testing I'm talking about in the top comment in this thread.
| get "/fulfillment", to: "orders#fulfillment", as: "get_orders" | ||
| get "/fulfillment/paid", to: "orders#paid", as: "paid_orders" | ||
| get "/fulfillment/pending", to: "orders#completed", as: "completed_orders" | ||
| get "/fulfillment/cancelled", to: "orders#cancelled", as: "cancelled_orders" |
There was a problem hiding this comment.
This might be collapsed to a single route:
get "/fulfillment/:method", to: "orders#fulfillment", as: "manage_fulfillment_orders"
| @orderproducts = Order.find_orderproducts(@current_user, nil) | ||
| @total_revenue = Order.products_sold_total(@current_user, @orderproducts) | ||
| @count = Order.count_orders(@current_user, nil) | ||
| else |
There was a problem hiding this comment.
@jfahmy , so when I look at these 4 controller actions, I see that most of the code is identical. I think that the right way to manage this set of pages might be to use a single rout with a "method" param that allows you to choose between a few different options. The :method would control the kind of error, and could be passed to the find_orderproducts and count_orders methods directly. for any of these that render views, a render command in a switch statement would work well.
| .user-products-img img{ | ||
| width: 200px; | ||
| height: auto; | ||
| padding: 1em; |
There was a problem hiding this comment.
@marshi23 This is a good example of something I would have moved into a model-specific .scss file. This only ends up styling things on product pages, so having it appear anywhere else as part of the css is unnecessiary, and makes looking for page specific CSS really hard.
|
|
||
| it "will update a product with a valid post request" do | ||
| tan = users(:tan) | ||
| perform_login(tan) |
There was a problem hiding this comment.
@Naltrexone This two-line element appears a lot in this set of tests, and is a good candidate for a let command.
bEtsy
Congratulations! You're submitting your assignment! These comprehension questions should be answered by all members of your team, not by a single teammate.
Comprehension Questions