From e0fd9e226c0159c449806adb4b02f6b64ce53c92 Mon Sep 17 00:00:00 2001 From: Bianca Necula Date: Fri, 18 Sep 2020 17:51:58 +0300 Subject: [PATCH 1/5] Comment section for code review --- interface/codeview.py | 4 ++ interface/forms.py | 11 +++ interface/migrations/0023_comment.py | 28 ++++++++ interface/models.py | 14 ++++ interface/templates/interface/code_view.html | 76 +++++++++++++++++++- interface/views.py | 32 ++++++++- 6 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 interface/migrations/0023_comment.py diff --git a/interface/codeview.py b/interface/codeview.py index 3ac9dae2..c7df5e8f 100644 --- a/interface/codeview.py +++ b/interface/codeview.py @@ -57,3 +57,7 @@ def make_dict(submission_archive): @register.filter def get_item(dictionary, key): return dictionary.get(key) + + +def table_maker(file): + return file.read().splitlines() diff --git a/interface/forms.py b/interface/forms.py index e2164e22..4f5d35fd 100644 --- a/interface/forms.py +++ b/interface/forms.py @@ -1,6 +1,7 @@ from django import forms from django.conf import settings from django.template.defaultfilters import filesizeformat +from .models import Comment class UploadFileForm(forms.Form): @@ -24,3 +25,13 @@ def clean_file(self): class LoginForm(forms.Form): username = forms.CharField(label="Username") password = forms.CharField(label="Password", widget=forms.PasswordInput) + + +class CommentForm(forms.ModelForm): + class Meta: + model = Comment + fields = ("text",) + + widgets = { + "text": forms.Textarea(attrs={"class": "form-control"}), + } diff --git a/interface/migrations/0023_comment.py b/interface/migrations/0023_comment.py new file mode 100644 index 00000000..e4b6f901 --- /dev/null +++ b/interface/migrations/0023_comment.py @@ -0,0 +1,28 @@ +# Generated by Django 3.0.6 on 2020-09-07 17:48 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('interface', '0022_auto_20200709_2326'), + ] + + operations = [ + migrations.CreateModel( + name='Comment', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('path', models.CharField(max_length=256)), + ('line', models.IntegerField(null=True)), + ('text', models.TextField(blank=True, default='', max_length=4096)), + ('created', models.DateTimeField(auto_now_add=True)), + ('submission', models.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, related_name='comments', to='interface.Submission')), + ('user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/interface/models.py b/interface/models.py index e32df2ee..42d54a2c 100644 --- a/interface/models.py +++ b/interface/models.py @@ -213,4 +213,18 @@ def verify_jwt(self, message): return decoded_message["data"] == str(self.id) +class Comment(models.Model): + path = models.CharField(max_length=256, blank=False) + line = models.IntegerField(null=True) + submission = models.ForeignKey( + Submission, + on_delete=models.PROTECT, + null=True, + related_name="comments", + ) + user = models.ForeignKey(User, on_delete=models.PROTECT, null=True) + text = models.TextField(max_length=4096, default="", blank=True) + created = models.DateTimeField(auto_now_add=True) + + pre_save.connect(signals.update_total_score, sender=Submission) diff --git a/interface/templates/interface/code_view.html b/interface/templates/interface/code_view.html index 890b5d22..aa1396aa 100644 --- a/interface/templates/interface/code_view.html +++ b/interface/templates/interface/code_view.html @@ -1,5 +1,19 @@ {% extends "interface/generics/base.html" %} + + + {% block body_content %}
@@ -10,7 +24,67 @@ {% endwith %}
- 
 {{ file_content }}
+
+ + + + + + + + + {% for line in file_content %} + + + + + + + + {% endfor %} + +
{{ forloop.counter }} + 
{{ line }}
+
+
+
+ {% with forloop.counter as index %} + {% for comment in path_comments %} + {% if comment.line == index %} +
+

+ Comment {{ forloop.counter }} by {{ comment.user }} + {{ comment.created }} +

+ {{ comment.text|linebreaks }} +
+ {% endif %} + {% empty %} +

