Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,9 @@
# Waffle related utilities
'openedx.core.djangoapps.waffle_utils',

# Dynamic schedules
'openedx.core.djangoapps.schedules.apps.SchedulesConfig',

# DRF filters
'django_filters',
]
Expand Down
9 changes: 8 additions & 1 deletion common/djangoapps/student/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,12 @@
import request_cache
from certificates.models import GeneratedCertificate
from course_modes.models import CourseMode
from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration
from enrollment.api import _default_course_mode
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.schedules.models import ScheduleConfig
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.theming.helpers import get_current_site
from openedx.core.djangoapps.xmodule_django.models import CourseKeyField, NoneToEmptyManager
from track import contexts
from util.milestones_helpers import is_entrance_exams_enabled
Expand Down Expand Up @@ -1715,7 +1718,11 @@ def upgrade_deadline(self):
return None

try:
if self.schedule:
schedule_driven_deadlines_enabled = (
DynamicUpgradeDeadlineConfiguration.is_enabled()
or CourseDynamicUpgradeDeadlineConfiguration.is_enabled(self.course_id)
)
if schedule_driven_deadlines_enabled and self.schedule and self.schedule.upgrade_deadline is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment for why we're checking for a non-Null value for upgrade_deadline.

log.debug(
'Schedules: Pulling upgrade deadline for CourseEnrollment %d from Schedule %d.',
self.id, self.schedule.id
Expand Down
4 changes: 3 additions & 1 deletion common/djangoapps/student/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,9 @@ class Meta(object):
model = CourseEnrollment

user = factory.SubFactory(UserFactory)
course_id = CourseKey.from_string('edX/toy/2012_Fall')
course = factory.SubFactory(
'openedx.core.djangoapps.content.course_overviews.tests.factories.CourseOverviewFactory',
)


class CourseAccessRoleFactory(DjangoModelFactory):
Expand Down
2 changes: 2 additions & 0 deletions common/djangoapps/student/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from course_modes.models import CourseMode
from course_modes.tests.factories import CourseModeFactory
from courseware.models import DynamicUpgradeDeadlineConfiguration
from openedx.core.djangoapps.schedules.models import Schedule
from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
Expand Down Expand Up @@ -131,6 +132,7 @@ def test_upgrade_deadline(self):
self.assertEqual(enrollment.upgrade_deadline, course_mode.expiration_datetime)

# The schedule's upgrade deadline should be used if a schedule exists
DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True)
schedule = ScheduleFactory(enrollment=enrollment)
self.assertEqual(enrollment.upgrade_deadline, schedule.upgrade_deadline)

Expand Down
20 changes: 20 additions & 0 deletions lms/djangoapps/bulk_email/policies.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@

from edx_ace.policy import Policy, PolicyResult
from edx_ace.channel import ChannelType
from opaque_keys.edx.keys import CourseKey

from bulk_email.models import Optout


class CourseEmailOptout(Policy):

def check(self, message):
course_id = message.context.get('course_id')
if not course_id:
return PolicyResult(deny=frozenset())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can make this logic more readable by having 2 (or even 1) return statements in this method instead of 3.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having only a single return seems less readable to me. Matter of taste, I guess.


course_key = CourseKey.from_string(course_id)
if Optout.objects.filter(user__username=message.recipient.username, course_id=course_key).exists():
return PolicyResult(deny={ChannelType.EMAIL})

return PolicyResult(deny=frozenset())
81 changes: 76 additions & 5 deletions lms/djangoapps/bulk_email/tests/test_course_optout.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
from nose.plugins.attrib import attr

from bulk_email.models import BulkEmailFlag
from bulk_email.policies import CourseEmailOptout
from edx_ace.message import Message
from edx_ace.recipient import Recipient
from edx_ace.policy import PolicyResult
from edx_ace.channel import ChannelType
from student.models import CourseEnrollment
from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
Expand All @@ -27,7 +32,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase):
def setUp(self):
super(TestOptoutCourseEmails, self).setUp()
course_title = u"ẗëṡẗ title イ乇丂イ ᄊ乇丂丂ムg乇 キo尺 ムレレ тэѕт мэѕѕаБэ"
self.course = CourseFactory.create(display_name=course_title)
self.course = CourseFactory.create(run='testcourse1', display_name=course_title)
self.instructor = AdminFactory.create()
self.student = UserFactory.create()
CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
Expand All @@ -44,10 +49,6 @@ def setUp(self):
}
BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=False)

def tearDown(self):
super(TestOptoutCourseEmails, self).tearDown()
BulkEmailFlag.objects.all().delete()

def navigate_to_email_view(self):
"""Navigate to the instructor dash's email view"""
# Pull up email view on instructor dashboard
Expand Down Expand Up @@ -114,3 +115,73 @@ def test_optin_course(self):
sent_addresses = [message.to[0] for message in mail.outbox]
self.assertIn(self.student.email, sent_addresses)
self.assertIn(self.instructor.email, sent_addresses)


@attr(shard=1)
@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True))
class TestACEOptoutCourseEmails(ModuleStoreTestCase):
"""
Test that optouts are referenced in sending course email.
"""
def setUp(self):
super(TestACEOptoutCourseEmails, self).setUp()
course_title = u"ẗëṡẗ title イ乇丂イ ᄊ乇丂丂ムg乇 キo尺 ムレレ тэѕт мэѕѕаБэ"
self.course = CourseFactory.create(run='testcourse1', display_name=course_title)
self.instructor = AdminFactory.create()
self.student = UserFactory.create()
CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)

