Skip to content

Commit

Permalink
feat: Update command to restructure url slugs for 2U executive educat…
Browse files Browse the repository at this point in the history
…ion courses
  • Loading branch information
AliAdnanSohail committed Aug 1, 2023
1 parent 7be63b0 commit 3e29406
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 35 deletions.
2 changes: 1 addition & 1 deletion course_discovery/apps/course_metadata/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
REFRESH_COURSE_SKILLS_URL_NAME = 'refresh_course_skills'
REFRESH_PROGRAM_SKILLS_URL_NAME = 'refresh_program_skills'
COURSE_UUID_REGEX = r'[0-9a-f-]+'
SUBDIRECTORY_SLUG_FORMAT_REGEX = r'learn\/[a-zA-Z0-9-_]+\/[a-zA-Z0-9-_]+$'
SUBDIRECTORY_SLUG_FORMAT_REGEX = r'learn\/[a-zA-Z0-9-_]+\/[a-zA-Z0-9-_]+$|executive-education\/[a-zA-Z0-9-_]+$'
SLUG_FORMAT_REGEX = r'[a-zA-Z0-9-_]+$'

DEFAULT_SLUG_FORMAT_ERROR_MSG = 'Enter a valid “slug” consisting of letters, numbers, underscores or hyphens.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,23 @@
from django.core.management import BaseCommand, CommandError

from course_discovery.apps.course_metadata.emails import send_email_for_slug_updates
from course_discovery.apps.course_metadata.models import Course, MigrateCourseSlugConfiguration
from course_discovery.apps.course_metadata.models import Course, CourseType, MigrateCourseSlugConfiguration
from course_discovery.apps.course_metadata.utils import get_slug_for_course, is_valid_slug_format, is_valid_uuid

logger = logging.getLogger(__name__)


class Command(BaseCommand):
help = """
It will update course url slugs to the format 'learn/<primary_subject>/<organization_name>-course_title'
It will update course url slugs to the format 'learn/<primary_subject>/<organization_name>-<course_title>' for
open courses and 'executive-education/<organization_name>-<course_title>' for executive education courses
"""

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.slug_update_report = []
self.course_type = None
self.product_source = None

def add_arguments(self, parser):
parser.add_argument(
Expand Down Expand Up @@ -48,6 +51,19 @@ def add_arguments(self, parser):
action='store_true',
help='Use arguments from the MigrateCourseSlugConfiguration model instead of the command line.',
)
parser.add_argument(
'--course_type',
help='Course Type to update slug',
type=str,
choices=[CourseType.EXECUTIVE_EDUCATION_2U, 'open-course'],
default='open-course',
)
parser.add_argument(
'--product_source',
help='Product source slug of the courses',
type=str,
default='edx',
)

def get_args_from_database(self):
config = MigrateCourseSlugConfiguration.current()
Expand All @@ -61,13 +77,16 @@ def get_args_from_database(self):
'dry_run': config.dry_run,
'course_uuids': config.course_uuids.split() if config.course_uuids else None,
'csv_from_config': config.csv_file if bool(config.csv_file) else None,
'limit': config.count
'limit': config.count,
'course_type': config.course_type,
'product_source': config.product_source.slug,
}

def handle(self, *args, **options):
"""
It will execute the command to update slugs to the format
'learn/<primary_subject>/<organization_name>-course_title'
It will execute the command to update slugs to the sub directory format i.e
'learn/<primary_subject>/<organization_name>-<course_title>' for open courses
'executive-education/<organization_name>-<course_title>' for executive education courses
"""
args_from_database = options.get('args_from_database', None)
if args_from_database:
Expand All @@ -76,6 +95,8 @@ def handle(self, *args, **options):
course_uuids = options.get('course_uuids', None)
csv_from_config = options.get('csv_from_config', None)
limit = options.get('limit', None)
self.course_type = options.get('course_type', 'open-course')
self.product_source = options.get('product_source', 'edx')
courses = []

