Skip to content

Commit

Permalink
Merge pull request #34078 from openedx/rijuma/add-active-courses-opti…
Browse files Browse the repository at this point in the history
…on-to-reindex_courses

Added --active option for reindex_courses
  • Loading branch information
rijuma authored Jan 23, 2024
2 parents 672b8c5 + a6dd0fe commit e00b79c
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 8 deletions.
35 changes: 30 additions & 5 deletions cms/djangoapps/contentstore/management/commands/reindex_course.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
from textwrap import dedent
from time import time
from datetime import date

from django.core.management import BaseCommand, CommandError
from elasticsearch import exceptions
Expand Down Expand Up @@ -38,6 +39,9 @@ def add_arguments(self, parser):
parser.add_argument('--all',
action='store_true',
help='Reindex all courses')
parser.add_argument('--active',
action='store_true',
help='Reindex active courses only')
parser.add_argument('--setup',
action='store_true',
help='Reindex all courses on developers stack setup')
Expand Down Expand Up @@ -65,12 +69,17 @@ def handle(self, *args, **options):
"""
course_ids = options['course_ids']
all_option = options['all']
active_option = options['active']
setup_option = options['setup']
readable_option = options['warning']
index_all_courses_option = all_option or setup_option

if (not len(course_ids) and not index_all_courses_option) or (len(course_ids) and index_all_courses_option): # lint-amnesty, pylint: disable=len-as-condition
raise CommandError("reindex_course requires one or more <course_id>s OR the --all or --setup flags.")
if ((not course_ids and not (index_all_courses_option or active_option)) or
(course_ids and (index_all_courses_option or active_option))):
raise CommandError((
"reindex_course requires one or more <course_id>s"
" OR the --all, --active or --setup flags."
))

store = modulestore()

Expand Down Expand Up @@ -104,12 +113,28 @@ def handle(self, *args, **options):
course_keys = [course.id for course in modulestore().get_courses()]
else:
return
elif active_option:
# in case of --active, we get the list of course keys from all courses
# that are stored in the modulestore and filter out the non-active
course_keys = []

today = date.today()
all_courses = modulestore().get_courses()
for course in all_courses:
# Omitting courses without a start date as well as
# couses that already ended (end date is in the past)
if not course.start or (course.end and course.end.date() < today):
continue
course_keys.append(course.id)

logging.warning(f'Selected {len(course_keys)} active courses over a total of {len(all_courses)}.')

else:
# in case course keys are provided as arguments
course_keys = list(map(self._parse_course_key, course_ids))

total = len(course_keys)
logging.warning(f'Reindexing {total} courses')
logging.warning(f'Reindexing {total} courses...')
reindexed = 0
start = time()

Expand All @@ -120,6 +145,6 @@ def handle(self, *args, **options):
if reindexed % 10 == 0 or reindexed == total:
now = time()
t = now - start
logging.warning(f'{reindexed}/{total} reindexed in {t:.1f} seconds')
logging.warning(f'{reindexed}/{total} reindexed in {t:.1f} seconds.')
except Exception as exc: # lint-amnesty, pylint: disable=broad-except
logging.exception('Error indexing course %s due to the error: %s', course_key, exc)
logging.exception('Error indexing course %s due to the error: %s.', course_key, exc)
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory # lint-amnesty, pylint: disable=wrong-import-order
from datetime import datetime, timedelta


@ddt.ddt
Expand All @@ -26,11 +27,18 @@ def setUp(self):
org="test", library="lib2", display_name="run2", default_store=ModuleStoreEnum.Type.split
)

yesterday = datetime.min.today() - timedelta(days=1)

self.first_course = CourseFactory.create(
org="test", course="course1", display_name="run1"
org="test", course="course1", display_name="run1", start=yesterday, end=None
)

self.second_course = CourseFactory.create(
org="test", course="course2", display_name="run1"
org="test", course="course2", display_name="run1", start=yesterday, end=yesterday
)

self.third_course = CourseFactory.create(
org="test", course="course3", display_name="run1", start=None, end=None
)

REINDEX_PATH_LOCATION = (
Expand Down Expand Up @@ -103,7 +111,7 @@ def test_given_all_key_prompts_and_reindexes_all_courses(self):
call_command('reindex_course', all=True)

patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no')
expected_calls = self._build_calls(self.first_course, self.second_course)
expected_calls = self._build_calls(self.first_course, self.second_course, self.third_course)
self.assertCountEqual(patched_index.mock_calls, expected_calls)

def test_given_all_key_prompts_and_reindexes_all_courses_cancelled(self):
Expand All @@ -116,3 +124,15 @@ def test_given_all_key_prompts_and_reindexes_all_courses_cancelled(self):

patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no')
patched_index.assert_not_called()

def test_given_active_key_prompt(self):
"""
Test that reindexes all active courses when --active key is given
Active courses have a start date but no end date, or the end date is in the future.
"""
with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \
mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)):
call_command('reindex_course', active=True)

expected_calls = self._build_calls(self.first_course)
self.assertCountEqual(patched_index.mock_calls, expected_calls)

0 comments on commit e00b79c

Please sign in to comment.