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 - Sudsy (Dani, Dianna, Katie, Natalie, Raisah) #86

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

Conversation

dtingg
Copy link

@dtingg dtingg commented Nov 1, 2019

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? Dani - Search bar, reviews and its tests. I’m proud that our website has a search bar as it wasn’t in the requirements, but is a nice feature. Dianna - I mostly worked on orderitems and orders. It was challenging to figure out how to make everything work together, but I’m proud of how it turned out. Katie - I was mostly responsible for the merchant dashboard. I worked with Dianna and Raisah on the page to figure out the methods and testing. Natalie - I created the join table and migrations that enabled many-to-many relationships between products and categories. I’m also super proud of some of the layout in the view that we worked on as a team. Raisah - I worked on recently_viewed products and I worked on the Products controller and model. I’m particularly proud of the recently_confirmed cookie that allows the user to see their order confirmation page for one minute and then restricts access after that time has passed.
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on? Dani - I don’t really think any area where I would like targetted feedback as initially, I wanted the search bar to be case insensitive and do multiple matches by word, and I couldn’t make it to work, but Dianna was able to do it, so this was solved. Dianna - I’d like feedback on other ways to structure the orderitems and orders. Katie - I’d like feedback on the merchant dashboard and some guidance on how to use view helpers or partials. Natalie - Are the model tests for Products and Categories, especially relating to relations, comprehensive? Raisah - Are the tests for Products comprehensive? Generally is this application following OOD principles? If not, how could the relationships between the classes be improved?
How did your team break up the work to be done? At the beginning of the project, we all gave input on what we wanted to work on. Luckily everyone on the team was flexible and completed the tasks based on need instead of interest. Starting by breaking the projects into merchants, products, data, we grouped up and pair programmed through most of the setup. After the main structure of the site was assembled we used our Kanban board and work tickets to divide up the work. The last few days we used physical sticky notes after team huddles to tie up loose ends on the project, going over user stories a few times to make sure that our project meets the requirements.
How did your team utilize git to collaborate? From the beginning, our team prioritized using branches. In order to complete pull requests, we worked in a round robin style. We utilized this approach so that we could verify that the code could be merged in and the tests would still pass. We ran into some git trouble during a round of pull requests where we had to go into detached head mode to navigate back to a previous version. With Kaida’s help, we were able to get back on track. Due to the amount of branches, we started deleting those that had already been merged.
What did your group do to try to keep your code DRY while many people collaborated on it? Communication was key for our ability to keep our code DRY. We used view partials for new/edit/update actions to avoid repeating forms. We also used private methods which are called before all controller actions relating to finding the current user and instantiating a cart if one does not already exist. Finally, we used global color CSS variables to dry up our CSS. We might, in the future, use view partials for each time we perform an iteration through a collection of objects in order to dry up our code.
What was a technical challenge that you faced as a group? We found it challenging to retain form data whenever one field fails. This especially crops up when a user enters billing information and runs up against model validation for this data. Meaningful errors flash for the user, but the input data is not retained for the user to edit. Additionally, we found it challenging right from the outset to manage migrations as a distributed group. In the future, we might instead perform all the migrations at once as a team on one machine to set up the baseline needs for our database. FYI - if you try to clone our repo, you may need to run "db:schema:load" if "db:migrate" doesn't work.
What was a team/personal challenge that you faced as a group? Individual tasks were assigned before the weekend and on return we had discovered that we had overlapped on many parts of the project. This overlap of work, although initially a pain point, helped our team better value communication and sticking to working on your own task assignments. A major challenge that we faced was merging a pull request that wasn’t compatible. Going back to a desired state was challenging due to our lack of experience with git and working with many developers.
What was your application's ERD? (include a link) https://www.lucidchart.com/invitations/accept/b67ed213-fe6c-43ac-9448-f34ed397d87f
What is your Trello URL? https://trello.com/b/NEapJFZP/betsy
What is the Heroku URL of your deployed application? https://sudsy.herokuapp.com/

NatalieTapias and others added 30 commits October 29, 2019 18:30
Seeds for categories and categoryproducts
@dtingg dtingg changed the title Leaves - Sudsy: Dani, Dianna, Katie, Natalie, Raisah Leaves - Sudsy (Dani, Dianna, Katie, Natalie, Raisah) Nov 4, 2019
@CheezItMan
Copy link

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 I can't create a product. I did notice if I tried to edit a category as a guest the site crashed rather than redirecting.
After logging in
Create a category
Create a product in that category with stock 10
Add the product you created to your cart Yes, but I don't see any visual feedback that it was successful.
Add it again (should update quantity) Yes
Verify unable to increase quantity beyond stock Check
Add another merchant's product Check
Check out Check
Check that stock was reduced Check
Change order-item's status on dashboard Check
Verify unable to leave a review for your own product Check
Verify unable to edit another merchant's product by manually editing URL Check
Verify unable to see another merchant's dashboard by manually editing URL Check

Code Review

Area yes / no
Routes
No un-needed routes generated (check reviews) A little weirdness in the routes file, like trying to except a put route. You also have a few unused routes defined.
Routes not overly-nested (check products and merchants) Check
Merchant dashboard and cart page use a non-parameterized routes (should pull merchant or cart ID from session) Check
Controllers
Controller-filter to require login by default Only in products controller
Helper methods or filters to find logged-in user, cart, product, etc You have some good filter helpers.
No excessive business logic Check
Business logic that ought to live in the model Check
Add / remove / update product on order Check
Checkout -> decrease inventory Check
Merchant's total revenue Check
Find all orders for this merchant (instance method on Merchant) Check
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
Check, well done
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
Check, well done
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)
Not enough negative tests here. I don't see a test to see if the product is inactive or without quantity
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
Doesn't check validations. Otherwise quite good

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 have a nice well organized and styled view, and covered all the essential features. The site functions and functions well. I also like the business logic you've put in the models. Your tests could use a bit more of a once over, but this is good.

I do see some room for improvement around better use of filters, and organizing tests and making sure you don't have unused routes.

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! You've should all be very proud of your teamwork and learning!

Only the person who submitted the PR will get an email about this feedback. Please let the rest of your team know about 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.

Nice work, you had well organized tests, and a really nicely functional store. Well done.

resources :orderitems, except: [:index, :put, :new, :show]
patch "/orderitems/:id/cancel/", to: "orderitems#cancel", as: "cancel"

resources :products, except: [:put]

Choose a reason for hiding this comment

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

You don't do except put you would do except update or something along those lines.

resources :reviews, only: [:new, :create]
end

resources :reviews, only: [:new, :create]

Choose a reason for hiding this comment

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

You shouldn't be able to make a review without a product in the URL

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.

6 participants