Blue Yourself - Addie, Bo, Janice, Rana

[add2point71dots]

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
How did your team break up the work to be done?	We all did testing in various forms. Rana mostly focused the product (model, controller, and views) and seeding our data. Addie mostly worked with vendor/oauth and reviews (models, controller, and views). Bo did most of the work on the cart (orders, orderitems model, controller and views). Janice worked on the vendor fulfillment pages, categories (model, controller, and views), some cart functionality, and the vast majority of CSS. We all pitched in to debug each other's code, as well.
How did your team utilize git to collaborate?	We worked on our own branches, and then pushed our branches to github and used pull requests to merge them.
What did your group do to try to keep your code DRY while many people collaborated on it?	We had some helper methods and before actions, and fixtures in our tests. We definitely could have done more, but with the amount of time we had, we prioritized functionality.
What was a technical challenge that you faced as a group?	Figuring out how everything relates and which pieces of code will affect code that others were working on. We learned our lesson to run all the tests before merging a branch, even if we didn't think our branch affected other parts of the project. We also found it difficult to manage model relationships when the involved models are owned by more than one team member.
What was a team/personal challenge that you faced as a group?	We had a some challenges splitting up the work in a way that everyone was happy with, communicating exactly which pieces we were working on, and honoring agreements regarding how to divvy work and supporting one and another when said agreements were broken. But by the end, we all pulled together and made something we were happy with. A personal challenge was to not become too attached to any one part of the project, as well as find the right balance between learning opportunities and project completion.
What could your team have done better?	TDD - we started out strong writing tests first, but as we went along, we mostly wrote tests after the fact. Our code could definitely be DRY-er, too.
What was your application's ERD? (include a link)	This is the ERD that was gem generated from our project (though it doesn't represent the "many" side of relationships well, it seems: https://github.com/add2point71dots/betsy/blob/master/erd.pdf Here is an ERD that Rana made early on in our project where the relationships are more accurate but the fields have changed a bit: https://github.com/add2point71dots/betsy/blob/master/ERD-betsy.png
What is your Trello URL?	https://trello.com/b/Q83lG9BC/blue-yourself
What is the Heroku URL of your deployed application?	https://blueyourself.herokuapp.com/

[kariabancroft]

bEtsy

What We're Looking For

Feature	Feedback
Baseline
Appropriate Git Usage with all members contributing	Yes
Answered comprehension questions	Yes
Trello board is created and utilized in project management	Yes
Heroku instance is online	Yes
General
Nested routes follow RESTful conventions	Yes
oAuth used for User authentication	Yes - set up with Google. Neat!
Functionality restricted based on user roles	Yes - I like the way you've created the require_login_match method to ensure appropriate vendor login
Products can be added and removed from cart	Yes
Users can view past orders	Yes
Merchants can add, edit and view their products	Yes
Errors are reported to the user	Yes - the order fulfillment page is nice with the error descriptions as well as the overall errors
Order Functionality
Reduces products' inventory	Yes
Cannot order products that are out of stock	Yes
Changes order state	Yes
Clears current cart	Yes
Database
ERD includes all necessary tables and relationships	Yes
Table relationships	Schema looks good
Models
Validation rules for Models	Yes - lots of different types used
Business logic is in the models	Yes - though I still think you could do more. For ex, the total quantity and total price logic in the OrdersController should be done in a model rather than in the controller. The way order items are created could also be greatly simplified by using some sort of method that would check the types of things you are looking for.
Controllers
Controller Filters used to DRY up controller code	Yes - though some are used unnecessarily. Using a controller filter for a single controller action seems overkill - like the show_cart actions in the OrdersController
Testing
Model Testing	Yes
Controller Testing	Yes
Session Testing	Yes
SimpleCov at 90% for controller and model tests	Yes
Front-end
The app is styled to create an attractive user interface	Yes
The app layout uses Foundation's Grid	Yes - though I think that some of the pages like the product index would really be well-served to have foundation implemented
Content is organized with HTML5 semantic tags	Yes
CSS is DRY	No - there are a lot of duplicated declarations related to flex box stuff, font sizes, margins, etc. I think it is primarily based on the way you're using one class for one element, as opposed to considering using multiple classes for a single element to clean up your CSS.
Overall
	Using the template views seem like a nice idea but this causes you to duplicate a great deal of the "normal" application view stuff. If you changed the nav bar, then you'd have to change it in several other places. You should be using partials not full page templates to prevent this duplication
	See some other notes inline on the PR

[kariabancroft]

Here you're re-querying the database which is unnecessary. The above result should be stored in a variable so you don't do a duplicate query.

[kariabancroft]

Room to DRY up these conditionals since only the first line is unique within each statement.

[kariabancroft]

Seems like since each of these controller actions are the same, that you don't need separate controller actions for this action.

[kariabancroft]

Duplication here you can avoid by putting outside the conditional statement

[kariabancroft]

This is not appropriate for the controller and should all be within the model

[kariabancroft]

This drop down is cool! In the future - you could use "locales" and put this type of thing into a separate file so that it's not directly in your view

[kariabancroft]

The filtering generally should not be done within the view - but rather the controller (or even the model). Whatever order items you're looking for should be passed to the view - already filtered

[kariabancroft]

Might be good to create a view helper for this in the future so that you can format all prices the same way across all views without needing to duplicate this logic

[kariabancroft]

These tests seems to be using this hash throughout so it would be good to DRY this up and use a shared variable

[kariabancroft]

I like the way you're using this array to test multiple scenarios for the same outcome