feat: Make level moderation proactive (#1773)

* feat: Make level moderation proactive

* Clean

* Migration

* Remove icon and fix redirect

* Fix tests

* Reuse variable

* Student can only access approved levels

* Approval needed by default, set exceptions and constraints

* Logic fixes

* Notify teacher on creation

game/level_management.py

@@ -35,8 +35,12 @@ def levels_shared_with(user):
         return []
 
     shared_levels = user.shared
+    shared_levels = add_related_fields(shared_levels)
 
-    return add_related_fields(shared_levels)
+    if hasattr(user.userprofile, "student"):
+        shared_levels = shared_levels.filter(needs_approval=False)
+
+    return shared_levels
 
 
 def levels_owned_by(user):
@@ -195,9 +199,9 @@ def unshare_level(level, *users):
 
 
 def email_new_custom_level(
-    teacher_email, moderate_url, level_url, home_url, student_name, class_name
+    teacher_email, moderate_url, level_url, student_name, class_name
 ):
-    # email teacher when a new custom level is created by a pupil, so it can be moderated ASAP
+    # email teacher when a new custom level is created by a pupil, so it can be moderated
 
     send_dotdigital_email(
         campaign_ids["level_creation"],

game/migrations/0113_level_needs_approval.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.2.18 on 2025-02-10 20:18
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("game", "0112_worksheet_locked_classes"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="level",
+            name="needs_approval",
+            field=models.BooleanField(default=True),
+        ),
+    ]

game/migrations/0114_default_and_non_student_levels_no_approval.py

@@ -0,0 +1,31 @@
+from django.apps.registry import Apps
+from django.db import migrations
+from django.db.models import Q
+
+
+def mark_default_and_non_student_levels_as_not_needing_approval(apps: Apps, *args):
+    Level = apps.get_model("game", "Level")
+
+    Level.objects.filter(Q(default=True) | Q(owner__user__email__isnull=False)).update(
+        needs_approval=False
+    )
+
+
+def unmark_default_and_non_student_levels_as_not_needing_approval(apps: Apps, *args):
+    Level = apps.get_model("game", "Level")
+
+    Level.objects.filter(Q(default=True) | Q(owner__user__email__isnull=False)).update(
+        needs_approval=True
+    )
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [("game", "0113_level_needs_approval")]
+
+    operations = [
+        migrations.RunPython(
+            code=mark_default_and_non_student_levels_as_not_needing_approval,
+            reverse_code=unmark_default_and_non_student_levels_as_not_needing_approval,
+        )
+    ]

game/migrations/0115_level_level__default_does_not_need_approval.py

@@ -0,0 +1,22 @@
+# Generated by Django 4.2.18 on 2025-02-14 15:40
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("game", "0114_default_and_non_student_levels_no_approval"),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="level",
+            constraint=models.CheckConstraint(
+                check=models.Q(
+                    ("default", True), ("needs_approval", True), _negated=True
+                ),
+                name="level__default_does_not_need_approval",
+            ),
+        ),
+    ]

game/models.py

@@ -16,9 +16,7 @@ def theme_choices():
 def character_choices():
     from game.character import get_all_character
 
-    return [
-        (character.name, character.name) for character in get_all_character()
-    ]
+    return [(character.name, character.name) for character in get_all_character()]
 
 
 class Block(models.Model):
@@ -44,7 +42,7 @@ class Meta:
 
 class Episode(models.Model):
     """Variables prefixed with r_ signify they are parameters for random level generation"""
-    
+
     worksheets: QuerySet["Worksheet"]
 
     name = models.CharField(max_length=200)
@@ -161,9 +159,7 @@ class Level(models.Model):
         on_delete=models.SET_NULL,
         related_name="prev_level",
     )
-    shared_with = models.ManyToManyField(
-        User, related_name="shared", blank=True
-    )
+    shared_with = models.ManyToManyField(User, related_name="shared", blank=True)
     model_solution = models.CharField(blank=True, max_length=20, default="[]")
     disable_route_score = models.BooleanField(default=False)
     disable_algorithm_score = models.BooleanField(default=False)
@@ -229,8 +225,20 @@ class Level(models.Model):
     locked_for_class = models.ManyToManyField(
         Class, blank=True, related_name="locked_levels"
     )
+    needs_approval = models.BooleanField(default=True)
     objects = LevelManager()
 
+    class Meta:
+        constraints = [
+            models.CheckConstraint(
+                check=~models.Q(
+                    default=True,
+                    needs_approval=True,
+                ),
+                name="level__default_does_not_need_approval",
+            ),
+        ]
+
     def __str__(self):
         return f"Level {self.name}"
 
@@ -303,9 +311,7 @@ def __str__(self):
 
 class Attempt(models.Model):
     start_time = models.DateTimeField(auto_now_add=True)
-    level = models.ForeignKey(
-        Level, related_name="attempts", on_delete=models.CASCADE
-    )
+    level = models.ForeignKey(Level, related_name="attempts", on_delete=models.CASCADE)
     student = models.ForeignKey(
         Student,
         related_name="attempts",
@@ -344,18 +350,14 @@ class Worksheet(models.Model):
     lesson_plan_link = models.CharField(
         max_length=500, null=True, blank=True, default=None
     )
-    slides_link = models.CharField(
-        max_length=500, null=True, blank=True, default=None
-    )
+    slides_link = models.CharField(max_length=500, null=True, blank=True, default=None)
     student_worksheet_link = models.CharField(
         max_length=500, null=True, blank=True, default=None
     )
     indy_worksheet_link = models.CharField(
         max_length=500, null=True, blank=True, default=None
     )
-    video_link = models.CharField(
-        max_length=500, null=True, blank=True, default=None
-    )
+    video_link = models.CharField(max_length=500, null=True, blank=True, default=None)
     locked_classes = models.ManyToManyField(
         Class, blank=True, related_name="locked_worksheets"
     )

game/permissions.py

@@ -54,10 +54,14 @@ def can_play_or_delete_level(user, level):
         return user.userprofile.teacher.teaches(level.owner)
 
 
+def can_approve_level(user, level):
+    return hasattr(user.userprofile, "teacher") and level.shared_with.filter(id=user.id).exists()
+
+
 def can_play_level(user, level, early_access):
     if not user.is_anonymous and hasattr(user.userprofile, "student") and user.userprofile.student.class_field:
         # If the user is a student, check that the level isn't locked for their class
-        return user.userprofile.student.class_field not in level.locked_for_class.all()
+        return user.userprofile.student.class_field not in level.locked_for_class.all() and not level.needs_approval
     elif level.default and not level.episode.in_development:
         return True
     elif level.anonymous:
@@ -112,8 +116,8 @@ def can_delete_level(user, level):
 class CanShareLevel(permissions.BasePermission):
     """
     Used to verify that an incoming request is made by a user who is authorised to share
-    the level - that is, that they are the owner of the level as a student, or if they're a teacher that the level was
-    shared with them.
+    the level - that is, that they are the owner of the level as a student and the level has been approved by a teacher,
+    or if they're a teacher that the level was shared with them.
     """
 
     def has_permission(self, request, view):
@@ -128,7 +132,7 @@ def has_object_permission(self, request, view, obj):
         elif hasattr(request.user.userprofile, "teacher") and obj.shared_with.filter(id=request.user.id).exists():
             return True
         else:
-            return obj.owner == request.user.userprofile
+            return obj.owner == request.user.userprofile and not obj.needs_approval
 
 
 class CanShareLevelWith(permissions.BasePermission):

game/static/game/css/dataTables.custom.css

@@ -1,5 +1,6 @@
-.tableWrapper {
-  width: 100%;
+.tableWrapper,
+#moderateTable {
+  width: 100% !important;
 }
 
 div.DTFC_LeftBodyLiner,

game/static/game/js/level_editor.js

@@ -87,6 +87,7 @@ ocargo.LevelEditor = function(levelId) {
     var originNode = null;
     var houseNodes = [];
     var currentTheme = THEMES.grass;
+    var needsApproval = false;
 
     // Reference to the Raphael elements for each square
     var grid;
@@ -731,7 +732,7 @@ ocargo.LevelEditor = function(levelId) {
         }
 
         function setupShareTab() {
-            // Setup the behaviour for when the tab is selected
+            // Set up the behaviour for when the tab is selected
             tabs.share.setOnChange(function() {
                 if (!isIndependentStudent() ||  !isLoggedIn("share") || !canShare() || !isLevelOwned()) {
                     restorePreviousTab();
@@ -2745,6 +2746,8 @@ ocargo.LevelEditor = function(levelId) {
         if(state.max_fuel) {
             $('#max_fuel').val(state.max_fuel);
         }
+
+        needsApproval = state.needs_approval;
     }
 
     function loadLevel(levelID) {
@@ -2853,10 +2856,13 @@ ocargo.LevelEditor = function(levelId) {
 
     function canShare() {
         if (!saveState.isSaved()) {
-            ocargo.Drawing.startPopup(gettext('Sharing'), '', gettext('Please save your level before continuing!'));
+            ocargo.Drawing.startPopup("Sharing", "", "Please save your level before continuing!");
             return false;
         } else if (hasLevelChangedSinceSave()) {
-            ocargo.Drawing.startPopup(gettext('Sharing'), '', gettext('Please save your latest changes!'));
+            ocargo.Drawing.startPopup("Sharing", "", "Please save your latest changes!");
+            return false;
+        } else if (needsApproval) {
+            ocargo.Drawing.startPopup("Sharing", "", "Your teacher hasn't approved your level so you can't share it yet. Please let your teacher know they need to approve it first.")
             return false;
         }
         return true;
@@ -3174,7 +3180,7 @@ ocargo.LevelEditor = function(levelId) {
 /******************/
 
 $(function() {
-    var editor = new ocargo.LevelEditor(LEVEL); // This seems unused but removing it breaks the editor page.
+    new ocargo.LevelEditor(LEVEL);
     var subtitle = interpolate(
         gettext('Click %(help_icon)s%(help_label)s for clues on getting started.'), {
             help_icon: ocargo.jsElements.image(ocargo.Drawing.imageDir + 'icons/help.svg', 'popupHelp'),

game/static/game/js/level_moderation.js

@@ -1,7 +1,6 @@
 /* global showPopupConfirmation */
 
 var levelID;
-var classID;
 var students;
 
 var saving = new ocargo.Saving();
@@ -57,6 +56,27 @@ jQuery.extend(jQuery.fn.dataTableExt.oSort, {
   },
 });
 
+approveLevel = function (id, callback, errorCallback) {
+  csrftoken = Cookies.get('csrftoken');
+  $.ajax({
+    url: Urls.approve_level(id),
+    type: 'POST',
+    dataType: 'json',
+    data: {csrfmiddlewaretoken: csrftoken},
+    beforeSend: function (xhr, settings) {
+      if (!csrfSafeMethod(settings.type) && !this.crossDomain) {
+        xhr.setRequestHeader("X-CSRFToken", csrftoken);
+      }
+    },
+    success: function (json) {
+      callback();
+    },
+    error: function (xhr, errmsg, err) {
+      errorCallback(xhr.status + ": " + errmsg + " " + err + " " + xhr.responseText);
+    }
+  });
+};
+
 $(document).ready(function () {
   $(".delete").click(function () {
     levelID = this.getAttribute("value");
@@ -67,6 +87,17 @@ $(document).ready(function () {
     window.location.href = Urls.play_custom_level(this.getAttribute("value"));
   });
 
+  $(".approve").click(function () {
+    levelID = this.getAttribute("value");
+    approveLevel(levelID)
+    // Waiting half a second before redirecting because otherwise the Approve button doesn't become disabled
+    setTimeout(
+        function(){
+          window.location.href = Urls.level_moderation();
+          },
+        500);
+  });
+
   $("#clear-classes").on("click", () => {
     $('[id^="id_classes_"],#select-all-classes').prop("checked", false);
   });
@@ -105,7 +136,7 @@ $(document).ready(function () {
       {
         orderable: false,
         searchable: false,
-        targets: [-1, -2], // Play and Delete columns
+        targets: [-1, -2, -3], // Play, Approve and Delete columns
       },
       {
         type: "student-with-teacher",

game/templates/game/level_editor.html

@@ -17,9 +17,9 @@
     var COW_LEVELS_ENABLED = {{cow_level_enabled|booltojs}};
     var NIGHT_MODE_FEATURE_ENABLED = {{night_mode_feature_enabled}};
     {% if level %}
-    var LEVEL = {{ level }};
+      var LEVEL = {{ level }};
     {% else %}
-    var LEVEL = null;
+      var LEVEL = null;
     {% endif %}
   </script>
   <script src="{% static 'game/js/utils.js' %}"></script>