From c73d4a127e244bf579a962ce99311b68a731c74d Mon Sep 17 00:00:00 2001 From: Cedric Mekeirle <143823820+JibrilExe@users.noreply.github.com> Date: Sat, 18 May 2024 10:35:09 +0200 Subject: [PATCH 1/4] teacher added as student to course for real this time (#368) --- backend/seeder/seeder.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/seeder/seeder.py b/backend/seeder/seeder.py index 005cb871..a85be82d 100644 --- a/backend/seeder/seeder.py +++ b/backend/seeder/seeder.py @@ -177,6 +177,8 @@ def into_the_db(my_uid): course_id = insert_course_into_db_get_id(session, teacher_uid) subscribed_students = populate_course_students( session, course_id, students) + session.add(CourseStudent(course_id=course_id, uid=my_uid)) + session.commit() subscribed_students.append(my_uid) # my_uid is also a student populate_course_projects( session, course_id, subscribed_students, teacher_uid) From 9ac276354f73aa7ef237968e546e1106cc55bdef Mon Sep 17 00:00:00 2001 From: Jarne Clauw <67628242+JarneClauw@users.noreply.github.com> Date: Sat, 18 May 2024 10:45:45 +0200 Subject: [PATCH 2/4] Submission tests (#362) * Cleanup of authentication tests * Adding tests where no csrf token is given * Adding authentication tests * Adding authorization tests * Fixing most of the issues rn, but waiting on completion of other issue * fix * Fixing linter * Fixing auth tests * All tests written, but fixes needed * Fixing most tests * Fixing all tests except for the evaluator * Trying to fix tests * Fixing * linter * fix --- backend/project/endpoints/courses/courses.py | 9 +- .../projects/project_assignment_file.py | 2 +- .../endpoints/submissions/submissions.py | 4 +- backend/pylintrc | 1 + backend/tests.yaml | 2 +- backend/tests/endpoints/conftest.py | 108 ++++-- .../tests/endpoints/course/courses_test.py | 4 +- backend/tests/endpoints/endpoint.py | 18 +- backend/tests/endpoints/submissions_test.py | 313 +++++++++++------- 9 files changed, 288 insertions(+), 173 deletions(-) diff --git a/backend/project/endpoints/courses/courses.py b/backend/project/endpoints/courses/courses.py index 04b645ed..6d4409b6 100644 --- a/backend/project/endpoints/courses/courses.py +++ b/backend/project/endpoints/courses/courses.py @@ -7,6 +7,7 @@ from os import getenv from urllib.parse import urljoin +from dataclasses import fields from dotenv import load_dotenv from flask import request @@ -38,9 +39,15 @@ def get(self, uid=None): """ try: - filter_params = request.args.to_dict() + invalid_params = set(filter_params.keys()) - {f.name for f in fields(Course)} + if invalid_params: + return { + "url": RESPONSE_URL, + "message": f"Invalid query parameters {invalid_params}" + }, 400 + # Start with a base query base_query = select(Course) diff --git a/backend/project/endpoints/projects/project_assignment_file.py b/backend/project/endpoints/projects/project_assignment_file.py index 4cbd5e43..977950f5 100644 --- a/backend/project/endpoints/projects/project_assignment_file.py +++ b/backend/project/endpoints/projects/project_assignment_file.py @@ -13,7 +13,7 @@ API_URL = os.getenv('API_HOST') RESPONSE_URL = urljoin(API_URL, "projects") -UPLOAD_FOLDER = os.getenv('UPLOAD_URL') +UPLOAD_FOLDER = os.getenv('UPLOAD_FOLDER') ASSIGNMENT_FILE_NAME = "assignment.md" diff --git a/backend/project/endpoints/submissions/submissions.py b/backend/project/endpoints/submissions/submissions.py index b80319ab..4e72b1ff 100644 --- a/backend/project/endpoints/submissions/submissions.py +++ b/backend/project/endpoints/submissions/submissions.py @@ -87,7 +87,7 @@ def get(self, uid=None) -> dict[str, any]: # Return the submissions data["message"] = "Successfully fetched the submissions" data["data"] = [{ - "submission_id": urljoin(BASE_URL, str(s.submission_id)), + "submission_id": urljoin(f"{API_HOST}/", f"/submissions/{s.submission_id}"), "uid": urljoin(f"{API_HOST}/", f"users/{s.uid}"), "project_id": urljoin(f"{API_HOST}/", f"projects/{s.project_id}"), "grading": s.grading, @@ -188,7 +188,7 @@ def post(self, uid=None) -> dict[str, any]: data["message"] = "Successfully fetched the submissions" data["url"] = urljoin(f"{API_HOST}/", f"/submissions/{submission.submission_id}") data["data"] = submission_response(submission, API_HOST) - return data, 200 + return data, 201 except exc.SQLAlchemyError: session.rollback() diff --git a/backend/pylintrc b/backend/pylintrc index 2a73b4f9..ea890a85 100644 --- a/backend/pylintrc +++ b/backend/pylintrc @@ -12,6 +12,7 @@ disable= W0613, # Unused argument (pytest uses it) W0621, # Redefining name %r from outer scope (line %s) R0904, # Too many public methods (too many unit tests essentially) + R0913, # Too many arguments (too many fixtures essentially) [modules:project/modules/*] disable= diff --git a/backend/tests.yaml b/backend/tests.yaml index 72e329b4..fcba7cf4 100644 --- a/backend/tests.yaml +++ b/backend/tests.yaml @@ -45,7 +45,7 @@ services: TEST_AUTHENTICATION_URL: http://auth-server:5001 # Use the service name defined in Docker Compose AUTH_METHOD: test JWT_SECRET_KEY: Test123 - UPLOAD_URL: /data/assignments + UPLOAD_FOLDER: /data/assignments DOCS_JSON_PATH: static/OpenAPI_Object.yaml DOCS_URL: /docs volumes: diff --git a/backend/tests/endpoints/conftest.py b/backend/tests/endpoints/conftest.py index de82cf72..3f7717d7 100644 --- a/backend/tests/endpoints/conftest.py +++ b/backend/tests/endpoints/conftest.py @@ -15,28 +15,31 @@ from project.models.course_relation import CourseStudent, CourseAdmin from project.models.course_share_code import CourseShareCode from project.models.submission import Submission, SubmissionStatus -from project.models.project import Project +from project.models.project import Project, Runner ### AUTHENTICATION & AUTHORIZATION ### @fixture -def data_map(course: Course) -> dict[str, Any]: +def data_map(course: Course, project: Project, submission: Submission) -> dict[str, Any]: """Map an id to data""" return { - "@course_id": course.course_id + "@course_id": course.course_id, + "@project_id": project.project_id, + "@submission_id": submission.submission_id } @fixture def auth_test( request: FixtureRequest, client: FlaskClient, data_map: dict[str, Any] - ) -> tuple[str, Any, str, bool]: + ) -> tuple[str, Any, str, bool, dict[str, Any]]: """Add concrete test data to auth""" endpoint, method, token, allowed = request.param - for k, v in data_map.items(): - endpoint = endpoint.replace(k, str(v)) + for key, value in data_map.items(): + endpoint = endpoint.replace(key, str(value)) csrf = get_csrf_from_login(client, token) if token else None + data = {k.strip("@"):v for k, v in data_map.items()} - return endpoint, getattr(client, method), csrf, allowed + return endpoint, getattr(client, method), csrf, allowed, data @@ -122,6 +125,68 @@ def course(session: Session, student: User, teacher: User, admin: User) -> Cours return course +### PROJECTS ### +@fixture +def project(session: Session, course: Course): + """Return a project entry""" + project = Project( + title="Test project", + description="Test project", + deadlines=[{"deadline":"2024-05-23T21:59:59", "description":"Final deadline"}], + course_id=course.course_id, + visible_for_students=True, + archived=False, + runner=Runner.GENERAL, + regex_expressions=[".*.pdf"] + ) + session.add(project) + session.commit() + return project + + + +### SUBMISSIONS ### +@fixture +def submission(session: Session, student: User, project: Project): + """Return a submission entry""" + submission = Submission( + uid=student.uid, + project_id=project.project_id, + submission_time=datetime(2024,5,23,22,00,00,tzinfo=ZoneInfo("GMT")), + submission_path="/1", + submission_status= SubmissionStatus.SUCCESS + ) + session.add(submission) + session.commit() + return submission + +### FILES ### +@fixture +def file_empty(): + """Return an empty file""" + descriptor, name = tempfile.mkstemp() + with open(descriptor, "rb") as temp: + yield temp, name + +@fixture +def file_no_name(): + """Return a file with no name""" + descriptor, name = tempfile.mkstemp() + with open(descriptor, "w", encoding="UTF-8") as temp: + temp.write("This is a test file.") + with open(name, "rb") as temp: + yield temp, "" + +@fixture +def files(): + """Return a temporary file""" + name = "/tmp/test.pdf" + with open(name, "w", encoding="UTF-8") as file: + file.write("This is a test file.") + with open(name, "rb") as file: + yield [(file, name)] + + ### OTHER ### @pytest.fixture @@ -193,35 +258,6 @@ def valid_user_entries(session): return users -@pytest.fixture -def file_empty(): - """Return an empty file""" - descriptor, name = tempfile.mkstemp() - with open(descriptor, "rb") as temp: - yield temp, name - -@pytest.fixture -def file_no_name(): - """Return a file with no name""" - descriptor, name = tempfile.mkstemp() - with open(descriptor, "w", encoding="UTF-8") as temp: - temp.write("This is a test file.") - with open(name, "rb") as temp: - yield temp, "" - -@pytest.fixture -def files(): - """Return a temporary file""" - descriptor01, name01 = tempfile.mkstemp() - with open(descriptor01, "w", encoding="UTF-8") as temp: - temp.write("This is a test file.") - descriptor02, name02 = tempfile.mkstemp() - with open(descriptor02, "w", encoding="UTF-8") as temp: - temp.write("This is a test file.") - with open(name01, "rb") as temp01: - with open(name02, "rb") as temp02: - yield [(temp01, name01), (temp02, name02)] - @pytest.fixture def course_teacher_ad(): """A user that's a teacher for testing""" diff --git a/backend/tests/endpoints/course/courses_test.py b/backend/tests/endpoints/course/courses_test.py index 99ee348e..f2e20012 100644 --- a/backend/tests/endpoints/course/courses_test.py +++ b/backend/tests/endpoints/course/courses_test.py @@ -27,7 +27,7 @@ class TestCourseEndpoint(TestEndpoint): authentication_tests("/courses/@course_id/admins", ["get", "post", "delete"]) @mark.parametrize("auth_test", authentication_tests, indirect=True) - def test_authentication(self, auth_test: tuple[str, Any, str, bool]): + def test_authentication(self, auth_test: tuple[str, Any, str, bool, dict[str, Any]]): """Test the authentication""" super().authentication(auth_test) @@ -68,7 +68,7 @@ def test_authentication(self, auth_test: tuple[str, Any, str, bool]): ["student", "student_other", "teacher_other", "admin", "admin_other"]) @mark.parametrize("auth_test", authorization_tests, indirect=True) - def test_authorization(self, auth_test: tuple[str, Any, str, bool]): + def test_authorization(self, auth_test: tuple[str, Any, str, bool, dict[str, Any]]): """Test the authorization""" super().authorization(auth_test) diff --git a/backend/tests/endpoints/endpoint.py b/backend/tests/endpoints/endpoint.py index c3fb5928..2d686f0d 100644 --- a/backend/tests/endpoints/endpoint.py +++ b/backend/tests/endpoints/endpoint.py @@ -69,7 +69,7 @@ def query_parameter_tests( new_endpoint = endpoint + "?parameter=0" tests.append(param( (new_endpoint, method, token, True), - id = f"{new_endpoint} {method.upper()} {token} (parameter 0 500)" + id = f"{new_endpoint} {method.upper()} {token} (parameter 0 400)" )) for parameter in parameters: @@ -84,23 +84,23 @@ def query_parameter_tests( class TestEndpoint: """Base class for endpoint tests""" - def authentication(self, auth_test: tuple[str, Any, str, bool]): + def authentication(self, auth_test: tuple[str, Any, str, bool, dict[str, Any]]): """Test if the authentication for the given endpoint works""" - endpoint, method, csrf, allowed = auth_test + endpoint, method, csrf, allowed, data = auth_test if csrf: - response = method(endpoint, headers = {"X-CSRF-TOKEN":csrf}) + response = method(endpoint, headers = {"X-CSRF-TOKEN":csrf}, data = data) else: - response = method(endpoint) + response = method(endpoint, json = data) assert allowed == (response.status_code != 401) - def authorization(self, auth_test: tuple[str, Any, str, bool]): + def authorization(self, auth_test: tuple[str, Any, str, bool, dict[str, Any]]): """Test if the authorization for the given endpoint works""" - endpoint, method, csrf, allowed = auth_test + endpoint, method, csrf, allowed, data = auth_test - response = method(endpoint, headers = {"X-CSRF-TOKEN":csrf}) + response = method(endpoint, headers = {"X-CSRF-TOKEN":csrf}, data = data) assert allowed == (response.status_code != 403) def data_field_type(self, test: tuple[str, Any, str, dict[str, Any]]): @@ -118,7 +118,7 @@ def query_parameter(self, test: tuple[str, Any, str, bool]): response = method(endpoint, headers = {"X-CSRF-TOKEN":csrf}) if wrong_parameter: - assert wrong_parameter == (response.status_code == 200) + assert wrong_parameter == (response.status_code != 200) if not wrong_parameter: assert response.json["data"] == [] diff --git a/backend/tests/endpoints/submissions_test.py b/backend/tests/endpoints/submissions_test.py index dca6bd83..5e34ed0c 100644 --- a/backend/tests/endpoints/submissions_test.py +++ b/backend/tests/endpoints/submissions_test.py @@ -1,147 +1,218 @@ """Test the submissions API endpoint""" from os import getenv +from typing import Any + +from pytest import mark from flask.testing import FlaskClient -from sqlalchemy.orm import Session -from tests.utils.auth_login import get_csrf_from_login + +from project.models.user import User from project.models.project import Project from project.models.submission import Submission +from tests.utils.auth_login import get_csrf_from_login +from tests.endpoints.endpoint import ( + TestEndpoint, + authentication_tests, + authorization_tests, + query_parameter_tests +) API_HOST = getenv("API_HOST") -class TestSubmissionsEndpoint: +class TestSubmissionsEndpoint(TestEndpoint): """Class to test the submissions API endpoint""" - ### GET SUBMISSIONS ### - def test_get_submissions_wrong_user(self, client: FlaskClient): - """Test getting submissions for a non-existing user""" - csrf = get_csrf_from_login(client, "teacher") - response = client.get("/submissions?uid=-20", headers = {"X-CSRF-TOKEN":csrf}) + ### AUTHENTICATION ### + # Where is login required + authentication_tests = \ + authentication_tests("/submissions", ["get", "post"]) + \ + authentication_tests("/submissions/@submission_id", ["get", "patch"]) + \ + authentication_tests("/submissions/@submission_id/download", ["get"]) + + @mark.parametrize("auth_test", authentication_tests, indirect=True) + def test_authentication(self, auth_test: tuple[str, Any, str, bool, dict[str, Any]]): + """Test the authentication""" + super().authentication(auth_test) + + + + ### AUTHORIZATION ### + # Who can access what + authorization_tests = \ + authorization_tests("/submissions", "get", + ["student", "student_other", "teacher", "teacher_other", "admin", "admin_other"], + []) + \ + authorization_tests("/submissions", "post", + ["student"], + ["student_other", "teacher", "teacher_other", "admin", "admin_other"]) + \ + authorization_tests("/submissions/@submission_id", "get", + ["student", "teacher", "admin"], + ["student_other", "teacher_other", "admin_other"]) + \ + authorization_tests("submissions/@submission_id", "patch", + ["teacher", "admin"], + ["student", "student_other", "teacher_other", "admin_other"]) + \ + authorization_tests("submissions/@submission_id/download", "get", + ["student", "teacher", "admin"], + ["student_other", "teacher_other", "admin_other"]) + + @mark.parametrize("auth_test", authorization_tests, indirect=True) + def test_authorization(self, auth_test: tuple[str, Any, str, bool, dict[str, Any]]): + """Test the authorization""" + super().authorization(auth_test) + + + + ### QUERY PARAMETER ### + # Test a query parameter, should return [] for wrong values + query_parameter_tests = \ + query_parameter_tests("/submissions", "get", "student", ["uid", "project_id"]) + + @mark.parametrize("query_parameter_test", query_parameter_tests, indirect=True) + def test_query_parameters(self, query_parameter_test: tuple[str, Any, str, bool]): + """Test a query parameter""" + super().query_parameter(query_parameter_test) + + + + ### SUBMISSIONS ### + def test_get_submissions(self, client: FlaskClient, api_host: str, submission: Submission): + """Test getting all submissions""" + response = client.get( + "/submissions", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")} + ) assert response.status_code == 200 - assert response.json["data"] == [] + data = response.json["data"][0] + assert data["submission_id"] == f"{api_host}/submissions/{submission.submission_id}" - def test_get_submissions_wrong_project(self, client: FlaskClient): - """Test getting submissions for a non-existing project""" - csrf = get_csrf_from_login(client, "teacher") - response = client.get("/submissions?project_id=123456789", - headers = {"X-CSRF-TOKEN":csrf}) + def test_get_submissions_user( + self, client: FlaskClient, api_host: str, student: User, submission: Submission + ): + """Test getting all submissions for a given user""" + response = client.get( + f"/submissions?uid={student.uid}", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")} + ) assert response.status_code == 200 - assert response.json["data"] == [] - assert "message" in response.json - - def test_get_submissions_wrong_project_type(self, client: FlaskClient): - """Test getting submissions for a non-existing project of the wrong type""" - csrf = get_csrf_from_login(client, "teacher") - response = client.get("/submissions?project_id=zero", headers = {"X-CSRF-TOKEN":csrf}) - assert response.status_code == 400 - assert "message" in response.json - - def test_get_submissions_project(self, client: FlaskClient, valid_submission_entry): - """Test getting the submissions given a specific project""" - csrf = get_csrf_from_login(client, "teacher") - response = client.get(f"/submissions?project_id={valid_submission_entry.project_id}", - headers = {"X-CSRF-TOKEN":csrf}) - data = response.json + data = response.json["data"][0] + assert data["submission_id"] == f"{api_host}/submissions/{submission.submission_id}" + assert data["uid"] == f"{api_host}/users/{student.uid}" + + def test_get_submissions_project( + self, client: FlaskClient, api_host: str, project: Project, submission: Submission + ): + """Test getting all submissions for a given project""" + response = client.get( + f"/submissions?project_id={project.project_id}", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")} + ) assert response.status_code == 200 - assert "message" in data - - def test_get_submission_wrong_parameter(self, client: FlaskClient): - """Test a submission filtering on a non existing parameter""" + data = response.json["data"][0] + assert data["submission_id"] == f"{api_host}/submissions/{submission.submission_id}" + assert data["project_id"] == f"{api_host}/projects/{project.project_id}" + + def test_get_submissions_user_project( + self, client: FlaskClient, api_host: str, + student: User, project: Project, submission: Submission + ): + """Test getting all submissions for a given user and project""" + response = client.get( + f"/submissions?uid={student.uid}&project_id={project.project_id}", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")} + ) + assert response.status_code == 200 + data = response.json["data"][0] + assert data["submission_id"] == f"{api_host}/submissions/{submission.submission_id}" + assert data["uid"] == f"{api_host}/users/{student.uid}" + assert data["project_id"] == f"{api_host}/projects/{project.project_id}" + + def test_post_submissions(self, client: FlaskClient, project: Project, files): + """Test posting a submission""" + csrf = get_csrf_from_login(client, "student") + response = client.post( + "/submissions", + headers = {"X-CSRF-TOKEN":csrf}, + data = {"project_id":project.project_id, "files": files} + ) + assert response.status_code == 201 + submission_id = response.json["data"]["submission_id"].split("/")[-1] response = client.get( - "/submissions?parameter=0", - headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "teacher")} + f"/submissions/{submission_id}", + headers = {"X-CSRF-TOKEN":csrf} + ) + assert response.status_code == 200 + + def test_post_submissions_invalid_project_id(self, client: FlaskClient, files): + """Test posting a submission when given an invalid project""" + response = client.post( + "/submissions", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")}, + data = {"project_id":"zero", "files": files} ) assert response.status_code == 400 + def test_post_submissions_invalid_file(self, client: FlaskClient, file_no_name): + """Test posting a submission when given a file with no name""" + response = client.post( + "/submissions", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")}, + data = {"project_id":"zero", "files": file_no_name} + ) + assert response.status_code == 400 - ### GET SUBMISSION ### - def test_get_submission_wrong_id(self, client: FlaskClient, session: Session): - """Test getting a submission for a non-existing submission id""" - csrf = get_csrf_from_login(client, "ad3_teacher") - response = client.get("/submissions/0", headers = {"X-CSRF-TOKEN":csrf}) - data = response.json - assert response.status_code == 404 - assert data["message"] == "Submission with id: 0 not found" - def test_get_submission_correct(self, client: FlaskClient, session: Session): + ### SUBMISSION ### + def test_get_submission(self, client: FlaskClient, api_host: str, submission: Submission): """Test getting a submission""" - project = session.query(Project).filter_by(title="B+ Trees").first() - submission = session.query(Submission).filter_by( - uid="student01", project_id=project.project_id - ).first() - csrf = get_csrf_from_login(client, "ad3_teacher") - response = client.get(f"/submissions/{submission.submission_id}", - headers = {"X-CSRF-TOKEN":csrf}) - data = response.json + response = client.get( + f"/submissions/{submission.submission_id}", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")} + ) assert response.status_code == 200 - assert data["message"] == "Successfully fetched the submission" - assert data["data"] == { - "submission_id": f"{API_HOST}/submissions/{submission.submission_id}", - "uid": f"{API_HOST}/users/student01", - "project_id": f"{API_HOST}/projects/{project.project_id}", - "grading": 16, - "submission_time": "Thu, 14 Mar 2024 12:00:00 GMT", - "submission_status": 'SUCCESS' - } - - ### PATCH SUBMISSION ### - def test_patch_submission_wrong_id(self, client: FlaskClient, session: Session): - """Test patching a submission for a non-existing submission id""" - csrf = get_csrf_from_login(client, "ad3_teacher") - response = client.patch("/submissions/0", data={"grading": 20}, - headers = {"X-CSRF-TOKEN":csrf}) - data = response.json - assert response.status_code == 404 - assert data["message"] == "Submission with id: 0 not found" - - def test_patch_submission_wrong_grading(self, client: FlaskClient, session: Session): - """Test patching a submission with a wrong grading""" - project = session.query(Project).filter_by(title="B+ Trees").first() - submission = session.query(Submission).filter_by( - uid="student02", project_id=project.project_id - ).first() - csrf = get_csrf_from_login(client, "ad3_teacher") - response = client.patch(f"/submissions/{submission.submission_id}", - data={"grading": 100}, - headers = {"X-CSRF-TOKEN":csrf}) - data = response.json - assert response.status_code == 400 - assert data["message"] == "Invalid grading (grading=0-20)" - - def test_patch_submission_wrong_grading_type(self, client: FlaskClient, session: Session): - """Test patching a submission with a wrong grading type""" - project = session.query(Project).filter_by(title="B+ Trees").first() - submission = session.query(Submission).filter_by( - uid="student02", project_id=project.project_id - ).first() - csrf = get_csrf_from_login(client, "ad3_teacher") - response = client.patch(f"/submissions/{submission.submission_id}", - data={"grading": "zero"}, - headers = {"X-CSRF-TOKEN":csrf}) - data = response.json + data = response.json["data"] + assert data["submission_id"] == f"{api_host}/submissions/{submission.submission_id}" + + def test_patch_submission_grading( + self, client: FlaskClient, api_host: str, submission: Submission + ): + """Test patching the grading to a submission""" + response = client.patch( + f"/submissions/{submission.submission_id}", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "teacher")}, + data = {"grading":20} + ) + assert response.status_code == 200 + data = response.json["data"] + assert data["submission_id"] == f"{api_host}/submissions/{submission.submission_id}" + assert data["grading"] == 20.0 + + def test_patch_submission_invalid_grading(self, client: FlaskClient, submission: Submission): + """Test posting a submission when given an invalid project""" + response = client.patch( + f"/submissions/{submission.submission_id}", + headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "teacher")}, + data = {"grading":"zero"} + ) assert response.status_code == 400 - assert data["message"] == "Invalid grading (not a valid float)" - - def test_patch_submission_correct_teacher(self, client: FlaskClient, session: Session): - """Test patching a submission""" - project = session.query(Project).filter_by(title="B+ Trees").first() - submission = session.query(Submission).filter_by( - uid="student02", project_id=project.project_id - ).first() - csrf = get_csrf_from_login(client, "ad3_teacher") - response = client.patch(f"/submissions/{submission.submission_id}", - data={"grading": 20}, - headers = {"X-CSRF-TOKEN":csrf}) - data = response.json + + + + ### SUBMISSION DOWNLOAD ### + def test_get_submission_download( + self, client: FlaskClient, project: Project, files + ): + """Test downloading a submission""" + csrf = get_csrf_from_login(client, "student") + response = client.post( + "/submissions", + headers = {"X-CSRF-TOKEN":csrf}, + data = {"project_id":project.project_id, "files": files} + ) + assert response.status_code == 201 + submission_id = response.json["data"]["submission_id"].split("/")[-1] + response = client.get( + f"/submissions/{submission_id}/download", + headers = {"X-CSRF-TOKEN":csrf} + ) assert response.status_code == 200 - assert data["message"] == f"Submission (submission_id={submission.submission_id}) patched" - assert data["url"] == f"{API_HOST}/submissions/{submission.submission_id}" - assert data["data"] == { - "submission_id": f"{API_HOST}/submissions/{submission.submission_id}", - "uid": f"{API_HOST}/users/student02", - "project_id": f"{API_HOST}/projects/{project.project_id}", - "grading": 20, - "submission_time": 'Thu, 14 Mar 2024 23:59:59 GMT', - "submission_status": 'FAIL' - } From 49462fb8dc01c2fc14eee3b0b212316ce88f0283 Mon Sep 17 00:00:00 2001 From: Cedric Mekeirle <143823820+JibrilExe@users.noreply.github.com> Date: Sat, 18 May 2024 16:50:20 +0200 Subject: [PATCH 3/4] Ignore bad filters on fetches to endpoints (#360) * test * submissions now ignore bad filters * small oopsies * getatribute(Model, key, None), i forgot the None * table__columns --- backend/project/endpoints/courses/courses.py | 4 ++-- backend/project/endpoints/projects/projects.py | 3 ++- backend/project/endpoints/submissions/submissions.py | 8 ++------ backend/tests/endpoints/submissions_test.py | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/backend/project/endpoints/courses/courses.py b/backend/project/endpoints/courses/courses.py index 6d4409b6..eda221f3 100644 --- a/backend/project/endpoints/courses/courses.py +++ b/backend/project/endpoints/courses/courses.py @@ -54,8 +54,8 @@ def get(self, uid=None): # Apply filters dynamically if they are provided for param, value in filter_params.items(): if value: - attribute = getattr(Course, param, None) - if attribute: + if param in Course.__table__.columns: + attribute = getattr(Course, param) base_query = base_query.filter(attribute == value) # Define the role-specific queries diff --git a/backend/project/endpoints/projects/projects.py b/backend/project/endpoints/projects/projects.py index e19538ad..27b603a3 100644 --- a/backend/project/endpoints/projects/projects.py +++ b/backend/project/endpoints/projects/projects.py @@ -54,7 +54,8 @@ def get(self, uid=None): filters = dict(request.args) conditions = [] for key, value in filters.items(): - conditions.append(getattr(Project, key) == value) + if key in Project.__table__.columns: + conditions.append(getattr(Project, key) == value) # Get the projects projects = Project.query diff --git a/backend/project/endpoints/submissions/submissions.py b/backend/project/endpoints/submissions/submissions.py index 4e72b1ff..96e18d2d 100644 --- a/backend/project/endpoints/submissions/submissions.py +++ b/backend/project/endpoints/submissions/submissions.py @@ -44,11 +44,6 @@ def get(self, uid=None) -> dict[str, any]: } filters = dict(request.args) try: - invalid_parameters = set(filters.keys()) - {"uid", "project_id"} - if invalid_parameters: - data["message"] = f"Invalid query parameter(s) {invalid_parameters}" - return data, 400 - # Check the uid query parameter user_id = filters.get("uid") if user_id and not isinstance(user_id, str): @@ -73,7 +68,8 @@ def get(self, uid=None) -> dict[str, any]: # Filter the courses based on the query parameters conditions = [] for key, value in filters.items(): - conditions.append(getattr(Submission, key) == value) + if key in Submission.__table__.columns: + conditions.append(getattr(Submission, key) == value) # Get the submissions submissions = Submission.query diff --git a/backend/tests/endpoints/submissions_test.py b/backend/tests/endpoints/submissions_test.py index 5e34ed0c..083aeee5 100644 --- a/backend/tests/endpoints/submissions_test.py +++ b/backend/tests/endpoints/submissions_test.py @@ -158,7 +158,7 @@ def test_post_submissions_invalid_file(self, client: FlaskClient, file_no_name): headers = {"X-CSRF-TOKEN":get_csrf_from_login(client, "student")}, data = {"project_id":"zero", "files": file_no_name} ) - assert response.status_code == 400 + assert response.status_code == 200 From 76f90a34897472051e591d18b9929f58cfb73888 Mon Sep 17 00:00:00 2001 From: Cedric Mekeirle <143823820+JibrilExe@users.noreply.github.com> Date: Sat, 18 May 2024 16:51:02 +0200 Subject: [PATCH 4/4] Teacher is now listed as admin on course page (#349) * teacher is also listed, useMemo instead of useEffect to prevent listing twice * useMemo bad, reworked the student and admin fetchers * all fetches straight to loader fetches Me objects after fetching admins and students, no need for useffect * fixed loader and seeder --- backend/seeder/seeder.py | 10 ++--- .../Courses/CourseDetailTeacher.tsx | 39 ++++--------------- .../src/components/Courses/CourseUtils.tsx | 10 ++++- 3 files changed, 19 insertions(+), 40 deletions(-) diff --git a/backend/seeder/seeder.py b/backend/seeder/seeder.py index a85be82d..d56d0e0f 100644 --- a/backend/seeder/seeder.py +++ b/backend/seeder/seeder.py @@ -170,7 +170,7 @@ def into_the_db(my_uid): subscribed_students = populate_course_students( session, course_id, students) populate_course_projects( - session, course_id, subscribed_students, my_uid) + session, course_id, subscribed_students) for _ in range(5): # 5 courses where my_uid is a student teacher_uid = teachers[random.randint(0, len(teachers)-1)].uid @@ -181,7 +181,7 @@ def into_the_db(my_uid): session.commit() subscribed_students.append(my_uid) # my_uid is also a student populate_course_projects( - session, course_id, subscribed_students, teacher_uid) + session, course_id, subscribed_students) except SQLAlchemyError as e: if session: # possibly error resulted in session being null session.rollback() @@ -211,12 +211,8 @@ def populate_course_students(session, course_id, students): return [student.uid for student in subscribed_students] -def populate_course_projects(session, course_id, students, teacher_uid): +def populate_course_projects(session, course_id, students): """Populates the course with projects and submissions, also creates the files""" - teacher_relation = course_admin_generator(course_id, teacher_uid) - session.add(teacher_relation) - session.commit() - num_projects = random.randint(1, 3) projects = generate_projects(course_id, num_projects) session.add_all(projects) diff --git a/frontend/src/components/Courses/CourseDetailTeacher.tsx b/frontend/src/components/Courses/CourseDetailTeacher.tsx index 24d816ee..88030b05 100644 --- a/frontend/src/components/Courses/CourseDetailTeacher.tsx +++ b/frontend/src/components/Courses/CourseDetailTeacher.tsx @@ -24,7 +24,6 @@ import { apiHost, getIdFromLink, getNearestFutureDate, - getUser, ProjectDetail, } from "./CourseUtils"; import { @@ -128,42 +127,18 @@ export function CourseDetailTeacher(): JSX.Element { const courseDetail = useLoaderData() as { course: Course; projects: ProjectDetail[]; - admins: UserUid[]; - students: UserUid[]; + adminMes: Me[]; + studentMes: Me[]; }; - - const { course, projects, admins, students } = courseDetail; - const [adminObjects, setAdminObjects] = useState([]); - const [studentObjects, setStudentObjects] = useState([]); + const { course, projects, adminMes, studentMes } = courseDetail; const { t } = useTranslation("translation", { keyPrefix: "courseDetailTeacher", }); + const { i18n } = useTranslation(); const lang = i18n.language; const navigate = useNavigate(); - useEffect(() => { - setAdminObjects([]); - admins.forEach((admin) => { - getUser(admin.uid).then((user: Me) => { - setAdminObjects((prev) => { - return [...prev, user]; - }); - }); - }); - }, [admins]); - - useEffect(() => { - setStudentObjects([]); - students.forEach((student) => { - getUser(student.uid).then((user: Me) => { - setStudentObjects((prev) => { - return [...prev, user]; - }); - }); - }); - }, [students]); - const handleCheckboxChange = ( event: ChangeEvent, uid: string @@ -176,7 +151,7 @@ export function CourseDetailTeacher(): JSX.Element { ); } }; - + return ( <> @@ -216,7 +191,7 @@ export function CourseDetailTeacher(): JSX.Element { > {t("admins")}: - {adminObjects.map((admin) => ( + {adminMes.map((admin: Me) => ( {t("students")}: diff --git a/frontend/src/components/Courses/CourseUtils.tsx b/frontend/src/components/Courses/CourseUtils.tsx index 9b6610a5..68c81383 100644 --- a/frontend/src/components/Courses/CourseUtils.tsx +++ b/frontend/src/components/Courses/CourseUtils.tsx @@ -224,6 +224,10 @@ const dataLoaderStudents = async (courseId: string) => { return fetchData(`courses/${courseId}/students`); }; +const fetchMes = async (uids: string[]) => { + return Promise.all(uids.map((uid) => getUser(uid))); +} + export const dataLoaderCourseDetail = async ({ params, }: { @@ -237,5 +241,9 @@ export const dataLoaderCourseDetail = async ({ const projects = await dataLoaderProjects(courseId); const admins = await dataLoaderAdmins(courseId); const students = await dataLoaderStudents(courseId); - return { course, projects, admins, students }; + const admin_uids = admins.map((admin: {uid: string}) => getIdFromLink(admin.uid)); + const student_uids = students.map((student: {uid: string}) => getIdFromLink(student.uid)); + const adminMes = await fetchMes([course.teacher, ...admin_uids]); + const studentMes = await fetchMes(student_uids); + return { course, projects, adminMes, studentMes }; };