From 3fcb4abb33725e73f4f14cb80ba2dd521b338597 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 22 Aug 2023 13:08:54 -0500 Subject: [PATCH 1/5] feat: add tracking log for completion events --- completion/models.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/completion/models.py b/completion/models.py index 777d4691..d09ea815 100644 --- a/completion/models.py +++ b/completion/models.py @@ -14,12 +14,17 @@ from model_utils.models import TimeStampedModel +from eventtracking import tracker + from . import waffle log = logging.getLogger(__name__) User = auth.get_user_model() +BLOCK_COMPLETION_CHANGED_EVENT_TYPE = 'edx.completion.block_completion.changed' + + def validate_percent(value): """ Verify that the passed value is between 0.0 and 1.0. @@ -126,6 +131,20 @@ def submit_completion(self, user, block_key, completion): "BlockCompletion.objects.submit_completion should not be \ called when the feature is disabled." ) + + tracker.emit( + str(BLOCK_COMPLETION_CHANGED_EVENT_TYPE), + { + 'user_id': user.id, + 'course_id': str(block_key.course_key), + 'context_key': str(context_key), + 'block_key': str(block_key), + 'block_type': block_type, + 'completion': completion, + 'is_new': is_new, + } + ) + return obj, is_new @transaction.atomic() From 79b670f63696550cb685dea3f9b1203c81dd1356 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 22 Aug 2023 14:05:45 -0500 Subject: [PATCH 2/5] test: add event-tracking test case --- completion/test_utils.py | 51 +++++++++++++++++++++++++++++++++ completion/tests/test_models.py | 4 +-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/completion/test_utils.py b/completion/test_utils.py index e110c53d..0f9a5ef4 100644 --- a/completion/test_utils.py +++ b/completion/test_utils.py @@ -7,7 +7,11 @@ from datetime import datetime from django.contrib import auth +from django.test.utils import override_settings from edx_toggles.toggles.testutils import override_waffle_switch +from eventtracking import tracker +from django.test import TestCase +from eventtracking.django import DjangoTracker import factory from factory.django import DjangoModelFactory from opaque_keys.edx.keys import UsageKey @@ -120,3 +124,50 @@ def override_completion_switch(self, enabled): """ with override_waffle_switch(waffle.ENABLE_COMPLETION_TRACKING_SWITCH, enabled): yield + + +IN_MEMORY_BACKEND_CONFIG = { + 'mem': { + 'ENGINE': 'completion.test_utils.InMemoryBackend' + } +} + + +class InMemoryBackend: + """A backend that simply stores all events in memory""" + + def __init__(self): + super().__init__() # lint-amnesty, pylint: disable=super-with-arguments + self.events = [] + + def send(self, event): + """Store the event in a list""" + self.events.append(event) + + +@override_settings( + EVENT_TRACKING_BACKENDS=IN_MEMORY_BACKEND_CONFIG +) +class EventTrackingTestCase(TestCase): + """ + Supports capturing of emitted events in memory and inspecting them. + + Each test gets a "clean slate" and can retrieve any events emitted during their execution. + + """ + + # Make this more robust to the addition of new events that the test doesn't care about. + + def setUp(self): + super().setUp() # lint-amnesty, pylint: disable=super-with-arguments + + self.recreate_tracker() + + def recreate_tracker(self): + """ + Re-initialize the tracking system using updated django settings. + + Use this if you make use of the @override_settings decorator to customize the tracker configuration. + """ + self.tracker = DjangoTracker() + tracker.register_tracker(self.tracker) diff --git a/completion/tests/test_models.py b/completion/tests/test_models.py index 3269217b..624f2fe6 100644 --- a/completion/tests/test_models.py +++ b/completion/tests/test_models.py @@ -12,7 +12,7 @@ from opaque_keys.edx.keys import CourseKey, UsageKey from .. import models -from ..test_utils import CompletionSetUpMixin, UserFactory, submit_completions_for_testing +from ..test_utils import CompletionSetUpMixin, EventTrackingTestCase, UserFactory, submit_completions_for_testing class PercentValidatorTestCase(TestCase): @@ -29,7 +29,7 @@ def test_invalid_percent(self): self.assertRaises(ValidationError, models.validate_percent, value) -class SubmitCompletionTestCase(CompletionSetUpMixin, TestCase): +class SubmitCompletionTestCase(CompletionSetUpMixin, EventTrackingTestCase, TestCase): """ Test that BlockCompletion.objects.submit_completion has the desired semantics. From ea9664e9df259455dd90d0d51cb265c333fff385 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Wed, 23 Aug 2023 12:18:17 -0500 Subject: [PATCH 3/5] chore: refactor tracking log event emissions --- completion/models.py | 31 ++++++++++++++++++------------- completion/tests/test_models.py | 4 ++-- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/completion/models.py b/completion/models.py index d09ea815..23bca046 100644 --- a/completion/models.py +++ b/completion/models.py @@ -120,10 +120,13 @@ def submit_completion(self, user, block_key, completion): block_key=block_key, ) is_new = False + if not is_new and obj.completion != completion: obj.completion = completion obj.full_clean() obj.save(update_fields={'completion', 'modified'}) + + obj.emit_tracking_log() else: # If the feature is not enabled, this method should not be called. # Error out with a RuntimeError. @@ -132,19 +135,6 @@ def submit_completion(self, user, block_key, completion): called when the feature is disabled." ) - tracker.emit( - str(BLOCK_COMPLETION_CHANGED_EVENT_TYPE), - { - 'user_id': user.id, - 'course_id': str(block_key.course_key), - 'context_key': str(context_key), - 'block_key': str(block_key), - 'block_type': block_type, - 'completion': completion, - 'is_new': is_new, - } - ) - return obj, is_new @transaction.atomic() @@ -340,3 +330,18 @@ class Meta: def __unicode__(self): return f'BlockCompletion: {self.user.username}, {self.context_key}, {self.block_key}: {self.completion}' + + def emit_tracking_log(self): + """ + Emit a tracking log when a block completion is created or updated. + """ + tracker.emit( + BLOCK_COMPLETION_CHANGED_EVENT_TYPE, + { + 'user_id': self.user.id, + 'course_id': str(self.context_key), + 'block_id': str(self.block_key), + 'block_type': self.block_type, + 'completion': self.completion, + } + ) diff --git a/completion/tests/test_models.py b/completion/tests/test_models.py index 624f2fe6..70533a2d 100644 --- a/completion/tests/test_models.py +++ b/completion/tests/test_models.py @@ -41,7 +41,7 @@ def setUp(self): self.set_up_completion() def test_changed_value(self): - with self.assertNumQueries(6): # Get, update, 2 * savepoints, 2 * exists checks + with self.assertNumQueries(7): # 2 * Get, update, 2 * savepoints, 2 * exists checks completion, isnew = models.BlockCompletion.objects.submit_completion( user=self.user, block_key=self.block_key, @@ -53,7 +53,7 @@ def test_changed_value(self): self.assertEqual(models.BlockCompletion.objects.count(), 1) def test_unchanged_value(self): - with self.assertNumQueries(3): # Get + 2 * savepoints + with self.assertNumQueries(4): # 2 * Get + 2 * savepoints completion, isnew = models.BlockCompletion.objects.submit_completion( user=self.user, block_key=self.block_key, From 2fe7815055f0f2dc0ba95ba968a6fe9bd26dcf20 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Thu, 31 Aug 2023 08:13:48 -0500 Subject: [PATCH 4/5] feat: add modified and created to tracking log --- completion/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/completion/models.py b/completion/models.py index 23bca046..946db431 100644 --- a/completion/models.py +++ b/completion/models.py @@ -343,5 +343,7 @@ def emit_tracking_log(self): 'block_id': str(self.block_key), 'block_type': self.block_type, 'completion': self.completion, + 'modified': self.modified, + 'created': self.created, } ) From 82551e6ba9d255a14ec6a51dded2f69e14de994b Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 12 Sep 2023 13:05:59 -0500 Subject: [PATCH 5/5] chore: install event-tracking --- requirements/base.in | 1 + requirements/dev.txt | 47 +++++++++++++++++++++++++++++++++++++++- requirements/doc.txt | 47 +++++++++++++++++++++++++++++++++++++++- requirements/pip.txt | 2 +- requirements/quality.txt | 47 +++++++++++++++++++++++++++++++++++++++- requirements/test.txt | 47 +++++++++++++++++++++++++++++++++++++++- 6 files changed, 186 insertions(+), 5 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index 31ef4e66..a24d40ac 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -9,3 +9,4 @@ edx-drf-extensions>=1.11.0 edx-toggles>=1.2.0 pytz XBlock>=1.2.2 +event-tracking diff --git a/requirements/dev.txt b/requirements/dev.txt index 96164895..a08c3a4a 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -4,6 +4,8 @@ # # make upgrade # +amqp==5.1.1 + # via kombu appdirs==1.4.4 # via fs asgiref==3.7.2 @@ -12,8 +14,16 @@ astroid==2.15.6 # via # pylint # pylint-celery +backports-zoneinfo[tzdata]==0.2.1 + # via + # celery + # kombu +billiard==4.1.0 + # via celery build==1.0.3 # via pip-tools +celery==5.3.4 + # via event-tracking certifi==2023.7.22 # via requests cffi==1.15.1 @@ -26,13 +36,23 @@ charset-normalizer==3.2.0 # via requests click==8.1.7 # via + # celery + # click-didyoumean # click-log + # click-plugins + # click-repl # code-annotations # edx-django-utils # edx-lint # pip-tools +click-didyoumean==0.3.0 + # via celery click-log==0.4.0 # via edx-lint +click-plugins==1.1.1 + # via celery +click-repl==0.3.0 + # via celery code-annotations==1.5.0 # via # edx-lint @@ -64,6 +84,7 @@ django==3.2.21 # edx-drf-extensions # edx-i18n-tools # edx-toggles + # event-tracking django-crum==0.7.9 # via # edx-django-utils @@ -88,6 +109,7 @@ edx-django-utils==5.7.0 # via # edx-drf-extensions # edx-toggles + # event-tracking edx-drf-extensions==8.9.2 # via -r requirements/base.in edx-i18n-tools==1.1.0 @@ -102,6 +124,8 @@ edx-opaque-keys[django]==2.5.0 # edx-drf-extensions edx-toggles==5.1.0 # via -r requirements/base.in +event-tracking==2.2.0 + # via -r requirements/base.in exceptiongroup==1.1.3 # via pytest factory-boy==3.3.0 @@ -143,6 +167,8 @@ jinja2==3.1.2 # diff-cover keyring==24.2.0 # via twine +kombu==5.3.2 + # via celery lazy-object-proxy==1.9.0 # via astroid lxml==4.9.3 @@ -189,6 +215,8 @@ pluggy==1.3.0 # tox polib==1.2.0 # via edx-i18n-tools +prompt-toolkit==3.0.39 + # via click-repl psutil==5.9.5 # via edx-django-utils py==1.11.0 @@ -223,7 +251,9 @@ pylint-plugin-utils==0.8.2 # pylint-celery # pylint-django pymongo==3.13.0 - # via edx-opaque-keys + # via + # edx-opaque-keys + # event-tracking pynacl==1.5.0 # via edx-django-utils pyproject-hooks==1.0.0 @@ -238,6 +268,7 @@ pytest-django==4.5.2 # via -r requirements/test.in python-dateutil==2.8.2 # via + # celery # edx-drf-extensions # faker # freezegun @@ -249,6 +280,7 @@ pytz==2023.3.post1 # -r requirements/base.in # django # djangorestframework + # event-tracking # xblock pyyaml==6.0.1 # via @@ -276,6 +308,7 @@ six==1.16.0 # via # edx-drf-extensions # edx-lint + # event-tracking # fs # python-dateutil # tox @@ -317,14 +350,26 @@ typing-extensions==4.7.1 # edx-opaque-keys # faker # filelock + # kombu # pylint # rich +tzdata==2023.3 + # via + # backports-zoneinfo + # celery urllib3==2.0.4 # via # requests # twine +vine==5.0.0 + # via + # amqp + # celery + # kombu virtualenv==20.24.5 # via tox +wcwidth==0.2.6 + # via prompt-toolkit web-fragments==2.1.0 # via xblock webob==1.8.7 diff --git a/requirements/doc.txt b/requirements/doc.txt index d987d235..a07bb77a 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -8,6 +8,8 @@ accessible-pygments==0.0.4 # via pydata-sphinx-theme alabaster==0.7.13 # via sphinx +amqp==5.1.1 + # via kombu appdirs==1.4.4 # via fs asgiref==3.7.2 @@ -16,8 +18,16 @@ babel==2.12.1 # via # pydata-sphinx-theme # sphinx +backports-zoneinfo[tzdata]==0.2.1 + # via + # celery + # kombu beautifulsoup4==4.12.2 # via pydata-sphinx-theme +billiard==4.1.0 + # via celery +celery==5.3.4 + # via event-tracking certifi==2023.7.22 # via requests cffi==1.15.1 @@ -28,8 +38,18 @@ charset-normalizer==3.2.0 # via requests click==8.1.7 # via + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils +click-didyoumean==0.3.0 + # via celery +click-plugins==1.1.1 + # via celery +click-repl==0.3.0 + # via celery code-annotations==1.5.0 # via edx-toggles coverage[toml]==7.3.1 @@ -50,6 +70,7 @@ django==3.2.21 # edx-django-utils # edx-drf-extensions # edx-toggles + # event-tracking django-crum==0.7.9 # via # edx-django-utils @@ -80,6 +101,7 @@ edx-django-utils==5.7.0 # via # edx-drf-extensions # edx-toggles + # event-tracking edx-drf-extensions==8.9.2 # via -r requirements/base.in edx-opaque-keys[django]==2.5.0 @@ -88,6 +110,8 @@ edx-opaque-keys[django]==2.5.0 # edx-drf-extensions edx-toggles==5.1.0 # via -r requirements/base.in +event-tracking==2.2.0 + # via -r requirements/base.in exceptiongroup==1.1.3 # via pytest factory-boy==3.3.0 @@ -110,6 +134,8 @@ jinja2==3.1.2 # via # code-annotations # sphinx +kombu==5.3.2 + # via celery lxml==4.9.3 # via xblock markupsafe==2.1.3 @@ -129,6 +155,8 @@ pbr==5.11.1 # via stevedore pluggy==1.3.0 # via pytest +prompt-toolkit==3.0.39 + # via click-repl psutil==5.9.5 # via edx-django-utils pycparser==2.21 @@ -146,7 +174,9 @@ pyjwt[crypto]==2.8.0 # drf-jwt # edx-drf-extensions pymongo==3.13.0 - # via edx-opaque-keys + # via + # edx-opaque-keys + # event-tracking pynacl==1.5.0 # via edx-django-utils pytest==7.4.2 @@ -159,6 +189,7 @@ pytest-django==4.5.2 # via -r requirements/test.in python-dateutil==2.8.2 # via + # celery # edx-drf-extensions # faker # freezegun @@ -171,6 +202,7 @@ pytz==2023.3.post1 # babel # django # djangorestframework + # event-tracking # xblock pyyaml==6.0.1 # via @@ -187,6 +219,7 @@ semantic-version==2.10.0 six==1.16.0 # via # edx-drf-extensions + # event-tracking # fs # python-dateutil snowballstemmer==2.2.0 @@ -232,9 +265,21 @@ typing-extensions==4.7.1 # asgiref # edx-opaque-keys # faker + # kombu # pydata-sphinx-theme +tzdata==2023.3 + # via + # backports-zoneinfo + # celery urllib3==2.0.4 # via requests +vine==5.0.0 + # via + # amqp + # celery + # kombu +wcwidth==0.2.6 + # via prompt-toolkit web-fragments==2.1.0 # via xblock webob==1.8.7 diff --git a/requirements/pip.txt b/requirements/pip.txt index a53f711a..3e7d8f4a 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -10,5 +10,5 @@ wheel==0.41.2 # The following packages are considered to be unsafe in a requirements file: pip==23.2.1 # via -r requirements/pip.in -setuptools==68.2.1 +setuptools==68.2.2 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index c8113918..33880b22 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -4,6 +4,8 @@ # # make upgrade # +amqp==5.1.1 + # via kombu appdirs==1.4.4 # via fs asgiref==3.7.2 @@ -12,6 +14,14 @@ astroid==2.15.6 # via # pylint # pylint-celery +backports-zoneinfo[tzdata]==0.2.1 + # via + # celery + # kombu +billiard==4.1.0 + # via celery +celery==5.3.4 + # via event-tracking certifi==2023.7.22 # via requests cffi==1.15.1 @@ -22,12 +32,22 @@ charset-normalizer==3.2.0 # via requests click==8.1.7 # via + # celery + # click-didyoumean # click-log + # click-plugins + # click-repl # code-annotations # edx-django-utils # edx-lint +click-didyoumean==0.3.0 + # via celery click-log==0.4.0 # via edx-lint +click-plugins==1.1.1 + # via celery +click-repl==0.3.0 + # via celery code-annotations==1.5.0 # via # edx-lint @@ -54,6 +74,7 @@ django==3.2.21 # edx-django-utils # edx-drf-extensions # edx-toggles + # event-tracking django-crum==0.7.9 # via # edx-django-utils @@ -78,6 +99,7 @@ edx-django-utils==5.7.0 # via # edx-drf-extensions # edx-toggles + # event-tracking edx-drf-extensions==8.9.2 # via -r requirements/base.in edx-lint==5.3.4 @@ -88,6 +110,8 @@ edx-opaque-keys[django]==2.5.0 # edx-drf-extensions edx-toggles==5.1.0 # via -r requirements/base.in +event-tracking==2.2.0 + # via -r requirements/base.in exceptiongroup==1.1.3 # via pytest factory-boy==3.3.0 @@ -122,6 +146,8 @@ jinja2==3.1.2 # via code-annotations keyring==24.2.0 # via twine +kombu==5.3.2 + # via celery lazy-object-proxy==1.9.0 # via astroid lxml==4.9.3 @@ -154,6 +180,8 @@ platformdirs==3.10.0 # via pylint pluggy==1.3.0 # via pytest +prompt-toolkit==3.0.39 + # via click-repl psutil==5.9.5 # via edx-django-utils pycodestyle==2.11.0 @@ -185,7 +213,9 @@ pylint-plugin-utils==0.8.2 # pylint-celery # pylint-django pymongo==3.13.0 - # via edx-opaque-keys + # via + # edx-opaque-keys + # event-tracking pynacl==1.5.0 # via edx-django-utils pytest==7.4.2 @@ -198,6 +228,7 @@ pytest-django==4.5.2 # via -r requirements/test.in python-dateutil==2.8.2 # via + # celery # edx-drf-extensions # faker # freezegun @@ -209,6 +240,7 @@ pytz==2023.3.post1 # -r requirements/base.in # django # djangorestframework + # event-tracking # xblock pyyaml==6.0.1 # via @@ -235,6 +267,7 @@ six==1.16.0 # via # edx-drf-extensions # edx-lint + # event-tracking # fs # python-dateutil snowballstemmer==2.2.0 @@ -263,12 +296,24 @@ typing-extensions==4.7.1 # astroid # edx-opaque-keys # faker + # kombu # pylint # rich +tzdata==2023.3 + # via + # backports-zoneinfo + # celery urllib3==2.0.4 # via # requests # twine +vine==5.0.0 + # via + # amqp + # celery + # kombu +wcwidth==0.2.6 + # via prompt-toolkit web-fragments==2.1.0 # via xblock webob==1.8.7 diff --git a/requirements/test.txt b/requirements/test.txt index 3d7a9ee0..c22f51ee 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -4,10 +4,20 @@ # # make upgrade # +amqp==5.1.1 + # via kombu appdirs==1.4.4 # via fs asgiref==3.7.2 # via django +backports-zoneinfo[tzdata]==0.2.1 + # via + # celery + # kombu +billiard==4.1.0 + # via celery +celery==5.3.4 + # via event-tracking certifi==2023.7.22 # via requests cffi==1.15.1 @@ -18,8 +28,18 @@ charset-normalizer==3.2.0 # via requests click==8.1.7 # via + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils +click-didyoumean==0.3.0 + # via celery +click-plugins==1.1.1 + # via celery +click-repl==0.3.0 + # via celery code-annotations==1.5.0 # via edx-toggles coverage[toml]==7.3.1 @@ -39,6 +59,7 @@ ddt==1.6.0 # edx-django-utils # edx-drf-extensions # edx-toggles + # event-tracking django-crum==0.7.9 # via # edx-django-utils @@ -60,6 +81,7 @@ edx-django-utils==5.7.0 # via # edx-drf-extensions # edx-toggles + # event-tracking edx-drf-extensions==8.9.2 # via -r requirements/base.in edx-opaque-keys[django]==2.5.0 @@ -68,6 +90,8 @@ edx-opaque-keys[django]==2.5.0 # edx-drf-extensions edx-toggles==5.1.0 # via -r requirements/base.in +event-tracking==2.2.0 + # via -r requirements/base.in exceptiongroup==1.1.3 # via pytest factory-boy==3.3.0 @@ -84,6 +108,8 @@ iniconfig==2.0.0 # via pytest jinja2==3.1.2 # via code-annotations +kombu==5.3.2 + # via celery lxml==4.9.3 # via xblock markupsafe==2.1.3 @@ -100,6 +126,8 @@ pbr==5.11.1 # via stevedore pluggy==1.3.0 # via pytest +prompt-toolkit==3.0.39 + # via click-repl psutil==5.9.5 # via edx-django-utils pycparser==2.21 @@ -109,7 +137,9 @@ pyjwt[crypto]==2.8.0 # drf-jwt # edx-drf-extensions pymongo==3.13.0 - # via edx-opaque-keys + # via + # edx-opaque-keys + # event-tracking pynacl==1.5.0 # via edx-django-utils pytest==7.4.2 @@ -122,6 +152,7 @@ pytest-django==4.5.2 # via -r requirements/test.in python-dateutil==2.8.2 # via + # celery # edx-drf-extensions # faker # freezegun @@ -133,6 +164,7 @@ pytz==2023.3.post1 # -r requirements/base.in # django # djangorestframework + # event-tracking # xblock pyyaml==6.0.1 # via @@ -145,6 +177,7 @@ semantic-version==2.10.0 six==1.16.0 # via # edx-drf-extensions + # event-tracking # fs # python-dateutil sqlparse==0.4.4 @@ -165,8 +198,20 @@ typing-extensions==4.7.1 # asgiref # edx-opaque-keys # faker + # kombu +tzdata==2023.3 + # via + # backports-zoneinfo + # celery urllib3==2.0.4 # via requests +vine==5.0.0 + # via + # amqp + # celery + # kombu +wcwidth==0.2.6 + # via prompt-toolkit web-fragments==2.1.0 # via xblock webob==1.8.7