TLMatthies / smart_food_app

0 stars 0 forks source link

SCHEMA/API Design Comments Russell Ferrall #15

Open rferrall1738 opened 3 weeks ago

rferrall1738 commented 3 weeks ago

1) remove extra /users and /stores already defined in router = APIRouter( prefix="/users") and APIRouter( prefix="/stores") 2) Update data for longitude and latitude for the two stores because they are the same as of now 3) Naming conventions for tables, all of the other tables are lowercase why isn't Users? 4) Change the inserted values for one of the stores since they are not at the same location' 5) Use store_id and name to make a composite key since the name of the store shouldn't be null 6) API differs from the actual routes because of the of the redundant /users or /stores 7) The whole API endpoint catalogs.py could be deleted because the sole purpose of it is to get the catalog while there is already a get catalog route in stores. 8) 2.1 when creating a new user, what is the point of getting a user's location when that is not used in the creation of the user? 9) 2.4 I am confused on what name is? Is it the name of the list or the customer. If it is the customer then it doesn't seem necessary since user_id is also required. 10) 2.4, 2.5, 2.7 on the APISpec missing comma in the json responses 11) price is defined as an integer where it would be better use numeric to get a value like 10.99 unless this business doesn't believe in cents. 12) For food_int there are two kind of data types bigint and integer keep the conventions the same. 13) For post /users/users/{user_id}/lists/{list_id} it returns a 500 error. Looks like no food for me. For .post("/users/{user_id}/preferences")

Aiden-Rodriguez commented 3 days ago

5 Made store names not nullable instead. 8 I am honestly just confused on what this is saying. The user's location is stored when a user is created. 9 Added clarity to API spec for this. Name is the name of the list 11 Information is always stored as cents. Updated clarity here to make it clearer to user in the API