Add separate visibility to problems

Tests are missing, but existing ones were adapted to the new
functionality.

Author: schrjako

web/attempts/admin.py

@@ -24,6 +24,7 @@ class AttemptAdmin(SimpleHistoryAdmin):
         "part__problem__problem_set__course__institution",
         "part__problem__problem_set__course",
         "part__problem__problem_set",
+        "part__problem__visible",
     )
     search_fields = (
         "part__pk",

web/attempts/tests.py

@@ -11,7 +11,7 @@ class AttemptSubmitTestCase(TestCase):
     def setUp(self):
         course = baker.make("courses.Course")
         problem_set = baker.make("courses.ProblemSet", course=course, visible=True)
-        problem = baker.make("problems.Problem", problem_set=problem_set)
+        problem = baker.make("problems.Problem", problem_set=problem_set, visible=True)
         self.part1 = baker.make("problems.Part", problem=problem, secret='["1"]')
         self.part2 = baker.make("problems.Part", problem=problem, secret='["1", "2"]')
         self.part3 = baker.make(

web/courses/models.py

@@ -46,6 +46,7 @@ def recent_problem_sets(self, n=3):
         return self.problem_sets.reverse().filter(visible=True)[:n]
 
     def user_attempts(self, user):
+        """This function ignores problem visibility, because it assumes it is only called from problems/models.py:marking_file() by a teacher user."""
         attempts = {}
         for attempt in user.attempts.filter(part__problem__problem_set__course=self):
             attempts[attempt.part_id] = attempt
@@ -108,7 +109,7 @@ def annotate_for_teacher(self):
         student_count = len(students)
 
         part_sets = Part.objects.filter(
-            problem__problem_set__in=self.annotated_problem_sets
+            problem__problem_set__in=self.annotated_problem_sets, problem__visible=True
         )
         parts_count = (
             part_sets.values("problem__problem_set_id")
@@ -123,6 +124,7 @@ def annotate_for_teacher(self):
         attempts_full = Attempt.objects.filter(
             user__in=students,
             part__problem__problem_set__in=self.annotated_problem_sets,
+            part__problem__visible=True,
         )
         attempts = attempts_full.values("valid", "part__problem__problem_set_id")
         attempts_dict = {}
@@ -197,8 +199,12 @@ def observed_students(self):
     def student_success(self):
         students = self.observed_students()
         problem_sets = self.problem_sets.filter(visible=True)
-        part_count = Part.objects.filter(problem__problem_set__in=problem_sets).count()
-        attempts = Attempt.objects.filter(part__problem__problem_set__in=problem_sets)
+        part_count = Part.objects.filter(
+            problem__problem_set__in=problem_sets, problem__visible=True
+        ).count()
+        attempts = Attempt.objects.filter(
+            part__problem__problem_set__in=problem_sets, part__problem__visible=True
+        )
         valid_attempts = (
             attempts.filter(valid=True).values("user").annotate(Count("user"))
         )
@@ -253,9 +259,8 @@ def student_success_by_problem_set(self):
             "problems", "problems__parts"
         ):
             different_subtasks = 0
-            for problem in problem_set.problems.all():
-                for part in problem.parts.all():
-                    different_subtasks += 1
+            for problem in problem_set.problems.filter(visible=True):
+                different_subtasks += problem.parts.count()
 
             # In case there are no parts, we do not want to divide by 0
             if different_subtasks == 0:
@@ -268,7 +273,9 @@ def student_success_by_problem_set(self):
                 ]  # Valid, invalid
 
             attempts = Attempt.objects.filter(
-                part__problem__problem_set=problem_set, user__in=students
+                part__problem__problem_set=problem_set,
+                user__in=students,
+                part__problem__visible=True,
             )
             for attempt in attempts:
                 if attempt.valid:
@@ -312,13 +319,13 @@ def student_success_by_problemset_grouped_by_groups(self):
         groups = self.groups.all()
         student_success = self.student_success_by_problem_set()
 
-        student_sucess_by_groups = {}
+        student_success_by_groups = {}
         for group in groups:
-            student_sucess_by_groups[group] = {}
+            student_success_by_groups[group] = {}
             for student in group.students.all():
-                student_sucess_by_groups[group][student] = student_success[student]
+                student_success_by_groups[group][student] = student_success[student]
 
-        return student_sucess_by_groups
+        return student_success_by_groups
 
 
 class StudentEnrollment(models.Model):
@@ -395,7 +402,13 @@ def get_absolute_url(self):
         return reverse("problem_set_detail", args=[str(self.pk)])
 
     def attempts_archive(self, user):
-        files = [problem.attempt_file(user) for problem in self.problems.all()]
+        if user.can_edit_problem_set(self):
+            files = [problem.attempt_file(user) for problem in self.problems.all()]
+        else:
+            files = [
+                problem.attempt_file(user)
+                for problem in self.problems.filter(visible=True)
+            ]
         archive_name = slugify(self.title)
         return archive_name, files
 
@@ -433,33 +446,33 @@ def results_archive(self, user):
             attempt_dict[user_id] = user_attempts
         users = User.objects.filter(id__in=user_ids)
 
-        archive_name = "{0}-results".format(slugify(self.title))
+        archive_name = f"{slugify(self.title)}-results"
         files = []
 
         bare_files = {}
         for problem in self.problems.all():
             folder = slugify(problem.title)
             for user in users.all():
                 filename, contents = problem.marking_file(user)
-                files.append(("{0}/{1}".format(folder, filename), contents))
+                files.append((f"{folder}/{filename}", contents))
                 filename, contents = problem.bare_file(user)
                 bare_files[filename] = bare_files.get(filename, "") + contents + "\n\n"
 
         for filename, contents in bare_files.items():
-            files.append(("bare/{0}".format(filename), contents))
+            files.append((f"bare/{filename}", contents))
 
         for user, history in self.attempt_history().items():
             username = user.get_full_name() or user.username
             problem_slug = slugify(username).replace("-", "_")
             extension = "py"
-            filename = "{0}.{1}".format(problem_slug, extension)
+            filename = f"{problem_slug}.{extension}"
             contents = render_to_string(
-                "history.{0}".format(extension),
+                f"history.{extension}",
                 {
                     "history": history,
                 },
             )
-            files.append(("history/{0}".format(filename), contents))
+            files.append((f"history/{filename}", contents))
 
         users = []
         for user in User.objects.filter(id__in=user_ids).order_by("last_name"):
@@ -524,6 +537,7 @@ def student_statistics(self):
                     "title": problem.title,
                     "pk": problem.pk,
                     "parts": parts,
+                    "visible": problem.visible,
                 }
             )
         return statistics
