Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beyoncy - Rachel, Lauren, Joanna, and Alyssa #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alyssahursh
Copy link

@CheezItMan
Rachel, Lauren, Joanna, and Alyssa

@alyssahursh alyssahursh changed the title Beyoncy Beyoncy - Rachel, Lauren, Joanna, and Alyssa Oct 28, 2016
@alyssahursh
Copy link
Author

alyssahursh commented Oct 28, 2016

This is a copy of our original repository, which is here. We'd recommend checking that repository instead.

See Beyoncy on Heroku.

@rpavilanis
Copy link

rpavilanis commented Oct 28, 2016

How did your team work together?

  • Organization: Our team initially made a list of all of our user stories, and separated them out by customer, guest, and admin on our Trello Boards. We also made separate ERDs and then came together and compared, before incorporating feedback into a final ERD. I think this was really helpful because it allowed all of us to think through the model relationships and as a result, we all had a stronger overall idea of the project conceptualization before starting work on it. On Trello, we also made a board for each of us, and we dragged user stories and other tasks over to it as we were working on them, and after they were completed, we moved them to a complete column. I think that after the first week, we all started using the Trello more regularly, and this helped a lot with seeing how far we had yet to go. I think that regular stand-ups and communication, along with regular PRs (describing what we did and showing code and localhost project) also helped. I feel like this became more solid after the first week. I really appreciate that Lauren took the time to put together an "end-of-project" plan on Wednesday of the second week, which helped us see how much time we had to work (and what we were working), when we were planning to push to Heroku again, and also allotted time for two presentation practice sessions.
  • What I might do differently next time: I think after the first week, we all realized we had some room for improvement in terms of organization, leadership, and communication, and as a result we changed things (frequency of using Trello boards, people taking more leadership and ownership, reviewing commits/PRs with each other, and more frequent communication/check-ins (at least it felt like that to me)). I think that next time, it might be helpful to take more time determining project areas of focus for each person before jumping in. Again, we narrowed this focus in week two, but it would have been nice to know this more specifically going into Week 1 of the project (e.g., Joanna doing testing, Lauren doing OAuth/Admin stuff, etc.).
  • To C7: I think all doing your own ERD before coming to talk about it and jumping into the project was so helpful, so I would recommend that. Use your Trello Board! Communicate regularly. Okay, so that was more than one...just know you will get through it! :) Breathe!

TECHNICAL

  • What went well: Personally, I was really happy that I pushed myself on a couple of things that I wasn't sure I could do (mainly because we hadn't tried them in class), and then got them working. Specifically, I was proud of my work on the search bar, lemons for reviews, dropdowns, and image gem (even though it didn't end up in the final project because it only worked locally). I feel like I became more comfortable with merge conflicts over the week as well. I also feel more confident overall in approaching a project from the very beginning, without as much direction. In the end, I also feel a lot better about collaborating with a team around completing a big project like this.
  • What was most challenging?: I think it is hard to not touch every aspect of a project. I didn't do anything with the shopping cart, and I still don't have a strong understanding of how I would do one. I did really appreciate that we all had to figure out OAuth with our Task Lists, because even though completing that was stressful while also doing Beyoncy, I felt like I still had that experience even though Lauren and Joanna tackled most of that on Beyoncy. It was also tricky to have four hands touching the code, because we all have slightly different styles and I tend to worry that I am going to mess something up or do something in a way that won't be what others would want. I think that generally this worked out okay though - I think I tend to be too hard on myself or not give myself enough credit for things...something that I am working on improving. I definitely feel as though our code wasn't as clean or organized as it could be, and I think that on individual projects, it is easier to do this because you can go back in and update/change things as you want to improve. This code base got pretty large, so it wasn't as simple to do that, because changing one thing can effect other things.

Overall, I feel so proud of my team and I am excited with the final project. There is always room to improve, but I think with the time we had, we rocked it. :)

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rpavilanis @alyssahursh @laurenfb @jm-rives

I've added an overall view in your progress reports document, but I have specific code-related comments here, so you can see what I'm talking about specifically.

Overall well done. Looks like some more work needed to be done with testing, styling and some work with controllers, but overall very nice.


already_in = false

@order.order_products.each do |order_product|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can work around the need for this if statement by doing an:

Order_Product.where(order_id: @order.id).where(product_id: params[:order_products][:id].to_i)

@@ -0,0 +1,4 @@
<% if flash[:notice] %>
<%= flash[:notice] %>
<% flash[:notice] = nil %>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize I'd shown the <% flash[:notice] = nil %>, but it's not needed. You can just display the flash notice and it'll be gone in the next page load.

Good use of a partial though, although you might consider some formatting.

<body>
<header>
<nav>
<ul class="vertical menu medium-horizontal" data-responsive-menu="accordion medium-dropdown">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work using responsive menus. Some more work needs to be done on backgrounds to the top-bar when it gets to smaller screen sizes.

@@ -0,0 +1,16 @@
<table>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of partials

@@ -0,0 +1,46 @@
require 'test_helper'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work putting in tests for the view helpers.

@@ -2,4 +2,11 @@ class ApplicationController < ActionController::Base
# Prevent CSRF attacks by raising an exception.
# For APIs, you may want to use :null_session instead.
protect_from_forgery with: :exception

before_filter :set_categories

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to load the categories on each page? This will always run.


private
def find_category
@category = Category.find(params[:id])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find will actually throw an Error if the method fails, so the if @category.nil? will never execute. If you want to do this you should use Category.find_by instead.

if order_product.product_id.to_i == params[:order_products][:id].to_i
already_in = true
@order_product = OrderProduct.find(order_product.id)
@order_product.save

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You find an order_product and then save it to the database without changing anything here.


if @order_product.save
redirect_to '/cart'
else

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably use a named route instead of '/cart'

# end
# end

def edit

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think the edit method here should be using session to find the user or using the filter to find the value for the @user attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants