diff --git a/.gitignore b/.gitignore index e92d49a0f2e4..4df5d7991671 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,8 @@ node_modules *.scssc lms/static/sass/*.css lms/static/sass/application.scss +lms/static/sass/application-extend1.scss +lms/static/sass/application-extend2.scss lms/static/sass/course.scss cms/static/sass/*.css diff --git a/AUTHORS b/AUTHORS index 94963e463041..2f4d7efeadd5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -89,3 +89,4 @@ Akshay Jagadeesh Nick Parlante Marko Seric Felipe Montoya +Julia Hansbrough diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bd28c47a741d..ff74b9873b55 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,7 +5,14 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. -LMS: Add PaidCourseRegistration mode, where payment is required before course registration. +LMS: Fix issue with CourseMode expiration dates + +LMS: Ported bulk emailing to the beta instructor dashboard. + +LMS: Add monitoring of bulk email subtasks to display progress on instructor dash. + +LMS: Add PaidCourseRegistration mode, where payment is required before course +registration. LMS: Add split testing functionality for internal use. diff --git a/cms/djangoapps/contentstore/features/video.feature b/cms/djangoapps/contentstore/features/video.feature index ad6fea083be2..8f6ca144b861 100644 --- a/cms/djangoapps/contentstore/features/video.feature +++ b/cms/djangoapps/contentstore/features/video.feature @@ -2,15 +2,18 @@ Feature: CMS.Video Component As a course author, I want to be able to view my created videos in Studio. + # 1 # Video Alpha Features will work in Firefox only when Firefox is the active window Scenario: Autoplay is disabled in Studio Given I have created a Video component Then when I view the video it does not have autoplay enabled + # 2 Scenario: Creating a video takes a single click Given I have clicked the new unit button Then creating a video takes a single click + # 3 # Sauce Labs cannot delete cookies @skip_sauce Scenario: Captions are hidden correctly @@ -18,12 +21,14 @@ Feature: CMS.Video Component And I have hidden captions Then when I view the video it does not show the captions + # 4 # Sauce Labs cannot delete cookies @skip_sauce Scenario: Captions are shown correctly Given I have created a Video component with subtitles Then when I view the video it does show the captions + # 5 # Sauce Labs cannot delete cookies @skip_sauce Scenario: Captions are toggled correctly @@ -31,6 +36,35 @@ Feature: CMS.Video Component And I have toggled captions Then when I view the video it does show the captions + # 6 Scenario: Video data is shown correctly Given I have created a video with only XML data Then the correct Youtube video is shown + + # 7 + Scenario: Closed captions become visible when the mouse hovers over CC button + Given I have created a Video component with subtitles + And Make sure captions are closed + Then Captions become "invisible" after 3 seconds + And I hover over button "CC" + Then Captions become "visible" + And I hover over button "volume" + Then Captions become "invisible" after 3 seconds + + # 8 + Scenario: Open captions never become invisible + Given I have created a Video component with subtitles + And Make sure captions are open + Then Captions are "visible" + And I hover over button "CC" + Then Captions are "visible" + And I hover over button "volume" + Then Captions are "visible" + + # 9 + Scenario: Closed captions are invisible when mouse doesn't hover on CC button + Given I have created a Video component with subtitles + And Make sure captions are closed + Then Captions become "invisible" after 3 seconds + And I hover over button "volume" + Then Captions are "invisible" diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index 59c5f2bab9e1..1b422294d0a1 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -5,6 +5,11 @@ from xmodule.modulestore import Location from contentstore.utils import get_modulestore +BUTTONS = { + 'CC': '.hide-subtitles', + 'volume': '.volume', +} + @step('I have created a Video component$') def i_created_a_video_component(step): @@ -17,8 +22,13 @@ def i_created_a_video_component(step): @step('I have created a Video component with subtitles$') -def i_created_a_video_component_subtitles(step): - step.given('I have created a Video component') +def i_created_a_video_with_subs(_step): + _step.given('I have created a Video component with subtitles "OEoXaMPEzfM"') + + +@step('I have created a Video component with subtitles "([^"]*)"$') +def i_created_a_video_with_subs_with_name(_step, sub_id): + _step.given('I have created a Video component') # Store the current URL so we can return here video_url = world.browser.url @@ -108,3 +118,37 @@ def the_youtube_video_is_shown(_step): ele = world.css_find('.video').first assert ele['data-streams'].split(':')[1] == world.scenario_dict['YOUTUBE_ID'] + +@step('Make sure captions are (.+)$') +def set_captions_visibility_state(_step, captions_state): + if captions_state == 'closed': + if world.css_visible('.subtitles'): + world.browser.find_by_css('.hide-subtitles').click() + else: + if not world.css_visible('.subtitles'): + world.browser.find_by_css('.hide-subtitles').click() + + +@step('I hover over button "([^"]*)"$') +def hover_over_button(_step, button): + world.css_find(BUTTONS[button.strip()]).mouse_over() + + +@step('Captions (?:are|become) "([^"]*)"$') +def are_captions_visibile(_step, visibility_state): + _step.given('Captions become "{0}" after 0 seconds'.format(visibility_state)) + + +@step('Captions (?:are|become) "([^"]*)" after (.+) seconds$') +def check_captions_visibility_state(_step, visibility_state, timeout): + timeout = int(timeout.strip()) + + # Captions become invisible by fading out. We must wait by a specified + # time. + world.wait(timeout) + + if visibility_state == 'visible': + assert world.css_visible('.subtitles') + else: + assert not world.css_visible('.subtitles') + diff --git a/cms/envs/common.py b/cms/envs/common.py index 13faf5520e94..a82ec4ab4157 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -150,6 +150,7 @@ # Instead of AuthenticationMiddleware, we use a cache-backed version 'cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + 'student.middleware.UserStandingMiddleware', 'contentserver.middleware.StaticContentServer', 'django.contrib.messages.middleware.MessageMiddleware', diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index fa1a687e47c6..ce1505964313 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -55,8 +55,8 @@ def modes_for_course(cls, course_id): """ now = datetime.now(pytz.UTC) found_course_modes = cls.objects.filter(Q(course_id=course_id) & - Q(expiration_date__isnull=True) | - Q(expiration_date__gte=now)) + (Q(expiration_date__isnull=True) | + Q(expiration_date__gte=now))) modes = ([Mode(mode.mode_slug, mode.mode_display_name, mode.min_price, mode.suggested_prices, mode.currency) for mode in found_course_modes]) if not modes: diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index deeed6ea9ba0..651c7c51a56d 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -102,3 +102,12 @@ def test_modes_for_course_expired(self): self.create_mode(mode1.slug, mode1.name, mode1.min_price, mode1.suggested_prices) modes = CourseMode.modes_for_course(self.course_id) self.assertEqual([mode1], modes) + + expired_mode.expiration_date = datetime.now(pytz.UTC) + timedelta(days=1) + expired_mode.save() + expired_mode_value = Mode(u'verified', u'Verified Certificate', 0, '', 'usd') + modes = CourseMode.modes_for_course(self.course_id) + self.assertEqual([expired_mode_value, mode1], modes) + + modes = CourseMode.modes_for_course('second_test_course') + self.assertEqual([CourseMode.DEFAULT_MODE], modes) diff --git a/common/djangoapps/student/middleware.py b/common/djangoapps/student/middleware.py new file mode 100644 index 000000000000..b4f6ad0686ff --- /dev/null +++ b/common/djangoapps/student/middleware.py @@ -0,0 +1,37 @@ +""" +Middleware that checks user standing for the purpose of keeping users with +disabled accounts from accessing the site. +""" +from django.http import HttpResponseForbidden +from django.utils.translation import ugettext as _ +from django.conf import settings +from student.models import UserStanding + +class UserStandingMiddleware(object): + """ + Checks a user's standing on request. Returns a 403 if the user's + status is 'disabled'. + """ + def process_request(self, request): + user = request.user + try: + user_account = UserStanding.objects.get(user=user.id) + # because user is a unique field in UserStanding, there will either be + # one or zero user_accounts associated with a UserStanding + except UserStanding.DoesNotExist: + pass + else: + if user_account.account_status == UserStanding.ACCOUNT_DISABLED: + msg = _( + 'Your account has been disabled. If you believe ' + 'this was done in error, please contact us at ' + '{link_start}{support_email}{link_end}' + ).format( + support_email=settings.DEFAULT_FEEDBACK_EMAIL, + link_start=u''.format( + address=settings.DEFAULT_FEEDBACK_EMAIL, + subject_line=_('Disabled Account'), + ), + link_end=u'' + ) + return HttpResponseForbidden(msg) diff --git a/common/djangoapps/student/migrations/0028_auto__add_userstanding.py b/common/djangoapps/student/migrations/0028_auto__add_userstanding.py new file mode 100644 index 000000000000..d26699448a08 --- /dev/null +++ b/common/djangoapps/student/migrations/0028_auto__add_userstanding.py @@ -0,0 +1,188 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'UserStanding' + db.create_table('student_userstanding', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('user', self.gf('django.db.models.fields.related.ForeignKey')(related_name='standing', unique=True, to=orm['auth.User'])), + ('account_status', self.gf('django.db.models.fields.CharField')(max_length=31, blank=True)), + ('changed_by', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'], blank=True)), + ('standing_last_changed_at', self.gf('django.db.models.fields.DateTimeField')(auto_now=True, blank=True)), + )) + db.send_create_signal('student', ['UserStanding']) + + + def backwards(self, orm): + # Deleting model 'UserStanding' + db.delete_table('student_userstanding') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'student.courseenrollment': { + 'Meta': {'ordering': "('user', 'course_id')", 'unique_together': "(('user', 'course_id'),)", 'object_name': 'CourseEnrollment'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '100'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.courseenrollmentallowed': { + 'Meta': {'unique_together': "(('email', 'course_id'),)", 'object_name': 'CourseEnrollmentAllowed'}, + 'auto_enroll': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'student.pendingemailchange': { + 'Meta': {'object_name': 'PendingEmailChange'}, + 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_email': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.pendingnamechange': { + 'Meta': {'object_name': 'PendingNameChange'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'rationale': ('django.db.models.fields.CharField', [], {'max_length': '1024', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.registration': { + 'Meta': {'object_name': 'Registration', 'db_table': "'auth_registration'"}, + 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.testcenterregistration': { + 'Meta': {'object_name': 'TestCenterRegistration'}, + 'accommodation_code': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}), + 'accommodation_request': ('django.db.models.fields.CharField', [], {'max_length': '1024', 'blank': 'True'}), + 'authorization_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'db_index': 'True'}), + 'client_authorization_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '20', 'db_index': 'True'}), + 'confirmed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'eligibility_appointment_date_first': ('django.db.models.fields.DateField', [], {'db_index': 'True'}), + 'eligibility_appointment_date_last': ('django.db.models.fields.DateField', [], {'db_index': 'True'}), + 'exam_series_code': ('django.db.models.fields.CharField', [], {'max_length': '15', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'processed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'testcenter_user': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': "orm['student.TestCenterUser']"}), + 'updated_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'upload_error_message': ('django.db.models.fields.CharField', [], {'max_length': '512', 'blank': 'True'}), + 'upload_status': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '20', 'blank': 'True'}), + 'uploaded_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'user_updated_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}) + }, + 'student.testcenteruser': { + 'Meta': {'object_name': 'TestCenterUser'}, + 'address_1': ('django.db.models.fields.CharField', [], {'max_length': '40'}), + 'address_2': ('django.db.models.fields.CharField', [], {'max_length': '40', 'blank': 'True'}), + 'address_3': ('django.db.models.fields.CharField', [], {'max_length': '40', 'blank': 'True'}), + 'candidate_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'db_index': 'True'}), + 'city': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}), + 'client_candidate_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '50', 'db_index': 'True'}), + 'company_name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '50', 'blank': 'True'}), + 'confirmed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'country': ('django.db.models.fields.CharField', [], {'max_length': '3', 'db_index': 'True'}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'extension': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '8', 'blank': 'True'}), + 'fax': ('django.db.models.fields.CharField', [], {'max_length': '35', 'blank': 'True'}), + 'fax_country_code': ('django.db.models.fields.CharField', [], {'max_length': '3', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '50', 'db_index': 'True'}), + 'middle_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'phone': ('django.db.models.fields.CharField', [], {'max_length': '35'}), + 'phone_country_code': ('django.db.models.fields.CharField', [], {'max_length': '3', 'db_index': 'True'}), + 'postal_code': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '16', 'blank': 'True'}), + 'processed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'salutation': ('django.db.models.fields.CharField', [], {'max_length': '50', 'blank': 'True'}), + 'state': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '20', 'blank': 'True'}), + 'suffix': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'updated_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'upload_error_message': ('django.db.models.fields.CharField', [], {'max_length': '512', 'blank': 'True'}), + 'upload_status': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '20', 'blank': 'True'}), + 'uploaded_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': "orm['auth.User']", 'unique': 'True'}), + 'user_updated_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}) + }, + 'student.userprofile': { + 'Meta': {'object_name': 'UserProfile', 'db_table': "'auth_userprofile'"}, + 'allow_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'courseware': ('django.db.models.fields.CharField', [], {'default': "'course.xml'", 'max_length': '255', 'blank': 'True'}), + 'gender': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}), + 'goals': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'language': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'level_of_education': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}), + 'location': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'mailing_address': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'meta': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'profile'", 'unique': 'True', 'to': "orm['auth.User']"}), + 'year_of_birth': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}) + }, + 'student.userstanding': { + 'Meta': {'object_name': 'UserStanding'}, + 'account_status': ('django.db.models.fields.CharField', [], {'max_length': '31', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'standing_last_changed_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'standing'", 'unique': 'True', 'to': "orm['auth.User']"}) + }, + 'student.usertestgroup': { + 'Meta': {'object_name': 'UserTestGroup'}, + 'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}), + 'users': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.User']", 'db_index': 'True', 'symmetrical': 'False'}) + } + } + + complete_apps = ['student'] \ No newline at end of file diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index da16a2fda227..78e673df715e 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -33,6 +33,27 @@ log = logging.getLogger(__name__) AUDIT_LOG = logging.getLogger("audit") +class UserStanding(models.Model): + """ + This table contains a student's account's status. + Currently, we're only disabling accounts; in the future we can imagine + taking away more specific privileges, like forums access, or adding + more specific karma levels or probationary stages. + """ + ACCOUNT_DISABLED = "disabled" + ACCOUNT_ENABLED = "enabled" + USER_STANDING_CHOICES = ( + (ACCOUNT_DISABLED, u"Account Disabled"), + (ACCOUNT_ENABLED, u"Account Enabled"), + ) + + user = models.ForeignKey(User, db_index=True, related_name='standing', unique=True) + account_status = models.CharField( + blank=True, max_length=31, choices=USER_STANDING_CHOICES + ) + changed_by = models.ForeignKey(User, blank=True) + standing_last_changed_at = models.DateTimeField(auto_now=True) + class UserProfile(models.Model): """This is where we store all the user demographic fields. We have a diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index 49864fcbd4bb..b2f4d95776e0 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -1,6 +1,7 @@ from student.models import (User, UserProfile, Registration, CourseEnrollmentAllowed, CourseEnrollment, - PendingEmailChange) + PendingEmailChange, UserStanding, + ) from django.contrib.auth.models import Group from datetime import datetime from factory import DjangoModelFactory, SubFactory, PostGenerationMethodCall, post_generation, Sequence @@ -16,6 +17,13 @@ class GroupFactory(DjangoModelFactory): name = u'staff_MITx/999/Robot_Super_Course' +class UserStandingFactory(DjangoModelFactory): + FACTORY_FOR = UserStanding + + user = None + account_status = None + changed_by = None + class UserProfileFactory(DjangoModelFactory): FACTORY_FOR = UserProfile diff --git a/common/djangoapps/student/tests/test_userstanding.py b/common/djangoapps/student/tests/test_userstanding.py new file mode 100644 index 000000000000..730984c91875 --- /dev/null +++ b/common/djangoapps/student/tests/test_userstanding.py @@ -0,0 +1,115 @@ +""" +These are tests for disabling and enabling student accounts, and for making sure +that students with disabled accounts are unable to access the courseware. +""" +from student.tests.factories import UserFactory, UserStandingFactory +from student.models import UserStanding +from django.test import TestCase, Client +from django.core.urlresolvers import reverse, NoReverseMatch +from nose.plugins.skip import SkipTest + + +class UserStandingTest(TestCase): + """test suite for user standing view for enabling and disabling accounts""" + + def setUp(self): + # create users + self.bad_user = UserFactory.create( + username='bad_user', + ) + self.good_user = UserFactory.create( + username='good_user', + ) + self.non_staff = UserFactory.create( + username='non_staff', + ) + self.admin = UserFactory.create( + username='admin', + is_staff=True, + ) + + # create clients + self.bad_user_client = Client() + self.good_user_client = Client() + self.non_staff_client = Client() + self.admin_client = Client() + + for user, client in [ + (self.bad_user, self.bad_user_client), + (self.good_user, self.good_user_client), + (self.non_staff, self.non_staff_client), + (self.admin, self.admin_client), + ]: + client.login(username=user.username, password='test') + + UserStandingFactory.create( + user=self.bad_user, + account_status=UserStanding.ACCOUNT_DISABLED, + changed_by=self.admin + ) + + # set different stock urls for lms and cms + # to test disabled accounts' access to site + try: + self.some_url = reverse('dashboard') + except NoReverseMatch: + self.some_url = reverse('index') + + # since it's only possible to disable accounts from lms, we're going + # to skip tests for cms + + def test_disable_account(self): + self.assertEqual( + UserStanding.objects.filter(user=self.good_user).count(), 0 + ) + try: + response = self.admin_client.post(reverse('disable_account_ajax'), { + 'username': self.good_user.username, + 'account_action': 'disable', + }) + except NoReverseMatch: + raise SkipTest() + self.assertEqual( + UserStanding.objects.get(user=self.good_user).account_status, + UserStanding.ACCOUNT_DISABLED + ) + + def test_disabled_account_403s(self): + response = self.bad_user_client.get(self.some_url) + self.assertEqual(response.status_code, 403) + + def test_reenable_account(self): + try: + response = self.admin_client.post(reverse('disable_account_ajax'), { + 'username': self.bad_user.username, + 'account_action': 'reenable' + }) + except NoReverseMatch: + raise SkipTest() + self.assertEqual( + UserStanding.objects.get(user=self.bad_user).account_status, + UserStanding.ACCOUNT_ENABLED + ) + + def test_non_staff_cant_access_disable_view(self): + try: + response = self.non_staff_client.get(reverse('manage_user_standing'), { + 'user': self.non_staff, + }) + except NoReverseMatch: + raise SkipTest() + self.assertEqual(response.status_code, 404) + + def test_non_staff_cant_disable_account(self): + try: + response = self.non_staff_client.post(reverse('disable_account_ajax'), { + 'username': self.good_user.username, + 'user': self.non_staff, + 'account_action': 'disable' + }) + except NoReverseMatch: + raise SkipTest() + self.assertEqual(response.status_code, 404) + self.assertEqual( + UserStanding.objects.filter(user=self.good_user).count(), 0 + ) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index db2ce5b4a452..6360f7f2e653 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -16,6 +16,7 @@ from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required from django.contrib.auth.views import password_reset_confirm +# from django.contrib.sessions.models import Session from django.core.cache import cache from django.core.context_processors import csrf from django.core.mail import send_mail @@ -29,18 +30,21 @@ from django_future.csrf import ensure_csrf_cookie from django.utils.http import cookie_date, base36_to_int, urlencode from django.utils.translation import ugettext as _ -from django.views.decorators.http import require_POST +from django.views.decorators.http import require_POST, require_GET +from django.contrib.admin.views.decorators import staff_member_required +from django.utils.translation import ugettext as _u from ratelimitbackend.exceptions import RateLimitException from mitxmako.shortcuts import render_to_response, render_to_string from course_modes.models import CourseMode -from student.models import (Registration, UserProfile, TestCenterUser, TestCenterUserForm, - TestCenterRegistration, TestCenterRegistrationForm, - PendingNameChange, PendingEmailChange, - CourseEnrollment, unique_id_for_user, - get_testcenter_registration, CourseEnrollmentAllowed) +from student.models import ( + Registration, UserProfile, TestCenterUser, TestCenterUserForm, + TestCenterRegistration, TestCenterRegistrationForm, PendingNameChange, + PendingEmailChange, CourseEnrollment, unique_id_for_user, + get_testcenter_registration, CourseEnrollmentAllowed, UserStanding, +) from student.forms import PasswordResetFormNoActive from certificates.models import CertificateStatuses, certificate_status_for_student @@ -65,6 +69,8 @@ from dogapi import dog_stats_api from pytz import UTC +from util.json_request import JsonResponse + log = logging.getLogger("mitx.student") AUDIT_LOG = logging.getLogger("audit") @@ -597,6 +603,81 @@ def logout_user(request): domain=settings.SESSION_COOKIE_DOMAIN) return response +@require_GET +@login_required +@ensure_csrf_cookie +def manage_user_standing(request): + """ + Renders the view used to manage user standing. Also displays a table + of user accounts that have been disabled and who disabled them. + """ + if not request.user.is_staff: + raise Http404 + all_disabled_accounts = UserStanding.objects.filter( + account_status=UserStanding.ACCOUNT_DISABLED + ) + + all_disabled_users = [standing.user for standing in all_disabled_accounts] + + headers = ['username', 'account_changed_by'] + rows = [] + for user in all_disabled_users: + row = [user.username, user.standing.all()[0].changed_by] + rows.append(row) + + + context = {'headers': headers, 'rows': rows} + + return render_to_response("manage_user_standing.html", context) + +@require_POST +@login_required +@ensure_csrf_cookie +def disable_account_ajax(request): + """ + Ajax call to change user standing. Endpoint of the form + in manage_user_standing.html + """ + if not request.user.is_staff: + raise Http404 + username = request.POST.get('username') + context = {} + if username is None or username.strip() == '': + context['message'] = _u('Please enter a username') + return JsonResponse(context, status=400) + + account_action = request.POST.get('account_action') + if account_action is None: + context['message'] = _u('Please choose an option') + return JsonResponse(context, status=400) + + username = username.strip() + try: + user = User.objects.get(username=username) + except User.DoesNotExist: + context['message'] = _u("User with username {} does not exist").format(username) + return JsonResponse(context, status=400) + else: + user_account, _ = UserStanding.objects.get_or_create( + user=user, defaults={'changed_by': request.user}, + ) + if account_action == 'disable': + user_account.account_status = UserStanding.ACCOUNT_DISABLED + context['message'] = _u("Successfully disabled {}'s account").format(username) + log.info("{} disabled {}'s account".format(request.user, username)) + elif account_action == 'reenable': + user_account.account_status = UserStanding.ACCOUNT_ENABLED + context['message'] = _u("Successfully reenabled {}'s account").format(username) + log.info("{} reenabled {}'s account".format(request.user, username)) + else: + context['message'] = _u("Unexpected account status") + return JsonResponse(context, status=400) + user_account.changed_by = request.user + user_account.standing_last_changed_at = datetime.datetime.now(UTC) + user_account.save() + + return JsonResponse(context) + @login_required @ensure_csrf_cookie diff --git a/common/djangoapps/terrain/course_helpers.py b/common/djangoapps/terrain/course_helpers.py index 22222d30a460..26a60ef9946a 100644 --- a/common/djangoapps/terrain/course_helpers.py +++ b/common/djangoapps/terrain/course_helpers.py @@ -2,7 +2,7 @@ # pylint: disable=W0621 from lettuce import world -from django.contrib.auth.models import User +from django.contrib.auth.models import User, Group from student.models import CourseEnrollment from xmodule.modulestore.django import editable_modulestore from xmodule.contentstore.django import contentstore @@ -41,15 +41,30 @@ def log_in(username='robot', password='test', email='robot@edx.org', name='Robot @world.absorb -def register_by_course_id(course_id, is_staff=False): - create_user('robot', 'password') - u = User.objects.get(username='robot') +def register_by_course_id(course_id, username='robot', password='test', is_staff=False): + create_user(username, password) + user = User.objects.get(username=username) if is_staff: u.is_staff = True u.save() CourseEnrollment.enroll(u, course_id) +@world.absorb +def add_to_course_staff(username, course_num): + """ + Add the user with `username` to the course staff group + for `course_num`. + """ + # Based on code in lms/djangoapps/courseware/access.py + group_name = "instructor_{}".format(course_num) + group, _ = Group.objects.get_or_create(name=group_name) + group.save() + + user = User.objects.get(username=username) + user.groups.add(group) + + @world.absorb def clear_courses(): # Flush and initialize the module store diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py index e400ca9b95d9..52ab7462a5ae 100644 --- a/common/djangoapps/terrain/ui_helpers.py +++ b/common/djangoapps/terrain/ui_helpers.py @@ -45,8 +45,46 @@ def is_css_not_present(css_selector, wait_time=5): world.browser.driver.implicitly_wait(world.IMPLICIT_WAIT) @world.absorb -def css_has_text(css_selector, text, index=0): - return world.css_text(css_selector, index=index) == text +def css_has_text(css_selector, text, index=0, strip=False): + """ + Return a boolean indicating whether the element with `css_selector` + has `text`. + + If `strip` is True, strip whitespace at beginning/end of both + strings before comparing. + + If there are multiple elements matching the css selector, + use `index` to indicate which one. + """ + # If we're expecting a non-empty string, give the page + # a chance to fill in text fields. + if text: + world.wait_for(lambda _: world.css_text(css_selector, index=index)) + + actual_text = world.css_text(css_selector, index=index) + + if strip: + actual_text = actual_text.strip() + text = text.strip() + + return actual_text == text + + +@world.absorb +def css_has_value(css_selector, value, index=0): + """ + Return a boolean indicating whether the element with + `css_selector` has the specified `value`. + + If there are multiple elements matching the css selector, + use `index` to indicate which one. + """ + # If we're expecting a non-empty string, give the page + # a chance to fill in values + if value: + world.wait_for(lambda _: world.css_value(css_selector, index=index)) + + return world.css_value(css_selector, index=index) == value @world.absorb diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index c087d180985d..2b4620ebeea2 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -392,7 +392,7 @@ div.video { @include transition(none); -webkit-font-smoothing: antialiased; width: 30px; - + &:hover, &:active { background-color: #444; color: #fff; @@ -457,7 +457,7 @@ div.video { text-indent: -9999px; @include transition(none); width: 30px; - + &:hover, &:active { background-color: #444; color: #fff; @@ -610,6 +610,8 @@ div.video { ol.subtitles { width: 0; height: 0; + + visibility: hidden; } ol.subtitles.html5 { @@ -643,6 +645,8 @@ div.video { ol.subtitles { right: -(flex-grid(4)); width: auto; + + visibility: hidden; } } diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index f607430ba06d..e6589128851f 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -12,6 +12,7 @@ data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + data-autohide-html5="True" >
diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_all.html b/common/lib/xmodule/xmodule/js/fixtures/video_all.html index 57052bf65dab..b774134cf7e5 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_all.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_all.html @@ -15,6 +15,7 @@ data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + data-autohide-html5="True" >
diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_html5.html b/common/lib/xmodule/xmodule/js/fixtures/video_html5.html index 32789b6ba991..fcb5a3c319d8 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_html5.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_html5.html @@ -15,6 +15,7 @@ data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + data-autohide-html5="True" >
diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html b/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html index 61975784c171..ceb24299e936 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html @@ -12,6 +12,7 @@ data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + data-autohide-html5="True" >
diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html index c6b40cdf164d..bf9272d230c7 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -12,6 +12,7 @@ data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + data-autohide-html5="True" >
@@ -73,6 +74,8 @@

Speed

data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + + data-autohide-html5="True" >
@@ -130,6 +133,8 @@

Speed

data-autoplay="False" data-yt-test-timeout="1500" data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" + + data-autohide-html5="True" >
diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index 0f729da62d4d..061576efd23a 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js @@ -1,673 +1,798 @@ -(function() { - describe('VideoCaption', function() { - var state, videoPlayer, videoCaption, videoSpeedControl, oldOTBD; - - function initialize() { - loadFixtures('video_all.html'); - state = new Video('#example'); - videoPlayer = state.videoPlayer; - videoCaption = state.videoCaption; - videoSpeedControl = state.videoSpeedControl; - videoControl = state.videoControl; - } - - beforeEach(function() { - oldOTBD = window.onTouchBasedDevice; - window.onTouchBasedDevice = jasmine.createSpy('onTouchBasedDevice').andReturn(false); - initialize(); - }); - - afterEach(function() { - YT.Player = void 0; - $.fn.scrollTo.reset(); - $('.subtitles').remove(); - $('source').remove(); - window.onTouchBasedDevice = oldOTBD; - }); - - describe('constructor', function() { - describe('always', function() { - beforeEach(function() { - spyOn($, 'ajaxWithPrefix').andCallThrough(); - initialize(); - }); - - it('create the caption element', function() { - expect($('.video')).toContain('ol.subtitles'); - }); - - it('add caption control to video player', function() { - expect($('.video')).toContain('a.hide-subtitles'); - }); - - it('fetch the caption', function() { - waitsFor(function () { - if (videoCaption.loaded === true) { - return true; - } - - return false; - }, 'Expect captions to be loaded.', 1000); - - runs(function () { - expect($.ajaxWithPrefix).toHaveBeenCalledWith({ - url: videoCaption.captionURL(), - notifyOnError: false, - success: jasmine.any(Function), - error: jasmine.any(Function), - }); - }); - }); - - it('bind window resize event', function() { - expect($(window)).toHandleWith('resize', videoCaption.resize); - }); - - it('bind the hide caption button', function() { - expect($('.hide-subtitles')).toHandleWith('click', videoCaption.toggle); - }); - - it('bind the mouse movement', function() { - expect($('.subtitles')).toHandleWith('mouseover', videoCaption.onMouseEnter); - expect($('.subtitles')).toHandleWith('mouseout', videoCaption.onMouseLeave); - expect($('.subtitles')).toHandleWith('mousemove', videoCaption.onMovement); - expect($('.subtitles')).toHandleWith('mousewheel', videoCaption.onMovement); - expect($('.subtitles')).toHandleWith('DOMMouseScroll', videoCaption.onMovement); - }); - - it('bind the scroll', function() { - expect($('.subtitles')).toHandleWith('scroll', videoCaption.autoShowCaptions); - expect($('.subtitles')).toHandleWith('scroll', videoControl.showControls); - }); - }); - - describe('when on a non touch-based device', function() { - beforeEach(function() { - initialize(); - }); - - it('render the caption', function() { - var captionsData; - - captionsData = jasmine.stubbedCaption; - $('.subtitles li[data-index]').each(function(index, link) { - expect($(link)).toHaveData('index', index); - expect($(link)).toHaveData('start', captionsData.start[index]); - expect($(link)).toHaveAttr('tabindex', 0); - expect($(link)).toHaveText(captionsData.text[index]); - }); - }); - - it('add a padding element to caption', function() { - expect($('.subtitles li:first').hasClass('spacing')).toBe(true); - expect($('.subtitles li:last').hasClass('spacing')).toBe(true); - }); - - it('bind all the caption link', function() { - $('.subtitles li[data-index]').each(function(index, link) { - expect($(link)).toHandleWith('mouseover', videoCaption.captionMouseOverOut); - expect($(link)).toHandleWith('mouseout', videoCaption.captionMouseOverOut); - expect($(link)).toHandleWith('mousedown', videoCaption.captionMouseDown); - expect($(link)).toHandleWith('click', videoCaption.captionClick); - expect($(link)).toHandleWith('focus', videoCaption.captionFocus); - expect($(link)).toHandleWith('blur', videoCaption.captionBlur); - expect($(link)).toHandleWith('keydown', videoCaption.captionKeyDown); - }); - }); - - it('set rendered to true', function() { - expect(videoCaption.rendered).toBeTruthy(); - }); - }); - - describe('when on a touch-based device', function() { - beforeEach(function() { - window.onTouchBasedDevice.andReturn(true); - initialize(); - }); - - it('show explaination message', function() { - expect($('.subtitles li')).toHaveHtml("Caption will be displayed when you start playing the video."); - }); - - it('does not set rendered to true', function() { - expect(videoCaption.rendered).toBeFalsy(); - }); - }); - - describe('when no captions file was specified', function () { - beforeEach(function () { - loadFixtures('video_all.html'); - - // Unspecify the captions file. - $('#example').find('#video_id').data('sub', ''); - - state = new Video('#example'); - videoCaption = state.videoCaption; - }); - - it('captions panel is not shown', function () { - expect(videoCaption.hideSubtitlesEl).toBeHidden(); - }); - }); - }); - - describe('mouse movement', function() { - // We will store default window.setTimeout() function here. - var oldSetTimeout = null; - - beforeEach(function() { - // Store original window.setTimeout() function. If we do not do this, then - // all other tests that rely on code which uses window.setTimeout() - // function might (and probably will) fail. - oldSetTimeout = window.setTimeout; - // Redefine window.setTimeout() function as a spy. - window.setTimeout = jasmine.createSpy().andCallFake(function(callback, timeout) { return 5; }) - window.setTimeout.andReturn(100); - spyOn(window, 'clearTimeout'); - }); - - afterEach(function () { - // Reset the default window.setTimeout() function. If we do not do this, - // then all other tests that rely on code which uses window.setTimeout() - // function might (and probably will) fail. - window.setTimeout = oldSetTimeout; - }); - - describe('when cursor is outside of the caption box', function() { - beforeEach(function() { - $(window).trigger(jQuery.Event('mousemove')); - }); - - it('does not set freezing timeout', function() { - expect(videoCaption.frozen).toBeFalsy(); - }); - }); - - describe('when cursor is in the caption box', function() { - beforeEach(function() { - $('.subtitles').trigger(jQuery.Event('mouseenter')); - }); - - it('set the freezing timeout', function() { - expect(videoCaption.frozen).toEqual(100); - }); - - describe('when the cursor is moving', function() { - beforeEach(function() { - $('.subtitles').trigger(jQuery.Event('mousemove')); - }); - - it('reset the freezing timeout', function() { - expect(window.clearTimeout).toHaveBeenCalledWith(100); - }); - }); - - describe('when the mouse is scrolling', function() { - beforeEach(function() { - $('.subtitles').trigger(jQuery.Event('mousewheel')); - }); - - it('reset the freezing timeout', function() { - expect(window.clearTimeout).toHaveBeenCalledWith(100); - }); - }); - }); - - describe('when cursor is moving out of the caption box', function() { - beforeEach(function() { - videoCaption.frozen = 100; - $.fn.scrollTo.reset(); - }); - - describe('always', function() { - beforeEach(function() { - $('.subtitles').trigger(jQuery.Event('mouseout')); - }); - - it('reset the freezing timeout', function() { - expect(window.clearTimeout).toHaveBeenCalledWith(100); - }); - - it('unfreeze the caption', function() { - expect(videoCaption.frozen).toBeNull(); - }); - }); - - describe('when the player is playing', function() { - beforeEach(function() { - videoCaption.playing = true; - $('.subtitles li[data-index]:first').addClass('current'); - $('.subtitles').trigger(jQuery.Event('mouseout')); - }); - - it('scroll the caption', function() { - expect($.fn.scrollTo).toHaveBeenCalled(); - }); - }); - - describe('when the player is not playing', function() { - beforeEach(function() { - videoCaption.playing = false; - $('.subtitles').trigger(jQuery.Event('mouseout')); - }); - - it('does not scroll the caption', function() { - expect($.fn.scrollTo).not.toHaveBeenCalled(); - }); - }); - }); - }); - - describe('search', function() { - it('return a correct caption index', function() { - expect(videoCaption.search(0)).toEqual(-1); - expect(videoCaption.search(3120)).toEqual(1); - expect(videoCaption.search(6270)).toEqual(2); - expect(videoCaption.search(8490)).toEqual(2); - expect(videoCaption.search(21620)).toEqual(4); - expect(videoCaption.search(24920)).toEqual(5); - }); - }); - - describe('play', function() { - describe('when the caption was not rendered', function() { - beforeEach(function() { - window.onTouchBasedDevice.andReturn(true); - initialize(); - videoCaption.play(); - }); - - it('render the caption', function() { - var captionsData; - - captionsData = jasmine.stubbedCaption; - $('.subtitles li[data-index]').each(function(index, link) { - expect($(link)).toHaveData('index', index); - expect($(link)).toHaveData('start', captionsData.start[index]); - expect($(link)).toHaveAttr('tabindex', 0); - expect($(link)).toHaveText(captionsData.text[index]); - }); - }); - - it('add a padding element to caption', function() { - expect($('.subtitles li:first')).toBe('.spacing'); - expect($('.subtitles li:last')).toBe('.spacing'); - }); - - it('bind all the caption link', function() { - $('.subtitles li[data-index]').each(function(index, link) { - expect($(link)).toHandleWith('mouseover', videoCaption.captionMouseOverOut); - expect($(link)).toHandleWith('mouseout', videoCaption.captionMouseOverOut); - expect($(link)).toHandleWith('mousedown', videoCaption.captionMouseDown); - expect($(link)).toHandleWith('click', videoCaption.captionClick); - expect($(link)).toHandleWith('focus', videoCaption.captionFocus); - expect($(link)).toHandleWith('blur', videoCaption.captionBlur); - expect($(link)).toHandleWith('keydown', videoCaption.captionKeyDown); - }); - }); - - it('set rendered to true', function() { - expect(videoCaption.rendered).toBeTruthy(); - }); - - it('set playing to true', function() { - expect(videoCaption.playing).toBeTruthy(); - }); - }); - }); - - describe('pause', function() { - beforeEach(function() { - videoCaption.playing = true; - videoCaption.pause(); - }); - - it('set playing to false', function() { - expect(videoCaption.playing).toBeFalsy(); - }); - }); - - describe('updatePlayTime', function() { - describe('when the video speed is 1.0x', function() { - beforeEach(function() { - videoSpeedControl.currentSpeed = '1.0'; - videoCaption.updatePlayTime(25.000); - }); - - it('search the caption based on time', function() { - expect(videoCaption.currentIndex).toEqual(5); - }); - }); - - describe('when the video speed is not 1.0x', function() { - beforeEach(function() { - videoSpeedControl.currentSpeed = '0.75'; - videoCaption.updatePlayTime(25.000); - }); - - it('search the caption based on 1.0x speed', function() { - expect(videoCaption.currentIndex).toEqual(5); - }); - }); - - describe('when the index is not the same', function() { - beforeEach(function() { - videoCaption.currentIndex = 1; - $('.subtitles li[data-index=5]').addClass('current'); - videoCaption.updatePlayTime(25.000); - }); - - it('deactivate the previous caption', function() { - expect($('.subtitles li[data-index=1]')).not.toHaveClass('current'); - }); - - it('activate new caption', function() { - expect($('.subtitles li[data-index=5]')).toHaveClass('current'); - }); - - it('save new index', function() { - expect(videoCaption.currentIndex).toEqual(5); - }); - - it('scroll caption to new position', function() { - expect($.fn.scrollTo).toHaveBeenCalled(); - }); - }); - - describe('when the index is the same', function() { - beforeEach(function() { - videoCaption.currentIndex = 1; - $('.subtitles li[data-index=3]').addClass('current'); - videoCaption.updatePlayTime(15.000); - }); - - it('does not change current subtitle', function() { - expect($('.subtitles li[data-index=3]')).toHaveClass('current'); - }); - }); - }); - - describe('resize', function() { - beforeEach(function() { - initialize(); - $('.subtitles li[data-index=1]').addClass('current'); - videoCaption.resize(); - }); - - describe('set the height of caption container', function(){ - // Temporarily disabled due to intermittent failures - // with error "Expected 745 to be close to 805, 2." in Firefox - xit('when CC button is enabled', function() { - var realHeight = parseInt($('.subtitles').css('maxHeight'), 10), - shouldBeHeight = $('.video-wrapper').height(); - - // Because of some problems with rounding on different enviroments: - // Linux * Mac * FF * Chrome - expect(realHeight).toBeCloseTo(shouldBeHeight, 2); - }); - - it('when CC button is disabled ', function() { - var realHeight, videoWrapperHeight, progressSliderHeight, - controlHeight, shouldBeHeight; - - state.captionsHidden = true; - videoCaption.setSubtitlesHeight(); - - realHeight = parseInt($('.subtitles').css('maxHeight'), 10); - videoWrapperHeight = $('.video-wrapper').height(); - progressSliderHeight = videoControl.sliderEl.height(); - controlHeight = videoControl.el.height(); - shouldBeHeight = videoWrapperHeight - - 0.5 * progressSliderHeight - - controlHeight; - - expect(realHeight).toBe(shouldBeHeight); - }); - }); - - it('set the height of caption spacing', function() { - var firstSpacing, lastSpacing; - firstSpacing = Math.abs(parseInt($('.subtitles .spacing:first').css('height'), 10)); - lastSpacing = Math.abs(parseInt($('.subtitles .spacing:last').css('height'), 10)); - expect(firstSpacing - videoCaption.topSpacingHeight()).toBeLessThan(1); - expect(lastSpacing - videoCaption.bottomSpacingHeight()).toBeLessThan(1); - }); - - it('scroll caption to new position', function() { - expect($.fn.scrollTo).toHaveBeenCalled(); - }); - }); - - describe('scrollCaption', function() { - beforeEach(function() { - initialize(); - }); - - describe('when frozen', function() { - beforeEach(function() { - videoCaption.frozen = true; - $('.subtitles li[data-index=1]').addClass('current'); - videoCaption.scrollCaption(); - }); - - it('does not scroll the caption', function() { - expect($.fn.scrollTo).not.toHaveBeenCalled(); - }); - }); - - describe('when not frozen', function() { - beforeEach(function() { - videoCaption.frozen = false; - }); - - describe('when there is no current caption', function() { - beforeEach(function() { - videoCaption.scrollCaption(); - }); - - it('does not scroll the caption', function() { - expect($.fn.scrollTo).not.toHaveBeenCalled(); - }); - }); - - describe('when there is a current caption', function() { - beforeEach(function() { - $('.subtitles li[data-index=1]').addClass('current'); - videoCaption.scrollCaption(); - }); - - it('scroll to current caption', function() { - expect($.fn.scrollTo).toHaveBeenCalled(); - }); - }); - }); - }); - - describe('seekPlayer', function() { - describe('when the video speed is 1.0x', function() { - beforeEach(function() { - videoSpeedControl.currentSpeed = '1.0'; - $('.subtitles li[data-start="14910"]').trigger('click'); - }); - - // Temporarily disabled due to intermittent failures - // Fails with error: "InvalidStateError: An attempt was made to - // use an object that is not, or is no longer, usable - // Expected 0 to equal 14.91." - // on Firefox - xit('trigger seek event with the correct time', function() { - expect(videoPlayer.currentTime).toEqual(14.91); - }); - }); - - describe('when the video speed is not 1.0x', function() { - beforeEach(function() { - initialize(); - videoSpeedControl.currentSpeed = '0.75'; - $('.subtitles li[data-start="14910"]').trigger('click'); - }); - - it('trigger seek event with the correct time', function() { - expect(videoPlayer.currentTime).toEqual(14.91); - }); - }); +(function () { + describe('VideoCaption', function () { + var state, videoPlayer, videoCaption, videoSpeedControl, oldOTBD; + + function initialize() { + loadFixtures('video_all.html'); + state = new Video('#example'); + videoPlayer = state.videoPlayer; + videoCaption = state.videoCaption; + videoSpeedControl = state.videoSpeedControl; + videoControl = state.videoControl; + } - describe('when the player type is Flash at speed 0.75x', function () { beforeEach(function () { + oldOTBD = window.onTouchBasedDevice; + window.onTouchBasedDevice = jasmine.createSpy('onTouchBasedDevice') + .andReturn(false); initialize(); - videoSpeedControl.currentSpeed = '0.75'; - state.currentPlayerMode = 'flash'; - $('.subtitles li[data-start="14910"]').trigger('click'); - }); - - it('trigger seek event with the correct time', function () { - expect(videoPlayer.currentTime).toEqual(15); - }); - }); - }); - - describe('toggle', function() { - beforeEach(function() { - initialize(); - spyOn(videoPlayer, 'log'); - $('.subtitles li[data-index=1]').addClass('current'); - }); - - describe('when the caption is visible', function() { - beforeEach(function() { - state.el.removeClass('closed'); - videoCaption.toggle(jQuery.Event('click')); - }); - - it('log the hide_transcript event', function() { - expect(videoPlayer.log).toHaveBeenCalledWith('hide_transcript', { - currentTime: videoPlayer.currentTime - }); - }); - - it('hide the caption', function() { - expect(state.el).toHaveClass('closed'); - }); - }); - - describe('when the caption is hidden', function() { - beforeEach(function() { - state.el.addClass('closed'); - videoCaption.toggle(jQuery.Event('click')); - }); - - it('log the show_transcript event', function() { - expect(videoPlayer.log).toHaveBeenCalledWith('show_transcript', { - currentTime: videoPlayer.currentTime - }); - }); - - it('show the caption', function() { - expect(state.el).not.toHaveClass('closed'); - }); - - it('scroll the caption', function() { - expect($.fn.scrollTo).toHaveBeenCalled(); - }); - }); - }); - - describe('caption accessibility', function() { - beforeEach(function() { - initialize(); - }); - - describe('when getting focus through TAB key', function() { - beforeEach(function() { - videoCaption.isMouseFocus = false; - $('.subtitles li[data-index=0]').trigger(jQuery.Event('focus')); - }); - - it('shows an outline around the caption', function() { - expect($('.subtitles li[data-index=0]')).toHaveClass('focused'); - }); - - it('has automatic scrolling disabled', function() { - expect(videoCaption.autoScrolling).toBe(false); - }); - }); - - describe('when loosing focus through TAB key', function() { - beforeEach(function() { - $('.subtitles li[data-index=0]').trigger(jQuery.Event('blur')); - }); - - it('does not show an outline around the caption', function() { - expect($('.subtitles li[data-index=0]')).not.toHaveClass('focused'); - }); - - it('has automatic scrolling enabled', function() { - expect(videoCaption.autoScrolling).toBe(true); - }); - }); - - describe('when same caption gets the focus through mouse after having focus through TAB key', function() { - beforeEach(function() { - videoCaption.isMouseFocus = false; - $('.subtitles li[data-index=0]').trigger(jQuery.Event('focus')); - $('.subtitles li[data-index=0]').trigger(jQuery.Event('mousedown')); - }); - - it('does not show an outline around it', function() { - expect($('.subtitles li[data-index=0]')).not.toHaveClass('focused'); - }); - - it('has automatic scrolling enabled', function() { - expect(videoCaption.autoScrolling).toBe(true); - }); - }); - - describe('when a second caption gets focus through mouse after first had focus through TAB key', function() { - beforeEach(function() { - videoCaption.isMouseFocus = false; - $('.subtitles li[data-index=0]').trigger(jQuery.Event('focus')); - $('.subtitles li[data-index=0]').trigger(jQuery.Event('blur')); - videoCaption.isMouseFocus = true; - $('.subtitles li[data-index=1]').trigger(jQuery.Event('mousedown')); - }); - - it('does not show an outline around the first', function() { - expect($('.subtitles li[data-index=0]')).not.toHaveClass('focused'); - }); - - it('does not show an outline around the second', function() { - expect($('.subtitles li[data-index=1]')).not.toHaveClass('focused'); - }); - - it('has automatic scrolling enabled', function() { - expect(videoCaption.autoScrolling).toBe(true); - }); - }); - - xdescribe('when enter key is pressed on a caption', function() { - beforeEach(function() { - var e; - spyOn(videoCaption, 'seekPlayer').andCallThrough(); - videoCaption.isMouseFocus = false; - $('.subtitles li[data-index=0]').trigger(jQuery.Event('focus')); - e = jQuery.Event('keydown'); - e.which = 13; // ENTER key - $('.subtitles li[data-index=0]').trigger(e); - }); - - // Temporarily disabled due to intermittent failures - // Fails with error: "InvalidStateError: InvalidStateError: An attempt - // was made to use an object that is not, or is no longer, usable" - xit('shows an outline around it', function() { - expect($('.subtitles li[data-index=0]')).toHaveClass('focused'); }); - xit('calls seekPlayer', function() { - expect(videoCaption.seekPlayer).toHaveBeenCalled(); + afterEach(function () { + YT.Player = undefined; + $.fn.scrollTo.reset(); + $('.subtitles').remove(); + $('source').remove(); + window.onTouchBasedDevice = oldOTBD; + }); + + describe('constructor', function () { + describe('always', function () { + beforeEach(function () { + spyOn($, 'ajaxWithPrefix').andCallThrough(); + initialize(); + }); + + it('create the caption element', function () { + expect($('.video')).toContain('ol.subtitles'); + }); + + it('add caption control to video player', function () { + expect($('.video')).toContain('a.hide-subtitles'); + }); + + it('fetch the caption', function () { + waitsFor(function () { + if (videoCaption.loaded === true) { + return true; + } + + return false; + }, 'Expect captions to be loaded.', 1000); + + runs(function () { + expect($.ajaxWithPrefix).toHaveBeenCalledWith({ + url: videoCaption.captionURL(), + notifyOnError: false, + success: jasmine.any(Function), + error: jasmine.any(Function) + }); + }); + }); + + it('bind window resize event', function () { + expect($(window)).toHandleWith( + 'resize', videoCaption.resize + ); + }); + + it('bind the hide caption button', function () { + expect($('.hide-subtitles')).toHandleWith( + 'click', videoCaption.toggle + ); + }); + + it('bind the mouse movement', function () { + expect($('.subtitles')).toHandleWith( + 'mouseover', videoCaption.onMouseEnter + ); + expect($('.subtitles')).toHandleWith( + 'mouseout', videoCaption.onMouseLeave + ); + expect($('.subtitles')).toHandleWith( + 'mousemove', videoCaption.onMovement + ); + expect($('.subtitles')).toHandleWith( + 'mousewheel', videoCaption.onMovement + ); + expect($('.subtitles')).toHandleWith( + 'DOMMouseScroll', videoCaption.onMovement + ); + }); + + it('bind the scroll', function () { + expect($('.subtitles')) + .toHandleWith('scroll', videoCaption.autoShowCaptions); + expect($('.subtitles')) + .toHandleWith('scroll', videoControl.showControls); + }); + }); + + describe('when on a non touch-based device', function () { + beforeEach(function () { + initialize(); + }); + + it('render the caption', function () { + var captionsData; + + captionsData = jasmine.stubbedCaption; + $('.subtitles li[data-index]').each( + function (index, link) { + + expect($(link)).toHaveData('index', index); + expect($(link)).toHaveData( + 'start', captionsData.start[index] + ); + expect($(link)).toHaveAttr('tabindex', 0); + expect($(link)).toHaveText(captionsData.text[index]); + }); + }); + + it('add a padding element to caption', function () { + expect($('.subtitles li:first').hasClass('spacing')) + .toBe(true); + expect($('.subtitles li:last').hasClass('spacing')) + .toBe(true); + }); + + it('bind all the caption link', function () { + $('.subtitles li[data-index]').each( + function (index, link) { + + expect($(link)).toHandleWith( + 'mouseover', videoCaption.captionMouseOverOut + ); + expect($(link)).toHandleWith( + 'mouseout', videoCaption.captionMouseOverOut + ); + expect($(link)).toHandleWith( + 'mousedown', videoCaption.captionMouseDown + ); + expect($(link)).toHandleWith( + 'click', videoCaption.captionClick + ); + expect($(link)).toHandleWith( + 'focus', videoCaption.captionFocus + ); + expect($(link)).toHandleWith( + 'blur', videoCaption.captionBlur + ); + expect($(link)).toHandleWith( + 'keydown', videoCaption.captionKeyDown + ); + }); + }); + + it('set rendered to true', function () { + expect(videoCaption.rendered).toBeTruthy(); + }); + }); + + describe('when on a touch-based device', function () { + beforeEach(function () { + window.onTouchBasedDevice.andReturn(true); + initialize(); + }); + + it('show explaination message', function () { + expect($('.subtitles li')).toHaveHtml( + 'Caption will be displayed when you start playing ' + + 'the video.' + ); + }); + + it('does not set rendered to true', function () { + expect(videoCaption.rendered).toBeFalsy(); + }); + }); + + describe('when no captions file was specified', function () { + beforeEach(function () { + loadFixtures('video_all.html'); + + // Unspecify the captions file. + $('#example').find('#video_id').data('sub', ''); + + state = new Video('#example'); + videoCaption = state.videoCaption; + }); + + it('captions panel is not shown', function () { + expect(videoCaption.hideSubtitlesEl).toBeHidden(); + }); + }); + }); + + describe('mouse movement', function () { + // We will store default window.setTimeout() function here. + var oldSetTimeout = null; + + beforeEach(function () { + // Store original window.setTimeout() function. If we do not do + // this, then all other tests that rely on code which uses + // window.setTimeout() function might (and probably will) fail. + oldSetTimeout = window.setTimeout; + // Redefine window.setTimeout() function as a spy. + window.setTimeout = jasmine.createSpy().andCallFake( + function (callback, timeout) { + return 5; + } + ); + window.setTimeout.andReturn(100); + spyOn(window, 'clearTimeout'); + }); + + afterEach(function () { + // Reset the default window.setTimeout() function. If we do not + // do this, then all other tests that rely on code which uses + // window.setTimeout() function might (and probably will) fail. + window.setTimeout = oldSetTimeout; + }); + + describe('when cursor is outside of the caption box', function () { + beforeEach(function () { + $(window).trigger(jQuery.Event('mousemove')); + }); + + it('does not set freezing timeout', function () { + expect(videoCaption.frozen).toBeFalsy(); + }); + }); + + describe('when cursor is in the caption box', function () { + beforeEach(function () { + $('.subtitles').trigger(jQuery.Event('mouseenter')); + }); + + it('set the freezing timeout', function () { + expect(videoCaption.frozen).toEqual(100); + }); + + describe('when the cursor is moving', function () { + beforeEach(function () { + $('.subtitles').trigger(jQuery.Event('mousemove')); + }); + + it('reset the freezing timeout', function () { + expect(window.clearTimeout).toHaveBeenCalledWith(100); + }); + }); + + describe('when the mouse is scrolling', function () { + beforeEach(function () { + $('.subtitles').trigger(jQuery.Event('mousewheel')); + }); + + it('reset the freezing timeout', function () { + expect(window.clearTimeout).toHaveBeenCalledWith(100); + }); + }); + }); + + describe( + 'when cursor is moving out of the caption box', + function () { + + beforeEach(function () { + videoCaption.frozen = 100; + $.fn.scrollTo.reset(); + }); + + describe('always', function () { + beforeEach(function () { + $('.subtitles').trigger(jQuery.Event('mouseout')); + }); + + it('reset the freezing timeout', function () { + expect(window.clearTimeout).toHaveBeenCalledWith(100); + }); + + it('unfreeze the caption', function () { + expect(videoCaption.frozen).toBeNull(); + }); + }); + + describe('when the player is playing', function () { + beforeEach(function () { + videoCaption.playing = true; + $('.subtitles li[data-index]:first') + .addClass('current'); + $('.subtitles').trigger(jQuery.Event('mouseout')); + }); + + it('scroll the caption', function () { + expect($.fn.scrollTo).toHaveBeenCalled(); + }); + }); + + describe('when the player is not playing', function () { + beforeEach(function () { + videoCaption.playing = false; + $('.subtitles').trigger(jQuery.Event('mouseout')); + }); + + it('does not scroll the caption', function () { + expect($.fn.scrollTo).not.toHaveBeenCalled(); + }); + }); + }); + }); + + describe('search', function () { + it('return a correct caption index', function () { + expect(videoCaption.search(0)).toEqual(-1); + expect(videoCaption.search(3120)).toEqual(1); + expect(videoCaption.search(6270)).toEqual(2); + expect(videoCaption.search(8490)).toEqual(2); + expect(videoCaption.search(21620)).toEqual(4); + expect(videoCaption.search(24920)).toEqual(5); + }); + }); + + describe('play', function () { + describe('when the caption was not rendered', function () { + beforeEach(function () { + window.onTouchBasedDevice.andReturn(true); + initialize(); + videoCaption.play(); + }); + + it('render the caption', function () { + var captionsData; + + captionsData = jasmine.stubbedCaption; + $('.subtitles li[data-index]').each( + function (index, link) { + + expect($(link)).toHaveData('index', index); + expect($(link)).toHaveData( + 'start', captionsData.start[index] + ); + expect($(link)).toHaveAttr('tabindex', 0); + expect($(link)).toHaveText(captionsData.text[index]); + }); + }); + + it('add a padding element to caption', function () { + expect($('.subtitles li:first')).toBe('.spacing'); + expect($('.subtitles li:last')).toBe('.spacing'); + }); + + it('bind all the caption link', function () { + $('.subtitles li[data-index]').each( + function (index, link) { + + expect($(link)).toHandleWith( + 'mouseover', videoCaption.captionMouseOverOut + ); + expect($(link)).toHandleWith( + 'mouseout', videoCaption.captionMouseOverOut + ); + expect($(link)).toHandleWith( + 'mousedown', videoCaption.captionMouseDown + ); + expect($(link)).toHandleWith( + 'click', videoCaption.captionClick + ); + expect($(link)).toHandleWith( + 'focus', videoCaption.captionFocus + ); + expect($(link)).toHandleWith( + 'blur', videoCaption.captionBlur + ); + expect($(link)).toHandleWith( + 'keydown', videoCaption.captionKeyDown + ); + }); + }); + + it('set rendered to true', function () { + expect(videoCaption.rendered).toBeTruthy(); + }); + + it('set playing to true', function () { + expect(videoCaption.playing).toBeTruthy(); + }); + }); + }); + + describe('pause', function () { + beforeEach(function () { + videoCaption.playing = true; + videoCaption.pause(); + }); + + it('set playing to false', function () { + expect(videoCaption.playing).toBeFalsy(); + }); + }); + + describe('updatePlayTime', function () { + describe('when the video speed is 1.0x', function () { + beforeEach(function () { + videoSpeedControl.currentSpeed = '1.0'; + videoCaption.updatePlayTime(25.000); + }); + + it('search the caption based on time', function () { + expect(videoCaption.currentIndex).toEqual(5); + }); + }); + + describe('when the video speed is not 1.0x', function () { + beforeEach(function () { + videoSpeedControl.currentSpeed = '0.75'; + videoCaption.updatePlayTime(25.000); + }); + + it('search the caption based on 1.0x speed', function () { + expect(videoCaption.currentIndex).toEqual(5); + }); + }); + + describe('when the index is not the same', function () { + beforeEach(function () { + videoCaption.currentIndex = 1; + $('.subtitles li[data-index=5]').addClass('current'); + videoCaption.updatePlayTime(25.000); + }); + + it('deactivate the previous caption', function () { + expect($('.subtitles li[data-index=1]')) + .not.toHaveClass('current'); + }); + + it('activate new caption', function () { + expect($('.subtitles li[data-index=5]')) + .toHaveClass('current'); + }); + + it('save new index', function () { + expect(videoCaption.currentIndex).toEqual(5); + }); + + it('scroll caption to new position', function () { + expect($.fn.scrollTo).toHaveBeenCalled(); + }); + }); + + describe('when the index is the same', function () { + beforeEach(function () { + videoCaption.currentIndex = 1; + $('.subtitles li[data-index=3]').addClass('current'); + videoCaption.updatePlayTime(15.000); + }); + + it('does not change current subtitle', function () { + expect($('.subtitles li[data-index=3]')) + .toHaveClass('current'); + }); + }); + }); + + describe('resize', function () { + beforeEach(function () { + initialize(); + $('.subtitles li[data-index=1]').addClass('current'); + videoCaption.resize(); + }); + + describe('set the height of caption container', function () { + // Temporarily disabled due to intermittent failures + // with error "Expected 745 to be close to 805, 2." in Firefox + xit('when CC button is enabled', function () { + var realHeight = parseInt( + $('.subtitles').css('maxHeight'), 10 + ), + shouldBeHeight = $('.video-wrapper').height(); + + // Because of some problems with rounding on different + // environments: Linux * Mac * FF * Chrome + expect(realHeight).toBeCloseTo(shouldBeHeight, 2); + }); + + it('when CC button is disabled ', function () { + var realHeight, videoWrapperHeight, progressSliderHeight, + controlHeight, shouldBeHeight; + + state.captionsHidden = true; + videoCaption.setSubtitlesHeight(); + + realHeight = parseInt( + $('.subtitles').css('maxHeight'), 10 + ); + videoWrapperHeight = $('.video-wrapper').height(); + progressSliderHeight = videoControl.sliderEl.height(); + controlHeight = videoControl.el.height(); + shouldBeHeight = videoWrapperHeight - + 0.5 * progressSliderHeight - + controlHeight; + + expect(realHeight).toBe(shouldBeHeight); + }); + }); + + it('set the height of caption spacing', function () { + var firstSpacing, lastSpacing; + + firstSpacing = Math.abs(parseInt( + $('.subtitles .spacing:first').css('height'), 10 + )); + lastSpacing = Math.abs(parseInt( + $('.subtitles .spacing:last').css('height'), 10 + )); + + expect(firstSpacing - videoCaption.topSpacingHeight()) + .toBeLessThan(1); + expect(lastSpacing - videoCaption.bottomSpacingHeight()) + .toBeLessThan(1); + }); + + it('scroll caption to new position', function () { + expect($.fn.scrollTo).toHaveBeenCalled(); + }); + }); + + describe('scrollCaption', function () { + beforeEach(function () { + initialize(); + }); + + describe('when frozen', function () { + beforeEach(function () { + videoCaption.frozen = true; + $('.subtitles li[data-index=1]').addClass('current'); + videoCaption.scrollCaption(); + }); + + it('does not scroll the caption', function () { + expect($.fn.scrollTo).not.toHaveBeenCalled(); + }); + }); + + describe('when not frozen', function () { + beforeEach(function () { + videoCaption.frozen = false; + }); + + describe('when there is no current caption', function () { + beforeEach(function () { + videoCaption.scrollCaption(); + }); + + it('does not scroll the caption', function () { + expect($.fn.scrollTo).not.toHaveBeenCalled(); + }); + }); + + describe('when there is a current caption', function () { + beforeEach(function () { + $('.subtitles li[data-index=1]').addClass('current'); + videoCaption.scrollCaption(); + }); + + it('scroll to current caption', function () { + expect($.fn.scrollTo).toHaveBeenCalled(); + }); + }); + }); + }); + + describe('seekPlayer', function () { + describe('when the video speed is 1.0x', function () { + beforeEach(function () { + videoSpeedControl.currentSpeed = '1.0'; + $('.subtitles li[data-start="14910"]').trigger('click'); + }); + + // Temporarily disabled due to intermittent failures + // Fails with error: "InvalidStateError: An attempt was made to + // use an object that is not, or is no longer, usable + // Expected 0 to equal 14.91." + // on Firefox + xit('trigger seek event with the correct time', function () { + expect(videoPlayer.currentTime).toEqual(14.91); + }); + }); + + describe('when the video speed is not 1.0x', function () { + beforeEach(function () { + initialize(); + videoSpeedControl.currentSpeed = '0.75'; + $('.subtitles li[data-start="14910"]').trigger('click'); + }); + + it('trigger seek event with the correct time', function () { + expect(videoPlayer.currentTime).toEqual(14.91); + }); + }); + + describe('when the player type is Flash at speed 0.75x', + function () { + + beforeEach(function () { + initialize(); + videoSpeedControl.currentSpeed = '0.75'; + state.currentPlayerMode = 'flash'; + $('.subtitles li[data-start="14910"]').trigger('click'); + }); + + it('trigger seek event with the correct time', function () { + expect(videoPlayer.currentTime).toEqual(15); + }); + }); + }); + + describe('toggle', function () { + beforeEach(function () { + initialize(); + spyOn(videoPlayer, 'log'); + $('.subtitles li[data-index=1]').addClass('current'); + }); + + describe('when the caption is visible', function () { + beforeEach(function () { + state.el.removeClass('closed'); + videoCaption.toggle(jQuery.Event('click')); + }); + + it('log the hide_transcript event', function () { + expect(videoPlayer.log).toHaveBeenCalledWith( + 'hide_transcript', + { + currentTime: videoPlayer.currentTime + } + ); + }); + + it('hide the caption', function () { + expect(state.el).toHaveClass('closed'); + }); + }); + + describe('when the caption is hidden', function () { + beforeEach(function () { + state.el.addClass('closed'); + videoCaption.toggle(jQuery.Event('click')); + }); + + it('log the show_transcript event', function () { + expect(videoPlayer.log).toHaveBeenCalledWith( + 'show_transcript', + { + currentTime: videoPlayer.currentTime + } + ); + }); + + it('show the caption', function () { + expect(state.el).not.toHaveClass('closed'); + }); + + it('scroll the caption', function () { + expect($.fn.scrollTo).toHaveBeenCalled(); + }); + }); + }); + + describe('caption accessibility', function () { + beforeEach(function () { + initialize(); + }); + + describe('when getting focus through TAB key', function () { + beforeEach(function () { + videoCaption.isMouseFocus = false; + $('.subtitles li[data-index=0]').trigger( + jQuery.Event('focus') + ); + }); + + it('shows an outline around the caption', function () { + expect($('.subtitles li[data-index=0]')) + .toHaveClass('focused'); + }); + + it('has automatic scrolling disabled', function () { + expect(videoCaption.autoScrolling).toBe(false); + }); + }); + + describe('when loosing focus through TAB key', function () { + beforeEach(function () { + $('.subtitles li[data-index=0]').trigger( + jQuery.Event('blur') + ); + }); + + it('does not show an outline around the caption', function () { + expect($('.subtitles li[data-index=0]')) + .not.toHaveClass('focused'); + }); + + it('has automatic scrolling enabled', function () { + expect(videoCaption.autoScrolling).toBe(true); + }); + }); + + describe( + 'when same caption gets the focus through mouse after ' + + 'having focus through TAB key', + function () { + + beforeEach(function () { + videoCaption.isMouseFocus = false; + $('.subtitles li[data-index=0]') + .trigger(jQuery.Event('focus')); + $('.subtitles li[data-index=0]') + .trigger(jQuery.Event('mousedown')); + }); + + it('does not show an outline around it', function () { + expect($('.subtitles li[data-index=0]')) + .not.toHaveClass('focused'); + }); + + it('has automatic scrolling enabled', function () { + expect(videoCaption.autoScrolling).toBe(true); + }); + }); + + describe( + 'when a second caption gets focus through mouse after ' + + 'first had focus through TAB key', + function () { + + var subDataLiIdx__0, subDataLiIdx__1; + + beforeEach(function () { + subDataLiIdx__0 = $('.subtitles li[data-index=0]'); + subDataLiIdx__1 = $('.subtitles li[data-index=1]'); + + videoCaption.isMouseFocus = false; + + subDataLiIdx__0.trigger(jQuery.Event('focus')); + subDataLiIdx__0.trigger(jQuery.Event('blur')); + + videoCaption.isMouseFocus = true; + + subDataLiIdx__1.trigger(jQuery.Event('mousedown')); + }); + + it('does not show an outline around the first', function () { + expect(subDataLiIdx__0).not.toHaveClass('focused'); + }); + + it('does not show an outline around the second', function () { + expect(subDataLiIdx__1).not.toHaveClass('focused'); + }); + + it('has automatic scrolling enabled', function () { + expect(videoCaption.autoScrolling).toBe(true); + }); + }); + + xdescribe('when enter key is pressed on a caption', function () { + var subDataLiIdx__0; + + beforeEach(function () { + var e; + + subDataLiIdx__0 = $('.subtitles li[data-index=0]'); + + spyOn(videoCaption, 'seekPlayer').andCallThrough(); + videoCaption.isMouseFocus = false; + subDataLiIdx__0.trigger(jQuery.Event('focus')); + e = jQuery.Event('keydown'); + e.which = 13; // ENTER key + subDataLiIdx__0.trigger(e); + }); + + // Temporarily disabled due to intermittent failures. + // + // Fails with error: "InvalidStateError: InvalidStateError: An + // attempt was made to use an object that is not, or is no + // longer, usable". + xit('shows an outline around it', function () { + expect(subDataLiIdx__0).toHaveClass('focused'); + }); + + xit('calls seekPlayer', function () { + expect(videoCaption.seekPlayer).toHaveBeenCalled(); + }); + }); }); - }); }); - }); }).call(this); diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js index 7f8057c02523..845d79ddb9bd 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -264,15 +264,21 @@ function (VideoPlayer) { // The function set initial configuration and preparation. function initialize(element) { - var _this = this, tempYtTestTimeout; + var _this = this, + regExp = /^true$/i, + data, tempYtTestTimeout; // This is used in places where we instead would have to check if an // element has a CSS class 'fullscreen'. this.isFullScreen = false; // The parent element of the video, and the ID. this.el = $(element).find('.video'); + this.elVideoWrapper = this.el.find('.video-wrapper'); this.id = this.el.attr('id').replace(/video_/, ''); + // jQuery .data() return object with keys in lower camelCase format. + data = this.el.data(); + console.log( '[Video info]: Initializing video with id "' + this.id + '".' ); @@ -283,32 +289,26 @@ function (VideoPlayer) { this.config = { element: element, - start: this.el.data('start'), - end: this.el.data('end'), - - caption_data_dir: this.el.data('caption-data-dir'), - caption_asset_path: this.el.data('caption-asset-path'), - show_captions: ( - this.el.data('show-captions') - .toString().toLowerCase() === 'true' - ), - youtubeStreams: this.el.data('streams'), - - sub: this.el.data('sub'), - mp4Source: this.el.data('mp4-source'), - webmSource: this.el.data('webm-source'), - oggSource: this.el.data('ogg-source'), - - ytTestUrl: this.el.data('yt-test-url'), - + start: data['start'], + end: data['end'], + caption_data_dir: data['captionDataDir'], + caption_asset_path: data['captionAssetPath'], + show_captions: regExp.test(data['showCaptions'].toString()), + youtubeStreams: data['streams'], + autohideHtml5: regExp.test(data['autohideHtml5'].toString()), + sub: data['sub'], + mp4Source: data['mp4Source'], + webmSource: data['webmSource'], + oggSource: data['oggSource'], + ytTestUrl: data['ytTestUrl'], fadeOutTimeout: 1400, - + captionsFreezeTime: 10000, availableQualities: ['hd720', 'hd1080', 'highres'] }; // Check if the YT test timeout has been set. If not, or it is in // improper format, then set to default value. - tempYtTestTimeout = parseInt(this.el.data('yt-test-timeout'), 10); + tempYtTestTimeout = parseInt(data['ytTestTimeout'], 10); if (!isFinite(tempYtTestTimeout)) { tempYtTestTimeout = 1500; } diff --git a/common/lib/xmodule/xmodule/js/src/video/04_video_control.js b/common/lib/xmodule/xmodule/js/src/video/04_video_control.js index 796ba07060d5..8ba7490cefd9 100644 --- a/common/lib/xmodule/xmodule/js/src/video/04_video_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/04_video_control.js @@ -57,7 +57,7 @@ function () { state.videoControl.play(); } - if (state.videoType === 'html5') { + if ((state.videoType === 'html5') && (state.config.autohideHtml5)) { state.videoControl.fadeOutTimeout = state.config.fadeOutTimeout; state.videoControl.el.addClass('html5'); @@ -81,7 +81,7 @@ function () { state.videoControl.fullScreenEl.on('click', state.videoControl.toggleFullScreen); $(document).on('keyup', state.videoControl.exitFullScreen); - if (state.videoType === 'html5') { + if ((state.videoType === 'html5') && (state.config.autohideHtml5)) { state.el.on('mousemove', state.videoControl.showControls); state.el.on('keydown', state.videoControl.showControls); } diff --git a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js index b97717e97c54..f69bcc160050 100644 --- a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js +++ b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js @@ -34,46 +34,63 @@ function () { // function _makeFunctionsPublic(state) // - // Functions which will be accessible via 'state' object. When called, these functions will - // get the 'state' object as a context. + // Functions which will be accessible via 'state' object. When called, + // these functions will get the 'state' object as a context. function _makeFunctionsPublic(state) { - state.videoCaption.autoShowCaptions = _.bind(autoShowCaptions, state); - state.videoCaption.autoHideCaptions = _.bind(autoHideCaptions, state); - state.videoCaption.resize = _.bind(resize, state); - state.videoCaption.toggle = _.bind(toggle, state); - state.videoCaption.onMouseEnter = _.bind(onMouseEnter, state); - state.videoCaption.onMouseLeave = _.bind(onMouseLeave, state); - state.videoCaption.onMovement = _.bind(onMovement, state); - state.videoCaption.renderCaption = _.bind(renderCaption, state); - state.videoCaption.captionHeight = _.bind(captionHeight, state); - state.videoCaption.topSpacingHeight = _.bind(topSpacingHeight, state); - state.videoCaption.bottomSpacingHeight = _.bind(bottomSpacingHeight, state); - state.videoCaption.scrollCaption = _.bind(scrollCaption, state); - state.videoCaption.search = _.bind(search, state); - state.videoCaption.play = _.bind(play, state); - state.videoCaption.pause = _.bind(pause, state); - state.videoCaption.seekPlayer = _.bind(seekPlayer, state); - state.videoCaption.hideCaptions = _.bind(hideCaptions, state); - state.videoCaption.calculateOffset = _.bind(calculateOffset, state); - state.videoCaption.updatePlayTime = _.bind(updatePlayTime, state); - state.videoCaption.setSubtitlesHeight = _.bind(setSubtitlesHeight, state); - - state.videoCaption.renderElements = _.bind(renderElements, state); - state.videoCaption.bindHandlers = _.bind(bindHandlers, state); - state.videoCaption.fetchCaption = _.bind(fetchCaption, state); - state.videoCaption.captionURL = _.bind(captionURL, state); - state.videoCaption.captionMouseOverOut = _.bind(captionMouseOverOut, state); - state.videoCaption.captionMouseDown = _.bind(captionMouseDown, state); - state.videoCaption.captionClick = _.bind(captionClick, state); - state.videoCaption.captionFocus = _.bind(captionFocus, state); - state.videoCaption.captionBlur = _.bind(captionBlur, state); - state.videoCaption.captionKeyDown = _.bind(captionKeyDown, state); + state.videoCaption.autoShowCaptions = _.bind( + autoShowCaptions, state + ); + state.videoCaption.autoHideCaptions = _.bind( + autoHideCaptions, state + ); + state.videoCaption.resize = _.bind(resize, state); + state.videoCaption.toggle = _.bind(toggle, state); + state.videoCaption.onMouseEnter = _.bind(onMouseEnter, state); + state.videoCaption.onMouseLeave = _.bind(onMouseLeave, state); + state.videoCaption.onMovement = _.bind(onMovement, state); + state.videoCaption.renderCaption = _.bind(renderCaption, state); + state.videoCaption.captionHeight = _.bind(captionHeight, state); + state.videoCaption.topSpacingHeight = _.bind( + topSpacingHeight, state + ); + state.videoCaption.bottomSpacingHeight = _.bind( + bottomSpacingHeight, state + ); + state.videoCaption.scrollCaption = _.bind(scrollCaption, state); + state.videoCaption.search = _.bind(search, state); + state.videoCaption.play = _.bind(play, state); + state.videoCaption.pause = _.bind(pause, state); + state.videoCaption.seekPlayer = _.bind(seekPlayer, state); + state.videoCaption.hideCaptions = _.bind(hideCaptions, state); + state.videoCaption.calculateOffset = _.bind( + calculateOffset, state + ); + state.videoCaption.updatePlayTime = _.bind(updatePlayTime, state); + state.videoCaption.setSubtitlesHeight = _.bind( + setSubtitlesHeight, state + ); + + state.videoCaption.renderElements = _.bind(renderElements, state); + state.videoCaption.bindHandlers = _.bind(bindHandlers, state); + state.videoCaption.fetchCaption = _.bind(fetchCaption, state); + state.videoCaption.captionURL = _.bind(captionURL, state); + state.videoCaption.captionMouseOverOut = _.bind( + captionMouseOverOut, state + ); + state.videoCaption.captionMouseDown = _.bind( + captionMouseDown, state + ); + state.videoCaption.captionClick = _.bind(captionClick, state); + state.videoCaption.captionFocus = _.bind(captionFocus, state); + state.videoCaption.captionBlur = _.bind(captionBlur, state); + state.videoCaption.captionKeyDown = _.bind(captionKeyDown, state); } // *************************************************************** // Public functions start here. - // These are available via the 'state' object. Their context ('this' keyword) is the 'state' object. - // The magic private function that makes them available and sets up their context is makeFunctionsPublic(). + // These are available via the 'state' object. Their context ('this' + // keyword) is the 'state' object. The magic private function that makes + // them available and sets up their context is makeFunctionsPublic(). // *************************************************************** /** @@ -109,10 +126,13 @@ function () { // function bindHandlers() // - // Bind any necessary function callbacks to DOM events (click, mousemove, etc.). + // Bind any necessary function callbacks to DOM events (click, + // mousemove, etc.). function bindHandlers() { $(window).bind('resize', this.videoCaption.resize); - this.videoCaption.hideSubtitlesEl.on('click', this.videoCaption.toggle); + this.videoCaption.hideSubtitlesEl.on( + 'click', this.videoCaption.toggle + ); this.videoCaption.subtitlesEl .on( @@ -132,14 +152,41 @@ function () { this.videoCaption.onMovement ); - if (this.videoType === 'html5') { - this.el.on('mousemove', this.videoCaption.autoShowCaptions); - this.el.on('keydown', this.videoCaption.autoShowCaptions); + if ((this.videoType === 'html5') && (this.config.autohideHtml5)) { + this.el.on({ + mousemove: this.videoCaption.autoShowCaptions, + keydown: this.videoCaption.autoShowCaptions + }); - // Moving slider on subtitles is not a mouse move, - // but captions and controls should be showed. - this.videoCaption.subtitlesEl.on('scroll', this.videoCaption.autoShowCaptions); - this.videoCaption.subtitlesEl.on('scroll', this.videoControl.showControls); + // Moving slider on subtitles is not a mouse move, but captions and + // controls should be shown. + this.videoCaption.subtitlesEl + .on( + 'scroll', this.videoCaption.autoShowCaptions + ) + .on( + 'scroll', this.videoControl.showControls + ); + } else if (!this.config.autohideHtml5) { + this.videoCaption.subtitlesEl.on({ + keydown: this.videoCaption.autoShowCaptions, + focus: this.videoCaption.autoShowCaptions, + + // Moving slider on subtitles is not a mouse move, but captions + // should not be auto-hidden. + scroll: this.videoCaption.autoShowCaptions, + + mouseout: this.videoCaption.autoHideCaptions, + blur: this.videoCaption.autoHideCaptions + }); + + this.videoCaption.hideSubtitlesEl.on({ + mousemove: this.videoCaption.autoShowCaptions, + focus: this.videoCaption.autoShowCaptions, + + mouseout: this.videoCaption.autoHideCaptions, + blur: this.videoCaption.autoHideCaptions + }); } } @@ -209,7 +256,8 @@ function () { } function captionURL() { - return '' + this.config.caption_asset_path + this.youtubeId('1.0') + '.srt.sjson'; + return '' + this.config.caption_asset_path + + this.youtubeId('1.0') + '.srt.sjson'; } function autoShowCaptions(event) { @@ -224,13 +272,19 @@ function () { this.videoCaption.subtitlesEl.show(); this.captionState = 'visible'; } else if (this.captionState === 'hiding') { - this.videoCaption.subtitlesEl.stop(true, false).css('opacity', 1).show(); + this.videoCaption.subtitlesEl + .stop(true, false).css('opacity', 1).show(); this.captionState = 'visible'; } else if (this.captionState === 'visible') { clearTimeout(this.captionHideTimeout); } - this.captionHideTimeout = setTimeout(this.videoCaption.autoHideCaptions, this.videoCaption.fadeOutTimeout); + if (this.config.autohideHtml5) { + this.captionHideTimeout = setTimeout( + this.videoCaption.autoHideCaptions, + this.videoCaption.fadeOutTimeout + ); + } this.captionsShowLock = false; } @@ -249,15 +303,21 @@ function () { _this = this; - this.videoCaption.subtitlesEl.fadeOut(this.videoCaption.fadeOutTimeout, function () { - _this.captionState = 'invisible'; - }); + this.videoCaption.subtitlesEl + .fadeOut( + this.videoCaption.fadeOutTimeout, + function () { + _this.captionState = 'invisible'; + } + ); } function resize() { this.videoCaption.subtitlesEl - .find('.spacing:first').height(this.videoCaption.topSpacingHeight()) - .find('.spacing:last').height(this.videoCaption.bottomSpacingHeight()); + .find('.spacing:first') + .height(this.videoCaption.topSpacingHeight()) + .find('.spacing:last') + .height(this.videoCaption.bottomSpacingHeight()); this.videoCaption.scrollCaption(); @@ -269,7 +329,10 @@ function () { clearTimeout(this.videoCaption.frozen); } - this.videoCaption.frozen = setTimeout(this.videoCaption.onMouseLeave, 10000); + this.videoCaption.frozen = setTimeout( + this.videoCaption.onMouseLeave, + this.config.captionsFreezeTime + ); } function onMouseLeave() { @@ -285,6 +348,10 @@ function () { } function onMovement() { + if (!this.config.autohideHtml5) { + this.videoCaption.autoShowCaptions(); + } + this.videoCaption.onMouseEnter(); } @@ -292,16 +359,28 @@ function () { var container = $('
    '), _this = this; - this.el.find('.video-wrapper').after(this.videoCaption.subtitlesEl); - this.el.find('.video-controls .secondary-controls').append(this.videoCaption.hideSubtitlesEl); + this.elVideoWrapper.after(this.videoCaption.subtitlesEl); + this.el.find('.video-controls .secondary-controls') + .append(this.videoCaption.hideSubtitlesEl); this.videoCaption.setSubtitlesHeight(); - if (this.videoType === 'html5') { + if ((this.videoType === 'html5') && (this.config.autohideHtml5)) { this.videoCaption.fadeOutTimeout = this.config.fadeOutTimeout; this.videoCaption.subtitlesEl.addClass('html5'); - this.captionHideTimeout = setTimeout(this.videoCaption.autoHideCaptions, this.videoCaption.fadeOutTimeout); + this.captionHideTimeout = setTimeout( + this.videoCaption.autoHideCaptions, + this.videoCaption.fadeOutTimeout + ); + } else if (!this.config.autohideHtml5) { + this.videoCaption.fadeOutTimeout = this.config.fadeOutTimeout; + this.videoCaption.subtitlesEl.addClass('html5'); + + this.captionHideTimeout = setTimeout( + this.videoCaption.autoHideCaptions, + 0 + ); } this.videoCaption.hideCaptions(this.hide_captions); @@ -322,38 +401,47 @@ function () { container.append(liEl); }); - this.videoCaption.subtitlesEl.html(container.html()); - - this.videoCaption.subtitlesEl.find('li[data-index]').on({ - mouseover: this.videoCaption.captionMouseOverOut, - mouseout: this.videoCaption.captionMouseOverOut, - mousedown: this.videoCaption.captionMouseDown, - click: this.videoCaption.captionClick, - focus: this.videoCaption.captionFocus, - blur: this.videoCaption.captionBlur, - keydown: this.videoCaption.captionKeyDown - }); + this.videoCaption.subtitlesEl + .html(container.html()) + .find('li[data-index]') + .on({ + mouseover: this.videoCaption.captionMouseOverOut, + mouseout: this.videoCaption.captionMouseOverOut, + mousedown: this.videoCaption.captionMouseDown, + click: this.videoCaption.captionClick, + focus: this.videoCaption.captionFocus, + blur: this.videoCaption.captionBlur, + keydown: this.videoCaption.captionKeyDown + }); // Enables or disables automatic scrolling of the captions when the // video is playing. This feature has to be disabled when tabbing // through them as it interferes with that action. Initially, have this - // flag enabled as we assume mouse use. Then, if the first caption + // flag enabled as we assume mouse use. Then, if the first caption // (through forward tabbing) or the last caption (through backwards // tabbing) gets the focus, disable that feature. Renable it if tabbing - // then cycles out of the the captions. + // then cycles out of the the captions. this.videoCaption.autoScrolling = true; // Keeps track of where the focus is situated in the array of captions. // Used to implement the automatic scrolling behavior and decide if the - // outline around a caption has to be hidden or shown on a mouseenter or - // mouseleave. Initially, no caption has the focus, set the index to -1. + // outline around a caption has to be hidden or shown on a mouseenter + // or mouseleave. Initially, no caption has the focus, set the + // index to -1. this.videoCaption.currentCaptionIndex = -1; - // Used to track if the focus is coming from a click or tabbing. This - // has to be known to decide if, when a caption gets the focus, an + // Used to track if the focus is coming from a click or tabbing. This + // has to be known to decide if, when a caption gets the focus, an // outline has to be drawn (tabbing) or not (mouse click). this.videoCaption.isMouseFocus = false; - this.videoCaption.subtitlesEl.prepend($('
  1. ').height(this.videoCaption.topSpacingHeight())); - this.videoCaption.subtitlesEl.append($('
  2. ').height(this.videoCaption.bottomSpacingHeight())); + this.videoCaption.subtitlesEl + .prepend( + $('
  3. ') + .height(this.videoCaption.topSpacingHeight()) + ) + .append( + $('
  4. ') + .height(this.videoCaption.bottomSpacingHeight()) + ); this.videoCaption.rendered = true; } @@ -362,7 +450,7 @@ function () { // On mouseOut, show the outline of a caption that has been tabbed to. function captionMouseOverOut(event) { var caption = $(event.target), - captionIndex = parseInt(caption.attr('data-index'), 10); + captionIndex = parseInt(caption.attr('data-index'), 10); if (captionIndex === this.videoCaption.currentCaptionIndex) { if (event.type === 'mouseover') { caption.removeClass('focused'); @@ -370,7 +458,7 @@ function () { else { // mouseout caption.addClass('focused'); } - } + } } function captionMouseDown(event) { @@ -390,36 +478,41 @@ function () { captionIndex = parseInt(caption.attr('data-index'), 10); // If the focus comes from a mouse click, hide the outline, turn on // automatic scrolling and set currentCaptionIndex to point outside of - // caption list (ie -1) to disable mouseenter, mouseleave behavior. + // caption list (ie -1) to disable mouseenter, mouseleave behavior. if (this.videoCaption.isMouseFocus) { this.videoCaption.autoScrolling = true; caption.removeClass('focused'); this.videoCaption.currentCaptionIndex = -1; } - // If the focus comes from tabbing, show the outline and turn off + // If the focus comes from tabbing, show the outline and turn off // automatic scrolling. else { this.videoCaption.currentCaptionIndex = captionIndex; caption.addClass('focused'); // The second and second to last elements turn automatic scrolling - // off again as it may have been enabled in captionBlur. - if (captionIndex <= 1 || captionIndex >= this.videoCaption.captions.length-2) { + // off again as it may have been enabled in captionBlur. + if ( + captionIndex <= 1 || + captionIndex >= this.videoCaption.captions.length - 2 + ) { this.videoCaption.autoScrolling = false; } } } function captionBlur(event) { - var caption = $(event.target), + var caption = $(event.target), captionIndex = parseInt(caption.attr('data-index'), 10); caption.removeClass('focused'); - // If we are on first or last index, we have to turn automatic scroll on - // again when losing focus. There is no way to know in what direction we - // are tabbing. So we could be on the first element and tabbing back out - // of the captions or on the last element and tabbing forward out of the - // captions. - if (captionIndex === 0 || + // If we are on first or last index, we have to turn automatic scroll + // on again when losing focus. There is no way to know in what + // direction we are tabbing. So we could be on the first element and + // tabbing back out of the captions or on the last element and tabbing + // forward out of the captions. + if (captionIndex === 0 || captionIndex === this.videoCaption.captions.length-1) { + this.videoCaption.autoHideCaptions(); + this.videoCaption.autoScrolling = true; } } @@ -434,9 +527,13 @@ function () { function scrollCaption() { var el = this.videoCaption.subtitlesEl.find('.current:first'); - // Automatic scrolling gets disabled if one of the captions has received - // focus through tabbing. - if (!this.videoCaption.frozen && el.length && this.videoCaption.autoScrolling) { + // Automatic scrolling gets disabled if one of the captions has + // received focus through tabbing. + if ( + !this.videoCaption.frozen && + el.length && + this.videoCaption.autoScrolling + ) { this.videoCaption.subtitlesEl.scrollTo( el, { @@ -565,11 +662,15 @@ function () { } function topSpacingHeight() { - return this.videoCaption.calculateOffset(this.videoCaption.subtitlesEl.find('li:not(.spacing):first')); + return this.videoCaption.calculateOffset( + this.videoCaption.subtitlesEl.find('li:not(.spacing):first') + ); } function bottomSpacingHeight() { - return this.videoCaption.calculateOffset(this.videoCaption.subtitlesEl.find('li:not(.spacing):last')); + return this.videoCaption.calculateOffset( + this.videoCaption.subtitlesEl.find('li:not(.spacing):last') + ); } function toggle(event) { @@ -579,23 +680,45 @@ function () { this.videoCaption.hideCaptions(false); } else { this.videoCaption.hideCaptions(true); + + // In the case when captions are not auto-hidden based on mouse + // movement anywhere on the video, we must hide them explicitly + // after the "CC" button has been clicked (to hide captions). + // + // Otherwise, in order for the captions to disappear again, the + // user must move the mouse button over the "CC" button, or over + // the captions themselves. In this case, an "autoShow" will be + // triggered, and after a timeout, an "autoHide". + if (!this.config.autohideHtml5) { + this.captionHideTimeout = setTimeout( + this.videoCaption.autoHideCaptions(), + 0 + ); + } } } - function hideCaptions(hide_captions) { - var type; + function hideCaptions(hide_captions, update_cookie) { + var hideSubtitlesEl = this.videoCaption.hideSubtitlesEl, + type; if (hide_captions) { type = 'hide_transcript'; this.captionsHidden = true; - this.videoCaption.hideSubtitlesEl.attr('title', gettext('Turn on captions')); - this.videoCaption.hideSubtitlesEl.text(gettext('Turn on captions')); + + hideSubtitlesEl + .attr('title', gettext('Turn on captions')) + .text(gettext('Turn on captions')); + this.el.addClass('closed'); } else { type = 'show_transcript'; this.captionsHidden = false; - this.videoCaption.hideSubtitlesEl.attr('title', gettext('Turn off captions')); - this.videoCaption.hideSubtitlesEl.text(gettext('Turn off captions')); + + hideSubtitlesEl + .attr('title', gettext('Turn off captions')) + .text(gettext('Turn off captions')); + this.el.removeClass('closed'); this.videoCaption.scrollCaption(); } @@ -615,27 +738,43 @@ function () { } function captionHeight() { + var paddingTop; + if (this.isFullScreen) { - return $(window).height() - this.el.find('.video-controls').height() - - 0.5 * this.videoControl.sliderEl.height() - - 2 * parseInt(this.videoCaption.subtitlesEl.css('padding-top'), 10); + paddingTop = parseInt( + this.videoCaption.subtitlesEl.css('padding-top'), 10 + ); + + return $(window).height() - + this.videoControl.el.height() - + 0.5 * this.videoControl.sliderEl.height() - + 2 * paddingTop; } else { - return this.el.find('.video-wrapper').height(); + return this.elVideoWrapper.height(); } } function setSubtitlesHeight() { var height = 0; - if (this.videoType === 'html5'){ + if ( + ((this.videoType === 'html5') && (this.config.autohideHtml5)) || + (!this.config.autohideHtml5) + ){ // on page load captionHidden = undefined if ( - (this.captionsHidden === undefined && this.hide_captions === true ) || - (this.captionsHidden === true) ) { - // In case of html5 autoshowing subtitles, - // we ajdust height of subs, by height of scrollbar - height = this.videoControl.el.height() + 0.5 * this.videoControl.sliderEl.height(); - // height of videoControl does not contain height of slider. - // (css is set to absolute, to avoid yanking when slider autochanges its height) + ( + this.captionsHidden === undefined && + this.hide_captions === true + ) || + (this.captionsHidden === true) + ) { + // In case of html5 autoshowing subtitles, we adjust height of + // subs, by height of scrollbar. + height = this.videoControl.el.height() + + 0.5 * this.videoControl.sliderEl.height(); + // Height of videoControl does not contain height of slider. + // css is set to absolute, to avoid yanking when slider + // autochanges its height. } } this.videoCaption.subtitlesEl.css({ diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index e2c483225199..cd330ab9a20e 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -10,6 +10,7 @@ import urllib from xmodule.editing_module import MetadataOnlyEditingDescriptor +from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.x_module import XModule from xmodule.course_module import CourseDescriptor from pkg_resources import resource_string @@ -254,8 +255,8 @@ def oauth_params(self, custom_parameters, client_key, client_secret): return params -class LTIModuleDescriptor(LTIFields, MetadataOnlyEditingDescriptor): +class LTIModuleDescriptor(LTIFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor): """ - LTIModuleDescriptor provides no export/import to xml. + Descriptor for LTI Xmodule. """ module_class = LTIModule diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index b816ab246dc0..f7a025015552 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -234,12 +234,26 @@ def publish(self, location, published_by_id): """ Save a current draft to the underlying modulestore """ + try: + original_published = super(DraftModuleStore, self).get_item(location) + except ItemNotFoundError: + original_published = None + draft = self.get_item(location) draft.published_date = datetime.now(UTC) draft.published_by = published_by_id super(DraftModuleStore, self).update_item(location, draft.get_explicitly_set_fields_by_scope(Scope.content)) if draft.has_children: + if original_published is not None: + # see if children were deleted. 2 reasons for children lists to differ: + # 1) child deleted + # 2) child moved + for child in original_published.children: + if child not in draft.children: + rents = [Location(mom) for mom in self.get_parent_locations(child, None)] + if (len(rents) == 1 and rents[0] == Location(location)): # the 1 is this original_published + self.delete_item(child, True) super(DraftModuleStore, self).update_children(location, draft.children) super(DraftModuleStore, self).update_metadata(location, own_metadata(draft)) self.delete_item(location) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py new file mode 100644 index 000000000000..3b535c673801 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -0,0 +1,189 @@ +""" +Test the publish code (primary causing orphans) +""" +import uuid +import mock +import unittest +import datetime +import random + +from xmodule.modulestore.inheritance import InheritanceMixin +from xmodule.modulestore.mongo import MongoModuleStore, DraftMongoModuleStore +from xmodule.modulestore import Location +from xmodule.fields import Date +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.mongo.draft import DIRECT_ONLY_CATEGORIES + + +class TestPublish(unittest.TestCase): + """ + Test the publish code (primary causing orphans) + """ + + # Snippet of what would be in the django settings envs file + db_config = { + 'host': 'localhost', + 'db': 'test_xmodule', + } + + modulestore_options = dict({ + 'default_class': 'xmodule.raw_module.RawDescriptor', + 'fs_root': '', + 'render_template': mock.Mock(return_value=""), + 'xblock_mixins': (InheritanceMixin,) + }, **db_config) + + def setUp(self): + self.modulestore_options['collection'] = 'modulestore{0}'.format(uuid.uuid4().hex) + + self.old_mongo = MongoModuleStore(**self.modulestore_options) + self.draft_mongo = DraftMongoModuleStore(**self.modulestore_options) + self.addCleanup(self.tear_down_mongo) + self.course_location = None + + def tear_down_mongo(self): + # old_mongo doesn't give a db attr, but all of the dbs are the same and draft and pub use same collection + dbref = self.old_mongo.collection.database + dbref.drop_collection(self.old_mongo.collection) + dbref.connection.close() + + def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime): + """ + Create the item in either draft or direct based on category and attach to its parent. + """ + location = self.course_location.replace(category=category, name=name) + if category in DIRECT_ONLY_CATEGORIES: + mongo = self.old_mongo + else: + mongo = self.draft_mongo + mongo.create_and_save_xmodule(location, data, metadata, runtime) + if isinstance(data, basestring): + fields = {'data': data} + else: + fields = data.copy() + fields.update(metadata) + if parent_name: + # add child to parent in mongo + parent_location = self.course_location.replace(category=parent_category, name=parent_name) + parent = self.draft_mongo.get_item(parent_location) + parent.children.append(location.url()) + if parent_category in DIRECT_ONLY_CATEGORIES: + mongo = self.old_mongo + else: + mongo = self.draft_mongo + mongo.update_children(parent_location, parent.children) + + def _create_course(self): + """ + Create the course, publish all verticals + * some detached items + """ + date_proxy = Date() + metadata = { + 'start': date_proxy.to_json(datetime.datetime(2000, 3, 13, 4)), + 'display_name': 'Migration test course', + } + data = { + 'wiki_slug': 'test_course_slug' + } + fields = metadata.copy() + fields.update(data) + + self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid') + self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata) + runtime = self.draft_mongo.get_item(self.course_location).runtime + + self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', runtime) + self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', runtime) + self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', runtime) + self._create_item('vertical', 'Vert2', {}, {'display_name': 'Vertical 2'}, 'chapter', 'Chapter1', runtime) + self._create_item('html', 'Html1', "

    Goodbye

    ", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', runtime) + self._create_item( + 'discussion', 'Discussion1', + "discussion discussion_category=\"Lecture 1\" discussion_id=\"a08bfd89b2aa40fa81f2c650a9332846\" discussion_target=\"Lecture 1\"/>\n", + { + "discussion_category": "Lecture 1", + "discussion_target": "Lecture 1", + "display_name": "Lecture 1 Discussion", + "discussion_id": "a08bfd89b2aa40fa81f2c650a9332846" + }, + 'vertical', 'Vert1', runtime + ) + self._create_item('html', 'Html2', "

    Hellow

    ", {'display_name': 'Hollow Html'}, 'vertical', 'Vert1', runtime) + self._create_item( + 'discussion', 'Discussion2', + "discussion discussion_category=\"Lecture 2\" discussion_id=\"b08bfd89b2aa40fa81f2c650a9332846\" discussion_target=\"Lecture 2\"/>\n", + { + "discussion_category": "Lecture 2", + "discussion_target": "Lecture 2", + "display_name": "Lecture 2 Discussion", + "discussion_id": "b08bfd89b2aa40fa81f2c650a9332846" + }, + 'vertical', 'Vert2', runtime + ) + self._create_item('static_tab', 'staticuno', "

    tab

    ", {'display_name': 'Tab uno'}, None, None, runtime) + self._create_item('about', 'overview', "

    overview

    ", {}, None, None, runtime) + self._create_item('course_info', 'updates', "
    1. Sep 22

      test

    ", {}, None, None, runtime) + + def _xmodule_recurse(self, item, action): + """ + Applies action depth-first down tree and to item last. + + A copy of cms.djangoapps.contentstore.views.requests._xmodule_recurse to reproduce its use and behavior + outside of django. + """ + for child in item.get_children(): + self._xmodule_recurse(child, action) + + action(item) + + def test_publish_draft_delete(self): + """ + To reproduce a bug (STUD-811) publish a vertical, convert to draft, delete a child, move a child, publish. + See if deleted and moved children still is connected or exists in db (bug was disconnected but existed) + """ + self._create_course() + userid = random.getrandbits(32) + location = self.course_location.replace(category='vertical', name='Vert1') + item = self.draft_mongo.get_item(location, 2) + self._xmodule_recurse( + item, + lambda i: self.draft_mongo.publish(i.location, userid) + ) + # verify status + item = self.draft_mongo.get_item(location, 0) + self.assertFalse(getattr(item, 'is_draft', False), "Item was published. Draft should not exist") + # however, children are still draft, but I'm not sure that's by design + + # convert back to draft + self.draft_mongo.convert_to_draft(location) + # both draft and published should exist + draft_vert = self.draft_mongo.get_item(location, 0) + self.assertTrue(getattr(draft_vert, 'is_draft', False), "Item was converted to draft but doesn't say so") + item = self.old_mongo.get_item(location, 0) + self.assertFalse(getattr(item, 'is_draft', False), "Published item doesn't say so") + + # delete the discussion (which oddly is not in draft mode) + location = self.course_location.replace(category='discussion', name='Discussion1') + self.draft_mongo.delete_item(location) + # remove pointer from draft vertical (verify presence first to ensure process is valid) + self.assertIn(location.url(), draft_vert.children) + draft_vert.children.remove(location.url()) + # move the other child + other_child_loc = self.course_location.replace(category='html', name='Html2') + draft_vert.children.remove(other_child_loc.url()) + other_vert = self.draft_mongo.get_item(self.course_location.replace(category='vertical', name='Vert2'), 0) + other_vert.children.append(other_child_loc.url()) + self.draft_mongo.update_children(draft_vert.location, draft_vert.children) + self.draft_mongo.update_children(other_vert.location, other_vert.children) + # publish + self._xmodule_recurse( + draft_vert, + lambda i: self.draft_mongo.publish(i.location, userid) + ) + item = self.old_mongo.get_item(draft_vert.location, 0) + self.assertNotIn(location.url(), item.children) + with self.assertRaises(ItemNotFoundError): + self.draft_mongo.get_item(location) + self.assertNotIn(other_child_loc.url(), item.children) + self.assertTrue(self.draft_mongo.has_item(None, other_child_loc), "Oops, lost moved item") diff --git a/lms/djangoapps/bulk_email/admin.py b/lms/djangoapps/bulk_email/admin.py index 3550e1eb4200..f4529a9e70db 100644 --- a/lms/djangoapps/bulk_email/admin.py +++ b/lms/djangoapps/bulk_email/admin.py @@ -3,8 +3,8 @@ """ from django.contrib import admin -from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate -from bulk_email.forms import CourseEmailTemplateForm +from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate, CourseAuthorization +from bulk_email.forms import CourseEmailTemplateForm, CourseAuthorizationAdminForm class CourseEmailAdmin(admin.ModelAdmin): @@ -57,6 +57,23 @@ def has_delete_permission(self, request, obj=None): return False +class CourseAuthorizationAdmin(admin.ModelAdmin): + """Admin for enabling email on a course-by-course basis.""" + form = CourseAuthorizationAdminForm + fieldsets = ( + (None, { + 'fields': ('course_id', 'email_enabled'), + 'description': ''' +Enter a course id in the following form: Org/Course/CourseRun, eg MITx/6.002x/2012_Fall +Do not enter leading or trailing slashes. There is no need to surround the course ID with quotes. +Validation will be performed on the course name, and if it is invalid, an error message will display. + +To enable email for the course, check the "Email enabled" box, then click "Save". +''' + }), + ) + admin.site.register(CourseEmail, CourseEmailAdmin) admin.site.register(Optout, OptoutAdmin) admin.site.register(CourseEmailTemplate, CourseEmailTemplateAdmin) +admin.site.register(CourseAuthorization, CourseAuthorizationAdmin) diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email/forms.py index fc75d4e4b642..bec05fba52c6 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/lms/djangoapps/bulk_email/forms.py @@ -6,12 +6,16 @@ from django import forms from django.core.exceptions import ValidationError -from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG +from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG, CourseAuthorization + +from courseware.courses import get_course_by_id +from xmodule.modulestore import MONGO_MODULESTORE_TYPE +from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) -class CourseEmailTemplateForm(forms.ModelForm): +class CourseEmailTemplateForm(forms.ModelForm): # pylint: disable=R0924 """Form providing validation of CourseEmail templates.""" class Meta: # pylint: disable=C0111 @@ -43,3 +47,32 @@ def clean_plain_template(self): template = self.cleaned_data["plain_template"] self._validate_template(template) return template + + +class CourseAuthorizationAdminForm(forms.ModelForm): # pylint: disable=R0924 + """Input form for email enabling, allowing us to verify data.""" + + class Meta: # pylint: disable=C0111 + model = CourseAuthorization + + def clean_course_id(self): + """Validate the course id""" + course_id = self.cleaned_data["course_id"] + try: + # Just try to get the course descriptor. + # If we can do that, it's a real course. + get_course_by_id(course_id, depth=1) + except Exception as exc: + msg = 'Error encountered ({0})'.format(str(exc).capitalize()) + msg += ' --- Entered course id was: "{0}". '.format(course_id) + msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN' + raise forms.ValidationError(msg) + + # Now, try and discern if it is a Studio course - HTML editor doesn't work with XML courses + is_studio_course = modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE + if not is_studio_course: + msg = "Course Email feature is only available for courses authored in Studio. " + msg += '"{0}" appears to be an XML backed course.'.format(course_id) + raise forms.ValidationError(msg) + + return course_id diff --git a/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py b/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py new file mode 100644 index 000000000000..b200ea2bde9e --- /dev/null +++ b/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py @@ -0,0 +1,95 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'CourseAuthorization' + db.create_table('bulk_email_courseauthorization', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('course_id', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('email_enabled', self.gf('django.db.models.fields.BooleanField')(default=False)), + )) + db.send_create_signal('bulk_email', ['CourseAuthorization']) + + + def backwards(self, orm): + # Deleting model 'CourseAuthorization' + db.delete_table('bulk_email_courseauthorization') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'bulk_email.courseauthorization': { + 'Meta': {'object_name': 'CourseAuthorization'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'email_enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'bulk_email.courseemail': { + 'Meta': {'object_name': 'CourseEmail'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'html_message': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}), + 'sender': ('django.db.models.fields.related.ForeignKey', [], {'default': '1', 'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}), + 'slug': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}), + 'subject': ('django.db.models.fields.CharField', [], {'max_length': '128', 'blank': 'True'}), + 'text_message': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'to_option': ('django.db.models.fields.CharField', [], {'default': "'myself'", 'max_length': '64'}) + }, + 'bulk_email.courseemailtemplate': { + 'Meta': {'object_name': 'CourseEmailTemplate'}, + 'html_template': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'plain_template': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}) + }, + 'bulk_email.optout': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'Optout'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + } + } + + complete_apps = ['bulk_email'] \ No newline at end of file diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index 9d32dbd70c0b..83a691f18e3c 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -12,11 +12,21 @@ """ import logging -from django.db import models +from django.db import models, transaction from django.contrib.auth.models import User +from html_to_text import html_to_text + +from django.conf import settings log = logging.getLogger(__name__) +# Bulk email to_options - the send to options that users can +# select from when they send email. +SEND_TO_MYSELF = 'myself' +SEND_TO_STAFF = 'staff' +SEND_TO_ALL = 'all' +TO_OPTIONS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_ALL] + class Email(models.Model): """ @@ -33,12 +43,8 @@ class Email(models.Model): class Meta: # pylint: disable=C0111 abstract = True -SEND_TO_MYSELF = 'myself' -SEND_TO_STAFF = 'staff' -SEND_TO_ALL = 'all' - -class CourseEmail(Email, models.Model): +class CourseEmail(Email): """ Stores information for an email to a course. """ @@ -51,17 +57,66 @@ class CourseEmail(Email, models.Model): # * All: This sends an email to anyone enrolled in the course, with any role # (student, staff, or instructor) # - TO_OPTIONS = ( + TO_OPTION_CHOICES = ( (SEND_TO_MYSELF, 'Myself'), (SEND_TO_STAFF, 'Staff and instructors'), (SEND_TO_ALL, 'All') ) course_id = models.CharField(max_length=255, db_index=True) - to_option = models.CharField(max_length=64, choices=TO_OPTIONS, default=SEND_TO_MYSELF) + to_option = models.CharField(max_length=64, choices=TO_OPTION_CHOICES, default=SEND_TO_MYSELF) def __unicode__(self): return self.subject + @classmethod + def create(cls, course_id, sender, to_option, subject, html_message, text_message=None): + """ + Create an instance of CourseEmail. + + The CourseEmail.save_now method makes sure the CourseEmail entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, an autocommit buried within here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. + """ + # automatically generate the stripped version of the text from the HTML markup: + if text_message is None: + text_message = html_to_text(html_message) + + # perform some validation here: + if to_option not in TO_OPTIONS: + fmt = 'Course email being sent to unrecognized to_option: "{to_option}" for "{course}", subject "{subject}"' + msg = fmt.format(to_option=to_option, course=course_id, subject=subject) + raise ValueError(msg) + + # create the task, then save it immediately: + course_email = cls( + course_id=course_id, + sender=sender, + to_option=to_option, + subject=subject, + html_message=html_message, + text_message=text_message, + ) + course_email.save_now() + + return course_email + + @transaction.autocommit + def save_now(self): + """ + Writes CourseEmail immediately, ensuring the transaction is committed. + + Autocommit annotation makes sure the database entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, this autocommit here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. + """ + self.save() + class Optout(models.Model): """ @@ -101,7 +156,11 @@ def get_template(): If one isn't stored, an exception is thrown. """ - return CourseEmailTemplate.objects.get() + try: + return CourseEmailTemplate.objects.get() + except CourseEmailTemplate.DoesNotExist: + log.exception("Attempting to fetch a non-existent course email template") + raise @staticmethod def _render(format_string, message_body, context): @@ -153,3 +212,38 @@ def render_htmltext(self, htmltext, context): stored HTML template and the provided `context` dict. """ return CourseEmailTemplate._render(self.html_template, htmltext, context) + + +class CourseAuthorization(models.Model): + """ + Enable the course email feature on a course-by-course basis. + """ + # The course that these features are attached to. + course_id = models.CharField(max_length=255, db_index=True) + + # Whether or not to enable instructor email + email_enabled = models.BooleanField(default=False) + + @classmethod + def instructor_email_enabled(cls, course_id): + """ + Returns whether or not email is enabled for the given course id. + + If email has not been explicitly enabled, returns False. + """ + # If settings.MITX_FEATURES['REQUIRE_COURSE_EMAIL_AUTH'] is + # set to False, then we enable email for every course. + if not settings.MITX_FEATURES['REQUIRE_COURSE_EMAIL_AUTH']: + return True + + try: + record = cls.objects.get(course_id=course_id) + return record.email_enabled + except cls.DoesNotExist: + return False + + def __unicode__(self): + not_en = "Not " + if self.email_enabled: + not_en = "" + return u"Course '{}': Instructor Email {}Enabled".format(self.course_id, not_en) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 1f15a8b4d727..a2f376cdcc5a 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -4,174 +4,500 @@ """ import math import re -import time +import random +import json +from uuid import uuid4 +from time import sleep from dogapi import dog_stats_api -from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError +from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException +from boto.ses.exceptions import ( + SESDailyQuotaExceededError, + SESMaxSendingRateExceededError, + SESAddressBlacklistedError, + SESIllegalAddressError, + SESLocalAddressCharacterError, +) +from boto.exception import AWSConnectionError + +from celery import task, current_task, group +from celery.utils.log import get_task_logger +from celery.states import SUCCESS, FAILURE, RETRY +from celery.exceptions import RetryTaskError from django.conf import settings from django.contrib.auth.models import User, Group from django.core.mail import EmailMultiAlternatives, get_connection -from django.http import Http404 -from celery import task, current_task -from celery.utils.log import get_task_logger from django.core.urlresolvers import reverse from bulk_email.models import ( CourseEmail, Optout, CourseEmailTemplate, - SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_ALL, + SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS, ) from courseware.access import _course_staff_group_name, _course_instructor_group_name -from courseware.courses import get_course_by_id, course_image_url +from courseware.courses import get_course, course_image_url +from instructor_task.models import InstructorTask +from instructor_task.subtasks import ( + update_subtask_status, + create_subtask_status, + increment_subtask_status, + initialize_subtask_info, + check_subtask_is_valid, +) log = get_task_logger(__name__) -@task(default_retry_delay=10, max_retries=5) # pylint: disable=E1102 -def delegate_email_batches(email_id, user_id): - """ - Delegates emails by querying for the list of recipients who should - get the mail, chopping up into batches of settings.EMAILS_PER_TASK size, - and queueing up worker jobs. +# Errors that an individual email is failing to be sent, and should just +# be treated as a fail. +SINGLE_EMAIL_FAILURE_ERRORS = (SESAddressBlacklistedError, SESIllegalAddressError, SESLocalAddressCharacterError) - Returns the number of batches (workers) kicked off. - """ - try: - email_obj = CourseEmail.objects.get(id=email_id) - except CourseEmail.DoesNotExist as exc: - # The retry behavior here is necessary because of a race condition between the commit of the transaction - # that creates this CourseEmail row and the celery pipeline that starts this task. - # We might possibly want to move the blocking into the view function rather than have it in this task. - log.warning("Failed to get CourseEmail with id %s, retry %d", email_id, current_task.request.retries) - raise delegate_email_batches.retry(arg=[email_id, user_id], exc=exc) +# Exceptions that, if caught, should cause the task to be re-tried. +# These errors will be caught a limited number of times before the task fails. +LIMITED_RETRY_ERRORS = (SMTPConnectError, SMTPServerDisconnected, AWSConnectionError) - to_option = email_obj.to_option - course_id = email_obj.course_id +# Errors that indicate that a mailing task should be retried without limit. +# An example is if email is being sent too quickly, but may succeed if sent +# more slowly. When caught by a task, it triggers an exponential backoff and retry. +# Retries happen continuously until the email is sent. +# Note that the SMTPDataErrors here are only those within the 4xx range. +# Those not in this range (i.e. in the 5xx range) are treated as hard failures +# and thus like SINGLE_EMAIL_FAILURE_ERRORS. +INFINITE_RETRY_ERRORS = (SESMaxSendingRateExceededError, SMTPDataError) - try: - course = get_course_by_id(course_id, depth=1) - except Http404 as exc: - log.exception("get_course_by_id failed: %s", exc.args[0]) - raise Exception("get_course_by_id failed: " + exc.args[0]) +# Errors that are known to indicate an inability to send any more emails, +# and should therefore not be retried. For example, exceeding a quota for emails. +# Also, any SMTP errors that are not explicitly enumerated above. +BULK_EMAIL_FAILURE_ERRORS = (SESDailyQuotaExceededError, SMTPException) - course_url = 'https://{}{}'.format( - settings.SITE_NAME, - reverse('course_root', kwargs={'course_id': course_id}) - ) - image_url = 'https://{}{}'.format(settings.SITE_NAME, course_image_url(course)) + +def _get_recipient_queryset(user_id, to_option, course_id, course_location): + """ + Returns a query set of email recipients corresponding to the requested to_option category. + + `to_option` is either SEND_TO_MYSELF, SEND_TO_STAFF, or SEND_TO_ALL. + + Recipients who are in more than one category (e.g. enrolled in the course and are staff or self) + will be properly deduped. + """ + if to_option not in TO_OPTIONS: + log.error("Unexpected bulk email TO_OPTION found: %s", to_option) + raise Exception("Unexpected bulk email TO_OPTION found: {0}".format(to_option)) if to_option == SEND_TO_MYSELF: recipient_qset = User.objects.filter(id=user_id) - elif to_option == SEND_TO_ALL or to_option == SEND_TO_STAFF: - staff_grpname = _course_staff_group_name(course.location) + else: + staff_grpname = _course_staff_group_name(course_location) staff_group, _ = Group.objects.get_or_create(name=staff_grpname) staff_qset = staff_group.user_set.all() - instructor_grpname = _course_instructor_group_name(course.location) + instructor_grpname = _course_instructor_group_name(course_location) instructor_group, _ = Group.objects.get_or_create(name=instructor_grpname) instructor_qset = instructor_group.user_set.all() recipient_qset = staff_qset | instructor_qset - if to_option == SEND_TO_ALL: - enrollment_qset = User.objects.filter(courseenrollment__course_id=course_id, - courseenrollment__is_active=True) + # We also require students to have activated their accounts to + # provide verification that the provided email address is valid. + enrollment_qset = User.objects.filter( + is_active=True, + courseenrollment__course_id=course_id, + courseenrollment__is_active=True + ) recipient_qset = recipient_qset | enrollment_qset recipient_qset = recipient_qset.distinct() - else: - log.error("Unexpected bulk email TO_OPTION found: %s", to_option) - raise Exception("Unexpected bulk email TO_OPTION found: {0}".format(to_option)) recipient_qset = recipient_qset.order_by('pk') + return recipient_qset + + +def _get_course_email_context(course): + """ + Returns context arguments to apply to all emails, independent of recipient. + """ + course_id = course.id + course_title = course.display_name + course_url = 'https://{}{}'.format( + settings.SITE_NAME, + reverse('course_root', kwargs={'course_id': course_id}) + ) + image_url = 'https://{}{}'.format(settings.SITE_NAME, course_image_url(course)) + email_context = { + 'course_title': course_title, + 'course_url': course_url, + 'course_image_url': image_url, + 'account_settings_url': 'https://{}{}'.format(settings.SITE_NAME, reverse('dashboard')), + 'platform_name': settings.PLATFORM_NAME, + } + return email_context + + +def _generate_subtasks(create_subtask_fcn, recipient_qset): + """ + Generates a list of subtasks to send email to a given set of recipients. + + Arguments: + `create_subtask_fcn` : a function whose inputs are a list of recipients and a subtask_id + to assign to the new subtask. Returns the subtask that will send email to that + list of recipients. + `recipient_qset` : a query set that defines the recipients who should receive emails. + + Returns: a tuple, containing: + + * A list of subtasks that will send emails to all recipients. + * A list of subtask_ids corresponding to those subtasks. + * A count of the total number of emails being sent. + + """ total_num_emails = recipient_qset.count() - num_queries = int(math.ceil(float(total_num_emails) / float(settings.EMAILS_PER_QUERY))) + num_queries = int(math.ceil(float(total_num_emails) / float(settings.BULK_EMAIL_EMAILS_PER_QUERY))) last_pk = recipient_qset[0].pk - 1 - num_workers = 0 + num_emails_queued = 0 + task_list = [] + subtask_id_list = [] for _ in range(num_queries): - recipient_sublist = list(recipient_qset.order_by('pk').filter(pk__gt=last_pk) - .values('profile__name', 'email', 'pk')[:settings.EMAILS_PER_QUERY]) + recipient_sublist = list(recipient_qset.order_by('pk').filter(pk__gt=last_pk).values('profile__name', 'email', 'pk')[:settings.BULK_EMAIL_EMAILS_PER_QUERY]) last_pk = recipient_sublist[-1]['pk'] num_emails_this_query = len(recipient_sublist) - num_tasks_this_query = int(math.ceil(float(num_emails_this_query) / float(settings.EMAILS_PER_TASK))) + num_tasks_this_query = int(math.ceil(float(num_emails_this_query) / float(settings.BULK_EMAIL_EMAILS_PER_TASK))) chunk = int(math.ceil(float(num_emails_this_query) / float(num_tasks_this_query))) for i in range(num_tasks_this_query): to_list = recipient_sublist[i * chunk:i * chunk + chunk] - course_email.delay( - email_id, - to_list, - course.display_name, - course_url, - image_url, - False - ) - num_workers += num_tasks_this_query - return num_workers + subtask_id = str(uuid4()) + subtask_id_list.append(subtask_id) + new_subtask = create_subtask_fcn(to_list, subtask_id) + task_list.append(new_subtask) + + num_emails_queued += num_emails_this_query + + # Sanity check: we expect the chunking to be properly summing to the original count: + if num_emails_queued != total_num_emails: + error_msg = "Task {}: number of emails generated by chunking {} not equal to original total {}".format(num_emails_queued, total_num_emails) + log.error(error_msg) + raise ValueError(error_msg) + return task_list, subtask_id_list, total_num_emails -@task(default_retry_delay=15, max_retries=5) # pylint: disable=E1102 -def course_email(email_id, to_list, course_title, course_url, image_url, throttle=False): + +def perform_delegate_email_batches(entry_id, course_id, task_input, action_name): """ - Takes a primary id for a CourseEmail object and a 'to_list' of recipient objects--keys are - 'profile__name', 'email' (address), and 'pk' (in the user table). - course_title, course_url, and image_url are to memoize course properties and save lookups. + Delegates emails by querying for the list of recipients who should + get the mail, chopping up into batches of settings.BULK_EMAIL_EMAILS_PER_TASK size, + and queueing up worker jobs. - Sends to all addresses contained in to_list. Emails are sent multi-part, in both plain - text and html. + Returns the number of batches (workers) kicked off. """ - with dog_stats_api.timer('course_email.single_task.time.overall', tags=[_statsd_tag(course_title)]): - _send_course_email(email_id, to_list, course_title, course_url, image_url, throttle) + entry = InstructorTask.objects.get(pk=entry_id) + # Get inputs to use in this task from the entry. + user_id = entry.requester.id + task_id = entry.task_id + + # Perfunctory check, since expansion is made for convenience of other task + # code that doesn't need the entry_id. + if course_id != entry.course_id: + format_msg = "Course id conflict: explicit value {} does not match task value {}" + raise ValueError(format_msg.format(course_id, entry.course_id)) + + # Fetch the CourseEmail. + email_id = task_input['email_id'] + try: + email_obj = CourseEmail.objects.get(id=email_id) + except CourseEmail.DoesNotExist: + # The CourseEmail object should be committed in the view function before the task + # is submitted and reaches this point. + log.warning("Task %s: Failed to get CourseEmail with id %s", task_id, email_id) + raise + + # Check to see if email batches have already been defined. This seems to + # happen sometimes when there is a loss of connection while a task is being + # queued. When this happens, the same task gets called again, and a whole + # new raft of subtasks gets queued up. We will assume that if subtasks + # have already been defined, there is no need to redefine them below. + # So we just return right away. We don't raise an exception, because we want + # the current task to be marked with whatever it had been marked with before. + if len(entry.subtasks) > 0 and len(entry.task_output) > 0: + log.warning("Task %s has already been processed for email %s! InstructorTask = %s", task_id, email_id, entry) + progress = json.loads(entry.task_output) + return progress + + # Sanity check that course for email_obj matches that of the task referencing it. + if course_id != email_obj.course_id: + format_msg = "Course id conflict: explicit value {} does not match email value {}" + raise ValueError(format_msg.format(course_id, email_obj.course_id)) + + # Fetch the course object. + try: + course = get_course(course_id) + except ValueError: + log.exception("Task %s: course not found: %s", task_id, course_id) + raise -def _send_course_email(email_id, to_list, course_title, course_url, image_url, throttle): + to_option = email_obj.to_option + recipient_qset = _get_recipient_queryset(user_id, to_option, course_id, course.location) + global_email_context = _get_course_email_context(course) + + def _create_send_email_subtask(to_list, subtask_id): + """Creates a subtask to send email to a given recipient list.""" + subtask_status = create_subtask_status(subtask_id) + new_subtask = send_course_email.subtask( + ( + entry_id, + email_id, + to_list, + global_email_context, + subtask_status, + ), + task_id=subtask_id, + routing_key=settings.BULK_EMAIL_ROUTING_KEY, + ) + return new_subtask + + log.info("Task %s: Preparing to generate subtasks for course %s, email %s, to_option %s", + task_id, course_id, email_id, to_option) + task_list, subtask_id_list, total_num_emails = _generate_subtasks(_create_send_email_subtask, recipient_qset) + + # Update the InstructorTask with information about the subtasks we've defined. + log.info("Task %s: Preparing to update task for sending %d emails for course %s, email %s, to_option %s", + task_id, total_num_emails, course_id, email_id, to_option) + progress = initialize_subtask_info(entry, action_name, total_num_emails, subtask_id_list) + num_subtasks = len(subtask_id_list) + + # Now group the subtasks, and start them running. This allows all the subtasks + # in the list to be submitted at the same time. + log.info("Task %s: Preparing to queue %d email tasks (%d emails) for course %s, email %s, to %s", + task_id, num_subtasks, total_num_emails, course_id, email_id, to_option) + task_group = group(task_list) + task_group.apply_async(routing_key=settings.BULK_EMAIL_ROUTING_KEY) + + # We want to return progress here, as this is what will be stored in the + # AsyncResult for the parent task as its return value. + # The AsyncResult will then be marked as SUCCEEDED, and have this return value as its "result". + # That's okay, for the InstructorTask will have the "real" status, and monitoring code + # should be using that instead. + return progress + + +@task(default_retry_delay=settings.BULK_EMAIL_DEFAULT_RETRY_DELAY, max_retries=settings.BULK_EMAIL_MAX_RETRIES) # pylint: disable=E1102 +def send_course_email(entry_id, email_id, to_list, global_email_context, subtask_status): """ - Performs the email sending task. + Sends an email to a list of recipients. + + Inputs are: + * `entry_id`: id of the InstructorTask object to which progress should be recorded. + * `email_id`: id of the CourseEmail model that is to be emailed. + * `to_list`: list of recipients. Each is represented as a dict with the following keys: + - 'profile__name': full name of User. + - 'email': email address of User. + - 'pk': primary key of User model. + * `global_email_context`: dict containing values that are unique for this email but the same + for all recipients of this email. This dict is to be used to fill in slots in email + template. It does not include 'name' and 'email', which will be provided by the to_list. + * `subtask_status` : dict containing values representing current status. Keys are: + + 'task_id' : id of subtask. This is used to pass task information across retries. + 'attempted' : number of attempts -- should equal succeeded plus failed + 'succeeded' : number that succeeded in processing + 'skipped' : number that were not processed. + 'failed' : number that failed during processing + 'retried_nomax' : number of times the subtask has been retried for conditions that + should not have a maximum count applied + 'retried_withmax' : number of times the subtask has been retried for conditions that + should have a maximum count applied + 'state' : celery state of the subtask (e.g. QUEUING, PROGRESS, RETRY, FAILURE, SUCCESS) + + Most values will be zero on initial call, but may be different when the task is + invoked as part of a retry. + + Sends to all addresses contained in to_list that are not also in the Optout table. + Emails are sent multi-part, in both plain text and html. Updates InstructorTask object + with status information (sends, failures, skips) and updates number of subtasks completed. """ + current_task_id = subtask_status['task_id'] + num_to_send = len(to_list) + log.info("Preparing to send email %s to %d recipients as subtask %s for instructor task %d: context = %s, status=%s", + email_id, num_to_send, current_task_id, entry_id, global_email_context, subtask_status) + + # Check that the requested subtask is actually known to the current InstructorTask entry. + # If this fails, it throws an exception, which should fail this subtask immediately. + # This can happen when the parent task has been run twice, and results in duplicate + # subtasks being created for the same InstructorTask entry. We hope to catch this condition + # in perform_delegate_email_batches(), but just in case we fail to do so there, + # we check here as well. + check_subtask_is_valid(entry_id, current_task_id) + + send_exception = None + new_subtask_status = None try: - msg = CourseEmail.objects.get(id=email_id) - except CourseEmail.DoesNotExist: - log.exception("Could not find email id:{} to send.".format(email_id)) + course_title = global_email_context['course_title'] + with dog_stats_api.timer('course_email.single_task.time.overall', tags=[_statsd_tag(course_title)]): + new_subtask_status, send_exception = _send_course_email( + entry_id, + email_id, + to_list, + global_email_context, + subtask_status, + ) + except Exception: + # Unexpected exception. Try to write out the failure to the entry before failing. + log.exception("Send-email task %s: failed unexpectedly!", current_task_id) + # We got here for really unexpected reasons. Since we don't know how far + # the task got in emailing, we count all recipients as having failed. + # It at least keeps the counts consistent. + new_subtask_status = increment_subtask_status(subtask_status, failed=num_to_send, state=FAILURE) + update_subtask_status(entry_id, current_task_id, new_subtask_status) raise - # exclude optouts - optouts = (Optout.objects.filter(course_id=msg.course_id, - user__in=[i['pk'] for i in to_list]) - .values_list('user__email', flat=True)) + if send_exception is None: + # Update the InstructorTask object that is storing its progress. + log.info("Send-email task %s: succeeded", current_task_id) + update_subtask_status(entry_id, current_task_id, new_subtask_status) + elif isinstance(send_exception, RetryTaskError): + # If retrying, a RetryTaskError needs to be returned to Celery. + # We assume that the the progress made before the retry condition + # was encountered has already been updated before the retry call was made, + # so we only log here. + log.warning("Send-email task %s: being retried", current_task_id) + raise send_exception # pylint: disable=E0702 + else: + log.error("Send-email task %s: failed: %s", current_task_id, send_exception) + update_subtask_status(entry_id, current_task_id, new_subtask_status) + raise send_exception # pylint: disable=E0702 + + log.info("Send-email task %s: returning status %s", current_task_id, new_subtask_status) + return new_subtask_status + +def _filter_optouts_from_recipients(to_list, course_id): + """ + Filters a recipient list based on student opt-outs for a given course. + + Returns the filtered recipient list, as well as the number of optouts + removed from the list. + """ + optouts = Optout.objects.filter( + course_id=course_id, + user__in=[i['pk'] for i in to_list] + ).values_list('user__email', flat=True) optouts = set(optouts) + # Only count the num_optout for the first time the optouts are calculated. + # We assume that the number will not change on retries, and so we don't need + # to calculate it each time. num_optout = len(optouts) - to_list = [recipient for recipient in to_list if recipient['email'] not in optouts] + return to_list, num_optout - subject = "[" + course_title + "] " + msg.subject +def _get_source_address(course_id, course_title): + """ + Calculates an email address to be used as the 'from-address' for sent emails. + + Makes a unique from name and address for each course, e.g. + + "COURSE_TITLE" Course Staff + + """ course_title_no_quotes = re.sub(r'"', '', course_title) - from_addr = '"{0}" Course Staff <{1}>'.format(course_title_no_quotes, settings.DEFAULT_BULK_FROM_EMAIL) - course_email_template = CourseEmailTemplate.get_template() + # The course_id is assumed to be in the form 'org/course_num/run', + # so pull out the course_num. Then make sure that it can be used + # in an email address, by substituting a '_' anywhere a non-(ascii, period, or dash) + # character appears. + course_num = course_id.split('/')[1] + invalid_chars = re.compile(r"[^\w.-]") + course_num = invalid_chars.sub('_', course_num) + + from_addr = '"{0}" Course Staff <{1}-{2}>'.format(course_title_no_quotes, course_num, settings.BULK_EMAIL_DEFAULT_FROM_EMAIL) + return from_addr + + +def _send_course_email(entry_id, email_id, to_list, global_email_context, subtask_status): + """ + Performs the email sending task. + + Sends an email to a list of recipients. + + Inputs are: + * `entry_id`: id of the InstructorTask object to which progress should be recorded. + * `email_id`: id of the CourseEmail model that is to be emailed. + * `to_list`: list of recipients. Each is represented as a dict with the following keys: + - 'profile__name': full name of User. + - 'email': email address of User. + - 'pk': primary key of User model. + * `global_email_context`: dict containing values that are unique for this email but the same + for all recipients of this email. This dict is to be used to fill in slots in email + template. It does not include 'name' and 'email', which will be provided by the to_list. + * `subtask_status` : dict containing values representing current status. Keys are: + + 'task_id' : id of subtask. This is used to pass task information across retries. + 'attempted' : number of attempts -- should equal succeeded plus failed + 'succeeded' : number that succeeded in processing + 'skipped' : number that were not processed. + 'failed' : number that failed during processing + 'retried_nomax' : number of times the subtask has been retried for conditions that + should not have a maximum count applied + 'retried_withmax' : number of times the subtask has been retried for conditions that + should have a maximum count applied + 'state' : celery state of the subtask (e.g. QUEUING, PROGRESS, RETRY, FAILURE, SUCCESS) + + Sends to all addresses contained in to_list that are not also in the Optout table. + Emails are sent multi-part, in both plain text and html. + + Returns a tuple of two values: + * First value is a dict which represents current progress at the end of this call. Keys are + the same as for the input subtask_status. + + * Second value is an exception returned by the innards of the method, indicating a fatal error. + In this case, the number of recipients that were not sent have already been added to the + 'failed' count above. + """ + # Get information from current task's request: + task_id = subtask_status['task_id'] + + # collect stats on progress: + num_optout = 0 + num_sent = 0 + num_error = 0 + + try: + course_email = CourseEmail.objects.get(id=email_id) + except CourseEmail.DoesNotExist as exc: + log.exception("Task %s: could not find email id:%s to send.", task_id, email_id) + raise + # Exclude optouts (if not a retry): + # Note that we don't have to do the optout logic at all if this is a retry, + # because we have presumably already performed the optout logic on the first + # attempt. Anyone on the to_list on a retry has already passed the filter + # that existed at that time, and we don't need to keep checking for changes + # in the Optout list. + if (subtask_status['retried_nomax'] + subtask_status['retried_withmax']) == 0: + to_list, num_optout = _filter_optouts_from_recipients(to_list, course_email.course_id) + + course_title = global_email_context['course_title'] + subject = "[" + course_title + "] " + course_email.subject + from_addr = _get_source_address(course_email.course_id, course_title) + + course_email_template = CourseEmailTemplate.get_template() try: connection = get_connection() connection.open() - num_sent = 0 - num_error = 0 # Define context values to use in all course emails: - email_context = { - 'name': '', - 'email': '', - 'course_title': course_title, - 'course_url': course_url, - 'course_image_url': image_url, - 'account_settings_url': 'https://{}{}'.format(settings.SITE_NAME, reverse('dashboard')), - 'platform_name': settings.PLATFORM_NAME, - } + email_context = {'name': '', 'email': ''} + email_context.update(global_email_context) while to_list: - # Update context with user-specific values: - email = to_list[-1]['email'] + # Update context with user-specific values from the user at the end of the list. + # At the end of processing this user, they will be popped off of the to_list. + # That way, the to_list will always contain the recipients remaining to be emailed. + # This is convenient for retries, which will need to send to those who haven't + # yet been emailed, but not send to those who have already been sent to. + current_recipient = to_list[-1] + email = current_recipient['email'] email_context['email'] = email - email_context['name'] = to_list[-1]['profile__name'] + email_context['name'] = current_recipient['profile__name'] # Construct message content using templates and context: - plaintext_msg = course_email_template.render_plaintext(msg.text_message, email_context) - html_msg = course_email_template.render_htmltext(msg.html_message, email_context) + plaintext_msg = course_email_template.render_plaintext(course_email.text_message, email_context) + html_msg = course_email_template.render_htmltext(course_email.html_message, email_context) # Create email: email_msg = EmailMultiAlternatives( @@ -183,66 +509,247 @@ def _send_course_email(email_id, to_list, course_title, course_url, image_url, t ) email_msg.attach_alternative(html_msg, 'text/html') - # Throttle if we tried a few times and got the rate limiter - if throttle or current_task.request.retries > 0: - time.sleep(0.2) + # Throttle if we have gotten the rate limiter. This is not very high-tech, + # but if a task has been retried for rate-limiting reasons, then we sleep + # for a period of time between all emails within this task. Choice of + # the value depends on the number of workers that might be sending email in + # parallel, and what the SES throttle rate is. + if subtask_status['retried_nomax'] > 0: + sleep(settings.BULK_EMAIL_RETRY_DELAY_BETWEEN_SENDS) try: + log.debug('Email with id %s to be sent to %s', email_id, email) + with dog_stats_api.timer('course_email.single_send.time.overall', tags=[_statsd_tag(course_title)]): connection.send_messages([email_msg]) - dog_stats_api.increment('course_email.sent', tags=[_statsd_tag(course_title)]) - - log.info('Email with id %s sent to %s', email_id, email) - num_sent += 1 except SMTPDataError as exc: - # According to SMTP spec, we'll retry error codes in the 4xx range. 5xx range indicates hard failure + # According to SMTP spec, we'll retry error codes in the 4xx range. 5xx range indicates hard failure. if exc.smtp_code >= 400 and exc.smtp_code < 500: - # This will cause the outer handler to catch the exception and retry the entire task + # This will cause the outer handler to catch the exception and retry the entire task. raise exc else: - # This will fall through and not retry the message, since it will be popped - log.warning('Email with id %s not delivered to %s due to error %s', email_id, email, exc.smtp_error) - + # This will fall through and not retry the message. + log.warning('Task %s: email with id %s not delivered to %s due to error %s', task_id, email_id, email, exc.smtp_error) dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)]) - num_error += 1 + except SINGLE_EMAIL_FAILURE_ERRORS as exc: + # This will fall through and not retry the message. + log.warning('Task %s: email with id %s not delivered to %s due to error %s', task_id, email_id, email, exc) + dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)]) + num_error += 1 + + else: + dog_stats_api.increment('course_email.sent', tags=[_statsd_tag(course_title)]) + if settings.BULK_EMAIL_LOG_SENT_EMAILS: + log.info('Email with id %s sent to %s', email_id, email) + else: + log.debug('Email with id %s sent to %s', email_id, email) + num_sent += 1 + + # Pop the user that was emailed off the end of the list only once they have + # successfully been processed. (That way, if there were a failure that + # needed to be retried, the user is still on the list.) to_list.pop() + except INFINITE_RETRY_ERRORS as exc: + dog_stats_api.increment('course_email.infinite_retry', tags=[_statsd_tag(course_title)]) + # Increment the "retried_nomax" counter, update other counters with progress to date, + # and set the state to RETRY: + subtask_progress = increment_subtask_status( + subtask_status, + succeeded=num_sent, + failed=num_error, + skipped=num_optout, + retried_nomax=1, + state=RETRY + ) + return _submit_for_retry( + entry_id, email_id, to_list, global_email_context, exc, subtask_progress, skip_retry_max=True + ) + + except LIMITED_RETRY_ERRORS as exc: + # Errors caught here cause the email to be retried. The entire task is actually retried + # without popping the current recipient off of the existing list. + # Errors caught are those that indicate a temporary condition that might succeed on retry. + dog_stats_api.increment('course_email.limited_retry', tags=[_statsd_tag(course_title)]) + # Increment the "retried_withmax" counter, update other counters with progress to date, + # and set the state to RETRY: + subtask_progress = increment_subtask_status( + subtask_status, + succeeded=num_sent, + failed=num_error, + skipped=num_optout, + retried_withmax=1, + state=RETRY + ) + return _submit_for_retry( + entry_id, email_id, to_list, global_email_context, exc, subtask_progress, skip_retry_max=False + ) + + except BULK_EMAIL_FAILURE_ERRORS as exc: + dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)]) + num_pending = len(to_list) + log.exception('Task %s: email with id %d caused send_course_email task to fail with "fatal" exception. %d emails unsent.', + task_id, email_id, num_pending) + # Update counters with progress to date, counting unsent emails as failures, + # and set the state to FAILURE: + subtask_progress = increment_subtask_status( + subtask_status, + succeeded=num_sent, + failed=(num_error + num_pending), + skipped=num_optout, + state=FAILURE + ) + return subtask_progress, exc + + except Exception as exc: + # Errors caught here cause the email to be retried. The entire task is actually retried + # without popping the current recipient off of the existing list. + # These are unexpected errors. Since they might be due to a temporary condition that might + # succeed on retry, we give them a retry. + dog_stats_api.increment('course_email.limited_retry', tags=[_statsd_tag(course_title)]) + log.exception('Task %s: email with id %d caused send_course_email task to fail with unexpected exception. Generating retry.', + task_id, email_id) + # Increment the "retried_withmax" counter, update other counters with progress to date, + # and set the state to RETRY: + subtask_progress = increment_subtask_status( + subtask_status, + succeeded=num_sent, + failed=num_error, + skipped=num_optout, + retried_withmax=1, + state=RETRY + ) + return _submit_for_retry( + entry_id, email_id, to_list, global_email_context, exc, subtask_progress, skip_retry_max=False + ) + + else: + # All went well. Update counters with progress to date, + # and set the state to SUCCESS: + subtask_progress = increment_subtask_status( + subtask_status, + succeeded=num_sent, + failed=num_error, + skipped=num_optout, + state=SUCCESS + ) + # Successful completion is marked by an exception value of None. + return subtask_progress, None + finally: + # Clean up at the end. connection.close() - return course_email_result(num_sent, num_error, num_optout) - - except (SMTPDataError, SMTPConnectError, SMTPServerDisconnected) as exc: - # Error caught here cause the email to be retried. The entire task is actually retried without popping the list - # Reasoning is that all of these errors may be temporary condition. - log.warning('Email with id %d not delivered due to temporary error %s, retrying send to %d recipients', - email_id, exc, len(to_list)) - raise course_email.retry( - arg=[ + + +def _get_current_task(): + """ + Stub to make it easier to test without actually running Celery. + + This is a wrapper around celery.current_task, which provides access + to the top of the stack of Celery's tasks. When running tests, however, + it doesn't seem to work to mock current_task directly, so this wrapper + is used to provide a hook to mock in tests, while providing the real + `current_task` in production. + """ + return current_task + + +def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current_exception, subtask_status, skip_retry_max=False): + """ + Helper function to requeue a task for retry, using the new version of arguments provided. + + Inputs are the same as for running a task, plus two extra indicating the state at the time of retry. + These include the `current_exception` that the task encountered that is causing the retry attempt, + and the `subtask_status` that is to be returned. A third extra argument `skip_retry_max` + indicates whether the current retry should be subject to a maximum test. + + Returns a tuple of two values: + * First value is a dict which represents current progress. Keys are: + + 'task_id' : id of subtask. This is used to pass task information across retries. + 'attempted' : number of attempts -- should equal succeeded plus failed + 'succeeded' : number that succeeded in processing + 'skipped' : number that were not processed. + 'failed' : number that failed during processing + 'retried_nomax' : number of times the subtask has been retried for conditions that + should not have a maximum count applied + 'retried_withmax' : number of times the subtask has been retried for conditions that + should have a maximum count applied + 'state' : celery state of the subtask (e.g. QUEUING, PROGRESS, RETRY, FAILURE, SUCCESS) + + * Second value is an exception returned by the innards of the method. If the retry was + successfully submitted, this value will be the RetryTaskError that retry() returns. + Otherwise, it (ought to be) the current_exception passed in. + """ + task_id = subtask_status['task_id'] + log.info("Task %s: Successfully sent to %s users; failed to send to %s users (and skipped %s users)", + task_id, subtask_status['succeeded'], subtask_status['failed'], subtask_status['skipped']) + + # Calculate time until we retry this task (in seconds): + # The value for max_retries is increased by the number of times an "infinite-retry" exception + # has been retried. We want the regular retries to trigger max-retry checking, but not these + # special retries. So we count them separately. + max_retries = _get_current_task().max_retries + subtask_status['retried_nomax'] + base_delay = _get_current_task().default_retry_delay + if skip_retry_max: + # once we reach five retries, don't increase the countdown further. + retry_index = min(subtask_status['retried_nomax'], 5) + exception_type = 'sending-rate' + # if we have a cap, after all, apply it now: + if hasattr(settings, 'BULK_EMAIL_INFINITE_RETRY_CAP'): + retry_cap = settings.BULK_EMAIL_INFINITE_RETRY_CAP + subtask_status['retried_withmax'] + max_retries = min(max_retries, retry_cap) + else: + retry_index = subtask_status['retried_withmax'] + exception_type = 'transient' + + # Skew the new countdown value by a random factor, so that not all + # retries are deferred by the same amount. + countdown = ((2 ** retry_index) * base_delay) * random.uniform(.75, 1.25) + + log.warning('Task %s: email with id %d not delivered due to %s error %s, retrying send to %d recipients in %s seconds (with max_retry=%s)', + task_id, email_id, exception_type, current_exception, len(to_list), countdown, max_retries) + + # we make sure that we update the InstructorTask with the current subtask status + # *before* actually calling retry(), to be sure that there is no race + # condition between this update and the update made by the retried task. + update_subtask_status(entry_id, task_id, subtask_status) + + # Now attempt the retry. If it succeeds, it returns a RetryTaskError that + # needs to be returned back to Celery. If it fails, we return the existing + # exception. + try: + send_course_email.retry( + args=[ + entry_id, email_id, to_list, - course_title, - course_url, - image_url, - current_task.request.retries > 0 + global_email_context, + subtask_status, ], - exc=exc, - countdown=(2 ** current_task.request.retries) * 15 + exc=current_exception, + countdown=countdown, + max_retries=max_retries, + throw=True, ) - except: - log.exception('Email with id %d caused course_email task to fail with uncaught exception. To list: %s', - email_id, - [i['email'] for i in to_list]) - # Close the connection before we exit - connection.close() - raise - - -# This string format code is wrapped in this function to allow mocking for a unit test -def course_email_result(num_sent, num_error, num_optout): - """Return the formatted result of course_email sending.""" - return "Sent {0}, Fail {1}, Optout {2}".format(num_sent, num_error, num_optout) + except RetryTaskError as retry_error: + # If the retry call is successful, update with the current progress: + log.exception('Task %s: email with id %d caused send_course_email task to retry.', + task_id, email_id) + return subtask_status, retry_error + except Exception as retry_exc: + # If there are no more retries, because the maximum has been reached, + # we expect the original exception to be raised. We catch it here + # (and put it in retry_exc just in case it's different, but it shouldn't be), + # and update status as if it were any other failure. That means that + # the recipients still in the to_list are counted as failures. + log.exception('Task %s: email with id %d caused send_course_email task to fail to retry. To list: %s', + task_id, email_id, [i['email'] for i in to_list]) + num_failed = len(to_list) + new_subtask_progress = increment_subtask_status(subtask_status, failed=num_failed, state=FAILURE) + return new_subtask_progress, retry_exc def _statsd_tag(course_title): diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email/tests/test_course_optout.py index 0adf11952701..378610eeb369 100644 --- a/lms/djangoapps/bulk_email/tests/test_course_optout.py +++ b/lms/djangoapps/bulk_email/tests/test_course_optout.py @@ -59,7 +59,7 @@ def navigate_to_email_view(self): selected_email_link = 'Email' self.assertTrue(selected_email_link in response.content) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_optout_course(self): """ Make sure student does not receive course email after opting out. @@ -88,7 +88,7 @@ def test_optout_course(self): # Assert that self.student.email not in mail.to, outbox should be empty self.assertEqual(len(mail.outbox), 0) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_optin_course(self): """ Make sure student receives course email after opting in. diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index dab7812763b4..9da787dbaa31 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -2,6 +2,8 @@ """ Unit tests for sending course email """ +from mock import patch + from django.conf import settings from django.core import mail from django.core.urlresolvers import reverse @@ -12,11 +14,8 @@ from student.tests.factories import UserFactory, GroupFactory, CourseEnrollmentFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory - -from bulk_email.tasks import delegate_email_batches, course_email -from bulk_email.models import CourseEmail, Optout - -from mock import patch +from bulk_email.models import Optout +from instructor_task.subtasks import increment_subtask_status STAFF_COUNT = 3 STUDENT_COUNT = 10 @@ -30,13 +29,13 @@ class MockCourseEmailResult(object): """ emails_sent = 0 - def get_mock_course_email_result(self): + def get_mock_increment_subtask_status(self): """Wrapper for mock email function.""" - def mock_course_email_result(sent, failed, output, **kwargs): # pylint: disable=W0613 + def mock_increment_subtask_status(original_status, **kwargs): # pylint: disable=W0613 """Increments count of number of emails sent.""" - self.emails_sent += sent - return True - return mock_course_email_result + self.emails_sent += kwargs.get('succeeded', 0) + return increment_subtask_status(original_status, **kwargs) + return mock_increment_subtask_status @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) @@ -45,7 +44,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): Test that emails send correctly. """ - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def setUp(self): self.course = CourseFactory.create() self.instructor = UserFactory.create(username="instructor", email="robot+instructor@edx.org") @@ -244,14 +243,14 @@ def test_unicode_students_send_to_all(self): [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students] ) - @override_settings(EMAILS_PER_TASK=3, EMAILS_PER_QUERY=7) - @patch('bulk_email.tasks.course_email_result') + @override_settings(BULK_EMAIL_EMAILS_PER_TASK=3, BULK_EMAIL_EMAILS_PER_QUERY=7) + @patch('bulk_email.tasks.increment_subtask_status') def test_chunked_queries_send_numerous_emails(self, email_mock): """ Test sending a large number of emails, to test the chunked querying """ mock_factory = MockCourseEmailResult() - email_mock.side_effect = mock_factory.get_mock_course_email_result() + email_mock.side_effect = mock_factory.get_mock_increment_subtask_status() added_users = [] for _ in xrange(LARGE_NUM_EMAILS): user = UserFactory() @@ -281,14 +280,3 @@ def test_chunked_queries_send_numerous_emails(self, email_mock): [s.email for s in self.students] + [s.email for s in added_users if s not in optouts]) self.assertItemsEqual(outbox_contents, should_send_contents) - - -@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) -class TestEmailSendExceptions(ModuleStoreTestCase): - """ - Test that exceptions are handled correctly. - """ - def test_no_course_email_obj(self): - # Make sure course_email handles CourseEmail.DoesNotExist exception. - with self.assertRaises(CourseEmail.DoesNotExist): - course_email(101, [], "_", "_", "_", False) diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index abdbf4dc3b13..a7ec72c56ed8 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -2,21 +2,24 @@ Unit tests for handling email sending errors """ from itertools import cycle +from mock import patch +from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError + from django.test.utils import override_settings from django.conf import settings from django.core.management import call_command from django.core.urlresolvers import reverse + from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory -from bulk_email.models import CourseEmail -from bulk_email.tasks import delegate_email_batches - -from mock import patch, Mock -from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError +from bulk_email.models import CourseEmail, SEND_TO_ALL +from bulk_email.tasks import perform_delegate_email_batches, send_course_email +from instructor_task.models import InstructorTask +from instructor_task.subtasks import create_subtask_status, initialize_subtask_info class EmailTestException(Exception): @@ -43,7 +46,7 @@ def tearDown(self): patch.stopall() @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.course_email.retry') + @patch('bulk_email.tasks.send_course_email.retry') def test_data_err_retry(self, retry, get_conn): """ Test that celery handles transient SMTPDataErrors by retrying. @@ -64,15 +67,16 @@ def test_data_err_retry(self, retry, get_conn): self.assertTrue(type(exc) == SMTPDataError) @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.course_email_result') - @patch('bulk_email.tasks.course_email.retry') + @patch('bulk_email.tasks.increment_subtask_status') + @patch('bulk_email.tasks.send_course_email.retry') def test_data_err_fail(self, retry, result, get_conn): """ Test that celery handles permanent SMTPDataErrors by failing and not retrying. """ + # have every fourth email fail due to blacklisting: get_conn.return_value.send_messages.side_effect = cycle([SMTPDataError(554, "Email address is blacklisted"), - None]) - students = [UserFactory() for _ in xrange(settings.EMAILS_PER_TASK)] + None, None, None]) + students = [UserFactory() for _ in xrange(settings.BULK_EMAIL_EMAILS_PER_TASK)] for student in students: CourseEnrollmentFactory.create(user=student, course_id=self.course.id) @@ -87,13 +91,14 @@ def test_data_err_fail(self, retry, result, get_conn): # We shouldn't retry when hitting a 5xx error self.assertFalse(retry.called) # Test that after the rejected email, the rest still successfully send - ((sent, fail, optouts), _) = result.call_args - self.assertEquals(optouts, 0) - self.assertEquals(fail, settings.EMAILS_PER_TASK / 2) - self.assertEquals(sent, settings.EMAILS_PER_TASK / 2) + ((_initial_results), kwargs) = result.call_args + self.assertEquals(kwargs['skipped'], 0) + expected_fails = int((settings.BULK_EMAIL_EMAILS_PER_TASK + 3) / 4.0) + self.assertEquals(kwargs['failed'], expected_fails) + self.assertEquals(kwargs['succeeded'], settings.BULK_EMAIL_EMAILS_PER_TASK - expected_fails) @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.course_email.retry') + @patch('bulk_email.tasks.send_course_email.retry') def test_disconn_err_retry(self, retry, get_conn): """ Test that celery handles SMTPServerDisconnected by retrying. @@ -113,7 +118,7 @@ def test_disconn_err_retry(self, retry, get_conn): self.assertTrue(type(exc) == SMTPServerDisconnected) @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.course_email.retry') + @patch('bulk_email.tasks.send_course_email.retry') def test_conn_err_retry(self, retry, get_conn): """ Test that celery handles SMTPConnectError by retrying. @@ -133,67 +138,107 @@ def test_conn_err_retry(self, retry, get_conn): exc = kwargs['exc'] self.assertTrue(type(exc) == SMTPConnectError) - @patch('bulk_email.tasks.course_email_result') - @patch('bulk_email.tasks.course_email.retry') + @patch('bulk_email.tasks.increment_subtask_status') @patch('bulk_email.tasks.log') - @patch('bulk_email.tasks.get_connection', Mock(return_value=EmailTestException)) - def test_general_exception(self, mock_log, retry, result): - """ - Tests the if the error is not SMTP-related, we log and reraise - """ - test_email = { - 'action': 'Send email', - 'to_option': 'myself', - 'subject': 'test subject for myself', - 'message': 'test message for myself' - } - # For some reason (probably the weirdness of testing with celery tasks) assertRaises doesn't work here - # so we assert on the arguments of log.exception - self.client.post(self.url, test_email) - ((log_str, email_id, to_list), _) = mock_log.exception.call_args - self.assertTrue(mock_log.exception.called) - self.assertIn('caused course_email task to fail with uncaught exception.', log_str) - self.assertEqual(email_id, 1) - self.assertEqual(to_list, [self.instructor.email]) - self.assertFalse(retry.called) - self.assertFalse(result.called) - - @patch('bulk_email.tasks.course_email_result') - @patch('bulk_email.tasks.delegate_email_batches.retry') - @patch('bulk_email.tasks.log') - def test_nonexist_email(self, mock_log, retry, result): + def test_nonexistent_email(self, mock_log, result): """ Tests retries when the email doesn't exist """ - delegate_email_batches.delay(-1, self.instructor.id) - ((log_str, email_id, _num_retries), _) = mock_log.warning.call_args + # create an InstructorTask object to pass through + course_id = self.course.id + entry = InstructorTask.create(course_id, "task_type", "task_key", "task_input", self.instructor) + task_input = {"email_id": -1} + with self.assertRaises(CourseEmail.DoesNotExist): + perform_delegate_email_batches(entry.id, course_id, task_input, "action_name") # pylint: disable=E1101 + ((log_str, _, email_id), _) = mock_log.warning.call_args self.assertTrue(mock_log.warning.called) self.assertIn('Failed to get CourseEmail with id', log_str) self.assertEqual(email_id, -1) - self.assertTrue(retry.called) self.assertFalse(result.called) - @patch('bulk_email.tasks.log') - def test_nonexist_course(self, mock_log): + def test_nonexistent_course(self): """ Tests exception when the course in the email doesn't exist """ - email = CourseEmail(course_id="I/DONT/EXIST") + course_id = "I/DONT/EXIST" + email = CourseEmail(course_id=course_id) email.save() - delegate_email_batches.delay(email.id, self.instructor.id) - ((log_str, _), _) = mock_log.exception.call_args - self.assertTrue(mock_log.exception.called) - self.assertIn('get_course_by_id failed:', log_str) + entry = InstructorTask.create(course_id, "task_type", "task_key", "task_input", self.instructor) + task_input = {"email_id": email.id} # pylint: disable=E1101 + with self.assertRaisesRegexp(ValueError, "Course not found"): + perform_delegate_email_batches(entry.id, course_id, task_input, "action_name") # pylint: disable=E1101 - @patch('bulk_email.tasks.log') - def test_nonexist_to_option(self, mock_log): + def test_nonexistent_to_option(self): """ Tests exception when the to_option in the email doesn't exist """ email = CourseEmail(course_id=self.course.id, to_option="IDONTEXIST") email.save() - delegate_email_batches.delay(email.id, self.instructor.id) - ((log_str, opt_str), _) = mock_log.error.call_args - self.assertTrue(mock_log.error.called) - self.assertIn('Unexpected bulk email TO_OPTION found', log_str) - self.assertEqual("IDONTEXIST", opt_str) + entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor) + task_input = {"email_id": email.id} # pylint: disable=E1101 + with self.assertRaisesRegexp(Exception, 'Unexpected bulk email TO_OPTION found: IDONTEXIST'): + perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name") # pylint: disable=E1101 + + def test_wrong_course_id_in_task(self): + """ + Tests exception when the course_id in task is not the same as one explicitly passed in. + """ + email = CourseEmail(course_id=self.course.id, to_option=SEND_TO_ALL) + email.save() + entry = InstructorTask.create("bogus_task_id", "task_type", "task_key", "task_input", self.instructor) + task_input = {"email_id": email.id} # pylint: disable=E1101 + with self.assertRaisesRegexp(ValueError, 'does not match task value'): + perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name") # pylint: disable=E1101 + + def test_wrong_course_id_in_email(self): + """ + Tests exception when the course_id in CourseEmail is not the same as one explicitly passed in. + """ + email = CourseEmail(course_id="bogus_course_id", to_option=SEND_TO_ALL) + email.save() + entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor) + task_input = {"email_id": email.id} # pylint: disable=E1101 + with self.assertRaisesRegexp(ValueError, 'does not match email value'): + perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name") # pylint: disable=E1101 + + def test_send_email_undefined_subtask(self): + # test at a lower level, to ensure that the course gets checked down below too. + entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor) + entry_id = entry.id # pylint: disable=E1101 + to_list = ['test@test.com'] + global_email_context = {'course_title': 'dummy course'} + subtask_id = "subtask-id-value" + subtask_status = create_subtask_status(subtask_id) + email_id = 1001 + with self.assertRaisesRegexp(ValueError, 'unable to find email subtasks of instructor task'): + send_course_email(entry_id, email_id, to_list, global_email_context, subtask_status) + + def test_send_email_missing_subtask(self): + # test at a lower level, to ensure that the course gets checked down below too. + entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor) + entry_id = entry.id # pylint: disable=E1101 + to_list = ['test@test.com'] + global_email_context = {'course_title': 'dummy course'} + subtask_id = "subtask-id-value" + initialize_subtask_info(entry, "emailed", 100, [subtask_id]) + different_subtask_id = "bogus-subtask-id-value" + subtask_status = create_subtask_status(different_subtask_id) + bogus_email_id = 1001 + with self.assertRaisesRegexp(ValueError, 'unable to find status for email subtask of instructor task'): + send_course_email(entry_id, bogus_email_id, to_list, global_email_context, subtask_status) + + def dont_test_send_email_undefined_email(self): + # test at a lower level, to ensure that the course gets checked down below too. + entry = InstructorTask.create(self.course.id, "task_type", "task_key", "task_input", self.instructor) + entry_id = entry.id # pylint: disable=E1101 + to_list = ['test@test.com'] + global_email_context = {'course_title': 'dummy course'} + subtask_id = "subtask-id-value" + initialize_subtask_info(entry, "emailed", 100, [subtask_id]) + subtask_status = create_subtask_status(subtask_id) + bogus_email_id = 1001 + with self.assertRaises(CourseEmail.DoesNotExist): + # we skip the call that updates subtask status, since we've not set up the InstructorTask + # for the subtask, and it's not important to the test. + with patch('bulk_email.tasks.update_subtask_status'): + send_course_email(entry_id, bogus_email_id, to_list, global_email_context, subtask_status) diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py new file mode 100644 index 000000000000..60a8abf95e1c --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_forms.py @@ -0,0 +1,105 @@ +""" +Unit tests for bulk-email-related forms. +""" +from django.test.utils import override_settings +from django.conf import settings + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE +from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE + +from xmodule.modulestore.django import modulestore +from xmodule.modulestore import MONGO_MODULESTORE_TYPE + +from mock import patch + +from bulk_email.models import CourseAuthorization +from bulk_email.forms import CourseAuthorizationAdminForm + + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class CourseAuthorizationFormTest(ModuleStoreTestCase): + """Test the CourseAuthorizationAdminForm form for Mongo-backed courses.""" + + def setUp(self): + # Make a mongo course + self.course = CourseFactory.create() + + def tearDown(self): + """ + Undo all patches. + """ + patch.stopall() + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_authorize_mongo_course(self): + # Initially course shouldn't be authorized + self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id)) + # Test authorizing the course, which should totally work + form_data = {'course_id': self.course.id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation should work + self.assertTrue(form.is_valid()) + form.save() + # Check that this course is authorized + self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id)) + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_form_typo(self): + # Munge course id + bad_id = self.course.id + '_typo' + + form_data = {'course_id': bad_id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation shouldn't work + self.assertFalse(form.is_valid()) + + msg = u'Error encountered (Course not found.)' + msg += ' --- Entered course id was: "{0}". '.format(bad_id) + msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN' + self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + + with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): + form.save() + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_course_name_only(self): + # Munge course id - common + bad_id = self.course.id.split('/')[-1] + + form_data = {'course_id': bad_id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation shouldn't work + self.assertFalse(form.is_valid()) + + msg = u'Error encountered (Need more than 1 value to unpack)' + msg += ' --- Entered course id was: "{0}". '.format(bad_id) + msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN' + self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + + with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): + form.save() + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class CourseAuthorizationXMLFormTest(ModuleStoreTestCase): + """Check that XML courses cannot be authorized for email.""" + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_xml_course_authorization(self): + course_id = 'edX/toy/2012_Fall' + # Assert this is an XML course + self.assertTrue(modulestore().get_modulestore_type(course_id) != MONGO_MODULESTORE_TYPE) + + form_data = {'course_id': course_id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation shouldn't work + self.assertFalse(form.is_valid()) + + msg = u"Course Email feature is only available for courses authored in Studio. " + msg += '"{0}" appears to be an XML backed course.'.format(course_id) + self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + + with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): + form.save() diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py new file mode 100644 index 000000000000..30713e178c57 --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_models.py @@ -0,0 +1,147 @@ +""" +Unit tests for bulk-email-related models. +""" +from django.test import TestCase +from django.core.management import call_command +from django.conf import settings + +from student.tests.factories import UserFactory + +from mock import patch + +from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization + + +class CourseEmailTest(TestCase): + """Test the CourseEmail model.""" + + def test_creation(self): + course_id = 'abc/123/doremi' + sender = UserFactory.create() + to_option = SEND_TO_STAFF + subject = "dummy subject" + html_message = "dummy message" + email = CourseEmail.create(course_id, sender, to_option, subject, html_message) + self.assertEquals(email.course_id, course_id) + self.assertEquals(email.to_option, SEND_TO_STAFF) + self.assertEquals(email.subject, subject) + self.assertEquals(email.html_message, html_message) + self.assertEquals(email.sender, sender) + + def test_bad_to_option(self): + course_id = 'abc/123/doremi' + sender = UserFactory.create() + to_option = "fake" + subject = "dummy subject" + html_message = "dummy message" + with self.assertRaises(ValueError): + CourseEmail.create(course_id, sender, to_option, subject, html_message) + + +class NoCourseEmailTemplateTest(TestCase): + """Test the CourseEmailTemplate model without loading the template data.""" + + def test_get_missing_template(self): + with self.assertRaises(CourseEmailTemplate.DoesNotExist): + CourseEmailTemplate.get_template() + + +class CourseEmailTemplateTest(TestCase): + """Test the CourseEmailTemplate model.""" + + def setUp(self): + # load initial content (since we don't run migrations as part of tests): + call_command("loaddata", "course_email_template.json") + + def _get_sample_plain_context(self): + """Provide sample context sufficient for rendering plaintext template""" + context = { + 'course_title': "Bogus Course Title", + 'course_url': "/location/of/course/url", + 'account_settings_url': "/location/of/account/settings/url", + 'platform_name': 'edX', + 'email': 'your-email@test.com', + } + return context + + def _get_sample_html_context(self): + """Provide sample context sufficient for rendering HTML template""" + context = self._get_sample_plain_context() + context['course_image_url'] = "/location/of/course/image/url" + return context + + def test_get_template(self): + template = CourseEmailTemplate.get_template() + self.assertIsNotNone(template.html_template) + self.assertIsNotNone(template.plain_template) + + def test_render_html_without_context(self): + template = CourseEmailTemplate.get_template() + base_context = self._get_sample_html_context() + for keyname in base_context: + context = dict(base_context) + del context[keyname] + with self.assertRaises(KeyError): + template.render_htmltext("My new html text.", context) + + def test_render_plaintext_without_context(self): + template = CourseEmailTemplate.get_template() + base_context = self._get_sample_plain_context() + for keyname in base_context: + context = dict(base_context) + del context[keyname] + with self.assertRaises(KeyError): + template.render_plaintext("My new plain text.", context) + + def test_render_html(self): + template = CourseEmailTemplate.get_template() + context = self._get_sample_html_context() + template.render_htmltext("My new html text.", context) + + def test_render_plain(self): + template = CourseEmailTemplate.get_template() + context = self._get_sample_plain_context() + template.render_plaintext("My new plain text.", context) + + +class CourseAuthorizationTest(TestCase): + """Test the CourseAuthorization model.""" + + @patch.dict(settings.MITX_FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_creation_auth_on(self): + course_id = 'abc/123/doremi' + # Test that course is not authorized by default + self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id)) + + # Authorize + cauth = CourseAuthorization(course_id=course_id, email_enabled=True) + cauth.save() + # Now, course should be authorized + self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id)) + self.assertEquals( + cauth.__unicode__(), + "Course 'abc/123/doremi': Instructor Email Enabled" + ) + + # Unauthorize by explicitly setting email_enabled to False + cauth.email_enabled = False + cauth.save() + # Test that course is now unauthorized + self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id)) + self.assertEquals( + cauth.__unicode__(), + "Course 'abc/123/doremi': Instructor Email Not Enabled" + ) + + @patch.dict(settings.MITX_FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': False}) + def test_creation_auth_off(self): + course_id = 'blahx/blah101/ehhhhhhh' + # Test that course is authorized by default, since auth is turned off + self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id)) + + # Use the admin interface to unauthorize the course + cauth = CourseAuthorization(course_id=course_id, email_enabled=False) + cauth.save() + + # Now, course should STILL be authorized! + self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id)) diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email/tests/test_tasks.py new file mode 100644 index 000000000000..e49a04147a3d --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_tasks.py @@ -0,0 +1,398 @@ +""" +Unit tests for LMS instructor-initiated background tasks. + +Runs tasks on answers to course problems to validate that code +paths actually work. + +""" +import json +from uuid import uuid4 +from itertools import cycle, chain, repeat +from mock import patch, Mock +from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPAuthenticationError +from boto.ses.exceptions import ( + SESDailyQuotaExceededError, + SESMaxSendingRateExceededError, + SESAddressBlacklistedError, + SESIllegalAddressError, + SESLocalAddressCharacterError, +) +from boto.exception import AWSConnectionError + +from celery.states import SUCCESS, FAILURE + +from django.conf import settings +from django.core.management import call_command + +from bulk_email.models import CourseEmail, Optout, SEND_TO_ALL + +from instructor_task.tasks import send_bulk_course_email +from instructor_task.subtasks import update_subtask_status +from instructor_task.models import InstructorTask +from instructor_task.tests.test_base import InstructorTaskCourseTestCase +from instructor_task.tests.factories import InstructorTaskFactory + + +class TestTaskFailure(Exception): + """Dummy exception used for unit tests.""" + pass + + +def my_update_subtask_status(entry_id, current_task_id, new_subtask_status): + """ + Check whether a subtask has been updated before really updating. + + Check whether a subtask which has been retried + has had the retry already write its results here before the code + that was invoking the retry had a chance to update this status. + + This is the norm in "eager" mode (used by tests) where the retry is called + and run to completion before control is returned to the code that + invoked the retry. If the retries eventually end in failure (e.g. due to + a maximum number of retries being attempted), the "eager" code will return + the error for each retry as it is popped off the stack. We want to just ignore + the later updates that are called as the result of the earlier retries. + + This should not be an issue in production, where status is updated before + a task is retried, and is then updated afterwards if the retry fails. + """ + entry = InstructorTask.objects.get(pk=entry_id) + subtask_dict = json.loads(entry.subtasks) + subtask_status_info = subtask_dict['status'] + current_subtask_status = subtask_status_info[current_task_id] + + def _get_retry_count(subtask_result): + """Return the number of retries counted for the given subtask.""" + retry_count = subtask_result.get('retried_nomax', 0) + retry_count += subtask_result.get('retried_withmax', 0) + return retry_count + + current_retry_count = _get_retry_count(current_subtask_status) + new_retry_count = _get_retry_count(new_subtask_status) + if current_retry_count <= new_retry_count: + update_subtask_status(entry_id, current_task_id, new_subtask_status) + + +class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): + """Tests instructor task that send bulk email.""" + + def setUp(self): + super(TestBulkEmailInstructorTask, self).setUp() + self.initialize_course() + self.instructor = self.create_instructor('instructor') + + # load initial content (since we don't run migrations as part of tests): + call_command("loaddata", "course_email_template.json") + + def _create_input_entry(self, course_id=None): + """ + Creates a InstructorTask entry for testing. + + Overrides the base class version in that this creates CourseEmail. + """ + to_option = SEND_TO_ALL + course_id = course_id or self.course.id + course_email = CourseEmail.create(course_id, self.instructor, to_option, "Test Subject", "

    This is a test message

    ") + task_input = {'email_id': course_email.id} # pylint: disable=E1101 + task_id = str(uuid4()) + instructor_task = InstructorTaskFactory.create( + course_id=course_id, + requester=self.instructor, + task_input=json.dumps(task_input), + task_key='dummy value', + task_id=task_id, + ) + return instructor_task + + def _run_task_with_mock_celery(self, task_class, entry_id, task_id): + """Submit a task and mock how celery provides a current_task.""" + mock_current_task = Mock() + mock_current_task.max_retries = settings.BULK_EMAIL_MAX_RETRIES + mock_current_task.default_retry_delay = settings.BULK_EMAIL_DEFAULT_RETRY_DELAY + task_args = [entry_id, {}] + + with patch('bulk_email.tasks._get_current_task') as mock_get_task: + mock_get_task.return_value = mock_current_task + return task_class.apply(task_args, task_id=task_id).get() + + def test_email_missing_current_task(self): + task_entry = self._create_input_entry() + with self.assertRaises(ValueError): + send_bulk_course_email(task_entry.id, {}) + + def test_email_undefined_course(self): + # Check that we fail when passing in a course that doesn't exist. + task_entry = self._create_input_entry(course_id="bogus/course/id") + with self.assertRaises(ValueError): + self._run_task_with_mock_celery(send_bulk_course_email, task_entry.id, task_entry.task_id) + + def test_bad_task_id_on_update(self): + task_entry = self._create_input_entry() + + def dummy_update_subtask_status(entry_id, _current_task_id, new_subtask_status): + """Passes a bad value for task_id to test update_subtask_status""" + bogus_task_id = "this-is-bogus" + update_subtask_status(entry_id, bogus_task_id, new_subtask_status) + + with self.assertRaises(ValueError): + with patch('bulk_email.tasks.update_subtask_status', dummy_update_subtask_status): + send_bulk_course_email(task_entry.id, {}) # pylint: disable=E1101 + + def _create_students(self, num_students): + """Create students for testing""" + return [self.create_student('robot%d' % i) for i in xrange(num_students)] + + def _assert_single_subtask_status(self, entry, succeeded, failed=0, skipped=0, retried_nomax=0, retried_withmax=0): + """Compare counts with 'subtasks' entry in InstructorTask table.""" + subtask_info = json.loads(entry.subtasks) + # verify subtask-level counts: + self.assertEquals(subtask_info.get('total'), 1) + self.assertEquals(subtask_info.get('succeeded'), 1 if succeeded > 0 else 0) + self.assertEquals(subtask_info.get('failed'), 0 if succeeded > 0 else 1) + # verify individual subtask status: + subtask_status_info = subtask_info.get('status') + task_id_list = subtask_status_info.keys() + self.assertEquals(len(task_id_list), 1) + task_id = task_id_list[0] + subtask_status = subtask_status_info.get(task_id) + print("Testing subtask status: {}".format(subtask_status)) + self.assertEquals(subtask_status.get('task_id'), task_id) + self.assertEquals(subtask_status.get('attempted'), succeeded + failed) + self.assertEquals(subtask_status.get('succeeded'), succeeded) + self.assertEquals(subtask_status.get('skipped'), skipped) + self.assertEquals(subtask_status.get('failed'), failed) + self.assertEquals(subtask_status.get('retried_nomax'), retried_nomax) + self.assertEquals(subtask_status.get('retried_withmax'), retried_withmax) + self.assertEquals(subtask_status.get('state'), SUCCESS if succeeded > 0 else FAILURE) + + def _test_run_with_task(self, task_class, action_name, total, succeeded, failed=0, skipped=0, retried_nomax=0, retried_withmax=0): + """Run a task and check the number of emails processed.""" + task_entry = self._create_input_entry() + parent_status = self._run_task_with_mock_celery(task_class, task_entry.id, task_entry.task_id) + + # check return value + self.assertEquals(parent_status.get('total'), total) + self.assertEquals(parent_status.get('action_name'), action_name) + + # compare with task_output entry in InstructorTask table: + entry = InstructorTask.objects.get(id=task_entry.id) + status = json.loads(entry.task_output) + self.assertEquals(status.get('attempted'), succeeded + failed) + self.assertEquals(status.get('succeeded'), succeeded) + self.assertEquals(status.get('skipped'), skipped) + self.assertEquals(status.get('failed'), failed) + self.assertEquals(status.get('total'), total) + self.assertEquals(status.get('action_name'), action_name) + self.assertGreater(status.get('duration_ms'), 0) + self.assertEquals(entry.task_state, SUCCESS) + self._assert_single_subtask_status(entry, succeeded, failed, skipped, retried_nomax, retried_withmax) + return entry + + def test_successful(self): + # Select number of emails to fit into a single subtask. + num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK + # We also send email to the instructor: + self._create_students(num_emails - 1) + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + get_conn.return_value.send_messages.side_effect = cycle([None]) + self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, num_emails) + + def test_successful_twice(self): + # Select number of emails to fit into a single subtask. + num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK + # We also send email to the instructor: + self._create_students(num_emails - 1) + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + get_conn.return_value.send_messages.side_effect = cycle([None]) + task_entry = self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, num_emails) + + # submit the same task a second time, and confirm that it is not run again. + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + get_conn.return_value.send_messages.side_effect = cycle([Exception("This should not happen!")]) + parent_status = self._run_task_with_mock_celery(send_bulk_course_email, task_entry.id, task_entry.task_id) + self.assertEquals(parent_status.get('total'), num_emails) + self.assertEquals(parent_status.get('succeeded'), num_emails) + self.assertEquals(parent_status.get('failed'), 0) + + def test_unactivated_user(self): + # Select number of emails to fit into a single subtask. + num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK + # We also send email to the instructor: + students = self._create_students(num_emails - 1) + # mark a student as not yet having activated their email: + student = students[0] + student.is_active = False + student.save() + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + get_conn.return_value.send_messages.side_effect = cycle([None]) + self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails - 1, num_emails - 1) + + def test_skipped(self): + # Select number of emails to fit into a single subtask. + num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK + # We also send email to the instructor: + students = self._create_students(num_emails - 1) + # have every fourth student optout: + expected_skipped = int((num_emails + 3) / 4.0) + expected_succeeds = num_emails - expected_skipped + for index in range(0, num_emails, 4): + Optout.objects.create(user=students[index], course_id=self.course.id) + # mark some students as opting out + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + get_conn.return_value.send_messages.side_effect = cycle([None]) + self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, skipped=expected_skipped) + + def _test_email_address_failures(self, exception): + """Test that celery handles bad address errors by failing and not retrying.""" + # Select number of emails to fit into a single subtask. + num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK + # We also send email to the instructor: + self._create_students(num_emails - 1) + expected_fails = int((num_emails + 3) / 4.0) + expected_succeeds = num_emails - expected_fails + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + # have every fourth email fail due to some address failure: + get_conn.return_value.send_messages.side_effect = cycle([exception, None, None, None]) + self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, failed=expected_fails) + + def test_smtp_blacklisted_user(self): + # Test that celery handles permanent SMTPDataErrors by failing and not retrying. + self._test_email_address_failures(SMTPDataError(554, "Email address is blacklisted")) + + def test_ses_blacklisted_user(self): + # Test that celery handles permanent SMTPDataErrors by failing and not retrying. + self._test_email_address_failures(SESAddressBlacklistedError(554, "Email address is blacklisted")) + + def test_ses_illegal_address(self): + # Test that celery handles permanent SMTPDataErrors by failing and not retrying. + self._test_email_address_failures(SESIllegalAddressError(554, "Email address is illegal")) + + def test_ses_local_address_character_error(self): + # Test that celery handles permanent SMTPDataErrors by failing and not retrying. + self._test_email_address_failures(SESLocalAddressCharacterError(554, "Email address contains a bad character")) + + def _test_retry_after_limited_retry_error(self, exception): + """Test that celery handles connection failures by retrying.""" + # If we want the batch to succeed, we need to send fewer emails + # than the max retries, so that the max is not triggered. + num_emails = settings.BULK_EMAIL_MAX_RETRIES + # We also send email to the instructor: + self._create_students(num_emails - 1) + expected_fails = 0 + expected_succeeds = num_emails + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + # Have every other mail attempt fail due to disconnection. + get_conn.return_value.send_messages.side_effect = cycle([exception, None]) + self._test_run_with_task( + send_bulk_course_email, + 'emailed', + num_emails, + expected_succeeds, + failed=expected_fails, + retried_withmax=num_emails + ) + + def _test_max_retry_limit_causes_failure(self, exception): + """Test that celery can hit a maximum number of retries.""" + # Doesn't really matter how many recipients, since we expect + # to fail on the first. + num_emails = 10 + # We also send email to the instructor: + self._create_students(num_emails - 1) + expected_fails = num_emails + expected_succeeds = 0 + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + # always fail to connect, triggering repeated retries until limit is hit: + get_conn.return_value.send_messages.side_effect = cycle([exception]) + with patch('bulk_email.tasks.update_subtask_status', my_update_subtask_status): + self._test_run_with_task( + send_bulk_course_email, + 'emailed', + num_emails, + expected_succeeds, + failed=expected_fails, + retried_withmax=(settings.BULK_EMAIL_MAX_RETRIES + 1) + ) + + def test_retry_after_smtp_disconnect(self): + self._test_retry_after_limited_retry_error(SMTPServerDisconnected(425, "Disconnecting")) + + def test_max_retry_after_smtp_disconnect(self): + self._test_max_retry_limit_causes_failure(SMTPServerDisconnected(425, "Disconnecting")) + + def test_retry_after_smtp_connect_error(self): + self._test_retry_after_limited_retry_error(SMTPConnectError(424, "Bad Connection")) + + def test_max_retry_after_smtp_connect_error(self): + self._test_max_retry_limit_causes_failure(SMTPConnectError(424, "Bad Connection")) + + def test_retry_after_aws_connect_error(self): + self._test_retry_after_limited_retry_error(AWSConnectionError("Unable to provide secure connection through proxy")) + + def test_max_retry_after_aws_connect_error(self): + self._test_max_retry_limit_causes_failure(AWSConnectionError("Unable to provide secure connection through proxy")) + + def test_retry_after_general_error(self): + self._test_retry_after_limited_retry_error(Exception("This is some random exception.")) + + def test_max_retry_after_general_error(self): + self._test_max_retry_limit_causes_failure(Exception("This is some random exception.")) + + def _test_retry_after_unlimited_retry_error(self, exception): + """Test that celery handles throttling failures by retrying.""" + num_emails = 8 + # We also send email to the instructor: + self._create_students(num_emails - 1) + expected_fails = 0 + expected_succeeds = num_emails + # Note that because celery in eager mode will call retries synchronously, + # each retry will increase the stack depth. It turns out that there is a + # maximum depth at which a RuntimeError is raised ("maximum recursion depth + # exceeded"). The maximum recursion depth is 90, so + # num_emails * expected_retries < 90. + expected_retries = 10 + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + # Cycle through N throttling errors followed by a success. + get_conn.return_value.send_messages.side_effect = cycle( + chain(repeat(exception, expected_retries), [None]) + ) + self._test_run_with_task( + send_bulk_course_email, + 'emailed', + num_emails, + expected_succeeds, + failed=expected_fails, + retried_nomax=(expected_retries * num_emails) + ) + + def test_retry_after_smtp_throttling_error(self): + self._test_retry_after_unlimited_retry_error(SMTPDataError(455, "Throttling: Sending rate exceeded")) + + def test_retry_after_ses_throttling_error(self): + self._test_retry_after_unlimited_retry_error(SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded")) + + def _test_immediate_failure(self, exception): + """Test that celery can hit a maximum number of retries.""" + # Doesn't really matter how many recipients, since we expect + # to fail on the first. + num_emails = 10 + # We also send email to the instructor: + self._create_students(num_emails - 1) + expected_fails = num_emails + expected_succeeds = 0 + with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + # always fail to connect, triggering repeated retries until limit is hit: + get_conn.return_value.send_messages.side_effect = cycle([exception]) + self._test_run_with_task( + send_bulk_course_email, + 'emailed', + num_emails, + expected_succeeds, + failed=expected_fails, + ) + + def test_failure_on_unhandled_smtp(self): + self._test_immediate_failure(SMTPAuthenticationError(403, "That password doesn't work!")) + + def test_failure_on_ses_quota_exceeded(self): + self._test_immediate_failure(SESDailyQuotaExceededError(403, "You're done for the day!")) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 78e1d51cbc31..106373139cf0 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -36,11 +36,31 @@ def get_request_for_thread(): del frame +def get_course(course_id, depth=0): + """ + Given a course id, return the corresponding course descriptor. + + If course_id is not valid, raises a ValueError. This is appropriate + for internal use. + + depth: The number of levels of children for the modulestore to cache. + None means infinite depth. Default is to fetch no children. + """ + try: + course_loc = CourseDescriptor.id_to_location(course_id) + return modulestore().get_instance(course_id, course_loc, depth=depth) + except (KeyError, ItemNotFoundError): + raise ValueError("Course not found: {}".format(course_id)) + except InvalidLocationError: + raise ValueError("Invalid location: {}".format(course_id)) + + def get_course_by_id(course_id, depth=0): """ Given a course id, return the corresponding course descriptor. If course_id is not valid, raises a 404. + depth: The number of levels of children for the modulestore to cache. None means infinite depth """ try: @@ -51,6 +71,7 @@ def get_course_by_id(course_id, depth=0): except InvalidLocationError: raise Http404("Invalid location") + def get_course_with_access(user, course_id, action, depth=0): """ Given a course_id, look up the corresponding course descriptor, @@ -182,7 +203,6 @@ def get_course_about_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) - def get_course_info_section(request, course, section_key): """ This returns the snippet of html to be rendered on the course info page, @@ -194,8 +214,6 @@ def get_course_info_section(request, course, section_key): - updates - guest_updates """ - - loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key) # Use an empty cache diff --git a/lms/djangoapps/courseware/features/video.feature b/lms/djangoapps/courseware/features/video.feature index 001481a6a5c1..7518bf6bdb96 100644 --- a/lms/djangoapps/courseware/features/video.feature +++ b/lms/djangoapps/courseware/features/video.feature @@ -2,38 +2,45 @@ Feature: LMS.Video component As a student, I want to view course videos in LMS. + # 1 Scenario: Video component is fully rendered in the LMS in HTML5 mode Given the course has a Video component in HTML5 mode Then when I view the video it has rendered in HTML5 mode And all sources are correct + # 2 # Firefox doesn't have HTML5 (only mp4 - fix here) @skip_firefox Scenario: Autoplay is disabled in LMS for a Video component Given the course has a Video component in HTML5 mode Then when I view the video it does not have autoplay enabled + # 3 # Youtube testing Scenario: Video component is fully rendered in the LMS in Youtube mode with HTML5 sources Given youtube server is up and response time is 0.4 seconds And the course has a Video component in Youtube_HTML5 mode Then when I view the video it has rendered in Youtube mode + # 4 Scenario: Video component is not rendered in the LMS in Youtube mode with HTML5 sources Given youtube server is up and response time is 2 seconds And the course has a Video component in Youtube_HTML5 mode Then when I view the video it has rendered in HTML5 mode + # 5 Scenario: Video component is rendered in the LMS in Youtube mode without HTML5 sources Given youtube server is up and response time is 2 seconds And the course has a Video component in Youtube mode Then when I view the video it has rendered in Youtube mode + # 6 Scenario: Video component is rendered in the LMS in Youtube mode with HTML5 sources that doesn't supported by browser Given youtube server is up and response time is 2 seconds And the course has a Video component in Youtube_HTML5_Unsupported_Video mode Then when I view the video it has rendered in Youtube mode + # 7 Scenario: Video component is rendered in the LMS in HTML5 mode with HTML5 sources that doesn't supported by browser Given the course has a Video component in HTML5_Unsupported_Video mode Then error message is shown diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 20cb83a411b9..7960515ae0bd 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -2,15 +2,18 @@ from django.test import TestCase from django.http import Http404 from django.test.utils import override_settings -from courseware.courses import get_course_by_id, get_cms_course_link_by_id +from courseware.courses import get_course_by_id, get_course, get_cms_course_link_by_id CMS_BASE_TEST = 'testcms' + class CoursesTest(TestCase): + """Test methods related to fetching courses.""" + def test_get_course_by_id_invalid_chars(self): """ Test that `get_course_by_id` throws a 404, rather than - an exception, when faced with unexpected characters + an exception, when faced with unexpected characters (such as unicode characters, and symbols such as = and ' ') """ with self.assertRaises(Http404): @@ -18,6 +21,17 @@ def test_get_course_by_id_invalid_chars(self): get_course_by_id('MITx/foobar/business and management') get_course_by_id('MITx/foobar/NiñøJoséMaríáßç') + def test_get_course_invalid_chars(self): + """ + Test that `get_course` throws a ValueError, rather than + a 404, when faced with unexpected characters + (such as unicode characters, and symbols such as = and ' ') + """ + with self.assertRaises(ValueError): + get_course('MITx/foobar/statistics=introduction') + get_course('MITx/foobar/business and management') + get_course('MITx/foobar/NiñøJoséMaríáßç') + @override_settings(CMS_BASE=CMS_BASE_TEST) def test_get_cms_course_link_by_id(self): """ diff --git a/lms/djangoapps/instructor/features/bulk_email.feature b/lms/djangoapps/instructor/features/bulk_email.feature new file mode 100644 index 000000000000..8d3784c1ea36 --- /dev/null +++ b/lms/djangoapps/instructor/features/bulk_email.feature @@ -0,0 +1,20 @@ +@shard_2 +Feature: Bulk Email + As an instructor or course staff, + In order to communicate with students and staff + I want to send email to staff and students in a course. + + Scenario: Send bulk email + Given I am "" for a course + When I send email to "" + Then Email is sent to "" + + Examples: + | Role | Recipient | + | instructor | myself | + | instructor | course staff | + | instructor | students, staff, and instructors | + | staff | myself | + | staff | course staff | + | staff | students, staff, and instructors | + diff --git a/lms/djangoapps/instructor/features/bulk_email.py b/lms/djangoapps/instructor/features/bulk_email.py new file mode 100644 index 000000000000..220ab9ecee55 --- /dev/null +++ b/lms/djangoapps/instructor/features/bulk_email.py @@ -0,0 +1,162 @@ +""" +Define steps for bulk email acceptance test. +""" + +#pylint: disable=C0111 +#pylint: disable=W0621 + +from lettuce import world, step +from lettuce.django import mail +from nose.tools import assert_in, assert_true, assert_equal # pylint: disable=E0611 +from django.core.management import call_command +from django.conf import settings + + +@step(u'I am an instructor for a course') +def i_am_an_instructor(step): # pylint: disable=W0613 + + # Clear existing courses to avoid conflicts + world.clear_courses() + + # Register the instructor as staff for the course + world.register_by_course_id( + 'edx/999/Test_Course', + username='instructor', + password='password', + is_staff=True + ) + world.add_to_course_staff('instructor', '999') + + # Register another staff member + world.register_by_course_id( + 'edx/999/Test_Course', + username='staff', + password='password', + is_staff=True + ) + world.add_to_course_staff('staff', '999') + + # Register a student + world.register_by_course_id( + 'edx/999/Test_Course', + username='student', + password='password', + is_staff=False + ) + + # Log in as the instructor for the course + world.log_in( + username='instructor', + password='password', + email="instructor@edx.org", + name="Instructor" + ) + + +# Dictionary mapping a description of the email recipient +# to the corresponding