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

Space & Time - Charlotte, Katie, Mair, Yoyo #34

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

Conversation

ktvoort123
Copy link

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

Prompt Response
Each team member: what is one thing you were primarily responsible for that you're proud of? Katie: I am proud of the logic behind order items and orders. Mair: worked on products controller, order and order_item. Worked mainly on placing an order and connected order, order_item, user, and products together. Charlotte: I worked primarily on product , establishing relationships and the testing related to product. Yoyo: Styling and functionality of homepage.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on? Yoyo: Reviews Controller, Model, routes and relationships. Mair: I would like targeted feedback on making the checkout process more smooth and intuitive, making the cart more user friendly. Katie: I would like feedback on how to DRY up the order items create method.
How did your team break up the work to be done? We primarily did pair programming, and met up twice a day -- in the morning and afternoon -- to update each other and merge branches when necessary. We did testing on our own because we felt it was more efficient that way.
How did your team utilize git to collaborate? We created new branches for features and merged them with the master branch as they were completed.
What did your group do to try to keep your code DRY while many people collaborated on it? Once the features were functional, we looked back at the methods to see if they could be simplified and DRYed up.
What was a technical challenge that you faced as a group? We had some troubles with merging for the coverage folder, which should not have even been merged because it was in the git ignore file, but we eventually fixed it.
What was a team/personal challenge that you faced as a group? Prioritization when we got to the finish line. Also, testing and fixing our tests when we merged.
What was your application's ERD? (upload this to Google Drive, change the share settings to viewable by everyone with a link, and include a link) https://drive.google.com/file/d/1TYqJm0cDw7m28z7JcEtdc4cdI1NEu-Qn/view?usp=sharing
What is your Trello URL? https://trello.com/b/ddENJLnt/petsy
What is the Heroku URL of your deployed application? https://p3tsy.herokuapp.com/

mheshmati-tech and others added 30 commits June 9, 2020 11:43
…ntroller, added partial forms for edit and show.
@jmaddox19
Copy link

jmaddox19 commented Jun 26, 2020

bEtsy

Functional Requirements: Manual Testing

Workflow yes / no
Deployed to Heroku ✔️
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 ✔️
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) ✔️ Yes, except the cart page still has a parameterized route even though the parameter isn't used. (Meaning it the url is "orders/8" even though the 8 isn't used for anything. It would be cleaner for the route to not include the 8 and for the path to be more accurate to what is happening, probably just "cart/".
Controllers
Controller-filter to require login by default ✔️
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 I don't specifically see merchant revenue listed. The total_revenue and filtered_revenue methods aren't being called from anywhere. Not a big deal :)
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
This code is in the controller but I see why. I don't see any logic or tests around changing behavior when a product is inactive. Maybe I'm missing it?
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
No tests for this in the model
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)
✔️ Great tests here!
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
✔️Yes, all but checking out without being signed in.

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Elegant/Clever
Descriptive/Readable
Concise
Logical/Organized

Overall Feedback

Before I get into all the positive stuff, I wanted to name a little but I found: clicking the "add to cart" button on a product that has 0 stock results in a generic rails error page.

But forget about that! Because holy crappp! Great work y'all! Be so proud of what y'all did together! You built this whole dang thing and you did it while working in a group for the first time. So much coordinating and delegating and communicating! All remotely! 👏👏👏

Also y'all pulled through with such fun and impressive styling with so little time at the end!

Only the person who submitted the PR will get an email about this feedback. Please let the rest of your team know about it.

end
@product.user_id = @logged_user.id
if @product.valid? && @product.photo_url.empty?
@product.photo_url = "https://lh3.googleusercontent.com/0hJSje8_Z034MV_zDnQFGVt1Id-P21tDb3AP-J7WFtH0ITgL47YvUV642kZg1fXxtZaO47A14pRjWP2tG-2AlHm7jst2i8mB-yZQPX0I67SB4X9iJs4Co9lge5RlgsMoK8BeP1pXNtKuQkneNuBrxPr-x6n2KpVj7tA_E4k1krdZPj6Z_kZT8XzNIN_ztY3rD9HdwInpgaQ7HYImlTdGo_A50RoLEJ-7sK2_gFJAbSLIwnxEAAcaw7ELKxhO0p2QUbBk8Qwp3GnleViM2LVIUIQMcQDt7JvcKgPWH1DpNYpv8GRitVPOIBOt4mVvsVSJ5I6iBXU_7Tz1b1HQC97OajROBAWNSYryi_wqrac6MjfwFS0VbRxiAthPtUxApWpwTmxSF8LTqtUh2MQfLnOGp4wnX-CkfIQzqazG20xxXvsv24iLE1PZwjwkFb2vE8fglhUr0PAvEHseJz7D1PJwt5MtH38gdDPTK8o8WhJ1R4vGaiCqcekvR6OUGy4DYUEjW-zd3psZpwuQLxPLlgefvSvL4lErgVrxV8bg61cn7WcQ2Avev4L36DAS40RyY0cqInt_CJ-9Ovp3oRPTwTp0EwEL2XmHHKoW8yaSgJloyjAzWT9SHAAnoAoTjHWeHvXot8qdWvT8dwmq-LRsR_-cpE5-qNo2ejrkDUrCD5k7R35r5F_TKXLG9CX-Tkrf7g=w600-h512-no?authuser=0"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal but this is a good example of where defining a constant (ALL_CAPS) variable at the top of the file can be used to make the code cleaner.

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