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

Leaves - Georgina, Hallie, Samantha, Sabrina #93

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

Conversation

Galaxylaughing
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
Each team member: what is one thing you were primarily responsible for that you're proud of? Hallie: I was primarily responsible for the shopping cart and I'm happy with the functionality of not being able to add a greater quantity than what is in stock. This displays on the product page but does not actually update stock until the transaction is paid since the shopper could still bail out.

Sabrina: I was responsible for the merchant dashboard and I'm pretty happy with what it allows you to do and the way it looks. I'm proud of the code around filtering an order by merchant so that a merchant only sees the status of their part of the order and can only see and effect order-items that have their products.

Georgina: At first I was responsible for orders and then we realized that orders were very related to order_item so I worked with Hallie to do the logic. I also worked on the reviews and the checkout part.

Sam: I was primarily responsible for the Product Class, including products controller, model validations, tests, products views to show "all products" and "products by category" (working on the category relationship), also created the product "show view" shopping product page, created the seeds for products. Also supported the merchant dashboard features on creating actions to edit and create a product, create and add a category to products, also developed logic to retire/reactivate a product as a merchant and applied general CSS on the project. In general for the common features I worked using pair programming with all other team members which was always an addition to my features.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on? Hallie: I would like targeted feedback on the OrderItems#create action. A piece of this action makes sure that the user doesn't add more items than are available and if the item is already in the cart, it increases the quantity rather than making a new OrderItem. I debated about carving this out into the model to keep the controller light, but since it's really a few lines followed by corresponding flash messages, I kept it in the controller. Was that approach right/wrong?

Sabrina: I guess the methods around finding orders by status in the user model, and the tests. I'm happy I was able to get them to work but I'm not sure it was the best way, and I wonder if maybe our underlying data structure could be changed to make these sorts of operations more efficient.

Georgina: I was primarily responsible for the Order checkout functionality. This meant validating the fields in the Orders model and figuring out how to work with OrderItems and with session to create a cart and then LATER validate user information.

Sam: Product Class.
How did your team break up the work to be done? We mapped out the requirements via a wire frame and then each took models, with an effort to divide work evenly. These initial assignments for building the model and validations ended up being where we each focused for the duration.
How did your team utilize git to collaborate? We regularly made pull requests and reviewed and incorporated them. Typically, upon submitting a PR, the owner would message our Slack channel with an FYI and someone else would communicate that they intended to review.
What did your group do to try to keep your code DRY while many people collaborated on it? We tried to be clear about what each of us was working on so that we didn't overlap. In addition to the stand up meeting, we usually met for at least 30 minutes each day to re-map our progress and prioritize. Additionally, we aimed to keep our Trello board current.
What was a technical challenge that you faced as a group? We each had respective challenges with git, and we learned that for proper testing we couldn't use incognito but had to clear cookies. As each of us hit our git/browser issues, another person in the group was often able to suggest solutions.
What was a team/personal challenge that you faced as a group? When we were sorting out relationships between some models, it was difficult to come to a mutual understanding of both the problem and the solution. For us as a group, it really just meant that we need to take the time to present and listen to different ideas. In the end, we all agreed on a path forward and relationships remained in-tact.
What was your application's ERD? (include a link) https://www.lucidchart.com/documents/edit/a547316e-05ae-4e1c-b41a-f06531653434/0_0
What is your Trello URL? https://trello.com/b/EWgK5Prc/plantsy-ada-orchids
What is the Heroku URL of your deployed application? https://ada-orchids.herokuapp.com/

Galaxylaughing and others added 30 commits October 28, 2019 09:40
idhallie and others added 28 commits November 1, 2019 10:24
prevented user from editing another user's product
prevented guest from updating product
changes not to allow guest user to create category
@beccaelenzil
Copy link

Becca Elenzil

Tue, Nov 5, 2:35 PM (22 hours ago)

to me

bEtsy

What We're Looking For

Manual testing

