Alice, Noor, Schanen, Blaine Fire & Earth AloeBae bEtsy #73
Conversation
Product/views
Skr/fixtures
Validations branch
modifed order summary page after checkout
Bga/price float migration
Fixed dash order totals
There was a problem hiding this comment.
bEtsy
Functional Requirements: Manual Testing
| Workflow | yes / no |
|---|---|
| Before logging in | |
| Browse all products, by category, by merchant | ✔️ |
| Leave a review | ✔️ |
| Verify unable to create a new product | ✔️ |
| After logging in | |
| Create a category | ✔️, although it's as little hard to find where to add a category. |
| Create a product in that category with stock 10 | ✔️ |
| Add the product you created to your cart | ✔️ |
| Add it again (should update quantity) | ✔️ |
| Verify unable to increase quantity beyond stock | ✔️ |
| Add another merchant's product | ✔️ |
| Check out | ✔️ |
| Check that stock was reduced | ✔️ |
| Change order-item's status on dashboard | |
| Verify unable to leave a review for your own product | ✔️ |
| Verify unable to edit another merchant's product by manually editing URL | |
| Verify unable to see another merchant's dashboard by manually editing URL | ✔️ |
Major Learning Goals/Code Review
| Criteria | yes / no |
|---|---|
| 90% reported coverage for all controller and model classes using SimpleCov | |
| Routes | |
No un-needed routes generated (check reviews) |
|
| Routes not overly-nested (check products and merchants) | ✔️, no triple-nested routes. |
| Merchant dashboard and cart page use a non-parameterized routes (should pull merchant or cart ID from session) | ✔️ |
| Controllers | |
| Controller-filter to require login is applied to all merchant-specific actions (update/add item, add category, view merchant dashboard, etc.) - filter method is not duplicated across multiple files | ✔️ |
| Helper methods or filters to find logged-in user, cart, product, etc | ✔️ |
| No excessive business logic | ✔️ |
| Business logic that ought to live in the model | |
| Add / remove / update product on order | ✔️ |
| Checkout -> decrease inventory | ✔️ |
| Merchant's average revenue | ✔️ |
Find all orders for this merchant (instance method on Merchant) |
|
| Selected Model Tests | |
| Add item to cart: - Can add a good product - Can't add a product w/o enough stock - Can't add a retired product - Can't add to an order that's not in cart mode - Logic specific to this implementation |
|
| Get orders for this merchant: - Includes all orders from this merchant - Doesn't include orders from another merchant - Orders are not included more than once - Does something reasonable when there are no orders for this merchant |
|
| Selected Controller Tests | |
| Add item to cart: - Empty cart (should be created) - Cart already exists (should add to same order) - Product already in cart (should update quantity) - Bad product ID, product is retired, quantity too high, or something like that (error) |
|
| Leave a review: - Works when not logged in - Works when logged in as someone other than the product's merchant - Doesn't work if logged in as this product's merchant - Doesn't work if validations fail |
Code Style Bonus Awards
Was the code particularly impressive in code style for any of these reasons (or more...?)
| Quality | Yes? |
|---|---|
| Descriptive/Readable | ✅ |
| Logical/Organized | ✅ |
Overall Feedback
A few weird notes. If I edit another merchant's products suddenly their orders show up in my dashboard. Granted I shouldn't be able to edit their product, but this was unexpected. I suddenly had an order for a hairy chest, which was amusing.
Areas that aren't working: Merchant Dashboard doesn't calculate total earnings or other numbers beyond number of orders and orders in each category. You also don't have much in the way of authorization to prevent malicious merchants from hacking other products and taking them over.
You also have very little in the way of effective testing. I think you all observed how this caused issues when you tried to merge in features and discovered things weren't working. More through testing could help reduce team conflict and make the process of absorbing new features smoother. I would have sacrificed the reviews feature to have more confidence in the code via tests.
Positive Notes
That said, your bEtsy project works pretty well in the browser. The site looks good and has the core functionality. You made very good use of filters and your oauth works really well. This is a solid project submission.
bEtsy is a huge project on a very short timeline, and this feedback should not at all diminish the magnitude of what you've accomplished. Keep up the hard work!
Only the person who submitted the PR will get an email about this feedback. Please let the rest of your team know about it.
| Rails.application.routes.draw do | ||
| # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html | ||
| root to: 'homepages#homepage' | ||
| resources :homepages, only: [:index] |
There was a problem hiding this comment.
Since you're using the root path for this controller you don't need this.
| resources :homepages, only: [:index] |
| delete '/logout', to: 'merchants#destroy', as: 'logout' | ||
|
|
||
| resources :products | ||
| resources :merchants |
There was a problem hiding this comment.
Do you need all the CRUD routes for merchants?
| resources :homepages, only: [:index] | ||
|
|
||
| resources :products do | ||
| resources :order_items, only: [:create, :destroy] |
There was a problem hiding this comment.
You don't seem to be using the nested destroy route for order items.
| patch '/orders/:id/cancel', to: 'orders#cancel', as: 'cancel' | ||
|
|
||
| resources :reviews, only: [:create, :update] | ||
| resources :order_items |
There was a problem hiding this comment.
Again you don't need all the CRUD routes here?
| @@ -0,0 +1,15 @@ | |||
| joinone: | |||
There was a problem hiding this comment.
In this fixture you're missing inventory, description, and photo.
| post product_order_items_path(products(:product_one)) | ||
| id = Product.find_by_id(products(:product_one).id) | ||
| expect { | ||
| delete product_order_item_path(id)}.must_change 'OrderItem.count', -1 |
There was a problem hiding this comment.
Your form requires the quantity you're adding to the product, and you need both the product and order ids.
| post product_order_items_path(products(:product_one)) | |
| id = Product.find_by_id(products(:product_one).id) | |
| expect { | |
| delete product_order_item_path(id)}.must_change 'OrderItem.count', -1 | |
| post product_order_items_path(products(:product_one).id), params: { order_item: { quantity: 1 } } | |
| product_id = Product.find_by_id(products(:product_one).id).id | |
| id = OrderItem.find_by(product_id: product_id, order_id: session[:cart_id]).id | |
| expect { | |
| delete product_order_item_path(product_id, id) | |
| }.must_change 'OrderItem.count', -1 |
Assignment Submission: bEtsy
CHECK THE PULL REQUEST TEMPLATE IN CODE
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions. These should be answered by all members of your team, not by a single teammate.
Reflection