Skip to content
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

Team Secretsy #74

Open
wants to merge 407 commits into
base: master
Choose a base branch
from
Open

Team Secretsy #74

wants to merge 407 commits into from

Conversation

tatsqui
Copy link

@tatsqui tatsqui 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? Evelynn: Orders Controller and some of the shopping cart logic. Erica: I'm proud of the dashboard page, particularly the logic behind filtering the order. Tatiana: I am proud of how many tests I wrote, and also some of the custom methods that I figured out, and using bootstrap to style the checkout page. Shirley: she was really proud of figuring the logic for shopping cart and making the logo.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on? Tatiana: I would like feedback on merchant controller, it needed quite a bit of functionality, I think that there were some model relationships that could have been more explicit, which would have made the logic in our custom methods for total revenue / revenue by status easier. I would like some insight around that, and the tests that were written for that model.
Erica: I would like feedback on the products show and merchants show views. I feel like I probably used to much logic within the view that can be simplified. Evelynn: Feedback on Orders CRUD Test & Controllers. Shirley: Shopping cart and OmniAuth.
How did your team break up the work to be done? We divided the work in the trello board with our label colors, and each person would say what they were working on and what other kinds of things they wanted to work on. We also prioritized what sections of the project needed to get done and folks volunteered.
How did your team utilize git to collaborate? Most of the work was done together in person during class time. We also always merged to master together, and mostly did each feature in a branch before merging it.
What did your group do to try to keep your code DRY while many people collaborated on it? For the most part, we made sure that we were working in separate files. In cases, where two or more people were working on similarly functionality and we worked together, side by side.
What was a technical challenge that you faced as a group? The shopping cart!!! There was so many requirements that needed to be coordinated between the validations, form validations, and then the corresponding tests.
What was a team/personal challenge that you faced as a group? Overcoming the many bugs in the test.
What was your application's ERD? (include a link) https://www.lucidchart.com/invitations/accept/8a11510a-9805-4079-9e7c-cb5d555dfa93
What is your Trello URL? https://trello.com/b/4qSaS7kO/secretsy
What is the Heroku URL of your deployed application? https://secretsy.herokuapp.com

@dHelmgren
Copy link

bEtsy

What We're Looking For

Manual testing

Workflow yes
Deployed to Heroku Yes
Before logging in
Browse all products, by category, by merchant yes
Leave a review Page breaks, DNE
Verify unable to create a new product yes
After logging in
Create a category Could not find
Create a product in that category with stock 10 yes
Add the product you created to your cart yes
Add it again (should update quantity) No
Verify unable to increase quantity beyond stock yes
Add another merchant's product yes
Check out yes
Check that stock was reduced yes
Change order-item's status on dashboard yes
Verify unable to leave a review for your own product yes
Verify unable to edit another merchant's product by manually editing URL yes
Verify unable to see another merchant's dashboard by manually editing URL yes

Code Review

--- | ---
Routes |
No un-needed routes generated (check reviews) | yes
Routes not overly-nested (check products and merchants) | yes
Merchant dashboard and cart page use a non-parameterized routes (should pull merchant or cart ID from session) | yes
Controllers |
Controller-filter to require login by default | no
Helper methods or filters to find logged-in user, cart, product, etc | yes
No excessive business logic | yes
Business logic that ought to live in the model |
Add / remove / update product on order | in a controller method
Checkout -> decrease inventory | no, in a controller method
Merchant's total revenue | yes
Find all orders for this merchant (instance method on Merchant) | yes
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 | No, covered in controller testings though
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 | missing all orders, doesn't include from another, duplicate test
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) | missing product already in cart
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 | missing some

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 you organized your routes, and kept a neat and consistent style across the whole site. You also did a great job of preventing people from seeing parts of the site that they weren't supposed to have access to.

I do see some room for improvement around keeping more logic in the model, particularly your cart logic, which could have happily lived in your Order model. Also, while you have a lot of thorough testing, take a look at some of the tests outlined above. There are a lot of cases to think about! Finally, your deployment has a couple of broken routes. Deploy earlier and try to spot check your whole site, things can go funny when they are live in unexpected ways!

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