There are no comments yet.

+ {% endfor %} + {% endwith %} + {% if new_comment %} +

Your comment has been added.

+ {% else %} +
+
+
+ {% csrf_token %} + {{ form.as_p }} +
+
+ +
+
+

+
+
+
+ {% endif %} +
+
+
+
diff --git a/interface/views.py b/interface/views.py index cd090941..cbb505f9 100644 --- a/interface/views.py +++ b/interface/views.py @@ -21,7 +21,7 @@ from interface import models from interface import utils -from interface.forms import UploadFileForm, LoginForm +from interface.forms import UploadFileForm, LoginForm, CommentForm from interface.models import Submission, Course, User from interface.backend.submission.submission import ( handle_submission, @@ -30,7 +30,7 @@ ) from .scoring import calculate_total_score from interface.actions_logger import log_action -from interface.codeview import extract_file, tree_view +from interface.codeview import extract_file, tree_view, table_maker log = logging.getLogger(__name__) @@ -357,7 +357,33 @@ def code_view(request, pk, filename): file = extract_file(request, submission, filename) - context = {"file_content": file.read(), "tree": tree, "pk": submission.pk} + table = table_maker(file) + + comment = None + if request.method == "POST": + form = CommentForm(request.POST) + if form.is_valid(): + comment = form.save(commit=False) + comment.submission = submission + comment.user = request.user + comment.path = filename + comment.line = request.POST.get("line", "") + comment.save() + return redirect("code_view", pk=pk, filename=filename) + else: + form = CommentForm() + + path_comments = submission.comments.filter(path=filename) + + context = { + "file_content": table, + "tree": tree, + "pk": submission.pk, + "path": filename, + "path_comments": path_comments, + "new_comment": comment, + "form": form, + } file.close() From 4df6a042e7b4e48b29b219b0fb07ed1a1945141a Mon Sep 17 00:00:00 2001 From: Bianca Necula Date: Sat, 19 Sep 2020 21:29:27 +0300 Subject: [PATCH 2/5] Highlight lines and files with comments --- interface/codeview.py | 10 ++++++++++ interface/templates/interface/code_view.html | 16 +++++++++++----- interface/templates/interface/tree_view.html | 8 +++++++- interface/views.py | 1 + 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/interface/codeview.py b/interface/codeview.py index c7df5e8f..1712dc92 100644 --- a/interface/codeview.py +++ b/interface/codeview.py @@ -61,3 +61,13 @@ def get_item(dictionary, key): def table_maker(file): return file.read().splitlines() + + +@register.filter +def with_path(things, path): + return things.filter(path=path) + + +@register.filter +def with_line(things, line): + return things.filter(line=line) diff --git a/interface/templates/interface/code_view.html b/interface/templates/interface/code_view.html index aa1396aa..2cde9115 100644 --- a/interface/templates/interface/code_view.html +++ b/interface/templates/interface/code_view.html @@ -25,7 +25,7 @@
- +
@@ -34,10 +34,16 @@ {% for line in file_content %} - - - + + diff --git a/interface/templates/interface/tree_view.html b/interface/templates/interface/tree_view.html index 79f9aa11..a35981a1 100644 --- a/interface/templates/interface/tree_view.html +++ b/interface/templates/interface/tree_view.html @@ -3,7 +3,13 @@
    {% for key, value in dict %} {% if '$path' in value and value.items %} - {{ key }}
    + {% with value|get_item:'$path' as path %} + {% if sub.comments|with_path:path %} + {{ key }}
    + {% else %} + {{ key }}
    + {% endif %} + {% endwith %} {% else %} {{ key }} {% with dict=value.items template="interface/tree_view.html" %} diff --git a/interface/views.py b/interface/views.py index cbb505f9..9b078dae 100644 --- a/interface/views.py +++ b/interface/views.py @@ -379,6 +379,7 @@ def code_view(request, pk, filename): "file_content": table, "tree": tree, "pk": submission.pk, + "sub": submission, "path": filename, "path_comments": path_comments, "new_comment": comment, From dec3558fac32192b2ae5fbdfae7b943437e8fd9c Mon Sep 17 00:00:00 2001 From: Bianca Necula Date: Sat, 26 Sep 2020 21:40:35 +0300 Subject: [PATCH 3/5] Review in comments --- interface/scoring.py | 20 ++- interface/templates/interface/code_view.html | 144 ++++++++++-------- .../interface/code_view_homepage.html | 39 +++++ .../interface/generics/review_modal.html | 4 +- .../interface/submission_result.html | 22 +-- interface/urls.py | 2 +- interface/views.py | 66 +++++--- testsuite/test_student_workflow.py | 4 +- testsuite/test_submission_actions.py | 2 +- testsuite/test_ta_workflow.py | 8 +- 10 files changed, 199 insertions(+), 112 deletions(-) create mode 100644 interface/templates/interface/code_view_homepage.html diff --git a/interface/scoring.py b/interface/scoring.py index 4f3ab082..fd453a7e 100644 --- a/interface/scoring.py +++ b/interface/scoring.py @@ -94,9 +94,27 @@ def compute_review_score(submission): return sum([decimal.Decimal(mark) for mark in marks]) +def compute_comments_review(submission): + total_sum = 0 + for comment in submission.comments.all(): + if ( + comment.user + in submission.assignment.course.teaching_assistants.all() + ): + marks = re.findall( + r"^([+-]\d+\.*\d*):", comment.text, re.MULTILINE, + ) + log.debug("Marks found: " + str(marks)) + total_sum += sum([decimal.Decimal(mark) for mark in marks]) + + return total_sum + + def calculate_total_score(submission): score = submission.score if submission.score else 0 - submission.review_score = compute_review_score(submission) + submission.review_score = compute_review_score( + submission + ) + compute_comments_review(submission) (penalties, holiday_start, holiday_finish) = get_penalty_info(submission) timestamp = submission.timestamp or datetime.datetime.now() diff --git a/interface/templates/interface/code_view.html b/interface/templates/interface/code_view.html index 2cde9115..9f66bab9 100644 --- a/interface/templates/interface/code_view.html +++ b/interface/templates/interface/code_view.html @@ -19,80 +19,94 @@
    - {% with dict=tree.items template="interface/tree_view.html" %} - {% include template %} - {% endwith %} +
    + {% if user in sub.assignment.course.teaching_assistants.all %} +

    + +

    + {% endif %} +
    + {% with dict=tree.items template="interface/tree_view.html" %} + {% include template %} + {% endwith %}
    -
{{ forloop.counter }} - 
{{ line }}
+
{{ forloop.counter }} + {% with forloop.counter as index %} + {% if sub.comments|with_path:path|with_line:index %} +
{{ line }}
+ {% else %} +
{{ line }}
+ {% endif %} + {% endwith %}
- - - - - - - - {% for line in file_content %} - - - - - - + + {% endfor %} + +
{{ forloop.counter }} - {% with forloop.counter as index %} - {% if sub.comments|with_path:path|with_line:index %} -
{{ line }}
- {% else %} -
{{ line }}
- {% endif %} - {% endwith %} -
-
-
- {% with forloop.counter as index %} - {% for comment in path_comments %} - {% if comment.line == index %} -
-

- Comment {{ forloop.counter }} by {{ comment.user }} - {{ comment.created }} -

- {{ comment.text|linebreaks }} -
- {% endif %} - {% empty %} -

There are no comments yet.

- {% endfor %} - {% endwith %} - {% if new_comment %} -

Your comment has been added.

- {% else %} -
-
-
- {% csrf_token %} - {{ form.as_p }} + {% if not file_exists %} + {{ file_content }} + {% else %} + + + + + + + + + {% for line in file_content %} + + + + + + - - {% endfor %} - -
+
{{ forloop.counter }} + {% with forloop.counter as index %} + {% if sub.comments|with_path:path|with_line:index %} +
{{ line }}
+ {% else %} +
{{ line }}
+ {% endif %} + {% endwith %} +
+
+
+ {% with forloop.counter as index %} + {% for comment in path_comments %} + {% if comment.line == index %} +
+

+ Comment {{ forloop.counter }} by {{ comment.user }} + {{ comment.created }} +

+ {{ comment.text|linebreaks }} +
+ {% endif %} + {% empty %} +

There are no comments yet.

+ {% endfor %} + {% endwith %} + {% if new_comment %} +

Your comment has been added.

+ {% else %} + +
+
+ {% csrf_token %} + {{ form.as_p }} +
-
- -
-
-

