[enhancement] Canvas server #11
Conversation
atb00ker
force-pushed
the
canvas-server
branch
from
cbe24c4
to
55fb631
Compare
There was a problem hiding this comment.
Awesome work, Vallari.
Do as many of these changes you find time for today and create an issue for the rest to do them when you find time in the future! 😄
Let me know when you are done fixing / making issues, I'll merge it.
To fix the commit problem, I did:
git fetch origin/main
git rebase origin main
| @@ -0,0 +1,24 @@ | |||
| # Generated by Django 3.2.9 on 2021-11-18 16:59 | |||
|
|
|||
| from django.db import migrations, models | |||
There was a problem hiding this comment.
Usually (not mandatory), we give descriptive names to files instead of 0002_auto_20211118_1659.py.
It's also nice to summarize the work of one PR in one migration file only. (Unless multiple make sense!) to keep the number of migration files low.
There was a problem hiding this comment.
Oh, I've created an issue for this. Let me know if I do merging and renaming of these migration files right.
| class Canvas(models.Model): | ||
| id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) | ||
| image_path = models.CharField(max_length=128) | ||
| username = models.ForeignKey(User, related_name='canvas', on_delete=models.CASCADE) |
There was a problem hiding this comment.
we would need a drawing column here as well, do store the image drawings. 😄
There was a problem hiding this comment.
done! It created another migration file. I'll merge all three of them in one.
| fields = ( | ||
| 'id', | ||
| 'image_path', | ||
| 'username', |
There was a problem hiding this comment.
We don't need username in our response.
| def get(self, request): | ||
| username = request.user | ||
| all_canvas = Canvas.objects.filter(username=username) | ||
| serializer = CanvasSerializer(all_canvas, many=True) | ||
| return Response(serializer.data) |
There was a problem hiding this comment.
Want to do this in less lines?
Checkout List API View: https://www.django-rest-framework.org/api-guide/generic-views/#listapiview
(You can check it out in the future, right now this will work.)
There was a problem hiding this comment.
This is awesome! 🎉
They will probably handle any error conditions I missed like not passing correct data in body..
I'll change all these to generic-views later 👍
src/server/canvas/views.py
Outdated
| def post(self, request): | ||
| img_path = request.data["image_path"] | ||
| canvas_id = uuid.uuid4() | ||
| new_canvas = { | ||
| 'image_path': img_path, | ||
| 'id': canvas_id, | ||
| 'username': str(request.user) | ||
| } | ||
| serializer = CanvasSerializer(data=new_canvas) | ||
| if not serializer.is_valid(): | ||
| return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| canvas = Canvas.objects.create(username=request.user, image_path=img_path, id=canvas_id) | ||
| canvas.save() | ||
| return Response(serializer.data, status=status.HTTP_201_CREATED) |
There was a problem hiding this comment.
Checkout CreateAPIView: https://www.django-rest-framework.org/api-guide/generic-views/#createapiview
(You can check it out in the future, right now this will work.)
| class CanvasWithIDView(APIView): | ||
| permission_classes = (IsAuthenticated,) | ||
| authentication_classes = (TokenAuthentication,) | ||
|
|
||
|
|
||
| def get(self, request, *args, **kwargs): | ||
| canvas_id = self.kwargs['canvas_id'] | ||
| all_canvas = Canvas.objects.filter(id=canvas_id) | ||
| serializer = CanvasSerializer(all_canvas, many=True) | ||
| return Response(serializer.data) | ||
|
|
||
| def post(self, request, *args, **kwargs): | ||
| img_path = request.data["image_path"] | ||
| canvas_id = self.kwargs['canvas_id'] | ||
| new_canvas = { | ||
| 'image_path': img_path, | ||
| 'id': canvas_id, | ||
| 'username': str(request.user) | ||
| } | ||
| serializer = CanvasSerializer(data=new_canvas) | ||
| if not serializer.is_valid(): | ||
| return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| canvas = Canvas.objects.filter(id=canvas_id) | ||
| canvas.update(image_path=img_path) | ||
| return Response(serializer.data, status=status.HTTP_201_CREATED) |
There was a problem hiding this comment.
Checkout RetrieveUpdateAPIView: https://www.django-rest-framework.org/api-guide/generic-views/#retrieveupdateapiview
(You can check it out in the future, right now this will work.)
| @@ -19,6 +19,6 @@ | |||
|
|
|||
| urlpatterns = [ | |||
| path("api/v1/", include("server.users.urls")), | |||
| path("api/v1/", include("server.face_paint.urls")), | |||
| path("api/v1/image/", include("server.canvas.urls")), | |||
There was a problem hiding this comment.
Usually, more specific path comes first.
Imagine if someone creates image/ path in server.users.urls mistakenly.
Wouldn't all our request get to that module instead of this one?
(Because django matches URL patterns in order, first come, first serve!)
path("api/v1/image/", include("server.canvas.urls")),
path("api/v1/", include("server.users.urls")),
There was a problem hiding this comment.
That makes sense, I understand! Fixed it :)
atb00ker
force-pushed
the
canvas-server
branch
from
9504799
to
4fdc588
Compare
Solves #3
GET /api/v1/image/ -> return all images of the signed in user
POST /api/v1/image/ -> add a new image
GET /api/v1/image/<canvas_id> -> gets the image by id
POST /api/v1/image/<canvas_id> -> updates the image location
