forked from AdaGold/betsy
-
Notifications
You must be signed in to change notification settings - Fork 14
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
cactus&clover | pauline&roshni - fire | mackenzie&leah - water #76
Open
scottzec
wants to merge
329
commits into
Ada-C14:master
Choose a base branch
from
scottzec:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… quantity through
…ers, updated routes
Form orderitem product into cart - not pulling quantity
Product validations
Orders controller and model updated
More reviews merchants tests
Mvl - orderitem creates orderitem from product show
orderitems talk to cart, need to add cancel/update orderitem
…ller methods, views, not currently working
Complete purchase form - still needs work, running into order empty error
…ev-mer-model-tests
…oved url from table
… see about footer not staying at bottom
Bootstrap pages 3
Bootstrap pages 4
Bootstrap pages 5
Bootstrap pages 6
dHelmgren
reviewed
Dec 8, 2020
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.
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 | ✔️ |
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 | don't let people even get to the form! prevent them from looking at someone else's form |
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) | ✔️ |
Merchant dashboard and cart page use a non-parameterized routes (should pull merchant or cart ID from session) | see comment |
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 total 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 |
✔️ |
Overall Feedback
Only the person who submitted the PR will get an email about this feedback. Please let the rest of your team know about it.
Comment on lines
+54
to
+70
@orderitem = Orderitem.new | ||
@orderitem.order_id = @cart.id | ||
@orderitem.product_id = product.id | ||
@orderitem.quantity = quantity | ||
@orderitem.shipped = false | ||
|
||
|
||
if @orderitem.save! | ||
flash[:success] = 'Product successfully added to cart!' | ||
redirect_to cart_path | ||
return | ||
else | ||
flash[:warning] = 'A problem occurred: could not add item to cart' | ||
redirect_back(fallback_location: root_path) | ||
return | ||
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 will create a ton of bloat data! why not just store info in session until you need to save all of it? (as in during a checkout?)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Assignment Submission: bEtsy
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