diff --git a/cms/envs/common.py b/cms/envs/common.py index 4f31b4a4d85d938a6367e7ae0910f7d85b41d2d6..acc9b682ba6765fbd8cee73df225f475a9b1d0a0 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -545,6 +545,9 @@ MIDDLEWARE_CLASSES = [ 'edx_rest_framework_extensions.auth.jwt.middleware.EnsureJWTAuthSettingsMiddleware', + # Handles automatically storing user ids in django-simple-history tables when possible. + 'simple_history.middleware.HistoryRequestMiddleware', + # This must be last so that it runs first in the process_response chain 'openedx.core.djangoapps.site_configuration.middleware.SessionCookieDomainOverrideMiddleware', ] diff --git a/common/djangoapps/student/apps.py b/common/djangoapps/student/apps.py index 9498dd2ef18e85a256cc9d45276c0f3163482471..4966cbb7b659108236bcc1bdc72586a5cbdcfbb8 100644 --- a/common/djangoapps/student/apps.py +++ b/common/djangoapps/student/apps.py @@ -3,6 +3,8 @@ Configuration for the ``student`` Django application. """ from __future__ import absolute_import +import os + from django.apps import AppConfig from django.contrib.auth.signals import user_logged_in from django.db.models.signals import pre_save @@ -23,3 +25,11 @@ class StudentConfig(AppConfig): from django.contrib.auth.models import User from .signals.receivers import on_user_updated pre_save.connect(on_user_updated, sender=User) + + # The django-simple-history model on CourseEnrollment creates performance + # problems in testing, we mock it here so that the mock impacts all tests. + if os.environ.get('DISABLE_COURSEENROLLMENT_HISTORY', False): + import student.models as student_models + from mock import MagicMock + + student_models.CourseEnrollment.history = MagicMock() diff --git a/common/djangoapps/student/migrations/0021_historicalcourseenrollment.py b/common/djangoapps/student/migrations/0021_historicalcourseenrollment.py new file mode 100644 index 0000000000000000000000000000000000000000..5d08cc59086b2e4e10518bb8fe7bed3e10aee100 --- /dev/null +++ b/common/djangoapps/student/migrations/0021_historicalcourseenrollment.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-04-25 20:18 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import simple_history.models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_overviews', '0014_courseoverview_certificate_available_date'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('student', '0020_auto_20190227_2019'), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalCourseEnrollment', + fields=[ + ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('created', models.DateTimeField(blank=True, db_index=True, editable=False, null=True)), + ('is_active', models.BooleanField(default=True)), + ('mode', models.CharField(default=b'audit', max_length=100)), + ('history_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('history_date', models.DateTimeField()), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('course', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='course_overviews.CourseOverview')), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'ordering': ('-history_date', '-history_id'), + 'db_table': 'student_courseenrollment_history', + 'verbose_name': 'historical course enrollment', + 'get_latest_by': 'history_date', + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index b8cad61cd0b8ca4618b1ab90de476268984f5f7b..da4d6176c10acec4c1a19885b17acd7adfb95fe0 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -47,6 +47,7 @@ from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField from opaque_keys.edx.keys import CourseKey from pytz import UTC +from simple_history.models import HistoricalRecords from six import text_type from six.moves import range from six.moves.urllib.parse import urlencode @@ -1130,6 +1131,14 @@ class CourseEnrollment(models.Model): # list of possible values. mode = models.CharField(default=CourseMode.DEFAULT_MODE_SLUG, max_length=100) + # An audit row will be created for every change to a CourseEnrollment. This + # will create a new model behind the scenes - HistoricalCourseEnrollment and a + # table named 'student_courseenrollment_history'. + history = HistoricalRecords( + history_id_field=models.UUIDField(default=uuid.uuid4), + table_name='student_courseenrollment_history' + ) + objects = CourseEnrollmentManager() # cache key format e.g enrollment.<username>.<course_key>.mode = 'honor' diff --git a/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py b/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py index e6306f90e1a925984f0bcc127aa1e856d510b3e6..486fd6856567ab0282424f6d2da7746b6160bbaa 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py @@ -1680,14 +1680,8 @@ class GroupModeratorPermissionsTestCase(ModuleStoreTestCase): # Create course, seed permissions roles, and create team self.course = CourseFactory.create() seed_permissions_roles(self.course.id) - verified_coursemode = CourseModeFactory.create( - course_id=self.course.id, - mode_slug=CourseMode.VERIFIED - ) - audit_coursemode = CourseModeFactory.create( - course_id=self.course.id, - mode_slug=CourseMode.AUDIT - ) + verified_coursemode = CourseMode.VERIFIED + audit_coursemode = CourseMode.AUDIT # Create four users: group_moderator (who is within the verified enrollment track and in the cohort), # verified_user (who is in the verified enrollment track but not the cohort), @@ -1781,18 +1775,6 @@ class GroupModeratorPermissionsTestCase(ModuleStoreTestCase): 'can_vote': True, 'can_report': True }) - RequestCache.clear_all_namespaces() - - set_discussion_division_settings(self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK) - content = {'user_id': self.verified_user.id, 'type': 'thread', 'username': self.verified_user.username} - self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { - 'editable': True, - 'can_reply': True, - 'can_delete': True, - 'can_openclose': True, - 'can_vote': True, - 'can_report': True - }) @mock.patch( 'lms.djangoapps.discussion.django_comment_client.permissions._check_condition', diff --git a/lms/envs/common.py b/lms/envs/common.py index c5471478ddf242808c98d03ca4934b28ca3b881a..855722576389589345b3591282d972b758b3ff11 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1324,6 +1324,9 @@ MIDDLEWARE_CLASSES = [ 'edx_rest_framework_extensions.auth.jwt.middleware.EnsureJWTAuthSettingsMiddleware', + # Handles automatically storing user ids in django-simple-history tables when possible. + 'simple_history.middleware.HistoryRequestMiddleware', + # This must be last 'openedx.core.djangoapps.site_configuration.middleware.SessionCookieDomainOverrideMiddleware', ] diff --git a/pavelib/paver_tests/test_paver_pytest_cmds.py b/pavelib/paver_tests/test_paver_pytest_cmds.py index b2bb55a3f592e79ee4570cce7d5db15c6236781e..a73ed11c0a939db38c0f36f7e343ac041b20073a 100644 --- a/pavelib/paver_tests/test_paver_pytest_cmds.py +++ b/pavelib/paver_tests/test_paver_pytest_cmds.py @@ -55,10 +55,12 @@ class TestPaverPytestCmd(unittest.TestCase): else: django_env_var_cmd = "export DJANGO_SETTINGS_MODULE='openedx.tests.settings'" + env_var_cmd = "{} DISABLE_COURSEENROLLMENT_HISTORY=1".format(django_env_var_cmd) + xdist_string = u'--tx {}*ssh="ubuntu@{} -o StrictHostKeyChecking=no"' \ '//python="source /edx/app/edxapp/edxapp_env; {}; python"' \ '//chdir="/edx/app/edxapp/edx-platform"' \ - .format(processes, ip, django_env_var_cmd) + .format(processes, ip, env_var_cmd) expected_statement.append(xdist_string) for rsync_dir in Env.rsync_dirs(): expected_statement.append(u'--rsyncdir {}'.format(rsync_dir)) diff --git a/pavelib/tests.py b/pavelib/tests.py index 7def944b1aeebb8e131e2690943cdd33e6e5ba4c..26ff37915ae7b5b67fee0618c9400c7f1c7f37bc 100644 --- a/pavelib/tests.py +++ b/pavelib/tests.py @@ -70,6 +70,19 @@ __test__ = False # do not collect dest='disable_migrations', help="Create tables by applying migrations." ), + make_option( + '--disable_courseenrollment_history', + action='store_true', + dest='disable_courseenrollment_history', + help="Disable history on student.CourseEnrollent. Can also be used by exporting" + "DISABLE_COURSEENROLLMENT_HISTORY=1." + ), + make_option( + '--enable_courseenrollment_history', + action='store_false', + dest='disable_courseenrollment_history', + help="Enable django-simple-history on student.CourseEnrollment." + ), make_option( '--xdist_ip_addresses', dest='xdist_ip_addresses', @@ -230,6 +243,19 @@ def test_lib(options, passthrough_options): dest='disable_migrations', help="Create tables directly from apps' models. Can also be used by exporting DISABLE_MIGRATIONS=1." ), + make_option( + '--disable_courseenrollment_history', + action='store_true', + dest='disable_courseenrollment_history', + help="Disable history on student.CourseEnrollent. Can also be used by exporting" + "DISABLE_COURSEENROLLMENT_HISTORY=1." + ), + make_option( + '--enable_courseenrollment_history', + action='store_false', + dest='disable_courseenrollment_history', + help="Enable django-simple-history on student.CourseEnrollment." + ), ]) @PassthroughTask @timed diff --git a/pavelib/utils/test/suites/pytest_suite.py b/pavelib/utils/test/suites/pytest_suite.py index 4e21de11b65d07a14bc294e0a55ad3d3203acb94..3360d9a811ef0d8f5b6a5958e3ac0da94bd974b6 100644 --- a/pavelib/utils/test/suites/pytest_suite.py +++ b/pavelib/utils/test/suites/pytest_suite.py @@ -26,6 +26,7 @@ class PytestSuite(TestSuite): self.django_toxenv = None else: self.django_toxenv = 'py27-django{}'.format(django_version.replace('.', '')) + self.disable_courseenrollment_history = kwargs.get('disable_courseenrollment_history', '1') self.disable_capture = kwargs.get('disable_capture', None) self.report_dir = Env.REPORT_DIR / self.root @@ -36,6 +37,10 @@ class PytestSuite(TestSuite): if os.environ.get("SHARD", None): shard_str = "shard_{}".format(os.environ.get("SHARD")) self.report_dir = self.report_dir / shard_str + + if self.disable_courseenrollment_history: + os.environ['DISABLE_COURSEENROLLMENT_HISTORY'] = '1' + self.xunit_report = self.report_dir / "nosetests.xml" self.cov_args = kwargs.get('cov_args', '') @@ -161,13 +166,14 @@ class SystemTestSuite(PytestSuite): else: xdist_remote_processes = self.processes for ip in self.xdist_ip_addresses.split(','): - # The django settings runtime command does not propagate to xdist remote workers - django_env_var_cmd = u'export DJANGO_SETTINGS_MODULE={}' \ - .format('{}.envs.{}'.format(self.root, self.settings)) + # Propogate necessary env vars to xdist containers + env_var_cmd = u'export DJANGO_SETTINGS_MODULE={} DISABLE_COURSEENROLLMENT_HISTORY={}'\ + .format('{}.envs.{}'.format(self.root, self.settings), + self.disable_courseenrollment_history) xdist_string = u'--tx {}*ssh="ubuntu@{} -o StrictHostKeyChecking=no"' \ '//python="source /edx/app/edxapp/edxapp_env; {}; python"' \ '//chdir="/edx/app/edxapp/edx-platform"' \ - .format(xdist_remote_processes, ip, django_env_var_cmd) + .format(xdist_remote_processes, ip, env_var_cmd) cmd.append(xdist_string) for rsync_dir in Env.rsync_dirs(): cmd.append(u'--rsyncdir {}'.format(rsync_dir)) @@ -280,15 +286,19 @@ class LibTestSuite(PytestSuite): else: xdist_remote_processes = self.processes for ip in self.xdist_ip_addresses.split(','): - # The django settings runtime command does not propagate to xdist remote workers + # Propogate necessary env vars to xdist containers if 'pavelib/paver_tests' in self.test_id: django_env_var_cmd = "export DJANGO_SETTINGS_MODULE='lms.envs.test'" else: django_env_var_cmd = "export DJANGO_SETTINGS_MODULE='openedx.tests.settings'" + + env_var_cmd = u'{} DISABLE_COURSEENROLLMENT_HISTORY={}' \ + .format(django_env_var_cmd, self.disable_courseenrollment_history) + xdist_string = u'--tx {}*ssh="ubuntu@{} -o StrictHostKeyChecking=no"' \ '//python="source /edx/app/edxapp/edxapp_env; {}; python"' \ '//chdir="/edx/app/edxapp/edx-platform"' \ - .format(xdist_remote_processes, ip, django_env_var_cmd) + .format(xdist_remote_processes, ip, env_var_cmd) cmd.append(xdist_string) for rsync_dir in Env.rsync_dirs(): cmd.append(u'--rsyncdir {}'.format(rsync_dir))