feat: Remove worksheet model (#1587)

* wip: removed worksheet model and created dataclass

* fixed tests

* update migration

* Merge branch 'development' into remove-worksheet-model

* get portal from branch

* fix get portal from branch

* fix worksheets starter code

* try to install common first

* fix cypress test

* code review changes

* update worksheets descriptions

.github/workflows/cicd.yml

@@ -27,6 +27,10 @@ jobs:
         run: |
           pip install pipenv
           pipenv install --dev --system
+          # TODO: remove after portal branch is merged
+          pip install -U "git+https://github.com/ocadotechnology/codeforlife-portal@remove-worksheet-model#egg=cfl-common&subdirectory=cfl_common"
+          # TODO: remove after portal branch is merged
+          pip install -U "git+https://github.com/ocadotechnology/codeforlife-portal@remove-worksheet-model#egg=codeforlife-portal"
           cd game_frontend
           yarn --frozen-lockfile
       - name: Run Javascript tests

aimmo/admin.py

@@ -2,12 +2,12 @@
 
 from django.contrib import admin
 
-from .models import Avatar, Game, Worksheet
+from .models import Avatar, Game
 
 
 class GameDataAdmin(admin.ModelAdmin):
     search_fields = ["id", "owner__username", "owner__email"]
-    list_display = ["id", "owner", "game_class", "school", "worksheet", "status"]
+    list_display = ["id", "owner", "game_class", "school", "worksheet_id", "status"]
     raw_id_fields = ["owner", "main_user", "can_play", "game_class"]
     readonly_fields = ["players", "auth_token"]
 
@@ -38,11 +38,5 @@ def game_id(self, obj):
         return obj.game
 
 
-class WorksheetDataAdmin(admin.ModelAdmin):
-    search_fields = ["id", "name", "era"]
-    list_display = ["id", "name", "era"]
-
-
 admin.site.register(Game, GameDataAdmin)
 admin.site.register(Avatar, AvatarDataAdmin)
-admin.site.register(Worksheet, WorksheetDataAdmin)

aimmo/migrations/0026_remove_worksheets.py

@@ -0,0 +1,31 @@
+# Generated by Django 2.2.24 on 2021-11-18 11:09
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("aimmo", "0025_generate_auth_token"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="game",
+            name="auth_token",
+            field=models.CharField(blank=True, max_length=48),
+        ),  # Generated by Django, but doesn't actually change anything
+        migrations.AlterField(
+            model_name="game",
+            name="worksheet",
+            field=models.IntegerField(default=1),
+        ),
+        migrations.DeleteModel(
+            name="Worksheet",
+        ),
+        migrations.RenameField(
+            model_name="game",
+            old_name="worksheet",
+            new_name="worksheet_id",
+        ),
+    ]

aimmo/models.py

@@ -4,9 +4,9 @@
 from common.models import Class
 from django.contrib.auth.models import User
 from django.db import models
-from wagtail.snippets.models import register_snippet
 
 from aimmo import app_settings
+from aimmo.worksheets import WORKSHEETS
 
 DEFAULT_WORKSHEET_ID = 1
 
@@ -19,43 +19,6 @@ def generate_auth_token():
     return urlsafe_b64encode(urandom(16))
 
 
-class WorksheetManager(models.Manager):
-    def sorted(self):
-        return self.get_queryset().order_by("sort_order")
-
-
-@register_snippet
-class Worksheet(models.Model):
-    ERA_CHOICES = [
-        (1, "future"),
-        (2, "ancient"),
-        (3, "modern day"),
-        (4, "prehistoric"),
-        (5, "broken future"),
-    ]
-
-    name = models.CharField(max_length=100)
-    description = models.TextField(blank=True)
-    short_description = models.TextField(blank=True)
-    image_path = models.CharField(max_length=255, blank=True)
-    active_image_path = models.CharField(max_length=255, blank=True)
-    era = models.PositiveSmallIntegerField(
-        choices=ERA_CHOICES, default=ERA_CHOICES[0][0]
-    )
-    thumbnail_text = models.CharField(max_length=100, blank=True)
-    thumbnail_image_path = models.CharField(max_length=255, blank=True)
-    teacher_pdf_name = models.CharField(max_length=255, blank=True)
-    student_pdf_name = models.CharField(max_length=255, blank=True)
-    sort_order = models.IntegerField(default=0)
-
-    objects = WorksheetManager()
-
-    starter_code = models.TextField()
-
-    def __str__(self):
-        return f"{self.id}: {self.name}"
-
-
 class Game(models.Model):
     RUNNING = "r"
     STOPPED = "s"
@@ -107,14 +70,16 @@ class Game(models.Model):
     start_height = models.IntegerField(default=31)
     start_width = models.IntegerField(default=31)
     status = models.CharField(max_length=1, choices=STATUS_CHOICES, default=RUNNING)
-    worksheet = models.ForeignKey(
-        Worksheet, on_delete=models.PROTECT, default=DEFAULT_WORKSHEET_ID
-    )
+    worksheet_id = models.IntegerField(default=DEFAULT_WORKSHEET_ID)
 
     @property
     def is_active(self):
         return not self.completed
 
+    @property
+    def worksheet(self):
+        return WORKSHEETS.get(self.worksheet_id)
+
     def __str__(self):
         return str(self.id)
 

