diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5eca07c7..c153b3aa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,10 @@ Change Log Unreleased +[1.32.0] - 2022-12-20 +--------------------- +* Added refresh_xblock_skills command to update skills for xblocks. + [1.31.2] - 2022-12-23 --------------------- * Added ACCESS_TOKEN_EXPIRY_THRESHOLD_IN_SECONDS in EMSI client. diff --git a/requirements/base.in b/requirements/base.in index 2f243217..54161cc7 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -5,10 +5,12 @@ django-solo pytz edx-rest-api-client edx-django-utils +edx-opaque-keys celery algoliasearch django-ses beautifulsoup4 django-choices django-filter -openedx-events +# FIXME: remove pin once https://github.com/openedx/openedx-events/pull/143 is merged. +openedx-events @ git+https://github.com/open-craft/openedx-events.git@1934a839b6b04db1b02634cd74320045f2e6ed17 diff --git a/requirements/ci.txt b/requirements/ci.txt index f784732e..bca0a977 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -4,13 +4,13 @@ # # make upgrade # -certifi==2022.9.24 +certifi==2022.12.7 # via requests charset-normalizer==2.1.1 # via requests codecov==2.1.12 # via -r requirements/ci.in -coverage==6.5.0 +coverage==7.0.1 # via codecov distlib==0.3.6 # via virtualenv @@ -20,24 +20,23 @@ filelock==3.8.2 # virtualenv idna==3.4 # via requests -packaging==21.3 +packaging==22.0 # via tox -platformdirs==2.5.4 +platformdirs==2.6.0 # via virtualenv pluggy==1.0.0 # via tox py==1.11.0 # via tox -pyparsing==3.0.9 - # via packaging requests==2.28.1 # via codecov six==1.16.0 # via tox tomli==2.0.1 # via tox -tox==3.27.1 +tox==3.28.0 # via + # -c requirements/constraints.txt # -r requirements/ci.in # tox-battery tox-battery==0.6.1 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 3722f5e1..4cc3e891 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -21,3 +21,7 @@ edx-lint<=1.5.2 # latest version causing issues in discovery. algoliasearch-django<4.0.0 algoliasearch<2.0.0 + +# latest version i.e. 4.0.9 fails with dependency "hookimpl" +# https://github.com/tox-dev/tox/issues/2637 +tox<4.0.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index c4a493fc..1ae48521 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -12,7 +12,7 @@ amqp==2.6.1 # via # -r requirements/test.txt # kombu -asgiref==3.5.2 +asgiref==3.6.0 # via # -r requirements/test.txt # django @@ -20,7 +20,7 @@ astroid==2.3.3 # via # pylint # pylint-celery -attrs==22.1.0 +attrs==22.2.0 # via # -r requirements/test.txt # openedx-events @@ -31,11 +31,11 @@ billiard==3.6.4.0 # via # -r requirements/test.txt # celery -boto3==1.26.23 +boto3==1.26.37 # via # -r requirements/test.txt # django-ses -botocore==1.29.23 +botocore==1.29.37 # via # -r requirements/test.txt # boto3 @@ -48,7 +48,7 @@ celery==4.4.7 # via # -c requirements/constraints.txt # -r requirements/test.txt -certifi==2022.9.24 +certifi==2022.12.7 # via # -r requirements/ci.txt # -r requirements/test.txt @@ -79,7 +79,7 @@ code-annotations==1.3.0 # via -r requirements/test.txt codecov==2.1.12 # via -r requirements/ci.txt -coverage[toml]==6.5.0 +coverage[toml]==7.0.1 # via # -r requirements/ci.txt # -r requirements/test.txt @@ -87,7 +87,7 @@ coverage[toml]==6.5.0 # pytest-cov ddt==1.6.0 # via -r requirements/test.txt -diff-cover==7.2.0 +diff-cover==7.3.0 # via -r requirements/dev.in distlib==0.3.6 # via @@ -117,7 +117,7 @@ django-filter==22.1 # via -r requirements/test.txt django-model-utils==4.3.1 # via -r requirements/test.txt -django-ses==3.2.2 +django-ses==3.3.0 # via -r requirements/test.txt django-solo==2.0.0 # via -r requirements/test.txt @@ -143,7 +143,7 @@ edx-opaque-keys[django]==2.3.0 # openedx-events edx-rest-api-client==5.5.0 # via -r requirements/test.txt -exceptiongroup==1.0.4 +exceptiongroup==1.1.0 # via # -r requirements/test.txt # pytest @@ -199,14 +199,13 @@ mccabe==0.6.1 # via pylint mock==4.0.3 # via -r requirements/test.txt -newrelic==8.4.0 +newrelic==8.5.0 # via # -r requirements/test.txt # edx-django-utils -openedx-events @ git+https://github.com/open-craft/openedx-events.git@04b5014a6a97242cc20ac03b20a73470f56e253d +openedx-events @ git+https://github.com/open-craft/openedx-events.git@1934a839b6b04db1b02634cd74320045f2e6ed17 # via -r requirements/test.txt - # FIXME: replace with latest tag before merge. -packaging==21.3 +packaging==22.0 # via # -r requirements/ci.txt # -r requirements/pip-tools.txt @@ -226,9 +225,9 @@ pep517==0.13.0 # via # -r requirements/pip-tools.txt # build -pip-tools==6.11.0 +pip-tools==6.12.1 # via -r requirements/pip-tools.txt -platformdirs==2.5.4 +platformdirs==2.6.0 # via # -r requirements/ci.txt # virtualenv @@ -285,12 +284,6 @@ pynacl==1.5.0 # via # -r requirements/test.txt # edx-django-utils -pyparsing==3.0.9 - # via - # -r requirements/ci.txt - # -r requirements/pip-tools.txt - # -r requirements/test.txt - # packaging pytest==7.2.0 # via # -r requirements/test.txt @@ -309,7 +302,7 @@ python-slugify==7.0.0 # via # -r requirements/test.txt # code-annotations -pytz==2022.6 +pytz==2022.7 # via # -r requirements/test.txt # celery @@ -385,8 +378,9 @@ tomli==2.0.1 # pep517 # pytest # tox -tox==3.27.1 +tox==3.28.0 # via + # -c requirements/constraints.txt # -r requirements/ci.txt # tox-battery tox-battery==0.6.1 diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index 11d87600..eb36cff0 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -1,6 +1,6 @@ # -# This file is autogenerated by pip-compile with python 3.8 -# To update, run: +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: # # make upgrade # @@ -8,14 +8,12 @@ build==0.9.0 # via pip-tools click==8.1.3 # via pip-tools -packaging==21.3 +packaging==22.0 # via build pep517==0.13.0 # via build -pip-tools==6.11.0 +pip-tools==6.12.1 # via -r requirements/pip-tools.in -pyparsing==3.0.9 - # via packaging tomli==2.0.1 # via # build diff --git a/requirements/pip.txt b/requirements/pip.txt index 4fad87a5..7555f4bb 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -1,14 +1,12 @@ # -# This file is autogenerated by pip-compile with python 3.8 -# To update, run: +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: # # make upgrade # -wheel==0.38.4 - # via -r requirements/pip.in - -# The following packages are considered to be unsafe in a requirements file: pip==22.3.1 # via -r requirements/pip.in setuptools==65.6.3 # via -r requirements/pip.in +wheel==0.38.4 + # via -r requirements/pip.in diff --git a/requirements/test.txt b/requirements/test.txt index ee3ff8ca..9866dd35 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -10,9 +10,9 @@ algoliasearch==1.20.0 # -r requirements/base.in amqp==2.6.1 # via kombu -asgiref==3.5.2 +asgiref==3.6.0 # via django -attrs==22.1.0 +attrs==22.2.0 # via # openedx-events # pytest @@ -20,9 +20,9 @@ beautifulsoup4==4.11.1 # via -r requirements/base.in billiard==3.6.4.0 # via celery -boto3==1.26.23 +boto3==1.26.37 # via django-ses -botocore==1.29.23 +botocore==1.29.37 # via # boto3 # s3transfer @@ -30,7 +30,7 @@ celery==4.4.7 # via # -c requirements/constraints.txt # -r requirements/base.in -certifi==2022.9.24 +certifi==2022.12.7 # via requests cffi==1.15.1 # via pynacl @@ -42,7 +42,7 @@ click==8.1.3 # edx-django-utils code-annotations==1.3.0 # via -r requirements/test.in -coverage[toml]==6.5.0 +coverage[toml]==7.0.1 # via pytest-cov ddt==1.6.0 # via -r requirements/test.in @@ -66,7 +66,7 @@ django-filter==22.1 # via -r requirements/base.in django-model-utils==4.3.1 # via -r requirements/base.in -django-ses==3.2.2 +django-ses==3.3.0 # via -r requirements/base.in django-solo==2.0.0 # via -r requirements/base.in @@ -79,10 +79,12 @@ edx-django-utils==5.2.0 # -r requirements/base.in # edx-rest-api-client edx-opaque-keys[django]==2.3.0 - # via openedx-events + # via + # -r requirements/base.in + # openedx-events edx-rest-api-client==5.5.0 # via -r requirements/base.in -exceptiongroup==1.0.4 +exceptiongroup==1.1.0 # via pytest factory-boy==3.2.1 # via -r requirements/test.in @@ -108,12 +110,11 @@ markupsafe==2.1.1 # via jinja2 mock==4.0.3 # via -r requirements/test.in -newrelic==8.4.0 +newrelic==8.5.0 # via edx-django-utils -openedx-events @ git+https://github.com/open-craft/openedx-events.git@04b5014a6a97242cc20ac03b20a73470f56e253d +openedx-events @ git+https://github.com/open-craft/openedx-events.git@1934a839b6b04db1b02634cd74320045f2e6ed17 # via -r requirements/base.in - # FIXME: replace with latest tag before merge. -packaging==21.3 +packaging==22.0 # via pytest pbr==5.11.0 # via stevedore @@ -129,8 +130,6 @@ pymongo==3.13.0 # via edx-opaque-keys pynacl==1.5.0 # via edx-django-utils -pyparsing==3.0.9 - # via packaging pytest==7.2.0 # via # pytest-cov @@ -145,7 +144,7 @@ python-dateutil==2.8.2 # faker python-slugify==7.0.0 # via code-annotations -pytz==2022.6 +pytz==2022.7 # via # -r requirements/base.in # celery diff --git a/taxonomy/__init__.py b/taxonomy/__init__.py index 15a7b8b1..18251a60 100644 --- a/taxonomy/__init__.py +++ b/taxonomy/__init__.py @@ -15,6 +15,6 @@ # 2. MINOR version when you add functionality in a backwards compatible manner, and # 3. PATCH version when you make backwards compatible bug fixes. # More details can be found at https://semver.org/ -__version__ = '1.31.2' +__version__ = '1.32.0' default_app_config = 'taxonomy.apps.TaxonomyConfig' # pylint: disable=invalid-name diff --git a/taxonomy/exceptions.py b/taxonomy/exceptions.py index 2bebfaec..fb655f8b 100644 --- a/taxonomy/exceptions.py +++ b/taxonomy/exceptions.py @@ -22,6 +22,12 @@ class ProgramMetadataNotFoundError(Exception): """ +class XBlockMetadataNotFoundError(Exception): + """ + Exception to raise when metadata was not found for an XBlock. + """ + + class InvalidCommandOptionsError(Exception): """ Exception to raise when incorrect command options are provided. diff --git a/taxonomy/management/commands/refresh_xblock_skills.py b/taxonomy/management/commands/refresh_xblock_skills.py new file mode 100644 index 00000000..8ea73869 --- /dev/null +++ b/taxonomy/management/commands/refresh_xblock_skills.py @@ -0,0 +1,137 @@ +# -*- coding: utf-8 -*- +""" +Management command for refreshing the skills associated with xblocks. +""" + +import logging + +from django.core.management.base import BaseCommand +from django.utils.translation import gettext as _ +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey + +from taxonomy import utils +from taxonomy.choices import ProductTypes +from taxonomy.exceptions import InvalidCommandOptionsError, XBlockMetadataNotFoundError +from taxonomy.models import RefreshXBlockSkillsConfig +from taxonomy.providers.utils import get_course_metadata_provider, get_xblock_metadata_provider + +LOGGER = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Command to refresh skills associated with the XBlocks. + + Example usage: + $ ./manage.py refresh_xblock_skills --xblock 'xblock-usage-key1' --xblock 'xblock-usage-key2' --commit + $ # To refresh all xblock skills under given courses. + $ ./manage.py refresh_xblock_skills --course 'course-v1:edX+DemoX+1' --course 'course-v1:edX+DemoY+1' --commit + $ # args-from-database means command line arguments will be picked from the database. + $ ./manage.py refresh_xblock_skills --args-from-database + $ # To update all xblocks in all the courses + $ ./manage.py refresh_xblock_skills --all --commit + """ + help = 'Refreshes the skills associated with XBlocks.' + product_type = ProductTypes.XBlock + + def add_arguments(self, parser): + """ + Add arguments to the command parser. + """ + parser.add_argument( + '--course', + metavar=_('COURSE_KEY'), + action='append', + help=_('Update skills for XBlocks under given course keys. For eg. course-v1:edX+DemoX.1+2014'), + default=[], + ) + parser.add_argument( + '--xblock', + metavar=_('USAGE_KEY'), + action='append', + help=_('Update skills for given Xblock usage keys.'), + default=[], + ) + parser.add_argument( + '--args-from-database', + action='store_true', + help=_('Use arguments from the RefreshXBlockSkillsConfig model instead of the command line.'), + ) + parser.add_argument( + '--all', + action='store_true', + help=_('Create xblock skill mapping for all xblocks in all the courses.'), + ) + parser.add_argument( + '--commit', + action='store_true', + default=False, + help=_('Commits the skills to storage.') + ) + + def get_args_from_database(self): + """ + Return an options dictionary from the current RefreshXBlockSkillsConfig model. + """ + config = RefreshXBlockSkillsConfig.get_solo() + argv = config.arguments.split() + parser = self.create_parser('manage.py', 'refresh_xblock_skills') + return parser.parse_args(argv).__dict__ + + @staticmethod + def is_valid_key(key, key_cls, key_cls_str): + """ + Validates usage and course keys. + """ + try: + key_cls.from_string(key) + return True + except InvalidKeyError: + LOGGER.error('[TAXONOMY] Invalid %s: [%s]', key_cls_str, key) + return False + + def handle(self, *args, **options): + """ + Entry point for management command execution. + """ + if not (options['args_from_database'] or options['all'] or options['course'] or options['xblock']): + raise InvalidCommandOptionsError( + 'Either course, xblock, args_from_database or all argument must be provided.', + ) + + if options['args_from_database']: + options = self.get_args_from_database() + + if options['course'] and options['xblock']: + raise InvalidCommandOptionsError('Either course or xblock argument should be provided and not both.') + + LOGGER.info('[TAXONOMY] Refresh XBlock Skills. Options: [%s]', options) + + courses = [] + xblocks_from_args = [] + xblock_provider = get_xblock_metadata_provider() + if options['all']: + courses = get_course_metadata_provider().get_all_courses() + elif options['course']: + courses = [{"key": course} for course in options['course']] + elif options['xblock']: + valid_usage_keys = set(key for key in options['xblock'] if self.is_valid_key(key, UsageKey, "UsageKey")) + xblocks_from_args = xblock_provider.get_xblocks(xblock_ids=list(valid_usage_keys)) + if not xblocks_from_args: + raise XBlockMetadataNotFoundError( + 'No xblock metadata was found for following xblocks. {}'.format(options['xblock']) + ) + else: + raise InvalidCommandOptionsError('Either course, xblock or --all argument must be provided.') + + for course in courses: + course_key = course.get("key") + if self.is_valid_key(course_key, CourseKey, "CourseKey"): + xblocks = xblock_provider.get_all_xblocks_in_course(course_key) + LOGGER.info('[TAXONOMY] Refresh xblocks skills process started for course: {course_key}.') + utils.refresh_product_skills(xblocks, options['commit'], self.product_type) + + if xblocks_from_args: + LOGGER.info('[TAXONOMY] Refresh XBlock skills process started for xblocks: [%s]', options['xblock']) + utils.refresh_product_skills(xblocks_from_args, options['commit'], self.product_type) diff --git a/taxonomy/migrations/0029_xblock_refresh_argmuments_table.py b/taxonomy/migrations/0029_xblock_refresh_argmuments_table.py new file mode 100644 index 00000000..1cedff2b --- /dev/null +++ b/taxonomy/migrations/0029_xblock_refresh_argmuments_table.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.16 on 2022-12-06 04:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('taxonomy', '0028_xblock_skills'), + ] + + operations = [ + migrations.CreateModel( + name='RefreshXBlockSkillsConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('arguments', models.TextField(blank=True, default='', help_text='Useful for manually running a Jenkins job. Specify like "--course=key1 --course=key2".')), + ], + options={ + 'verbose_name': 'refresh_xblock_skills argument', + }, + ), + ] diff --git a/taxonomy/models.py b/taxonomy/models.py index 6da96a44..3f34b3d6 100644 --- a/taxonomy/models.py +++ b/taxonomy/models.py @@ -367,6 +367,40 @@ def __repr__(self): return ''.format(self.id) +class RefreshXBlockSkillsConfig(SingletonModel): + """ + Configuration for the refresh_xblock_skills management command. + + .. no_pii: + """ + + class Meta: + """ + Meta configuration for RefreshXBlockSkillsConfig model. + """ + + app_label = 'taxonomy' + verbose_name = 'refresh_xblock_skills argument' + + arguments = models.TextField( + blank=True, + help_text='Useful for manually running a Jenkins job. Specify like "--course=key1 --course=key2".', + default='', + ) + + def __str__(self): + """ + Create a human-readable string representation of the object. + """ + return ''.format(self.arguments) + + def __repr__(self): + """ + Create a unique string representation of the object. + """ + return ''.format(self.id) + + class RefreshProgramSkillsConfig(SingletonModel): """ Configuration for the refresh_program_skills management command. diff --git a/taxonomy/utils.py b/taxonomy/utils.py index 8ffefa4e..5acfac44 100644 --- a/taxonomy/utils.py +++ b/taxonomy/utils.py @@ -175,7 +175,7 @@ def process_skills_data(product, skills, should_commit_to_db, product_type, **kw except KeyError: message = f'[TAXONOMY] Missing keys in skills data for key: {product[key_or_uuid]}' LOGGER.error(message) - failures.append((product['uuid'], message)) + failures.append((product[key_or_uuid], message)) except (ValueError, TypeError): message = f'[TAXONOMY] Invalid type for `confidence` in skills for key: {product[key_or_uuid]}' LOGGER.error(message) @@ -306,7 +306,7 @@ def refresh_product_skills(products, should_commit_to_db, product_type): except TaxonomyAPIError: message = f'[TAXONOMY] API Error for key: {product[key_or_uuid]}' LOGGER.error(message) - all_failures.append((product['uuid'], message)) + all_failures.append((product[key_or_uuid], message)) continue try: diff --git a/test_utils/factories.py b/test_utils/factories.py index 24fe09a1..484df261 100644 --- a/test_utils/factories.py +++ b/test_utils/factories.py @@ -11,8 +11,8 @@ from taxonomy.models import ( CourseSkills, Job, JobPostings, JobSkills, Skill, Translation, SkillCategory, SkillSubCategory, ProgramSkill, - SkillsQuiz, RefreshCourseSkillsConfig, RefreshProgramSkillsConfig, Industry, IndustryJobSkill, - XBlockSkillData, XBlockSkills + SkillsQuiz, RefreshCourseSkillsConfig, RefreshProgramSkillsConfig, RefreshXBlockSkillsConfig, Industry, + IndustryJobSkill, XBlockSkillData, XBlockSkills ) from taxonomy.choices import UserGoal @@ -56,6 +56,24 @@ class Meta: arguments = factory.LazyAttribute(lambda x: FAKER.word()) +# pylint: disable=no-member, invalid-name +class RefreshXBlockSkillsConfigFactory(factory.django.DjangoModelFactory): + """ + Factory class for RefreshXBlockSkillsConfig model. + """ + + class Meta: + """ + Meta for ``RefreshXBlockSkillsConfig``. + """ + + model = RefreshXBlockSkillsConfig + django_get_or_create = ('id',) + + id = factory.Sequence(lambda n: n) + arguments = factory.LazyAttribute(lambda x: FAKER.word()) + + # pylint: disable=no-member, invalid-name class SkillCategoryFactory(factory.django.DjangoModelFactory): """ diff --git a/test_utils/mocks.py b/test_utils/mocks.py index 7d6d15d0..27529645 100644 --- a/test_utils/mocks.py +++ b/test_utils/mocks.py @@ -27,7 +27,7 @@ def __init__( super().__init__(*args, spec=dict, **kwargs) self.uuid = uuid if uuid is not DEFAULT else uuid4() - self.key = key if key is not DEFAULT else 'course-id/{}'.format(FAKER.slug()) + self.key = key if key is not DEFAULT else 'course-v1:{}'.format("+".join(FAKER.words(3))) self.title = title if title is not DEFAULT else 'Test Course {}'.format(FAKER.sentence()) self.short_description = short_description if short_description is not DEFAULT else FAKER.sentence(nb_words=10) self.full_description = full_description if full_description is not DEFAULT else FAKER.sentence(nb_words=50) @@ -65,7 +65,10 @@ def __init__( """ super().__init__(*args, spec=dict, **kwargs) - self.key = key if key is not DEFAULT else 'xblock-id/{}'.format(FAKER.slug()) + self.key = key if key is not DEFAULT else 'block-v1:edx+D+D+type@{}+block@{}'.format( + FAKER.word(), + FAKER.uuid4(), + ) self.content_type = content_type if content_type is not DEFAULT else 'Video' self.content = content if content is not DEFAULT else FAKER.sentence(nb_words=50) diff --git a/tests/management/test_refresh_xblock_skills.py b/tests/management/test_refresh_xblock_skills.py new file mode 100644 index 00000000..a97617ae --- /dev/null +++ b/tests/management/test_refresh_xblock_skills.py @@ -0,0 +1,398 @@ +# -*- coding: utf-8 -*- +""" +Tests for the django management command `refresh_xblock_skills`. +""" + +import logging +from uuid import uuid4 + +import mock +import responses +from pytest import mark +from testfixtures import LogCapture + +from django.core.management import call_command + +from taxonomy.exceptions import InvalidCommandOptionsError, TaxonomyAPIError, XBlockMetadataNotFoundError +from taxonomy.models import Skill, RefreshXBlockSkillsConfig, XBlockSkillData, XBlockSkills +from test_utils.mocks import MockCourse, MockXBlock, mock_as_dict +from test_utils.providers import DiscoveryCourseMetadataProvider, DiscoveryXBlockMetadataProvider +from test_utils.sample_responses.skills import MISSING_NAME_SKILLS, SKILLS_EMSI_CLIENT_RESPONSE, TYPE_ERROR_SKILLS +from test_utils.testcase import TaxonomyTestCase + + +@mark.django_db +class RefreshXBlockSkillsCommandTests(TaxonomyTestCase): + """ + Test command `refresh_xblock_skills`. + """ + command = 'refresh_xblock_skills' + + def setUp(self): + super().setUp() + self.skills_emsi_client_response = SKILLS_EMSI_CLIENT_RESPONSE + self.missing_skills = MISSING_NAME_SKILLS + self.type_error_skills = TYPE_ERROR_SKILLS + self.course_1 = mock_as_dict(MockCourse()) + self.course_2 = mock_as_dict(MockCourse()) + self.course_3 = mock_as_dict(MockCourse()) + self.xblock_1 = mock_as_dict(MockXBlock()) + self.xblock_2 = mock_as_dict(MockXBlock()) + self.xblock_3 = mock_as_dict(MockXBlock()) + self.mock_access_token() + + def assert_xblock_skill_count(self, skill_count, xblock_skill_count, xblock_skill_data_count): + """ + Asserts that the number of skills, xblock skills, and xblock skill data + objects in the database are as expected. + + Args: + skill_count (int): The expected number of skills in the database. + xblock_skill_count (int): The expected number of xblock skills in the database. + xblock_skill_data_count (int): The expected number of xblock skill data in the database. + """ + self.assertEqual(Skill.objects.count(), skill_count) + self.assertEqual(XBlockSkills.objects.count(), xblock_skill_count) + self.assertEqual(XBlockSkillData.objects.count(), xblock_skill_data_count) + + def test_missing_arguments(self): + """ + Test missing arguments. + """ + with self.assertRaisesRegex( + InvalidCommandOptionsError, + 'Either course, xblock, args_from_database or all argument must be provided.' + ): + call_command(self.command) + + def test_missing_arguments_from_database_config(self): + """ + Test missing arguments from --args-from-database. + """ + config = RefreshXBlockSkillsConfig.get_solo() + config.arguments = '' + config.save() + with self.assertRaisesRegex( + InvalidCommandOptionsError, + 'Either course, xblock or --all argument must be provided.', + ): + call_command(self.command, '--args-from-database') + + def test_course_and_xblock_argument_raise_error(self): + """ + Test that the command raises an error with both course and xblock arguments. + """ + with self.assertRaises(InvalidCommandOptionsError) as assert_context: + call_command(self.command, '--course', self.course_1.key, '--xblock', self.xblock_2.key) + self.assertEqual( + assert_context.exception.args[0], + 'Either course or xblock argument should be provided and not both.' + ) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + def test_non_existant_xblock(self, get_xblock_metadata_provider): + """ + Test that command throws XBlockMetadataNotFoundError if xblock does not + exist for a xblock key. + """ + xblock_key = str(uuid4()) + get_xblock_metadata_provider.return_value = DiscoveryXBlockMetadataProvider([]) + + with self.assertRaises(XBlockMetadataNotFoundError) as assert_context: + call_command(self.command, '--xblock', xblock_key) + self.assertEqual( + assert_context.exception.args[0], + 'No xblock metadata was found for following xblocks. {}'.format([xblock_key]) + ) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + def test_xblock_without_content(self, get_xblock_metadata_provider): + """ + Test that command work as expected if xblock content does not exist. + """ + self.xblock_1.content = '' + get_xblock_metadata_provider.return_value = DiscoveryXBlockMetadataProvider([self.xblock_1]) + + self.assert_xblock_skill_count(0, 0, 0) + + with LogCapture(level=logging.INFO) as log_capture: + call_command(self.command, '--xblock', self.xblock_1.key, '--commit') + self.assertEqual(len(log_capture.records), 3) + messages = [record.msg for record in log_capture.records] + self.assertEqual( + messages, + [ + '[TAXONOMY] Refresh XBlock Skills. Options: [%s]', + '[TAXONOMY] Refresh XBlock skills process started for xblocks: [%s]', + '[TAXONOMY] Refresh %s skills process completed. \n' + 'Failures: %s \n' + 'Total %s Updated Successfully: %s \n' + 'Total %s Skipped: %s \n' + 'Total Failures: %s \n' + ] + ) + + self.assert_xblock_skill_count(0, 0, 0) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_course_metadata_provider') + def test_missing_course_key_with_all(self, get_course_metadata_provider, get_xblock_metadata_provider): + """ + Test that command logs error and skips processing for it if course key is missing. + """ + self.course_1.key = None + get_course_metadata_provider.return_value = DiscoveryCourseMetadataProvider([self.course_1]) + get_xblock_metadata_provider.return_value = DiscoveryXBlockMetadataProvider([self.xblock_1]) + + self.assert_xblock_skill_count(0, 0, 0) + + with LogCapture(level=logging.INFO) as log_capture: + call_command(self.command, '--all', '--commit') + messages = [record.msg for record in log_capture.records] + self.assertEqual(len(log_capture.records), 2) + self.assertEqual( + messages, + [ + '[TAXONOMY] Refresh XBlock Skills. Options: [%s]', + '[TAXONOMY] Invalid %s: [%s]', + ] + ) + + self.assert_xblock_skill_count(0, 0, 0) + + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + def test_course_xblock_skills_saved(self, get_product_skills_mock, get_xblock_provider_mock): + """ + Test that the command creates a Skill and many XBlockSkillData records. + """ + get_product_skills_mock.return_value = self.skills_emsi_client_response + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_1, self.xblock_2] + ) + self.assert_xblock_skill_count(0, 0, 0) + + call_command(self.command, '--course', self.course_1.key, '--course', self.course_2.key, '--commit') + + self.assert_xblock_skill_count(4, 2, 8) + + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_3, self.xblock_1] + ) + call_command(self.command, '--course', self.course_3.key, '--course', self.course_1.key, '--commit') + + self.assert_xblock_skill_count(4, 3, 12) + + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_course_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + def test_course_xblock_skill_saved_with_all_param( + self, + get_product_skills_mock, + get_course_provider_mock, + get_xblock_provider_mock, + ): + """ + Test that the command creates a Skill and many XBlockSkillData records using --all param. + """ + get_product_skills_mock.return_value = self.skills_emsi_client_response + get_course_provider_mock.return_value = DiscoveryCourseMetadataProvider( + [self.course_1, self.course_2, self.course_3] + ) + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_3, self.xblock_1, self.xblock_2] + ) + self.assert_xblock_skill_count(0, 0, 0) + + call_command(self.command, '--all', '--commit') + + self.assert_xblock_skill_count(4, 3, 12) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + def test_xblock_skill_not_saved_upon_exception(self, + get_product_skills_mock, + get_xblock_provider_mock): + """ + Test that the command does not create any records when the API throws an exception. + """ + get_product_skills_mock.side_effect = TaxonomyAPIError() + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_1, self.xblock_2] + ) + self.assert_xblock_skill_count(0, 0, 0) + + with LogCapture(level=logging.INFO) as log_capture: + call_command(self.command, '--xblock', self.xblock_1.key, '--xblock', self.xblock_2.key, '--commit') + # Validate a descriptive and readable log message. + self.assertEqual(len(log_capture.records), 5) + messages = [record.msg for record in log_capture.records] + self.assertEqual( + messages, + [ + '[TAXONOMY] Refresh XBlock Skills. Options: [%s]', + '[TAXONOMY] Refresh XBlock skills process started for xblocks: [%s]', + '[TAXONOMY] API Error for key: {}'.format(self.xblock_1.key), + '[TAXONOMY] API Error for key: {}'.format(self.xblock_2.key), + '[TAXONOMY] Refresh %s skills process completed. \n' + 'Failures: %s \n' + 'Total %s Updated Successfully: %s \n' + 'Total %s Skipped: %s \n' + 'Total Failures: %s \n' + ] + ) + + self.assert_xblock_skill_count(0, 0, 0) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + def test_args_from_database_config(self, get_product_skills_mock, get_xblock_provider_mock): + """ + Test that the command works via args from database config. + """ + config = RefreshXBlockSkillsConfig.get_solo() + config.arguments = ' --xblock {} --xblock {} --commit '.format(self.xblock_1.key, self.xblock_2.key) + config.save() + get_product_skills_mock.return_value = self.skills_emsi_client_response + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_1, self.xblock_2], + ) + self.assert_xblock_skill_count(0, 0, 0) + + call_command(self.command, '--args-from-database') + + self.assert_xblock_skill_count(4, 2, 8) + for skill_details in self.skills_emsi_client_response['data']: + assert Skill.objects.filter( + name=skill_details['skill']['name'], + description=skill_details['skill']['description'], + type_id=skill_details['skill']['type']['id'], + type_name=skill_details['skill']['type']['name'], + info_url=skill_details['skill']['infoUrl'], + ).exists() + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + def test_xblock_skill_not_saved_for_key_error( + self, + get_product_skills_mock, + get_xblock_provider_mock + ): + """ + Test that the command does not create any records when a Skill key error occurs. + """ + get_product_skills_mock.return_value = self.missing_skills + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_1, self.xblock_2], + ) + self.assert_xblock_skill_count(0, 0, 0) + + with LogCapture(level=logging.INFO) as log_capture: + call_command(self.command, '--xblock', self.xblock_1.key, '--xblock', self.xblock_2.key, '--commit') + # Validate a descriptive and readable log message. + messages = [record.msg for record in log_capture.records] + self.assertEqual(len(log_capture.records), 7) + self.assertEqual( + messages, + [ + '[TAXONOMY] Refresh XBlock Skills. Options: [%s]', + '[TAXONOMY] Refresh XBlock skills process started for xblocks: [%s]', + f'[TAXONOMY] Missing keys in skills data for key: {self.xblock_1.key}', + '[TAXONOMY] Skills data received from EMSI. Skills: [%s]', + f'[TAXONOMY] Missing keys in skills data for key: {self.xblock_2.key}', + '[TAXONOMY] Skills data received from EMSI. Skills: [%s]', + '[TAXONOMY] Refresh %s skills process completed. \n' + 'Failures: %s \n' + 'Total %s Updated Successfully: %s \n' + 'Total %s Skipped: %s \n' + 'Total Failures: %s \n' + ] + ) + + self.assert_xblock_skill_count(0, 0, 0) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + def test_xblock_skill_not_saved_for_type_error( + self, + get_product_skills_mock, + get_xblock_provider_mock + ): + """ + Test that the command does not create any records when a record value error occurs. + """ + get_product_skills_mock.return_value = self.type_error_skills + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider( + [self.xblock_1, self.xblock_2], + ) + self.assert_xblock_skill_count(0, 0, 0) + + with LogCapture(level=logging.INFO) as log_capture: + call_command(self.command, '--xblock', self.xblock_1.key, '--xblock', self.xblock_2.key, '--commit') + # Validate a descriptive and readable log message. + self.assertEqual(len(log_capture.records), 7) + messages = [record.msg for record in log_capture.records] + self.assertEqual( + messages, + [ + '[TAXONOMY] Refresh XBlock Skills. Options: [%s]', + '[TAXONOMY] Refresh XBlock skills process started for xblocks: [%s]', + f'[TAXONOMY] Invalid type for `confidence` in skills for key: {self.xblock_1.key}', + '[TAXONOMY] Skills data received from EMSI. Skills: [%s]', + f'[TAXONOMY] Invalid type for `confidence` in skills for key: {self.xblock_2.key}', + '[TAXONOMY] Skills data received from EMSI. Skills: [%s]', + '[TAXONOMY] Refresh %s skills process completed. \n' + 'Failures: %s \n' + 'Total %s Updated Successfully: %s \n' + 'Total %s Skipped: %s \n' + 'Total Failures: %s \n' + ] + ) + + self.assert_xblock_skill_count(0, 0, 0) + + @responses.activate + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.get_xblock_metadata_provider') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.EMSISkillsApiClient.get_product_skills') + @mock.patch('taxonomy.management.commands.refresh_xblock_skills.utils.process_skills_data') + def test_xblock_skill_not_saved_for_exception( + self, + mock_process_xblock_skills_data, + get_product_skills_mock, + get_xblock_provider_mock, + + ): + """ + Test that the command does not create any records when a record value error occurs. + """ + get_product_skills_mock.return_value = self.skills_emsi_client_response + get_xblock_provider_mock.return_value = DiscoveryXBlockMetadataProvider([self.xblock_1]) + mock_process_xblock_skills_data.side_effect = Exception("UNKNOWN ERROR.") + self.assert_xblock_skill_count(0, 0, 0) + + with LogCapture(level=logging.INFO) as log_capture: + call_command(self.command, '--xblock', self.xblock_1.key, '--commit') + # Validate a descriptive and readable log message. + self.assertEqual(len(log_capture.records), 5) + messages = [record.msg for record in log_capture.records] + self.assertEqual( + messages, + ['[TAXONOMY] Refresh XBlock Skills. Options: [%s]', + '[TAXONOMY] Refresh XBlock skills process started for xblocks: [%s]', + '[TAXONOMY] Skills data received from EMSI. Skills: [%s]', + f'[TAXONOMY] Exception for key: {self.xblock_1.key} Error: UNKNOWN ERROR.', + '[TAXONOMY] Refresh %s skills process completed. \n' + 'Failures: %s \n' + 'Total %s Updated Successfully: %s \n' + 'Total %s Skipped: %s \n' + 'Total Failures: %s \n'] + ) + + self.assert_xblock_skill_count(0, 0, 0) diff --git a/tests/test_models.py b/tests/test_models.py index a7c2c114..ab669009 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -275,6 +275,24 @@ def test_string_representation(self): assert expected_repr == course_skill_config.__repr__() +@mark.django_db +class TestRefreshXBlockSkillConfig(TestCase): + """ + Tests for the ``RefreshXBlockSkillsConfig`` model. + """ + + def test_string_representation(self): + """ + Test the string representation of the RefreshXBlockSkillsConfig model. + """ + xblock_skill_config = factories.RefreshXBlockSkillsConfigFactory() + expected_str = ''.format(xblock_skill_config.arguments) + expected_repr = ''.format(xblock_skill_config.id) + + assert expected_str == xblock_skill_config.__str__() + assert expected_repr == xblock_skill_config.__repr__() + + @mark.django_db class TestJob(TestCase): """