From 78267e06f38e74887a3ff04f106f6bfafa7dc8bd Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Mon, 2 Dec 2024 20:13:35 +0000 Subject: [PATCH 01/17] feat: add Global Alumni in external course sync --- courses/sync_external_courses/emeritus_api.py | 46 +++++++++++++++---- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/courses/sync_external_courses/emeritus_api.py b/courses/sync_external_courses/emeritus_api.py index 8d0dee015..8aa5fa4e3 100644 --- a/courses/sync_external_courses/emeritus_api.py +++ b/courses/sync_external_courses/emeritus_api.py @@ -30,13 +30,10 @@ log = logging.getLogger(__name__) -class EmeritusKeyMap(Enum): +class BaseKeyMap(Enum): """ - Emeritus course sync keys. + Base class for external course sync keys with common attributes. """ - - REPORT_NAMES = ["Batch"] - PLATFORM_NAME = "Emeritus" DATE_FORMAT = "%Y-%m-%d" REQUIRED_FIELDS = [ "course_title", @@ -47,11 +44,40 @@ class EmeritusKeyMap(Enum): COURSE_PAGE_SUBHEAD = "Delivered in collaboration with Emeritus." WHO_SHOULD_ENROLL_PAGE_HEADING = "WHO SHOULD ENROLL" LEARNING_OUTCOMES_PAGE_HEADING = "WHAT YOU WILL LEARN" - LEARNING_OUTCOMES_PAGE_SUBHEAD = ( - "MIT xPRO is collaborating with online education provider Emeritus to " - "deliver this online course. By clicking LEARN MORE, you will be taken to " - "a page where you can download the brochure and apply to the program via Emeritus." - ) + + @property + def COURSE_PAGE_SUBHEAD(self): + """ + Generate the course page sub heading dynamically based on the platform name. + """ + return f"Delivered in collaboration with {self.PLATFORM_NAME}." + @property + def LEARNING_OUTCOMES_PAGE_SUBHEAD(self): + """ + Generate the learning outcome page sub heading dynamically based on the platform name. + """ + return ( + f"MIT xPRO is collaborating with online education provider {self.PLATFORM_NAME} to " + "deliver this online course. By clicking LEARN MORE, you will be taken to " + "a page where you can download the brochure and apply to the program via " + f"{self.PLATFORM_NAME}." + ) +class EmeritusKeyMap(BaseKeyMap): + """ + Emeritus course sync keys. + """ + + REPORT_NAMES = ["Batch"] + PLATFORM_NAME = "Emeritus" + + +class GlobalAlumniKeyMap(BaseKeyMap): + """ + Emeritus course sync keys. + """ + + REPORT_NAMES = ["GA - Batch"] + PLATFORM_NAME = "Global Alumni" class EmeritusJobStatus(Enum): From aeb423236518ddf0dbe65a0410e53ca50b1845f9 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Tue, 3 Dec 2024 17:07:39 +0000 Subject: [PATCH 02/17] refactor: made code generic and adjusted tests --- .env.example | 2 +- .github/workflows/ci.yml | 2 +- app.json | 4 +- .../commands/sync_external_course_runs.py | 34 +- ...tus_api.py => external_course_sync_api.py} | 361 +++++++++--------- ....py => external_course_sync_api_client.py} | 8 +- ...> external_course_sync_api_client_test.py} | 38 +- ...st.py => external_course_sync_api_test.py} | 323 ++++++++-------- courses/tasks.py | 8 +- courses/tasks_test.py | 10 +- mitxpro/settings.py | 10 +- 11 files changed, 405 insertions(+), 395 deletions(-) rename courses/sync_external_courses/{emeritus_api.py => external_course_sync_api.py} (62%) rename courses/sync_external_courses/{emeritus_api_client.py => external_course_sync_api_client.py} (88%) rename courses/sync_external_courses/{emeritus_api_client_test.py => external_course_sync_api_client_test.py} (63%) rename courses/sync_external_courses/{emeritus_api_test.py => external_course_sync_api_test.py} (68%) diff --git a/.env.example b/.env.example index 8c887b0f2..9b9e77c89 100644 --- a/.env.example +++ b/.env.example @@ -40,7 +40,7 @@ HUBSPOT_ID_PREFIX= MITOL_DIGITAL_CREDENTIALS_VERIFY_SERVICE_BASE_URL=http://host.docker.internal:5000/ MITOL_DIGITAL_CREDENTIALS_HMAC_SECRET=test-hmac-secret # pragma: allowlist secret -EMERITUS_API_KEY=fake_api_key +EXTERNAL_COURSE_SYNC_API_KEY=fake_api_key POSTHOG_PROJECT_API_KEY= POSTHOG_API_HOST=https://app.posthog.com/ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16420b71c..ea4a75a9b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,7 +117,7 @@ jobs: DATABASE_URL: postgres://postgres:postgres@localhost:5432/postgres # pragma: allowlist secret WEBPACK_DISABLE_LOADER_STATS: "True" ELASTICSEARCH_URL: localhost:9200 - EMERITUS_API_KEY: fake_emeritus_api_key # pragma: allowlist secret + EXTERNAL_COURSE_SYNC_API_KEY: fake_EXTERNAL_COURSE_SYNC_API_KEY # pragma: allowlist secret MAILGUN_KEY: fake_mailgun_key MAILGUN_SENDER_DOMAIN: other.fake.site MITOL_DIGITAL_CREDENTIALS_VERIFY_SERVICE_BASE_URL: http://localhost:5000 diff --git a/app.json b/app.json index 1e79a70ec..814ccbe09 100644 --- a/app.json +++ b/app.json @@ -234,11 +234,11 @@ "description": "Timeout (in seconds) for requests made via the edX API client", "required": false }, - "EMERITUS_API_BASE_URL": { + "EXTERNAL_COURSE_SYNC_API_BASE_URL": { "description": "Base API URL for Emeritus API", "required": false }, - "EMERITUS_API_KEY": { + "EXTERNAL_COURSE_SYNC_API_KEY": { "description": "The API Key for Emeritus API", "required": true }, diff --git a/courses/management/commands/sync_external_course_runs.py b/courses/management/commands/sync_external_course_runs.py index 9b84d09c9..4243d7da8 100644 --- a/courses/management/commands/sync_external_course_runs.py +++ b/courses/management/commands/sync_external_course_runs.py @@ -2,10 +2,13 @@ from django.core.management.base import BaseCommand -from courses.sync_external_courses.emeritus_api import ( +from courses.sync_external_courses.external_course_sync_api import ( + EMERITUS_PLATFORM_NAME, + GLOBAL_ALUMNI_PLATFORM_NAME, EmeritusKeyMap, - fetch_emeritus_courses, - update_emeritus_course_runs, + GlobalAlumniKeyMap, + fetch_external_courses, + update_external_course_runs, ) from mitxpro import settings @@ -36,18 +39,23 @@ def handle(self, *args, **options): # noqa: ARG002 return vendor_name = options["vendor_name"] - if vendor_name.lower() == EmeritusKeyMap.PLATFORM_NAME.value.lower(): - self.stdout.write(f"Starting course sync for {vendor_name}.") - emeritus_course_runs = fetch_emeritus_courses() - stats = update_emeritus_course_runs(emeritus_course_runs) - self.log_stats(stats) - self.stdout.write( - self.style.SUCCESS( - f"External course sync successful for {vendor_name}." - ) - ) + if vendor_name.lower() == EMERITUS_PLATFORM_NAME.lower(): + keymap = EmeritusKeyMap() + elif vendor_name.lower() == GLOBAL_ALUMNI_PLATFORM_NAME.lower(): + keymap = GlobalAlumniKeyMap() else: self.stdout.write(self.style.ERROR(f"Unknown vendor name {vendor_name}.")) + return + self.stdout.write(f"Starting course sync for {vendor_name}.") + emeritus_course_runs = fetch_external_courses(keymap) + stats = update_external_course_runs(emeritus_course_runs, keymap) + self.log_stats(stats) + self.stdout.write( + self.style.SUCCESS( + f"External course sync successful for {vendor_name}." + ) + ) + def log_stats(self, stats): """ diff --git a/courses/sync_external_courses/emeritus_api.py b/courses/sync_external_courses/external_course_sync_api.py similarity index 62% rename from courses/sync_external_courses/emeritus_api.py rename to courses/sync_external_courses/external_course_sync_api.py index 8aa5fa4e3..572ef3f92 100644 --- a/courses/sync_external_courses/emeritus_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -1,4 +1,4 @@ -"""API for Emeritus course sync""" +"""API for external course sync""" import json import logging @@ -23,16 +23,19 @@ ) from courses.api import generate_course_readable_id from courses.models import Course, CourseRun, CourseTopic, Platform -from courses.sync_external_courses.emeritus_api_client import EmeritusAPIClient +from courses.sync_external_courses.external_course_sync_api_client import ExternalCourseSyncAPIClient from ecommerce.models import Product, ProductVersion from mitxpro.utils import clean_url, now_in_utc, strip_datetime log = logging.getLogger(__name__) -class BaseKeyMap(Enum): +EMERITUS_PLATFORM_NAME = "Emeritus" +GLOBAL_ALUMNI_PLATFORM_NAME = "Global Alumni" + +class BaseKeyMap: """ - Base class for external course sync keys with common attributes. + Base class for course sync keys with common attributes. """ DATE_FORMAT = "%Y-%m-%d" REQUIRED_FIELDS = [ @@ -41,48 +44,46 @@ class BaseKeyMap(Enum): "course_run_code", "list_currency", ] - COURSE_PAGE_SUBHEAD = "Delivered in collaboration with Emeritus." WHO_SHOULD_ENROLL_PAGE_HEADING = "WHO SHOULD ENROLL" LEARNING_OUTCOMES_PAGE_HEADING = "WHAT YOU WILL LEARN" - + + def __init__(self, platform_name, report_names): + self.platform_name = platform_name + self.report_names = report_names + @property def COURSE_PAGE_SUBHEAD(self): - """ - Generate the course page sub heading dynamically based on the platform name. - """ - return f"Delivered in collaboration with {self.PLATFORM_NAME}." + return f"Delivered in collaboration with {self.platform_name}." + @property def LEARNING_OUTCOMES_PAGE_SUBHEAD(self): - """ - Generate the learning outcome page sub heading dynamically based on the platform name. - """ return ( - f"MIT xPRO is collaborating with online education provider {self.PLATFORM_NAME} to " + f"MIT xPRO is collaborating with online education provider {self.platform_name} to " "deliver this online course. By clicking LEARN MORE, you will be taken to " "a page where you can download the brochure and apply to the program via " - f"{self.PLATFORM_NAME}." + f"{self.platform_name}." ) + + class EmeritusKeyMap(BaseKeyMap): """ Emeritus course sync keys. """ - - REPORT_NAMES = ["Batch"] - PLATFORM_NAME = "Emeritus" + def __init__(self): + super().__init__(platform_name=EMERITUS_PLATFORM_NAME, report_names=["Batch"]) class GlobalAlumniKeyMap(BaseKeyMap): """ - Emeritus course sync keys. + Global Alumni course sync keys. """ - - REPORT_NAMES = ["GA - Batch"] - PLATFORM_NAME = "Global Alumni" + def __init__(self): + super().__init__(platform_name=GLOBAL_ALUMNI_PLATFORM_NAME, report_names=["GA - Batch"]) -class EmeritusJobStatus(Enum): +class ExternalCourseSyncAPIJobStatus(Enum): """ - Status of an Emeritus Job. + Status of an External Course API Job. """ READY = 3 @@ -90,79 +91,79 @@ class EmeritusJobStatus(Enum): CANCELLED = 5 -class EmeritusCourse: +class ExternalCourse: """ - Emeritus course object. + External course object. - Parses an Emeritus course JSON to Python object. + Parses an External course JSON to Python object. """ - def __init__(self, emeritus_course_json): - program_name = emeritus_course_json.get("program_name", None) + def __init__(self, external_course_json, keymap): + program_name = external_course_json.get("program_name", None) self.course_title = program_name.strip() if program_name else None - self.course_code = emeritus_course_json.get("course_code") + self.course_code = external_course_json.get("course_code") - # Emeritus course code format is `MO-`, where course tag can contain `.`, + # External course code format is `MO-`, where course tag can contain `.`, # we will replace `.` with `_` to follow the internal readable id format. self.course_readable_id = generate_course_readable_id( self.course_code.split("-")[1].replace(".", "_") ) - self.course_run_code = emeritus_course_json.get("course_run_code") - self.course_run_tag = generate_emeritus_course_run_tag(self.course_run_code) + self.course_run_code = external_course_json.get("course_run_code") + self.course_run_tag = generate_external_course_run_tag(self.course_run_code) self.price = ( - float(emeritus_course_json.get("list_price")) - if emeritus_course_json.get("list_price") + float(external_course_json.get("list_price")) + if external_course_json.get("list_price") else None ) - self.list_currency = emeritus_course_json.get("list_currency") + self.list_currency = external_course_json.get("list_currency") self.start_date = strip_datetime( - emeritus_course_json.get("start_date"), EmeritusKeyMap.DATE_FORMAT.value + external_course_json.get("start_date"), keymap.DATE_FORMAT ) end_datetime = strip_datetime( - emeritus_course_json.get("end_date"), EmeritusKeyMap.DATE_FORMAT.value + external_course_json.get("end_date"), keymap.DATE_FORMAT ) self.end_date = ( end_datetime.replace(hour=23, minute=59) if end_datetime else None ) - # Emeritus does not allow enrollments after start date. + # External Courses does not allow enrollments after start date. # We set the course run enrollment_end to the start date to # hide the course run from the course details page. self.enrollment_end = self.start_date self.marketing_url = clean_url( - emeritus_course_json.get("landing_page_url"), remove_query_params=True + external_course_json.get("landing_page_url"), remove_query_params=True ) - total_weeks = int(emeritus_course_json.get("total_weeks")) + total_weeks = int(external_course_json.get("total_weeks")) self.duration = f"{total_weeks} Weeks" if total_weeks != 0 else "" - # Description can be null in Emeritus API data, we cannot store `None` as description is Non-Nullable + # Description can be null in External Course API data, we cannot store `None` as description is Non-Nullable self.description = ( - emeritus_course_json.get("description") - if emeritus_course_json.get("description") + external_course_json.get("description") + if external_course_json.get("description") else "" ) - self.format = emeritus_course_json.get("format") - self.category = emeritus_course_json.get("Category", None) - self.image_name = emeritus_course_json.get("image_name", None) - self.CEUs = str(emeritus_course_json.get("ceu") or "") + self.format = external_course_json.get("format") + self.category = external_course_json.get("Category", None) + self.image_name = external_course_json.get("image_name", None) + self.CEUs = str(external_course_json.get("ceu") or "") self.learning_outcomes_list = ( - parse_emeritus_data_str(emeritus_course_json.get("learning_outcomes")) - if emeritus_course_json.get("learning_outcomes") + parse_external_course_data_str(external_course_json.get("learning_outcomes")) + if external_course_json.get("learning_outcomes") else [] ) self.who_should_enroll_list = ( - parse_emeritus_data_str(emeritus_course_json.get("program_for")) - if emeritus_course_json.get("program_for") + parse_external_course_data_str(external_course_json.get("program_for")) + if external_course_json.get("program_for") else [] ) - def validate_required_fields(self): + def validate_required_fields(self, keymap): """ Validates the course data. """ - for field in EmeritusKeyMap.REQUIRED_FIELDS.value: + for field in keymap.REQUIRED_FIELDS: if not getattr(self, field, None): log.info(f"Missing required field {field}") # noqa: G004 return False @@ -186,28 +187,28 @@ def validate_end_date(self): return self.end_date and now_in_utc() < self.end_date -def fetch_emeritus_courses(): +def fetch_external_courses(keymap): """ - Fetches Emeritus courses data. + Fetches external courses data. Makes a request to get the list of available queries and then queries the required reports. """ end_date = now_in_utc() start_date = end_date - timedelta(days=1) - emeritus_api_client = EmeritusAPIClient() - queries = emeritus_api_client.get_queries_list() + external_course_sync_api_client = ExternalCourseSyncAPIClient() + queries = external_course_sync_api_client.get_queries_list() for query in queries: # noqa: RET503 # Check if query is in list of desired reports - if query["name"] not in EmeritusKeyMap.REPORT_NAMES.value: + if query["name"] not in keymap.report_names: log.info( "Report: {} not specified for extract...skipping".format(query["name"]) # noqa: G001 ) continue log.info("Requesting data for {}...".format(query["name"])) # noqa: G001 - query_response = emeritus_api_client.get_query_response( + query_response = external_course_sync_api_client.get_query_response( query["id"], start_date, end_date ) if "job" in query_response: @@ -219,17 +220,17 @@ def fetch_emeritus_courses(): f"Job id: {job_id} found... waiting for completion..." # noqa: G004 ) while True: - job_status = emeritus_api_client.get_job_status(job_id) - if job_status["job"]["status"] == EmeritusJobStatus.READY.value: + job_status = external_course_sync_api_client.get_job_status(job_id) + if job_status["job"]["status"] == ExternalCourseSyncAPIJobStatus.READY.value: # If true, the query_result is ready to be collected. log.info("Job complete... requesting results...") - query_response = emeritus_api_client.get_query_result( + query_response = external_course_sync_api_client.get_query_result( job_status["job"]["query_result_id"] ) break elif job_status["job"]["status"] in [ - EmeritusJobStatus.FAILED.value, - EmeritusJobStatus.CANCELLED.value, + ExternalCourseSyncAPIJobStatus.FAILED.value, + ExternalCourseSyncAPIJobStatus.CANCELLED.value, ]: log.error("Job failed!") break @@ -245,20 +246,20 @@ def fetch_emeritus_courses(): log.error("Something unexpected happened!") -def update_emeritus_course_runs(emeritus_courses): # noqa: C901, PLR0915 +def update_external_course_runs(external_courses, keymap): # noqa: C901, PLR0915 """ Updates or creates the required course data i.e. Course, CourseRun, ExternalCoursePage, CourseTopic, WhoShouldEnrollPage, and LearningOutcomesPage Args: - emeritus_courses(list[dict]): A list of Emeritus Courses as a dict. + external_courses(list[dict]): A list of External Courses as a dict. Returns: dict: Stats of all the objects created/updated. """ platform, _ = Platform.objects.get_or_create( - name__iexact=EmeritusKeyMap.PLATFORM_NAME.value, - defaults={"name": EmeritusKeyMap.PLATFORM_NAME.value}, + name__iexact=keymap.platform_name, + defaults={"name": keymap.platform_name}, ) course_index_page = Page.objects.get(id=CourseIndexPage.objects.first().id).specific stats = { @@ -277,83 +278,83 @@ def update_emeritus_course_runs(emeritus_courses): # noqa: C901, PLR0915 "certificates_updated": set(), } - for emeritus_course_json in emeritus_courses: - emeritus_course = EmeritusCourse(emeritus_course_json) + for external_course_json in external_courses: + external_course = ExternalCourse(external_course_json, keymap) log.info( "Creating or updating course metadata for title: {}, course_code: {}, course_run_code: {}".format( # noqa: G001, UP032 - emeritus_course.course_title, - emeritus_course.course_code, - emeritus_course.course_run_code, + external_course.course_title, + external_course.course_code, + external_course.course_run_code, ) ) if ( - not emeritus_course.validate_required_fields() - or not emeritus_course.validate_list_currency() + not external_course.validate_required_fields(keymap) + or not external_course.validate_list_currency() ): log.info( - f"Skipping due to bad data... Course data: {json.dumps(emeritus_course_json)}" # noqa: G004 + f"Skipping due to bad data... Course data: {json.dumps(external_course_json)}" # noqa: G004 ) - stats["course_runs_skipped"].add(emeritus_course.course_run_code) + stats["course_runs_skipped"].add(external_course.course_run_code) continue - if not emeritus_course.validate_end_date(): + if not external_course.validate_end_date(): log.info( - f"Course run is expired, Skipping... Course data: {json.dumps(emeritus_course_json)}" # noqa: G004 + f"Course run is expired, Skipping... Course data: {json.dumps(external_course_json)}" # noqa: G004 ) - stats["course_runs_expired"].add(emeritus_course.course_run_code) + stats["course_runs_expired"].add(external_course.course_run_code) continue with transaction.atomic(): course, course_created = Course.objects.get_or_create( - external_course_id=emeritus_course.course_code, + external_course_id=external_course.course_code, platform=platform, is_external=True, defaults={ - "title": emeritus_course.course_title, - "readable_id": emeritus_course.course_readable_id, + "title": external_course.course_title, + "readable_id": external_course.course_readable_id, # All new courses are live by default, we will change the status manually "live": True, }, ) if course_created: - stats["courses_created"].add(emeritus_course.course_code) + stats["courses_created"].add(external_course.course_code) log.info( - f"Created course, title: {emeritus_course.course_title}, readable_id: {emeritus_course.course_readable_id}" # noqa: G004 + f"Created course, title: {external_course.course_title}, readable_id: {external_course.course_readable_id}" # noqa: G004 ) else: - stats["existing_courses"].add(emeritus_course.course_code) + stats["existing_courses"].add(external_course.course_code) log.info( - f"Course already exists, title: {emeritus_course.course_title}, readable_id: {emeritus_course.course_readable_id}" # noqa: G004 + f"Course already exists, title: {external_course.course_title}, readable_id: {external_course.course_readable_id}" # noqa: G004 ) log.info( - f"Creating or Updating course run, title: {emeritus_course.course_title}, course_run_code: {emeritus_course.course_run_code}" # noqa: G004 + f"Creating or Updating course run, title: {external_course.course_title}, course_run_code: {external_course.course_run_code}" # noqa: G004 ) course_run, course_run_created, course_run_updated = ( - create_or_update_emeritus_course_run(course, emeritus_course) + create_or_update_external_course_run(course, external_course) ) if course_run_created: stats["course_runs_created"].add(course_run.external_course_run_id) log.info( - f"Created Course Run, title: {emeritus_course.course_title}, external_course_run_id: {course_run.external_course_run_id}" # noqa: G004 + f"Created Course Run, title: {external_course.course_title}, external_course_run_id: {course_run.external_course_run_id}" # noqa: G004 ) elif course_run_updated: stats["course_runs_updated"].add(course_run.external_course_run_id) log.info( - f"Updated Course Run, title: {emeritus_course.course_title}, external_course_run_id: {course_run.external_course_run_id}" # noqa: G004 + f"Updated Course Run, title: {external_course.course_title}, external_course_run_id: {course_run.external_course_run_id}" # noqa: G004 ) log.info( - f"Creating or Updating Product and Product Version, course run courseware_id: {course_run.external_course_run_id}, Price: {emeritus_course.price}" # noqa: G004 + f"Creating or Updating Product and Product Version, course run courseware_id: {course_run.external_course_run_id}, Price: {external_course.price}" # noqa: G004 ) - if emeritus_course.price: + if external_course.price: product_created, product_version_created = ( create_or_update_product_and_product_version( - emeritus_course, course_run + external_course, course_run ) ) if product_created: @@ -367,69 +368,69 @@ def update_emeritus_course_runs(emeritus_courses): # noqa: C901, PLR0915 course_run.external_course_run_id ) log.info( - f"Created Product Version for course run: {course_run.courseware_id}, Price: {emeritus_course.price}" # noqa: G004 + f"Created Product Version for course run: {course_run.courseware_id}, Price: {external_course.price}" # noqa: G004 ) else: log.info( - f"Price is Null for course run code: {emeritus_course.course_run_code}" # noqa: G004 + f"Price is Null for course run code: {external_course.course_run_code}" # noqa: G004 ) - stats["course_runs_without_prices"].add(emeritus_course.course_run_code) + stats["course_runs_without_prices"].add(external_course.course_run_code) log.info( - f"Creating or Updating course page, title: {emeritus_course.course_title}, course_code: {emeritus_course.course_run_code}" # noqa: G004 + f"Creating or Updating course page, title: {external_course.course_title}, course_code: {external_course.course_run_code}" # noqa: G004 ) course_page, course_page_created, course_page_updated = ( - create_or_update_emeritus_course_page( - course_index_page, course, emeritus_course + create_or_update_external_course_page( + course_index_page, course, external_course, keymap ) ) if course_page_created: - stats["course_pages_created"].add(emeritus_course.course_code) + stats["course_pages_created"].add(external_course.course_code) log.info( - f"Created external course page for course title: {emeritus_course.course_title}" # noqa: G004 + f"Created external course page for course title: {external_course.course_title}" # noqa: G004 ) elif course_page_updated: - stats["course_pages_updated"].add(emeritus_course.course_code) + stats["course_pages_updated"].add(external_course.course_code) log.info( - f"Updated external course page for course title: {emeritus_course.course_title}" # noqa: G004 + f"Updated external course page for course title: {external_course.course_title}" # noqa: G004 ) - if emeritus_course.category: + if external_course.category: topic = CourseTopic.objects.filter( - name__iexact=emeritus_course.category + name__iexact=external_course.category ).first() if topic: course_page.topics.add(topic) course_page.save() log.info( - f"Added topic {topic.name} for {emeritus_course.course_title}" # noqa: G004 + f"Added topic {topic.name} for {external_course.course_title}" # noqa: G004 ) outcomes_page = course_page.get_child_page_of_type_including_draft( LearningOutcomesPage ) - if not outcomes_page and emeritus_course.learning_outcomes_list: + if not outcomes_page and external_course.learning_outcomes_list: create_learning_outcomes_page( - course_page, emeritus_course.learning_outcomes_list + course_page, external_course.learning_outcomes_list, keymap ) log.info("Created LearningOutcomesPage.") who_should_enroll_page = course_page.get_child_page_of_type_including_draft( WhoShouldEnrollPage ) - if not who_should_enroll_page and emeritus_course.who_should_enroll_list: + if not who_should_enroll_page and external_course.who_should_enroll_list: create_who_should_enroll_in_page( - course_page, emeritus_course.who_should_enroll_list + course_page, external_course.who_should_enroll_list, keymap ) log.info("Created WhoShouldEnrollPage.") - if emeritus_course.CEUs: + if external_course.CEUs: log.info( - f"Creating or Updating Certificate Page for title: {emeritus_course.course_title}, course_code: {course.readable_id}, CEUs: {emeritus_course.CEUs}" # noqa: G004 + f"Creating or Updating Certificate Page for title: {external_course.course_title}, course_code: {course.readable_id}, CEUs: {external_course.CEUs}" # noqa: G004 ) _, is_certificatepage_created, is_certificatepage_updated = ( - create_or_update_certificate_page(course_page, emeritus_course) + create_or_update_certificate_page(course_page, external_course) ) if is_certificatepage_created: @@ -450,43 +451,43 @@ def update_emeritus_course_runs(emeritus_courses): # noqa: C901, PLR0915 return stats -def create_or_update_product_and_product_version(emeritus_course, course_run): +def create_or_update_product_and_product_version(external_course, course_run): """ Creates or Updates Product and Product Version for the course run. Args: - emeritus_course(EmeritusCourse): EmeritusCourse object + external_course(ExternalCourse): ExternalCourse object course_run(CourseRun): CourseRun object Returns: tuple: (product is created, product version is created) """ current_price = course_run.current_price - if not current_price or current_price != emeritus_course.price: + if not current_price or current_price != external_course.price: product, product_created = Product.objects.get_or_create( content_type=ContentType.objects.get_for_model(CourseRun), object_id=course_run.id, ) ProductVersion.objects.create( product=product, - price=emeritus_course.price, + price=external_course.price, description=course_run.courseware_id, ) return product_created, True return False, False -def generate_emeritus_course_run_tag(course_run_code): +def generate_external_course_run_tag(course_run_code): """ - Returns the course run tag generated using the Emeritus Course run code. + Returns the course run tag generated using the External Course run code. - Emeritus course run codes follow a pattern `MO--`. This method returns the run tag. + External course run codes follow a pattern `MO--`. This method returns the run tag. Args: - course_run_code(str): Emeritus course code + course_run_code(str): External course code Returns: - str: Course tag generated from the Emeritus Course Code + str: Course tag generated from the External Course Code """ run_tag = re.search(r"[0-9]{2}-[0-9]{2}#[0-9]+$", course_run_code).group(0) return run_tag.replace("#", "-") @@ -506,14 +507,14 @@ def generate_external_course_run_courseware_id(course_run_tag, course_readable_i return f"{course_readable_id}+{course_run_tag}" -def create_or_update_emeritus_course_page(course_index_page, course, emeritus_course): +def create_or_update_external_course_page(course_index_page, course, external_course, keymap): """ - Creates or updates external course page for Emeritus course. + Creates or updates external course page for External course. Args: course_index_page(CourseIndexPage): A course index page object. course(Course): A course object. - emeritus_course(EmeritusCourse): A EmeritusCourse object. + external_course(ExternalCourse): A ExternalCourse object. Returns: tuple(ExternalCoursePage, is_created, is_updated): ExternalCoursePage object, is_created, is_updated @@ -523,15 +524,15 @@ def create_or_update_emeritus_course_page(course_index_page, course, emeritus_co ) image = None - if emeritus_course.image_name: + if external_course.image_name: image = ( - Image.objects.filter(title=emeritus_course.image_name) + Image.objects.filter(title=external_course.image_name) .order_by("-created_at") .first() ) if not image: - image_title = Path(emeritus_course.image_name).stem + image_title = Path(external_course.image_name).stem image = ( Image.objects.filter(title=image_title).order_by("-created_at").first() ) @@ -540,12 +541,12 @@ def create_or_update_emeritus_course_page(course_index_page, course, emeritus_co if not course_page: course_page = ExternalCoursePage( course=course, - title=emeritus_course.course_title, - external_marketing_url=emeritus_course.marketing_url, - subhead=EmeritusKeyMap.COURSE_PAGE_SUBHEAD.value, - duration=emeritus_course.duration, - format=emeritus_course.format, - description=emeritus_course.description, + title=external_course.course_title, + external_marketing_url=external_course.marketing_url, + subhead=keymap.COURSE_PAGE_SUBHEAD, + duration=external_course.duration, + format=external_course.format, + description=external_course.description, background_image=image, thumbnail_image=image, ) @@ -556,16 +557,16 @@ def create_or_update_emeritus_course_page(course_index_page, course, emeritus_co latest_revision = course_page.get_latest_revision_as_object() # Only update course page fields with API if they are empty in the latest revision. - if not latest_revision.external_marketing_url and emeritus_course.marketing_url: - latest_revision.external_marketing_url = emeritus_course.marketing_url + if not latest_revision.external_marketing_url and external_course.marketing_url: + latest_revision.external_marketing_url = external_course.marketing_url is_updated = True - if not latest_revision.duration and emeritus_course.duration: - latest_revision.duration = emeritus_course.duration + if not latest_revision.duration and external_course.duration: + latest_revision.duration = external_course.duration is_updated = True - if not latest_revision.description and emeritus_course.description: - latest_revision.description = emeritus_course.description + if not latest_revision.description and external_course.description: + latest_revision.description = external_course.description is_updated = True if not latest_revision.background_image and image: @@ -582,73 +583,73 @@ def create_or_update_emeritus_course_page(course_index_page, course, emeritus_co return course_page, is_created, is_updated -def create_or_update_emeritus_course_run(course, emeritus_course): +def create_or_update_external_course_run(course, external_course): """ - Creates or updates the external emeritus course run. + Creates or updates the external course run. Args: course (courses.Course): Course object - emeritus_course (EmeritusCourse): EmeritusCourse object + external_course (ExternalCourse): ExternalCourse object Returns: tuple(CourseRun, is_created, is_updated): A tuple containing course run, is course run created, is course run updated """ course_run_courseware_id = generate_external_course_run_courseware_id( - emeritus_course.course_run_tag, course.readable_id + external_course.course_run_tag, course.readable_id ) course_run = ( CourseRun.objects.select_for_update() - .filter(external_course_run_id=emeritus_course.course_run_code, course=course) + .filter(external_course_run_id=external_course.course_run_code, course=course) .first() ) is_created = is_updated = False if not course_run: course_run = CourseRun.objects.create( - external_course_run_id=emeritus_course.course_run_code, + external_course_run_id=external_course.course_run_code, course=course, - title=emeritus_course.course_title, + title=external_course.course_title, courseware_id=course_run_courseware_id, - run_tag=emeritus_course.course_run_tag, - start_date=emeritus_course.start_date, - end_date=emeritus_course.end_date, - enrollment_end=emeritus_course.enrollment_end, + run_tag=external_course.course_run_tag, + start_date=external_course.start_date, + end_date=external_course.end_date, + enrollment_end=external_course.enrollment_end, live=True, ) is_created = True elif ( - (not course_run.start_date and emeritus_course.start_date) + (not course_run.start_date and external_course.start_date) or ( course_run.start_date - and emeritus_course.start_date - and course_run.start_date.date() != emeritus_course.start_date.date() + and external_course.start_date + and course_run.start_date.date() != external_course.start_date.date() ) - or (not course_run.end_date and emeritus_course.end_date) + or (not course_run.end_date and external_course.end_date) or ( course_run.end_date - and emeritus_course.end_date - and course_run.end_date.date() != emeritus_course.end_date.date() + and external_course.end_date + and course_run.end_date.date() != external_course.end_date.date() ) - or (not course_run.enrollment_end and emeritus_course.enrollment_end) + or (not course_run.enrollment_end and external_course.enrollment_end) or ( course_run.enrollment_end - and emeritus_course.enrollment_end + and external_course.enrollment_end and course_run.enrollment_end.date() - != emeritus_course.enrollment_end.date() + != external_course.enrollment_end.date() ) ): - course_run.start_date = emeritus_course.start_date - course_run.end_date = emeritus_course.end_date - course_run.enrollment_end = emeritus_course.enrollment_end + course_run.start_date = external_course.start_date + course_run.end_date = external_course.end_date + course_run.enrollment_end = external_course.enrollment_end course_run.save() is_updated = True return course_run, is_created, is_updated -def create_who_should_enroll_in_page(course_page, who_should_enroll_list): +def create_who_should_enroll_in_page(course_page, who_should_enroll_list, keymap): """ - Creates `WhoShouldEnrollPage` for Emeritus course. + Creates `WhoShouldEnrollPage` for external course. Args: course_page(ExternalCoursePage): ExternalCoursePage object. @@ -662,16 +663,16 @@ def create_who_should_enroll_in_page(course_page, who_should_enroll_list): ) who_should_enroll_page = WhoShouldEnrollPage( - heading=EmeritusKeyMap.WHO_SHOULD_ENROLL_PAGE_HEADING.value, + heading=keymap.WHO_SHOULD_ENROLL_PAGE_HEADING, content=content, ) course_page.add_child(instance=who_should_enroll_page) who_should_enroll_page.save() -def create_learning_outcomes_page(course_page, outcomes_list): +def create_learning_outcomes_page(course_page, outcomes_list, keymap): """ - Creates `LearningOutcomesPage` for Emeritus course. + Creates `LearningOutcomesPage` for external course. Args: course_page(ExternalCoursePage): ExternalCoursePage object. @@ -682,21 +683,21 @@ def create_learning_outcomes_page(course_page, outcomes_list): ) learning_outcome_page = LearningOutcomesPage( - heading=EmeritusKeyMap.LEARNING_OUTCOMES_PAGE_HEADING.value, - sub_heading=EmeritusKeyMap.LEARNING_OUTCOMES_PAGE_SUBHEAD.value, + heading=keymap.LEARNING_OUTCOMES_PAGE_HEADING, + sub_heading=keymap.LEARNING_OUTCOMES_PAGE_SUBHEAD, outcome_items=outcome_items, ) course_page.add_child(instance=learning_outcome_page) learning_outcome_page.save() -def create_or_update_certificate_page(course_page, emeritus_course): +def create_or_update_certificate_page(course_page, external_course): """ Creates or Updates certificate page for a course page. Args: course_page(ExternalCoursePage): ExternalCoursePage object - emeritus_course(EmeritusCourse): EmeritusCourse object + external_course(ExternalCourse): ExternalCourse object Returns: tuple: (CertificatePage, Is Page Created, Is Page Updated) @@ -708,8 +709,8 @@ def create_or_update_certificate_page(course_page, emeritus_course): if not certificate_page: certificate_page = CertificatePage( - product_name=f"Certificate for {emeritus_course.course_title}", - CEUs=emeritus_course.CEUs, + product_name=f"Certificate for {external_course.course_title}", + CEUs=external_course.CEUs, live=False, ) course_page.add_child(instance=certificate_page) @@ -718,8 +719,8 @@ def create_or_update_certificate_page(course_page, emeritus_course): else: latest_revision = certificate_page.get_latest_revision_as_object() - if latest_revision.CEUs != emeritus_course.CEUs: - latest_revision.CEUs = emeritus_course.CEUs + if latest_revision.CEUs != external_course.CEUs: + latest_revision.CEUs = external_course.CEUs is_updated = True if is_updated: @@ -728,9 +729,9 @@ def create_or_update_certificate_page(course_page, emeritus_course): return certificate_page, is_created, is_updated -def parse_emeritus_data_str(items_str): +def parse_external_course_data_str(items_str): """ - Parses `WhoShouldEnrollPage` and `LearningOutcomesPage` items for the Emeritus API. + Parses `WhoShouldEnrollPage` and `LearningOutcomesPage` items for the external API. Args: items_str(str): String containing a list of items separated by `\r\n`. diff --git a/courses/sync_external_courses/emeritus_api_client.py b/courses/sync_external_courses/external_course_sync_api_client.py similarity index 88% rename from courses/sync_external_courses/emeritus_api_client.py rename to courses/sync_external_courses/external_course_sync_api_client.py index 448fb275d..8a4c71b6c 100644 --- a/courses/sync_external_courses/emeritus_api_client.py +++ b/courses/sync_external_courses/external_course_sync_api_client.py @@ -8,15 +8,15 @@ from django.conf import settings -class EmeritusAPIClient: +class ExternalCourseSyncAPIClient: """ API client for Emeritus """ def __init__(self): - self.api_key = settings.EMERITUS_API_KEY - self.base_url = settings.EMERITUS_API_BASE_URL - self.request_timeout = settings.EMERITUS_API_REQUEST_TIMEOUT + self.api_key = settings.EXTERNAL_COURSE_SYNC_API_KEY + self.base_url = settings.EXTERNAL_COURSE_SYNC_API_BASE_URL + self.request_timeout = settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT def get_queries_list(self): """ diff --git a/courses/sync_external_courses/emeritus_api_client_test.py b/courses/sync_external_courses/external_course_sync_api_client_test.py similarity index 63% rename from courses/sync_external_courses/emeritus_api_client_test.py rename to courses/sync_external_courses/external_course_sync_api_client_test.py index 3cd6589d7..222a51415 100644 --- a/courses/sync_external_courses/emeritus_api_client_test.py +++ b/courses/sync_external_courses/external_course_sync_api_client_test.py @@ -1,5 +1,5 @@ """ -Tests for emeritus_api_client +Tests for external_course_sync_api_client """ import json @@ -7,7 +7,7 @@ import pytest -from courses.sync_external_courses.emeritus_api_client import EmeritusAPIClient +from courses.sync_external_courses.external_course_sync_api_client import ExternalCourseSyncAPIClient from mitxpro.test_utils import MockResponse from mitxpro.utils import now_in_utc @@ -22,7 +22,7 @@ ), [ ( - "courses.sync_external_courses.emeritus_api_client.requests.get", + "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse( { "results": [ @@ -35,25 +35,25 @@ ), "get_queries_list", [], - "https://test-emeritus-api.io/api/queries?api_key=test_emeritus_api_key", + "https://test-emeritus-api.io/api/queries?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", ), ( - "courses.sync_external_courses.emeritus_api_client.requests.get", + "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse({"job": {"status": 1}}), "get_job_status", [12], - "https://test-emeritus-api.io/api/jobs/12?api_key=test_emeritus_api_key", + "https://test-emeritus-api.io/api/jobs/12?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", ), ( - "courses.sync_external_courses.emeritus_api_client.requests.get", + "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse({"query_result": {"data": {}}}), "get_query_result", [20], - "https://test-emeritus-api.io/api/query_results/20?api_key=test_emeritus_api_key", + "https://test-emeritus-api.io/api/query_results/20?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", ), ], ) -def test_emeritus_api_client_get_requests( # noqa: PLR0913 +def test_external_course_sync_api_client_get_requests( # noqa: PLR0913 mocker, settings, patch_request_path, @@ -62,14 +62,14 @@ def test_emeritus_api_client_get_requests( # noqa: PLR0913 args, expected_api_url, ): - settings.EMERITUS_API_KEY = "test_emeritus_api_key" - settings.EMERITUS_API_BASE_URL = "https://test-emeritus-api.io" - settings.EMERITUS_API_REQUEST_TIMEOUT = 60 + settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-emeritus-api.io" + settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = 60 mock_get = mocker.patch(patch_request_path) mock_get.return_value = mock_response - client = EmeritusAPIClient() + client = ExternalCourseSyncAPIClient() client_method_map = { "get_queries_list": client.get_queries_list, "get_job_status": client.get_job_status, @@ -84,23 +84,23 @@ def test_emeritus_api_client_get_requests( # noqa: PLR0913 def test_get_query_response(mocker, settings): """ - Tests that `EmeritusAPIClient.get_query_response` makes the expected post request. + Tests that `ExternalCourseSyncAPIClient.get_query_response` makes the expected post request. """ end_date = now_in_utc() start_date = end_date - timedelta(days=1) - settings.EMERITUS_API_KEY = "test_emeritus_api_key" - settings.EMERITUS_API_BASE_URL = "https://test-emeritus-api.io" + settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-emeritus-api.io" mock_post = mocker.patch( - "courses.sync_external_courses.emeritus_api_client.requests.post" + "courses.sync_external_courses.external_course_sync_api_client.requests.post" ) mock_post.return_value = MockResponse({"job": {"id": 1}}) - client = EmeritusAPIClient() + client = ExternalCourseSyncAPIClient() client.get_query_response(1, start_date, end_date) mock_post.assert_called_once_with( - "https://test-emeritus-api.io/api/queries/1/results?api_key=test_emeritus_api_key", + "https://test-emeritus-api.io/api/queries/1/results?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", data=json.dumps( { "parameters": { diff --git a/courses/sync_external_courses/emeritus_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py similarity index 68% rename from courses/sync_external_courses/emeritus_api_test.py rename to courses/sync_external_courses/external_course_sync_api_test.py index e42e3c286..b373cbbe3 100644 --- a/courses/sync_external_courses/emeritus_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -20,21 +20,22 @@ from cms.models import CertificatePage from courses.factories import CourseFactory, CourseRunFactory, PlatformFactory from courses.models import Course -from courses.sync_external_courses.emeritus_api import ( - EmeritusCourse, +from courses.sync_external_courses.external_course_sync_api import ( + ExternalCourse, EmeritusKeyMap, + EMERITUS_PLATFORM_NAME, create_learning_outcomes_page, create_or_update_certificate_page, - create_or_update_emeritus_course_page, - create_or_update_emeritus_course_run, + create_or_update_external_course_page, + create_or_update_external_course_run, create_or_update_product_and_product_version, create_who_should_enroll_in_page, - fetch_emeritus_courses, - generate_emeritus_course_run_tag, + fetch_external_courses, + generate_external_course_run_tag, generate_external_course_run_courseware_id, - parse_emeritus_data_str, + parse_external_course_data_str, save_page_revision, - update_emeritus_course_runs, + update_external_course_runs, ) from ecommerce.factories import ProductFactory, ProductVersionFactory from mitxpro.test_utils import MockResponse @@ -42,69 +43,69 @@ @pytest.fixture -def emeritus_course_data(): +def external_course_data(): """ - Emeritus Course data with Future dates. + External Course data with Future dates. """ with Path( "courses/sync_external_courses/test_data/batch_test.json" ).open() as test_data_file: - emeritus_course_data = json.load(test_data_file)["rows"][0] + external_course_data = json.load(test_data_file)["rows"][0] - emeritus_course_data["start_date"] = "2099-09-30" - emeritus_course_data["end_date"] = "2099-11-30" - emeritus_course_data["course_run_code"] = "MO-DBIP.ELE-99-09#1" - return emeritus_course_data + external_course_data["start_date"] = "2099-09-30" + external_course_data["end_date"] = "2099-11-30" + external_course_data["course_run_code"] = "MO-DBIP.ELE-99-09#1" + return external_course_data @pytest.fixture -def emeritus_expired_course_data(emeritus_course_data): +def external_expired_course_data(external_course_data): """ - Emeritus course JSON with expired dates. + External course JSON with expired dates. """ - expired_emeritus_course_json = emeritus_course_data.copy() - expired_emeritus_course_json["start_date"] = ( + expired_external_course_json = external_course_data.copy() + expired_external_course_json["start_date"] = ( datetime.now() - timedelta(days=2) # noqa: DTZ005 ).strftime("%Y-%m-%d") - expired_emeritus_course_json["end_date"] = ( + expired_external_course_json["end_date"] = ( datetime.now() - timedelta(days=1) # noqa: DTZ005 ).strftime("%Y-%m-%d") - return expired_emeritus_course_json + return expired_external_course_json @pytest.fixture -def emeritus_course_with_bad_data(emeritus_course_data): +def external_course_with_bad_data(external_course_data): """ - Emeritus course JSON with bad data, i.e. program_name, course_code, course_run_code is null. + External course JSON with bad data, i.e. program_name, course_code, course_run_code is null. """ - bad_data_emeritus_course_json = emeritus_course_data.copy() - bad_data_emeritus_course_json["program_name"] = None - return bad_data_emeritus_course_json + bad_data_external_course_json = external_course_data.copy() + bad_data_external_course_json["program_name"] = None + return bad_data_external_course_json @pytest.fixture -def emeritus_course_data_with_null_price(emeritus_course_data): +def external_course_data_with_null_price(external_course_data): """ - Emeritus course JSON with null price. + External course JSON with null price. """ - emeritus_course_json = emeritus_course_data.copy() - emeritus_course_json["list_price"] = None - return emeritus_course_json + external_course_json = external_course_data.copy() + external_course_json["list_price"] = None + return external_course_json @pytest.fixture -def emeritus_course_data_with_non_usd_price(emeritus_course_data): +def external_course_data_with_non_usd_price(external_course_data): """ - Emeritus course JSON with non USD price. + External course JSON with non USD price. """ - emeritus_course_json = emeritus_course_data.copy() - emeritus_course_json["list_currency"] = "INR" - emeritus_course_json["course_run_code"] = "MO-INRC-98-10#1" - return emeritus_course_json + external_course_json = external_course_data.copy() + external_course_json["list_currency"] = "INR" + external_course_json["course_run_code"] = "MO-INRC-98-10#1" + return external_course_json @pytest.mark.parametrize( - ("emeritus_course_run_code", "expected_course_run_tag"), + ("external_course_run_code", "expected_course_run_tag"), [ ("MO-EOB-18-01#1", "18-01-1"), ("MO-EOB-08-01#1", "08-01-1"), @@ -113,14 +114,14 @@ def emeritus_course_data_with_non_usd_price(emeritus_course_data): ("MO-EOB-18-01#212", "18-01-212"), ], ) -def test_generate_emeritus_course_run_tag( - emeritus_course_run_code, expected_course_run_tag +def test_generate_external_course_run_tag( + external_course_run_code, expected_course_run_tag ): """ - Tests that `generate_emeritus_course_run_tag` generates the expected course tag for Emeritus Course Run Codes. + Tests that `generate_external_course_run_tag` generates the expected course tag for External Course Run Codes. """ assert ( - generate_emeritus_course_run_tag(emeritus_course_run_code) + generate_external_course_run_tag(external_course_run_code) == expected_course_run_tag ) @@ -163,33 +164,33 @@ def test_generate_external_course_run_courseware_id( ], ) @pytest.mark.django_db -def test_create_or_update_emeritus_course_page( # noqa: PLR0913 +def test_create_or_update_external_course_page( # noqa: PLR0913 create_course_page, publish_page, is_live_and_draft, create_image, test_image_name_without_extension, - emeritus_course_data, + external_course_data, ): """ - Test that `create_or_update_emeritus_course_page` creates a new course or updates the existing. + Test that `create_or_update_external_course_page` creates a new course or updates the existing. """ home_page = HomePageFactory.create(title="Home Page", subhead="

subhead

") course_index_page = CourseIndexPageFactory.create(parent=home_page, title="Courses") course = CourseFactory.create(is_external=True) if test_image_name_without_extension: - emeritus_course_data["image_name"] = emeritus_course_data["image_name"].split( + external_course_data["image_name"] = external_course_data["image_name"].split( "." )[0] if create_image: - ImageFactory.create(title=emeritus_course_data["image_name"]) + ImageFactory.create(title=external_course_data["image_name"]) if create_course_page: external_course_page = ExternalCoursePageFactory.create( course=course, - title=emeritus_course_data["program_name"], + title=external_course_data["program_name"], external_marketing_url="", duration="", description="", @@ -205,20 +206,20 @@ def test_create_or_update_emeritus_course_page( # noqa: PLR0913 external_course_page.unpublish() external_course_page, course_page_created, course_page_updated = ( - create_or_update_emeritus_course_page( - course_index_page, course, EmeritusCourse(emeritus_course_data) + create_or_update_external_course_page( + course_index_page, course, ExternalCourse(external_course_data, keymap=EmeritusKeyMap()), keymap=EmeritusKeyMap() ) ) external_course_page = external_course_page.revisions.last().as_object() assert external_course_page.external_marketing_url == clean_url( - emeritus_course_data["landing_page_url"], remove_query_params=True + external_course_data["landing_page_url"], remove_query_params=True ) assert external_course_page.course == course assert ( - external_course_page.duration == f"{emeritus_course_data['total_weeks']} Weeks" + external_course_page.duration == f"{external_course_data['total_weeks']} Weeks" ) - assert external_course_page.description == emeritus_course_data["description"] + assert external_course_page.description == external_course_data["description"] assert course_page_created == (not create_course_page) assert course_page_updated == create_course_page @@ -231,19 +232,19 @@ def test_create_or_update_emeritus_course_page( # noqa: PLR0913 assert external_course_page.live assert ( external_course_page.title - == emeritus_course_data["program_name"] + " Draft" + == external_course_data["program_name"] + " Draft" ) else: - assert external_course_page.title == emeritus_course_data["program_name"] + assert external_course_page.title == external_course_data["program_name"] if create_image: assert ( external_course_page.background_image.title - == emeritus_course_data["image_name"] + == external_course_data["image_name"] ) assert ( external_course_page.thumbnail_image.title - == emeritus_course_data["image_name"] + == external_course_data["image_name"] ) @@ -258,7 +259,7 @@ def test_create_or_update_emeritus_course_page( # noqa: PLR0913 ) @pytest.mark.django_db def test_create_or_update_certificate_page( - emeritus_course_data, existing_cert_page, publish_certificate, is_live_and_draft + external_course_data, existing_cert_page, publish_certificate, is_live_and_draft ): """ Tests that `create_or_update_certificate_page` updates the CEUs and does not change the draft or live state. @@ -269,7 +270,7 @@ def test_create_or_update_certificate_page( external_course_page = ExternalCoursePageFactory.create( parent=course_index_page, course=course, - title=emeritus_course_data["program_name"], + title=external_course_data["program_name"], external_marketing_url="", duration="", description="", @@ -287,10 +288,10 @@ def test_create_or_update_certificate_page( certificate_page.unpublish() certificate_page, is_created, is_updated = create_or_update_certificate_page( - external_course_page, EmeritusCourse(emeritus_course_data) + external_course_page, ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) ) certificate_page = certificate_page.revisions.last().as_object() - assert certificate_page.CEUs == emeritus_course_data["ceu"] + assert certificate_page.CEUs == external_course_data["ceu"] assert is_created == (not existing_cert_page) assert is_updated == existing_cert_page @@ -317,9 +318,9 @@ def test_create_who_should_enroll_in_page(): "looking to add critical cybersecurity knowledge and foundational lessons to their resume" ) create_who_should_enroll_in_page( - course_page, parse_emeritus_data_str(who_should_enroll_str) + course_page, parse_external_course_data_str(who_should_enroll_str), keymap=EmeritusKeyMap() ) - assert parse_emeritus_data_str(who_should_enroll_str) == [ + assert parse_external_course_data_str(who_should_enroll_str) == [ item.value.source for item in course_page.who_should_enroll.content ] assert course_page.who_should_enroll is not None @@ -340,17 +341,17 @@ def test_create_learning_outcomes_page(): "organizations to prepare themselves against cybersecurity attacks" ) create_learning_outcomes_page( - course_page, parse_emeritus_data_str(learning_outcomes_str) + course_page, parse_external_course_data_str(learning_outcomes_str), keymap=EmeritusKeyMap() ) - assert parse_emeritus_data_str(learning_outcomes_str) == [ + assert parse_external_course_data_str(learning_outcomes_str) == [ item.value for item in course_page.outcomes.outcome_items ] assert course_page.outcomes is not None -def test_parse_emeritus_data_str(): +def test_parse_external_course_data_str(): """ - Tests that `parse_emeritus_data_str` parses who should enroll and learning outcomes strings as expected. + Tests that `parse_external_course_data_str` parses who should enroll and learning outcomes strings as expected. """ data_str = ( "This program will enable you to:\r\n● Gain an overview of cybersecurity risk " @@ -360,7 +361,7 @@ def test_parse_emeritus_data_str(): "of specific threat models and methodologies\r\n● Understand the guidelines for " "organizations to prepare themselves against cybersecurity attacks" ) - assert parse_emeritus_data_str(data_str) == [ + assert parse_external_course_data_str(data_str) == [ "Gain an overview of cybersecurity risk management, including " "its foundational concepts and relevant regulations", "Explore the domains covering various aspects of cloud technology", @@ -379,18 +380,18 @@ def test_parse_emeritus_data_str(): ], ) @pytest.mark.django_db -def test_create_or_update_emeritus_course_run( - create_existing_course_run, empty_dates, emeritus_course_data +def test_create_or_update_external_course_run( + create_existing_course_run, empty_dates, external_course_data ): """ - Tests that `create_or_update_emeritus_course_run` creates or updates a course run + Tests that `create_or_update_external_course_run` creates or updates a course run """ - emeritus_course = EmeritusCourse(emeritus_course_data) + external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) course = CourseFactory.create() if create_existing_course_run: run = CourseRunFactory.create( course=course, - external_course_run_id=emeritus_course.course_run_code, + external_course_run_id=external_course.course_run_code, enrollment_start=None, enrollment_end=None, expiration_date=None, @@ -400,12 +401,12 @@ def test_create_or_update_emeritus_course_run( run.end_date = None run.save() - run, run_created, run_updated = create_or_update_emeritus_course_run( - course, emeritus_course + run, run_created, run_updated = create_or_update_external_course_run( + course, external_course ) course_runs = course.courseruns.all() course_run_courseware_id = generate_external_course_run_courseware_id( - emeritus_course.course_run_tag, course.readable_id + external_course.course_run_tag, course.readable_id ) assert len(course_runs) == 1 @@ -414,20 +415,20 @@ def test_create_or_update_emeritus_course_run( assert run_updated == create_existing_course_run if create_existing_course_run: expected_data = { - "external_course_run_id": emeritus_course.course_run_code, - "start_date": emeritus_course.start_date, - "end_date": emeritus_course.end_date, - "enrollment_end": emeritus_course.enrollment_end, + "external_course_run_id": external_course.course_run_code, + "start_date": external_course.start_date, + "end_date": external_course.end_date, + "enrollment_end": external_course.enrollment_end, } else: expected_data = { - "title": emeritus_course.course_title, - "external_course_run_id": emeritus_course.course_run_code, + "title": external_course.course_title, + "external_course_run_id": external_course.course_run_code, "courseware_id": course_run_courseware_id, - "run_tag": emeritus_course.course_run_tag, - "start_date": emeritus_course.start_date, - "end_date": emeritus_course.end_date, - "enrollment_end": emeritus_course.enrollment_end, + "run_tag": external_course.course_run_tag, + "start_date": external_course.start_date, + "end_date": external_course.end_date, + "enrollment_end": external_course.enrollment_end, "live": True, } for attr_name, expected_value in expected_data.items(): @@ -436,25 +437,25 @@ def test_create_or_update_emeritus_course_run( @pytest.mark.parametrize("create_existing_data", [True, False]) @pytest.mark.django_db -def test_update_emeritus_course_runs( # noqa: PLR0915 +def test_update_external_course_runs( # noqa: PLR0915 create_existing_data, - emeritus_expired_course_data, - emeritus_course_with_bad_data, - emeritus_course_data_with_null_price, - emeritus_course_data_with_non_usd_price, + external_expired_course_data, + external_course_with_bad_data, + external_course_data_with_null_price, + external_course_data_with_non_usd_price, ): """ - Tests that `update_emeritus_course_runs` creates new courses and updates existing. + Tests that `update_external_course_runs` creates new courses and updates existing. """ with Path( "courses/sync_external_courses/test_data/batch_test.json" ).open() as test_data_file: - emeritus_course_runs = json.load(test_data_file)["rows"] + external_course_runs = json.load(test_data_file)["rows"] - platform = PlatformFactory.create(name=EmeritusKeyMap.PLATFORM_NAME.value) + platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) if create_existing_data: - for run in random.sample(emeritus_course_runs, len(emeritus_course_runs) // 2): + for run in random.sample(external_course_runs, len(external_course_runs) // 2): course = CourseFactory.create( title=run["program_name"], platform=platform, @@ -486,11 +487,11 @@ def test_update_emeritus_course_runs( # noqa: PLR0915 product = ProductFactory.create(content_object=course_run) ProductVersionFactory.create(product=product, price=run["list_price"]) - emeritus_course_runs.append(emeritus_expired_course_data) - emeritus_course_runs.append(emeritus_course_with_bad_data) - emeritus_course_runs.append(emeritus_course_data_with_null_price) - emeritus_course_runs.append(emeritus_course_data_with_non_usd_price) - stats = update_emeritus_course_runs(emeritus_course_runs) + external_course_runs.append(external_expired_course_data) + external_course_runs.append(external_course_with_bad_data) + external_course_runs.append(external_course_data_with_null_price) + external_course_runs.append(external_course_data_with_non_usd_price) + stats = update_external_course_runs(external_course_runs, keymap=EmeritusKeyMap()) courses = Course.objects.filter(platform=platform) num_courses_created = 2 if create_existing_data else 4 @@ -514,51 +515,51 @@ def test_update_emeritus_course_runs( # noqa: PLR0915 assert len(stats["product_versions_created"]) == num_product_versions_created assert len(stats["course_runs_without_prices"]) == 1 - for emeritus_course_run in emeritus_course_runs: + for external_course_run in external_course_runs: if ( - emeritus_course_run["course_run_code"] in stats["course_runs_skipped"] - or emeritus_course_run["course_run_code"] in stats["course_runs_expired"] + external_course_run["course_run_code"] in stats["course_runs_skipped"] + or external_course_run["course_run_code"] in stats["course_runs_expired"] ): continue course = Course.objects.filter( platform=platform, - external_course_id=emeritus_course_run["course_code"], + external_course_id=external_course_run["course_code"], is_external=True, ).first() assert course is not None assert ( course.courseruns.filter( - external_course_run_id=emeritus_course_run["course_run_code"] + external_course_run_id=external_course_run["course_run_code"] ).count() == 1 ) assert hasattr(course, "externalcoursepage") assert ( course.courseruns.filter( - external_course_run_id=emeritus_course_run["course_run_code"] + external_course_run_id=external_course_run["course_run_code"] ) .first() .current_price - == emeritus_course_run["list_price"] + == external_course_run["list_price"] ) course_page = course.externalcoursepage - if emeritus_course_run["program_for"]: + if external_course_run["program_for"]: assert course_page.who_should_enroll is not None - if emeritus_course_run["learning_outcomes"]: + if external_course_run["learning_outcomes"]: assert course_page.outcomes is not None - if emeritus_course_run.get("ceu", ""): + if external_course_run.get("ceu", ""): certificate_page = course_page.get_child_page_of_type_including_draft( CertificatePage ) assert certificate_page - assert certificate_page.CEUs == emeritus_course_run["ceu"] + assert certificate_page.CEUs == external_course_run["ceu"] -def test_fetch_emeritus_courses_success(settings, mocker): +def test_fetch_external_courses_success(settings, mocker): """ - Tests that `fetch_emeritus_courses` makes the required calls to the `Emeritus` API. Tests the success scenario. + Tests that `fetch_external_courses` makes the required calls to the `Emeritus` API. Tests the success scenario. Here is the expected flow: 1. Make a get request to get a list of reports. @@ -568,21 +569,21 @@ def test_fetch_emeritus_courses_success(settings, mocker): 5. If job status is 1 or 2, it is in progress. Wait for 2 seconds and make a get request for Job status. 6. If job status is 3, the results are ready, make a get request to collect the results and return the data. """ - settings.EMERITUS_API_BASE_URL = "https://test_emeritus_api.io" - settings.EMERITUS_API_KEY = "test_emeritus_api_key" - settings.EMERITUS_API_REQUEST_TIMEOUT = 60 + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test_external_course_sync_api.io" + settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" + settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = 60 mock_get = mocker.patch( - "courses.sync_external_courses.emeritus_api_client.requests.get" + "courses.sync_external_courses.external_course_sync_api_client.requests.get" ) mock_post = mocker.patch( - "courses.sync_external_courses.emeritus_api_client.requests.post" + "courses.sync_external_courses.external_course_sync_api_client.requests.post" ) with Path( "courses/sync_external_courses/test_data/batch_test.json" ).open() as test_data_file: - emeritus_course_runs = json.load(test_data_file) + external_course_runs = json.load(test_data_file) batch_query = { "id": 77, @@ -593,39 +594,39 @@ def test_fetch_emeritus_courses_success(settings, mocker): MockResponse({"job": {"status": 1}}), MockResponse({"job": {"status": 2}}), MockResponse({"job": {"status": 3, "query_result_id": 1}}), - MockResponse({"query_result": {"data": emeritus_course_runs}}), + MockResponse({"query_result": {"data": external_course_runs}}), ] mock_post.side_effect = [MockResponse({"job": {"id": 1}})] - actual_course_runs = fetch_emeritus_courses() + actual_course_runs = fetch_external_courses(keymap=EmeritusKeyMap()) mock_get.assert_any_call( - "https://test_emeritus_api.io/api/queries?api_key=test_emeritus_api_key", + "https://test_external_course_sync_api.io/api/queries?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", timeout=60, ) mock_post.assert_called_once() mock_get.assert_any_call( - "https://test_emeritus_api.io/api/jobs/1?api_key=test_emeritus_api_key", + "https://test_external_course_sync_api.io/api/jobs/1?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", timeout=60, ) mock_get.assert_any_call( - "https://test_emeritus_api.io/api/query_results/1?api_key=test_emeritus_api_key", + "https://test_external_course_sync_api.io/api/query_results/1?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", timeout=60, ) - assert actual_course_runs == emeritus_course_runs["rows"] + assert actual_course_runs == external_course_runs["rows"] -def test_fetch_emeritus_courses_error(settings, mocker, caplog): +def test_fetch_external_courses_error(settings, mocker, caplog): """ - Tests that `fetch_emeritus_courses` specific calls to the Emeritus API and Fails for Job status 3 and 4. + Tests that `fetch_external_courses` specific calls to the External Course Sync API and Fails for Job status 3 and 4. """ - settings.EMERITUS_API_BASE_URL = "https://test_emeritus_api.com" - settings.EMERITUS_API_KEY = "test_emeritus_api_key" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test_external_course_sync_api.com" + settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" mock_get = mocker.patch( - "courses.sync_external_courses.emeritus_api_client.requests.get" + "courses.sync_external_courses.external_course_sync_api_client.requests.get" ) mock_post = mocker.patch( - "courses.sync_external_courses.emeritus_api_client.requests.post" + "courses.sync_external_courses.external_course_sync_api_client.requests.post" ) batch_query = { @@ -640,7 +641,7 @@ def test_fetch_emeritus_courses_error(settings, mocker, caplog): ] mock_post.side_effect = [MockResponse({"job": {"id": 1}})] with caplog.at_level(logging.ERROR): - fetch_emeritus_courses() + fetch_external_courses(keymap=EmeritusKeyMap()) assert "Job failed!" in caplog.text assert "Something unexpected happened!" in caplog.text @@ -663,7 +664,7 @@ def test_fetch_emeritus_courses_error(settings, mocker, caplog): ) @pytest.mark.django_db def test_create_or_update_product_and_product_version( # noqa: PLR0913 - emeritus_course_data, + external_course_data, create_existing_product, existing_price, new_price, @@ -674,18 +675,18 @@ def test_create_or_update_product_and_product_version( # noqa: PLR0913 """ Tests that `create_or_update_product_and_product_version` creates or updates products and versions as required. """ - emeritus_course_data["list_price"] = new_price - emeritus_course = EmeritusCourse(emeritus_course_data) - platform = PlatformFactory.create(name=EmeritusKeyMap.PLATFORM_NAME) + external_course_data["list_price"] = new_price + external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) course = CourseFactory.create( - external_course_id=emeritus_course.course_code, + external_course_id=external_course.course_code, platform=platform, is_external=True, - title=emeritus_course.course_title, - readable_id=emeritus_course.course_readable_id, + title=external_course.course_title, + readable_id=external_course.course_readable_id, live=True, ) - course_run, _, _ = create_or_update_emeritus_course_run(course, emeritus_course) + course_run, _, _ = create_or_update_external_course_run(course, external_course) if create_existing_product: product = ProductFactory.create(content_object=course_run) @@ -694,7 +695,7 @@ def test_create_or_update_product_and_product_version( # noqa: PLR0913 ProductVersionFactory.create(product=product, price=existing_price) product_created, version_created = create_or_update_product_and_product_version( - emeritus_course, course_run + external_course, course_run ) assert course_run.current_price == expected_price assert product_created == expected_product_created @@ -730,7 +731,7 @@ def test_save_page_revision(is_draft_page, has_unpublished_changes): latest_revision = external_course_page.get_latest_revision_as_object() latest_revision.external_marketing_url = ( - "https://test-emeritus-api.io/Internet-of-things-iot-design-and-applications" + "https://test-external-course-sync-api.io/Internet-of-things-iot-design-and-applications" ) save_page_revision(external_course_page, latest_revision) @@ -769,17 +770,17 @@ def test_save_page_revision(is_draft_page, has_unpublished_changes): (None, None, None, False), ], ) -def test_emeritus_course_validate_required_fields( - emeritus_course_data, title, course_code, course_run_code, is_valid +def test_external_course_validate_required_fields( + external_course_data, title, course_code, course_run_code, is_valid ): """ - Tests that EmeritusCourse.validate_required_fields validates required fields. + Tests that ExternalCourse.validate_required_fields validates required fields. """ - emeritus_course = EmeritusCourse(emeritus_course_data) - emeritus_course.course_title = title.strip() if title else title - emeritus_course.course_code = course_code - emeritus_course.course_run_code = course_run_code - assert emeritus_course.validate_required_fields() == is_valid + external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + external_course.course_title = title.strip() if title else title + external_course.course_code = course_code + external_course.course_run_code = course_run_code + assert external_course.validate_required_fields(keymap=EmeritusKeyMap()) == is_valid @pytest.mark.parametrize( @@ -792,15 +793,15 @@ def test_emeritus_course_validate_required_fields( ("PKR", False), ], ) -def test_emeritus_course_validate_list_currency( - emeritus_course_data, list_currency, is_valid +def test_external_course_validate_list_currency( + external_course_data, list_currency, is_valid ): """ - Tests that the `USD` is the only valid currency for the Emeritus courses. + Tests that the `USD` is the only valid currency for the External courses. """ - emeritus_course = EmeritusCourse(emeritus_course_data) - emeritus_course.list_currency = list_currency - assert emeritus_course.validate_list_currency() == is_valid + external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + external_course.list_currency = list_currency + assert external_course.validate_list_currency() == is_valid @pytest.mark.parametrize( @@ -810,10 +811,10 @@ def test_emeritus_course_validate_list_currency( (now_in_utc() - timedelta(days=1), False), ], ) -def test_emeritus_course_validate_end_date(emeritus_course_data, end_date, is_valid): +def test_external_course_validate_end_date(external_course_data, end_date, is_valid): """ - Tests that the valid end date is in the future for Emeritus courses. + Tests that the valid end date is in the future for External courses. """ - emeritus_course = EmeritusCourse(emeritus_course_data) - emeritus_course.end_date = end_date - assert emeritus_course.validate_end_date() == is_valid + external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + external_course.end_date = end_date + assert external_course.validate_end_date() == is_valid diff --git a/courses/tasks.py b/courses/tasks.py index a546c1acd..1ebcb13a2 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -11,8 +11,8 @@ from courses.models import CourseRun, CourseRunCertificate from courses.sync_external_courses.emeritus_api import ( - fetch_emeritus_courses, - update_emeritus_course_runs, + fetch_external_courses, + update_external_course_runs, ) from courses.utils import ( ensure_course_run_grade, @@ -120,5 +120,5 @@ def task_sync_emeritus_course_runs(): log.info("External Course sync is disabled.") return - emeritus_course_runs = fetch_emeritus_courses() - update_emeritus_course_runs(emeritus_course_runs) + emeritus_course_runs = fetch_external_courses() + update_external_course_runs(emeritus_course_runs) diff --git a/courses/tasks_test.py b/courses/tasks_test.py index a615c25f9..dab9a664e 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -28,10 +28,10 @@ def test_sync_courseruns_data(mocker): def test_task_sync_emeritus_course_runs(mocker, settings): """Test task_sync_emeritus_course_runs calls the right api functionality""" settings.FEATURES["ENABLE_EXTERNAL_COURSE_SYNC"] = True - mock_fetch_emeritus_courses = mocker.patch("courses.tasks.fetch_emeritus_courses") - mock_update_emeritus_course_runs = mocker.patch( - "courses.tasks.update_emeritus_course_runs" + mock_fetch_external_courses = mocker.patch("courses.tasks.fetch_external_courses") + mock_update_external_course_runs = mocker.patch( + "courses.tasks.update_external_course_runs" ) task_sync_emeritus_course_runs.delay() - mock_fetch_emeritus_courses.assert_called_once() - mock_update_emeritus_course_runs.assert_called_once() + mock_fetch_external_courses.assert_called_once() + mock_update_external_course_runs.assert_called_once() diff --git a/mitxpro/settings.py b/mitxpro/settings.py index 4816f8f5d..973091853 100644 --- a/mitxpro/settings.py +++ b/mitxpro/settings.py @@ -1139,18 +1139,18 @@ description="Timeout (in seconds) for requests made via the edX API client", ) -EMERITUS_API_KEY = get_string( - name="EMERITUS_API_KEY", +EXTERNAL_COURSE_SYNC_API_KEY = get_string( + name="EXTERNAL_COURSE_SYNC_API_KEY", default=None, description="The API Key for Emeritus API", required=True, ) -EMERITUS_API_BASE_URL = get_string( - name="EMERITUS_API_BASE_URL", +EXTERNAL_COURSE_SYNC_API_BASE_URL = get_string( + name="EXTERNAL_COURSE_SYNC_API_BASE_URL", default="https://mit-xpro.emeritus-analytics.io/", description="Base API URL for Emeritus API", ) -EMERITUS_API_REQUEST_TIMEOUT = get_int( +EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = get_int( name="EMERITUS_API_TIMEOUT", default=60, description="API request timeout for Emeritus APIs in seconds", From 846f5c54b6ffb60d29c99e47912c1bd44a405033 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Wed, 4 Dec 2024 11:12:26 +0000 Subject: [PATCH 03/17] refactor: added sync_daily in platform and refactored courses/task.py to be more generic --- .../commands/sync_external_course_runs.py | 11 ++++------ .../migrations/0041_platform_sync_daily.py | 18 +++++++++++++++ courses/models.py | 1 + .../external_course_sync_api.py | 19 ++++++++-------- .../external_course_sync_api_test.py | 5 ++--- courses/tasks.py | 20 ++++++++++++----- courses/tasks_test.py | 22 ++++++++++++++----- mitxpro/settings.py | 20 ++++++++--------- 8 files changed, 77 insertions(+), 39 deletions(-) create mode 100644 courses/migrations/0041_platform_sync_daily.py diff --git a/courses/management/commands/sync_external_course_runs.py b/courses/management/commands/sync_external_course_runs.py index 4243d7da8..c0abfc002 100644 --- a/courses/management/commands/sync_external_course_runs.py +++ b/courses/management/commands/sync_external_course_runs.py @@ -3,8 +3,7 @@ from django.core.management.base import BaseCommand from courses.sync_external_courses.external_course_sync_api import ( - EMERITUS_PLATFORM_NAME, - GLOBAL_ALUMNI_PLATFORM_NAME, + VENDOR_KEYMAPS, EmeritusKeyMap, GlobalAlumniKeyMap, fetch_external_courses, @@ -39,13 +38,11 @@ def handle(self, *args, **options): # noqa: ARG002 return vendor_name = options["vendor_name"] - if vendor_name.lower() == EMERITUS_PLATFORM_NAME.lower(): - keymap = EmeritusKeyMap() - elif vendor_name.lower() == GLOBAL_ALUMNI_PLATFORM_NAME.lower(): - keymap = GlobalAlumniKeyMap() - else: + keymap = VENDOR_KEYMAPS.get(vendor_name.lower()) + if not keymap: self.stdout.write(self.style.ERROR(f"Unknown vendor name {vendor_name}.")) return + self.stdout.write(f"Starting course sync for {vendor_name}.") emeritus_course_runs = fetch_external_courses(keymap) stats = update_external_course_runs(emeritus_course_runs, keymap) diff --git a/courses/migrations/0041_platform_sync_daily.py b/courses/migrations/0041_platform_sync_daily.py new file mode 100644 index 000000000..000d5b2bf --- /dev/null +++ b/courses/migrations/0041_platform_sync_daily.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.16 on 2024-12-04 07:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('courses', '0040_alter_courserun_courseware_id'), + ] + + operations = [ + migrations.AddField( + model_name='platform', + name='sync_daily', + field=models.BooleanField(default=False), + ), + ] diff --git a/courses/models.py b/courses/models.py index 9cbf6cae8..28c1e3889 100644 --- a/courses/models.py +++ b/courses/models.py @@ -214,6 +214,7 @@ class Platform(TimestampedModel, ValidateOnSaveMixin): """ name = models.CharField(max_length=255, unique=True) + sync_daily = models.BooleanField(default=False) def __str__(self): return self.name diff --git a/courses/sync_external_courses/external_course_sync_api.py b/courses/sync_external_courses/external_course_sync_api.py index 572ef3f92..8c5e09bf9 100644 --- a/courses/sync_external_courses/external_course_sync_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -30,9 +30,6 @@ log = logging.getLogger(__name__) -EMERITUS_PLATFORM_NAME = "Emeritus" -GLOBAL_ALUMNI_PLATFORM_NAME = "Global Alumni" - class BaseKeyMap: """ Base class for course sync keys with common attributes. @@ -52,11 +49,11 @@ def __init__(self, platform_name, report_names): self.report_names = report_names @property - def COURSE_PAGE_SUBHEAD(self): + def course_page_subhead(self): return f"Delivered in collaboration with {self.platform_name}." @property - def LEARNING_OUTCOMES_PAGE_SUBHEAD(self): + def learning_outcomes_page_subhead(self): return ( f"MIT xPRO is collaborating with online education provider {self.platform_name} to " "deliver this online course. By clicking LEARN MORE, you will be taken to " @@ -70,7 +67,7 @@ class EmeritusKeyMap(BaseKeyMap): Emeritus course sync keys. """ def __init__(self): - super().__init__(platform_name=EMERITUS_PLATFORM_NAME, report_names=["Batch"]) + super().__init__(platform_name="Emeritus", report_names=["Batch"]) class GlobalAlumniKeyMap(BaseKeyMap): @@ -78,8 +75,12 @@ class GlobalAlumniKeyMap(BaseKeyMap): Global Alumni course sync keys. """ def __init__(self): - super().__init__(platform_name=GLOBAL_ALUMNI_PLATFORM_NAME, report_names=["GA - Batch"]) + super().__init__(platform_name="Global Alumni", report_names=["GA - Batch"]) +VENDOR_KEYMAPS = { + "emeritus": EmeritusKeyMap(), + "global alumni": GlobalAlumniKeyMap(), +} class ExternalCourseSyncAPIJobStatus(Enum): """ @@ -543,7 +544,7 @@ def create_or_update_external_course_page(course_index_page, course, external_co course=course, title=external_course.course_title, external_marketing_url=external_course.marketing_url, - subhead=keymap.COURSE_PAGE_SUBHEAD, + subhead=keymap.course_page_subhead, duration=external_course.duration, format=external_course.format, description=external_course.description, @@ -684,7 +685,7 @@ def create_learning_outcomes_page(course_page, outcomes_list, keymap): learning_outcome_page = LearningOutcomesPage( heading=keymap.LEARNING_OUTCOMES_PAGE_HEADING, - sub_heading=keymap.LEARNING_OUTCOMES_PAGE_SUBHEAD, + sub_heading=keymap.learning_outcomes_page_subhead, outcome_items=outcome_items, ) course_page.add_child(instance=learning_outcome_page) diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index b373cbbe3..53a9801d1 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -23,7 +23,6 @@ from courses.sync_external_courses.external_course_sync_api import ( ExternalCourse, EmeritusKeyMap, - EMERITUS_PLATFORM_NAME, create_learning_outcomes_page, create_or_update_certificate_page, create_or_update_external_course_page, @@ -452,7 +451,7 @@ def test_update_external_course_runs( # noqa: PLR0915 ).open() as test_data_file: external_course_runs = json.load(test_data_file)["rows"] - platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) + platform = PlatformFactory.create(name="Emeritus") if create_existing_data: for run in random.sample(external_course_runs, len(external_course_runs) // 2): @@ -677,7 +676,7 @@ def test_create_or_update_product_and_product_version( # noqa: PLR0913 """ external_course_data["list_price"] = new_price external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) - platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) + platform = PlatformFactory.create(name="Emeritus") course = CourseFactory.create( external_course_id=external_course.course_code, platform=platform, diff --git a/courses/tasks.py b/courses/tasks.py index 1ebcb13a2..9c7aed881 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -9,8 +9,9 @@ from django.db.models import Q from requests.exceptions import HTTPError -from courses.models import CourseRun, CourseRunCertificate -from courses.sync_external_courses.emeritus_api import ( +from courses.models import CourseRun, CourseRunCertificate, Platform +from courses.sync_external_courses.external_course_sync_api import ( + VENDOR_KEYMAPS, fetch_external_courses, update_external_course_runs, ) @@ -114,11 +115,20 @@ def sync_courseruns_data(): @app.task -def task_sync_emeritus_course_runs(): +def task_sync_external_course_runs(): """Task to sync Emeritus course runs""" if not settings.FEATURES.get("ENABLE_EXTERNAL_COURSE_SYNC", False): log.info("External Course sync is disabled.") return - emeritus_course_runs = fetch_external_courses() - update_external_course_runs(emeritus_course_runs) + platforms = Platform.objects.filter(sync_daily=True) + for platform in platforms: + keymap = VENDOR_KEYMAPS.get(platform.name.lower()) + if not keymap: + log.exception( + "The platform '%s' does not have a sync API configured. Please disable the 'sync_daily' setting for this platform.", + platform.name, + ) + continue + emeritus_course_runs = fetch_external_courses(keymap) + update_external_course_runs(emeritus_course_runs, keymap) diff --git a/courses/tasks_test.py b/courses/tasks_test.py index dab9a664e..2e7a69dd3 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -4,8 +4,8 @@ import pytest -from courses.factories import CourseRunFactory -from courses.tasks import sync_courseruns_data, task_sync_emeritus_course_runs +from courses.factories import CourseRunFactory, PlatformFactory +from courses.tasks import sync_courseruns_data, task_sync_external_course_runs pytestmark = [pytest.mark.django_db] @@ -25,13 +25,25 @@ def test_sync_courseruns_data(mocker): assert Counter(actual_course_runs) == Counter(course_runs) -def test_task_sync_emeritus_course_runs(mocker, settings): - """Test task_sync_emeritus_course_runs calls the right api functionality""" +def test_task_sync_external_course_runs(mocker, settings): + """Test task_sync_external_course_runs skips platforms not in VENDOR_KEYMAPS""" settings.FEATURES["ENABLE_EXTERNAL_COURSE_SYNC"] = True + mock_fetch_external_courses = mocker.patch("courses.tasks.fetch_external_courses") mock_update_external_course_runs = mocker.patch( "courses.tasks.update_external_course_runs" ) - task_sync_emeritus_course_runs.delay() + log_mock = mocker.patch("courses.tasks.log") + + PlatformFactory.create(name="Emeritus", sync_daily=True) + PlatformFactory.create(name="UnknownPlatform", sync_daily=True) + + task_sync_external_course_runs.delay() + mock_fetch_external_courses.assert_called_once() mock_update_external_course_runs.assert_called_once() + + log_mock.exception.assert_called_once_with( + "The platform '%s' does not have a sync API configured. Please disable the 'sync_daily' setting for this platform.", + "UnknownPlatform", + ) \ No newline at end of file diff --git a/mitxpro/settings.py b/mitxpro/settings.py index 973091853..37f164cb3 100644 --- a/mitxpro/settings.py +++ b/mitxpro/settings.py @@ -761,14 +761,14 @@ ) CRON_EXTERNAL_COURSERUN_SYNC_HOURS = get_string( - name="CRON_EMERITUS_COURSERUN_SYNC_HOURS", + name="CRON_EXTERNAL_COURSERUN_SYNC_HOURS", default="0", - description="'hours' value for the 'sync-emeritus-course-runs' scheduled task (defaults to midnight)", + description="'hours' value for the 'sync-external-course-runs' scheduled task (defaults to midnight)", ) CRON_EXTERNAL_COURSERUN_SYNC_DAYS = get_string( - name="CRON_EMERITUS_COURSERUN_SYNC_DAYS", + name="CRON_EXTERNAL_COURSERUN_SYNC_DAYS", default=None, - description="'day_of_week' value for 'sync-emeritus-course-runs' scheduled task (default will run once a day).", + description="'day_of_week' value for 'sync-external-course-runs' scheduled task (default will run once a day).", ) CRON_BASKET_DELETE_HOURS = get_string( @@ -885,8 +885,8 @@ month_of_year="*", ), }, - "sync-emeritus-course-runs": { - "task": "courses.tasks.task_sync_emeritus_course_runs", + "sync-external-course-runs": { + "task": "courses.tasks.task_sync_external_course_runs", "schedule": crontab( minute="0", hour=CRON_EXTERNAL_COURSERUN_SYNC_HOURS, @@ -1142,18 +1142,18 @@ EXTERNAL_COURSE_SYNC_API_KEY = get_string( name="EXTERNAL_COURSE_SYNC_API_KEY", default=None, - description="The API Key for Emeritus API", + description="The API Key for external course sync API", required=True, ) EXTERNAL_COURSE_SYNC_API_BASE_URL = get_string( name="EXTERNAL_COURSE_SYNC_API_BASE_URL", default="https://mit-xpro.emeritus-analytics.io/", - description="Base API URL for Emeritus API", + description="Base API URL for external course sync API", ) EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = get_int( - name="EMERITUS_API_TIMEOUT", + name="EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT", default=60, - description="API request timeout for Emeritus APIs in seconds", + description="API request timeout for external course sync APIs in seconds", ) # django debug toolbar only in debug mode From 10a171a4d0642d7582194146e1cf6203f812dd19 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Wed, 4 Dec 2024 11:49:07 +0000 Subject: [PATCH 04/17] fix: test --- app.json | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/app.json b/app.json index 814ccbe09..c46d189c3 100644 --- a/app.json +++ b/app.json @@ -98,12 +98,12 @@ "description": "'hours' value for the 'generate-course-certificate' scheduled task (defaults to midnight)", "required": false }, - "CRON_EMERITUS_COURSERUN_SYNC_DAYS": { - "description": "'day_of_week' value for 'sync-emeritus-course-runs' scheduled task (default will run once a day).", + "CRON_EXTERNAL_COURSERUN_SYNC_DAYS": { + "description": "'day_of_week' value for 'sync-external-course-runs' scheduled task (default will run once a day).", "required": false }, - "CRON_EMERITUS_COURSERUN_SYNC_HOURS": { - "description": "'hours' value for the 'sync-emeritus-course-runs' scheduled task (defaults to midnight)", + "CRON_EXTERNAL_COURSERUN_SYNC_HOURS": { + "description": "'hours' value for the 'sync-external-course-runs' scheduled task (defaults to midnight)", "required": false }, "CSRF_TRUSTED_ORIGINS": { @@ -234,20 +234,20 @@ "description": "Timeout (in seconds) for requests made via the edX API client", "required": false }, + "ENROLLMENT_CHANGE_SHEET_ID": { + "description": "ID of the Google Sheet that contains the enrollment change request worksheets (refunds, transfers, etc)", + "required": false + }, "EXTERNAL_COURSE_SYNC_API_BASE_URL": { - "description": "Base API URL for Emeritus API", + "description": "Base API URL for external course sync API", "required": false }, "EXTERNAL_COURSE_SYNC_API_KEY": { - "description": "The API Key for Emeritus API", + "description": "The API Key for external course sync API", "required": true }, - "EMERITUS_API_TIMEOUT": { - "description": "API request timeout for Emeritus APIs in seconds", - "required": false - }, - "ENROLLMENT_CHANGE_SHEET_ID": { - "description": "ID of the Google Sheet that contains the enrollment change request worksheets (refunds, transfers, etc)", + "EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT": { + "description": "API request timeout for external course sync APIs in seconds", "required": false }, "GA_TRACKING_ID": { From a961397d2804582776426550f94750f5c3435765 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Wed, 4 Dec 2024 15:23:57 +0000 Subject: [PATCH 05/17] fix: some issues --- .github/workflows/ci.yml | 2 +- .../external_course_sync_api.py | 18 +++++++++--------- .../external_course_sync_api_client_test.py | 12 ++++++------ courses/tasks_test.py | 6 +++--- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ea4a75a9b..bb741893b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,7 +117,7 @@ jobs: DATABASE_URL: postgres://postgres:postgres@localhost:5432/postgres # pragma: allowlist secret WEBPACK_DISABLE_LOADER_STATS: "True" ELASTICSEARCH_URL: localhost:9200 - EXTERNAL_COURSE_SYNC_API_KEY: fake_EXTERNAL_COURSE_SYNC_API_KEY # pragma: allowlist secret + EXTERNAL_COURSE_SYNC_API_KEY: fake_external_course_sync_api_key # pragma: allowlist secret MAILGUN_KEY: fake_mailgun_key MAILGUN_SENDER_DOMAIN: other.fake.site MITOL_DIGITAL_CREDENTIALS_VERIFY_SERVICE_BASE_URL: http://localhost:5000 diff --git a/courses/sync_external_courses/external_course_sync_api.py b/courses/sync_external_courses/external_course_sync_api.py index 8c5e09bf9..5f318e6ad 100644 --- a/courses/sync_external_courses/external_course_sync_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -34,15 +34,15 @@ class BaseKeyMap: """ Base class for course sync keys with common attributes. """ - DATE_FORMAT = "%Y-%m-%d" - REQUIRED_FIELDS = [ + date_format = "%Y-%m-%d" + required_fields = [ "course_title", "course_code", "course_run_code", "list_currency", ] - WHO_SHOULD_ENROLL_PAGE_HEADING = "WHO SHOULD ENROLL" - LEARNING_OUTCOMES_PAGE_HEADING = "WHAT YOU WILL LEARN" + who_should_enroll_page_heading = "WHO SHOULD ENROLL" + learning_outcomes_page_heading = "WHAT YOU WILL LEARN" def __init__(self, platform_name, report_names): self.platform_name = platform_name @@ -120,10 +120,10 @@ def __init__(self, external_course_json, keymap): self.list_currency = external_course_json.get("list_currency") self.start_date = strip_datetime( - external_course_json.get("start_date"), keymap.DATE_FORMAT + external_course_json.get("start_date"), keymap.date_format ) end_datetime = strip_datetime( - external_course_json.get("end_date"), keymap.DATE_FORMAT + external_course_json.get("end_date"), keymap.date_format ) self.end_date = ( end_datetime.replace(hour=23, minute=59) if end_datetime else None @@ -164,7 +164,7 @@ def validate_required_fields(self, keymap): """ Validates the course data. """ - for field in keymap.REQUIRED_FIELDS: + for field in keymap.required_fields: if not getattr(self, field, None): log.info(f"Missing required field {field}") # noqa: G004 return False @@ -664,7 +664,7 @@ def create_who_should_enroll_in_page(course_page, who_should_enroll_list, keymap ) who_should_enroll_page = WhoShouldEnrollPage( - heading=keymap.WHO_SHOULD_ENROLL_PAGE_HEADING, + heading=keymap.who_should_enroll_page_heading, content=content, ) course_page.add_child(instance=who_should_enroll_page) @@ -684,7 +684,7 @@ def create_learning_outcomes_page(course_page, outcomes_list, keymap): ) learning_outcome_page = LearningOutcomesPage( - heading=keymap.LEARNING_OUTCOMES_PAGE_HEADING, + heading=keymap.learning_outcomes_page_heading, sub_heading=keymap.learning_outcomes_page_subhead, outcome_items=outcome_items, ) diff --git a/courses/sync_external_courses/external_course_sync_api_client_test.py b/courses/sync_external_courses/external_course_sync_api_client_test.py index 222a51415..d612b166a 100644 --- a/courses/sync_external_courses/external_course_sync_api_client_test.py +++ b/courses/sync_external_courses/external_course_sync_api_client_test.py @@ -35,21 +35,21 @@ ), "get_queries_list", [], - "https://test-emeritus-api.io/api/queries?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", + "https://test-emeritus-api.io/api/queries?api_key=test_external_course_sync_api_key", ), ( "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse({"job": {"status": 1}}), "get_job_status", [12], - "https://test-emeritus-api.io/api/jobs/12?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", + "https://test-emeritus-api.io/api/jobs/12?api_key=test_external_course_sync_api_key", ), ( "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse({"query_result": {"data": {}}}), "get_query_result", [20], - "https://test-emeritus-api.io/api/query_results/20?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", + "https://test-emeritus-api.io/api/query_results/20?api_key=test_external_course_sync_api_key", ), ], ) @@ -62,7 +62,7 @@ def test_external_course_sync_api_client_get_requests( # noqa: PLR0913 args, expected_api_url, ): - settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" + settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_external_course_sync_api_key" settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-emeritus-api.io" settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = 60 @@ -89,7 +89,7 @@ def test_get_query_response(mocker, settings): end_date = now_in_utc() start_date = end_date - timedelta(days=1) - settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" + settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_external_course_sync_api_key" settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-emeritus-api.io" mock_post = mocker.patch( @@ -100,7 +100,7 @@ def test_get_query_response(mocker, settings): client = ExternalCourseSyncAPIClient() client.get_query_response(1, start_date, end_date) mock_post.assert_called_once_with( - "https://test-emeritus-api.io/api/queries/1/results?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", + "https://test-emeritus-api.io/api/queries/1/results?api_key=test_external_course_sync_api_key", data=json.dumps( { "parameters": { diff --git a/courses/tasks_test.py b/courses/tasks_test.py index 2e7a69dd3..177214cf7 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -26,14 +26,14 @@ def test_sync_courseruns_data(mocker): def test_task_sync_external_course_runs(mocker, settings): - """Test task_sync_external_course_runs skips platforms not in VENDOR_KEYMAPS""" + """Test task_sync_external_course_runs calls the right api functionality and skips platforms not in VENDOR_KEYMAPS""" settings.FEATURES["ENABLE_EXTERNAL_COURSE_SYNC"] = True mock_fetch_external_courses = mocker.patch("courses.tasks.fetch_external_courses") mock_update_external_course_runs = mocker.patch( "courses.tasks.update_external_course_runs" ) - log_mock = mocker.patch("courses.tasks.log") + mock_log = mocker.patch("courses.tasks.log") PlatformFactory.create(name="Emeritus", sync_daily=True) PlatformFactory.create(name="UnknownPlatform", sync_daily=True) @@ -43,7 +43,7 @@ def test_task_sync_external_course_runs(mocker, settings): mock_fetch_external_courses.assert_called_once() mock_update_external_course_runs.assert_called_once() - log_mock.exception.assert_called_once_with( + mock_log.exception.assert_called_once_with( "The platform '%s' does not have a sync API configured. Please disable the 'sync_daily' setting for this platform.", "UnknownPlatform", ) \ No newline at end of file From 5cfdfc5811904ecfb098d9c8293cf5c4c1a09481 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Wed, 4 Dec 2024 16:40:09 +0000 Subject: [PATCH 06/17] fix: some issues --- courses/tasks_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/courses/tasks_test.py b/courses/tasks_test.py index 177214cf7..bba63a28c 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -26,7 +26,7 @@ def test_sync_courseruns_data(mocker): def test_task_sync_external_course_runs(mocker, settings): - """Test task_sync_external_course_runs calls the right api functionality and skips platforms not in VENDOR_KEYMAPS""" + """Test task_sync_external_course_runs to call APIs for supported platforms and skip unsupported ones in VENDOR_KEYMAPS""" settings.FEATURES["ENABLE_EXTERNAL_COURSE_SYNC"] = True mock_fetch_external_courses = mocker.patch("courses.tasks.fetch_external_courses") From 1abce8f48328be509b9f8fb470a07f1eed0c7656 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Wed, 4 Dec 2024 16:44:26 +0000 Subject: [PATCH 07/17] fix: some issues --- .../external_course_sync_api_client_test.py | 12 ++++++------ .../sync_external_courses/test_data/batch_test.json | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/courses/sync_external_courses/external_course_sync_api_client_test.py b/courses/sync_external_courses/external_course_sync_api_client_test.py index d612b166a..1214754f9 100644 --- a/courses/sync_external_courses/external_course_sync_api_client_test.py +++ b/courses/sync_external_courses/external_course_sync_api_client_test.py @@ -35,21 +35,21 @@ ), "get_queries_list", [], - "https://test-emeritus-api.io/api/queries?api_key=test_external_course_sync_api_key", + "https://test-external-course-sync-api.io/api/queries?api_key=test_external_course_sync_api_key", ), ( "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse({"job": {"status": 1}}), "get_job_status", [12], - "https://test-emeritus-api.io/api/jobs/12?api_key=test_external_course_sync_api_key", + "https://test-external-course-sync-api.io/api/jobs/12?api_key=test_external_course_sync_api_key", ), ( "courses.sync_external_courses.external_course_sync_api_client.requests.get", MockResponse({"query_result": {"data": {}}}), "get_query_result", [20], - "https://test-emeritus-api.io/api/query_results/20?api_key=test_external_course_sync_api_key", + "https://test-external-course-sync-api.io/api/query_results/20?api_key=test_external_course_sync_api_key", ), ], ) @@ -63,7 +63,7 @@ def test_external_course_sync_api_client_get_requests( # noqa: PLR0913 expected_api_url, ): settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_external_course_sync_api_key" - settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-emeritus-api.io" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-external-course-sync-api.io" settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = 60 mock_get = mocker.patch(patch_request_path) @@ -90,7 +90,7 @@ def test_get_query_response(mocker, settings): start_date = end_date - timedelta(days=1) settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_external_course_sync_api_key" - settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-emeritus-api.io" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-external-course-sync-api.io" mock_post = mocker.patch( "courses.sync_external_courses.external_course_sync_api_client.requests.post" @@ -100,7 +100,7 @@ def test_get_query_response(mocker, settings): client = ExternalCourseSyncAPIClient() client.get_query_response(1, start_date, end_date) mock_post.assert_called_once_with( - "https://test-emeritus-api.io/api/queries/1/results?api_key=test_external_course_sync_api_key", + "https://test-external-course-sync-api.io/api/queries/1/results?api_key=test_external_course_sync_api_key", data=json.dumps( { "parameters": { diff --git a/courses/sync_external_courses/test_data/batch_test.json b/courses/sync_external_courses/test_data/batch_test.json index 90b8eed24..795b5b12f 100644 --- a/courses/sync_external_courses/test_data/batch_test.json +++ b/courses/sync_external_courses/test_data/batch_test.json @@ -114,8 +114,8 @@ "language": "English", "image_name": "test_emeritus_image.jpg", "ceu": "2.8", - "landing_page_url": "https://test-emeritus-api.io/Internet-of-things-iot-design-and-applications?utm_medium=EmWebsite&utm_campaign=direct_EmWebsite?utm_campaign=school_website&utm_medium=website&utm_source=MIT-web", - "Apply_now_url": "https://test-emeritus-api.io/?locale=en&program_sfid=01t2s000000OHA2AAO&source=applynowlp&utm_campaign=school&utm_medium=MITWebsite&utm_source=MIT-web", + "landing_page_url": "https://test-external-course-sync-api.io/Internet-of-things-iot-design-and-applications?utm_medium=EmWebsite&utm_campaign=direct_EmWebsite?utm_campaign=school_website&utm_medium=website&utm_source=MIT-web", + "Apply_now_url": "https://test-external-course-sync-api.io/?locale=en&program_sfid=01t2s000000OHA2AAO&source=applynowlp&utm_campaign=school&utm_medium=MITWebsite&utm_source=MIT-web", "description": "By 2030, McKinsey Digital research estimates that it could enable USD 5.5 trillion to USD 12.6 trillion in value globally, including the value captured by consumers and customers of Internet of Things ( IoT) products and services. This future dominated by IoT requires a paradigm shift in the way products and services are designed. MIT xPRO’s Internet of Things (IoT): Design and Applications program is designed to provide you with a strategic road map for creating user-centered, future-ready, differentiated products that drive business growth. Through activities, assignments, and industry examples, you will learn the fundamental principles of developing IoT-centric products and services.\r", "learning_outcomes": "This program will enable you to:\r\n● Understand how IoT mindsets are contributing to a global shift in product and business strategy\r\n● Discover how IoT impacts the design of both products and businesses\r\n● Identify hardware, software, and data technologies that support IoT adoption\r\n● Explore examples of successful IoT product and business strategies\r\n● Examine legal, ethical, privacy, and security concerns related to IoT", "program_for": "The program is ideal for:\r\n● Managers and functional leaders looking to learn the strategies for successfully leveraging IoT principles to drive business value\r\n● Product managers and designers who want to shift to an IoT-centric mindset to develop innovative products and services\r\n● Technology professionals keen on understanding the fundamental principles associated with the implementation of IoT and its wide array of applications\r\nNote: This is a nontechnical program for which there are no prerequisites.\r" @@ -183,8 +183,8 @@ "language": "English", "image_name": "test_emeritus_image.jpg", "ceu": "0.8", - "landing_page_url": "https://test-emeritus-api.io/professional-certificate-cybersecurity?utm_campaign=school_website&utm_medium=website&utm_source=MIT-web", - "Apply_now_url": "https://test-emeritus-api.io/?locale=en&program_sfid=01t2s000000ZdQKAA0&source=applynowlp&utm_campaign=school&utm_medium=MITWebsite&utm_source=MIT-web", + "landing_page_url": "https://test-external-course-sync-api.io/professional-certificate-cybersecurity?utm_campaign=school_website&utm_medium=website&utm_source=MIT-web", + "Apply_now_url": "https://test-external-course-sync-api.io/?locale=en&program_sfid=01t2s000000ZdQKAA0&source=applynowlp&utm_campaign=school&utm_medium=MITWebsite&utm_source=MIT-web", "description": "Cyberattacks are becoming more frequent, complex, and targeted, collectively costing organizations billions of dollars annually. It’s no wonder that cybersecurity is one of the fastest growing industries; by 2027, Forbes projects the value of the cybersecurity market to reach USD 403 billion. More and more companies and government agencies are seeking to hire cybersecurity professionals with the specialized technical skills needed to defend mission-critical computer systems, networks, and cloud applications against cyberattacks. If you’re keen to step into this high-growth field and advance your career, the MIT xPRO Professional Certificate in Cybersecurity is for you.\r", "learning_outcomes": "This program will enable you to:\r\n● Gain an overview of cybersecurity risk management, including its foundational concepts and relevant regulations\r\n● Explore the domains covering various aspects of cloud technology\r\n● Learn adversary tactics and techniques that are utilized as the foundational development of specific threat models and methodologies\r\n● Understand the guidelines for organizations to prepare themselves against cybersecurity attacks", "program_for": "The program is ideal for:\r\n● Early-career IT professionals, network engineers, and system administrators wanting to gain a comprehensive overview of cybersecurity and fast-track their career progression\r\n● IT project managers and engineers keen on gaining the ability to think critically about the threat landscape, including vulnerabilities in cybersecurity, and upgrading their resume for career advancement\r\n● Mid- or later-career professionals seeking a career change and looking to add critical cybersecurity knowledge and foundational lessons to their resume" From 8f4fbb6f8cb51292ab54a96c0a69a1ec86393709 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Thu, 5 Dec 2024 17:20:29 +0000 Subject: [PATCH 08/17] fix: issues after rebase --- courses/tasks.py | 9 +++++++-- courses/urls.py | 8 ++++---- courses/views/v1/__init__.py | 20 ++++++++++++++------ courses/views_test.py | 16 ++++++++-------- 4 files changed, 33 insertions(+), 20 deletions(-) diff --git a/courses/tasks.py b/courses/tasks.py index 9c7aed881..abbb460f7 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -130,5 +130,10 @@ def task_sync_external_course_runs(): platform.name, ) continue - emeritus_course_runs = fetch_external_courses(keymap) - update_external_course_runs(emeritus_course_runs, keymap) + try: + emeritus_course_runs = fetch_external_courses(keymap) + update_external_course_runs(emeritus_course_runs, keymap) + except Exception as e: # noqa: BLE001 + log.exception( + "Some error occurred. Details: %s", str(e), + ) diff --git a/courses/urls.py b/courses/urls.py index a0aae1d17..14dda4916 100644 --- a/courses/urls.py +++ b/courses/urls.py @@ -4,7 +4,7 @@ from rest_framework import routers from courses.views import v1 -from courses.views.v1 import EmeritusCourseListView +from courses.views.v1 import ExternalCourseListView router = routers.SimpleRouter() router.register(r"programs", v1.ProgramViewSet, basename="programs_api") @@ -31,8 +31,8 @@ r"^api/enrollments/", v1.UserEnrollmentsView.as_view(), name="user-enrollments" ), path( - "api/emeritus_courses/", - EmeritusCourseListView.as_view(), - name="emeritus_courses", + "api/external_courses//", + ExternalCourseListView.as_view(), + name="external_courses", ), ] diff --git a/courses/views/v1/__init__.py b/courses/views/v1/__init__.py index 951d197c2..5226dd904 100644 --- a/courses/views/v1/__init__.py +++ b/courses/views/v1/__init__.py @@ -27,7 +27,7 @@ ProgramEnrollmentSerializer, ProgramSerializer, ) -from courses.sync_external_courses.emeritus_api import fetch_emeritus_courses +from courses.sync_external_courses.external_course_sync_api import fetch_external_courses, VENDOR_KEYMAPS from ecommerce.models import Product @@ -205,19 +205,27 @@ def get_queryset(self): return CourseTopic.parent_topics_with_courses() -class EmeritusCourseListView(APIView): +class ExternalCourseListView(APIView): """ - ReadOnly View to list Emeritus courses. + ReadOnly View to list External courses. """ permission_classes = [IsAdminUser] def get(self, request, *args, **kwargs): # noqa: ARG002 """ - Get Emeritus courses list from the Emeritus API and return it. + Get External courses list from the External API and return it. """ - try: - data = fetch_emeritus_courses() + + vendor = kwargs.get("vendor") + keymap = VENDOR_KEYMAPS.get(vendor.lower()) + if not keymap: + return Response( + {"error": f"The vendor '{vendor}' is not supported."}, + status=status.HTTP_400_BAD_REQUEST, + ) + try: + data = fetch_external_courses(keymap) return Response(data, status=status.HTTP_200_OK) except Exception as e: # noqa: BLE001 return Response( diff --git a/courses/views_test.py b/courses/views_test.py index b252fbc01..cac12c5e3 100644 --- a/courses/views_test.py +++ b/courses/views_test.py @@ -616,9 +616,9 @@ def test_course_topics_api(client, django_assert_num_queries): @pytest.mark.parametrize("expected_status_code", [200, 500]) -def test_emeritus_course_list_view(admin_drf_client, mocker, expected_status_code): +def test_external_course_list_view(admin_drf_client, mocker, expected_status_code): """ - Test that the Emeritus API List calls fetch_emeritus_courses and returns its mocked response. + Test that the External API List calls fetch_external_courses and returns its mocked response. """ if expected_status_code == 200: with Path( @@ -626,12 +626,12 @@ def test_emeritus_course_list_view(admin_drf_client, mocker, expected_status_cod ).open() as test_data_file: mocked_response = json.load(test_data_file)["rows"] - patched_fetch_emeritus_courses = mocker.patch( - "courses.views.v1.fetch_emeritus_courses", return_value=mocked_response + patched_fetch_external_courses = mocker.patch( + "courses.views.v1.fetch_external_courses", return_value=mocked_response ) else: - patched_fetch_emeritus_courses = mocker.patch( - "courses.views.v1.fetch_emeritus_courses", + patched_fetch_external_courses = mocker.patch( + "courses.views.v1.fetch_external_courses", side_effect=Exception("Some error occurred."), ) mocked_response = { @@ -639,7 +639,7 @@ def test_emeritus_course_list_view(admin_drf_client, mocker, expected_status_cod "details": "Some error occurred.", } - response = admin_drf_client.get(reverse("emeritus_courses")) + response = admin_drf_client.get(reverse("external_courses", kwargs={"vendor": "emeritus"})) assert response.json() == mocked_response assert response.status_code == expected_status_code - patched_fetch_emeritus_courses.assert_called_once() + patched_fetch_external_courses.assert_called_once() From 3b38100f10d710b0e94a2b11e8e55ace576709dd Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 6 Dec 2024 07:36:41 +0000 Subject: [PATCH 09/17] fix: some issues --- courses/views/v1/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/courses/views/v1/__init__.py b/courses/views/v1/__init__.py index 5226dd904..e55e6807e 100644 --- a/courses/views/v1/__init__.py +++ b/courses/views/v1/__init__.py @@ -217,14 +217,14 @@ def get(self, request, *args, **kwargs): # noqa: ARG002 Get External courses list from the External API and return it. """ - vendor = kwargs.get("vendor") + vendor = kwargs.get("vendor").replace("_"," ") keymap = VENDOR_KEYMAPS.get(vendor.lower()) if not keymap: return Response( - {"error": f"The vendor '{vendor}' is not supported."}, + {"error": f"The vendor '{vendor}' is not supported. Supported vendors are {", ".join(VENDOR_KEYMAPS)}"}, status=status.HTTP_400_BAD_REQUEST, ) - try: + try: data = fetch_external_courses(keymap) return Response(data, status=status.HTTP_200_OK) except Exception as e: # noqa: BLE001 From 95482c8f66654221da4cc182b06b01419125c38a Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 6 Dec 2024 07:45:11 +0000 Subject: [PATCH 10/17] fix: some issues --- courses/tasks.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/courses/tasks.py b/courses/tasks.py index abbb460f7..684f0801f 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -116,7 +116,7 @@ def sync_courseruns_data(): @app.task def task_sync_external_course_runs(): - """Task to sync Emeritus course runs""" + """Task to sync external course runs""" if not settings.FEATURES.get("ENABLE_EXTERNAL_COURSE_SYNC", False): log.info("External Course sync is disabled.") return @@ -131,9 +131,7 @@ def task_sync_external_course_runs(): ) continue try: - emeritus_course_runs = fetch_external_courses(keymap) - update_external_course_runs(emeritus_course_runs, keymap) - except Exception as e: # noqa: BLE001 - log.exception( - "Some error occurred. Details: %s", str(e), - ) + external_course_runs = fetch_external_courses(keymap) + update_external_course_runs(external_course_runs, keymap) + except: # noqa: BLE001 + log.exception("Some error occurred") From 806eb9d846ff051557d1c00b2504e1422082e04c Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 6 Dec 2024 07:53:43 +0000 Subject: [PATCH 11/17] fix: pre-commit issues --- .../commands/sync_external_course_runs.py | 7 +--- .../migrations/0041_platform_sync_daily.py | 7 ++-- .../external_course_sync_api.py | 22 ++++++++++--- .../external_course_sync_api_client_test.py | 12 +++++-- .../external_course_sync_api_test.py | 32 ++++++++++++------- courses/tasks.py | 2 +- courses/tasks_test.py | 8 ++--- courses/views/v1/__init__.py | 13 +++++--- courses/views_test.py | 4 ++- 9 files changed, 69 insertions(+), 38 deletions(-) diff --git a/courses/management/commands/sync_external_course_runs.py b/courses/management/commands/sync_external_course_runs.py index c0abfc002..315b41d78 100644 --- a/courses/management/commands/sync_external_course_runs.py +++ b/courses/management/commands/sync_external_course_runs.py @@ -4,8 +4,6 @@ from courses.sync_external_courses.external_course_sync_api import ( VENDOR_KEYMAPS, - EmeritusKeyMap, - GlobalAlumniKeyMap, fetch_external_courses, update_external_course_runs, ) @@ -48,11 +46,8 @@ def handle(self, *args, **options): # noqa: ARG002 stats = update_external_course_runs(emeritus_course_runs, keymap) self.log_stats(stats) self.stdout.write( - self.style.SUCCESS( - f"External course sync successful for {vendor_name}." - ) + self.style.SUCCESS(f"External course sync successful for {vendor_name}.") ) - def log_stats(self, stats): """ diff --git a/courses/migrations/0041_platform_sync_daily.py b/courses/migrations/0041_platform_sync_daily.py index 000d5b2bf..536cca7fe 100644 --- a/courses/migrations/0041_platform_sync_daily.py +++ b/courses/migrations/0041_platform_sync_daily.py @@ -4,15 +4,14 @@ class Migration(migrations.Migration): - dependencies = [ - ('courses', '0040_alter_courserun_courseware_id'), + ("courses", "0040_alter_courserun_courseware_id"), ] operations = [ migrations.AddField( - model_name='platform', - name='sync_daily', + model_name="platform", + name="sync_daily", field=models.BooleanField(default=False), ), ] diff --git a/courses/sync_external_courses/external_course_sync_api.py b/courses/sync_external_courses/external_course_sync_api.py index 5f318e6ad..0b76b900a 100644 --- a/courses/sync_external_courses/external_course_sync_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -23,7 +23,9 @@ ) from courses.api import generate_course_readable_id from courses.models import Course, CourseRun, CourseTopic, Platform -from courses.sync_external_courses.external_course_sync_api_client import ExternalCourseSyncAPIClient +from courses.sync_external_courses.external_course_sync_api_client import ( + ExternalCourseSyncAPIClient, +) from ecommerce.models import Product, ProductVersion from mitxpro.utils import clean_url, now_in_utc, strip_datetime @@ -34,6 +36,7 @@ class BaseKeyMap: """ Base class for course sync keys with common attributes. """ + date_format = "%Y-%m-%d" required_fields = [ "course_title", @@ -66,6 +69,7 @@ class EmeritusKeyMap(BaseKeyMap): """ Emeritus course sync keys. """ + def __init__(self): super().__init__(platform_name="Emeritus", report_names=["Batch"]) @@ -74,14 +78,17 @@ class GlobalAlumniKeyMap(BaseKeyMap): """ Global Alumni course sync keys. """ + def __init__(self): super().__init__(platform_name="Global Alumni", report_names=["GA - Batch"]) + VENDOR_KEYMAPS = { "emeritus": EmeritusKeyMap(), "global alumni": GlobalAlumniKeyMap(), } + class ExternalCourseSyncAPIJobStatus(Enum): """ Status of an External Course API Job. @@ -150,7 +157,9 @@ def __init__(self, external_course_json, keymap): self.image_name = external_course_json.get("image_name", None) self.CEUs = str(external_course_json.get("ceu") or "") self.learning_outcomes_list = ( - parse_external_course_data_str(external_course_json.get("learning_outcomes")) + parse_external_course_data_str( + external_course_json.get("learning_outcomes") + ) if external_course_json.get("learning_outcomes") else [] ) @@ -222,7 +231,10 @@ def fetch_external_courses(keymap): ) while True: job_status = external_course_sync_api_client.get_job_status(job_id) - if job_status["job"]["status"] == ExternalCourseSyncAPIJobStatus.READY.value: + if ( + job_status["job"]["status"] + == ExternalCourseSyncAPIJobStatus.READY.value + ): # If true, the query_result is ready to be collected. log.info("Job complete... requesting results...") query_response = external_course_sync_api_client.get_query_result( @@ -508,7 +520,9 @@ def generate_external_course_run_courseware_id(course_run_tag, course_readable_i return f"{course_readable_id}+{course_run_tag}" -def create_or_update_external_course_page(course_index_page, course, external_course, keymap): +def create_or_update_external_course_page( + course_index_page, course, external_course, keymap +): """ Creates or updates external course page for External course. diff --git a/courses/sync_external_courses/external_course_sync_api_client_test.py b/courses/sync_external_courses/external_course_sync_api_client_test.py index 1214754f9..c369c6e53 100644 --- a/courses/sync_external_courses/external_course_sync_api_client_test.py +++ b/courses/sync_external_courses/external_course_sync_api_client_test.py @@ -7,7 +7,9 @@ import pytest -from courses.sync_external_courses.external_course_sync_api_client import ExternalCourseSyncAPIClient +from courses.sync_external_courses.external_course_sync_api_client import ( + ExternalCourseSyncAPIClient, +) from mitxpro.test_utils import MockResponse from mitxpro.utils import now_in_utc @@ -63,7 +65,9 @@ def test_external_course_sync_api_client_get_requests( # noqa: PLR0913 expected_api_url, ): settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_external_course_sync_api_key" - settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-external-course-sync-api.io" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = ( + "https://test-external-course-sync-api.io" + ) settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = 60 mock_get = mocker.patch(patch_request_path) @@ -90,7 +94,9 @@ def test_get_query_response(mocker, settings): start_date = end_date - timedelta(days=1) settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_external_course_sync_api_key" - settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test-external-course-sync-api.io" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = ( + "https://test-external-course-sync-api.io" + ) mock_post = mocker.patch( "courses.sync_external_courses.external_course_sync_api_client.requests.post" diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index 53a9801d1..6959193da 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -21,8 +21,8 @@ from courses.factories import CourseFactory, CourseRunFactory, PlatformFactory from courses.models import Course from courses.sync_external_courses.external_course_sync_api import ( - ExternalCourse, EmeritusKeyMap, + ExternalCourse, create_learning_outcomes_page, create_or_update_certificate_page, create_or_update_external_course_page, @@ -30,8 +30,8 @@ create_or_update_product_and_product_version, create_who_should_enroll_in_page, fetch_external_courses, - generate_external_course_run_tag, generate_external_course_run_courseware_id, + generate_external_course_run_tag, parse_external_course_data_str, save_page_revision, update_external_course_runs, @@ -206,7 +206,10 @@ def test_create_or_update_external_course_page( # noqa: PLR0913 external_course_page, course_page_created, course_page_updated = ( create_or_update_external_course_page( - course_index_page, course, ExternalCourse(external_course_data, keymap=EmeritusKeyMap()), keymap=EmeritusKeyMap() + course_index_page, + course, + ExternalCourse(external_course_data, keymap=EmeritusKeyMap()), + keymap=EmeritusKeyMap(), ) ) external_course_page = external_course_page.revisions.last().as_object() @@ -287,7 +290,8 @@ def test_create_or_update_certificate_page( certificate_page.unpublish() certificate_page, is_created, is_updated = create_or_update_certificate_page( - external_course_page, ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + external_course_page, + ExternalCourse(external_course_data, keymap=EmeritusKeyMap()), ) certificate_page = certificate_page.revisions.last().as_object() assert certificate_page.CEUs == external_course_data["ceu"] @@ -317,7 +321,9 @@ def test_create_who_should_enroll_in_page(): "looking to add critical cybersecurity knowledge and foundational lessons to their resume" ) create_who_should_enroll_in_page( - course_page, parse_external_course_data_str(who_should_enroll_str), keymap=EmeritusKeyMap() + course_page, + parse_external_course_data_str(who_should_enroll_str), + keymap=EmeritusKeyMap(), ) assert parse_external_course_data_str(who_should_enroll_str) == [ item.value.source for item in course_page.who_should_enroll.content @@ -340,7 +346,9 @@ def test_create_learning_outcomes_page(): "organizations to prepare themselves against cybersecurity attacks" ) create_learning_outcomes_page( - course_page, parse_external_course_data_str(learning_outcomes_str), keymap=EmeritusKeyMap() + course_page, + parse_external_course_data_str(learning_outcomes_str), + keymap=EmeritusKeyMap(), ) assert parse_external_course_data_str(learning_outcomes_str) == [ item.value for item in course_page.outcomes.outcome_items @@ -568,7 +576,9 @@ def test_fetch_external_courses_success(settings, mocker): 5. If job status is 1 or 2, it is in progress. Wait for 2 seconds and make a get request for Job status. 6. If job status is 3, the results are ready, make a get request to collect the results and return the data. """ - settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test_external_course_sync_api.io" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = ( + "https://test_external_course_sync_api.io" + ) settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" settings.EXTERNAL_COURSE_SYNC_API_REQUEST_TIMEOUT = 60 @@ -619,7 +629,9 @@ def test_fetch_external_courses_error(settings, mocker, caplog): """ Tests that `fetch_external_courses` specific calls to the External Course Sync API and Fails for Job status 3 and 4. """ - settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = "https://test_external_course_sync_api.com" + settings.EXTERNAL_COURSE_SYNC_API_BASE_URL = ( + "https://test_external_course_sync_api.com" + ) settings.EXTERNAL_COURSE_SYNC_API_KEY = "test_EXTERNAL_COURSE_SYNC_API_KEY" mock_get = mocker.patch( "courses.sync_external_courses.external_course_sync_api_client.requests.get" @@ -729,9 +741,7 @@ def test_save_page_revision(is_draft_page, has_unpublished_changes): external_course_page.save_revision() latest_revision = external_course_page.get_latest_revision_as_object() - latest_revision.external_marketing_url = ( - "https://test-external-course-sync-api.io/Internet-of-things-iot-design-and-applications" - ) + latest_revision.external_marketing_url = "https://test-external-course-sync-api.io/Internet-of-things-iot-design-and-applications" save_page_revision(external_course_page, latest_revision) assert external_course_page.live == (not is_draft_page) diff --git a/courses/tasks.py b/courses/tasks.py index 684f0801f..85c873c34 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -133,5 +133,5 @@ def task_sync_external_course_runs(): try: external_course_runs = fetch_external_courses(keymap) update_external_course_runs(external_course_runs, keymap) - except: # noqa: BLE001 + except Exception: log.exception("Some error occurred") diff --git a/courses/tasks_test.py b/courses/tasks_test.py index bba63a28c..63530b785 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -28,7 +28,7 @@ def test_sync_courseruns_data(mocker): def test_task_sync_external_course_runs(mocker, settings): """Test task_sync_external_course_runs to call APIs for supported platforms and skip unsupported ones in VENDOR_KEYMAPS""" settings.FEATURES["ENABLE_EXTERNAL_COURSE_SYNC"] = True - + mock_fetch_external_courses = mocker.patch("courses.tasks.fetch_external_courses") mock_update_external_course_runs = mocker.patch( "courses.tasks.update_external_course_runs" @@ -37,13 +37,13 @@ def test_task_sync_external_course_runs(mocker, settings): PlatformFactory.create(name="Emeritus", sync_daily=True) PlatformFactory.create(name="UnknownPlatform", sync_daily=True) - + task_sync_external_course_runs.delay() - + mock_fetch_external_courses.assert_called_once() mock_update_external_course_runs.assert_called_once() mock_log.exception.assert_called_once_with( "The platform '%s' does not have a sync API configured. Please disable the 'sync_daily' setting for this platform.", "UnknownPlatform", - ) \ No newline at end of file + ) diff --git a/courses/views/v1/__init__.py b/courses/views/v1/__init__.py index e55e6807e..e093df375 100644 --- a/courses/views/v1/__init__.py +++ b/courses/views/v1/__init__.py @@ -27,7 +27,10 @@ ProgramEnrollmentSerializer, ProgramSerializer, ) -from courses.sync_external_courses.external_course_sync_api import fetch_external_courses, VENDOR_KEYMAPS +from courses.sync_external_courses.external_course_sync_api import ( + VENDOR_KEYMAPS, + fetch_external_courses, +) from ecommerce.models import Product @@ -216,12 +219,14 @@ def get(self, request, *args, **kwargs): # noqa: ARG002 """ Get External courses list from the External API and return it. """ - - vendor = kwargs.get("vendor").replace("_"," ") + + vendor = kwargs.get("vendor").replace("_", " ") keymap = VENDOR_KEYMAPS.get(vendor.lower()) if not keymap: return Response( - {"error": f"The vendor '{vendor}' is not supported. Supported vendors are {", ".join(VENDOR_KEYMAPS)}"}, + { + "error": f"The vendor '{vendor}' is not supported. Supported vendors are {", ".join(VENDOR_KEYMAPS)}" + }, status=status.HTTP_400_BAD_REQUEST, ) try: diff --git a/courses/views_test.py b/courses/views_test.py index cac12c5e3..d44b7d05c 100644 --- a/courses/views_test.py +++ b/courses/views_test.py @@ -639,7 +639,9 @@ def test_external_course_list_view(admin_drf_client, mocker, expected_status_cod "details": "Some error occurred.", } - response = admin_drf_client.get(reverse("external_courses", kwargs={"vendor": "emeritus"})) + response = admin_drf_client.get( + reverse("external_courses", kwargs={"vendor": "emeritus"}) + ) assert response.json() == mocked_response assert response.status_code == expected_status_code patched_fetch_external_courses.assert_called_once() From 9933942c7da97374e9b255bd7f7668f8c8bb2c46 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 6 Dec 2024 13:56:25 +0000 Subject: [PATCH 12/17] fix: issues --- .../commands/sync_external_course_runs.py | 8 +++--- .../migrations/0041_platform_sync_daily.py | 11 ++++---- courses/models.py | 2 +- courses/models_test.py | 5 ++-- .../external_course_sync_api.py | 28 +++++++++++-------- .../external_course_sync_api_client.py | 4 +-- .../external_course_sync_api_test.py | 5 ++-- courses/tasks.py | 8 +++--- courses/tasks_test.py | 6 ++-- courses/views/v1/__init__.py | 8 +++--- courses/views_test.py | 3 +- 11 files changed, 50 insertions(+), 38 deletions(-) diff --git a/courses/management/commands/sync_external_course_runs.py b/courses/management/commands/sync_external_course_runs.py index 315b41d78..95e958982 100644 --- a/courses/management/commands/sync_external_course_runs.py +++ b/courses/management/commands/sync_external_course_runs.py @@ -3,7 +3,7 @@ from django.core.management.base import BaseCommand from courses.sync_external_courses.external_course_sync_api import ( - VENDOR_KEYMAPS, + EXTERNAL_COURSE_VENDOR_KEYMAPS, fetch_external_courses, update_external_course_runs, ) @@ -36,14 +36,14 @@ def handle(self, *args, **options): # noqa: ARG002 return vendor_name = options["vendor_name"] - keymap = VENDOR_KEYMAPS.get(vendor_name.lower()) + keymap = EXTERNAL_COURSE_VENDOR_KEYMAPS.get(vendor_name.lower()) if not keymap: self.stdout.write(self.style.ERROR(f"Unknown vendor name {vendor_name}.")) return self.stdout.write(f"Starting course sync for {vendor_name}.") - emeritus_course_runs = fetch_external_courses(keymap) - stats = update_external_course_runs(emeritus_course_runs, keymap) + external_course_runs = fetch_external_courses(keymap()) + stats = update_external_course_runs(external_course_runs, keymap()) self.log_stats(stats) self.stdout.write( self.style.SUCCESS(f"External course sync successful for {vendor_name}.") diff --git a/courses/migrations/0041_platform_sync_daily.py b/courses/migrations/0041_platform_sync_daily.py index 536cca7fe..e03d19a2a 100644 --- a/courses/migrations/0041_platform_sync_daily.py +++ b/courses/migrations/0041_platform_sync_daily.py @@ -1,17 +1,18 @@ -# Generated by Django 4.2.16 on 2024-12-04 07:39 +# Generated by Django 4.2.16 on 2024-12-06 13:47 from django.db import migrations, models class Migration(migrations.Migration): + dependencies = [ - ("courses", "0040_alter_courserun_courseware_id"), + ('courses', '0040_alter_courserun_courseware_id'), ] operations = [ migrations.AddField( - model_name="platform", - name="sync_daily", - field=models.BooleanField(default=False), + model_name='platform', + name='sync_daily', + field=models.BooleanField(default=False, help_text='Select this option to enable daily syncing for external course platforms.'), ), ] diff --git a/courses/models.py b/courses/models.py index 28c1e3889..8b8d10e99 100644 --- a/courses/models.py +++ b/courses/models.py @@ -214,7 +214,7 @@ class Platform(TimestampedModel, ValidateOnSaveMixin): """ name = models.CharField(max_length=255, unique=True) - sync_daily = models.BooleanField(default=False) + sync_daily = models.BooleanField(default=False, help_text="Select this option to enable daily syncing for external course platforms.") def __str__(self): return self.name diff --git a/courses/models_test.py b/courses/models_test.py index 647524b44..aaf14ddd3 100644 --- a/courses/models_test.py +++ b/courses/models_test.py @@ -27,6 +27,7 @@ ) from courses.models import CourseRunEnrollment, limit_to_certificate_pages from ecommerce.factories import ProductFactory, ProductVersionFactory +from sync_external_courses.external_course_sync_api import EMERITUS_PLATFORM_NAME from mitxpro.test_utils import format_as_iso8601 from mitxpro.utils import now_in_utc from users.factories import UserFactory @@ -812,7 +813,7 @@ def test_platform_name_is_unique(): """ Tests that case-insensitive platform name is unique. """ - PlatformFactory.create(name="Emeritus") + PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) with pytest.raises(ValidationError): - PlatformFactory.create(name="emeritus") + PlatformFactory.create(name=EMERITUS_PLATFORM_NAME.lower()) diff --git a/courses/sync_external_courses/external_course_sync_api.py b/courses/sync_external_courses/external_course_sync_api.py index 0b76b900a..06d5e4cc1 100644 --- a/courses/sync_external_courses/external_course_sync_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -31,8 +31,10 @@ log = logging.getLogger(__name__) +EMERITUS_PLATFORM_NAME = "Emeritus" +GLOBAL_ALUMNI_PLATFORM_NAME = "Global Alumni" -class BaseKeyMap: +class ExternalCourseVendorBaseKeyMap: """ Base class for course sync keys with common attributes. """ @@ -65,27 +67,27 @@ def learning_outcomes_page_subhead(self): ) -class EmeritusKeyMap(BaseKeyMap): +class EmeritusKeyMap(ExternalCourseVendorBaseKeyMap): """ Emeritus course sync keys. """ def __init__(self): - super().__init__(platform_name="Emeritus", report_names=["Batch"]) + super().__init__(platform_name=EMERITUS_PLATFORM_NAME, report_names=["Batch"]) -class GlobalAlumniKeyMap(BaseKeyMap): +class GlobalAlumniKeyMap(ExternalCourseVendorBaseKeyMap): """ Global Alumni course sync keys. """ def __init__(self): - super().__init__(platform_name="Global Alumni", report_names=["GA - Batch"]) + super().__init__(platform_name=GLOBAL_ALUMNI_PLATFORM_NAME, report_names=["GA - Batch"]) -VENDOR_KEYMAPS = { - "emeritus": EmeritusKeyMap(), - "global alumni": GlobalAlumniKeyMap(), +EXTERNAL_COURSE_VENDOR_KEYMAPS = { + EMERITUS_PLATFORM_NAME.lower(): EmeritusKeyMap, + GLOBAL_ALUMNI_PLATFORM_NAME.lower(): GlobalAlumniKeyMap, } @@ -111,7 +113,7 @@ def __init__(self, external_course_json, keymap): self.course_title = program_name.strip() if program_name else None self.course_code = external_course_json.get("course_code") - # External course code format is `MO-`, where course tag can contain `.`, + # External course code format is `-`, where course tag can contain `.`, # we will replace `.` with `_` to follow the internal readable id format. self.course_readable_id = generate_course_readable_id( self.course_code.split("-")[1].replace(".", "_") @@ -172,6 +174,8 @@ def __init__(self, external_course_json, keymap): def validate_required_fields(self, keymap): """ Validates the course data. + Args: + keymap(ExternalCourseVendorBaseKeyMap): An ExternalCourseVendorBaseKeyMap object """ for field in keymap.required_fields: if not getattr(self, field, None): @@ -200,6 +204,8 @@ def validate_end_date(self): def fetch_external_courses(keymap): """ Fetches external courses data. + Args: + keymap(ExternalCourseVendorBaseKeyMap): An ExternalCourseVendorBaseKeyMap object Makes a request to get the list of available queries and then queries the required reports. """ @@ -266,7 +272,7 @@ def update_external_course_runs(external_courses, keymap): # noqa: C901, PLR091 Args: external_courses(list[dict]): A list of External Courses as a dict. - + keymap(ExternalCourseVendorBaseKeyMap): An ExternalCourseVendorBaseKeyMap object Returns: dict: Stats of all the objects created/updated. """ @@ -494,7 +500,7 @@ def generate_external_course_run_tag(course_run_code): """ Returns the course run tag generated using the External Course run code. - External course run codes follow a pattern `MO--`. This method returns the run tag. + External course run codes follow a pattern `--`. This method returns the run tag. Args: course_run_code(str): External course code diff --git a/courses/sync_external_courses/external_course_sync_api_client.py b/courses/sync_external_courses/external_course_sync_api_client.py index 8a4c71b6c..82d19295c 100644 --- a/courses/sync_external_courses/external_course_sync_api_client.py +++ b/courses/sync_external_courses/external_course_sync_api_client.py @@ -1,5 +1,5 @@ """ -API client for Emeritus +External course sync API client """ import json @@ -10,7 +10,7 @@ class ExternalCourseSyncAPIClient: """ - API client for Emeritus + External course sync API client """ def __init__(self): diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index 6959193da..df944742f 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -21,6 +21,7 @@ from courses.factories import CourseFactory, CourseRunFactory, PlatformFactory from courses.models import Course from courses.sync_external_courses.external_course_sync_api import ( + EMERITUS_PLATFORM_NAME, EmeritusKeyMap, ExternalCourse, create_learning_outcomes_page, @@ -459,7 +460,7 @@ def test_update_external_course_runs( # noqa: PLR0915 ).open() as test_data_file: external_course_runs = json.load(test_data_file)["rows"] - platform = PlatformFactory.create(name="Emeritus") + platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) if create_existing_data: for run in random.sample(external_course_runs, len(external_course_runs) // 2): @@ -688,7 +689,7 @@ def test_create_or_update_product_and_product_version( # noqa: PLR0913 """ external_course_data["list_price"] = new_price external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) - platform = PlatformFactory.create(name="Emeritus") + platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) course = CourseFactory.create( external_course_id=external_course.course_code, platform=platform, diff --git a/courses/tasks.py b/courses/tasks.py index 85c873c34..bd482709c 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -11,7 +11,7 @@ from courses.models import CourseRun, CourseRunCertificate, Platform from courses.sync_external_courses.external_course_sync_api import ( - VENDOR_KEYMAPS, + EXTERNAL_COURSE_VENDOR_KEYMAPS, fetch_external_courses, update_external_course_runs, ) @@ -123,7 +123,7 @@ def task_sync_external_course_runs(): platforms = Platform.objects.filter(sync_daily=True) for platform in platforms: - keymap = VENDOR_KEYMAPS.get(platform.name.lower()) + keymap = EXTERNAL_COURSE_VENDOR_KEYMAPS.get(platform.name.lower()) if not keymap: log.exception( "The platform '%s' does not have a sync API configured. Please disable the 'sync_daily' setting for this platform.", @@ -131,7 +131,7 @@ def task_sync_external_course_runs(): ) continue try: - external_course_runs = fetch_external_courses(keymap) - update_external_course_runs(external_course_runs, keymap) + external_course_runs = fetch_external_courses(keymap()) + update_external_course_runs(external_course_runs, keymap()) except Exception: log.exception("Some error occurred") diff --git a/courses/tasks_test.py b/courses/tasks_test.py index 63530b785..dd8727b6a 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -7,6 +7,8 @@ from courses.factories import CourseRunFactory, PlatformFactory from courses.tasks import sync_courseruns_data, task_sync_external_course_runs +from sync_external_courses.external_course_sync_api import EMERITUS_PLATFORM_NAME + pytestmark = [pytest.mark.django_db] @@ -26,7 +28,7 @@ def test_sync_courseruns_data(mocker): def test_task_sync_external_course_runs(mocker, settings): - """Test task_sync_external_course_runs to call APIs for supported platforms and skip unsupported ones in VENDOR_KEYMAPS""" + """Test task_sync_external_course_runs to call APIs for supported platforms and skip unsupported ones in EXTERNAL_COURSE_VENDOR_KEYMAPS""" settings.FEATURES["ENABLE_EXTERNAL_COURSE_SYNC"] = True mock_fetch_external_courses = mocker.patch("courses.tasks.fetch_external_courses") @@ -35,7 +37,7 @@ def test_task_sync_external_course_runs(mocker, settings): ) mock_log = mocker.patch("courses.tasks.log") - PlatformFactory.create(name="Emeritus", sync_daily=True) + PlatformFactory.create(name=EMERITUS_PLATFORM_NAME, sync_daily=True) PlatformFactory.create(name="UnknownPlatform", sync_daily=True) task_sync_external_course_runs.delay() diff --git a/courses/views/v1/__init__.py b/courses/views/v1/__init__.py index e093df375..c4777cde3 100644 --- a/courses/views/v1/__init__.py +++ b/courses/views/v1/__init__.py @@ -28,7 +28,7 @@ ProgramSerializer, ) from courses.sync_external_courses.external_course_sync_api import ( - VENDOR_KEYMAPS, + EXTERNAL_COURSE_VENDOR_KEYMAPS, fetch_external_courses, ) from ecommerce.models import Product @@ -221,16 +221,16 @@ def get(self, request, *args, **kwargs): # noqa: ARG002 """ vendor = kwargs.get("vendor").replace("_", " ") - keymap = VENDOR_KEYMAPS.get(vendor.lower()) + keymap = EXTERNAL_COURSE_VENDOR_KEYMAPS.get(vendor.lower()) if not keymap: return Response( { - "error": f"The vendor '{vendor}' is not supported. Supported vendors are {", ".join(VENDOR_KEYMAPS)}" + "error": f"The vendor '{vendor}' is not supported. Supported vendors are {", ".join(EXTERNAL_COURSE_VENDOR_KEYMAPS)}" }, status=status.HTTP_400_BAD_REQUEST, ) try: - data = fetch_external_courses(keymap) + data = fetch_external_courses(keymap()) return Response(data, status=status.HTTP_200_OK) except Exception as e: # noqa: BLE001 return Response( diff --git a/courses/views_test.py b/courses/views_test.py index d44b7d05c..e5c94365f 100644 --- a/courses/views_test.py +++ b/courses/views_test.py @@ -37,6 +37,7 @@ ProgramCertificateSerializer, ProgramSerializer, ) +from courses.sync_external_courses.external_course_sync_api import EMERITUS_PLATFORM_NAME from ecommerce.factories import ProductFactory, ProductVersionFactory from mitxpro.test_utils import assert_drf_json_equal from mitxpro.utils import now_in_utc @@ -640,7 +641,7 @@ def test_external_course_list_view(admin_drf_client, mocker, expected_status_cod } response = admin_drf_client.get( - reverse("external_courses", kwargs={"vendor": "emeritus"}) + reverse("external_courses", kwargs={"vendor": EMERITUS_PLATFORM_NAME}) ) assert response.json() == mocked_response assert response.status_code == expected_status_code From 7a546c737da1fa3d0caf00fc383263479062e957 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 6 Dec 2024 14:59:05 +0000 Subject: [PATCH 13/17] fix: tests --- courses/migrations/0041_platform_sync_daily.py | 12 +++++++----- courses/models.py | 5 ++++- courses/models_test.py | 4 +++- .../external_course_sync_api.py | 5 ++++- courses/tasks_test.py | 5 +++-- courses/views_test.py | 4 +++- 6 files changed, 24 insertions(+), 11 deletions(-) diff --git a/courses/migrations/0041_platform_sync_daily.py b/courses/migrations/0041_platform_sync_daily.py index e03d19a2a..28aac4ec0 100644 --- a/courses/migrations/0041_platform_sync_daily.py +++ b/courses/migrations/0041_platform_sync_daily.py @@ -4,15 +4,17 @@ class Migration(migrations.Migration): - dependencies = [ - ('courses', '0040_alter_courserun_courseware_id'), + ("courses", "0040_alter_courserun_courseware_id"), ] operations = [ migrations.AddField( - model_name='platform', - name='sync_daily', - field=models.BooleanField(default=False, help_text='Select this option to enable daily syncing for external course platforms.'), + model_name="platform", + name="sync_daily", + field=models.BooleanField( + default=False, + help_text="Select this option to enable daily syncing for external course platforms.", + ), ), ] diff --git a/courses/models.py b/courses/models.py index 8b8d10e99..f226f8021 100644 --- a/courses/models.py +++ b/courses/models.py @@ -214,7 +214,10 @@ class Platform(TimestampedModel, ValidateOnSaveMixin): """ name = models.CharField(max_length=255, unique=True) - sync_daily = models.BooleanField(default=False, help_text="Select this option to enable daily syncing for external course platforms.") + sync_daily = models.BooleanField( + default=False, + help_text="Select this option to enable daily syncing for external course platforms.", + ) def __str__(self): return self.name diff --git a/courses/models_test.py b/courses/models_test.py index aaf14ddd3..b932385d1 100644 --- a/courses/models_test.py +++ b/courses/models_test.py @@ -26,8 +26,10 @@ ProgramRunFactory, ) from courses.models import CourseRunEnrollment, limit_to_certificate_pages +from courses.sync_external_courses.external_course_sync_api import ( + EMERITUS_PLATFORM_NAME, +) from ecommerce.factories import ProductFactory, ProductVersionFactory -from sync_external_courses.external_course_sync_api import EMERITUS_PLATFORM_NAME from mitxpro.test_utils import format_as_iso8601 from mitxpro.utils import now_in_utc from users.factories import UserFactory diff --git a/courses/sync_external_courses/external_course_sync_api.py b/courses/sync_external_courses/external_course_sync_api.py index 06d5e4cc1..4d58789e9 100644 --- a/courses/sync_external_courses/external_course_sync_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -34,6 +34,7 @@ EMERITUS_PLATFORM_NAME = "Emeritus" GLOBAL_ALUMNI_PLATFORM_NAME = "Global Alumni" + class ExternalCourseVendorBaseKeyMap: """ Base class for course sync keys with common attributes. @@ -82,7 +83,9 @@ class GlobalAlumniKeyMap(ExternalCourseVendorBaseKeyMap): """ def __init__(self): - super().__init__(platform_name=GLOBAL_ALUMNI_PLATFORM_NAME, report_names=["GA - Batch"]) + super().__init__( + platform_name=GLOBAL_ALUMNI_PLATFORM_NAME, report_names=["GA - Batch"] + ) EXTERNAL_COURSE_VENDOR_KEYMAPS = { diff --git a/courses/tasks_test.py b/courses/tasks_test.py index dd8727b6a..970b05268 100644 --- a/courses/tasks_test.py +++ b/courses/tasks_test.py @@ -5,10 +5,11 @@ import pytest from courses.factories import CourseRunFactory, PlatformFactory +from courses.sync_external_courses.external_course_sync_api import ( + EMERITUS_PLATFORM_NAME, +) from courses.tasks import sync_courseruns_data, task_sync_external_course_runs -from sync_external_courses.external_course_sync_api import EMERITUS_PLATFORM_NAME - pytestmark = [pytest.mark.django_db] diff --git a/courses/views_test.py b/courses/views_test.py index e5c94365f..477c4e928 100644 --- a/courses/views_test.py +++ b/courses/views_test.py @@ -37,7 +37,9 @@ ProgramCertificateSerializer, ProgramSerializer, ) -from courses.sync_external_courses.external_course_sync_api import EMERITUS_PLATFORM_NAME +from courses.sync_external_courses.external_course_sync_api import ( + EMERITUS_PLATFORM_NAME, +) from ecommerce.factories import ProductFactory, ProductVersionFactory from mitxpro.test_utils import assert_drf_json_equal from mitxpro.utils import now_in_utc From 1dd359cbfea982975ba6a8a52ea585557b78fb7d Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Fri, 6 Dec 2024 20:50:19 +0500 Subject: [PATCH 14/17] test: add GA tests --- .../external_course_sync_api_test.py | 123 ++++++++++++++++-- 1 file changed, 109 insertions(+), 14 deletions(-) diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index df944742f..9d1b93a8b 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -22,8 +22,10 @@ from courses.models import Course from courses.sync_external_courses.external_course_sync_api import ( EMERITUS_PLATFORM_NAME, + GLOBAL_ALUMNI_PLATFORM_NAME, EmeritusKeyMap, ExternalCourse, + GlobalAlumniKeyMap, create_learning_outcomes_page, create_or_update_certificate_page, create_or_update_external_course_page, @@ -43,7 +45,7 @@ @pytest.fixture -def external_course_data(): +def external_course_data(request): """ External Course data with Future dates. """ @@ -52,9 +54,16 @@ def external_course_data(): ).open() as test_data_file: external_course_data = json.load(test_data_file)["rows"][0] + params = request.param + platform = params.get("platform", EMERITUS_PLATFORM_NAME) + if platform == EMERITUS_PLATFORM_NAME: + external_course_data["course_run_code"] = "MO-DBIP.ELE-99-09#1" + elif platform == GLOBAL_ALUMNI_PLATFORM_NAME: + external_course_data["course_run_code"] = "MXP-DBIP.ELE-99-09#1" + external_course_data.pop("ceu", None) + external_course_data["start_date"] = "2099-09-30" external_course_data["end_date"] = "2099-11-30" - external_course_data["course_run_code"] = "MO-DBIP.ELE-99-09#1" return external_course_data @@ -100,7 +109,9 @@ def external_course_data_with_non_usd_price(external_course_data): """ external_course_json = external_course_data.copy() external_course_json["list_currency"] = "INR" - external_course_json["course_run_code"] = "MO-INRC-98-10#1" + external_course_json["course_run_code"] = ( + f"{external_course_data["course_run_code"].split("-")[0]}-INRC-98-10#1" + ) return external_course_json @@ -108,10 +119,15 @@ def external_course_data_with_non_usd_price(external_course_data): ("external_course_run_code", "expected_course_run_tag"), [ ("MO-EOB-18-01#1", "18-01-1"), + ("MXP-EOB-18-01#1", "18-01-1"), ("MO-EOB-08-01#1", "08-01-1"), + ("MXP-EOB-08-01#1", "08-01-1"), ("MO-EOB-08-12#1", "08-12-1"), + ("MXP-EOB-08-12#1", "08-12-1"), ("MO-EOB-18-01#12", "18-01-12"), + ("MXP-EOB-18-01#12", "18-01-12"), ("MO-EOB-18-01#212", "18-01-212"), + ("MXP-EOB-18-01#212", "18-01-212"), ], ) def test_generate_external_course_run_tag( @@ -148,6 +164,11 @@ def test_generate_external_course_run_courseware_id( ) +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize( ( "create_course_page", @@ -251,6 +272,9 @@ def test_create_or_update_external_course_page( # noqa: PLR0913 ) +@pytest.mark.parametrize( + "external_course_data", [{"platform": EMERITUS_PLATFORM_NAME}], indirect=True +) @pytest.mark.parametrize( ("existing_cert_page", "publish_certificate", "is_live_and_draft"), [ @@ -306,8 +330,11 @@ def test_create_or_update_certificate_page( assert certificate_page.live +@pytest.mark.parametrize( + "external_course_vendor_keymap", [EmeritusKeyMap, GlobalAlumniKeyMap] +) @pytest.mark.django_db -def test_create_who_should_enroll_in_page(): +def test_create_who_should_enroll_in_page(external_course_vendor_keymap): """ Tests that `create_who_should_enroll_in_page` creates the `WhoShouldEnrollPage`. """ @@ -324,7 +351,7 @@ def test_create_who_should_enroll_in_page(): create_who_should_enroll_in_page( course_page, parse_external_course_data_str(who_should_enroll_str), - keymap=EmeritusKeyMap(), + keymap=external_course_vendor_keymap(), ) assert parse_external_course_data_str(who_should_enroll_str) == [ item.value.source for item in course_page.who_should_enroll.content @@ -332,8 +359,11 @@ def test_create_who_should_enroll_in_page(): assert course_page.who_should_enroll is not None +@pytest.mark.parametrize( + "external_course_vendor_keymap", [EmeritusKeyMap, GlobalAlumniKeyMap] +) @pytest.mark.django_db -def test_create_learning_outcomes_page(): +def test_create_learning_outcomes_page(external_course_vendor_keymap): """ Tests that `create_learning_outcomes_page` creates the `LearningOutcomesPage`. """ @@ -349,7 +379,7 @@ def test_create_learning_outcomes_page(): create_learning_outcomes_page( course_page, parse_external_course_data_str(learning_outcomes_str), - keymap=EmeritusKeyMap(), + keymap=external_course_vendor_keymap(), ) assert parse_external_course_data_str(learning_outcomes_str) == [ item.value for item in course_page.outcomes.outcome_items @@ -379,6 +409,11 @@ def test_parse_external_course_data_str(): ] +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize( ("create_existing_course_run", "empty_dates"), [ @@ -443,9 +478,15 @@ def test_create_or_update_external_course_run( assert getattr(course_runs[0], attr_name) == expected_value +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize("create_existing_data", [True, False]) @pytest.mark.django_db -def test_update_external_course_runs( # noqa: PLR0915 +def test_update_external_course_runs( # noqa: PLR0915, PLR0913 + external_course_data, create_existing_data, external_expired_course_data, external_course_with_bad_data, @@ -565,7 +606,12 @@ def test_update_external_course_runs( # noqa: PLR0915 assert certificate_page.CEUs == external_course_run["ceu"] -def test_fetch_external_courses_success(settings, mocker): +@pytest.mark.parametrize( + "external_course_vendor_keymap", [EmeritusKeyMap, GlobalAlumniKeyMap] +) +def test_fetch_external_courses_success( + settings, mocker, external_course_vendor_keymap +): """ Tests that `fetch_external_courses` makes the required calls to the `Emeritus` API. Tests the success scenario. @@ -595,9 +641,10 @@ def test_fetch_external_courses_success(settings, mocker): ).open() as test_data_file: external_course_runs = json.load(test_data_file) + keymap = external_course_vendor_keymap() batch_query = { "id": 77, - "name": "Batch", + "name": keymap.report_names[0], } mock_get.side_effect = [ MockResponse({"results": [batch_query]}), @@ -608,7 +655,7 @@ def test_fetch_external_courses_success(settings, mocker): ] mock_post.side_effect = [MockResponse({"job": {"id": 1}})] - actual_course_runs = fetch_external_courses(keymap=EmeritusKeyMap()) + actual_course_runs = fetch_external_courses(keymap=keymap) mock_get.assert_any_call( "https://test_external_course_sync_api.io/api/queries?api_key=test_EXTERNAL_COURSE_SYNC_API_KEY", @@ -626,7 +673,12 @@ def test_fetch_external_courses_success(settings, mocker): assert actual_course_runs == external_course_runs["rows"] -def test_fetch_external_courses_error(settings, mocker, caplog): +@pytest.mark.parametrize( + "external_course_vendor_keymap", [EmeritusKeyMap, GlobalAlumniKeyMap] +) +def test_fetch_external_courses_error( + settings, mocker, caplog, external_course_vendor_keymap +): """ Tests that `fetch_external_courses` specific calls to the External Course Sync API and Fails for Job status 3 and 4. """ @@ -641,9 +693,10 @@ def test_fetch_external_courses_error(settings, mocker, caplog): "courses.sync_external_courses.external_course_sync_api_client.requests.post" ) + keymap = external_course_vendor_keymap() batch_query = { "id": 77, - "name": "Batch", + "name": keymap.report_names[0], } mock_get.side_effect = [ MockResponse({"results": [batch_query]}), @@ -653,11 +706,16 @@ def test_fetch_external_courses_error(settings, mocker, caplog): ] mock_post.side_effect = [MockResponse({"job": {"id": 1}})] with caplog.at_level(logging.ERROR): - fetch_external_courses(keymap=EmeritusKeyMap()) + fetch_external_courses(keymap=keymap) assert "Job failed!" in caplog.text assert "Something unexpected happened!" in caplog.text +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize( ( "create_existing_product", @@ -751,6 +809,11 @@ def test_save_page_revision(is_draft_page, has_unpublished_changes): assert external_course_page.has_unpublished_changes +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize( ("title", "course_code", "course_run_code", "is_valid"), [ @@ -760,22 +823,44 @@ def test_save_page_revision(is_draft_page, has_unpublished_changes): "MO-DBIP.ELE-99-07#1", True, ), + ( + "Internet of Things (IoT): Design and Applications ", + "MXP-DBIP", + "MXP-DBIP.ELE-99-07#1", + True, + ), ("", "MO-DBIP", "MO-DBIP.ELE-99-07#1", False), + ("", "MXP-DBIP", "MXP-DBIP.ELE-99-07#1", False), (None, "MO-DBIP", "MO-DBIP.ELE-99-07#1", False), + (None, "MXP-DBIP", "MXP-DBIP.ELE-99-07#1", False), ( " Internet of Things (IoT): Design and Applications ", "", "MO-DBIP.ELE-99-07#1", False, ), + ( + " Internet of Things (IoT): Design and Applications ", + "", + "MXP-DBIP.ELE-99-07#1", + False, + ), ( " Internet of Things (IoT): Design and Applications", None, "MO-DBIP.ELE-99-07#1", False, ), + ( + " Internet of Things (IoT): Design and Applications", + None, + "MXP-DBIP.ELE-99-07#1", + False, + ), ("Internet of Things (IoT): Design and Applications", "MO-DBIP", "", False), + ("Internet of Things (IoT): Design and Applications", "MXP-DBIP", "", False), ("Internet of Things (IoT): Design and Applications", "MO-DBIP", None, False), + ("Internet of Things (IoT): Design and Applications", "MXP-DBIP", None, False), ("", "", "", False), (None, None, None, False), ], @@ -793,6 +878,11 @@ def test_external_course_validate_required_fields( assert external_course.validate_required_fields(keymap=EmeritusKeyMap()) == is_valid +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize( ("list_currency", "is_valid"), [ @@ -814,6 +904,11 @@ def test_external_course_validate_list_currency( assert external_course.validate_list_currency() == is_valid +@pytest.mark.parametrize( + "external_course_data", + [{"platform": EMERITUS_PLATFORM_NAME}, {"platform": GLOBAL_ALUMNI_PLATFORM_NAME}], + indirect=True, +) @pytest.mark.parametrize( ("end_date", "is_valid"), [ From f0dc3fb2047eeb9c760ad5503fb4d40e97f77fb5 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Mon, 9 Dec 2024 16:08:41 +0500 Subject: [PATCH 15/17] fix: issues --- .../management/commands/sync_external_course_runs.py | 5 +++-- .../external_course_sync_api_test.py | 2 +- courses/tasks.py | 5 +++-- courses/views/v1/__init__.py | 2 +- courses/views_test.py | 10 ++++++++-- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/courses/management/commands/sync_external_course_runs.py b/courses/management/commands/sync_external_course_runs.py index 95e958982..31eb43e46 100644 --- a/courses/management/commands/sync_external_course_runs.py +++ b/courses/management/commands/sync_external_course_runs.py @@ -42,8 +42,9 @@ def handle(self, *args, **options): # noqa: ARG002 return self.stdout.write(f"Starting course sync for {vendor_name}.") - external_course_runs = fetch_external_courses(keymap()) - stats = update_external_course_runs(external_course_runs, keymap()) + keymap = keymap() + external_course_runs = fetch_external_courses(keymap) + stats = update_external_course_runs(external_course_runs, keymap) self.log_stats(stats) self.stdout.write( self.style.SUCCESS(f"External course sync successful for {vendor_name}.") diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index 9d1b93a8b..f8c305753 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -110,7 +110,7 @@ def external_course_data_with_non_usd_price(external_course_data): external_course_json = external_course_data.copy() external_course_json["list_currency"] = "INR" external_course_json["course_run_code"] = ( - f"{external_course_data["course_run_code"].split("-")[0]}-INRC-98-10#1" + f"{external_course_data['course_run_code'].split('-')[0]}-INRC-98-10#1" ) return external_course_json diff --git a/courses/tasks.py b/courses/tasks.py index bd482709c..52c308d5e 100644 --- a/courses/tasks.py +++ b/courses/tasks.py @@ -131,7 +131,8 @@ def task_sync_external_course_runs(): ) continue try: - external_course_runs = fetch_external_courses(keymap()) - update_external_course_runs(external_course_runs, keymap()) + keymap = keymap() + external_course_runs = fetch_external_courses(keymap) + update_external_course_runs(external_course_runs, keymap) except Exception: log.exception("Some error occurred") diff --git a/courses/views/v1/__init__.py b/courses/views/v1/__init__.py index c4777cde3..86177ae15 100644 --- a/courses/views/v1/__init__.py +++ b/courses/views/v1/__init__.py @@ -225,7 +225,7 @@ def get(self, request, *args, **kwargs): # noqa: ARG002 if not keymap: return Response( { - "error": f"The vendor '{vendor}' is not supported. Supported vendors are {", ".join(EXTERNAL_COURSE_VENDOR_KEYMAPS)}" + "error": f"The vendor '{vendor}' is not supported. Supported vendors are {', '.join(EXTERNAL_COURSE_VENDOR_KEYMAPS)}" }, status=status.HTTP_400_BAD_REQUEST, ) diff --git a/courses/views_test.py b/courses/views_test.py index 477c4e928..99b6e6864 100644 --- a/courses/views_test.py +++ b/courses/views_test.py @@ -39,6 +39,7 @@ ) from courses.sync_external_courses.external_course_sync_api import ( EMERITUS_PLATFORM_NAME, + GLOBAL_ALUMNI_PLATFORM_NAME, ) from ecommerce.factories import ProductFactory, ProductVersionFactory from mitxpro.test_utils import assert_drf_json_equal @@ -619,7 +620,12 @@ def test_course_topics_api(client, django_assert_num_queries): @pytest.mark.parametrize("expected_status_code", [200, 500]) -def test_external_course_list_view(admin_drf_client, mocker, expected_status_code): +@pytest.mark.parametrize( + "vendor_name", [EMERITUS_PLATFORM_NAME, GLOBAL_ALUMNI_PLATFORM_NAME] +) +def test_external_course_list_view( + admin_drf_client, mocker, expected_status_code, vendor_name +): """ Test that the External API List calls fetch_external_courses and returns its mocked response. """ @@ -643,7 +649,7 @@ def test_external_course_list_view(admin_drf_client, mocker, expected_status_cod } response = admin_drf_client.get( - reverse("external_courses", kwargs={"vendor": EMERITUS_PLATFORM_NAME}) + reverse("external_courses", kwargs={"vendor": vendor_name}) ) assert response.json() == mocked_response assert response.status_code == expected_status_code From ca1b667e9ab64a904601202a2607e9260e7db04c Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Mon, 9 Dec 2024 17:42:20 +0500 Subject: [PATCH 16/17] fix: tests --- .../external_course_sync_api_test.py | 72 +++++++++++++++---- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index f8c305753..e513e661c 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -226,12 +226,17 @@ def test_create_or_update_external_course_page( # noqa: PLR0913 else: external_course_page.unpublish() + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) external_course_page, course_page_created, course_page_updated = ( create_or_update_external_course_page( course_index_page, course, - ExternalCourse(external_course_data, keymap=EmeritusKeyMap()), - keymap=EmeritusKeyMap(), + ExternalCourse(external_course_data, keymap=keymap), + keymap=keymap, ) ) external_course_page = external_course_page.revisions.last().as_object() @@ -314,9 +319,14 @@ def test_create_or_update_certificate_page( else: certificate_page.unpublish() + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) certificate_page, is_created, is_updated = create_or_update_certificate_page( external_course_page, - ExternalCourse(external_course_data, keymap=EmeritusKeyMap()), + ExternalCourse(external_course_data, keymap=keymap), ) certificate_page = certificate_page.revisions.last().as_object() assert certificate_page.CEUs == external_course_data["ceu"] @@ -429,7 +439,12 @@ def test_create_or_update_external_course_run( """ Tests that `create_or_update_external_course_run` creates or updates a course run """ - external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) + external_course = ExternalCourse(external_course_data, keymap=keymap) course = CourseFactory.create() if create_existing_course_run: run = CourseRunFactory.create( @@ -501,7 +516,12 @@ def test_update_external_course_runs( # noqa: PLR0915, PLR0913 ).open() as test_data_file: external_course_runs = json.load(test_data_file)["rows"] - platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) + platform_name = ( + EMERITUS_PLATFORM_NAME + if external_course_data["course_run_code"].startswith("MO") + else GLOBAL_ALUMNI_PLATFORM_NAME + ) + platform = PlatformFactory.create(name=platform_name) if create_existing_data: for run in random.sample(external_course_runs, len(external_course_runs) // 2): @@ -540,7 +560,12 @@ def test_update_external_course_runs( # noqa: PLR0915, PLR0913 external_course_runs.append(external_course_with_bad_data) external_course_runs.append(external_course_data_with_null_price) external_course_runs.append(external_course_data_with_non_usd_price) - stats = update_external_course_runs(external_course_runs, keymap=EmeritusKeyMap()) + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) + stats = update_external_course_runs(external_course_runs, keymap=keymap) courses = Course.objects.filter(platform=platform) num_courses_created = 2 if create_existing_data else 4 @@ -746,8 +771,16 @@ def test_create_or_update_product_and_product_version( # noqa: PLR0913 Tests that `create_or_update_product_and_product_version` creates or updates products and versions as required. """ external_course_data["list_price"] = new_price - external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) - platform = PlatformFactory.create(name=EMERITUS_PLATFORM_NAME) + + if external_course_data["course_run_code"].startswith("MO"): + keymap = EmeritusKeyMap() + platform_name = EMERITUS_PLATFORM_NAME + else: + keymap = GlobalAlumniKeyMap() + platform_name = GLOBAL_ALUMNI_PLATFORM_NAME + + external_course = ExternalCourse(external_course_data, keymap=keymap) + platform = PlatformFactory.create(name=platform_name) course = CourseFactory.create( external_course_id=external_course.course_code, platform=platform, @@ -871,11 +904,16 @@ def test_external_course_validate_required_fields( """ Tests that ExternalCourse.validate_required_fields validates required fields. """ - external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) + external_course = ExternalCourse(external_course_data, keymap=keymap) external_course.course_title = title.strip() if title else title external_course.course_code = course_code external_course.course_run_code = course_run_code - assert external_course.validate_required_fields(keymap=EmeritusKeyMap()) == is_valid + assert external_course.validate_required_fields(keymap=keymap) == is_valid @pytest.mark.parametrize( @@ -899,7 +937,12 @@ def test_external_course_validate_list_currency( """ Tests that the `USD` is the only valid currency for the External courses. """ - external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) + external_course = ExternalCourse(external_course_data, keymap=keymap) external_course.list_currency = list_currency assert external_course.validate_list_currency() == is_valid @@ -920,6 +963,11 @@ def test_external_course_validate_end_date(external_course_data, end_date, is_va """ Tests that the valid end date is in the future for External courses. """ - external_course = ExternalCourse(external_course_data, keymap=EmeritusKeyMap()) + keymap = ( + EmeritusKeyMap() + if external_course_data["course_run_code"].startswith("MO") + else GlobalAlumniKeyMap() + ) + external_course = ExternalCourse(external_course_data, keymap=keymap) external_course.end_date = end_date assert external_course.validate_end_date() == is_valid From bae5af0449176c019a6b749cb49ecb4a658483b0 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Mon, 9 Dec 2024 18:30:34 +0500 Subject: [PATCH 17/17] fix: tests --- .../external_course_sync_api_test.py | 69 ++++++------------- 1 file changed, 22 insertions(+), 47 deletions(-) diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index e513e661c..b3cd7bd25 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -115,6 +115,18 @@ def external_course_data_with_non_usd_price(external_course_data): return external_course_json +def get_keymap(run_code): + return EmeritusKeyMap() if run_code.startswith("MO") else GlobalAlumniKeyMap() + + +def get_platform(run_code): + return ( + EMERITUS_PLATFORM_NAME + if run_code.startswith("MO") + else GLOBAL_ALUMNI_PLATFORM_NAME + ) + + @pytest.mark.parametrize( ("external_course_run_code", "expected_course_run_tag"), [ @@ -226,11 +238,7 @@ def test_create_or_update_external_course_page( # noqa: PLR0913 else: external_course_page.unpublish() - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) external_course_page, course_page_created, course_page_updated = ( create_or_update_external_course_page( course_index_page, @@ -319,11 +327,7 @@ def test_create_or_update_certificate_page( else: certificate_page.unpublish() - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) certificate_page, is_created, is_updated = create_or_update_certificate_page( external_course_page, ExternalCourse(external_course_data, keymap=keymap), @@ -439,11 +443,7 @@ def test_create_or_update_external_course_run( """ Tests that `create_or_update_external_course_run` creates or updates a course run """ - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) external_course = ExternalCourse(external_course_data, keymap=keymap) course = CourseFactory.create() if create_existing_course_run: @@ -516,11 +516,7 @@ def test_update_external_course_runs( # noqa: PLR0915, PLR0913 ).open() as test_data_file: external_course_runs = json.load(test_data_file)["rows"] - platform_name = ( - EMERITUS_PLATFORM_NAME - if external_course_data["course_run_code"].startswith("MO") - else GLOBAL_ALUMNI_PLATFORM_NAME - ) + platform_name = get_platform(external_course_data["course_run_code"]) platform = PlatformFactory.create(name=platform_name) if create_existing_data: @@ -560,11 +556,7 @@ def test_update_external_course_runs( # noqa: PLR0915, PLR0913 external_course_runs.append(external_course_with_bad_data) external_course_runs.append(external_course_data_with_null_price) external_course_runs.append(external_course_data_with_non_usd_price) - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) stats = update_external_course_runs(external_course_runs, keymap=keymap) courses = Course.objects.filter(platform=platform) @@ -772,13 +764,8 @@ def test_create_or_update_product_and_product_version( # noqa: PLR0913 """ external_course_data["list_price"] = new_price - if external_course_data["course_run_code"].startswith("MO"): - keymap = EmeritusKeyMap() - platform_name = EMERITUS_PLATFORM_NAME - else: - keymap = GlobalAlumniKeyMap() - platform_name = GLOBAL_ALUMNI_PLATFORM_NAME - + keymap = get_keymap(external_course_data["course_run_code"]) + platform_name = get_platform(external_course_data["course_run_code"]) external_course = ExternalCourse(external_course_data, keymap=keymap) platform = PlatformFactory.create(name=platform_name) course = CourseFactory.create( @@ -904,11 +891,7 @@ def test_external_course_validate_required_fields( """ Tests that ExternalCourse.validate_required_fields validates required fields. """ - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) external_course = ExternalCourse(external_course_data, keymap=keymap) external_course.course_title = title.strip() if title else title external_course.course_code = course_code @@ -937,11 +920,7 @@ def test_external_course_validate_list_currency( """ Tests that the `USD` is the only valid currency for the External courses. """ - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) external_course = ExternalCourse(external_course_data, keymap=keymap) external_course.list_currency = list_currency assert external_course.validate_list_currency() == is_valid @@ -963,11 +942,7 @@ def test_external_course_validate_end_date(external_course_data, end_date, is_va """ Tests that the valid end date is in the future for External courses. """ - keymap = ( - EmeritusKeyMap() - if external_course_data["course_run_code"].startswith("MO") - else GlobalAlumniKeyMap() - ) + keymap = get_keymap(external_course_data["course_run_code"]) external_course = ExternalCourse(external_course_data, keymap=keymap) external_course.end_date = end_date assert external_course.validate_end_date() == is_valid