CRUD APIs for Locations model #75
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Please look into the review comments with a pinch of salt and also make sure merge develop branch into this branch so that current conflicts could be resolved.
If you have any questions or any other comments regarding my suggestions, please feel free to discuss them here or any other place.
You could also ignore some comments that you think are not important at the moment but be sure to comment that you are ignoring it with a reason.
Great work, thank you! :)
furbaby/api/views.py
Outdated
| locations = Locations.objects.filter(user_id=user_id) | ||
| serialized_data = serialize("json", locations) | ||
| serialized_data = json.loads(serialized_data) | ||
| return serialized_data |
There was a problem hiding this comment.
let's try something else with this? I see that the response is still a JSON string instead of it being a JSON array
I'd really appreciate it if you could spend a little bit more time on this and get a better way of providing the response.
There was a problem hiding this comment.
We could create a list (ie array) of dictionaries containing the locations. Is that an acceptable format for the response?
There was a problem hiding this comment.
yeah, this works for now, I mean...we're not going to be dealing with 1000s of objects where performance of the API is affected at the moment. But in the meanwhile, if we do find something that can work...we should definitely come back to this and replace it! 👍
| except Locations.DoesNotExist: | ||
| return json_response( | ||
| data={ | ||
| "error": "location not found for user", | ||
| "location id": location_id, | ||
| "user id": request.user.id, | ||
| }, | ||
| status=status.HTTP_404_NOT_FOUND, | ||
| ) |
There was a problem hiding this comment.
nit-pick:
what you've done here is completely fine, but I was thinking that if we're going to check it every time, it can become a little redundant. What do you think of making it one common test in the main handler and we assume that the location is always present when you're in this function?
There was a problem hiding this comment.
Shall we take this up in the next PR?
|
Don't merge this yet please, there's an issue with Travis CI and so deployments won't go through. Just hold on until that has been figured out? You can see the error coming up here: https://app.travis-ci.com/github/gcivil-nyu-org/INET-Monday-Fall2023-Team-1?serverType=git
|
Added code for the locations API POST - Inserts a new location record for the logged in user (see image 1), it returns the location_id of the newly created record in the DB <img width="902" alt="image 1" src="https://github.com/gcivil-nyu-org/INET-Monday-Fall2023-Team-1/assets/57152208/698d5cab-370e-469e-b64f-50824951750a"> GET - It fetches all the locations for the logged in user (see image 2) <img width="1083" alt="image 2" src="https://github.com/gcivil-nyu-org/INET-Monday-Fall2023-Team-1/assets/57152208/1798a0f0-1165-428b-abbe-8fc4cf942c10"> PUT/PATCH - It takes as input location id and location details to update and returns the updated record (see image 3) <img width="1081" alt="image 3" src="https://github.com/gcivil-nyu-org/INET-Monday-Fall2023-Team-1/assets/57152208/cc4d4b12-9136-4a59-b2ce-4b81dd824663"> DELETE - Deletes the record for the given location id (see image 4) <img width="1079" alt="image 4" src="https://github.com/gcivil-nyu-org/INET-Monday-Fall2023-Team-1/assets/57152208/89b02640-e36a-4d91-903d-eb080bdca8bc">
Added code for the locations API
POST - Inserts a new location record for the logged in user (see image 1), it returns the location_id of the newly created record in the DB
GET - It fetches all the locations for the logged in user (see image 2)
PUT/PATCH - It takes as input location id and location details to update and returns the updated record (see image 3)
DELETE - Deletes the record for the given location id (see image 4)