search

n-shinde / PeakPeeps

Group Project for CSC 365
0 stars 0 forks source link

Code Review Comments Parshana Sekhon #11

Open Parshana007 opened 7 months ago

Parshana007 commented 7 months ago

Better Formatted Here: https://docs.google.com/document/d/1IDZithGogGBDjVl0LMlwllmq8nxXRsTdvXQ0FUo1FuI/edit?usp=sharing

  1. Users.py

    Post_create_account Include checking for if a user has already been added to not add again. Update_followers User_follows_other_user (500 Error) Honestly was kinda confused on update_followers and user_follows_other_users. Like could combine these two. Remove_follower Checking if the user doesn’t exist to return couldn’t remove. For example, they remove a user but then try to remove them again.

  2. Routes.py

Post_add_route (422 Error) Include a check to make sure that coordinates are only a list of size 2 for coordinates. Include duplicate checking for routes maybe based on if name, location, and coordinates are similar to something that already exists then it should instead add a count to the route_id that has been visited. Instead of having a user input the length in miles would it be cool to have like starting and ending coordinates and then find the distance between the two? Not sure if this is a good idea but the potential to make sure that people are accurately putting in the distance. Also mentioning that the coordinates are the start of the route might be good and the location is the city that it is in. Get_popular_routes (Not returning the right thing just an array of names of users like ["Steve", "Steve", "Lucas Pierce", "Steve"]) Add a check to make sure this isn’t a reported route. Only display not reported routes. Potentially adding the id for each route so that when people are reporting the route they can just provide the route_id. What happens if you have like 1,000 popular routes? Maybe adding some sort of filtering mechanism where users can filter by difficulty or by location for example. Get_followers_routes (500 Error) I kinda understand what is happening but how are you connecting a user to a friend? Might be better to have a connection between the user and the follower. So that you can only see your friends, not just any. Include some kind of pagination since what if you have a lot of friends? Have an endpoint that returns all my friends so that I can decide which friend/follower I want to look at. Report_route Only putting in the route_id instead of all the info might make it better.

  1. Reviews.py

    Post_add_review Check to make sure that a user isn’t writing a review to the same route_id. Also, add checks to make sure that the user has even done the route to be able to write a review on it. Could potentially have an update instead of an insert in case people want to adjust a review that they made. Adding a delete review in case the user wanted to delete this? Not sure if this would include taking away from peep coins.

  2. PeepCoins.py

Put_add_peepcoins Not sure if this is needed as a user can then just add as many peep coins to their account. Maybe change this to a get method that just returns the user and the current peep coins total. Post_buy_coupon There is a valid column mentioned here but I didn’t see that in the schema. The follow-up question is how are you determining if a coupon is valid? [Love the error checking btw] Your schema calls the coupon column cost but your code calls it price. I would say double-check the schema with your tables in Supabase.

  1. Coupons.py (500 Error)

    Add_coupon (can move me to business.py to have one file)

  2. Business.py Add_business

  3. General Improvement Recommendations:

Adding more error checking to all the endpoints instead of just “OK”. Also giving detailed error messages. What happens if a route is reported? Can people still view this route? Do people still get peep coins from this route? For the rating integer, you can do something similar to the pagination that we did in the potion shop where it gave a dropdown for the options to sort by but use a rating scale here. Note: I mentioned some places below in the API spec/ schema where I thought a drop-down might be good to have could implement this for those instances as well. It was hard to know what to put in for each endpoint as there is only “OK” returned for almost all functions making it hard to figure out my user_id, the routes, the total peep coins, and things like that. For example could have added the same route too many times. Wasn’t sure how to get the route_id information of a route I added to write a review on it.