Workflow yes / no
Deployed to Heroku yes
Before logging in
Browse all products, by category, by merchant yes
Leave a review yes
Verify unable to create a new product yes
After logging in
Create a category yes
Create a product in that category with stock 10 yes -- though initially I tried to create a project and was unsuccessful, and the flash error messages didn't give me any hints. Should the merchant ID be a hidden field?
Add the product you created to your cart yes
Add it again (should update quantity) yes
Verify unable to increase quantity beyond stock yes -- could you limit the drop down menu to only show up to the number in stock?
Add another merchant's product yes
Check out yes
Check that stock was reduced yes
Change order-item's status on dashboard yes
Verify unable to leave a review for your own product no - I was able to leave a review for mars
Verify unable to edit another merchant's product by manually editing URL yes
Verify unable to see another merchant's dashboard by manually editing URL yes

Code Review

Area yes / no
Routes
No un-needed routes generated (check reviews) yes
Routes not overly-nested (check products and merchants) yes
Merchant dashboard and cart page use a non-parameterized routes (should pull merchant or cart ID from session) no -- but works with parameterized routes. Consider using the following urls (URI/dashboard and URI/cart)
Controllers
Controller-filter to require login by default no, this is sprinkled throughout the actions
Helper methods or filters to find logged-in user, cart, product, etc yes -- get_user, find_order_item
No excessive business logic order_item create could have some of the logic in the model with a validation -- see comment
Business logic that ought to live in the model
Add / remove / update product on order see above and code comment
Checkout -> decrease inventory yes (update_stock)
Merchant's total revenue yes
Find all orders for this merchant (instance method on Merchant) yes
Selected Model Tests
Add item to cart:
- Can add a good product
- Can't add a product w/o enough stock
- Can't add a retired product
- Can't add to an order that's not in cart mode
- Logic specific to this implementation
missing some -- I believe this is all implemented in the controller
Get orders for this merchant:
- Includes all orders from this merchant
- Doesn't include orders from another merchant
- Orders are not included more than once
- Does something reasonable when there are no orders for this merchant
yes
Selected Controller Tests
Add item to cart:
- Empty cart (should be created)
- Cart already exists (should add to same order)
- Product already in cart (should update quantity)
- Bad product ID, product is retired, quantity too high, or something like that (error)
missing some
Leave a review:
- Works when not logged in
- Works when logged in as someone other than the product's merchant
- Doesn't work if logged in as this product's merchant
- Doesn't work if validations fail
missing some failture cases

Overall Feedback

Great work overall! You've built a fully functional web store from top to bottom. This represents a huge amount of work, and you should be proud of yourselves!.

I am particularly impressed by the way that you took a TDD approach and created thorough tests. There are still a few test cases missing, particularly around failure case (though I want to acknowledge that I may have missed a few that you had).

bEtsy is a huge project on a very short timeline, and this feedback should not at all diminish the magnitude of what you've accomplished. Keep up the hard work!

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

def create
order = Order.find_by(id: session[:order_id])

if session[:order_id].nil? || order.nil?

Choose a reason for hiding this comment

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

order.nil? should be enough here since if session[:order_id] is nil, order will be nil.

return
else
oi.quantity += order_items[:quantity]
oi.save

Choose a reason for hiding this comment

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

There needs to be a check on the .save


order = Order.find(session[:order_id])
# if order item already exists in cart, increase quantity.
order.order_items.each do |oi|

Choose a reason for hiding this comment

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

Per Hallie's requested focused feedback, some of this work could be done through a validation something like this:

#order_item.rb
validates :quantity, numericality: { less_than_or_equal_to: product.stock }, message: <THE MESSAGE> (edited) 

#order_items_controller
if oi.save
  flash[:success] = # ...
else
  flash[:failure] = oi.errors.messages.join("; ")
end

return result[:orders_by_status]
end

def find_order_statuses()

Choose a reason for hiding this comment

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

Per Sabrina's request for focused feedback. These group of methods look good. I wonder if find_order_statuses could have been broken up into smaller methods to simplify a bit.

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.

5 participants