search

2022-Spring-NYU-DevOps-Shopcarts / shopcarts

Shopcarts squad.
Apache License 2.0
2 stars 2 forks source link

initial working on reading shopcarts #32

Closed Adora2401 closed 2 years ago

Adora2401 commented 2 years ago

This is the initial work of reading shopcarts. Rewrite the function get_shopcarts in routes.py Added _create_items, test_get_shopcart, test_get_shopcart_not_found, test_method_not_supported in test_routes.py Added find and find_or_404 in models.py

kdokm commented 2 years ago

Thank you for your work! Here are my comments:

  1. Since our 'read shopcarts' should really list items in the given shopcart as the story specifies, in 'get_shopcarts', you may want to use a list like '[item.serialize() for item in shopcart]' except you need to filter out where item_id == -1 since it's a dummy value, and then jsonify it.

  2. For the same reason, in 'test_get_shopcarts', you may want to get a list of items based on the id, instead of getting the id itself.

  3. I'm not sure if 'find' and 'find_or_404' will work well, and I think maybe the original 'find' functions for shopcarts and items can cover all the needs.

If there are any disagreement or confusion, please let me know!

Adora2401 commented 2 years ago

Thank you for Yuwen & Jingwei's advice and help. This version: deleted the 'find' and 'find_or_404'; corrected the logic of 'read-shopcarts' including the changes of '_create_items', 'test_get_shopcart', 'get_shopcarts'.

TimothyXu commented 2 years ago

I think you accidentally merged your own request (per agile/homework guidelines, we cannot approve our own pull requests). Let's revert this and create another pull request for someone else to prove (e.g. me)

Adora2401 commented 2 years ago

Thanks so much for your notification. It's an accidently merge. I will create a pull request again.