Hala/Leah (Space) & Hannah/Jocelyn (Time)

[wangjoc]

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?	Hala - I was primarily responsible for Product, I am proud of the methods and tests I wrote, also proud of how I connected Product to the proper Category, it took some time to figure out, another thing I enjoyed was working on our seed data; Hannah - I was primarily responsible for all aspects of the merchant and the testing that followed it. I am proud of the communication, the sticking to schedules and the git merge conflict and git over all challenges I have overcome.; Leah - I was primarily responsible for views and front end style and also for some testing. I am proud of the ways we communicated as a team to get so much of the requirements accomplished and how well we worked together!; Jocelyn - I was primarily responsible for Orders and OrderItems. I enjoyed trying to figure out how to use session to keep track of the shopping cart and learning how to create relationships between OrderItems and Orders.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on?	Hala - it would be great to get some feedback on how to better join tables, the way we did it works, but I would love to know if there was a better way to do it. I would love feedback on our awesome seeded data as well :) ; Hannah - I would like to get feedback on the methods and if the testing were sufficient to cover any edge or nominal cases that I didn’t see to cover. ; Leah - I’m interested to hear any feedback on user experience as well as the separation of logic into the model vs. in the view or controller. ; Jocelyn - Any feedback on the Orders or OrderItems controllers, models, and related tests would be appreciated! The way we interpreted the meaning of pending orders meant that we didn’t create an order until the user had made adjustments to their cart beforehand and confirmed it, therefore we ended up having most of that logic in the controller. It would be helpful to get some feedback on how we could move this logic to be in the model.
How did your team break up the work to be done?	On the first day, we all collaborated in creating ERD for the relationships, and Trello board to assign tasks. We all talked about what enjoy the most (front/back end) and divided the work accordingly. Hala, Jocelyn and I were doing the backend, Leah was doing the front end. Jocelyn also floated around filling anything that was missing and adding things. We utilized slack to do handoffs and daily standups.
How did your team utilize git to collaborate?	We mostly created our own branches to do work for the day and tested and pushed to master at the end of our shifts. At first I experienced conflict aton as I didn’t push and pull often. As the days passed by communicating our git standards and hygiene polished, we all were able to adapt the habit of pulling and pushing as often as possible which led to great hand offs and preventing merge conflicts.
What did your group do to try to keep your code DRY while many people collaborated on it?	Taking ownership of our individual parts helped a lot, another was refactoring teammates code after we were done with our own part.
What was a technical challenge that you faced as a group?	We ran into a lot of problems with git and overriding each other’s work at the beginning. We would also sometimes overlap in the work we were doing because a method might interact with multiple models. We were able to straighten out some of these issues by having a clearer segregation of duties that we would go over during standups. We also used standup meetings to demo the work that we did so that our teammates would be aware of any major changes. Another way we communicated major changes was to sending a summary of the work performed through the group slack after pushing changes to the master.
What was a team/personal challenge that you faced as a group?	We worked really well together as a group. Despite sometimes having git issues and work getting reverted or erased, everyone was positive and incredibly supportive of one another as we navigated through working remotely together. One thing we worked around as a team was having different schedules; we were able to decide on some handoff strategies that were effective for us as a group and everyone got their scheduling needs met.
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://app.lucidchart.com/invitations/accept/ff237cbe-c951-48ab-bc12-6c789a7e4e07
What is your Trello URL?	https://trello.com/b/aK0F0J9l/regretsy
What is the Heroku URL of your deployed application?	https://blooming-wildwood-52941.herokuapp.com/

[beccaelenzil]

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)	✔️stock changes once you check out, works for me!
Verify unable to increase quantity beyond stock	✔️
Add another merchant's product	✔️
Check out	✔️
Check that stock was reduced	✔️I see that you can no longer see products with zero quantity.
Change order-item's status on dashboard	✔️I'm able to retire the product and I no longer see it when browsing products, but there's not indication on the dashboard of this changed status
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	Your code is very well tested in general, but your coverage is a little low for products_controller, products model, reviews controllers, and merchants controller. Take a look at the simple cov report. It seems that you've missed testing some of the search functionality and a couple failure cases.
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)	✔️
Controllers
Controller-filter to require login by default	✔️
Helper methods or filters to find logged-in user, cart, product, etc	✔️See in-line comment about Order controller filter
No excessive business logic	See in-line comments for Order
Business logic that ought to live in the model
Add / remove / update product on order	See comments above
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	Missing test for validation.

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

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

[beccaelenzil]

Hala - Your seeded data is AMAZING. I'm not sure what suggestions I have around the categories_products join table. It looks good to me! Happy to have a conversation about it.

Hannah - The testing is quite thorough. There are a couple failure cases that are missing tests that are shown by simple cov (ex. Merchant_controller: could not create a new user account, Product model and product: search).

Leah - The user experience was pretty flawless. I commented on one user experience issue in the 'retire product' feature. There is no indication that the product is retired -- how do you un-retire it?

Jocelyn - Ack, sorry! I didn't read this comment until after I left comments about business logic in the controller. I understand now why this logic is in the controller. When you're dealing with session it makes sense to put that logic in the controller. Sometimes you can write controller helper methods if it's getting overly businessy. Looking back, it still seems that the order revenue calculation could be model method. Correct me if I'm wrong. Happy to talk about it more!

[beccaelenzil]

It looks like you've doubled up your work with a find_order controller filter and this code.

[beccaelenzil]

This logic looks "businessy". Could you do some of this work in a custom model method?

[beccaelenzil]

This code looks just like what you have on line 34. Putting this in a model method would help DRY up you code.

[beccaelenzil]

Again this seems pretty businessy. I wonder if it could be put in the model.

[beccaelenzil]

Consider whether you might be able to use a partial view for these 3 chunks of code that are quite similar (products, categories, and merchants)