if csv_from_config:
Expand All @@ -85,8 +106,8 @@ def handle(self, *args, **options):
courses = self._get_courses_from_uuids(course_uuids)

if limit:
logger.info(f"Getting first {limit} open course records")
courses = Course.everything.filter(product_source__slug='edx', draft=True)[:limit]
logger.info(f"Getting first {limit} {self.course_type} records")
courses = self._get_courses()[:limit]

for course in courses:
self._update_course_slug(course, dry_run)
Expand Down Expand Up @@ -159,8 +180,13 @@ def _get_courses_from_uuids(self, course_uuids):
}
)
logger.info(error)
return self._get_courses().filter(uuid__in=valid_course_uuids)

return Course.everything.filter(product_source__slug='edx', uuid__in=valid_course_uuids, draft=True)
def _get_courses(self):
courses = Course.everything.filter(product_source__slug=self.product_source, draft=True)
if self.course_type == CourseType.EXECUTIVE_EDUCATION_2U:
return courses.filter(type__slug=CourseType.EXECUTIVE_EDUCATION_2U)
return courses

def _get_report_in_csv_format(self):
report_in_csv_format = "course_uuid,old_slug,new_slug,error\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
from django.core.management import call_command
from django.test import TestCase
from edx_toggles.toggles.testutils import override_waffle_switch
from slugify import slugify
from testfixtures import LogCapture

from course_discovery.apps.course_metadata.models import CourseType
from course_discovery.apps.course_metadata.tests.factories import (
CourseFactory, CourseUrlSlugFactory, MigrateCourseSlugConfigurationFactory, OrganizationFactory, PartnerFactory,
SourceFactory, SubjectFactory
CourseFactory, CourseTypeFactory, CourseUrlSlugFactory, MigrateCourseSlugConfigurationFactory, OrganizationFactory,
PartnerFactory, SourceFactory, SubjectFactory
)
from course_discovery.apps.course_metadata.toggles import IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED

Expand All @@ -17,10 +19,12 @@ class TestMigrateCourseSlugs(TestCase):
def setUp(self):
super().setUp()
self.slug_update_report = []
product_source = SourceFactory(slug='edx')
self.product_source = SourceFactory(slug='edx')
self.external_product_source = SourceFactory(slug='external-source')
ee_type_2u = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U)
partner = PartnerFactory()
self.course1 = CourseFactory(draft=True, product_source=product_source, partner=partner)
self.course2 = CourseFactory(draft=True, product_source=product_source, partner=partner)
self.course1 = CourseFactory(draft=True, product_source=self.product_source, partner=partner)
self.course2 = CourseFactory(draft=True, product_source=self.product_source, partner=partner)
self.organization = OrganizationFactory(name='test-organization')
self.subject = SubjectFactory(name='business')

Expand All @@ -30,7 +34,7 @@ def setUp(self):
self.course2.subjects.add(self.subject)

self.course3_draft = CourseFactory(
draft=True, product_source=product_source, partner=partner, url_slug='test_course',
draft=True, product_source=self.product_source, partner=partner, url_slug='test_course',
title='test_course'
)
course_url_slug = CourseUrlSlugFactory(
Expand All @@ -42,7 +46,7 @@ def setUp(self):

self.course3_non_draft = CourseFactory(
draft=False, draft_version=self.course3_draft, uuid=self.course3_draft.uuid,
product_source=product_source, partner=partner, title='test_course'
product_source=self.product_source, partner=partner, title='test_course'
)
self.course3_non_draft.url_slug_history.add(course_url_slug)
self.csv_header = 'course_uuids\n'
Expand All @@ -52,6 +56,10 @@ def setUp(self):
content=self.csv_file_content.encode('utf-8'),
content_type='text/csv'
)
self.exec_ed_course1 = CourseFactory(
draft=True, product_source=self.external_product_source, partner=partner, type=ee_type_2u
)
self.exec_ed_course1.authoring_organizations.add(self.organization)

def test_migrate_course_slug_success_flow(self):
with LogCapture(LOGGER_PATH) as log_capture:
Expand Down Expand Up @@ -121,7 +129,9 @@ def test_migrate_course_slug_success_flow__draft_version(self):
assert self.course3_non_draft.active_url_slug == expected_slug

def test_migrate_course_slug_success_flow__file_upload(self):
MigrateCourseSlugConfigurationFactory(enabled=True, csv_file=self.csv_file, dry_run=False)
MigrateCourseSlugConfigurationFactory(
enabled=True, csv_file=self.csv_file, dry_run=False, product_source=self.product_source
)
with LogCapture(LOGGER_PATH) as log_capture:
current_slug_course1 = self.course1.active_url_slug
current_slug_course2 = self.course2.active_url_slug
Expand Down Expand Up @@ -239,3 +249,35 @@ def test_migrate_course_slug_with_missing_subject_and_organization_errors(self):
)
)
assert self.course2.active_url_slug == f"learn/business/test-organization-{current_slug_course2}"

def test_migrate_course_slug_success_flow__executive_education(self):
"""
It will verify that command is generating and saving correct slugs for executive education courses
"""
with LogCapture(LOGGER_PATH) as log_capture:
current_slug_course1 = self.exec_ed_course1.active_url_slug

with override_waffle_switch(IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED, active=True):
call_command(
'migrate_course_slugs',
'--course_uuids', self.exec_ed_course1.uuid,
'--course_type', CourseType.EXECUTIVE_EDUCATION_2U,
'--product_source', self.external_product_source.slug
)
log_capture.check_present(
(
LOGGER_PATH,
'INFO',
f"Updating slug for course with uuid {self.exec_ed_course1.uuid} and title "
f"{self.exec_ed_course1.title}, current slug is '{current_slug_course1}'"
),

(
LOGGER_PATH,
'INFO',
f"course_uuid,old_slug,new_slug,error\n"
f"{self.exec_ed_course1.uuid},{current_slug_course1},{self.exec_ed_course1.active_url_slug},None\n"
)
)

assert self.exec_ed_course1.active_url_slug == f"executive-education/test-organization-{slugify(self.exec_ed_course1.title)}" # pylint: disable=line-too-long
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 3.2.20 on 2023-07-27 08:27

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('course_metadata', '0329_add_field_watchers'),
]

operations = [
migrations.AddField(
model_name='migratecourseslugconfiguration',
name='course_type',
field=models.CharField(choices=[('open-course', 'Open Courses'), ('executive-education-2u', '2U Executive Education Courses')], default='open-course', max_length=255),
),
migrations.AddField(
model_name='migratecourseslugconfiguration',
name='product_source',
field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='course_metadata.source'),
),
]
7 changes: 7 additions & 0 deletions course_discovery/apps/course_metadata/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4174,6 +4174,11 @@ class MigrateCourseSlugConfiguration(ConfigurationModel):
"""
Configuration to store a csv file that will be used in migrate_course_slugs and update_course_active_url_slugs.
"""
OPEN_COURSE = 'open-course'
COURSE_TYPE_CHOICES = (
(OPEN_COURSE, _('Open Courses')),
(CourseType.EXECUTIVE_EDUCATION_2U, _('2U Executive Education Courses')),
)
# Timeout set to 0 so that the model does not read from cached config in case the config entry is deleted.
cache_timeout = 0
csv_file = models.FileField(
Expand All @@ -4186,6 +4191,8 @@ class MigrateCourseSlugConfiguration(ConfigurationModel):
course_uuids = models.TextField(default=None, null=True, blank=True, verbose_name=_('Course UUIDs'))
dry_run = models.BooleanField(default=True)
count = models.IntegerField(null=True, blank=True)
course_type = models.CharField(choices=COURSE_TYPE_CHOICES, default=OPEN_COURSE, max_length=255)
product_source = models.ForeignKey(Source, models.CASCADE, null=True, blank=False)


class ProgramSubscriptionConfiguration(ConfigurationModel):
Expand Down
58 changes: 57 additions & 1 deletion course_discovery/apps/course_metadata/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.core.exceptions import ValidationError
from django.test import TestCase
from edx_toggles.toggles.testutils import override_waffle_switch
from slugify import slugify

from course_discovery.apps.api.tests.mixins import SiteMixin
from course_discovery.apps.api.v1.tests.test_views.mixins import OAuth2Mixin
Expand Down Expand Up @@ -1048,20 +1049,38 @@ def test_get_slug_for_course(self):
course = CourseFactory(title='test-title')
slug, error = utils.get_slug_for_course(course)
assert slug is None
assert error == f"Course with uuid {course.uuid} and title {course.title} does not have any authoring " \
f"organizations"

organization = OrganizationFactory(name='test-organization')
course.authoring_organizations.add(organization)
slug, error = utils.get_slug_for_course(course)
assert slug is None
assert error == f"Course with uuid {course.uuid} and title {course.title} does not have any subject"

subject = SubjectFactory(name='business')
course.subjects.add(subject)
slug, error = utils.get_slug_for_course(course)
assert error is None
assert slug == f"learn/{subject.slug}/{organization.name}-{course.active_url_slug}"

def test_get_slug_for_exec_ed_course(self):
"""
It will verify that slug are generated correctly for executive education courses
"""
ee_type_2u = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U)
course = CourseFactory(title='test-title', type=ee_type_2u)
slug, error = utils.get_slug_for_course(course)
assert slug is None
assert error == f"Course with uuid {course.uuid} and title {course.title} does not have any authoring " \
f"organizations"

organization = OrganizationFactory(name='test-organization')
course.authoring_organizations.add(organization)
slug, error = utils.get_slug_for_course(course)

assert error is None
assert slug == f"learn/{subject.slug}/{organization.name}-{course.active_url_slug}"
assert slug == f"executive-education/{organization.name}-{slugify(course.title)}"

def test_get_slug_for_course__with_no_url_slug(self):
course = CourseFactory(title='test-title')
Expand Down Expand Up @@ -1114,6 +1133,43 @@ def test_get_slug_for_course__with_existing_url_slug(self):
assert error is None
assert slug == f"learn/{subject.slug}/{organization.name}-{course3.title}-3"

def test_get_slug_for_exec_ed_course__with_existing_url_slug(self):
ee_type_2u = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U)
partner = PartnerFactory()
course1 = CourseFactory(title='test-title', type=ee_type_2u)
organization = OrganizationFactory(name='test-organization')
course1.authoring_organizations.add(organization)
course1.partner = partner
course1.save()
CourseUrlSlug.objects.filter(course=course1).delete()
slug, error = utils.get_slug_for_course(course1)
assert error is None
assert slug == f"executive-education/{organization.name}-{slugify(course1.title)}"

course1.set_active_url_slug(slug)
# duplicate a new course with same title, subject and organization
course2 = CourseFactory(title='test-title', type=ee_type_2u)
organization = OrganizationFactory(name='test-organization')
course2.authoring_organizations.add(organization)
course2.partner = partner
course2.save()
CourseUrlSlug.objects.filter(course=course2).delete()
slug, error = utils.get_slug_for_course(course2)
assert error is None
assert slug == f"executive-education/{organization.name}-{slugify(course2.title)}-2"

course2.set_active_url_slug(slug)
# duplicate a new course with same title, subject and organization
course3 = CourseFactory(title='test-title', type=ee_type_2u)
organization = OrganizationFactory(name='test-organization')
course3.authoring_organizations.add(organization)
course3.partner = partner
course3.save()
CourseUrlSlug.objects.filter(course=course3).delete()
slug, error = utils.get_slug_for_course(course3)
assert error is None
assert slug == f"executive-education/{organization.name}-{slugify(course2.title)}-3"

def test_get_existing_slug_count(self):
course1 = CourseFactory(title='test-title')
slug = 'learn/business/test-organization-test-title'
Expand Down
Loading

0 comments on commit 3e29406

Please sign in to comment.