Thrillow

[kayxn23]

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?
Katherine - Shopping cart, many related models and controllers coming together to complete a single task, it was a collaborative effort that I learnt so much from.
Cassy - I'm proud of the fact that only the logged in merchant can view their own page. That took me awhile to figure out. Also having to click a button and filter the order info on the dashboard based on what was clicked.
Kay - I was primarily responsible for writing the tests for Product and Category, so I was proud of learning how to debug these tests when all of our files were merged and I understood how it was working and how to update the tests to fix them.
Danielle - Primarily responsible for model testing for validation & relationships, form views, and I'm most proud of the shopping cart view functionality & how it updates the cart!
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on?
Katherine - I would like feedback on how to write controller tests and routes.
Kay - Making sure that test that shouldn't pass when someone isn't signed in are working properly. I would also like targeted feedback on how to test the order#update method when it calls on another helper method. If the helper is tested well separately then do I need to make sure its working with the main method again.	I left some notes in the code
Cassy - Guests being able to do things that they shouldn't. I'm sure I missed a few things but i'm not sure what.	I left some notes in your tests, the biggest thing is that you need to verify that to update an order, the person has to "own" that order.
Danielle - How might i write model tests that use session. I'm a little unclear on how to do this when a lot of our @current_orders are all stored in session so when we are trying to capture if checkout was updating and existing order it created a new one because it references the session and doesn't find an order in the yml file.	You don't test session in model tests. The models technically don't even know about the web component, just the DB and their data. It's the controller's job to worry about session.
How did your team break up the work to be done?	Depending on the highest priority task at the moment Scrum Master would ask and delegate who wanted to work on something, or if someone had a particular interest on a specific feature they would take it.
How did your team utilize git to collaborate?	We each created a branch for new features we worked on so that we would keep bugs out of the master. This made it possible for us to collaborate remotely.
What did your group do to try to keep your code DRY while many people collaborated on it?	We communicated with each other so that we could get rid of redundant code. At the end of the project phase we split up the responsibility to go through our files and delete comments we didnt need and delete routes and views we didn't need.
What was a technical challenge that you faced as a group?	The shopping cart in general was a big challenge. It was difficult to get used to referring to our join table (ex: OrdersItems join table connects Order and Product) it would have been better if we named it Item instead.
What was a team/personal challenge that you faced as a group?	Making decisions, and taking too long to do that. Too much planning was a big factor of that, in the beginning mostly.
What was your application's ERD? (include a link)
What is your Trello URL?	https://trello.com/b/TksWyeih/besty
What is the Heroku URL of your deployed application?	https://thrillow.herokuapp.com/products

[CheezItMan]

bEtsy

What We're Looking For

Feature	Feedback
Baseline
Appropriate Git Usage with all members contributing	Lots of commits and all members contributing
Answered comprehension questions	Check, I'm glad you saw that you overplanned at the start. I left some responses in your comprehension questions as well. Check the table above.
Trello board is created and utilized in project management	Check
Heroku instance is online	Check
General
Nested routes follow RESTful conventions	For the most part, your UsersController has some unused routes.
oAuth used for User authentication	Check
Functionality restricted based on user roles	Check
Products can be added and removed from cart	If I click twice on an add-to-cart button it remains only added once. Might be a good idea to either hide that button for products in the cart, or update the quantity.
Users can look up past orders by ID	Check
Merchants can add, edit and view their products	Check
Errors are reported to the user	Check
Order Functionality
Reduces products' inventory	Check
Cannot order products that are out of stock	Check
Changes order state	Placing an order moves a product to the state "paid", but I don't see a way as a merchant using the site to ship an order.
Clears current cart	Check
Database
ERD includes all necessary tables and relationships	Yes, I found it in your Trello board, but found it tough to read (fuzzy image)
Table relationships	They were in the ERD.
Models
Validation rules for Models	You are missing a lot of validations for Orders.
Business logic is in the models	You've got some good business logic here.
Controllers
Controller Filters used to DRY up controller code	Check
Testing
Model Testing	I left some notes, overall not bad testing. You do have some missing validations (lower priority)
Controller Testing	I also left some notes here, ways to DRY things up and a few things not covered.
Session Testing	Check, well done
SimpleCov at 90% for controller and model tests	78%, you got important stuff covered but you need more tests for your controllers.
Front-end
The app is styled to create an attractive user interface	Overall it looks pretty good
Content is organized with HTML5 semantic tags	Some non-semantic tags and a few odd <br /> tags.
CSS is DRY	You have some repeated rules which could be dried up a bit. Not a big concern.
Overall	Note please be more careful with the markdown table on the comprehension questions. The formatting is lost as-is. That makes it hard to read. I cleaned it up a bit. It's clear you didn't get to everything, but you did get the most critical things done and working. You have some issues with permissions, and you don't have great test coverage of your controllers as well. Overall, you hit the learning goals. Things to work on include effective testing, including permissions, making sure you don't define unused routes and being careful in your controllers for authorization issues (testing can help there).

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

[CheezItMan]

Do you use the /users/new route? What about `post '/users`` route to create users? I thought they happened through the sessions controller?

[CheezItMan]

Looks like you never resolved what to do about the cart of a user in this circumstance.

[CheezItMan]

I strongly suggest you create a method in your test_helper.rb file to log a user in, and then call that method in your tests when you want to log in the user. I would not do this piece over and over again.

[CheezItMan]

I also suggest grouping your tests by the type of user. 1 set for authenticated users, and one for guests, and maybe one for controller methods for which it doesn't matter.

Mixing and matching like this can get confusing and it's easier to miss one.

[CheezItMan]

Just a note, you can make the category route use the category name instead of the Id, especially if the category name is unique. That can make the URL more readable to the user.

[CheezItMan]

👍

[CheezItMan]

You also need to test these actions with guest users to verify redirection and the flash warning message.

[CheezItMan]

What about if you're logged in and it's not your dashboard?

[CheezItMan]

Can you do this? Shouldn't this be done with the SessionsController?

[CheezItMan]

This test seems to imply anyone can update an order...

I would think you would need to perform some actions to create an order (and have a session field set to record that this is that user's order, and then try to update it.

What you have in this test implies that anyone can send a patch request to update an order by it's id.

If so, I would love to go to your store with postman and place an expensive order!