Carets Power Pup - Christiane, Kayla, Shaunna, Victoria #69
Conversation
Merchant order view
Rework show page and form
Rework show
…lment and order views to allow marking an order as shipped
fulfillment link added to nav when signed in
…n controller page)
minor changes to picture for show page
Product status
… of flash notifications
…items for a specific order
Shopping cart titles centered
Add slogan to nav
Review seeds
Clickable merchant categories
makes dates pretty
Remove slogan on medium and small screens
Add second pagination at bottom of page and style
| @revenue_by_status[a_status] = orderitems.sum {|orderitem| orderitem.order.status == a_status ? (orderitem.quantity * orderitem.product.price) : 0} | ||
| end | ||
| @title = "Fulfillment" | ||
| else |
There was a problem hiding this comment.
this if/else case is redundant with your other controller filters; it will never hit this "else" block because it already checks if you're the right merchant logged in with find_merchant and account_owner?
| end | ||
|
|
||
| def update_quantity | ||
| if @order_item |
There was a problem hiding this comment.
in this controller, you check for the truthiness of @order_item a couple of times, and if @order_item is not truthy, it shows a specific (but repeated) error message and redirection. You can handle this in the controller filter find_orderitem that you made and reduce duplicate logic.
Same goes for a couple of the other instance variables in this controller where it makes sense
| @@ -0,0 +1,144 @@ | |||
| class ProductsController < ApplicationController | |||
| before_action :find_product, only: [:show, :edit, :update, :retire] | |||
| before_action :find_merchant, except: [:index, :destroy] | |||
There was a problem hiding this comment.
You don't assign @merchant on the index method, so the check on line 8 would never happen anyway and @merchant will always be not truthy. :) Also, you don't have a destroy action in this controller?
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.Overall, the app is great and functional and pretty bug-free! Here are just a few notes about the project to make it EVEN BETTER. (I have comments on the code when it applies)
Pretty nifty stuff with the regex you found for validation and the us_state helper you made Great job on the testing! It's thorough (with the exception of the skipped tests ;) ) but overall I'm really happy with it |
| validates :rating, presence: true | ||
| validates :rating, numericality: { only_integer: true } | ||
| validates :rating, numericality: { greater_than_or_equal_to: 1 } | ||
| validates :rating, numericality: { less_than_or_equal_to: 5 } |
There was a problem hiding this comment.
You can collapse this to one line:
validates :rating, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: 5 }
The only way this matters is because otherwise you get multiple entries on flash that talk about the errors on numericality on review. I've attached screenshots of before the suggestion and after the suggestion
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