Skip to content

Conversation

@aribray
Copy link

@aribray aribray commented May 10, 2019

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

Question Answer
Each team member: what is one thing you were primarily responsible for that you're proud of? Mina: I was responsible for some of the model tests. This has been a pain point for me so I’m really proud that I was able to learn about relationships and dependencies. // Amy: Real proud of the homepage animation and the checkout methods and testing. // Ari: Nested form! The create new products form allows you to select multiple existing categories, but also to create a new category while creating a new product. It took a long time to figure out (thanks, Chris), but I think it's cool that one form was talking to two different tables in the database. :D // Jessica: One thing that I worked on primarily was the cart and figuring out how an order_id persists in session, as well as figuring out how to do logic with quantity in stock, etc.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on? Mina: I am still having a hard time understanding edge cases so if one of the product model tests could be reviewed and some suggestions for edge cases, that would be amazing. // Amy: I would really like feedback on the orders controller testing. // Ari: I'd like feedback on when to let go of a feature. Having a nested form makes the experience more enjoyable for the user, but at what point of struggling with it do you let it go and think about if the feature/work is holding the team back? Also just general feedback about how to do a nested form like that more efficiently. // Jessica: I think I would love feedback on places where I might've put code in the wrong place (M vs. V vs. C). I don't have a specific place to look at, but like I said above, I did a lot with the cart (orders controller/views), so maybe that would be a place to look? Sorry that's not more specific!
How did your team break up the work to be done? Everyone was really good at pulling items from the Trello board that they wanted to work on. Everyone had ownership for their own roles. Our communication was great. We touch based before working on any major features to ensure work wasn’t being duplicated. We used the Trello board as well which was extremely helpful because we could visually see which work was being done and what needed to be done.
How did your team utilize git to collaborate? We did an amazing job of everyone working on their own branches and the submitting a PR for that branch. Whoever had time would review the PR and the owner would deal with merge conflicts (if any) and merge into the master branch.
What did your group do to try to keep your code DRY while many people collaborated on it? PR reviews and implementing controller filters early so that people can build upon them for later features. We refactored early, so that we didn't have to go through and DRY up the whole project. We used view helpers as well.
What was a technical challenge that you faced as a group? There was an issue with a git merge that a TA had helped us figure out. The TA wasn’t quite sure what the issue was but that issue required us to go through a few commits to manually recover work.
What was a team/personal challenge that you faced as a group? We initially divided up the work without taking into account the relationship dependencies, and quickly regrouped and broke things down by feature, which worked really well.
What was your application's ERD? (include a link) Please see ERD card in the Trello https://trello.com/b/azyI5T1u/project-board
What is your Trello URL? https://trello.com/b/azyI5T1u/project-board
What is the Heroku URL of your deployed application? https://scamazondotcom.herokuapp.com/

aribray and others added 30 commits May 3, 2019 11:20
Add new review - involved creating nested routes and refactoring tests
finished product-update action and tests
Create order (involves orderitems and products)
amyesh and others added 29 commits May 9, 2019 21:44
All Products will only show products with quantity above 0
Button, Table, and Cart Styling
Little fixes (see commit messages for details)
@tildeee
Copy link

tildeee commented May 20, 2019

bEtsy

What We're Looking For

Manual testing

Workflow yes / no
Deployed to Heroku x
Before logging in
Browse all products, by category, by merchant x
Leave a review x
Verify unable to create a new product x
After logging in
Create a category x
Create a product in that category with stock 10 x
Add the product you created to your cart x
Add it again (should update quantity) just updates it to the new quantity!
Verify unable to increase quantity beyond stock x
Add another merchant's product x
Check out x
Check that stock was reduced x
Change order-item's status on dashboard x
Verify unable to leave a review for your own product x
Verify unable to edit another merchant's product by manually editing URL I totally can! :o) https://scamazondotcom.herokuapp.com/products/32
Verify unable to see another merchant's dashboard by manually editing URL x

Code Review

Area yes / no
Routes
No un-needed routes generated (check reviews) x
Routes not overly-nested (check products and merchants) x
Merchant dashboard and cart page use a non-parameterized routes (should pull merchant or cart ID from session) x
Controllers
Controller-filter to require login by default x
Helper methods or filters to find logged-in user, cart, product, etc x
No excessive business logic looks good, although the Orders Controller and Products Controller could use some refactoring/cleaning up and are erring towards "excessive business logic"
Business logic that ought to live in the model
Add / remove / update product on order You can pull validations into Order model, also you can put the logic about updating/modifying an order's orderitems into the Model
Checkout -> decrease inventory Logic in OrdersController that decreases quantity should be moved into OrderItem
Merchant's total revenue x
Find all orders for this merchant (instance method on Merchant) uses has_many :orderitems, through: :products relationship instead
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
Doesn't have this logic tested in Order, but otherwise hits all of the other required model tests
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
Doesn't have this logic tested in User (I like that I'm the test user though :) :) ), but otherwise hits all of the other required model tests
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)
x, mostly tests the positive cases
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
x, perfect!

Overall Feedback

Great work overall! You've built a fully functional web store from top to bottom. This represents a huge amount of work, and you should be proud of yourselves!.

I am particularly impressed by the way that y'all did the following things:

  • I see overall the code style being sophisticated and elegant, in a good way! Overall, the code style used cool conditionals, loops and iteration, Enumerable methods, etc. It maintained being readable, concise, short, and clean. Comments were few; there were only necessary comments used to communicate quirks to others (this is the right number of comments!).
  • Overall, I felt like the tests were solid and well done!
  • The view code and the way the view turned out was extremely well-done and effective! I had a lot of fun playing around with the site.

I do see some room for improvement around:

  • The next push is to pull some of the code out of the controllers into models. We can see parts where there's a lot of chaining order.orderitems.product. ... is a good place to start!
  • There were parts of authorization that I feel were lacking in either testing and sometimes also implementation

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants