feat: add a config model for the backfill_course_tabs command

- Adds a BackfillCourseTabsConfig model to manage the arguments
  to that command
- Adds batching arguments using that model
- Adds some extra logging for the failed courses

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)

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)

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}'

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',
+            },
+        ),
+    ]

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,
+    )