J'All (Jocelyn Gonzalez, Averi Kitsch, Laura Robertson, Lauren Cardella) -- Carets by enigmagnetic · Pull Request #60 · Ada-C8/betsy

enigmagnetic · 2017-10-27T19:50:56Z

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 did a lot of the initialization of models, controllers, and database migrations as a group. We identified tasks that were more suitable for pairing and those more suited for individual work, and assigned them primarily by interest in working on those tasks.
How did your team utilize git to collaborate?	Locally each team member used branches for features, which were first merged to master locally before being pushed to the remote master. Merge conflicts were handled individually as needed.
What did your group do to try to keep your code DRY while many people collaborated on it?	At first we focused on getting functionality, then once there was a good chunk of work done with tests, some of the team reviewed others' code and refactored. We did this process a few times before project completion.
What was a technical challenge that you faced as a group?	Figuring out how to build the routes and validate the models for order creation and order_product creation took us a couple of days as a team to conceptualize. It was difficult because an order needs to have an order product, but an order product needs to have an order id, to be initialized. We had to decide which needed to be created first (order), how the routes should work, and how to make sure that an order could not be submitted/saved in the database without all of its necessary attributes.
What was a team/personal challenge that you faced as a group?	How to work individually on elements that should look or work cohesively, i.e. HTML structure for the pages, and routes that rely on other parts of the application to work. Also, Lauren found the order fulfillment page challenging in getting the correct model data for each user, which Averi was able to help review and refactor for a cleaner, more robust solution.
What could your team have done better?	We could have focused more on the 'correct' workflow of testing first, which we did not do a great job in the first phase of the project. We did some pair test-building after the first sprint to fill out the tests. However, we are generally very pleased with the pace we were able to keep and the product we made.
What was your application's ERD? (include a link)	https://www.lucidchart.com/documents/edit/744a2e8c-59ae-4f39-851a-9c803303fa42?shared=true&
What is your Trello URL?	https://trello.com/b/Lzr1zUP5/spooksy
What is the Heroku URL of your deployed application?	http://spooksy.herokuapp.com/

…ontroller tests for edit, update, and delete. Add edit action to order_products_controller. Edit action test passing, other tests failing.

… order index view with delete link method.

…ation page

…product_testing.

…redundant route. Put custom routes under standard resources for readability where appropriate.

CheezItMan · 2017-11-01T03:09:20Z

bEtsy

What We're Looking For

Feature	Feedback
Baseline
Appropriate Git Usage with all members contributing	Good number of commits and good messages. I like how you used branching.
Answered comprehension questions	Check, it sounds like things went well.
Trello board is created and utilized in project management	Check, it look like you accomplished almost everything you planned to do.
Heroku instance is online	Check
General
Nested routes follow RESTful conventions	RESTful routes where appropriate
oAuth used for User authentication	OAuth is working.
Functionality restricted based on user roles	Check, can't edit a product that doesn't belong to you. I nearly died laughing when I tried to edit a category I didn't have access to, "uh-oh, looks like a ghost.."
Products can be added and removed from cart	Check
Users can view past orders	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, not even if I try to add it twice
Changes order state	I click on the `shipped` link, but the order stays status, `paid` and not complete.
Clears current cart	Check
Database
ERD includes all necessary tables and relationships	Check
Table relationships	Check
Models
Validation rules for Models	Nice custom validations as well.
Business logic is in the models	The average review should be done in the model.
Controllers
Controller Filters used to DRY up controller code	Good use of `find_user` and `categories_all`, also good use of the `render_404` method as well as `find_op`
Testing
Model Testing	Lots of model testing here. Good. I left a few note in the code.
Controller Testing	Nice work, I left a few notes in the code
Session Testing	Good standard session testing, no problems that I noticed.
SimpleCov at 90% for controller and model tests	99% great!
Front-end
The app is styled to create an attractive user interface	The app looks really good. I love the ghost image.
The app layout uses Foundation's Grid	It's a responsive site and looks pretty good at a variety of screen sizes and on my phone.
Content is organized with HTML5 semantic tags	Pretty good semantic HTML
CSS is DRY	Some DRYing up could be done, but not a lot.
Overall	This is a pretty outstanding project. Y'all should be proud. Make sure to add this to your portfolios and if asked about important projects you've worked on, show them this. Very nice!

IMPORTANT: Whoever submitted the PR (and thus will get the notification about this feedback) should share this with their teammates.

CheezItMan

Very nice work, a few comments below.

CheezItMan · 2017-10-31T22:16:20Z

app/controllers/order_products_controller.rb

+  def cancel
+    if find_user
+      if @order_product.product.user.id == session[:user_id]
+        if !@order_product.shipped


This looks like a good candidate of content to move to the model.

CheezItMan · 2017-10-31T23:20:25Z

test/models/category_test.rb

+describe Category do
+  let(:category) { Category.new }
+  describe "validations" do
+    it "category without name is invalid" do


It's also useful to verify that category.errors.keys.must)include(:name)

CheezItMan · 2017-10-31T23:21:08Z

test/models/category_test.rb

+    end
+
+    it "doesn't create a new category with different capitalization" do
+      proc {Category.new(name: "PoTions")}.must_change 'Category.count', 0


You're missing a save here!

CheezItMan · 2017-10-31T23:22:03Z

test/models/category_test.rb

+      cat = Category.first
+      cat.must_respond_to :products
+      cat.products << prd
+      cat.products[0].must_equal products(:one)


Just to make this more generic I would use:

cat.products[0].must_include products(:one)

CheezItMan · 2017-10-31T23:35:32Z

test/models/order_test.rb

+        @order.valid?.must_equal false
+      end
+
+      ["09/18","11/20","9/18"].each do |num|


It would be a good idea for a generic test that uses the current date rather than hard-coded dates.

