team betsii (Barbara, Kat, Chantelle, Lindsay) - edges - betsy

[elle-terch]

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?	Barb: Not so proud of, but happy that I was able to traverse and extract data out of three different connected tables to produce a Manage Orders Page for the Merchant. Lindsay: I was glad to figure out the relationship and routing between items, order items and orders. Kat: I was primarily responsible for being Task Leader on the Trello board for the first week and I was kind of proud of the way I handled that. Chantelle: I think I did a decent job of learning how to debug issues in our code and generally making code more dry.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on?	Barb: Manage Orders Page for the Merchant--it used custom controller methods that I could not figure out how to test. Lindsay: Order Items Controller + tests. I spent a ton of time implementing that controller without testing first. As a result, I couldn't get all of my tests to pass, even though the controller seemed to work both, locally and on Heroku. Kat: I was primarily responsible for the Items controller and model and show pages. I would very much like feedback on the tests I wrote for the items controllers. There are notes of the ones I wrote and didn’t but I would love tips on how to make the tests better and what I need to improve on please thank you. Chantelle: I divided some model methods between orderitem and item and orderscontroller (for marking a product as paid and decreasing available items) and im not sure if that was the best implementation (so basically, good object oriented design)
How did your team break up the work to be done?	We started out giving each person control of one major feature of controller. Once those pieces were in place, we would regroup and figure out how to divide up next steps and whether that would involve pairing or solo work. We would periodically work as a group of 4, sometimes we would pair and in the evenings work solo and chat online to organize merging or task delegation.
How did your team utilize git to collaborate?	We would work on different branches for features and merge things to master once we were all either online or in person.
What did your group do to try to keep your code DRY while many people collaborated on it?	We maintained solid communication on who was responsible for what. It was really rare that two people were working on the same piece of the application at the same time.
What was a technical challenge that you faced as a group?	Merging and github was a challenge. We also had a really hard time understanding cookies and how those would impact our app if they weren't cleared regularly.
What was a team/personal challenge that you faced as a group?	Not feeling good--sick. Otherwise, we maintained pretty good communication and support.
What was your application's ERD? (include a link)	https://trello.com/c/ggxM5BPW/17-erds
What is your Trello URL?	https://trello.com/b/NDe8E3Ca/betsii
What is the Heroku URL of your deployed application?	https://kawaii-travel-site.herokuapp.com/

[droberts-sea]

bEtsy

Barb: Manage Orders Page for the Merchant--it used custom controller methods that I could not figure out how to test.

• See inline comments. Regarding testing controller methods, outside of the actions controller methods are not considered part of the controller's public interface (typically they're marked private), so we don't test them directly. See Metz ch 4.

Lindsay: Order Items Controller + tests. I spent a ton of time implementing that controller without testing first. As a result, I couldn't get all of my tests to pass, even though the controller seemed to work both, locally and on Heroku.

• This controller action expects a cart to have been created, and an OrderItem to have been added to it, before the action is run. On line 25 it pulls up the current cart, and on line 26 it searches for the order_item_id you sent within that cart. However, your test plucks an order_item from the database without making sure it's in the cart, which is why this test fails (the cart has no order_items, so the search on line 26 yields nil).
• You should be doing two things here:
  • In the controller, after line 26, check for nil, and send an error to the user if needed (you should do this whenever you call .find_by).
  • In the test, as part of the arrange step you'll need to establish a cart in the session and add an item to it. Since your app automatically creates a new, empty cart for any request, it might look like this:

    it "can update an item's quantity" do
      # Arrange
      # Establish the cart
      get root_route
      
      # Add an order_item
      order = Order.find(session[:order_id])
      order_item = Order.order_items.create!(product: Product.first, quantity: 1)
      new_quantity = 2
      
      # Act
      patch order_item_path(@order_item.id), params: { quantity_per_item: new_quantity }
      
    
      # Assert
      must_respond_with :success # or whatever's appropriate
      order_item.reload
      expect(order_item.quantity).must_equal new_quantity
    end

• You're also missing some interesting edge cases. Just for update:
  • What if the order_item_id you send isn't part of the current cart? What if it doesn't exist at all?
  • What if you send in a bad quantity (negative, more than the available inventory of the product, etc.)?

Kat: I was primarily responsible for the Items controller and model and show pages. I would very much like feedback on the tests I wrote for the items controllers. There are notes of the ones I wrote and didn’t but I would love tips on how to make the tests better and what I need to improve on please thank you.

• Good use of controller filters
• You should have tests for if the item does not exist for each action that takes an item ID in the URL
• You are missing logic for authorization. For example, on update you should check that a user is logged in, and that that user owns the product they're trying to update - otherwise I could edit someone else's products! See for example https://kawaii-travel-site.herokuapp.com/items/29
• You should also have tests for this: try to edit as a guest, try to edit as the wrong user.

Chantelle: I divided some model methods between orderitem and item and orderscontroller (for marking a product as paid and decreasing available items) and im not sure if that was the best implementation (so basically, good object oriented design)

• I think this looks pretty good. If you wanted to clean it up a little you could move it to a model method: Order.add_item(item, quantity), which could return true or false based on whether there was enough inventory.

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

[droberts-sea]

Line 25 should definitely be split across multiple lines.

[droberts-sea]

On line 25, you should be using the activerecord relation: order = order_item.order. Or you could omit the intermediate variable entirely, and on lines 26..28 say order_item.order.name, etc.

[droberts-sea]

You've got a lot of repeated work in this method, either things your application is already doing, or things that Rails can do for you.

First, you're already finding the currently logged in merchant in a controller filter, saving it in the instance variable @current_user

For lines 25..28, you already have an items relation on the User model, so you could say items_sold_by_merchant = @current_user.items.

However, we could go even further. In the User model, you could add an indirect relation to order_items: has_many :order_items, through: :items. Then, to get the list of all order_items for this user, you could say @current_user.order_items

See also our textbook resource on indirect relations

Lines 30..37 don't seem to check whether the order is pending before adding it to the list. However, we could achieve this with the following:

@pending_order_items = @current_user.order_items.select do |oi|
  oi.order.status == "pending"
end

The takeaway is to make sure you're familiar with the capabilities given to you by your library. We're about to leave our Rails unit, but this will be just as relevant when we work with JavaScript libraries like jQuery and React.