Merge pull request #187 from avantifellows/image_upload

NEW: image model + upload to s3

.env.example

@@ -25,6 +25,7 @@ GOOGLE_OAUTH2_CLIENT_SECRET=
 AWS_ACCESS_KEY_ID=
 AWS_SECRET_ACCESS_KEY=
 AWS_REGION=
+AWS_STORAGE_BUCKET_NAME=
 
 # redis details
 REDIS_HOSTNAME='redis'
@@ -62,5 +63,5 @@ EMAIL_HOST_PASSWORD=
 
 # The driver for sending SMSs. Possible values are `sns` or `log`.
 # Use `sns` to have AWS SNS support. The AWS credentials must be present for this.
-# Use `log` to log SMSs into a file instead. Recommended for development mode.
+# Use an empty string to log SMSs into a file instead. Recommended for development mode.
 SMS_DRIVER='sns'

.github/workflows/ci.yml

@@ -50,6 +50,10 @@ jobs:
           DB_USER: postgres
           DB_PASSWORD: postgres
           SECRET_KEY: wpurj&oym6m@kcp(m&z(q-g0bo-r*+!f_&j(94di8j&_j4m%2s  # random secret key
+          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
+          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
+          AWS_REGION: ${{ secrets.AWS_REGION }}
+          AWS_STORAGE_BUCKET_NAME: ${{ secrets.AWS_STORAGE_BUCKET_NAME }}
         # command to run tests and generate coverage metrics
         run: coverage run manage.py test
 

docs/ENV.md

@@ -70,6 +70,9 @@ AWS secret access key.
 #### `AWS_REGION`
 Region of the AWS IAM user.
 
+#### `AWS_STORAGE_BUCKET_NAME`
+AWS bucket where `django-storages` uploads the files
+
 #### `SMS_DRIVER`
 The driver to send sms. The only supported value is `sns` right now for AWS SNS. When in development mode, use an empty string to avoid SMS triggers while debugging/testing.
 

plio/migrations/0021_auto_20210613_1711.py

@@ -0,0 +1,44 @@
+# Generated by Django 3.1.1 on 2021-06-13 17:11
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("plio", "0020_question_has_char_limit"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="Image",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                ("deleted", models.DateTimeField(editable=False, null=True)),
+                ("url", models.ImageField(upload_to="images", verbose_name="Image")),
+                ("created_at", models.DateTimeField(auto_now_add=True)),
+                ("updated_at", models.DateTimeField(auto_now=True)),
+            ],
+            options={
+                "db_table": "image",
+            },
+        ),
+        migrations.AddField(
+            model_name="question",
+            name="image",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.DO_NOTHING,
+                to="plio.image",
+            ),
+        ),
+    ]

plio/migrations/0022_image_alt_text.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.1.1 on 2021-06-14 13:38
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("plio", "0021_auto_20210613_1711"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="image",
+            name="alt_text",
+            field=models.CharField(default="Image", max_length=255),
+        ),
+    ]

plio/models.py

@@ -2,10 +2,49 @@
 from django.db import models
 import string
 import random
+import os
 from safedelete.models import SafeDeleteModel, SOFT_DELETE
 from plio.config import plio_status_choices, item_type_choices, question_type_choices
 
 
+class Image(SafeDeleteModel):
+    _safedelte_policy = SOFT_DELETE
+
+    url = models.ImageField("Image", upload_to="images")
+    alt_text = models.CharField(max_length=255, default="Image")
+    created_at = models.DateTimeField(auto_now_add=True)
+    updated_at = models.DateTimeField(auto_now=True)
+
+    class Meta:
+        db_table = "image"
+
+    def __str__(self):
+        return f"{self.id}: {self.url}"
+
+    def _generate_random_string(self, length=10):
+        """Generates a random string of given length."""
+        return "".join(random.choices(string.ascii_lowercase, k=length))
+
+    def _generate_unique_uuid(self):
+        """Generates a unique file name for the image being uploaded."""
+        new_file_name = self._generate_random_string()
+
+        while Image.objects.filter(url__icontains=new_file_name).exists():
+            new_file_name = self._generate_random_string()
+        return new_file_name
+
+    def save(self, *args, **kwargs):
+        """Image save method. Before uploading, it creates a unique file name for the image."""
+        # to avoid setting a new name every time the model is saved
+        if not self.id:
+            dir_name = os.path.dirname(self.url.name)
+            file_name = "".join(
+                [self._generate_unique_uuid(), os.path.splitext(self.url.name)[1]]
+            )
+            self.url.name = os.path.join(dir_name, file_name)
+        super().save(*args, **kwargs)
+
+
 class Video(SafeDeleteModel):
     _safedelete_policy = SOFT_DELETE
 
