diff --git a/cms/djangoapps/contentstore/admin.py b/cms/djangoapps/contentstore/admin.py index cd70873b4b..12f9dfcf6f 100644 --- a/cms/djangoapps/contentstore/admin.py +++ b/cms/djangoapps/contentstore/admin.py @@ -10,7 +10,7 @@ from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.utils.translation import gettext as _ from edx_django_utils.admin.mixins import ReadOnlyAdminMixin -from cms.djangoapps.contentstore.models import VideoUploadConfig +from cms.djangoapps.contentstore.models import BackfillCourseTabsConfig, VideoUploadConfig from cms.djangoapps.contentstore.outlines_regenerate import CourseOutlineRegenerate from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines @@ -78,5 +78,6 @@ class CourseOutlineRegenerateAdmin(ReadOnlyAdminMixin, admin.ModelAdmin): return super().changelist_view(request, extra_context) +admin.site.register(BackfillCourseTabsConfig, ConfigurationModelAdmin) admin.site.register(VideoUploadConfig, ConfigurationModelAdmin) admin.site.register(CourseOutlineRegenerate, CourseOutlineRegenerateAdmin) diff --git a/cms/djangoapps/contentstore/management/commands/backfill_course_tabs.py b/cms/djangoapps/contentstore/management/commands/backfill_course_tabs.py index bcbea418f2..878a8dabaa 100644 --- a/cms/djangoapps/contentstore/management/commands/backfill_course_tabs.py +++ b/cms/djangoapps/contentstore/management/commands/backfill_course_tabs.py @@ -12,11 +12,12 @@ Search for the error message to detect any issues. import logging from django.core.management.base import BaseCommand - from xmodule.tabs import CourseTabList from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore +from cms.djangoapps.contentstore.models import BackfillCourseTabsConfig + logger = logging.getLogger(__name__) @@ -37,12 +38,19 @@ class Command(BaseCommand): if there are any new default course tabs. Else, makes no updates. """ store = modulestore() - course_keys = sorted( + all_course_keys = sorted( (course.id for course in store.get_course_summaries()), key=str # Different types of CourseKeys can't be compared without this. ) - logger.info(f'{len(course_keys)} courses read from modulestore.') + config = BackfillCourseTabsConfig.current() + start = config.start_index if config.enabled and config.start_index >= 0 else 0 + end = (start + config.count) if config.enabled and config.count > 0 else len(all_course_keys) + course_keys = all_course_keys[start:end] + + logger.info(f'{len(all_course_keys)} courses read from modulestore. Processing {start} to {end}.') + + error_keys = [] for course_key in course_keys: try: course = store.get_course(course_key, depth=1) @@ -59,3 +67,10 @@ class Command(BaseCommand): except Exception as err: # pylint: disable=broad-except logger.exception(err) logger.error(f'Course {course_key} encountered an Exception while trying to update.') + error_keys.append(course_key) + + if error_keys: + msg = 'The following courses encountered errors and were not updated:\n' + for error_key in error_keys: + msg += f' - {error_key}\n' + logger.info(msg) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_backfill_course_tabs.py b/cms/djangoapps/contentstore/management/commands/tests/test_backfill_course_tabs.py index 322f8f5064..258f00a133 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_backfill_course_tabs.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_backfill_course_tabs.py @@ -3,13 +3,16 @@ Tests for `backfill_course_outlines` Studio (cms) management command. """ from unittest import mock +import ddt from django.core.management import call_command - from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from cms.djangoapps.contentstore.models import BackfillCourseTabsConfig + +@ddt.ddt class BackfillCourseTabsTest(ModuleStoreTestCase): """ Test `backfill_course_tabs` @@ -75,7 +78,7 @@ class BackfillCourseTabsTest(ModuleStoreTestCase): assert len(course.tabs) == 7 assert 'dates' in {tab.type for tab in course.tabs} assert 'progress' in {tab.type for tab in course.tabs} - mock_logger.info.assert_any_call('4 courses read from modulestore.') + mock_logger.info.assert_any_call('4 courses read from modulestore. Processing 0 to 4.') mock_logger.info.assert_any_call(f'Updating tabs for {course.id}.') mock_logger.info.assert_any_call(f'Successfully updated tabs for {course.id}.') assert mock_logger.info.call_count == 3 @@ -109,7 +112,7 @@ class BackfillCourseTabsTest(ModuleStoreTestCase): assert len(course_2.tabs) == 7 assert 'dates' in {tab.type for tab in course_1.tabs} assert 'progress' in {tab.type for tab in course_2.tabs} - mock_logger.info.assert_any_call('2 courses read from modulestore.') + mock_logger.info.assert_any_call('2 courses read from modulestore. Processing 0 to 2.') mock_logger.info.assert_any_call(f'Updating tabs for {course_1.id}.') mock_logger.info.assert_any_call(f'Successfully updated tabs for {course_1.id}.') mock_logger.info.assert_any_call(f'Updating tabs for {course_2.id}.') @@ -149,9 +152,29 @@ class BackfillCourseTabsTest(ModuleStoreTestCase): # Course wasn't updated due to the ValueError assert error_course_tabs_before == error_course_tabs_after - mock_logger.info.assert_any_call('2 courses read from modulestore.') + mock_logger.info.assert_any_call('2 courses read from modulestore. Processing 0 to 2.') mock_logger.info.assert_any_call(f'Successfully updated tabs for {updated_course.id}.') mock_logger.exception.assert_called() mock_logger.error.assert_called_once_with( f'Course {error_course.id} encountered an Exception while trying to update.' ) + + @ddt.data( + (1, 2, [False, True, True, False]), + (1, 0, [False, True, True, True]), + (-1, -1, [True, True, True, True]), + ) + @ddt.unpack + def test_arguments_batching(self, start, count, expected_tabs_modified): + courses = CourseFactory.create_batch(4) + for course in courses: + course.tabs = [tab for tab in course.tabs if tab.type in ('course_info', 'courseware')] + course = self.update_course(course, ModuleStoreEnum.UserID.test) + assert len(course.tabs) == 2 + + BackfillCourseTabsConfig.objects.create(enabled=True, start_index=start, count=count) + call_command('backfill_course_tabs') + + for i, course in enumerate(courses): + course = self.store.get_course(course.id) + assert len(course.tabs) == (7 if expected_tabs_modified[i] else 2), f'Wrong tabs for course index {i}' diff --git a/cms/djangoapps/contentstore/migrations/0007_backfillcoursetabsconfig.py b/cms/djangoapps/contentstore/migrations/0007_backfillcoursetabsconfig.py new file mode 100644 index 0000000000..2123798d83 --- /dev/null +++ b/cms/djangoapps/contentstore/migrations/0007_backfillcoursetabsconfig.py @@ -0,0 +1,31 @@ +# Generated by Django 3.2.12 on 2022-03-18 13:49 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('contentstore', '0006_courseoutlineregenerate'), + ] + + operations = [ + migrations.CreateModel( + name='BackfillCourseTabsConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('start_index', models.IntegerField(default=0, help_text='Index of first course to start backfilling (in an alphabetically sorted list of courses)')), + ('count', models.IntegerField(default=0, help_text='How many courses to backfill in this run (or zero for all courses)')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ], + options={ + 'verbose_name': 'Arguments for backfill_course_tabs', + 'verbose_name_plural': 'Arguments for backfill_course_tabs', + }, + ), + ] diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index f6817ac59f..66d884d8dd 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -4,7 +4,7 @@ Models for contentstore from config_models.models import ConfigurationModel -from django.db.models.fields import TextField +from django.db.models.fields import IntegerField, TextField class VideoUploadConfig(ConfigurationModel): @@ -22,3 +22,24 @@ class VideoUploadConfig(ConfigurationModel): def get_profile_whitelist(cls): """Get the list of profiles to include in the encoding download""" return [profile for profile in cls.current().profile_whitelist.split(",") if profile] + + +class BackfillCourseTabsConfig(ConfigurationModel): + """ + Manages configuration for a run of the backfill_course_tabs management command. + + .. no_pii: + """ + + class Meta: + verbose_name = 'Arguments for backfill_course_tabs' + verbose_name_plural = 'Arguments for backfill_course_tabs' + + start_index = IntegerField( + help_text='Index of first course to start backfilling (in an alphabetically sorted list of courses)', + default=0, + ) + count = IntegerField( + help_text='How many courses to backfill in this run (or zero for all courses)', + default=0, + )