DanDeeLions - Edges - Fibonoshi

[kaganon]

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?	Pushpa: I was responsible for getting the merchant(session) figured out and getting the initial setup functioning. Anibel: One thing I was responsible for was the creation of orders and how those effects were perpetuated in related tables. Katrina: I was responsible for the wireframe/design and did most of the HTML/CSS/bootstrap implementation of the app. Carly: I was responsible for most things related to Products, and did the testing for Products and Categories. I had fun making the ByteSquad page, but was actually most proud of figuring out how to seed the database.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on?	Pushpa: I would like feedback on merchant controller and model testing, and logic flow for that controller and model. Anibel: I was primarily responsible for Order model and controller actions. While we were able to address some of the related user stories, I felt like it was a little “hack-ey” (i.e. having to create custom validations, creating instances of multiple classes in a single action, etc.). What’s the right way to do this? Carly: The Product piece (from controller, model, views, relationships, and nesting). Katrina: The reviews controller logic. It was a little tricky having the reviews on the same page, and my code wasn’t as DRY as I thought it should be, but wanted to get it working and felt crunched for time, so I think I would like to get feedback on how to DRY up that controller.
How did your team break up the work to be done?	We all had a really good plan set out in the beginning and had a great layout of what we wanted to do. As a team we did the initial controller, model, and schema setup once our ERD was drawn out. Talking through the model setup and relationships as a team, and doing all the initial migrations as a team on the second day was key. Once that was done we decided to pick a section that we wanted, and then also paired up with each other during the complex logic. For example, one person had picked merchant controller logic to work on and paired for the complex portions.
How did your team utilize git to collaborate?	We all communicated ahead of time when we were going to do a git push. We used branches to work on each feature and took turns branching to master once we were united as a group. This workflow helped keep our master branch clean.
What did your group do to try to keep your code DRY while many people collaborated on it?	Our group used view helpers and application controller methods and communicated to each other which methods we might use. We also used partials to consolidate redundant code.
What was a technical challenge that you faced as a group?	Testing was a challenge for the group as it was difficult to test complex controller logic as well as reading/comprehending other team member’s code.
What was a team/personal challenge that you faced as a group?	There was a general feeling of not contributing/inadequacy in the group. Imposter syndrome was real! Interestingly, we each felt it at various times throughout the week (both inadequacy and the feeling of not contributing enough). We dealt with these issues by trying to remain positive and supportive of each other.
What was your application's ERD? (include a link)	https://www.lucidchart.com/documents/view/1f166598-4f05-4aca-a82b-51d0b516d45b
What is your Trello URL?	https://trello.com/b/qxiDkltV/team-dandeelions
What is the Heroku URL of your deployed application?	https://fibonoshi.herokuapp.com/ ***Note: Because we used Font Awesome for the star ratings, there were some issues at times not seeing the stars when the site is down

[droberts-sea]

bEtsy

Pushpa: I would like feedback on merchant controller and model testing, and logic flow for that controller and model.

• Merchants controller looks good, most of the complex logic lives in the model.
• I like the model methods in Merchant, particularly the way they build on each other to create complex functionality.
• Tests for merchant model methods are lacking.
  • What if the no orders have been placed for that merchant's items?
  • What if they don't have any products?
  • Is the logic for handling different order statuses correct? i.e. only "paid" orders should be included.

Anibel: I was primarily responsible for Order model and controller actions. While we were able to address some of the related user stories, I felt like it was a little “hack-ey” (i.e. having to create custom validations, creating instances of multiple classes in a single action, etc.). What’s the right way to do this?

• There's certainly some complex logic here, but they reflect complex rules. I don't know if I would go so far as to call it "hack-ey". Custom validations are included in Rails for a reason.
• I don't see where you're creating instances of multiple classes though - maybe I'm missing something. I would be happy to go over this in a 1-on-1 if you have specific spots to look at.
• You're missing authorization for shipping order items. Before doing the work, you should check that the user is logged in as the owner of that order item's product.

Carly: The Product piece (from controller, model, views, relationships, and nesting).

• Products looks pretty good except for the authorization pieces. You don't have any restrictions on update, which means I could use a tool like postman to change someone else's products! You should be checking both that the merchant is logged in, and that they are the owner of this product. This might be a good thing to put in a controller filter. Of course, there should also be testing for this logic.

Katrina: The reviews controller logic. It was a little tricky having the reviews on the same page, and my code wasn’t as DRY as I thought it should be, but wanted to get it working and felt crunched for time, so I think I would like to get feedback on how to DRY up that controller.

• You're right, there is a lot of repetition here. The goal here should be to figure out what the different behaviors are, and try to consolidate them as much as possible. I would probably approach it like this:

# Instance method inside of Product
def can_be_reviewed_by(merchant)
  return true unless merchant
  return merchant.id == merchant_id
end


# In the ReviewsController
def create
  product = Product.find_by(id: params[:id])
  if product.nil?
    # not found
  elsif product.can_be_reviewed_by(@logged_in_merchant)
    # Create review, try to save, set flash
  else
    # Unauthorized! Set flash, redirect
  end
end

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]

Since this action operates on a product, it might make more sense in the ProductsController.

[droberts-sea]

These self methods should probably be view helpers.

[droberts-sea]

I don't know that saying order.product_name is any easier than order.product.name.

[droberts-sea]

Line 66 could be rewritten:

@order_item.status = !@order_item.status

[droberts-sea]

Nice! I like this test case, and I like that you thought to check this in the controller.