+ +
+
+

+
-
- - {% endif %} + + {% endif %} +
- -
+
+ {% endif %}
+{% include "interface/generics/review_modal.html" %} + {% endblock %} \ No newline at end of file diff --git a/interface/templates/interface/code_view_homepage.html b/interface/templates/interface/code_view_homepage.html new file mode 100644 index 00000000..2c050a54 --- /dev/null +++ b/interface/templates/interface/code_view_homepage.html @@ -0,0 +1,39 @@ +{% extends "interface/generics/base.html" %} + +{% block body_content %} + +
+
+
+ {% with dict=tree.items template="interface/tree_view.html" %} + {% include template %} + {% endwith %} +
+
+
+
+
+ 
+                        /^--^\     /^--^\     /^--^\
+                        \____/     \____/     \____/
+                       /      \   /      \   /      \
+                      |        | |        | |        |
+                       \__  __/   \__  __/   \__  __/
+  |^|^|^|^|^|^|^|^|^|^|^|^\ \^|^|^|^/ /^|^|^|^|^\ \^|^|^|^|^|^|^|^|^|^|^|^|
+  | | | | | | | | | | | | |\ \| | |/ /| | | | | | \ \ | | | | | | | | | | |
+  | | | | | | | | | | | | / / | | |\ \| | | | | |/ /| | | | | | | | | | | |
+  | | | | | | | | | | | | \/| | | | \/| | | | | |\/ | | | | | | | | | | | |
+  #########################################################################
+  | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | |
+  | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | |
+
+
+
+
+
+
+
+ + + +{% endblock %} \ No newline at end of file diff --git a/interface/templates/interface/generics/review_modal.html b/interface/templates/interface/generics/review_modal.html index 69b29854..cebf0c74 100644 --- a/interface/templates/interface/generics/review_modal.html +++ b/interface/templates/interface/generics/review_modal.html @@ -8,11 +8,11 @@
-
+ {% csrf_token %}
- +
diff --git a/interface/templates/interface/submission_result.html b/interface/templates/interface/submission_result.html index eea61f7c..3b09a1c4 100644 --- a/interface/templates/interface/submission_result.html +++ b/interface/templates/interface/submission_result.html @@ -14,26 +14,26 @@

Submission {{ sub }} ({{ sub.state }})

{% endif %} - {% if user in sub.assignment.course.teaching_assistants.all%} -

- -

- {% endif %} - {% if user in sub.assignment.course.teaching_assistants.all %}
{% csrf_token %}
- {% endif %} + {% endif %} - {% if user in sub.assignment.course.teaching_assistants.all %} + {% if user in sub.assignment.course.teaching_assistants.all %}
{% csrf_token %}
- {% endif %} - + {% endif %} + + {% if user == sub.user or user in sub.assignment.course.teaching_assistants.all %} +
+ +
+ {% endif %} + {% if sub.state == sub.STATE_DONE %} @@ -79,6 +79,6 @@

Fortune teller

