-
Notifications
You must be signed in to change notification settings - Fork 14
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
Gale, Maria, Sara, Kimberley - Carets #72
base: master
Are you sure you want to change the base?
Conversation
…g in your cart when you logout.
bEtsyWhat We're Looking For
IMPORTANT: Whoever submitted the PR (and thus will get the notification about this feedback) should share this with their teammates.ProductControllerTest gives an error when you run the tests: Your products model fixture It's best to have every fixture you define be valid, and when testing for invalidness on your model tests, to take a specific fixture and then set the target attribute to nil, like so:
|
|
||
.dropdown:hover .dropdown-content-cart { | ||
display: block; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are a couple of cases in this stylesheet that suffer from copy/paste and should be all on the same line, such as this:
.dropdown:hover .dropdown-content-browse, .dropdown:hover .dropdown-content-cart, .dropdown:hover .dropdown-content-cart {
display: block;
}
order.products << product1 | ||
order.order_total.must_equal (9.99 * 2) | ||
order.products << product2 | ||
order.order_total.must_equal (9.99 * 2) + 8.99 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the test data changes? Instead of using magic numbers/hardcoded numbers, it would be better if you use the data like product1.price
instead of 9.99
<span class= "index-image"><%= link_to image_tag("http://www.visitbuffaloniagara.com/content/uploads/sv-event-images/434cfb09-bb34-449b-ac6c-d400b6a53f1d.jpg"), product_path(product.id), alt: "A product image placeholder" %></span> | ||
<% end %> | ||
<span><em><%= link_to product.name.upcase, product_path(product.id) %></em></span> | ||
<span><%= product.price %></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your product.price
needs to be formatted to a correct dollar format. Otherwise numbers appear like 100.0
instead of 100.00
.
The other locations for this that I could find are:
- Product detail page
- Every Merchant order page
| What was your application's ERD? (include a link) | | | ||
| What is your Trello URL? | | | ||
| What is the Heroku URL of your deployed application? | | | ||
| How did your team break up the work to be done? | We started with assigning people to Trello cards. the models and controllers for testing and creating the methods. We set up the tasks in waves on Trello and as we completed the tasks, we moved on to different tasks in the waves. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, these questions shouldn't be answered like this within this file!
github uses this file to automatically populate the text box for the description when you open a new Pull Request. Just edit that PR description. I went ahead and copied and pasted this into your PR for you :P
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