Skip to content

Commit

Permalink
feat: skill validation exemption
Browse files Browse the repository at this point in the history
  • Loading branch information
muhammad-ammar committed Mar 27, 2024
1 parent dfba55d commit 46044fa
Show file tree
Hide file tree
Showing 12 changed files with 435 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ Change Log
Unreleased

[1.50.0] - 2024-03-27
---------------------
* feat: Skill validation can be disbaled for a course or an organization

[1.46.2] - 2024-02-14
---------------------
* feat: Optimized finalize_xblockskill_tags command for memory via chunking
Expand Down
2 changes: 1 addition & 1 deletion taxonomy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.46.2'
__version__ = '1.50.0'

default_app_config = 'taxonomy.apps.TaxonomyConfig' # pylint: disable=invalid-name
8 changes: 8 additions & 0 deletions taxonomy/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
Translation,
XBlockSkillData,
XBlockSkills,
SkillValidationConfiguration
)
from taxonomy.views import JobSkillsView

Expand Down Expand Up @@ -289,3 +290,10 @@ def job_name(self, obj):
Name of the related job.
"""
return obj.job.name


@admin.register(SkillValidationConfiguration)
class SkillValidationConfiguratonAdmin(admin.ModelAdmin):
"""
Admin view for SkillValidationConfiguration model.
"""
11 changes: 10 additions & 1 deletion taxonomy/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework import permissions
from rest_framework.filters import OrderingFilter
from rest_framework.generics import RetrieveAPIView, ListAPIView
from rest_framework.generics import ListAPIView, RetrieveAPIView
from rest_framework.mixins import ListModelMixin, RetrieveModelMixin
from rest_framework.response import Response
from rest_framework.views import APIView
Expand Down Expand Up @@ -35,6 +35,7 @@
SkillCategory,
SkillsQuiz,
SkillSubCategory,
SkillValidationConfiguration,
XBlockSkillData,
XBlockSkills,
)
Expand Down Expand Up @@ -292,6 +293,8 @@ def get(self, request, job_id):
class XBlockSkillsViewSet(TaxonomyAPIViewSetMixin, RetrieveModelMixin, ListModelMixin, GenericViewSet):
"""
ViewSet to list and retrieve all XBlockSkills in the system.
If skill validation is disabled for a course, then return an empty queryset.
"""
serializer_class = XBlocksSkillsSerializer
permission_classes = (permissions.IsAuthenticated, )
Expand All @@ -302,6 +305,12 @@ def get_queryset(self):
"""
Get all the xblocks skills with prefetch_related objects.
"""
skill_validation_disabled = SkillValidationConfiguration.is_disabled(
self.request.query_params.get('course_key')
)
if skill_validation_disabled:
return XBlockSkills.objects.none()

return XBlockSkills.objects.prefetch_related(
Prefetch(
'skills',
Expand Down
33 changes: 33 additions & 0 deletions taxonomy/migrations/0036_auto_20240325_1631.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Generated by Django 3.2.22 on 2024-03-25 16:31

from django.db import migrations, models
import django.utils.timezone
import model_utils.fields


class Migration(migrations.Migration):

dependencies = [
('taxonomy', '0035_auto_20231013_0324'),
]

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_key', models.CharField(blank=True, help_text='The course, for which skill validation is disabled.', max_length=255, null=True, unique=True)),
('organization', models.CharField(blank=True, help_text='The organization, for which skill validation is disabled.', max_length=255, null=True, unique=True)),
],
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_key__isnull', False), ('organization__isnull', True)), models.Q(('course_key__isnull', True), ('organization__isnull', False)), _connector='OR'), name='either_course_or_org'),
),
]
136 changes: 136 additions & 0 deletions taxonomy/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,24 @@
"""
from __future__ import unicode_literals

import logging
import uuid

from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from solo.models import SingletonModel

from django.core.exceptions import ValidationError
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 taxonomy.choices import UserGoal
from taxonomy.providers.utils import get_course_metadata_provider

LOGGER = logging.getLogger(__name__)


class Skill(TimeStampedModel):
Expand Down Expand Up @@ -1144,3 +1151,132 @@ class Meta:
app_label = 'taxonomy'
verbose_name = 'B2C Job Allow List entry'
verbose_name_plural = 'B2C Job Allow List entries'


class SkillValidationConfiguration(TimeStampedModel):
"""
Model to store the configuration for disabling skill validation for a course or organization.
"""

course_key = models.CharField(
max_length=255,
null=True,
blank=True,
unique=True,
help_text=_('The course, for which skill validation is disabled.'),
)
organization = models.CharField(
max_length=255,
null=True,
blank=True,
unique=True,
help_text=_('The organization, for which skill validation is disabled.'),
)

def __str__(self):
"""
Create a human-readable string representation of the object.
"""
message = ''

if self.course_key:
message = f'Skill validation disabled for course: {self.course_key}'
elif self.organization:
message = f'Skill validation disabled for organization: {self.organization}'

return message

class Meta:
"""
Meta configuration for SkillValidationConfiguration model.
"""

constraints = [
models.CheckConstraint(
check=(
Q(course_key__isnull=False) &
Q(organization__isnull=True)
) | (
Q(course_key__isnull=True) &
Q(organization__isnull=False)
),
name='either_course_or_org',
# This only work on django >= 4.1
# violation_error_message='Select either course or organization.'
),
]

verbose_name = 'Skill Validation Configuration'
verbose_name_plural = 'Skill Validation Configurations'

def clean(self):
"""Override to add custom validation for course and organization fields."""
if self.course_key:
if not get_course_metadata_provider().is_valid_course(self.course_key):
raise ValidationError({
'course_key': f'Course with key {self.course_key} does not exist.'
})

if self.organization:
if not get_course_metadata_provider().is_valid_organization(self.organization):
raise ValidationError({
'organization': f'Organization with key {self.organization} does not exist.'
})

# pylint: disable=no-member
def validate_constraints(self, exclude=None):
"""
Validate all constraints defined in Meta.constraints.
NOTE: We override this method only to return a human readable message.
We should remove this override once taxonomy-connector is updated to django 4.1
On django >= 4.1, add violation_error_message in models.CheckConstraint with an appropriate message.
"""
try:
super().validate_constraints(exclude=exclude)
except ValidationError as ex:
raise ValidationError({'__all__': 'Add either course key or organization.'}) from ex

def save(self, *args, **kwargs):
"""Override to ensure that custom validation is always called."""
self.full_clean()
return super().save(*args, **kwargs)

@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_locator = cls.is_valid_course_run_key(course_run_key)
if not is_valid_course_run_key:
return False

if cls.objects.filter(organization=course_run_locator.org).exists():
return True

course_key = get_course_metadata_provider().get_course_key(course_run_key)
if course_key and cls.objects.filter(course_key=course_key).exists():
return True

return False
36 changes: 36 additions & 0 deletions taxonomy/providers/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,39 @@ def get_all_courses(self):
4. short_description: Course's short description
5. full_description: Course's full description
"""

@abstractmethod
def get_course_key(self, course_run_key):
"""
Get the course key for the given course run key.
Arguments:
course_run_key(str): Course run key
Returns:
str: course key
"""

@abstractmethod
def is_valid_course(self, course_key):
"""
Validate the course key.
Arguments:
course_key(str): course key
Returns:
bool: True if course is valid, False otherwise
"""

@abstractmethod
def is_valid_organization(self, organization_key):
"""
Validate the organization.
Arguments:
organization(str): organization key
Returns:
bool: True if organization is valid, False otherwise
"""
37 changes: 37 additions & 0 deletions taxonomy/validators/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
All host platform must run this validator to make sure providers are working as expected.
"""
import inspect

from taxonomy.providers.utils import get_course_metadata_provider


Expand All @@ -23,6 +25,11 @@ def __init__(self, test_courses):
self.test_courses = test_courses
self.course_metadata_provider = get_course_metadata_provider()

@property
def provider_class_name(self):
"""Return the name of the provider class."""
return self.course_metadata_provider.__class__.__name__

def validate(self):
"""
Validate CourseMetadataProvider implements the interface as expected.
Expand All @@ -32,6 +39,9 @@ def validate(self):
"""
self.validate_get_courses()
self.validate_get_all_courses()
self.validate_get_course_key()
self.validate_is_valid_course()
self.validate_is_valid_organization()

def validate_get_courses(self):
"""
Expand Down Expand Up @@ -60,3 +70,30 @@ def validate_get_all_courses(self):
assert 'title' in course
assert 'short_description' in course
assert 'full_description' in course

def validate_get_course_key(self):
"""
Validate `get_course_key` attribute is a callable and has the correct signature.
"""
get_course_key = getattr(self.course_metadata_provider, 'get_course_key') # pylint: disable=literal-used-as-attribute
assert callable(get_course_key)
assert_msg = f'Invalid method signature for {self.provider_class_name}.get_course_key'
assert str(inspect.signature(get_course_key)) == '(course_run_key)', assert_msg

def validate_is_valid_course(self):
"""
Validate `is_valid_course` attribute is a callable and has the correct signature.
"""
is_valid_course = getattr(self.course_metadata_provider, 'is_valid_course') # pylint: disable=literal-used-as-attribute
assert callable(is_valid_course)
assert_msg = f'Invalid method signature for {self.provider_class_name}.is_valid_course'
assert str(inspect.signature(is_valid_course)) == '(course_key)', assert_msg

def validate_is_valid_organization(self):
"""
Validate `is_valid_organization` attribute is a callable and has the correct signature.
"""
is_valid_organization = getattr(self.course_metadata_provider, 'is_valid_organization') # pylint: disable=literal-used-as-attribute
assert callable(is_valid_organization)
assert_msg = f'Invalid method signature for {self.provider_class_name}.is_valid_organization'
assert str(inspect.signature(is_valid_organization)) == '(organization_key)', assert_msg
Loading

0 comments on commit 46044fa

Please sign in to comment.