aimmo/serializers.py

@@ -1,8 +1,9 @@
 import json
 from rest_framework import serializers
 
-from aimmo.models import Game, Avatar, Worksheet
+from aimmo.models import Game, Avatar
 from aimmo.game_manager import GameManager
+from aimmo.worksheets import WORKSHEETS
 
 
 class GameSerializer(serializers.Serializer):
@@ -45,7 +46,7 @@ def update(self, instance, validated_data):
 
         if "worksheet_id" in validated_data:
             avatars = Avatar.objects.filter(game=instance)
-            worksheet = Worksheet.objects.get(id=instance.worksheet_id)
+            worksheet = WORKSHEETS.get(instance.worksheet_id)
 
             for avatar in avatars:
                 avatar.code = worksheet.starter_code

aimmo/tests/test_models.py

@@ -1,9 +1,7 @@
-import pytest
 from django.contrib.auth.models import User
-from django.db.models.deletion import ProtectedError
 from django.test import TestCase
 
-from aimmo.models import Game, Worksheet
+from aimmo.models import Game
 
 
 class TestModels(TestCase):
@@ -14,10 +12,7 @@ def test_game_owner_on_delete(self):
         Then the game's owner field is set to null.
         """
         user = User.objects.create_user("test", "test@example.com", "password")
-        worksheet = Worksheet.objects.create(
-            name="test worksheet", starter_code="test code"
-        )
-        game = Game(id=1, name="Test Game", worksheet=worksheet)
+        game = Game(id=1, name="Test Game", worksheet_id=1)
         game.owner = user
         game.save()
 
@@ -33,31 +28,11 @@ def test_game_main_user_on_delete(self):
         Then the game's main_user field is set to null.
         """
         user = User.objects.create_user("test", "test@example.com", "password")
-        worksheet = Worksheet.objects.create(
-            name="test worksheet", starter_code="test code"
-        )
-        game = Game(id=1, name="Test Game", worksheet=worksheet)
+        game = Game(id=1, name="Test Game", worksheet_id=1)
         game.main_user = user
         game.save()
 
         user.delete()
         game = Game.objects.get(id=1)
 
         assert game.main_user is None
-
-    def test_game_worksheet_on_delete(self):
-        """
-        Given a game and its worksheet,
-        When anyone tries to delete the worksheet,
-        Then a ProtectedError is raised and the worksheet isn't deleted.
-        """
-        game = Game(id=1, name="Test Game")
-        worksheet = Worksheet.objects.create(
-            name="test worksheet", starter_code="test code"
-        )
-
-        game.worksheet = worksheet
-        game.save()
-
-        with pytest.raises(ProtectedError):
-            worksheet.delete()

aimmo/tests/test_views.py

@@ -13,9 +13,10 @@
 from rest_framework import status
 
 from aimmo import app_settings, models
-from aimmo.models import Game, Worksheet
+from aimmo.models import Game
 from aimmo.serializers import GameSerializer
 from aimmo.views import get_avatar_id
+from aimmo.worksheets import WORKSHEETS, Worksheet
 from unittest.mock import patch
 
 app_settings.GAME_SERVER_URL_FUNCTION = lambda game_id: (
@@ -53,15 +54,9 @@ def setUpTestData(cls):
         cls.klass.save()
         cls.klass2, _, _ = create_class_directly(cls.user.email)
         cls.klass2.save()
-        cls.worksheet: Worksheet = Worksheet.objects.create(
-            name="test worksheet", starter_code="test code 1"
-        )
-        cls.worksheet2: Worksheet = Worksheet.objects.create(
-            name="test worksheet 2", starter_code="test code 2"
-        )
-        cls.game = models.Game(
-            id=1, name="test", game_class=cls.klass, worksheet=cls.worksheet
-        )
+        cls.worksheet: Worksheet = WORKSHEETS.get(1)
+        cls.worksheet2: Worksheet = WORKSHEETS.get(2)
+        cls.game = models.Game(id=1, name="test", game_class=cls.klass, worksheet_id=1)
         cls.game.save()
 
     def setUp(self):
@@ -356,8 +351,7 @@ def test_delete_game(self):
         Check for 204 when deleting a game
         """
         client = self.login()
-        worksheet = Worksheet.objects.create(name="test", starter_code="test")
-        game2 = models.Game(id=2, name="test", worksheet=worksheet)
+        game2 = models.Game(id=2, name="test", worksheet_id=2)
         game2.save()
 
         response = client.delete(reverse("game-detail", kwargs={"pk": self.game.id}))
@@ -493,15 +487,11 @@ def test_delete_games(self):
         new_teacher.save()
         new_klass, _, _ = create_class_directly(new_user.email)
         new_user.save()
-        new_game = models.Game(
-            name="test2", game_class=new_klass, worksheet=self.worksheet
-        )
+        new_game = models.Game(name="test2", game_class=new_klass, worksheet_id=1)
         new_game.save()
 
         # Create a game for the second class
-        game2 = models.Game(
-            name="test", game_class=self.klass2, worksheet=self.worksheet
-        )
+        game2 = models.Game(name="test", game_class=self.klass2, worksheet_id=1)
         game2.save()
 
         data = {"game_ids": [self.game.id, game2.id, new_game.id]}