@@ -532,10 +546,13 @@ def valid_percentage(self, user):
         """
         Returns the percentage of parts (rounded to the nearest integer)
         of parts in this problem set for which the given user has a valid attempt.
+        Doesn't count parts of problems that have visible set to False.
         """
-        number_of_all_parts = Part.objects.filter(problem__problem_set=self).count()
+        number_of_all_parts = Part.objects.filter(
+            problem__problem_set=self, problem__visible=True
+        ).count()
         number_of_valid_parts = user.attempts.filter(
-            valid=True, part__problem__problem_set=self
+            valid=True, part__problem__problem_set=self, part__problem__visible=True
         ).count()
         if number_of_all_parts == 0:
             return None

web/courses/views.py

@@ -102,6 +102,10 @@ def problem_set_detail(request, problem_set_pk):
     user_attempts = request.user.attempts.filter(
         part__problem__problem_set__id=problem_set_pk
     )
+    student_statistics = problem_set.student_statistics()
+    if not request.user.is_teacher(problem_set.course):
+        user_attempts = user_attempts.filter(part__problem__visible=True)
+        student_statistics = filter(lambda p: p["visible"], student_statistics)
     valid_parts_ids = user_attempts.filter(valid=True).values_list("part_id", flat=True)
     invalid_parts_ids = user_attempts.filter(valid=False).values_list(
         "part_id", flat=True
@@ -115,14 +119,14 @@ def problem_set_detail(request, problem_set_pk):
             "valid_parts_ids": valid_parts_ids,
             "invalid_parts_ids": invalid_parts_ids,
             "show_teacher_forms": request.user.can_edit_problem_set(problem_set),
-            "student_statistics": problem_set.student_statistics(),
+            "student_statistics": student_statistics,
         },
     )
 
 
 @login_required
 def course_detail(request, course_pk):
-    """Show a list of all problems in a problem set."""
+    """Show a list of all problem sets in a course."""
     course = get_object_or_404(Course, pk=course_pk)
     verify(request.user.can_view_course(course))
     if request.user.can_edit_course(course):

web/problems/admin.py

@@ -16,6 +16,7 @@ class ProblemAdmin(SimpleHistoryAdmin):
         "course",
         "problem_set",
         "description",
+        "visible",
     )
     list_display_links = ("title",)
     ordering = (

web/problems/migrations/0004_historicalproblem_visible_problem_visible.py

@@ -0,0 +1,23 @@
+# Generated by Django 4.1.2 on 2023-02-18 21:42
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("problems", "0003_alter_part_problem_alter_problem_problem_set"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="historicalproblem",
+            name="visible",
+            field=models.BooleanField(default=True, verbose_name="Visible"),
+        ),
+        migrations.AddField(
+            model_name="problem",
+            name="visible",
+            field=models.BooleanField(default=True, verbose_name="Visible"),
+        ),
+    ]

web/problems/migrations/0005_alter_historicalproblem_visible_and_more.py

@@ -0,0 +1,23 @@
+# Generated by Django 4.1.2 on 2023-02-18 21:42
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("problems", "0004_historicalproblem_visible_problem_visible"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="historicalproblem",
+            name="visible",
+            field=models.BooleanField(default=False, verbose_name="Visible"),
+        ),
+        migrations.AlterField(
+            model_name="problem",
+            name="visible",
+            field=models.BooleanField(default=False, verbose_name="Visible"),
+        ),
+    ]

web/problems/models.py

@@ -7,6 +7,7 @@
 from django.template.defaultfilters import slugify
 from django.template.loader import render_to_string
 from django.urls import reverse
+from django.utils.translation import gettext_lazy as _
 from rest_framework.authtoken.models import Token
 from simple_history.models import HistoricalRecords
 from taggit.managers import TaggableManager
@@ -20,6 +21,7 @@ class Problem(OrderWithRespectToMixin, models.Model):
     problem_set = models.ForeignKey(
         "courses.ProblemSet", on_delete=models.CASCADE, related_name="problems"
     )
+    visible = models.BooleanField(default=False, verbose_name=_("Visible"))
     history = HistoricalRecords()
     tags = TaggableManager(blank=True)
     language = models.CharField(
@@ -48,7 +50,7 @@ def get_absolute_url(self):
         return "{}#{}".format(self.problem_set.get_absolute_url(), self.anchor())
 
     def anchor(self):
-        return "problem-{}".format(self.pk)
+        return f"problem-{self.pk}"
 
     def user_attempts(self, user):
         return user.attempts.filter(part__problem=self)
@@ -99,6 +101,9 @@ def solution_file(self):
         return filename, contents
 
     def marking_file(self, user):
+        """This function ignores problem visibility because it assumes its
+        called only from courses/models.py:results_archive() by a teacher user
+        """
         attempts = {attempt.part.id: attempt for attempt in self.user_attempts(user)}
         parts = [(part, attempts.get(part.id)) for part in self.parts.all()]
         username = user.get_full_name() or user.username
@@ -116,6 +121,9 @@ def marking_file(self, user):
         return filename, contents
 
     def bare_file(self, user):
+        """This function ignores problem visibility because it assumes its
+        called only from courses/models.py:results_archive() by a teacher user
+        """
         attempts = {attempt.part.id: attempt for attempt in self.user_attempts(user)}
         parts = [(part, attempts.get(part.id)) for part in self.parts.all()]
         username = user.get_full_name() or user.username
@@ -133,6 +141,9 @@ def bare_file(self, user):
         return filename, contents
 
     def edit_file(self, user):
+        """This function ignores problem visibility because it assumes its
+        called only from courses/models.py:edit_archive() by a teacher user
+        """
         authentication_token = Token.objects.get(user=user)
         url = settings.SUBMISSION_URL + reverse("problems-submit")
         problem_slug = slugify(self.title).replace("-", "_")

web/problems/views.py

@@ -52,7 +52,7 @@ class ProblemCreate(CreateView):
     """
 
     model = Problem
-    fields = ["title", "description", "language"]
+    fields = ["title", "description", "language", "visible"]
 
     def get_context_data(self, **kwargs):
         context = super(ProblemCreate, self).get_context_data(**kwargs)

web/templates/courses/coursegroup_form.html

@@ -12,7 +12,7 @@
 			<button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
 		    <h4 class="modal-title">
 		    	{% if form.instance.pk %}
-		  			{# Translators: Title of editing form for problems sets. #}
+		  			{# Translators: Title of editing form for problem sets. #}
 		    		{% trans "Edit course group" %} <em>{{ form.instance.title }}</em>
 	  			{% else %}
 	  				{# Translators: Title of 'add new problem set' form. #}