@@ -86,6 +125,7 @@ def __str__(self):
 class Question(SafeDeleteModel):
     _safedelete_policy = SOFT_DELETE
 
+    image = models.ForeignKey(Image, null=True, on_delete=models.DO_NOTHING)
     item = models.ForeignKey(Item, on_delete=models.DO_NOTHING)
     type = models.CharField(
         max_length=255, choices=question_type_choices, default="mcq"

plio/permissions.py

@@ -1,6 +1,7 @@
 from rest_framework import permissions
 from organizations.middleware import OrganizationTenantMiddleware
 from plio.settings import DEFAULT_TENANT_SHORTCODE
+from plio.models import Plio, Item, Question
 from users.models import OrganizationUser
 
 
@@ -14,7 +15,14 @@ def has_permission(self, request, view):
         return True
 
     def has_object_permission(self, request, view, obj):
-        """Object-level permissions for plio. This determines whether the request can access a plio instance or not."""
+        """Object-level permissions for plio/item/question. This determines whether the request can access a plio/item/question instance or not."""
+
+        # if the requested object is an Item or Question, filter the Plio record that it belongs to
+        # and use the permission logic on that Plio instance
+        if isinstance(obj, Question):
+            obj = Plio.objects.filter(id=obj.item.plio.id).first()
+        elif isinstance(obj, Item):
+            obj = Plio.objects.filter(id=obj.plio.id).first()
 
         if request.user.is_superuser:
             return True

plio/serializers.py

@@ -1,9 +1,21 @@
 from rest_framework import serializers
-from plio.models import Video, Plio, Item, Question
+from plio.models import Video, Plio, Item, Question, Image
 from users.models import User
 from users.serializers import UserSerializer
 
 
+class ImageSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = Image
+        fields = [
+            "id",
+            "url",
+            "alt_text",
+            "created_at",
+            "updated_at",
+        ]
+
+
 class VideoSerializer(serializers.ModelSerializer):
     class Meta:
         model = Video
@@ -81,8 +93,15 @@ class Meta:
             "type",
             "options",
             "correct_answer",
+            "image",
             "has_char_limit",
             "max_char_limit",
             "created_at",
             "updated_at",
         ]
+
+    def to_representation(self, instance):
+        response = super().to_representation(instance)
+        if instance.image:
+            response["image"] = ImageSerializer(instance.image).data
+        return response

plio/settings.py

@@ -71,7 +71,6 @@
     "django.contrib.sessions",
     "django.contrib.messages",
     "django.contrib.staticfiles",
-    "django_s3_storage",
     "rest_framework",
     "drf_yasg",
     "safedelete",
@@ -84,6 +83,7 @@
     "oauth2_provider",
     "social_django",
     "rest_framework_social_oauth2",
+    "storages",
 ]
 
 TENANT_MODEL = "organizations.Organization"
@@ -309,3 +309,9 @@
 EMAIL_HOST_PASSWORD = os.environ.get("EMAIL_HOST_PASSWORD")
 
 SMS_DRIVER = os.environ.get("SMS_DRIVER")
+
+# file storage for uploaded images
+DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"
+AWS_STORAGE_BUCKET_NAME = os.environ.get("AWS_STORAGE_BUCKET_NAME")
+AWS_QUERYSTRING_AUTH = False
+DATA_UPLOAD_MAX_MEMORY_SIZE = 10485760  # 10 mb

plio/tests.py

@@ -12,7 +12,7 @@
 
 from users.models import User
 from plio.settings import API_APPLICATION_NAME, OAUTH2_PROVIDER
-from plio.models import Plio, Video
+from plio.models import Plio, Video, Item, Question, Image
 
 
 class BaseTestCase(APITestCase):
@@ -23,7 +23,7 @@ def setUp(self):
 
         # User access and refresh tokens require an OAuth Provider application to be set up and use it as a foreign key.
         # As the test database is empty, we create an application instance before running the test cases.
