diff --git a/lms/events/event.py b/lms/events/event.py index 260d21915d..6e954af7ee 100644 --- a/lms/events/event.py +++ b/lms/events/event.py @@ -67,16 +67,6 @@ def from_request( ): assignment_id = assignment.id - if type_ in {EventType.Type.CONFIGURED_LAUNCH, EventType.Type.DEEP_LINKING}: - # Store the raw LTI parameters for launches - data = {} if data is None else data - if request.lti_jwt: - # For LTI1.3 include the decoded JWT as a dict - data["lti_params"] = request.lti_jwt - else: - # For LTI1.1 include the request form parameters - data["lti_params"] = request.lti_params.serialize() - return cls( request=request, type=type_, diff --git a/lms/tasks/event.py b/lms/tasks/event.py index 4cd678353e..abc0f575ae 100644 --- a/lms/tasks/event.py +++ b/lms/tasks/event.py @@ -1,18 +1,6 @@ -import logging -from datetime import UTC, datetime, timedelta - -from sqlalchemy import select, update - from lms.events.event import BaseEvent -from lms.models import Event, EventData from lms.tasks.celery import app -LOG = logging.getLogger(__name__) - - -PURGE_LAUNCH_DATA_BATCH_SIZE = 1000 -"How many rows to remove per call to purge_launch_data" - @app.task def insert_event(event: dict) -> None: @@ -23,28 +11,3 @@ def insert_event(event: dict) -> None: request.find_service(EventService).insert_event( BaseEvent(request=request, **event) ) - - -@app.task -def purge_launch_data(*, max_age_days=30) -> None: - with app.request_context() as request: - with request.tm: - events_with_old_lti_params = ( - select(Event.id) - .join(EventData) - .where( - # Find data that's is at least max_age_days old - Event.timestamp <= datetime.now(UTC) - timedelta(days=max_age_days), - # Limit the search for only twice as old as we'd expect, limiting the data set significally - Event.timestamp - >= datetime.now(UTC) - timedelta(days=max_age_days * 2), - EventData.data["lti_params"].is_not(None), - ) - .limit(PURGE_LAUNCH_DATA_BATCH_SIZE) - ) - results = request.db.execute( - update(EventData) - .where(EventData.event_id.in_(events_with_old_lti_params)) - .values(data=EventData.data - "lti_params") - ) - LOG.info("Removed lti_params from events for %d rows", results.rowcount) diff --git a/tests/factories/__init__.py b/tests/factories/__init__.py index 0dd8fe6fd2..486e1eabb3 100644 --- a/tests/factories/__init__.py +++ b/tests/factories/__init__.py @@ -22,7 +22,7 @@ USER_ID, ) from tests.factories.dashboard_admin import DashboardAdmin -from tests.factories.event import Event, EventData +from tests.factories.event import Event from tests.factories.file import File from tests.factories.grading_info import GradingInfo from tests.factories.grading_sync import GradingSync, GradingSyncGrade diff --git a/tests/factories/event.py b/tests/factories/event.py index f34560fc6a..d8745acc14 100644 --- a/tests/factories/event.py +++ b/tests/factories/event.py @@ -1,13 +1,6 @@ -from factory import SubFactory, make_factory +from factory import make_factory from factory.alchemy import SQLAlchemyModelFactory from lms import models Event = make_factory(models.Event, FACTORY_CLASS=SQLAlchemyModelFactory) - - -EventData = make_factory( - models.EventData, - FACTORY_CLASS=SQLAlchemyModelFactory, - event=SubFactory(Event), -) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 21a0f67ecd..d88d456c46 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -146,27 +146,6 @@ def pyramid_request(db_session, application_instance, lti_v11_params): return pyramid_request -@pytest.fixture -def lti_v13_pyramid_request( - pyramid_request, lti_v13_application_instance, lti_v11_params, lti_v13_params -): - pyramid_request.lti_user = factories.LTIUser( - application_instance_id=lti_v13_application_instance.id, - application_instance=lti_v13_application_instance, - user_id=lti_v11_params["user_id"], - roles=lti_v11_params["roles"], - ) - pyramid_request.user = factories.User( - application_instance_id=lti_v13_application_instance.id, - user_id=lti_v11_params["user_id"], - ) - - pyramid_request.lti_jwt = lti_v13_params - pyramid_request.lti_params = LTIParams.from_request(pyramid_request) - - return pyramid_request - - @pytest.fixture def product(pyramid_request): return pyramid_request.product diff --git a/tests/unit/lms/events/event_test.py b/tests/unit/lms/events/event_test.py index c738e97a9c..5f5eaf81a7 100644 --- a/tests/unit/lms/events/event_test.py +++ b/tests/unit/lms/events/event_test.py @@ -5,7 +5,6 @@ from lms.events import AuditTrailEvent, BaseEvent, LTIEvent, ModelChange from lms.events.event import _serialize_change -from lms.models import EventType from tests import factories @@ -33,12 +32,6 @@ def test_serialize(self, pyramid_request): } -@pytest.mark.usefixtures( - "lti_role_service", - "application_instance_service", - "assignment_service", - "course_service", -) class TestLTIEvent: def test_lti_event_no_lti_user(self, pyramid_request): pyramid_request.lti_user = None @@ -77,36 +70,24 @@ def test_lti_event( assert event.assignment_id == assignment_service.get_assignment.return_value.id assert event.data == sentinel.data + @pytest.mark.usefixtures( + "lti_role_service", "application_instance_service", "assignment_service" + ) def test_lti_event_when_no_course(self, pyramid_request, course_service): course_service.get_by_context_id.return_value = None event = LTIEvent.from_request(request=pyramid_request, type_=sentinel.type) assert not event.course_id + @pytest.mark.usefixtures( + "lti_role_service", "application_instance_service", "course_service" + ) def test_lti_event_when_no_assignment(self, pyramid_request, assignment_service): assignment_service.get_assignment.return_value = None event = LTIEvent.from_request(request=pyramid_request, type_=sentinel.type) assert not event.assignment_id - @pytest.mark.parametrize( - "type_", [EventType.Type.CONFIGURED_LAUNCH, EventType.Type.DEEP_LINKING] - ) - def test_lti_event_includes_launch_data_for_lti_v13( - self, lti_v13_pyramid_request, type_ - ): - event = LTIEvent.from_request(request=lti_v13_pyramid_request, type_=type_) - - assert event.data["lti_params"] == lti_v13_pyramid_request.lti_jwt - - @pytest.mark.parametrize( - "type_", [EventType.Type.CONFIGURED_LAUNCH, EventType.Type.DEEP_LINKING] - ) - def test_lti_event_includes_launch_data_for_lti_v11(self, pyramid_request, type_): - event = LTIEvent.from_request(request=pyramid_request, type_=type_) - - assert event.data["lti_params"] == pyramid_request.lti_params.serialize() - @pytest.fixture def pyramid_request(self, pyramid_request): pyramid_request.lti_params.update( diff --git a/tests/unit/lms/tasks/event_test.py b/tests/unit/lms/tasks/event_test.py index 999bd6c4d6..39c8e2896a 100644 --- a/tests/unit/lms/tasks/event_test.py +++ b/tests/unit/lms/tasks/event_test.py @@ -1,11 +1,8 @@ from contextlib import contextmanager -from datetime import datetime import pytest -from freezegun import freeze_time -from lms.tasks.event import insert_event, purge_launch_data -from tests import factories +from lms.tasks.event import insert_event def test_insert_event(event_service, BaseEvent, pyramid_request): @@ -15,31 +12,6 @@ def test_insert_event(event_service, BaseEvent, pyramid_request): event_service.insert_event.assert_called_once_with(BaseEvent.return_value) -@freeze_time("2024-1-25") -def test_purge_launch_data(): - recent_data = factories.EventData( - event=factories.Event(timestamp=datetime(2024, 1, 20)), - data={"lti_params": {"some": "data"}}, - ) - old_data = factories.EventData( - event=factories.Event(timestamp=datetime(2024, 1, 10)), - data={"lti_params": {"some": "data"}}, - ) - old_data_no_launch = factories.EventData( - event=factories.Event(timestamp=datetime(2024, 1, 10)), - data={"some_other_data": {"some": "data"}}, - ) - - purge_launch_data(max_age_days=10) - - # Kept data for recent launches - assert "lti_params" in recent_data.data - # Removed for events in the time window - assert "lti_params" not in old_data.data - # Other keys are not removed - assert "some_other_data" in old_data_no_launch.data - - @pytest.fixture(autouse=True) def BaseEvent(patch): return patch("lms.tasks.event.BaseEvent")