From 9e0f2b45af2d01e4f37bc23198f355425d140d66 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Thu, 21 Mar 2024 12:25:10 +0500 Subject: [PATCH] feat: skill validation configuration to disable validation for xblock skills --- .../apps/taxonomy_support/admin.py | 9 +- ...4_skillvalidationconfiguration_and_more.py | 43 +++++++ .../apps/taxonomy_support/models.py | 109 +++++++++++++++++- .../apps/taxonomy_support/providers.py | 13 +++ .../apps/taxonomy_support/tests/factories.py | 16 +++ .../taxonomy_support/tests/test_providers.py | 68 ++++++++++- 6 files changed, 254 insertions(+), 4 deletions(-) create mode 100644 course_discovery/apps/taxonomy_support/migrations/0004_skillvalidationconfiguration_and_more.py create mode 100644 course_discovery/apps/taxonomy_support/tests/factories.py diff --git a/course_discovery/apps/taxonomy_support/admin.py b/course_discovery/apps/taxonomy_support/admin.py index e15621a7708..0153dd2dcca 100644 --- a/course_discovery/apps/taxonomy_support/admin.py +++ b/course_discovery/apps/taxonomy_support/admin.py @@ -3,7 +3,9 @@ """ from django.contrib import admin -from course_discovery.apps.taxonomy_support.models import CourseRecommendation, UpdateCourseRecommendationsConfig +from course_discovery.apps.taxonomy_support.models import ( + CourseRecommendation, SkillValidationConfiguration, UpdateCourseRecommendationsConfig +) @admin.register(CourseRecommendation) @@ -20,3 +22,8 @@ class CourseRecommendationAdmin(admin.ModelAdmin): class UpdateCourseRecommendationsConfigAdmin(admin.ModelAdmin): fields = ('uuids', 'num_past_days', 'all_courses') list_display = ('id',) + + +@admin.register(SkillValidationConfiguration) +class SkillValidationConfigurationAdmin(admin.ModelAdmin): + autocomplete_fields = ['course', 'organization'] diff --git a/course_discovery/apps/taxonomy_support/migrations/0004_skillvalidationconfiguration_and_more.py b/course_discovery/apps/taxonomy_support/migrations/0004_skillvalidationconfiguration_and_more.py new file mode 100644 index 00000000000..5899b669078 --- /dev/null +++ b/course_discovery/apps/taxonomy_support/migrations/0004_skillvalidationconfiguration_and_more.py @@ -0,0 +1,43 @@ +# Generated by Django 4.2.9 on 2024-03-21 07:24 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_metadata', '0340_auto_20231211_1032'), + ('taxonomy_support', '0003_updatecourserecommendationsconfig_num_past_days'), + ] + + operations = [ + migrations.CreateModel( + name='SkillValidationConfiguration', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('course', models.ForeignKey(blank=True, help_text='The course, for which skill validation is disabled.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='course_metadata.course')), + ('organization', models.ForeignKey(blank=True, help_text='The organization, for which skill validation is disabled.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='course_metadata.organization')), + ], + options={ + 'verbose_name': 'Skill Validation Configuration', + 'verbose_name_plural': 'Skill Validation Configurations', + }, + ), + migrations.AddConstraint( + model_name='skillvalidationconfiguration', + constraint=models.CheckConstraint(check=models.Q(models.Q(('course__isnull', False), ('organization__isnull', True)), models.Q(('course__isnull', True), ('organization__isnull', False)), _connector='OR'), name='either_course_or_org', violation_error_message='Select either course or organization.'), + ), + migrations.AddConstraint( + model_name='skillvalidationconfiguration', + constraint=models.UniqueConstraint(fields=('course',), name='unique_course'), + ), + migrations.AddConstraint( + model_name='skillvalidationconfiguration', + constraint=models.UniqueConstraint(fields=('organization',), name='unique_organization'), + ), + ] diff --git a/course_discovery/apps/taxonomy_support/models.py b/course_discovery/apps/taxonomy_support/models.py index 8bec1550460..40a5adfe812 100644 --- a/course_discovery/apps/taxonomy_support/models.py +++ b/course_discovery/apps/taxonomy_support/models.py @@ -2,12 +2,19 @@ Models for taxonomy related tables, these tables are placed here as opposed to inside `taxonomy-connector` because they have direct dependency with models from course discovery. """ +import logging + from django.db import models +from django.db.models import Q from django.utils.translation import gettext_lazy as _ from model_utils.models import TimeStampedModel +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey from solo.models import SingletonModel -from course_discovery.apps.course_metadata.models import Course +from course_discovery.apps.course_metadata.models import Course, CourseRun, Organization + +LOGGER = logging.getLogger(__name__) class CourseRecommendation(TimeStampedModel): @@ -60,3 +67,103 @@ class UpdateCourseRecommendationsConfig(SingletonModel): def __str__(self): return 'Configuration for the update_course_recommendations management command' + + +class SkillValidationConfiguration(TimeStampedModel): + """ + Model to store the configuration for disabling skill validation for a course or organization. + """ + + course = models.ForeignKey( + Course, + null=True, + blank=True, + related_name='+', + on_delete=models.CASCADE, + help_text=_('The course, for which skill validation is disabled.'), + ) + organization = models.ForeignKey( + Organization, + null=True, + blank=True, + related_name='+', + on_delete=models.CASCADE, + help_text=_('The organization, for which skill validation is disabled.'), + ) + + def __str__(self): + """ + Create a human-readable string representation of the object. + """ + if self.course: + return f'Skill validation disabled for course: [{self.course.key}]' + if self.organization: + return f'Skill validation disabled for organization: [{self.organization.key}]' + + class Meta: + """ + Meta configuration for SkillValidationConfiguration model. + """ + constraints = [ + models.CheckConstraint( + check=( + Q(course__isnull=False) & + Q(organization__isnull=True) + ) | ( + Q(course__isnull=True) & + Q(organization__isnull=False) + ), + name='either_course_or_org', + violation_error_message='Select either course or organization.' + ), + models.UniqueConstraint(fields=['course'], name="unique_course"), + models.UniqueConstraint(fields=['organization'], name="unique_organization") + ] + + verbose_name = 'Skill Validation Configuration' + verbose_name_plural = 'Skill Validation Configurations' + + @staticmethod + def is_valid_course_run_key(course_run_key): + """ + Check if the given course run key is in valid format. + + Arguments: + course_run_key (str): Course run key + """ + try: + return True, CourseKey.from_string(course_run_key) + except InvalidKeyError: + LOGGER.error('[TAXONOMY_SKILL_VALIDATION_CONFIGURATION] Invalid course_run key: [%s]', course_run_key) + + return False, None + + @classmethod + def is_disabled(cls, course_run_key) -> bool: + """ + Check if skill validation is disabled for the given course run key. + + Arguments: + course_run_key (str): Course run key + + Returns: + bool: True if skill validation is disabled for the given course run key. + """ + is_valid_course_run_key, course_run_key = cls.is_valid_course_run_key(course_run_key) + if not is_valid_course_run_key: + return False + + course_run_org = course_run_key.org + if cls.objects.filter(organization__key=course_run_org).exists(): + return True + + try: + course_run = CourseRun.objects.select_related('course').get(key=course_run_key) + course_key = course_run.course.key + except CourseRun.DoesNotExist: + return False + + if cls.objects.filter(course__key=course_key).exists(): + return True + + return False diff --git a/course_discovery/apps/taxonomy_support/providers.py b/course_discovery/apps/taxonomy_support/providers.py index 136996ef78a..a9bd936edd1 100644 --- a/course_discovery/apps/taxonomy_support/providers.py +++ b/course_discovery/apps/taxonomy_support/providers.py @@ -28,6 +28,7 @@ aggregate_contentful_data, fetch_and_transform_bootcamp_contentful_data, fetch_and_transform_degree_contentful_data ) from course_discovery.apps.course_metadata.models import Course, CourseRun, Program +from course_discovery.apps.taxonomy_support.models import SkillValidationConfiguration class DiscoveryCourseMetadataProvider(CourseMetadataProvider): @@ -72,6 +73,18 @@ def get_all_courses(): # lint-amnesty, pylint: disable=arguments-differ ), } + def skill_validation_disabled(self, course_run_key) -> bool: + """ + Get whether skill validation is disabled for a course. + + Arguments: + course_run_key(str): A course run key. + + Returns: + bool: `True` if skill validation is disabled for the course run, `False` otherwise. + """ + return SkillValidationConfiguration.is_disabled(course_run_key) + class DiscoveryCourseRunMetadataProvider(CourseRunMetadataProvider): """ diff --git a/course_discovery/apps/taxonomy_support/tests/factories.py b/course_discovery/apps/taxonomy_support/tests/factories.py new file mode 100644 index 00000000000..b904c1ac398 --- /dev/null +++ b/course_discovery/apps/taxonomy_support/tests/factories.py @@ -0,0 +1,16 @@ +""" +Model factory classes for testing. +""" + +import factory + +from course_discovery.apps.course_metadata.tests.factories import CourseFactory, OrganizationFactory +from course_discovery.apps.taxonomy_support.models import SkillValidationConfiguration + + +class SkillValidationConfigurationFactory(factory.django.DjangoModelFactory): + # course = factory.SubFactory(CourseFactory) + # organization = factory.SubFactory(OrganizationFactory) + + class Meta: + model = SkillValidationConfiguration diff --git a/course_discovery/apps/taxonomy_support/tests/test_providers.py b/course_discovery/apps/taxonomy_support/tests/test_providers.py index f08fd936daf..f842114cda5 100644 --- a/course_discovery/apps/taxonomy_support/tests/test_providers.py +++ b/course_discovery/apps/taxonomy_support/tests/test_providers.py @@ -27,19 +27,25 @@ from unittest import mock from urllib.parse import urljoin +import pytest from django.conf import settings from django.test import TestCase from taxonomy.providers import CourseRunContent -from taxonomy.providers.utils import get_course_run_metadata_provider, get_xblock_metadata_provider +from taxonomy.providers.utils import ( + get_course_metadata_provider, get_course_run_metadata_provider, get_xblock_metadata_provider +) from taxonomy.validators import ( CourseMetadataProviderValidator, CourseRunMetadataProviderValidator, ProgramMetadataProviderValidator, XBlockMetadataProviderValidator ) +from taxonomy_support.tests.factories import SkillValidationConfigurationFactory from course_discovery.apps.core.tests.factories import PartnerFactory from course_discovery.apps.core.tests.mixins import LMSAPIClientMixin from course_discovery.apps.course_metadata.choices import CourseRunStatus -from course_discovery.apps.course_metadata.tests.factories import CourseFactory, CourseRunFactory, ProgramFactory +from course_discovery.apps.course_metadata.tests.factories import ( + CourseFactory, CourseRunFactory, OrganizationFactory, ProgramFactory +) class TaxonomyIntegrationTests(TestCase, LMSAPIClientMixin): @@ -138,3 +144,61 @@ def test_get_video_xblocks_content(self): provider = get_xblock_metadata_provider() xblocks = provider.get_xblocks(block_ids) assert 'Should not be included in tagging content.' not in xblocks[0].content + + +@pytest.mark.django_db +class DiscoveryCourseMetadataProviderTests(TestCase): + """ + Tests for `DiscoveryCourseMetadataProvider`. + """ + def setUp(self): + super().setUp() + + self.course_with_skill_validation_disabled = CourseFactory() + self.course_with_skill_validation_enabled = CourseFactory() + + self.courserun_with_skill_validation_disabled = CourseRunFactory( + course=self.course_with_skill_validation_disabled + ) + self.courserun_with_skill_validation_enabled = CourseRunFactory( + course=self.course_with_skill_validation_enabled + ) + + organization = OrganizationFactory(key='MAx') + self.course_with_org_skill_validation_disabled = CourseFactory(authoring_organizations=[organization]) + self.courserun_with_org_skill_validation_disabled_1 = CourseRunFactory( + key='course-v1:MAx+4084+1T2025', + authoring_organizations=[organization] + ) + self.courserun_with_org_skill_validation_disabled_2 = CourseRunFactory( + key='course-v1:MAx+3985+1T2025', + authoring_organizations=[organization] + ) + + # create a config that will disable skill validation for a course + SkillValidationConfigurationFactory( + course=self.course_with_skill_validation_disabled + ) + # create a config that will disable skill validation for an organization + SkillValidationConfigurationFactory( + organization=organization + ) + + def test_skill_validation_disabled(self): + """ + Test that `skill_validation_disabled` work as expected. + """ + assert get_course_metadata_provider().skill_validation_disabled( + self.courserun_with_skill_validation_disabled.key + ) is True + assert get_course_metadata_provider().skill_validation_disabled( + self.course_with_skill_validation_enabled.key + ) is False + + # skill validation should be disabled for all courses runs of an orgranization + assert get_course_metadata_provider().skill_validation_disabled( + self.courserun_with_org_skill_validation_disabled_1.key + ) is True + assert get_course_metadata_provider().skill_validation_disabled( + self.courserun_with_org_skill_validation_disabled_2.key + ) is True