Code Review Comments - Angelika Canete

[angelika930]

1. Function create_new_user does not match api spec, it should return a status string and user id but only returns a string

2. In users document make sure to take out leftover comment on line 45

3. In the /{user_id} endpoint for deleting a user, make sure to account for the case where a user attempts to delete a user account that does not exist instead of returning “OK”. Also, be careful if a user is capable of deleting other users accounts, unless that's out of scope for this class.

4. Consider adding additional code for /user/{user_id}/social in the case that a user attempts to follow a user that they already follow, it currently throws a 500 server error.

5. Consider changing the logic when joining on a users.id, perhaps users.user_pkey would ideologically be more related to social.user_id rather than the default users.id that is created.

6. Consider using a with statement for the sql query in /{user_id}/social/{following_id}. The portion:

   SELECT following_id
                    FROM users
                    WHERE users.name = :following_name

is repeated in both the delete and select statements. Using a with statement can help cut down on redundancy and improve the clarity of the SQL query.

7. In /{user_id}/social/{following_id}, consider using .scalar_one() over .first(). This is because the function should expect to have one row returned and scalar_one() will throw an error if more or fewer rows are returned.

8. Match the HTTP endpoints in code with API spec:

Api spec: user/{user_id}/social/{follower_name} (DELETE) Code: /{user_id}/social/{following_id}

9. user/{user_id}/social/{follower_name}/catalogs/{catalog}/recommendations expects a JSON object to get returned but instead receives an array in the following code:

    if result == None:
        print(f"User is not following {following_name}")
        return []

10. Take out this comment in game_entries.py -insert into catalog table a new row with unqiue catalog id -do we want this to have a composite key for userid, catalog id, and entry id (ie user 1 catalog 1 entry 1, user 2 catalog 1 entry 1 etc)"

11. Overall, I think having more consistency across the files would be good. Specifically, I noticed that data_sets.py was more commented than others. Additionally, spacing for SQL calls differs in game_entries.py vs. user_functions.py.

12. After using the /user/{user_id}/catalogs endpoint with user_id 12 I put in the following request body:

{
  "name": "games",
  "type": "video game",
  "private": true
}

But received an error 500 HTTP response.

13. In the files book_entries.py, game_entries.py, and movie_entries.py you have three functions for checking if a catalog belongs to a user, checking if an entry exists and if that book, game , or movie exists then calling them in another function. The connection string with db.engine.begin() as connection: Is used in multiple places, opening and closing the connection which may be slow. Consider restructuring the code so that the helper functions have the SQL call but do not all have the connection string in each function.

14. In version 2 example workflow 1, make sure to specify which entries endpoint you are using on line 37. Currently: ### POST users/{user_id}/catalog/{catalog_id}/entries: Change to: ### POST users/{user_id}/catalog/{catalog_id}/other_entries:

15. Update example workflow 2 on v2 testing results on line 129. GET user/{user_id}/social/{follower_id}/catalogs: This endpoint no longer exists but I believe follower_id should be replaced with follower_name.

16. I get a 500 internal server error on /user/{user_id}/social. I’m not quite sure why but my assumption is that I am trying to follow a user that does not exist in the database. Please provide additional logic to catch this error and alert the user that the user_id or user_name inputted may not exist.