-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Schema/API Design comment Luke Fanguna #10
Comments
this feels like a double entry. Couldn't you just use the user_id to get the author name? or would this be two different people. PeakPeeps/dataFiles/schema.sql Line 34 in c5e21b2
|
this references "users" but the table is not defined anywhere. Possibly a mistake? PeakPeeps/dataFiles/schema.sql Line 41 in c5e21b2
|
i see a lot of "user_id" but i don't see any user table. This makes me think that user_id is just inserted to every table but doesn't have any information linked to it. PeakPeeps/dataFiles/schema.sql Line 64 in c5e21b2
|
What exactly is "user_test"? Is this a first draft of the users table? PeakPeeps/dataFiles/schema.sql Line 82 in c5e21b2
|
Although this is mentioned in the specs, it looks like there is no implementation. If there will not be an implementation, remove from APISpec.md because it can be misleading. Line 5 in c5e21b2
|
'routes/add' does not match what it says in APISpec.md. this is what APISpec.md requests: [
{
"name": "string",
"date_added": "string",
"user_added": "string",
"location": "string",
"coordinates": [lat, long],
"length_in_miles": "double",
"difficulty": "string",
"activities": "string",
}
] but this is what it requests in routes.py: [
{
"name": route_to_add.name,
"user_id": route_to_add.user_id,
"location": route_to_add.location,
"length_in_miles": route_to_add.length,
"difficulty": route_to_add.difficulty,
"activities": route_to_add.activities,
"coords": route_to_add.coordinates,
}
] You have to match requests with the APISpec.md because it would be hard for a user to use it if the requests don't match the specs. Same goes for the return statement. It says returns a boolean when it actually returns "OK" Line 26 in c5e21b2
|
Again, the requests do not match with the specs. Line 96 in c5e21b2
|
Doesn't match the specs. The /popular endpoint request nothing and returns a list of routes. The json is also formatted weird. It is a list of a json that contains a list that is formatted like a json [
{
[ /* List of routes with the following attributes displayed:
"name": "string",
"date_added": "string",
"user_added": "string",
"location": "string",
"coordinates": [lat, long],
"length_in_miles": "double",
"difficulty": "string",
"activities": "string",
"popularity_index": int,
"five_star_reviews": int
]
}
] should be: [
{
/* List of routes with the following attributes displayed:
"name": "string",
"date_added": "string",
"user_added": "string",
"location": "string",
"coordinates": [lat, long],
"length_in_miles": "double",
"difficulty": "string",
"activities": "string",
"popularity_index": int,
"five_star_reviews": int
}
] Line 120 in c5e21b2
|
This endpoint was renamed to /followers in the code but not in the specs. Should fix that. Also, the same format issue with the return. list of json of list formatted like a json. Line 145 in c5e21b2
|
Code does not match the specs. /new was turned into /add and the requests/return is different. Line 182 in c5e21b2
|
Code doesn't match specs. Request/return is not the same. Line 210 in c5e21b2
|
The return in the code is different from the specs. Line 243 in c5e21b2
|
This table is quite large and holds a lot of info. I would say to create smaller tables to help spread the information, while keeping it organized. For example, I would create table called route_info that used the "name" as a primary key and hold the information of the route.
PeakPeeps/dataFiles/schema.sql
Line 46 in c5e21b2
The text was updated successfully, but these errors were encountered: