From 3484f914887467a30d611063d2654ba0adba46ee Mon Sep 17 00:00:00 2001 From: Matija Pretnar Date: Sun, 19 Feb 2023 12:00:40 +0100 Subject: [PATCH 1/3] Create ProblemInstance model --- web/attempts/admin.py | 3 ++ .../0004_attempt_problem_instance_and_more.py | 36 ++++++++++++++ web/attempts/models.py | 6 +++ .../migrations/0004_probleminstance.py | 49 +++++++++++++++++++ web/courses/models.py | 14 ++++++ 5 files changed, 108 insertions(+) create mode 100644 web/attempts/migrations/0004_attempt_problem_instance_and_more.py create mode 100644 web/courses/migrations/0004_probleminstance.py diff --git a/web/attempts/admin.py b/web/attempts/admin.py index 89bb5bf4..dee0d706 100644 --- a/web/attempts/admin.py +++ b/web/attempts/admin.py @@ -9,6 +9,7 @@ class AttemptAdmin(SimpleHistoryAdmin): "user", "part", "problem", + "problem_instance", "solution", "valid", "feedback", @@ -17,6 +18,7 @@ class AttemptAdmin(SimpleHistoryAdmin): list_display = ( "user", "problem", + "problem_instance", "part", "valid", ) @@ -37,6 +39,7 @@ class AttemptAdmin(SimpleHistoryAdmin): readonly_fields = [ "problem", + "problem_instance", ] def problem(self, obj): diff --git a/web/attempts/migrations/0004_attempt_problem_instance_and_more.py b/web/attempts/migrations/0004_attempt_problem_instance_and_more.py new file mode 100644 index 00000000..db8b8264 --- /dev/null +++ b/web/attempts/migrations/0004_attempt_problem_instance_and_more.py @@ -0,0 +1,36 @@ +# Generated by Django 4.1.2 on 2023-02-19 11:00 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("courses", "0004_probleminstance"), + ("attempts", "0003_alter_attempt_part"), + ] + + operations = [ + migrations.AddField( + model_name="attempt", + name="problem_instance", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="attempts", + to="courses.probleminstance", + ), + ), + migrations.AddField( + model_name="historicalattempt", + name="problem_instance", + field=models.ForeignKey( + blank=True, + db_constraint=False, + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="+", + to="courses.probleminstance", + ), + ), + ] diff --git a/web/attempts/models.py b/web/attempts/models.py index 48b7d1f7..887048da 100644 --- a/web/attempts/models.py +++ b/web/attempts/models.py @@ -11,6 +11,12 @@ class Attempt(models.Model): part = models.ForeignKey( "problems.Part", on_delete=models.CASCADE, related_name="attempts" ) + problem_instance = models.ForeignKey( + "courses.ProblemInstance", + on_delete=models.SET_NULL, + related_name="attempts", + null=True, + ) solution = models.TextField(blank=True) valid = models.BooleanField(default=False) feedback = models.TextField(default="[]", validators=[is_json_string_list]) diff --git a/web/courses/migrations/0004_probleminstance.py b/web/courses/migrations/0004_probleminstance.py new file mode 100644 index 00000000..b0eeda6b --- /dev/null +++ b/web/courses/migrations/0004_probleminstance.py @@ -0,0 +1,49 @@ +# Generated by Django 4.1.2 on 2023-02-19 11:00 + +from django.db import migrations, models +import django.db.models.deletion +import utils.models + + +class Migration(migrations.Migration): + dependencies = [ + ("problems", "0005_alter_historicalproblem_visible_and_more"), + ("courses", "0003_alter_coursegroup_options"), + ] + + operations = [ + migrations.CreateModel( + name="ProblemInstance", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("visible", models.BooleanField(default=False, verbose_name="Visible")), + ( + "problem", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="instances", + to="problems.problem", + ), + ), + ( + "problem_set", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="courses.problemset", + ), + ), + ], + options={ + "order_with_respect_to": "problem_set", + }, + bases=(utils.models.OrderWithRespectToMixin, models.Model), + ), + ] diff --git a/web/courses/models.py b/web/courses/models.py index 671134ac..12906be5 100644 --- a/web/courses/models.py +++ b/web/courses/models.py @@ -366,3 +366,17 @@ def copy_to(self, course): for problem in self.problems.all(): problem.copy_to(new_problem_set) return new_problem_set + + +class ProblemInstance(OrderWithRespectToMixin, models.Model): + problem = models.ForeignKey( + "problems.Problem", on_delete=models.CASCADE, related_name="instances" + ) + problem_set = models.ForeignKey("courses.ProblemSet", on_delete=models.CASCADE) + visible = models.BooleanField(default=False, verbose_name=_("Visible")) + + class Meta: + order_with_respect_to = "problem_set" + + def __str__(self): + return self.problem.title From fd5257f80c5ef4f62aeabfd77c1bb48acc33d93c Mon Sep 17 00:00:00 2001 From: Matija Pretnar Date: Sun, 19 Feb 2023 12:33:26 +0100 Subject: [PATCH 2/3] Add migration to generate probleminstances --- .../0005_generate_probleminstances.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 web/courses/migrations/0005_generate_probleminstances.py diff --git a/web/courses/migrations/0005_generate_probleminstances.py b/web/courses/migrations/0005_generate_probleminstances.py new file mode 100644 index 00000000..e01b7f55 --- /dev/null +++ b/web/courses/migrations/0005_generate_probleminstances.py @@ -0,0 +1,38 @@ +# Generated by Django 4.1.2 on 2023-02-19 11:02 + +from django.db import migrations +from django.db import models + + +def generate_probleminstances(apps, schema_editor): + # We can't import the Person model directly as it may be a newer + # version than this migration expects. We use the historical version. + Problem = apps.get_model('problems', 'Problem') + ProblemInstance = apps.get_model('courses', 'ProblemInstance') + problems = Problem.objects.values_list('id', 'problem_set_id', 'visible', '_order') + instances = [ProblemInstance( + problem_id=id, + problem_set_id=problem_set_id, + visible=visible, + _order=_order + ) for id, problem_set_id, visible, _order in problems + ] + instances = ProblemInstance.objects.bulk_create(instances, batch_size=1000) + problem_id_to_instance_id = {instance.problem_id: instance.id for instance in instances} + for class_name in ["Attempt", "HistoricalAttempt"]: + Attempt = apps.get_model('attempts', class_name) + attempts = Attempt.objects.annotate(problem_id=models.F('part__problem_id')) + for attempt in attempts: + attempt.problem_instance_id = problem_id_to_instance_id[attempt.problem_id] + Attempt.objects.bulk_update(attempts, ['problem_instance_id'], batch_size=1000) + + +class Migration(migrations.Migration): + dependencies = [ + ("courses", "0004_probleminstance"), + ("attempts", "0004_attempt_problem_instance_and_more"), + ] + + operations = [ + migrations.RunPython(generate_probleminstances), + ] From 1d1a54d5b2e5089741376fce669d7f99ec263728 Mon Sep 17 00:00:00 2001 From: Matija Pretnar Date: Sat, 6 May 2023 21:09:45 +0100 Subject: [PATCH 3/3] WIP premaknil metode iz Problem v ProblemInstance --- web/attempts/admin.py | 15 +-- web/courses/admin.py | 31 +++++ web/courses/models.py | 140 +++++++++++++++++++++-- web/problems/admin.py | 15 +-- web/problems/models.py | 110 +----------------- web/problems/templates/python/attempt.py | 1 + 6 files changed, 172 insertions(+), 140 deletions(-) diff --git a/web/attempts/admin.py b/web/attempts/admin.py index dee0d706..211a27ee 100644 --- a/web/attempts/admin.py +++ b/web/attempts/admin.py @@ -8,7 +8,6 @@ class AttemptAdmin(SimpleHistoryAdmin): fields = [ "user", "part", - "problem", "problem_instance", "solution", "valid", @@ -17,28 +16,26 @@ class AttemptAdmin(SimpleHistoryAdmin): list_display = ( "user", - "problem", "problem_instance", "part", "valid", ) list_filter = ( - "part__problem__problem_set__course__institution", - "part__problem__problem_set__course", - "part__problem__problem_set", - "part__problem__visible", + "problem_instance__problem_set__course__institution", + "problem_instance__problem_set__course", + "problem_instance__problem_set", + "problem_instance__visible", ) search_fields = ( "part__pk", - "problem__problem_set__title", - "problem__title", + "problem_instance__problem_set__title", + "problem_instance__problem__title", "user__username", "part__description", ) date_hierarchy = "submission_date" readonly_fields = [ - "problem", "problem_instance", ] diff --git a/web/courses/admin.py b/web/courses/admin.py index 2d412d37..3c5fc2d1 100644 --- a/web/courses/admin.py +++ b/web/courses/admin.py @@ -46,6 +46,37 @@ class InstitutionAdmin(admin.ModelAdmin): search_fields = ("name",) +class ProblemInstanceAdmin(admin.ModelAdmin): + list_display = ( + "problem", + "course", + "problem_set", + "visible", + ) + list_display_links = ("title",) + list_filter = ( + "problem_set__course__institution", + "problem_set__course", + "problem_set", + ) + ordering = ( + "problem_set__course", + "problem_set", + "_order", + ) + search_fields = ( + "title", + "description", + ) + + def title(self, obj): + return obj.problem.title + + def course(self, obj): + return obj.problem_set.course + + course.admin_order_field = "problem_set__course" + admin.site.register(Course, CourseAdmin) admin.site.register(ProblemSet, ProblemSetAdmin) admin.site.register(CourseGroup, CourseGroupAdmin) diff --git a/web/courses/models.py b/web/courses/models.py index 12906be5..012abd8e 100644 --- a/web/courses/models.py +++ b/web/courses/models.py @@ -52,7 +52,7 @@ def user_attempts(self, user): users = User.objects.filter(id=user.id) outcomes = Outcome.group_dict(parts, users, ("problem",), ()) attempts = {} - for attempt in user.attempts.filter(part__problem__problem_set__course=self): + for attempt in user.attempts.filter(problem_set__course=self): attempts[attempt.part_id] = attempt sorted_attempts = [] for problem_set in self.problem_sets.all().prefetch_related("problems__parts"): @@ -93,7 +93,7 @@ def annotate_for_user(self, user): def annotate_for_teacher(self): parts = Part.objects.filter( - problem__problem_set__course=self, problem__visible=True + problem_instance__problem_set__course=self, problem_instance__visible=True ) users = self.observed_students() outcomes = Outcome.group_dict(parts, users, ("problem__problem_set",), ()) @@ -128,7 +128,7 @@ def observed_students(self): def student_outcome(self): parts = Part.objects.filter( - problem__problem_set__course=self, problem__problem_set__visible=True + problem_instance__problem_set__course=self, problem_instance__visible=True, problem_instance__problem_set__visible=True ) users = self.observed_students() outcomes = Outcome.group_dict(parts, users, (), ("id",)) @@ -220,15 +220,22 @@ def get_absolute_url(self): return reverse("problem_set_detail", args=[str(self.pk)]) - @property + def visible_problem_instances(self): + return self.problem_instances.filter(visible=True) + def visible_problems(self): - return self.problems.filter(visible=True) + for instance in self.visible_problem_instances().prefetch_related("problem__part"): + yield instance.problem + + def all_problems(self): + for instance in self.problem_instances().prefetch_related("problem__part"): + yield instance.problem def attempts_archive(self, user): if user.can_edit_problem_set(self): - files = [problem.attempt_file(user) for problem in self.problems.all()] + files = [problem.attempt_file(user) for problem in self.all_problems()] else: - files = [problem.attempt_file(user) for problem in self.visible_problems] + files = [problem.attempt_file(user) for problem in self.visible_problems()] archive_name = slugify(self.title) return archive_name, files @@ -245,7 +252,7 @@ def edit_archive(self, user): def attempt_history(self): user_attempts = {} attempts = ( - HistoricalAttempt.objects.filter(part__problem__problem_set=self) + HistoricalAttempt.objects.filter(problem_set=self) .select_related("part__problem", "user") .distinct() .order_by("history_date") @@ -257,7 +264,7 @@ def attempt_history(self): def results_archive(self, user): user_ids = set() attempt_dict = {} - attempts = Attempt.objects.filter(part__problem__problem_set=self) + attempts = Attempt.objects.filter(problem_set=self) for attempt in attempts: user_id = attempt.user_id user_ids.add(user_id) @@ -334,13 +341,13 @@ def outcomes_statistics(self, outcomes): def single_student_statistics(self, student): students = User.objects.filter(id=student.id) - parts = Part.objects.filter(problem__problem_set=self, problem__visible=True) + parts = Part.objects.filter(problem_instance__problem_set=self, problem_instance__visible=True) outcomes = Outcome.group_dict(parts, students, ("id",), ()) return self.outcomes_statistics(outcomes) def all_students_statistics(self): students = self.course.observed_students() - parts = Part.objects.filter(problem__problem_set=self) + parts = Part.objects.filter(problem_instance__problem_set=self) outcomes = Outcome.group_dict(parts, students, ("id",), ()) return self.outcomes_statistics(outcomes) @@ -372,7 +379,7 @@ class ProblemInstance(OrderWithRespectToMixin, models.Model): problem = models.ForeignKey( "problems.Problem", on_delete=models.CASCADE, related_name="instances" ) - problem_set = models.ForeignKey("courses.ProblemSet", on_delete=models.CASCADE) + problem_set = models.ForeignKey("courses.ProblemSet", on_delete=models.CASCADE, related_name="problem_instances") visible = models.BooleanField(default=False, verbose_name=_("Visible")) class Meta: @@ -380,3 +387,112 @@ class Meta: def __str__(self): return self.problem.title + + def user_attempts(self, user): + return user.attempts.filter(problem_instance=self) + + def user_solutions(self, user): + return { + attempt.part.id: attempt.solution for attempt in self.user_attempts(user) + } + + def attempt_file(self, user): + authentication_token = Token.objects.get(user=user) + solutions = self.user_solutions(user) + parts = [ + (part, solutions.get(part.id, part.template), part.attempt_token(user)) + for part in self.parts.all() + ] + url = settings.SUBMISSION_URL + reverse("attempts-submit") + problem_slug = slugify(self.title).replace("-", "_") + extension = self.EXTENSIONS[self.language] + filename = f"{problem_slug}.{extension}" + contents = render_to_string( + f"{self.language}/attempt.{extension}", + { + "problem_instance": self, + "problem": self.problem, + "parts": parts, + "submission_url": url, + "authentication_token": authentication_token, + }, + ) + return filename, contents + + + def solution_file(self): + return self.problem.solution_file() + + 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.problem.parts.all()] + username = user.get_full_name() or user.username + problem_slug = slugify(username).replace("-", "_") + extension = self.problem.extension() + filename = "{0}.{1}".format(problem_slug, extension) + contents = render_to_string( + "{0}/marking.{1}".format(self.language, extension), + { + "problem": self.problem, + "parts": parts, + "user": 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.problem.parts.all()] + username = user.get_full_name() or user.username + problem_slug = slugify(username).replace("-", "_") + extension = self.problem.extension() + filename = "{0}.{1}".format(problem_slug, extension) + contents = render_to_string( + "{0}/bare.{1}".format(self.problem.language, extension), + { + "problem": self, + "parts": parts, + "user": user, + }, + ) + return filename, contents + + def attempts_by_user(self, active_only=True): + attempts = {} + parts = self.problem.parts.all().prefetch_related("attempts", "attempts__user") + for attempt in self.attempts.select_related("user", "part"): + if attempt.user in attempts: + attempts[attempt.user][attempt.part] = attempt + else: + attempts[attempt.user] = {attempt.part: attempt} + users = self.problem_set.course.observed_students() + if active_only: + users = users.filter(attempts__problem_instance=self).distinct() + outcomes = Outcome.group_dict(parts, users, (), ("id",)) + observed_students = list(users) + for user in observed_students: + user.these_attempts = [attempts.get(user, {}).get(part) for part in parts] + user.outcome = outcomes[(user.id,)] + return observed_students + + def attempts_by_user_all(self): + return self.attempts_by_user(active_only=False) + + def toggle_visible(self): + self.visible = not self.visible + self.save() + + def copy_to(self, problem_set, duplicate_problem): + new_instance = deepcopy(self) + new_instance.pk = None + if duplicate_problem: + new_instance.problem = self.problem.duplicate() + new_instance.problem_set = problem_set + new_instance.save() + return new_instance diff --git a/web/problems/admin.py b/web/problems/admin.py index 90607050..3b9fa7d6 100644 --- a/web/problems/admin.py +++ b/web/problems/admin.py @@ -13,16 +13,11 @@ class ProblemAdmin(SimpleHistoryAdmin): inlines = (PartInline,) list_display = ( "title", - "course", - "problem_set", "description", - "visible", ) list_display_links = ("title",) ordering = ( - "problem_set__course", - "problem_set", - "_order", + "title", ) search_fields = ( "title", @@ -31,15 +26,7 @@ class ProblemAdmin(SimpleHistoryAdmin): list_filter = ( "language", - "problem_set__course__institution", - "problem_set__course", - "problem_set", ) - def course(self, obj): - return obj.problem_set.course - - course.admin_order_field = "problem_set__course" - admin.site.register(Problem, ProblemAdmin) diff --git a/web/problems/models.py b/web/problems/models.py index fc0d9371..de19eae6 100644 --- a/web/problems/models.py +++ b/web/problems/models.py @@ -16,13 +16,9 @@ from utils.models import OrderWithRespectToMixin -class Problem(OrderWithRespectToMixin, models.Model): +class Problem(models.Model): title = models.CharField(max_length=70) description = models.TextField(blank=True) - 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( @@ -33,9 +29,6 @@ class Problem(OrderWithRespectToMixin, models.Model): EXTENSIONS = {"python": "py", "octave": "m", "r": "r"} MIMETYPES = {"python": "text/x-python", "octave": "text/x-octave", "r": "text/x-R"} - class Meta: - order_with_respect_to = "problem_set" - def __str__(self): return self.title @@ -53,40 +46,10 @@ def get_absolute_url(self): def anchor(self): return f"problem-{self.pk}" - def user_attempts(self, user): - return user.attempts.filter(part__problem=self) - - def user_solutions(self, user): - return { - attempt.part.id: attempt.solution for attempt in self.user_attempts(user) - } - @property def slug(self): return slugify(self.title).replace("-", "_") - def attempt_file(self, user): - authentication_token = Token.objects.get(user=user) - solutions = self.user_solutions(user) - parts = [ - (part, solutions.get(part.id, part.template), part.attempt_token(user)) - for part in self.parts.all() - ] - url = settings.SUBMISSION_URL + reverse("attempts-submit") - problem_slug = slugify(self.title).replace("-", "_") - extension = self.EXTENSIONS[self.language] - filename = f"{problem_slug}.{extension}" - contents = render_to_string( - f"{self.language}/attempt.{extension}", - { - "problem": self, - "parts": parts, - "submission_url": url, - "authentication_token": authentication_token, - }, - ) - return filename, contents - def solution_file(self): parts = [(part, part.solution) for part in self.parts.all()] problem_slug = slugify(self.title).replace("-", "_") @@ -101,45 +64,6 @@ 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 - problem_slug = slugify(username).replace("-", "_") - extension = self.EXTENSIONS[self.language] - filename = "{0}.{1}".format(problem_slug, extension) - contents = render_to_string( - "{0}/marking.{1}".format(self.language, extension), - { - "problem": self, - "parts": parts, - "user": 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 - problem_slug = slugify(username).replace("-", "_") - extension = self.EXTENSIONS[self.language] - filename = "{0}.{1}".format(problem_slug, extension) - contents = render_to_string( - "{0}/bare.{1}".format(self.language, extension), - { - "problem": self, - "parts": parts, - "user": user, - }, - ) - return filename, contents def edit_file(self, user): """This function ignores problem visibility because it assumes its @@ -159,32 +83,9 @@ def edit_file(self, user): ) return filename, contents - def attempts_by_user(self, active_only=True): - attempts = {} - parts = self.parts.all().prefetch_related("attempts", "attempts__user") - for part in parts: - for attempt in part.attempts.all(): - if attempt.user in attempts: - attempts[attempt.user][part] = attempt - else: - attempts[attempt.user] = {part: attempt} - users = self.problem_set.course.observed_students() - if active_only: - users = users.filter(attempts__part__problem=self).distinct() - outcomes = Outcome.group_dict(parts, users, (), ("id",)) - observed_students = list(users) - for user in observed_students: - user.these_attempts = [attempts.get(user, {}).get(part) for part in parts] - user.outcome = outcomes[(user.id,)] - return observed_students - - def attempts_by_user_all(self): - return self.attempts_by_user(active_only=False) - - def copy_to(self, problem_set): + def duplicate(self): new_problem = deepcopy(self) new_problem.pk = None - new_problem.problem_set = problem_set new_problem.save() for part in self.parts.all(): part.copy_to(new_problem) @@ -192,10 +93,9 @@ def copy_to(self, problem_set): def content_type(self): return self.MIMETYPES[self.language] - - def toggle_visible(self): - self.visible = not self.visible - self.save() + + def extension(self): + return self.EXTENSIONS[self.language] class Part(OrderWithRespectToMixin, models.Model): diff --git a/web/problems/templates/python/attempt.py b/web/problems/templates/python/attempt.py index 0827632d..c36388a0 100644 --- a/web/problems/templates/python/attempt.py +++ b/web/problems/templates/python/attempt.py @@ -166,6 +166,7 @@ def submit_parts(parts, url, token): "valid": part["valid"], "secret": [x for (x, _) in part["secret"]], "feedback": json.dumps(part["feedback"]), + "problem_instance": {{ problem_instance.pk }}, } if "token" in part: submitted_part["token"] = part["token"]