From 74887aa21654a042aa9fee57068fef333e4fb386 Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Mon, 22 Feb 2021 14:02:15 -0500 Subject: [PATCH] feat: turn on schedule creation by default This commit removes several waffle toggles that have been enabled on edx.org for years. It's time to remove the rollout gating for these features and enable them by default. This doesn't directly change any behavior. But it does create new database objects by default now and allows for enabling other schedule based features more easily. Specifically, the following toggles were affected. schedules.create_schedules_for_course - Waffle flag removed as always-enabled - We now always create a schedule when an enrollment is created schedules.send_updates_for_course - Waffle flag removed as always-enabled - Course update emails are sent as long as the ScheduleConfig allows it. - This is not a change in default behavior, because ScheduleConfig is off by default. dynamic_pacing.studio_course_update - Waffle switch removed as always-enabled - Course teams can now always edit course updates directly in Studio ScheduleConfig.create_schedules ScheduleConfig.hold_back_ratio - Model fields for rolling out the schedules feature - Schedules are now always created - This commit only removes references to these fields, they still exist in the database. A future commit will remove them entirely This commit also adds a new has_highlights field to CourseOverview. This is used to cache whether a course has highlights, used to decide which course update email behavior they get. Previously every enrollment had to dig into the modulestore to determine that. --- .../contentstore/api/tests/test_quality.py | 2 +- .../contentstore/api/views/course_quality.py | 3 +- cms/djangoapps/contentstore/views/item.py | 10 +- .../contentstore/views/tests/test_item.py | 12 +- .../spec/views/pages/course_outline_spec.js | 18 +-- cms/templates/js/course-outline.underscore | 2 +- cms/templates/js/highlights-editor.underscore | 6 - .../rest_api/v1/tests/test_views.py | 11 +- .../djangoapps/student/tests/test_models.py | 18 +-- common/djangoapps/student/tests/test_views.py | 7 -- common/djangoapps/student/tests/tests.py | 2 +- .../courseware/tests/test_course_tools.py | 10 -- .../courseware/tests/test_date_summary.py | 15 --- lms/djangoapps/courseware/tests/test_views.py | 2 +- lms/djangoapps/instructor/tests/test_api.py | 22 ++-- lms/djangoapps/instructor/tests/test_tools.py | 9 +- lms/djangoapps/mobile_api/users/tests.py | 5 - .../0024_overview_adds_has_highlights.py | 23 ++++ .../content/course_overviews/models.py | 15 ++- openedx/core/djangoapps/schedules/admin.py | 10 +- openedx/core/djangoapps/schedules/config.py | 23 +--- .../schedules/content_highlights.py | 34 +++-- .../core/djangoapps/schedules/docs/README.rst | 47 +------ .../commands/tests/test_send_course_update.py | 17 +-- openedx/core/djangoapps/schedules/models.py | 4 +- openedx/core/djangoapps/schedules/signals.py | 71 ++--------- .../djangoapps/schedules/tests/factories.py | 2 - .../tests/test_content_highlights.py | 34 ++--- .../schedules/tests/test_resolvers.py | 19 +-- .../schedules/tests/test_signals.py | 118 ++---------------- .../djangoapps/schedules/tests/test_utils.py | 11 +- .../tests/test_access.py | 12 +- .../tests/test_course_expiration.py | 22 ++-- .../api/v1/tests/test_views.py | 21 ++-- .../tests/views/test_course_home.py | 29 ++--- .../tests/views/test_course_outline.py | 47 +++---- 36 files changed, 184 insertions(+), 529 deletions(-) create mode 100644 openedx/core/djangoapps/content/course_overviews/migrations/0024_overview_adds_has_highlights.py diff --git a/cms/djangoapps/contentstore/api/tests/test_quality.py b/cms/djangoapps/contentstore/api/tests/test_quality.py index 1f5e055705..8625cbeb55 100644 --- a/cms/djangoapps/contentstore/api/tests/test_quality.py +++ b/cms/djangoapps/contentstore/api/tests/test_quality.py @@ -45,7 +45,7 @@ class CourseQualityViewTest(BaseCourseViewTest): 'number_with_highlights': 0, 'total_visible': 1, 'total_number': 1, - 'highlights_enabled': False, + 'highlights_enabled': True, 'highlights_active_for_course': False, }, 'subsections': { diff --git a/cms/djangoapps/contentstore/api/views/course_quality.py b/cms/djangoapps/contentstore/api/views/course_quality.py index 998bc828c5..b0dab122f2 100644 --- a/cms/djangoapps/contentstore/api/views/course_quality.py +++ b/cms/djangoapps/contentstore/api/views/course_quality.py @@ -9,7 +9,6 @@ from rest_framework.generics import GenericAPIView from rest_framework.response import Response from scipy import stats -from cms.djangoapps.contentstore.views.item import highlights_setting from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.lib.cache_utils import request_cached from openedx.core.lib.graph_traversals import traverse_pre_order @@ -149,7 +148,7 @@ class CourseQualityView(DeveloperErrorViewMixin, GenericAPIView): total_visible=len(visible_sections), number_with_highlights=len(sections_with_highlights), highlights_active_for_course=course.highlights_enabled_for_messaging, - highlights_enabled=highlights_setting.is_enabled(), + highlights_enabled=True, # used to be controlled by a waffle switch, now just always enabled ) def _subsections_quality(self, course, request): # lint-amnesty, pylint: disable=missing-function-docstring diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 69f61537b0..0cf5f405ba 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -1,7 +1,6 @@ """Views for items (modules).""" -import hashlib # lint-amnesty, pylint: disable=unused-import import logging from collections import OrderedDict from datetime import datetime @@ -21,7 +20,6 @@ from edx_proctoring.api import ( get_exam_configuration_dashboard_url ) from edx_proctoring.exceptions import ProctoredExamNotFoundException -from edx_toggles.toggles import LegacyWaffleSwitch from help_tokens.core import HelpUrlExpert from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryUsageLocator @@ -36,7 +34,6 @@ from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.djangoapps.xblock_config.models import CourseEditLTIFieldsEnabledFlag from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from common.djangoapps.edxmako.shortcuts import render_to_string -from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.lib.gating import api as gating_api from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside from openedx.core.djangoapps.bookmarks import api as bookmarks_api @@ -92,9 +89,6 @@ NEVER = lambda x: False ALWAYS = lambda x: True -highlights_setting = LegacyWaffleSwitch('dynamic_pacing', 'studio_course_update', __name__) - - def _filter_entrance_exam_grader(graders): """ If the entrance exams feature is enabled we need to hide away the grader from @@ -1262,8 +1256,8 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F 'highlights_enabled_for_messaging': course.highlights_enabled_for_messaging, }) xblock_info.update({ - 'highlights_enabled': highlights_setting.is_enabled(), - 'highlights_preview_only': not COURSE_UPDATE_WAFFLE_FLAG.is_enabled(course.id), + 'highlights_enabled': True, # used to be controlled by a waffle switch, now just always enabled + 'highlights_preview_only': False, # used to be controlled by a waffle flag, now just always disabled 'highlights_doc_url': HelpUrlExpert.the_one().url_for_token('content_highlights'), }) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 2124f572e3..8a53dae1f7 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -13,7 +13,6 @@ from django.test import TestCase from django.test.client import RequestFactory from django.urls import reverse from edx_proctoring.exceptions import ProctoredExamNotFoundException -from edx_toggles.toggles.testutils import override_waffle_switch from mock import Mock, PropertyMock, patch from opaque_keys import InvalidKeyError from opaque_keys.edx.asides import AsideUsageKeyV2 @@ -66,7 +65,6 @@ from ..item import ( _xblock_type_and_display_name, add_container_page_publishing_info, create_xblock_info, - highlights_setting ) @@ -2696,12 +2694,8 @@ class TestXBlockInfo(ItemTest): def test_highlights_enabled(self): self.course.highlights_enabled_for_messaging = True self.store.update_item(self.course, None) - chapter = self.store.get_item(self.chapter.location) - with override_waffle_switch(highlights_setting, active=True): - chapter_xblock_info = create_xblock_info(chapter) - course_xblock_info = create_xblock_info(self.course) - self.assertTrue(chapter_xblock_info['highlights_enabled']) - self.assertTrue(course_xblock_info['highlights_enabled_for_messaging']) + course_xblock_info = create_xblock_info(self.course) + self.assertTrue(course_xblock_info['highlights_enabled_for_messaging']) def validate_course_xblock_info(self, xblock_info, has_child_info=True, course_outline=False): """ @@ -2731,7 +2725,7 @@ class TestXBlockInfo(ItemTest): self.assertEqual(xblock_info['due'], None) self.assertEqual(xblock_info['format'], None) self.assertEqual(xblock_info['highlights'], self.chapter.highlights) - self.assertFalse(xblock_info['highlights_enabled']) + self.assertTrue(xblock_info['highlights_enabled']) # Finally, validate the entire response for consistency self.validate_xblock_info_consistency(xblock_info, has_child_info=has_child_info) diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index af26914fbf..afb5d5cfd3 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -573,8 +573,7 @@ describe('CourseOutlinePage', function() { }); describe('Content Highlights', function() { - var createCourse, createCourseWithHighlights, createCourseWithHighlightsDisabled, - clickSaveOnModal, clickCancelOnModal; + let createCourse, createCourseWithHighlights, clickSaveOnModal, clickCancelOnModal; beforeEach(function() { setSelfPaced(); @@ -592,11 +591,6 @@ describe('CourseOutlinePage', function() { createCourse({highlights: highlights}); }; - createCourseWithHighlightsDisabled = function() { - var highlightsDisabled = {highlights_enabled: false}; - createCourse(highlightsDisabled, highlightsDisabled); - }; - clickSaveOnModal = function() { $('.wrapper-modal-window .action-save').click(); }; @@ -643,11 +637,6 @@ describe('CourseOutlinePage', function() { $('button.status-highlights-enabled-value').click(); }; - it('does not display settings when disabled', function() { - createCourseWithHighlightsDisabled(); - expect(highlightsSetting()).not.toExist(); - }); - it('displays settings when enabled', function() { createCourseWithHighlights([]); expect(highlightsSetting()).toExist(); @@ -778,11 +767,6 @@ describe('CourseOutlinePage', function() { expectHighlightsToBe(updatedHighlights); }; - it('does not display link when disabled', function() { - createCourseWithHighlightsDisabled(); - expect(highlightsLink()).not.toExist(); - }); - it('displays link when no highlights exist', function() { createCourseWithHighlights([]); expectHighlightLinkNumberToBe(0); diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index e41860960e..0f83b02e59 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -212,7 +212,7 @@ if (is_proctored_exam) {

<% } %> - <% if (xblockInfo.get('highlights_enabled') && xblockInfo.isChapter()) { %> + <% if (xblockInfo.isChapter()) { %>
<% var number_of_highlights = (xblockInfo.get('highlights') || []).length; %>