Queues - Allison, Sahana, Marisol, Tehut - Andy's Kloset

[sahanamurthy]

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?	Assigning individual tasks was a collaborative activity. At the beginning of the project, the team read through the user stories and divided them up into smaller increments. Following that, the group sat together to create an ERD and create controllers, models, and relationships between different objects. Having everyone involved in this foundation was a strong move, as it ensured each team member was familiar with the overall behaviors of the app. Team members discussed their strengths, weaknesses, and primary focuses, as well as how much bandwidth they had. If a team member had more time/bandwidth, they took on more tasks.
How did your team utilize git to collaborate?	Like champs. We separated branches by function. For example, the Order branch was used to work on things related to order, the merchant branch for merchant, etc. We only merged to master when tests were passing and the code written was fully functional.
What did your group do to try to keep your code DRY while many people collaborated on it?	The main way our group was able to keep our code dry was by dividing up tasks and communicating with one another. We were sure to use our stand-ups and Trello board to help outline individual tasks, so no two people were working on the same task. And when working on related tasks, our team reached out to each other with questions about functionality and code, so that we were able to build off of each other’s designs.
What was a technical challenge that you faced as a group?	A technical challenge that we faced was with rails testing. As writing tests in rails was a fairly new concept to us, it was not something that came easy for anyone. Our team ardently worked to learn and practice the art of rails TDD.
What was a team/personal challenge that you faced as a group?	The main challenge the group faced was centered around working remotely. There were several days when class was not in session, and days when team members had other priorities, that hindered collaboration. We found ourselves working on different things, and were unable to simply look up from our screens to ask each other questions. One way our team worked around that was by communicating remotely as much as possible. We posted questions/thoughts/concerns onto our slack channel, and even had several remote stand-ups!
What could your team have done better?	There is nothing our team could have actually ‘done better’. We worked well together. We were teammates, confidantes, and helped each other learn. Our team reached our (unanimously declared) primary goal for this project : to learn as much from this as possible. We were all able to walk away with a better understanding of rails relationships and testing.
What was your application's ERD? (include a link)	https://www.lucidchart.com/documents/edit/ea67e9aa-eab7-401b-a9c9-4747c3b2b927
What is your Trello URL?	https://trello.com/b/WUaZXzsE/andy-s-kloset-betsy
What is the Heroku URL of your deployed application?	https://andys-kloset.herokuapp.com/

[PilgrimMemoirs]

bEtsy

What We're Looking For

Feature	Feedback
Baseline
Appropriate Git Usage with all members contributing	Well done - nice distribution of tasks and
Answered comprehension questions	Well Done - I agree, your team had a great dynamic, was well organized, communicative and did all that you could do. Great work.
Trello board is created and utilized in project management	Well Done
Heroku instance is online	Well Done
General
Nested routes follow RESTful conventions	❗️Sometimes (see below)
oAuth used for User authentication	Well Done
Functionality restricted based on user roles	❗️ Some - I am still able to access the create a new product page when not logged in. I was able to click the 'add new product' button on a product's show page after I logged out. However, it did not save when I tried to submit the form. No error message was displayed why I couldn't do that.
Products can be added and removed from cart	Well Done
Users can view past orders	Mostly Good - It does show products of other merchants
Merchants can add, edit and view their products	Mostly good - some issues with edit, see below (under CRUD)
Errors are reported to the user	❗️ Barely - (see below)
Order Functionality
Reduces products' inventory	❗️ Not Complete
Cannot order products that are out of stock	❗️ Not complete
Changes order state	❗️ Not Complete
Clears current cart	Well Done
Database
ERD includes all necessary tables and relationships	Well Done - (ERD should not mix in model relations into table, the belongs_to fields do not belong)
Table relationships	Well Done
Models
Validation rules for Models	Well Done - Nice touch with quantities needing to be more the 0 and requiring order fields on update
Business logic is in the models	Sometimes
Controllers
Controller Filters used to DRY up controller code	❗️ Only used in Merchant controller, to lookup merchant
Testing
Model Testing
Controller Testing	Mostly good w/ positive cases - ❗️ Missing a lot of negative cases
Session Testing	Well done
SimpleCov at 90% for controller and model tests
Front-end
The app is styled to create an attractive user interface	Well Done - Errors were hard to read, should be styled to read more easily.
The app layout uses Foundation's Grid	Sometimes - Did for application.html.erb, but not for any other view. Foundation can and still should be used for the content that is displayed in the yield area. Using the box grid would have been perfect for lists, like with products index. ❗️ Site is not responsive as a result.
Content is organized with HTML5 semantic tags	Sometimes - see below
CSS is DRY	Sometimes - Good use of selector but not multiple selectors. ❗️ Styles like background color, color and fonts are better defined once, with many selectors applied to the ruleset. If you needed to change a color you'd have to change it in a lot of places.
Overall

Routes

This route appears to be unnecessary: get 'merchants/merchants', to:"merchants#merchants", as:"all_merchant". Is it doing anything differently from /merchants that maps to "merchants#index"?

How could this route be more restful? get '/products/product_by_merchant/:id', to:'products#product_by_merchant', as: 'product_by_merchant'. What are the conventions with creating restful nested routes? Here are a couple resources on the topic: http://stackoverflow.com/questions/20951419/what-are-best-practices-for-rest-nested-resources && http://guides.rubyonrails.org/routing.html#nested-resources

There's A LOT of routes, are you using all of them? Nix any not being used, even when using the resources helper.

CRUD

I've run into issues with editing a product - when I am presented with the form, it's another product

Displaying Errors

When submitting the form for an order, I did not have any errors reported to me when I tried to submit it - ended up being the limit of 4 on the CC.

I did not have errors reported to me when saving a new product was unsuccessful.

After looking in the code, I see a lot of flash messages being set after a render or redirect - that code will not be executed. It should be before calling render or redirect.

Logic in Models

Order view in merchant has logic that should be in a model method, to calculate revenue

See Products#Create below.

TESTS
Has 13 errors & 1 Failure

Controller Testing

Merchants - Make sure to change title of blocks, ex: "displays a book that exists". NEW isn't redirecting, misleading test name. Redirecting to the new path only happens in create, when a resource does not save successfully. Should have more tests for create, to test that it will not save when specific fields are missing - instead of having one test that misses all of the required fields.

order_products - CREATE, EDIT and UPDATE missing negative cases. DELETE not complete.

orders - Summary, Destroy, edit, create, new, show and index are all missing negative cases. When creating or updating, have specific tests for each required field, to fail if they are missing.

products - new, edit and destroy missing negative cases.

reviews - new does not have negative case.

HTML

Some areas could use better tags, like in products index - the image isn't a section, should use div. Each product isn't an aside, often these items are still seen as a list and should be

's inside of

. Otherwise a section tag would suffice.

Some content does not have tags around it, like in products show. The content in show (merchants and products) should have it's own tags, like

Or an heading tag. The are not necessary if each of those bits of content were wrapped in an element.

Check for rouge tags, like products show has a on line 3.

Some pages missing sectioning tags all together, ex: reviews show,

Be mindful of indentation (like in order_summary, categories index).

Products

CREATE: The iteration of the list of category ids could be done within the add_category method. You had the right idea with wrapping the functionality into the add_category method, but keep all the logic in the model method, including the iteration.
is the @categories = Category.all necessary? If it's needed only for the render :new, it should only be called in that part of the condition.
your flash messages are being set after you're calling render - remember no code is executed after you call render.

DESTROY Does @product = Product.find_by(id: params[:id]) need to be called inside of the conditional again? Does product need to be an instance variable? Is that variable ever directly being used in a view? (a redirect path doesn't need an instance variable)