self.client.login(username=self.student.username, password="test")

self._set_email_optout(False)
self.policy = CourseEmailOptout()

def _set_email_optout(self, opted_out):
url = reverse('change_email_settings')
# This is a checkbox, so on the post of opting out (that is, an Un-check of the box),
# the Post that is sent will not contain 'receive_emails'
post_data = {'course_id': self.course.id.to_deprecated_string()}

if not opted_out:
post_data['receive_emails'] = 'on'

response = self.client.post(url, post_data)
self.assertEquals(json.loads(response.content), {'success': True})

def test_policy_optedout(self):
"""
Make sure the policy prevents ACE emails if the user is opted-out.
"""
self._set_email_optout(True)

channel_mods = self.policy.check(self.create_test_message())
self.assertEqual(channel_mods, PolicyResult(deny={ChannelType.EMAIL}))

def create_test_message(self):
return Message(
app_label='foo',
name='bar',
recipient=Recipient(
username=self.student.username,
email_address=self.student.email,
),
context={
'course_id': str(self.course.id)
},
)

def test_policy_optedin(self):
"""
Make sure the policy allows ACE emails if the user is opted-in.
"""
channel_mods = self.policy.check(self.create_test_message())
self.assertEqual(channel_mods, PolicyResult(deny=set()))

def test_policy_no_course_id(self):
"""
Make sure the policy denies ACE emails if there is no course id in the context.
"""
message = self.create_test_message()
message.context = {}
channel_mods = self.policy.check(message)
self.assertEqual(channel_mods, PolicyResult(deny=set()))
54 changes: 27 additions & 27 deletions lms/djangoapps/ccx/tests/test_field_override_performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,18 +237,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
# # of sql queries to default,
# # of mongo queries,
# )
('no_overrides', 1, True, False): (25, 1),
('no_overrides', 2, True, False): (25, 1),
('no_overrides', 3, True, False): (25, 1),
('ccx', 1, True, False): (25, 1),
('ccx', 2, True, False): (25, 1),
('ccx', 3, True, False): (25, 1),
('no_overrides', 1, False, False): (25, 1),
('no_overrides', 2, False, False): (25, 1),
('no_overrides', 3, False, False): (25, 1),
('ccx', 1, False, False): (25, 1),
('ccx', 2, False, False): (25, 1),
('ccx', 3, False, False): (25, 1),
('no_overrides', 1, True, False): (26, 1),
('no_overrides', 2, True, False): (26, 1),
('no_overrides', 3, True, False): (26, 1),
('ccx', 1, True, False): (26, 1),
('ccx', 2, True, False): (26, 1),
('ccx', 3, True, False): (26, 1),
('no_overrides', 1, False, False): (26, 1),
('no_overrides', 2, False, False): (26, 1),
('no_overrides', 3, False, False): (26, 1),
('ccx', 1, False, False): (26, 1),
('ccx', 2, False, False): (26, 1),
('ccx', 3, False, False): (26, 1),
}


Expand All @@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True

TEST_DATA = {
('no_overrides', 1, True, False): (25, 3),
('no_overrides', 2, True, False): (25, 3),
('no_overrides', 3, True, False): (25, 3),
('ccx', 1, True, False): (25, 3),
('ccx', 2, True, False): (25, 3),
('ccx', 3, True, False): (25, 3),
('ccx', 1, True, True): (26, 3),
('ccx', 2, True, True): (26, 3),
('ccx', 3, True, True): (26, 3),
('no_overrides', 1, False, False): (25, 3),
('no_overrides', 2, False, False): (25, 3),
('no_overrides', 3, False, False): (25, 3),
('ccx', 1, False, False): (25, 3),
('ccx', 2, False, False): (25, 3),
('ccx', 3, False, False): (25, 3),
('no_overrides', 1, True, False): (26, 3),
('no_overrides', 2, True, False): (26, 3),
('no_overrides', 3, True, False): (26, 3),
('ccx', 1, True, False): (26, 3),
('ccx', 2, True, False): (26, 3),
('ccx', 3, True, False): (26, 3),
('ccx', 1, True, True): (27, 3),
('ccx', 2, True, True): (27, 3),
('ccx', 3, True, True): (27, 3),
('no_overrides', 1, False, False): (26, 3),
('no_overrides', 2, False, False): (26, 3),
('no_overrides', 3, False, False): (26, 3),
('ccx', 1, False, False): (26, 3),
('ccx', 2, False, False): (26, 3),
('ccx', 3, False, False): (26, 3),
}
19 changes: 19 additions & 0 deletions lms/djangoapps/courseware/migrations/0003_auto_20170825_0935.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('courseware', '0002_coursedynamicupgradedeadlineconfiguration_dynamicupgradedeadlineconfiguration'),
]

operations = [
migrations.AlterField(
model_name='coursedynamicupgradedeadlineconfiguration',
name='opt_out',
field=models.BooleanField(default=False, help_text='This does not do anything and is no longer used. Setting enabled=False has the same effect.'),
),
]
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,5 +398,5 @@ class Meta(object):
)
opt_out = models.BooleanField(
default=False,
help_text=_('Disable the dynamic upgrade deadline for this course run.')
help_text=_('This does not do anything and is no longer used. Setting enabled=False has the same effect.')
)
Loading