From 582d81b14a2a5b97f50cac30165dcf850d8f3354 Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Thu, 3 Oct 2024 13:12:03 +0200 Subject: [PATCH] New JSON renderer that handles datetime objects Based on the default json serializer from pyramid but it formats datetime objects as isoformat(). It special cases datetimes without datetime information and assumes they are in UTC. This commit introduces this seralizer as a new one on top of the default `json` as `json_iso_utc` an starts using it on the assignments API. Short term we'll use this serializer in all dashboard related routes and longer term we'll replace all uses of json by this seralizer and then will make it the default. --- lms/app.py | 4 +++ lms/renderers.py | 23 +++++++++++++++ lms/views/dashboard/api/assignment.py | 12 ++++---- tests/unit/lms/renderers_test.py | 28 +++++++++++++++++++ .../views/dashboard/api/assignment_test.py | 14 +++++----- 5 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 lms/renderers.py create mode 100644 tests/unit/lms/renderers_test.py diff --git a/lms/app.py b/lms/app.py index f0f5197005..de03abd477 100644 --- a/lms/app.py +++ b/lms/app.py @@ -3,6 +3,7 @@ from sqlalchemy.exc import IntegrityError from lms.config import configure +from lms.renderers import json_iso_utc def configure_jinja2_assets(config): @@ -35,6 +36,9 @@ def create_app(global_config, **settings): # noqa: ARG001 config.include("pyramid_jinja2") config.include("pyramid_services") + # Add our own json renderer that handles datetime objects. + config.add_renderer("json_iso_utc", json_iso_utc()) + # Use pyramid_tm's explicit transaction manager. # # This means that trying to access a request's transaction after pyramid_tm diff --git a/lms/renderers.py b/lms/renderers.py new file mode 100644 index 0000000000..ff3ef3f8b8 --- /dev/null +++ b/lms/renderers.py @@ -0,0 +1,23 @@ +from datetime import UTC, datetime + +from pyramid.renderers import JSON + + +def json_iso_utc(): + """Return a JSON renderer that formats dates as `isoformat`. + + This renderer assumes datetimes without tz info are in UTC and + includes that in the datetime objects so the resulting string + includes tz information. + """ + + renderer = JSON() + + def _datetime_adapter(obj: datetime, _request) -> str: + if not obj.tzinfo: + obj = obj.replace(tzinfo=UTC) + return obj.isoformat() + + renderer.add_adapter(datetime, _datetime_adapter) + + return renderer diff --git a/lms/views/dashboard/api/assignment.py b/lms/views/dashboard/api/assignment.py index 73c70f9e1f..4f1fd1269d 100644 --- a/lms/views/dashboard/api/assignment.py +++ b/lms/views/dashboard/api/assignment.py @@ -59,7 +59,7 @@ def __init__(self, request) -> None: @view_config( route_name="api.dashboard.assignments", request_method="GET", - renderer="json", + renderer="json_iso_utc", permission=Permissions.DASHBOARD_VIEW, schema=ListAssignmentsSchema, ) @@ -85,7 +85,7 @@ def assignments(self) -> APIAssignments: APIAssignment( id=assignment.id, title=assignment.title, - created=assignment.created.isoformat(), + created=assignment.created, ) for assignment in assignments ], @@ -95,7 +95,7 @@ def assignments(self) -> APIAssignments: @view_config( route_name="api.dashboard.assignment", request_method="GET", - renderer="json", + renderer="json_iso_utc", permission=Permissions.DASHBOARD_VIEW, ) def assignment(self) -> APIAssignment: @@ -103,7 +103,7 @@ def assignment(self) -> APIAssignment: api_assignment = APIAssignment( id=assignment.id, title=assignment.title, - created=assignment.created.isoformat(), + created=assignment.created, course=APICourse( id=assignment.course.id, title=assignment.course.lms_name, @@ -128,7 +128,7 @@ def assignment(self) -> APIAssignment: @view_config( route_name="api.dashboard.course.assignments.metrics", request_method="GET", - renderer="json", + renderer="json_iso_utc", permission=Permissions.DASHBOARD_VIEW, schema=AssignmentsMetricsSchema, ) @@ -191,7 +191,7 @@ def course_assignments_metrics(self) -> APIAssignments: APIAssignment( id=assignment.id, title=assignment.title, - created=assignment.created.isoformat(), + created=assignment.created, course=api_course, annotation_metrics=metrics, ) diff --git a/tests/unit/lms/renderers_test.py b/tests/unit/lms/renderers_test.py new file mode 100644 index 0000000000..ec8beb7a01 --- /dev/null +++ b/tests/unit/lms/renderers_test.py @@ -0,0 +1,28 @@ +from datetime import UTC, datetime +from unittest.mock import sentinel +from zoneinfo import ZoneInfo + +import pytest + +from lms.renderers import json_iso_utc + + +@pytest.mark.parametrize( + "time,expected", + [ + # No timezone, UTC is assumed + (datetime(2024, 1, 1), "2024-01-01T00:00:00+00:00"), + # UTC, UTC is left intact + (datetime(2024, 1, 1, tzinfo=UTC), "2024-01-01T00:00:00+00:00"), + # Non-UTC, timezone is also left intact + ( + datetime(2024, 1, 1, tzinfo=ZoneInfo("Europe/Madrid")), + "2024-01-01T00:00:00+01:00", + ), + ], +) +def test_json_iso_utc(time, expected): + assert ( + json_iso_utc()(sentinel.info)({"time": time}, {}) + == f"""{{"time": "{expected}"}}""" + ) diff --git a/tests/unit/lms/views/dashboard/api/assignment_test.py b/tests/unit/lms/views/dashboard/api/assignment_test.py index 790087c467..ac4fa95c4b 100644 --- a/tests/unit/lms/views/dashboard/api/assignment_test.py +++ b/tests/unit/lms/views/dashboard/api/assignment_test.py @@ -39,7 +39,7 @@ def test_get_assignments( ) assert response == { "assignments": [ - {"id": a.id, "title": a.title, "created": a.created.isoformat()} + {"id": a.id, "title": a.title, "created": a.created} for a in assignments ], "pagination": sentinel.pagination, @@ -69,7 +69,7 @@ def test_assignment( assert response == { "id": assignment.id, "title": assignment.title, - "created": assignment.created.isoformat(), + "created": assignment.created, "course": {"id": assignment.course.id, "title": assignment.course.lms_name}, } @@ -96,7 +96,7 @@ def test_assignment_with_auto_grading( assert response == { "id": assignment.id, "title": assignment.title, - "created": assignment.created.isoformat(), + "created": assignment.created, "course": {"id": assignment.course.id, "title": assignment.course.lms_name}, "groups": [], "auto_grading_config": { @@ -125,7 +125,7 @@ def test_assignment_with_groups( assert response == { "id": assignment.id, "title": assignment.title, - "created": assignment.created.isoformat(), + "created": assignment.created, "course": {"id": assignment.course.id, "title": assignment.course.lms_name}, "groups": [ {"h_authority_provided_id": g.authority_provided_id, "name": g.lms_name} @@ -152,7 +152,7 @@ def test_assignment_with_sections( assert response == { "id": assignment.id, "title": assignment.title, - "created": assignment.created.isoformat(), + "created": assignment.created, "course": {"id": assignment.course.id, "title": assignment.course.lms_name}, "sections": [ {"h_authority_provided_id": g.authority_provided_id, "name": g.lms_name} @@ -210,7 +210,7 @@ def test_course_assignments( { "id": assignment.id, "title": assignment.title, - "created": assignment.created.isoformat(), + "created": assignment.created, "course": { "id": course.id, "title": course.lms_name, @@ -224,7 +224,7 @@ def test_course_assignments( { "id": assignment_with_no_annos.id, "title": assignment_with_no_annos.title, - "created": assignment_with_no_annos.created.isoformat(), + "created": assignment_with_no_annos.created, "course": { "id": course.id, "title": course.lms_name,