BAE BAE BAE #59
BAE BAE BAE #59
Conversation
…ogged in merchant and not logged in users. Still need to write 2 tests here - for #create. Currently when you select no rating, getting a id cannot be for nil class error
…y of an existing orderproduct
Anders tests 2
…stance of OrderProduct. Add #increase_inventory method to Product model, to put inventory back in stock
…rderproduct if doesn't already exist
…d if one with the same product_id already exists
… change if adding the same product to an order. This is currently failing.
merging css
…uctsController Anders playing with order products controller
Lauren css
font stolen from the HP page
clickable pics
add total to cart
Tests final wave
Fixed a bug on receipt page, and wrote a missing test for OrdersController#receipt. 91% coverage
product page
Anders update
Fix little bug on order show page
bEtsyWhat We're Looking For
IMPORTANT: Whoever submitted the PR (and thus will get the notification about this feedback) should share this with their teammates. |
| @@ -0,0 +1,40 @@ | |||
| class CategoriesController < ApplicationController | |||
| before_action :must_be_logged_in, only: [:new, :create] | |||
There was a problem hiding this comment.
In general, when working with a controller filter that restricts user actions like require_login, best practice is to apply it to every action via the ApplicationController, and then mark exceptions one by one. This prevents you from forgetting one and leaving a hole in your site's security.
| def show(status: nil) | ||
| @paid_orders = @merchant.sort_orders_by_status("paid") | ||
| @complete_orders = @merchant.sort_orders_by_status("complete") | ||
| if params[:status] == "paid" || params[:status] == nil |
There was a problem hiding this comment.
I'm curious about the argument to this method. It looks like status is never read, and as far as I'm aware Rails will never pass arguments to your controller actions.
| has_many :products, through: :order_products | ||
| has_one :payment_info | ||
|
|
||
| scope :not_retired, -> { where(retired: false) } |
|
|
||
| self.order_products.each do |op| | ||
| if op.status != "complete" | ||
| incomplete += 1 |
There was a problem hiding this comment.
Instead of using a counter, you could immediately return false here, and change the last line to return true. That would short-circuit the check as soon as an incomplete order is found.
You could also reduce this whole method to
return order_products.where.not(status: "complete").empty?| end | ||
|
|
||
| describe "update_status" do | ||
| it "if entire order is complete, sets flash[:status] to success when order is saved with specific message, redirects merchant show page, and sets the orderproduct status and orders status to complete" do |
There was a problem hiding this comment.
What if the logged in merchant is not the seller of that product? What if no one is logged in?
| end | ||
| end | ||
|
|
||
| #Do we need tests for #create and #update here? |
There was a problem hiding this comment.
Yes. I also think there's a set of interesting cases you're missing around trying to edit, update or destroy a product when logged in a someone other that the seller.
| end | ||
|
|
||
| #TODO: what if there are no orderproducts? | ||
| # it "sets flash[:status] to failure is there are no orderproducts" do |
There was a problem hiding this comment.
I was about to leave a comment with the same text as this TODO. This case is interesting.
|
|
||
| #TODO: | ||
| describe "increase_inventory" do | ||
|
|
|
|
||
| describe "decrease_inventory" do | ||
| let(:tricycle) { products(:tricycle) } | ||
| it "decreases the inventory by the given quantity" do |
There was a problem hiding this comment.
What if you try to decrease the inventory by 0, or a negative number? What if you try to decrease it past the number in stock?
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