-{% include "interface/generics/review_modal.html" %} + {% endblock %} diff --git a/interface/urls.py b/interface/urls.py index 2166b8f4..6e2eaf34 100644 --- a/interface/urls.py +++ b/interface/urls.py @@ -19,7 +19,7 @@ name="submission_result", ), path("submission//done", views.done), - path("submission//review", views.review), + path("submission//review/", views.review, name="review"), path("submission//download", views.download), path("submission//rerun", views.rerun_submission), path("submission//recompute", views.recompute_score), diff --git a/interface/views.py b/interface/views.py index 9b078dae..df3936a2 100644 --- a/interface/views.py +++ b/interface/views.py @@ -146,7 +146,7 @@ def review(request, pk): log_action("Review submission", request.user, submission) - return redirect(request.META.get("HTTP_REFERER", "/")) + return redirect(f"/submission/{submission.pk}/code/") @login_required @@ -220,7 +220,6 @@ def submission_result(request, pk): { "sub": sub, "homepage_url": redirect(homepage).url, - "submission_review_message": sub.review_message, "submission_list_url": redirect(submission_list).url, "fortune": fortune_msg, }, @@ -350,42 +349,59 @@ def code_view(request, pk, filename): submission = get_object_or_404(Submission, pk=pk) all_tas = submission.assignment.course.teaching_assistants.all() - if request.user not in all_tas: + if request.user not in all_tas and request.user is not submission.user: return HttpResponse(status=403) tree = tree_view(request, submission) - file = extract_file(request, submission, filename) - - table = table_maker(file) + if filename == "code": + context = { + "tree": tree, + "pk": submission.pk, + "sub": submission, + "path": filename, + } + return render(request, "interface/code_view_homepage.html", context) - comment = None - if request.method == "POST": - form = CommentForm(request.POST) - if form.is_valid(): - comment = form.save(commit=False) - comment.submission = submission - comment.user = request.user - comment.path = filename - comment.line = request.POST.get("line", "") - comment.save() - return redirect("code_view", pk=pk, filename=filename) - else: - form = CommentForm() - - path_comments = submission.comments.filter(path=filename) + file = extract_file(request, submission, filename) context = { - "file_content": table, + "file_content": file.getvalue(), "tree": tree, "pk": submission.pk, "sub": submission, "path": filename, - "path_comments": path_comments, - "new_comment": comment, - "form": form, + "file_exists": False, } + if ( + file.getvalue() != "The file is missing!" + and file.getvalue() != "The archive is missing!" + ): + table = table_maker(file) + + comment = None + if request.method == "POST": + form = CommentForm(request.POST) + if form.is_valid(): + comment = form.save(commit=False) + comment.submission = submission + comment.user = request.user + comment.path = filename + comment.line = request.POST.get("line", "") + comment.save() + return redirect("code_view", pk=pk, filename=filename) + else: + form = CommentForm() + + path_comments = submission.comments.filter(path=filename) + + context["file_content"] = table + context["file_exists"] = True + context["path_comments"] = path_comments + context["new_comment"] = comment + context["form"] = form + file.close() return render(request, "interface/code_view.html", context) diff --git a/testsuite/test_student_workflow.py b/testsuite/test_student_workflow.py index 9b1c6b8b..26fdf3c1 100644 --- a/testsuite/test_student_workflow.py +++ b/testsuite/test_student_workflow.py @@ -163,11 +163,11 @@ def test_user_cannot_review(client, STC, base_db_setup): review_message = "+10.0: Hacker" response = client.post( - f"/submission/{submission.pk}/review", + f"/submission/{submission.pk}/review/", data={"review-code": review_message}, ) STC.assertRedirects( - response, f"/admin/login/?next=/submission/{submission.pk}/review", + response, f"/admin/login/?next=/submission/{submission.pk}/review/", ) diff --git a/testsuite/test_submission_actions.py b/testsuite/test_submission_actions.py index 6a680a35..7f271665 100644 --- a/testsuite/test_submission_actions.py +++ b/testsuite/test_submission_actions.py @@ -27,7 +27,7 @@ def test_review(client, base_db_setup): review_message = "+10.0: Good Job\n-5.0: Bad style\n+0.5:Good Readme" client.post( - f"/submission/{submission.pk}/review", + f"/submission/{submission.pk}/review/", data={"review-code": review_message}, ) diff --git a/testsuite/test_ta_workflow.py b/testsuite/test_ta_workflow.py index ab90a192..116f33f4 100644 --- a/testsuite/test_ta_workflow.py +++ b/testsuite/test_ta_workflow.py @@ -486,12 +486,12 @@ def test_ta_review_submission(client, STC, base_db_setup): # Add review and add some points review_message = "+10.0: Babas" response = client.post( - f"/submission/{submission.pk}/review", + f"/submission/{submission.pk}/review/", data={"review-code": review_message}, follow=True, ) - STC.assertRedirects(response, "/homepage/") + STC.assertRedirects(response, f"/submission/{submission.pk}/code/") all_subs = assignment.submission_set.all() assert len(all_subs) == 1 @@ -503,12 +503,12 @@ def test_ta_review_submission(client, STC, base_db_setup): # Add review and substract some points review_message = "-20.0: Not nice" response = client.post( - f"/submission/{submission.pk}/review", + f"/submission/{submission.pk}/review/", data={"review-code": review_message}, follow=True, ) - STC.assertRedirects(response, "/homepage/") + STC.assertRedirects(response, f"/submission/{submission.pk}/code/") all_subs = assignment.submission_set.all() assert len(all_subs) == 1 From 8b757f6cb2da7169827ac1e7f74f2eaa08b272e5 Mon Sep 17 00:00:00 2001 From: Bianca Necula Date: Mon, 28 Sep 2020 15:29:13 +0300 Subject: [PATCH 4/5] Tests --- testsuite/bigtest.zip | Bin 348 -> 693 bytes testsuite/test_ta_workflow.py | 74 ++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/testsuite/bigtest.zip b/testsuite/bigtest.zip index 029b5f0abd5c374f171a4175b39380ba318e3807..b98e8f44862b14180c144308c35aa36397245ede 100644 GIT binary patch delta 425 zcmcb^w3T&&u`mZiXMs&%tl*TMNJa*RY9q95T_FZw{X;~k{F7Z_8G`G;2 z2aUDgOKc9VsNZzAY|HWGRU%9tflDmPYM*|Zer@Z4Z-T7Zr`IOWaPIoWyYlM7lUElz zhuzLqSuJfcTfbtnvlmZJEMGM1JuCT;-T(JIG!SoHB9iLiI_=20*Pm^3bSiphojaBj zozj?6-OpL)p|ZQac*)vp3Tq-t-)h^2Nmm{WzqllnE$nuLWSD|2>lOd7iTtzQ?KmX1 z>w)#eE5|0By_4&ponCkJgW1u-Ei(JKk8f4nRPJq6pbZ-|9|LW( zI5+04$buOxGcA_fw&$z+QD5nIZfU8I?|O^QrO7*{&E0>#=1?F{`R?fKR1xzN_ZMiK z{Jh|+*4z!|vgfMO5|;DU+HlNxqWnot@7%WYCHphk#7{f!L@49i+|Zf zo(ppDK2z{(-oIz5>EE`+z0|Ad)cdafj?2wIkIs3=9A{-@9@E delta 59 zcmdnWdWUI(F*5^$$mIQuDvXAcw=kB1c{xn7lRcRn7^5ejXA~D Date: Wed, 30 Sep 2020 18:45:01 +0300 Subject: [PATCH 5/5] Separate views in codeview --- interface/scoring.py | 8 ++-- .../interface/submission_result.html | 2 +- interface/templates/interface/tree_view.html | 10 ++--- interface/urls.py | 7 ++- interface/views.py | 35 +++++++++------ testsuite/test_student_workflow.py | 43 ++++++++++++++++++- testsuite/test_ta_workflow.py | 26 +++++++---- 7 files changed, 97 insertions(+), 34 deletions(-) diff --git a/interface/scoring.py b/interface/scoring.py index fd453a7e..8201f981 100644 --- a/interface/scoring.py +++ b/interface/scoring.py @@ -96,11 +96,11 @@ def compute_review_score(submission): def compute_comments_review(submission): total_sum = 0 + teaching_assistants = ( + submission.assignment.course.teaching_assistants.all() + ) for comment in submission.comments.all(): - if ( - comment.user - in submission.assignment.course.teaching_assistants.all() - ): + if comment.user in teaching_assistants: marks = re.findall( r"^([+-]\d+\.*\d*):", comment.text, re.MULTILINE, ) diff --git a/interface/templates/interface/submission_result.html b/interface/templates/interface/submission_result.html index 3b09a1c4..1d247f2b 100644 --- a/interface/templates/interface/submission_result.html +++ b/interface/templates/interface/submission_result.html @@ -30,7 +30,7 @@

Submission {{ sub }} ({{ sub.state }})

{% if user == sub.user or user in sub.assignment.course.teaching_assistants.all %}
- +
{% endif %} diff --git a/interface/templates/interface/tree_view.html b/interface/templates/interface/tree_view.html index a35981a1..dbe224e0 100644 --- a/interface/templates/interface/tree_view.html +++ b/interface/templates/interface/tree_view.html @@ -4,11 +4,11 @@ {% for key, value in dict %} {% if '$path' in value and value.items %} {% with value|get_item:'$path' as path %} - {% if sub.comments|with_path:path %} - {{ key }}
- {% else %} - {{ key }}
- {% endif %} + {% if sub.comments|with_path:path %} + {{ key }}
+ {% else %} + {{ key }}
+ {% endif %} {% endwith %} {% else %} {{ key }} diff --git a/interface/urls.py b/interface/urls.py index 6e2eaf34..69efa1f2 100644 --- a/interface/urls.py +++ b/interface/urls.py @@ -43,8 +43,13 @@ ), path("mysubmissions/", views.user_page, name="user_page"), path( - "submission///", + "submission//code//", views.code_view, name="code_view", ), + path( + "submission//code/", + views.code_view_homepage, + name="code_view_homepage", + ), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) diff --git a/interface/views.py b/interface/views.py index df3936a2..b2ff9ab2 100644 --- a/interface/views.py +++ b/interface/views.py @@ -354,16 +354,8 @@ def code_view(request, pk, filename): tree = tree_view(request, submission) - if filename == "code": - context = { - "tree": tree, - "pk": submission.pk, - "sub": submission, - "path": filename, - } - return render(request, "interface/code_view_homepage.html", context) - file = extract_file(request, submission, filename) + file_value = file.getvalue() context = { "file_content": file.getvalue(), @@ -374,10 +366,7 @@ def code_view(request, pk, filename): "file_exists": False, } - if ( - file.getvalue() != "The file is missing!" - and file.getvalue() != "The archive is missing!" - ): + if file_value not in ["The file is missing!", "The archive is missing!"]: table = table_maker(file) comment = None @@ -405,3 +394,23 @@ def code_view(request, pk, filename): file.close() return render(request, "interface/code_view.html", context) + + +@login_required +@staff_member_required +def code_view_homepage(request, pk): + submission = get_object_or_404(Submission, pk=pk) + all_tas = submission.assignment.course.teaching_assistants.all() + + if request.user not in all_tas and request.user is not submission.user: + return HttpResponse(status=403) + + tree = tree_view(request, submission) + + context = { + "tree": tree, + "pk": submission.pk, + "sub": submission, + } + + return render(request, "interface/code_view_homepage.html", context) diff --git a/testsuite/test_student_workflow.py b/testsuite/test_student_workflow.py index 26fdf3c1..8f167912 100644 --- a/testsuite/test_student_workflow.py +++ b/testsuite/test_student_workflow.py @@ -17,6 +17,7 @@ FILEPATH = settings.BASE_DIR / "testsuite" / "test.zip" +FILEPATH2 = settings.BASE_DIR / "testsuite" / "bigtest.zip" @pytest.fixture @@ -332,10 +333,48 @@ def test_user_code_view(client, STC, base_db_setup): submission = Submission.objects.all()[0] - response = client.get(f"/submission/{submission.pk}/test/", follow=True,) + response = client.get( + f"/submission/{submission.pk}/code/test/", follow=True, + ) STC.assertTemplateNotUsed(response, "interface/code_view.html") assert response.status_code == 200 STC.assertRedirects( - response, f"/admin/login/?next=/submission/{submission.pk}/test/", + response, f"/admin/login/?next=/submission/{submission.pk}/code/test/", ) + + +@pytest.mark.django_db +def test_user_cannot_comment_review(client, STC, base_db_setup): + (_, _, user, course, assignment) = base_db_setup + + client.login(username=user.username, password="pw") + + with open(FILEPATH2, "rb") as file: + upload = SimpleUploadedFile( + FILEPATH2.name, file.read(), content_type="application/zip", + ) + client.post( + f"/assignment/{course.pk}/{assignment.pk}/upload/", + data={"name": FILEPATH2.name, "file": upload}, + format="multipart", + ) + + submission = Submission.objects.all()[0] + submission.score = 100 + submission.save() + + client.post( + f"/submission/{submission.pk}/code/bigtest/dir1/file1/", + data={"text": "+10.0: I really did a nice job!", "line": "1"}, + ) + + submission.review_message = "" + submission.save() + + all_subs = assignment.submission_set.all() + assert len(all_subs) == 1 + + changed_sub = all_subs[0] + assert changed_sub.review_score == 0 + assert changed_sub.total_score == 100 diff --git a/testsuite/test_ta_workflow.py b/testsuite/test_ta_workflow.py index 8d441eab..7a5306ae 100644 --- a/testsuite/test_ta_workflow.py +++ b/testsuite/test_ta_workflow.py @@ -780,7 +780,9 @@ def test_ta_code_view(client, STC, base_db_setup): submission = Submission.objects.all()[0] - response = client.get(f"/submission/{submission.pk}/test", follow=True,) + response = client.get( + f"/submission/{submission.pk}/code/test", follow=True, + ) assert response.status_code == 200 STC.assertNotContains(response, "N/A") @@ -804,7 +806,9 @@ def test_ta_code_view_file_missing(client, STC, base_db_setup): submission = Submission.objects.all()[0] - response = client.get(f"/submission/{submission.pk}/test1", follow=True,) + response = client.get( + f"/submission/{submission.pk}/code/test1", follow=True, + ) assert response.status_code == 200 STC.assertContains(response, "The file is missing!") @@ -820,7 +824,9 @@ def test_ta_code_view_archive_missing(client, STC, base_db_setup): score=100.00, state=Submission.STATE_DONE, user=user, id=1000, ) - response = client.get(f"/submission/{submission.pk}/test", follow=True,) + response = client.get( + f"/submission/{submission.pk}/code/test", follow=True, + ) assert response.status_code == 200 STC.assertContains(response, "The archive is missing!") @@ -844,7 +850,9 @@ def test_ta_tree_view(client, STC, base_db_setup): submission = Submission.objects.all()[0] - response = client.get(f"/submission/{submission.pk}/bigtest/dir1/file1/",) + response = client.get( + f"/submission/{submission.pk}/code/bigtest/dir1/file1/", + ) assert response.status_code == 200 STC.assertContains(response, "dir1") @@ -871,15 +879,17 @@ def test_ta_comment(client, STC, base_db_setup): submission = Submission.objects.all()[0] response = client.post( - f"/submission/{submission.pk}/bigtest/dir1/file1/", + f"/submission/{submission.pk}/code/bigtest/dir1/file1/", data={"text": "This is a comment", "line": "1"}, ) STC.assertRedirects( - response, f"/submission/{submission.pk}/bigtest/dir1/file1/" + response, f"/submission/{submission.pk}/code/bigtest/dir1/file1/" ) - response = client.get(f"/submission/{submission.pk}/bigtest/dir1/file1/") + response = client.get( + f"/submission/{submission.pk}/code/bigtest/dir1/file1/" + ) STC.assertContains(response, "This is a comment") @@ -905,7 +915,7 @@ def test_ta_comment_review(client, STC, base_db_setup): submission.save() response = client.post( - f"/submission/{submission.pk}/bigtest/dir1/file1/", + f"/submission/{submission.pk}/code/bigtest/dir1/file1/", data={"text": "+10.0: Good job!", "line": "1"}, )