From 8fd2609b6f45fa3a88a10a9e1c363075d4ca4eb2 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Tue, 7 Jan 2025 15:57:05 -0600 Subject: [PATCH] Move the metadata retrieval to an api function, add a parser for readable ids, fall back to initial data - The metadata retrieval is now in an api function so it's easier to test. - Added a parser for readable ID - skus are generally readable IDs, but if we have a sku that's for a specific run, the learning_resources API won't know what to do with it. - If the data returned from the API doesn't contain data, or doesn't contain _some_ data, fall back to the data that was in the product already. - Added tests for API function - Fixed some names (we're not just pulling image data now) --- system_meta/api.py | 65 +++++++++++++++++++ system_meta/api_test.py | 50 ++++++++++++++ .../management/commands/manage_product.py | 2 +- system_meta/tasks.py | 33 ++++------ unified_ecommerce/utils.py | 23 +++++++ 5 files changed, 152 insertions(+), 21 deletions(-) create mode 100644 system_meta/api.py create mode 100644 system_meta/api_test.py diff --git a/system_meta/api.py b/system_meta/api.py new file mode 100644 index 00000000..0047cdd1 --- /dev/null +++ b/system_meta/api.py @@ -0,0 +1,65 @@ +"""API functions for system metadata.""" + +import logging + +import requests +from django.conf import settings + +from system_meta.models import Product +from unified_ecommerce.utils import parse_readable_id + +log = logging.getLogger(__name__) + + +def update_product_metadata(product_id: int) -> None: + """Get product metadata from the Learn API.""" + + try: + product = Product.objects.get(id=product_id) + resource, run = parse_readable_id(product.sku) + response = requests.get( + f"{settings.MITOL_LEARN_API_URL}learning_resources/", + params={"platform": product.system.slug, "readable_id": resource}, + timeout=10, + ) + response.raise_for_status() + results_data = response.json() + + if results_data.get("count", 0) == 0: + log.warning("No Learn results found for product %s", product) + return + + course_data = results_data.get("results")[0] + + image_data = course_data.get("image") + product.image_metadata = ( + { + "image_url": image_data.get("url"), + "alt_text": image_data.get("alt"), + "description": image_data.get("description"), + } + if image_data + else product.image_metadata + ) + + product.name = course_data.get("title", product.name) + product.description = course_data.get("description", product.description) + + # If there are runs, we'll overwrite this with the run's price later. + product_prices = course_data.get("prices", []) + product_prices.sort() + product.price = product_prices.pop() if len(product_prices) else product.price + + runs = course_data.get("runs") + if runs: + run = next((r for r in runs if r.get("run_id") == product.sku), None) + if run: + product_prices = run.get("prices", []) + product_prices.sort() + product.price = ( + product_prices.pop() if len(product_prices) else product.price + ) + + product.save() + except requests.RequestException: + log.exception("Failed to update metadata for product %s", product.id) diff --git a/system_meta/api_test.py b/system_meta/api_test.py new file mode 100644 index 00000000..330f4f1f --- /dev/null +++ b/system_meta/api_test.py @@ -0,0 +1,50 @@ +"""Tests for the system_meta APIs.""" + +import pytest + +from system_meta.api import update_product_metadata +from system_meta.factories import ProductFactory + +pytestmark = pytest.mark.django_db + + +def test_retrieve_product_metadata(mocker): + """Test that the retrieve_product_metadata function works.""" + + mocked_requests = mocker.patch("requests.get") + mocked_requests.return_value.json.return_value = { + "results": [ + { + "image": { + "id": 12345, + "url": "https://example.com/image.jpg", + "alt": "Example image", + "description": "Example description", + }, + "title": "Example title", + "description": "Example description", + "prices": [100, 200], + "readable_id": "course-v1:MITx+12.345x", + "runs": [ + { + "run_id": "123", + "prices": [150, 250], + "readable_id": "course-v1:MITx+12.345x+2T2099", + } + ], + } + ] + } + + product = ProductFactory.create( + sku="course-v1:MITx+12.345x+2T2099", + price=50, + description="This is the wrong description.", + ) + update_product_metadata(product.id) + product.refresh_from_db() + assert product.image_metadata is not None + assert product.name == "Example title" + assert product.description == "Example description" + # This has a run price, so we should have that, and it should be the highest price. + assert product.price == 250 diff --git a/system_meta/management/commands/manage_product.py b/system_meta/management/commands/manage_product.py index 4b56421b..09e44479 100644 --- a/system_meta/management/commands/manage_product.py +++ b/system_meta/management/commands/manage_product.py @@ -195,7 +195,7 @@ def _add_product( exception_message = "Product {sku} already exists in system {system_name}." raise CommandError(exception_message, returncode=2) - system = IntegratedSystem.objects.get(name=system_name) + system = IntegratedSystem.objects.get(slug=system_name) product = Product.objects.create( sku=sku, name=name, diff --git a/system_meta/tasks.py b/system_meta/tasks.py index 2589bbcf..aefb20d1 100644 --- a/system_meta/tasks.py +++ b/system_meta/tasks.py @@ -1,40 +1,33 @@ +"""Tasks for the system_meta app.""" + import logging from typing import Optional import requests from celery import shared_task -from django.conf import settings @shared_task def update_products(product_id: Optional[int] = None): """ - Update all product's image metadata. If product_id is provided, only update the - product with that ID. Otherwise, update all products. + Update product metadata from the Learn API. + + Updates all products if a product_id is not provided. Pulls the image metadata, + name, and description from the Learn API. If the product has a run ID, it also + pulls the price from the specific run; otherwise, pulls the price from the + resource. """ - from .models import Product + from system_meta.api import update_product_metadata + from system_meta.models import Product log = logging.getLogger(__name__) if product_id: products = Product.objects.filter(id=product_id) else: products = Product.objects.all() + for product in products: try: - response = requests.get( - f"{settings.MITOL_LEARN_API_URL}learning_resources/", - params={"platform": product.system.slug, "readable_id": product.sku}, - timeout=10, - ) - response.raise_for_status() - results_data = response.json() - course_data = results_data.get("results")[0] - image_data = course_data.get("image") - product.image_metadata = { - "image_url": image_data.get("url"), - "alt_text": image_data.get("alt"), - "description": image_data.get("description"), - } - product.save() + update_product_metadata(product.id) except requests.RequestException: - log.exception("Failed to retrieve image data for product %s", product.id) + log.exception("Failed to update metdata for product %s", product.id) diff --git a/unified_ecommerce/utils.py b/unified_ecommerce/utils.py index 937bfc67..4160117b 100644 --- a/unified_ecommerce/utils.py +++ b/unified_ecommerce/utils.py @@ -445,3 +445,26 @@ def get_user_from_apisix_headers(request): user.refresh_from_db() return user + + +def parse_readable_id(readable_id: str) -> tuple[str, str]: + """ + Parse a readable ID into a resource ID and a run ID. + + Readable IDs look like "course-v1:MITxT+12.345x" but they may also have a run + tacked onto the end ("+1T2024" for instance). If the readable ID isn't for a + run of the resource, you'll get a None in the run position. + + Args: + readable_id (str): The readable ID to parse + + Returns: + tuple[str, str]: The resource ID and the run ID (or None) + """ + if readable_id.count("+") > 1: + resource, run = readable_id.rsplit("+", 1) + else: + resource = readable_id + run = None + + return resource, run