From 70e0e1b0cdb61f9070e731916652148681ab3c9d Mon Sep 17 00:00:00 2001 From: JasonGrace2282 Date: Sun, 1 Sep 2024 19:53:25 -0400 Subject: [PATCH 1/5] Implement views for file actions --- tin/apps/assignments/forms.py | 57 ++++++++- .../migrations/0033_fileaction_description.py | 18 +++ tin/apps/assignments/models.py | 11 +- .../assignments/tests/test_file_actions.py | 68 ++++++++++ tin/apps/assignments/urls.py | 15 +++ tin/apps/assignments/views.py | 98 +++++++++++++- tin/static/css/base.css | 18 +++ tin/static/css/choose.css | 99 ++++++++++++++ tin/static/css/login.css | 2 +- .../assignments/choose_file_action.html | 121 ++++++++++++++++++ .../assignments/custom_file_action.html | 73 +++++++++++ tin/templates/assignments/manage_files.html | 2 + tin/templates/base.html | 5 +- tin/templates/courses/show.html | 1 + tin/templates/login.html | 4 +- tin/tests/fixtures.py | 13 ++ tin/tests/utils.py | 27 ++++ 17 files changed, 625 insertions(+), 7 deletions(-) create mode 100644 tin/apps/assignments/migrations/0033_fileaction_description.py create mode 100644 tin/apps/assignments/tests/test_file_actions.py create mode 100644 tin/static/css/choose.css create mode 100644 tin/templates/assignments/choose_file_action.html create mode 100644 tin/templates/assignments/custom_file_action.html diff --git a/tin/apps/assignments/forms.py b/tin/apps/assignments/forms.py index bca6c406..9b53d79c 100644 --- a/tin/apps/assignments/forms.py +++ b/tin/apps/assignments/forms.py @@ -7,7 +7,7 @@ from django.conf import settings from ..submissions.models import Submission -from .models import Assignment, Folder, MossResult +from .models import Assignment, FileAction, Folder, MossResult logger = getLogger(__name__) @@ -241,3 +241,58 @@ class Meta: "name", ] help_texts = {"name": "Note: Folders are ordered alphabetically."} + + +class FileActionForm(forms.ModelForm): + """A form to create (or edit) a :class:`.FileAction`.""" + + def clean(self): + cleaned_data = super().clean() + if cleaned_data is None: + cleaned_data = self.cleaned_data + cmd = cleaned_data.get("command", "") + + if "$FILE" in cmd or "$FILES" in cmd: + if not cleaned_data.get("match_type"): + self.add_error("match_type", "required if command uses $FILE or $FILES") + if not cleaned_data.get("match_value"): + self.add_error("match_value", "required if command uses $FILE or $FILES") + + return cleaned_data + + class Meta: + model = FileAction + fields = [ + "name", + "description", + "command", + "match_type", + "match_value", + "case_sensitive_match", + ] + widgets = { + "description": forms.Textarea(attrs={"cols": 32, "rows": 2}), + } + + +class ChooseFileActionForm(forms.Form): + """A form to choose a file action. + + .. warning:: + + This will allow a user to modify any file action, + including file actions that are added to a course the user + is not a teacher in. + + This form is primarily intended for use with Javascript, + where the file action id cannot be determined at template rendering + time. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.fields["file_action"] = forms.ModelChoiceField( + queryset=FileAction.objects.all(), + widget=forms.HiddenInput(), + ) diff --git a/tin/apps/assignments/migrations/0033_fileaction_description.py b/tin/apps/assignments/migrations/0033_fileaction_description.py new file mode 100644 index 00000000..a7163557 --- /dev/null +++ b/tin/apps/assignments/migrations/0033_fileaction_description.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.15 on 2024-10-14 15:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('assignments', '0032_assignment_quiz_description_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='fileaction', + name='description', + field=models.CharField(blank=True, max_length=100), + ), + ] diff --git a/tin/apps/assignments/models.py b/tin/apps/assignments/models.py index 1b1dfcb6..ec5d2c93 100644 --- a/tin/apps/assignments/models.py +++ b/tin/apps/assignments/models.py @@ -588,11 +588,20 @@ def run_action(command: list[str]) -> str: class FileAction(models.Model): - """Runs a user uploaded script on files uploaded to an assignment.""" + """Runs a user uploaded script on files uploaded to an assignment. + + This can also take (fake) environment variables like ``$FILE``/``$FILES``, + which are replaced with their actual value. + + ``$FILES`` is expanded to a space separated list of paths that match the filter. + + ``$FILE`` means the command will be called once with each file that matches the filter. + """ MATCH_TYPES = (("S", "Start with"), ("E", "End with"), ("C", "Contain")) name = models.CharField(max_length=50) + description = models.CharField(max_length=100, blank=True) courses = models.ManyToManyField(Course, related_name="file_actions") command = models.CharField(max_length=1024) diff --git a/tin/apps/assignments/tests/test_file_actions.py b/tin/apps/assignments/tests/test_file_actions.py new file mode 100644 index 00000000..c7166a33 --- /dev/null +++ b/tin/apps/assignments/tests/test_file_actions.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import json + +from django.urls import reverse + +from tin.tests import login, model_to_dict + +from ..models import FileAction + + +@login("teacher") +def test_choose_file_action_view(client, course, file_action) -> None: + # make sure the view works + response = client.get(reverse("assignments:choose_file_action", args=[course.id])) + assert response.status_code == 200 + + file_action.courses.clear() + response = client.post( + reverse("assignments:choose_file_action", args=[course.id]), + {"file_action": file_action.id}, + ) + assert json.loads(response.content.decode("utf-8")).get("success") is True + assert file_action.courses.filter(id=course.id).exists() + + +@login("teacher") +def test_create_file_action_view(client, course) -> None: + url = reverse("assignments:create_file_action", args=[course.id]) + # make sure the view works normally + response = client.get(url) + assert response.status_code == 200 + + response = client.post(url, {"name": "Hi", "command": "echo bye"}) + assert course.file_actions.count() == 1 + + response = client.post(url, {"name": "Hi", "command": "echo $FILES"}) + assert ( + course.file_actions.count() == 1 + ), f"Creation form should error if $FILES is a command without a match value (got {response})" + + file_action = course.file_actions.first() + assert file_action is not None + fa_data = model_to_dict(file_action) + + # try copying the data + response = client.post(f"{url}?action={file_action.id}", {**fa_data, "copy": True}) + assert ( + course.file_actions.count() == 2 + ), "Passing copy as a POST parameter should copy the file action" + + # or modifying the original instance + client.post(f"{url}?action={file_action.id}", fa_data | {"name": "New name!"}) + file_action.refresh_from_db() + assert file_action.name == "New name!" + + +@login("teacher") +def test_delete_file_action_view(client, course, file_action) -> None: + response = client.post( + f"{reverse('assignments:delete_file_action', args=[course.id])}", + {"file_action": file_action.id}, + ) + assert json.loads(response.content.decode("utf-8")).get("success") is True + + # it should be removed from the course, but should still exist + assert not course.file_actions.filter(id=file_action.id).exists() + assert FileAction.objects.filter(id=file_action.id).exists() diff --git a/tin/apps/assignments/urls.py b/tin/apps/assignments/urls.py index 13924cff..9b8d3383 100644 --- a/tin/apps/assignments/urls.py +++ b/tin/apps/assignments/urls.py @@ -24,6 +24,21 @@ views.delete_file_view, name="delete_file", ), + path( + "/files/actions/choose", + views.choose_file_action, + name="choose_file_action", + ), + path( + "/files/actions/choose/new", + views.create_file_action, + name="create_file_action", + ), + path( + "/files/actions/delete/", + views.delete_file_action_view, + name="delete_file_action", + ), path( "/files/action/", views.file_action_view, diff --git a/tin/apps/assignments/views.py b/tin/apps/assignments/views.py index 526444d3..d5b657b1 100644 --- a/tin/apps/assignments/views.py +++ b/tin/apps/assignments/views.py @@ -16,6 +16,7 @@ from django.urls import reverse from django.utils.text import slugify from django.utils.timezone import now +from django.views.decorators.http import require_POST from ... import sandboxing from ..auth.decorators import login_required, teacher_or_superuser_required @@ -25,6 +26,8 @@ from ..users.models import User from .forms import ( AssignmentForm, + ChooseFileActionForm, + FileActionForm, FileSubmissionForm, FileUploadForm, FolderForm, @@ -32,7 +35,7 @@ MossForm, TextSubmissionForm, ) -from .models import Assignment, CooldownPeriod, QuizLogMessage +from .models import Assignment, CooldownPeriod, FileAction, QuizLogMessage from .tasks import run_moss logger = logging.getLogger(__name__) @@ -477,6 +480,99 @@ def file_action_view(request, assignment_id, action_id): return redirect("assignments:manage_files", assignment.id) +@teacher_or_superuser_required +def choose_file_action(request, course_id: int): + """Choose a file action template.""" + course = get_object_or_404( + Course.objects.filter_editable(request.user), + id=course_id, + ) + + if request.method == "POST": + form = ChooseFileActionForm(request.POST) + if form.is_valid(): + file_action = form.cleaned_data["file_action"] + file_action.courses.add(course) + return http.JsonResponse({"success": True}) + return http.JsonResponse({"success": False, "errors": form.errors.as_json()}, status=400) + + actions = FileAction.objects.exclude(courses=course) + course_actions = course.file_actions.all() + return render( + request, + "assignments/choose_file_action.html", + { + "actions": actions, + "course_actions": course_actions, + "course": course, + "nav_item": "Choose file action", + }, + ) + + +@teacher_or_superuser_required +def create_file_action(request, course_id: int): + """Creates or edits a :class:`.FileAction` + + If the ``GET`` request has a ``action`` parameter, + the view will action as an edit view. + + Args: + request: The request + course_id: The primary key of the :class:`.Course` + """ + course = get_object_or_404(Course.objects.filter_editable(request.user), id=course_id) + if (action_id := request.GET.get("action", "")).isdigit(): + action = get_object_or_404(course.file_actions, id=action_id) + else: + action = None + + if request.method == "POST": + form = FileActionForm(request.POST, instance=action) + if form.is_valid(): + action = form.save(commit=False) + if request.POST.get("copy"): + action.pk = None + action._state.adding = True + action.save() + action.courses.add(course) + return redirect("courses:show", course.id) + else: + form = FileActionForm(instance=action) + + return render( + request, + "assignments/custom_file_action.html", + { + "form": form, + "action": action, + "course": course, + "nav_item": "Create file action", + }, + ) + + +@teacher_or_superuser_required +@require_POST +def delete_file_action_view(request, course_id: int): + """Removes a :class:`.FileAction` from a :class:`.Course`. + + This does NOT permanently delete the :class:`.FileAction`. + + Args: + request: The request + course_id: The primary key of the :class:`.Course` + action_id: The primary key of the :class:`.FileAction` + """ + course = get_object_or_404(Course.objects.filter_editable(request.user), id=course_id) + form = ChooseFileActionForm(request.POST) + if form.is_valid(): + action = form.cleaned_data["file_action"] + action.courses.remove(course) + return http.JsonResponse({"success": True}) + return http.JsonResponse({"success": False, "errors": form.errors.as_json()}, status=400) + + @teacher_or_superuser_required def student_submissions_view(request, assignment_id, student_id): """See the submissions of a student diff --git a/tin/static/css/base.css b/tin/static/css/base.css index b7c2a7f9..2da323f8 100644 --- a/tin/static/css/base.css +++ b/tin/static/css/base.css @@ -231,6 +231,12 @@ a:hover { text-decoration: underline; } +a.link { + color: blue; + text-decoration: underline; + font-weight: normal; +} + .italic { font-style: italic; } @@ -243,6 +249,10 @@ a:hover { text-align: center; } +.hidden { + visibility: hidden; +} + .tin-btn { border: 1px solid #cfcfcf; @@ -251,6 +261,10 @@ a:hover { background: linear-gradient(to bottom, #dfdfdf 0%, #cfcfcf 100%); } +.fake-btn { + cursor: pointer; +} + .form-input > :is(select.selectized, input):not(input[type='checkbox']) { min-width: 250px; } @@ -331,6 +345,10 @@ code > pre { color: red; } +.red { + color: red; +} + h3.errors { margin-bottom: 0.5em; } diff --git a/tin/static/css/choose.css b/tin/static/css/choose.css new file mode 100644 index 00000000..8c5f122b --- /dev/null +++ b/tin/static/css/choose.css @@ -0,0 +1,99 @@ +* { + box-sizing: border-box; +} + +.multi-part-container { + display: flex; + justify-content: space-between; + align-items: center; + width: 100%; +} + +.row { + display: flex; + flex-wrap: wrap; + margin-top: 0; + margin-right: -0.5rem; + margin-left: -0.5rem; +} + +.row > * { + max-width: 100%; + padding-right: 0.75rem; + padding-left: 0.75rem; + margin-top: 0; +} + +.col { + width: 25%; + display: flex; + flex-direction: column; + flex: 0 0 auto; + padding-top: 10px; +} + +@media (max-width: 768px) { + .row { + flex-direction: column; + } + + .col { + width: 100%; + } +} + +.card { + position: relative; + display: flex; + flex-direction: column; + flex: 1; + word-wrap: break-word; + color: #212529; + background-color: #fff; + background-clip: border-box; + border: 1px solid rgba(0, 0, 0, 0.125); + border-radius: 0.25rem; + box-shadow: + 7px 7px 14px #e8e8ea, + -7px -7px 14px #fcfcfe; +} + +.card-content { + display: flex; + flex-direction: column; + justify-content: space-between; + flex: 1 1 auto; + padding: 1em 1em; + word-wrap: break-word; +} + +.card-topright { + position: absolute; + top: 0; + right: 0; + padding: 10px; +} + +.card-title { + font-size: 1.25rem; + margin-top: 0; + margin-bottom: 0.5rem; + line-height: 1.2; + font-weight: 500; +} + +.card-description { + margin-top: 0; + margin-bottom: 1rem; +} + +.card-btn { + align-self: flex-end; + margin-top: auto; + font-weight: normal; + cursor: pointer; +} + +.card-btn:hover { + text-decoration: underline; +} diff --git a/tin/static/css/login.css b/tin/static/css/login.css index f3101d07..14e83116 100644 --- a/tin/static/css/login.css +++ b/tin/static/css/login.css @@ -1,4 +1,4 @@ -.btn.btn-ion { +.btn-ion { text-decoration: none; color: #484848; display: inline-block; diff --git a/tin/templates/assignments/choose_file_action.html b/tin/templates/assignments/choose_file_action.html new file mode 100644 index 00000000..9cd80cf7 --- /dev/null +++ b/tin/templates/assignments/choose_file_action.html @@ -0,0 +1,121 @@ +{% extends "base.html" %} +{% load static %} + +{% block head %} + + +{% endblock %} + +{% block main %} +
+

Your File Actions

+ Create new file action +
+ + +
+ {% for action in course_actions %} +
+
+
+ +
{{ action.name }}
+

{{ action.description }}

+ + Edit File Action + +
+
+
+ {% endfor %} +
+ + +
+

Choose a File Action

+ +
+ +
+ {% for action in actions %} +
+
+
+ +
{{ action.name }}
+

{{ action.description }}

+ +
+
+
+ {% endfor %} +
+ +{% endblock %} diff --git a/tin/templates/assignments/custom_file_action.html b/tin/templates/assignments/custom_file_action.html new file mode 100644 index 00000000..1084caa4 --- /dev/null +++ b/tin/templates/assignments/custom_file_action.html @@ -0,0 +1,73 @@ +{% extends "base.html" %} + +{% block title %} + {% if not action %} + Create a File Action + {% else %} + Edit File Action: {{ action.name | title }} + {% endif %} +{% endblock %} + +{% block head %} + +{% endblock %} + +{% block main %} + {% if action %} +

Edit File Action: {{ action.name | title }}

+ {% else %} +

Create a File Action

+ {% endif %} + + {% if action and action.courses.count > 1 %} +

+ You are modifying a file action that is used in multiple courses. + Any changes you make will affect all courses that use this file action. +
+ Consider making a copy. +

+ {% endif %} + +

For an explanation of what file actions are, and how to use them, check out Tin's documentation

+ + {% if form.errors %} +

Please correct the following errors:

+
    + {% for field in form %} + {% for error in field.errors %} +
  • {{ field.label }}: {{ error }}
    • + {% endfor %} + {% endfor %} + {% for error in assignment_form.non_field_errors %} +
  • {{ error }}
    • + {% endfor %} +
+ {% endif %} + +
+ {% csrf_token %} +
+ {% for field in form %} +
+ {{ field.label_tag }} + {{ field }}{% if field.help_text %}
{{ field.help_text }}{% endif %} +
+ {% endfor %} +
+ + {% if request.GET.copy %} + + {% endif %} + + + {% if action %} +
+ Delete +
+ {% endif %} +
+{% endblock main %} diff --git a/tin/templates/assignments/manage_files.html b/tin/templates/assignments/manage_files.html index fecf3906..81d3b5e2 100644 --- a/tin/templates/assignments/manage_files.html +++ b/tin/templates/assignments/manage_files.html @@ -64,4 +64,6 @@

Last action output

{% endif %} {% endif %} + Explore File Actions + {% endblock %} diff --git a/tin/templates/base.html b/tin/templates/base.html index 7629a206..7ff31643 100644 --- a/tin/templates/base.html +++ b/tin/templates/base.html @@ -8,7 +8,10 @@ {% include "meta.html" %} - + +