-        application = Application.objects.create(
+        self.application = Application.objects.create(
             name=API_APPLICATION_NAME,
             redirect_uris="",
             client_type=Application.CLIENT_CONFIDENTIAL,
@@ -40,7 +40,7 @@ def setUp(self):
         expires = timezone.now() + datetime.timedelta(seconds=expire_seconds)
         self.access_token = AccessToken.objects.create(
             user=self.user,
-            application=application,
+            application=self.application,
             token=random_token,
             expires=expires,
             scope=scopes,
@@ -151,3 +151,120 @@ def setUp(self):
     def test_for_question(self):
         # write API calls here
         self.assertTrue(True)
+
+
+class ImageTestCase(BaseTestCase):
+    """Tests the Image CRUD functionality."""
+
+    def setUp(self):
+        super().setUp()
+        # seed a video
+        self.video = Video.objects.create(
+            title="Video 1", url="https://www.youtube.com/watch?v=vnISjBbrMUM"
+        )
+        # seed a plio, item and question
+        self.test_plio = Plio.objects.create(
+            name="test_plio", video=self.video, created_by=self.user
+        )
+        self.test_item = Item.objects.create(
+            plio=self.test_plio, type="question", time=1
+        )
+        self.test_question = Question.objects.create(item=self.test_item)
+
+    def test_user_can_attach_images_to_their_question(self):
+        """
+        Tests whether a user can link images to the questions
+        that were created by themselves
+        """
+        # upload a test image and retrieve the id
+        with open("plio/static/plio/test_image.jpeg", "rb") as img:
+            response = self.client.post(reverse("images-list"), {"url": img})
+        uploaded_image_id = response.json()["id"]
+
+        # update the question with the newly uploaded image
+        response = self.client.put(
+            reverse("questions-detail", args=[self.test_question.id]),
+            {"item": self.test_item.id, "image": uploaded_image_id},
+        )
+        # the user should be able to update the question
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+    def test_user_cannot_attach_image_to_other_user_question(self):
+        """
+        Tests that a user should NOT be able to link images to
+        the questions that were created by some other user
+        """
+        # create a new user
+        new_user = User.objects.create(mobile="+919988776654")
+
+        # set up access token for the new user
+        random_token = "".join(random.choices(string.ascii_lowercase, k=30))
+        expire_seconds = OAUTH2_PROVIDER["ACCESS_TOKEN_EXPIRE_SECONDS"]
+        scopes = " ".join(OAUTH2_PROVIDER["DEFAULT_SCOPES"])
+        expires = timezone.now() + datetime.timedelta(seconds=expire_seconds)
+        new_access_token = AccessToken.objects.create(
+            user=new_user,
+            application=self.application,
+            token=random_token,
+            expires=expires,
+            scope=scopes,
+        )
+        # reset and set the new credentials
+        self.client.credentials(HTTP_AUTHORIZATION="Bearer " + new_access_token.token)
+
+        # create a new image entry using a test image
+        with open("plio/static/plio/test_image.jpeg", "rb") as img:
+            response = self.client.post(reverse("images-list"), {"url": img})
+
+        # try updating the other user's question entry with
+        # the newly created image
+        uploaded_image_id = response.json()["id"]
+        response = self.client.put(
+            reverse("questions-detail", args=[self.test_question.id]),
+            {"item": self.test_item.id, "image": uploaded_image_id},
+        )
+        # the user should not be able to link an image to
+        # a question created by some other user
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_user_can_upload_image(self):
+        """
+        Tests whether an authorized user can create/upload an image
+        """
+        with open("plio/static/plio/test_image.jpeg", "rb") as img:
+            response = self.client.post(reverse("images-list"), {"url": img})
+
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+    def test_guest_cannot_upload_image(self):
+        """
+        Tests whether a guest(unauthorized user) should
+        not be able to create/upload an image
+        """
+        # resetting the credentials to mock a guest user
+        self.client.credentials()
+
+        with open("plio/static/plio/test_image.jpeg", "rb") as img:
+            response = self.client.post(reverse("images-list"), {"url": img})
+
+        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
+
+    def test_upload_size_does_not_exceed_limit(self):
+        """
+        Tests whether any uploads more than
+        `settings.DATA_UPLOAD_MAX_MEMORY_SIZE` should not be allowed
+        """
+        with open("plio/static/plio/test_image_10mb.jpeg", "rb") as img:
+            response = self.client.post(reverse("images-list"), {"url": img})
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+    def test_each_image_has_unique_name(self):
+        """
+        Tests whether each image entry when generated, has a unique name
+        and should not conflict with a name that already exists
+        """
+        random.seed(10)
+        test_image_1 = Image.objects.create()
+        random.seed(10)
+        test_image_2 = Image.objects.create()
+        self.assertNotEqual(test_image_1.url.name, test_image_2.url.name)