CheezItMan · 2017-11-01T02:35:27Z

test/controllers/order_products_controller_test.rb

@@ -0,0 +1,158 @@
+require "test_helper"
+
+describe OrderProductsController do


There are some negative tests you're missing here. Like if the item being shipped or deleted is already gone.

CheezItMan · 2017-11-01T02:37:57Z

test/controllers/orders_controller_test.rb

+  end
+
+  describe "order show" do
+    it "should get show if logged in" do


Good testing here, but this looks like 3 tests not one.

CheezItMan · 2017-11-01T02:42:37Z

test/controllers/orders_controller_test.rb

+      must_respond_with :not_found
+
+    end
+    it "should get show" do


Some of your it blocks should be more specific, like this one which should say, "will redirect guest users on show actions."

CheezItMan · 2017-11-01T02:45:06Z

test/controllers/orders_controller_test.rb

+
+      Order.last.status.must_equal 'pending'
+
+      put order_path(Order.last), params: {order:


Given the two arrays above you could use a loop to set up this hash.

CheezItMan · 2017-11-01T02:46:32Z

test/controllers/orders_controller_test.rb

+      must_respond_with :success
+    end
+
+    it "can get found" do


This test makes no sense.

LauraAddams and others added 30 commits October 20, 2017 12:55

Baseline category routes and add a category tests

80e6d4f

Testing a blank name for category

4912338

can not add more than stock

554a961

fixed new product bug

df440a1

Merge branch 'retire_product'

c145dc6

Add update and delete routes for order_products. Add order_products_c…

126cc27

…ontroller tests for edit, update, and delete. Add edit action to order_products_controller. Edit action test passing, other tests failing.

testing stock method

966f485

testing print categories

a2a67ab

tested average rating

a03f9f2

test application helper

8eb72db

Merge branch 'controller-tests'

1631243

Add destroy action to order_product_controller. Test is passing. Edit…

3729ce3

… order index view with delete link method.

Fix merge conflicts

eafc783

started orders helper

518d83e

removed coverage folder files

f6a7030

updates for conflict

b4352b7

more secrets

9bd8e72

Merge branch 'testing'

431f7d8

Product thumbnails, footer coloring

3965f22

Better image sizing, promo banner, categories text

e747330

updated index page

963b00b

added toggle functionailty

bbd8483

added statement on products index if collection equals zero

46c7939

Sass structure for easier color changes, orgnaizing of previous css code

c5cc679

minimal changes to order model validations and controller and confirm…

0a26af9

…ation page

updated price input field for new product

57353fd

added user summary statement if products equal zero

390efbf

Update ignore file. Edit products fixtures with more stock for order_…

1bbfe7b

…product_testing.

Forms styling

7cca8e7

Cart styling added

4ddcf1f

enigmagnetic and others added 28 commits October 26, 2017 19:59

Use resources to consolidate necessary order_products routes. Remove …

ddd16a8

…redundant route. Put custom routes under standard resources for readability where appropriate.

Adding logo animation, checked for sections around all views

9f428e1

Fixed z-index for mobile nav issue with flash messages

86a94a8

Added short decriptions to employee page

f837e8d

Container applied to new review page

f667cb5

Subscribe form in footer

3377a2f

fixed products partial form

fc747b9

Merge branch 'product_testing'

4111cf1

images added and new seeds

d3d376e

Form email subscribe centered

1ab0281

Removed rais from new product page

339b672

Refactored model tests:category,op,order

d6c5237

new seeds

bc1d440

Merge branch 'master' of https://github.com/enigmagnetic/betsy

22ef739

precompiled images

3e16d77

Refactor model test: products

8ce2776

Removed commented out tests

261e1cb

fix id for sessions

aaa8655

Merge branch 'refactor'

14d5ad5

Placeholder image added

b69dd93

Placeholder image now default is no image added

561eee2

Added tombstone image back

d97df7c

new image placeholder

9f38db2

precompile assets'

f1de8d7

updated product_controller_test

de6de07

Merge branch 'product_testing'

4032139

update product test

09fba7b

Merge branch 'master' of https://github.com/enigmagnetic/betsy

0b3735a

CheezItMan reviewed Nov 1, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

J'All (Jocelyn Gonzalez, Averi Kitsch, Laura Robertson, Lauren Cardella) -- Carets#60

J'All (Jocelyn Gonzalez, Averi Kitsch, Laura Robertson, Lauren Cardella) -- Carets#60
enigmagnetic wants to merge 360 commits intoAda-C8:masterfrom
enigmagnetic:master

enigmagnetic commented Oct 27, 2017

Uh oh!

CheezItMan commented Nov 1, 2017

Uh oh!

CheezItMan left a comment

Uh oh!

CheezItMan Oct 31, 2017

Uh oh!

CheezItMan Oct 31, 2017

Uh oh!

CheezItMan Oct 31, 2017

Uh oh!

CheezItMan Oct 31, 2017

Uh oh!

CheezItMan Oct 31, 2017

Uh oh!

CheezItMan Nov 1, 2017

Uh oh!

CheezItMan Nov 1, 2017

Uh oh!

CheezItMan Nov 1, 2017

Uh oh!

CheezItMan Nov 1, 2017

Uh oh!

CheezItMan Nov 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -0,0 +1,158 @@
		require "test_helper"

		describe OrderProductsController do


		Order.last.status.must_equal 'pending'

		put order_path(Order.last), params: {order:

Conversation

enigmagnetic commented Oct 27, 2017

bEtsy

Comprehension Questions

Uh oh!

CheezItMan commented Nov 1, 2017

bEtsy

What We're Looking For

IMPORTANT: Whoever submitted the PR (and thus will get the notification about this feedback) should share this with their teammates.

Uh oh!

CheezItMan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants