Code Review Comments- Ella Hagen

[elhagen13]

1. For both create_account and create_shop, I think just for the sake of clarity when testing, it might be better to return the account id that was created instead of just "ok." This way if you don't have access to the database you can still follow the flow of that specific account/shop id.
2. For create_shop, I would first check to make sure that the shop name isn't already in the database, so that way two sellers won't have the same shop name.
3. Similarly to the one above, for create_account, I would add a check to make sure that the email that's passed in isn't already in the users table, so that way it's not possible to have multiple accounts under the same email.
4. With the search function, it might be better to have it output a string representing the shoe instead of just the listing_id, otherwise it's kind of hard to see what shoe is actually for sale.
5. This is kind of trivial since it's not an actual shop, but right now carts_checkout doesn't have a way for customers to pay. With the potion shop there was a payment string, so maybe you could add that as well.
6. In create_listing, there isn't any error handling, so even if a transaction fails, it still returns "ok." This can be pretty easily fixed though with a try, except clause.
7. Similarly, create shop and create account return "ok" no matter what, so maybe consider adding more error handling in case something fails.
8. I saw this when testing, but it doesn't look like the search list is updated when a shoe is bought. I bought a pair of shoes, but then when I ran the search function again, the shoe was still there. I think it's because the search function is looking at the original quantity that was entered when the seller put in the shoe, but then the quantity is continuously changed through the ledger (everything works during checkout).
9. Also, since the quantity of a certain shoe is maintained in a ledger based system, there should be a check in the search function that sums the quantity and if the quantity is zero then it shouldn't be added to the return list.
10. For checkout, this also isn't super necessary, but instead of returning false if the inventory isn't enough, it might be better to return "insufficient inventory," or something like that, so that way it's more clear for the customer why their transaction didn't go through.
11. In create_cart, if you put in an account_id that doesn't exist, it will return an Internal Server Error, so it might be good to check first to see if it exists, or use a try/except statement.
12. Similarly to the one above, in set_item_quantity, if you put a non-existent listing id, it'll cause an Internal Server Error. So it might be good to add more extensive error handling so instead of just saying Internal Server Error, it'll explain what went wrong.
13. I'm not quite sure how to test this since the search function doesn't update, but I think that if someone has multiple items in their cart it will only checkout the first item since .first() is called, instead of doing something like for row in res: