rcereno / REDX

0 stars 0 forks source link

Schema/API Design comments (Isaac Schiffler) #7

Open isaacschiffler opened 6 months ago

isaacschiffler commented 6 months ago
  1. API 1. (Get Catalog) - "sku": "string" / Matching regex ^[a-zA-Z0-9_]{1,20}$ / -> make sure the commented specifications, like length of string, are accurate. Some items in response of /catalog/ have an sku longer than 20 chars
  2. API 3. (Add Item to Cart) - Missing request specification in the documentation of the API Spec.
  3. API 7.1 (Account Registration) - Should this endpoint return the account id of the new created account? How would they already have an account id if they have not registered yet. Maybe change this so when registering with a name and email, the endpoint returns the new account id associated with the new registerer.
  4. API 7.3 (Cart View) - This API endpoint is inconsistent with the actual implementation of the endpoints in your api files. Decide whether you want to retrieve the view of the cart from the accounts API file or the carts API file. Likely leave in carts API file and then call cart view using the current cart id from account view. Make sure to make these changes in the APIspec file as well to match implementation.
  5. API 8 (Add to Wishlist) - This endpoint is also inconsistent with implementation. The API spec request should include the item sku that the account wants to add to their wishlist rather than the customer_name (provided account_id stays in the request). Also response doesn't seem to match as well.
  6. API 5 (Current Time) - I think this can be removed. I'm not sure what use if provides to your project. Seems like it was probably just left in there when you were using the potion shop api spec as a template.
  7. API 9 (Review Game) - Implementation and spec seems to be inconsistent. Likely need to change the api spec documentation to match that it is a endpoint of the accounts API file. Also, consider returning the new review id rather than just true/false success boolean.
  8. SQL Schema carts table - Rather than storing the total_games and total_cost in each individual cart, just sum up the games and cost for the given cart from cart_items on the fly when it is needed, or even just create a view that has this information. This will remove the need to update the cart row whenever a new game is added to the cart.
  9. SQL Schema - Remove commented out code, like the customers table, if you think it won't be used.
  10. SQL Schema games table - Think about maybe switching the price column to the dollar amount rather than having the value stored in cents for simplicity's sake. Storing the value in cents is viable still, so it is mostly just a preference.
  11. SQL Schema - Consider making certain columns not nullable. For example, in the games table, you probably wouldn't want to have a game without an sku, name, and price. However, missing the release_date is more permissible. Maybe make the three columns I listed first as 'not null' rather than 'null'. (
    item_sku text null,
    name text null
  12. SQL Schema (carts for accounts) - Perhaps instead of having a checked_out column for each cart, maybe remove it and use a current_cart column in the accounts table for the given account as a column to either hold the cart_id of the current open cart, or null if no cart is open. This will directly link the account to its current open cart if they have one. Then, in the carts table, the carts can still be linked to the account with the account_id column so all past carts can be linked to the account. This will make keeping track of past carts that were never checked out more difficult, but it will make the link between an account and their current cart stronger. It is a trade-off that you can consider based on your greater idea for this project. (Could also just add the current_cart column to accounts but leave in the checked_out column in carts, but then you need to decide when a non-checked-out cart gets closed for the account if you want to keep non-checked-out carts and make them close to the account after a certain amount of time.

Less important but going back through the api spec markdown file and fixing all the numbering would also help.

isaacschiffler commented 6 months ago

Another new comment I thought of while doing the test flows. Why is there a /catalog/ and /catalog/search/ endpoint? The search one just seems to be /catalog/ but including optional filters. Do you need both?

isaacschiffler commented 6 months ago

Assuming you are selling digital versions of the game, so you can sell unlimited once they're in your inventory, but if you are selling game CDs, make sure to add quantity in stock.