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

Ampers - Sweetsy (Alex, Aruna and Leti) #24

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

Conversation

LetiTran
Copy link

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 mostly divided work based on models in the beginning but we ended up all working together on all the aspects of the project.
How did your team utilize git to collaborate? Each person would create their own branches and then merge it with master when the work on that section was done.
What did your group do to try to keep your code DRY while many people collaborated on it? We used controller filter and view helpers methods. However, we wished we had more time to DRY it.
What was a technical challenge that you faced as a group? Git issues (merging) and model relationships.
What was a team/personal challenge that you faced as a group? Different communication styles and not setting personal expectations before starting the project.
What could your team have done better? Better division of work and more consistent code styling.
What was your application's ERD? (include a link) Here
What is your Trello URL? Here
What is the Heroku URL of your deployed application? Here

@CheezItMan
Copy link

bEtsy

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage with all members contributing Check!
Answered comprehension questions Check, but I don't have access to your ERD document.
Trello board is created and utilized in project management Check
Heroku instance is online Check
General
Nested routes follow RESTful conventions Check
oAuth used for User authentication Check
Functionality restricted based on user roles Check
Products can be added and removed from cart Check, although if I add item to my cart, I get ALL of them. I also can't edit my cart (extra comma in a view)
Users can view past orders Check, if I know the order ID, and it shows items in the cart even if the quantity was 0.
Merchants can add, edit and view their products I can add items, but I can't edit them
Errors are reported to the user With orders and new products they are
Order Functionality
Reduces products' inventory Yes, but should the inventory really be reduced until AFTER the order is processed. Otherwise if I put 1000 donuts in my cart, does that mean you can't sell them anymore, even if I never order them?
Cannot order products that are out of stock Check
Changes order state Check
Clears current cart Check
Database
ERD includes all necessary tables and relationships Can't see it.
Table relationships Check
Models
Validation rules for Models Check
Business logic is in the models Well done
Controllers
Controller Filters used to DRY up controller code Check
Testing
Model Testing Check
Controller Testing Check
Session Testing You're missing testing negative cases with users logging in.
SimpleCov at 90% for controller and model tests Model testing is good, you have missed a lot of coverage in controller testing
Front-end
The app is styled to create an attractive user interface I like the responsive design elements. There are some issues the review, for example, does not show the numbers on the form. Also you have issues with Turbolinks keeping the Foundation JavaScript from executing. You can see some support here.
Content is organized with HTML5 semantic tags You have a header element outside the body. NOOOOOOO! Your drop-down menu i actually a lot of small drop-down menus using div elements. Since they're for navigation using a nav element would be better, and making them one nav menu would be neater.
CSS is DRY Not particularly, several rules repeat components.
Overall Congrats you're done with bEtsy. You've got issues, but did fairly well for a 3-person group.

@@ -0,0 +1,12 @@
<h1> Edit Order Item Quantity </h1>

<% product = Product.find(@orderproduct.product_id) %>

Choose a reason for hiding this comment

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

This looks like something that should be in the controller.

<%= b.check_box %>
<%= b.label %>
<% end %>
<%= f.submit, class: "button" %>

Choose a reason for hiding this comment

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

You have a comma misplaced here!

must_redirect_to homepage_path
Merchant.count.must_equal before_count
end

Choose a reason for hiding this comment

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

What about if the route is hit without the proper information to log in. Like the user bypasses github and goes to the route.

<%= csrf_meta_tags %>
</head>

<header class="nav-bar">

Choose a reason for hiding this comment

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

Your header element is not inside the body element. All content on the page should be in the body. Review the format of an HTML document.

<header class="nav-bar">
<section class="top-bar" data-topbar role="navigation">

<div class="nav-drop">

Choose a reason for hiding this comment

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

Making this a nav element might make sense.

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

Successfully merging this pull request may close these issues.

4 participants