Skip to content

Commit cfae5c2

Browse files
Clue88Christopher Liu
and
Christopher Liu
authored
Add user profile pictures (#140)
* Add profile pic to User model * Add profile pic upload endpoint * Add pillow * Add documentation about pipfile problems * Switch to aws s3 * Prevent uploading non-images * Add tests and move s3 to prod * Add more tests * Simplify image uploader code * Clarify image filepath generator * Add input checking * Add env variables to readme * More tests :/ * ¯\_(ツ)_/¯ * Specify http status code in tests * Add file size check Co-authored-by: Christopher Liu <[email protected]>
1 parent afbdb82 commit cfae5c2

15 files changed

+506
-2
lines changed

Diff for: .gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,6 @@ db.sqlite3
2525

2626
# Docker
2727
docker-compose.yml
28+
29+
# Uploaded media files
30+
backend/accounts/mediafiles

Diff for: README.md

+3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ DATABASE_URL=mysql://USER:PASSWORD@HOST:PORT/NAME
1818
SECRET_KEY=secret
1919
DJANGO_SETTINGS_MODULE=Platform.settings.production
2020
SENTRY_URL=https://[email protected]/product
21+
AWS_ACCESS_KEY_ID
22+
AWS_ACCESS_SECRET_ID
23+
AWS_STORAGE_BUCKET_NAME
2124
```
2225

2326
1. Run using docker: `docker run -d pennlabs/platform`

Diff for: backend/Pipfile

+3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ django-email-tools = "*"
3636
twilio = "*"
3737
lxml = "*"
3838
requests = "*"
39+
pillow = "*"
40+
boto3 = "*"
41+
django-storages = "*"
3942

4043
[requires]
4144
python_version = "3"

Diff for: backend/Pipfile.lock

+224-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: backend/Platform/settings/base.py

+5
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
"options.apps.OptionsConfig",
7474
"accounts.apps.AccountsConfig",
7575
"identity.apps.IdentityConfig",
76+
"storages",
7677
]
7778

7879

@@ -203,3 +204,7 @@
203204
"FROM_EMAIL": "Penn Labs <[email protected]>",
204205
"TEMPLATE_DIRECTORY": os.path.join(BASE_DIR, "Platform", "templates", "emails"),
205206
}
207+
208+
# Media Upload Settings
209+
MEDIA_ROOT = os.path.join(BASE_DIR, "accounts", "mediafiles")
210+
MEDIA_URL = "/media/"

Diff for: backend/Platform/settings/production.py

+7
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,10 @@
4343
EMAIL_USE_TLS = True
4444

4545
IS_DEV_LOGIN = os.environ.get("DEV_LOGIN", "False") in ["True", "TRUE", "true"]
46+
47+
# AWS S3
48+
DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"
49+
AWS_ACCESS_KEY_ID = os.getenv("AWS_ACCESS_KEY_ID")
50+
AWS_ACCESS_SECRET_ID = os.getenv("AWS_SECRET_ACCESS_KEY")
51+
AWS_STORAGE_BUCKET_NAME = os.getenv("AWS_STORAGE_BUCKET_NAME")
52+
AWS_QUERYSTRING_AUTH = False

Diff for: backend/README.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
## Runbook
2+
This section is to collect thoughts/learnings from the codebase that have been hard-won, so we don't lose a record of it if and when the information proves useful again
3+
4+
### Dependency Installation on MacOS Ventura
5+
There's a couple of issues at play here. The first is that the current `Pipfile` and `Pipfile.lock` are out of sync, most significantly with regard to Django--the `Pipfile` would generate a lockfile with Django 4, however, we are using Django 3 in the current lockfile and our tests. For now, use the current lockfile and use `--keep-outdated` when adding a new package with `pipenv`.
6+
7+
However, there seems to be an issue with the way that lockfiles are interacting with MacOS Ventura. In that case, we have to ignore the lockfile entirely, using `pipenv install --dev --python 3.8 --skip-lock` to install dependenices. From what I can tell, this is the only way to actually install the dependencies without it failing, but it means that some tests might fail due to issues with Django 4.
8+
9+
The long term solution is to make sure that the `Pipfile` and lockfile are in sync, either by forcing Django 3 or by updating our code to support Django 4.

Diff for: backend/accounts/migrations/0004_user_profile_pic.py

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Generated by Django 4.1.3 on 2022-11-10 02:05
2+
3+
import accounts.models
4+
from django.db import migrations, models
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
("accounts", "0003_auto_20210918_2041"),
11+
]
12+
13+
operations = [
14+
migrations.AddField(
15+
model_name="user",
16+
name="profile_pic",
17+
field=models.ImageField(
18+
blank=True, null=True, upload_to=accounts.models.get_user_image_filepath
19+
),
20+
),
21+
]

Diff for: backend/accounts/models.py

+13
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import os
12
import uuid
23

34
from django.contrib.auth import get_user_model
@@ -9,6 +10,15 @@
910
from phonenumber_field.modelfields import PhoneNumberField
1011

1112

13+
def get_user_image_filepath(instance, fname):
14+
"""
15+
Returns the provided User's profile picture image path. Maintains the
16+
file extension of the provided image file if it exists.
17+
"""
18+
suffix = "." + fname.rsplit(".", 1)[-1] if "." in fname else ""
19+
return os.path.join("images", f"{instance.username}{suffix}")
20+
21+
1222
class School(models.Model):
1323
"""
1424
Represents a school at the University of Pennsylvania.
@@ -53,6 +63,9 @@ class User(AbstractUser):
5363
pennid = models.IntegerField(primary_key=True)
5464
uuid = models.UUIDField(unique=True, default=uuid.uuid4, editable=False)
5565
preferred_name = models.CharField(max_length=225, blank=True)
66+
profile_pic = models.ImageField(
67+
upload_to=get_user_image_filepath, blank=True, null=True
68+
)
5669

5770
VERIFICATION_EXPIRATION_MINUTES = 10
5871

Diff for: backend/accounts/serializers.py

+2
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ class UserSerializer(serializers.ModelSerializer):
174174
student = StudentSerializer()
175175
emails = EmailSerializer(many=True)
176176
phone_numbers = PhoneNumberSerializer(many=True)
177+
profile_pic = serializers.ImageField(required=False, allow_empty_file=True)
177178

178179
class Meta:
179180
model = User
@@ -189,6 +190,7 @@ class Meta:
189190
"student",
190191
"phone_numbers",
191192
"emails",
193+
"profile_pic",
192194
)
193195

194196
read_only_fields = (

Diff for: backend/accounts/urls.py

+4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from django.conf import settings
2+
from django.conf.urls.static import static
23
from django.urls import path
34
from oauth2_provider.views import AuthorizationView, TokenView
45
from rest_framework import routers
@@ -12,6 +13,7 @@
1213
MajorViewSet,
1314
PhoneNumberViewSet,
1415
ProductAdminView,
16+
ProfilePicViewSet,
1517
SchoolViewSet,
1618
UserSearchView,
1719
UserView,
@@ -24,6 +26,7 @@
2426
router = routers.SimpleRouter()
2527
router.register("me/phonenumber", PhoneNumberViewSet, basename="me-phonenumber")
2628
router.register("me/email", EmailViewSet, basename="me-email")
29+
router.register("me/pfp", ProfilePicViewSet, basename="me-pfp")
2730
router.register("majors", MajorViewSet, basename="majors")
2831
router.register("schools", SchoolViewSet, basename="schools")
2932

@@ -42,3 +45,4 @@
4245
]
4346

4447
urlpatterns += router.urls
48+
urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)

Diff for: backend/accounts/views.py

+94-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.contrib.auth.models import Permission
88
from django.contrib.contenttypes.models import ContentType
99
from django.core.exceptions import ObjectDoesNotExist
10+
from django.core.files.uploadedfile import UploadedFile
1011
from django.db.models import Case, IntegerField, Q, Value, When
1112
from django.http import HttpResponseServerError
1213
from django.http.response import HttpResponse, HttpResponseBadRequest
@@ -19,7 +20,7 @@
1920
from oauth2_provider.models import get_access_token_model
2021
from oauth2_provider.views import IntrospectTokenView
2122
from oauth2_provider.views.mixins import ProtectedResourceMixin
22-
from rest_framework import generics, viewsets
23+
from rest_framework import generics, status, viewsets
2324
from rest_framework.decorators import action
2425
from rest_framework.filters import SearchFilter
2526
from rest_framework.permissions import IsAuthenticated
@@ -39,6 +40,44 @@
3940
from accounts.verification import sendEmailVerification, sendSMSVerification
4041

4142

43+
def image_upload_helper(request, user, keyword, field):
44+
"""
45+
Given a User object, save the uploaded image to the image field specified
46+
in the argument. Returns a response that can be given to the end user.
47+
48+
keyword: the key corresponding to the image file in the request body
49+
field: the name of the User model field to which the file is saved
50+
"""
51+
if keyword not in request.data or not isinstance(
52+
request.data[keyword], UploadedFile
53+
):
54+
return Response(
55+
{"detail": "No image file was uploaded!"},
56+
status=status.HTTP_400_BAD_REQUEST,
57+
)
58+
59+
if "image" not in request.data[keyword].content_type:
60+
return Response(
61+
{"detail": "You must upload an image file!"},
62+
status=status.HTTP_400_BAD_REQUEST,
63+
)
64+
65+
if request.data[keyword].size > 500000:
66+
return Response(
67+
{"detail": "Image files must be smaller than 500 KB!"},
68+
status=status.HTTP_400_BAD_REQUEST,
69+
)
70+
71+
getattr(user, field).delete(save=False)
72+
setattr(user, field, request.data[keyword])
73+
return Response(
74+
{
75+
"detail": f"{user.__class__.__name__} image uploaded!",
76+
"url": getattr(user, field).url,
77+
}
78+
)
79+
80+
4281
class LoginView(View):
4382
"""
4483
Log in a user.
@@ -247,6 +286,60 @@ def get_object(self):
247286
return self.request.user
248287

249288

289+
class ProfilePicViewSet(viewsets.ViewSet):
290+
"""
291+
post:
292+
Replace the profile picture of the logged in user with a new photo.
293+
"""
294+
295+
serializer_class = UserSerializer
296+
permission_classes = [IsAuthenticated]
297+
queryset = User.objects.all()
298+
299+
@action(detail=False, methods=["post"])
300+
def upload(self, request, *args, **kwargs):
301+
"""
302+
Upload a profile picture.
303+
---
304+
requestBody:
305+
content:
306+
multipart/form-data:
307+
schema:
308+
type: object
309+
properties:
310+
profile_pic:
311+
type: object
312+
format: binary
313+
responses:
314+
"200":
315+
description: Returned if the file was successfully uploaded.
316+
content: &upload_resp
317+
application/json:
318+
schema:
319+
type: object
320+
properties:
321+
detail:
322+
type: string
323+
description: The status of the file upload.
324+
url:
325+
type: string
326+
nullable: true
327+
description: >
328+
The URL of the newly uploaded file.
329+
Only exists if the file was
330+
successfully uploaded.
331+
"400":
332+
description: Returned if there was an error while uploading the file.
333+
content: *upload_resp
334+
---
335+
"""
336+
user = self.request.user
337+
resp = image_upload_helper(request, user, "profile_pic", "profile_pic")
338+
if status.is_success(resp.status_code):
339+
user.save(update_fields=["profile_pic"])
340+
return resp
341+
342+
250343
class PhoneNumberViewSet(viewsets.ModelViewSet):
251344
"""
252345
retrieve:

Diff for: backend/tests/accounts/test_pfp.jpg

72.1 KB
Loading

Diff for: backend/tests/accounts/test_pfp_large.png

1.03 MB
Loading

0 commit comments

Comments
 (0)