From 1f732ca66c73c8302de23e65ed8c494bb4f3e1b4 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 13 Jan 2025 08:25:04 -0800 Subject: [PATCH] Pull more product data from the Learn API (#193) --- frontends/api/src/generated/v0/api.ts | 142 ++++++++++++++++++ openapi/specs/v0.yaml | 27 ++++ system_meta/api.py | 118 +++++++++++++++ system_meta/api_test.py | 70 +++++++++ .../management/commands/import_product.py | 48 ++++++ .../management/commands/manage_product.py | 4 +- ...t_image_data.py => update_product_data.py} | 2 +- .../management/tests/manage_product_test.py | 2 +- system_meta/private_urls.py | 6 - system_meta/tasks.py | 33 ++-- system_meta/urls.py | 6 + system_meta/views.py | 86 +++++++++-- unified_ecommerce/settings.py | 1 + unified_ecommerce/utils.py | 23 +++ 14 files changed, 523 insertions(+), 45 deletions(-) create mode 100644 system_meta/api.py create mode 100644 system_meta/api_test.py create mode 100644 system_meta/management/commands/import_product.py rename system_meta/management/commands/{update_product_image_data.py => update_product_data.py} (96%) diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index 2134d9b8..30281ed3 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -2761,6 +2761,58 @@ export const MetaApiAxiosParamCreator = function ( options: localVarRequestOptions, } }, + /** + * Pre-loads the product metadata for a given SKU, even if the SKU doesn\'t exist yet. + * @param {string} sku + * @param {string} system_slug + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + metaProductPreloadRetrieve: async ( + sku: string, + system_slug: string, + options: RawAxiosRequestConfig = {}, + ): Promise => { + // verify required parameter 'sku' is not null or undefined + assertParamExists("metaProductPreloadRetrieve", "sku", sku) + // verify required parameter 'system_slug' is not null or undefined + assertParamExists( + "metaProductPreloadRetrieve", + "system_slug", + system_slug, + ) + const localVarPath = `/api/v0/meta/product/preload/{system_slug}/{sku}/` + .replace(`{${"sku"}}`, encodeURIComponent(String(sku))) + .replace(`{${"system_slug"}}`, encodeURIComponent(String(system_slug))) + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL) + let baseOptions + if (configuration) { + baseOptions = configuration.baseOptions + } + + const localVarRequestOptions = { + method: "GET", + ...baseOptions, + ...options, + } + const localVarHeaderParameter = {} as any + const localVarQueryParameter = {} as any + + setSearchParams(localVarUrlObj, localVarQueryParameter) + let headersFromBaseOptions = + baseOptions && baseOptions.headers ? baseOptions.headers : {} + localVarRequestOptions.headers = { + ...localVarHeaderParameter, + ...headersFromBaseOptions, + ...options.headers, + } + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + } + }, /** * Viewset for Product model. * @param {number} id A unique integer value identifying this product. @@ -3185,6 +3237,37 @@ export const MetaApiFp = function (configuration?: Configuration) { configuration, )(axios, operationBasePath || basePath) }, + /** + * Pre-loads the product metadata for a given SKU, even if the SKU doesn\'t exist yet. + * @param {string} sku + * @param {string} system_slug + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async metaProductPreloadRetrieve( + sku: string, + system_slug: string, + options?: RawAxiosRequestConfig, + ): Promise< + (axios?: AxiosInstance, basePath?: string) => AxiosPromise + > { + const localVarAxiosArgs = + await localVarAxiosParamCreator.metaProductPreloadRetrieve( + sku, + system_slug, + options, + ) + const index = configuration?.serverIndex ?? 0 + const operationBasePath = + operationServerMap["MetaApi.metaProductPreloadRetrieve"]?.[index]?.url + return (axios, basePath) => + createRequestFunction( + localVarAxiosArgs, + globalAxios, + BASE_PATH, + configuration, + )(axios, operationBasePath || basePath) + }, /** * Viewset for Product model. * @param {number} id A unique integer value identifying this product. @@ -3420,6 +3503,24 @@ export const MetaApiFactory = function ( ) .then((request) => request(axios, basePath)) }, + /** + * Pre-loads the product metadata for a given SKU, even if the SKU doesn\'t exist yet. + * @param {MetaApiMetaProductPreloadRetrieveRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + metaProductPreloadRetrieve( + requestParameters: MetaApiMetaProductPreloadRetrieveRequest, + options?: RawAxiosRequestConfig, + ): AxiosPromise { + return localVarFp + .metaProductPreloadRetrieve( + requestParameters.sku, + requestParameters.system_slug, + options, + ) + .then((request) => request(axios, basePath)) + }, /** * Viewset for Product model. * @param {MetaApiMetaProductRetrieveRequest} requestParameters Request parameters. @@ -3644,6 +3745,27 @@ export interface MetaApiMetaProductPartialUpdateRequest { readonly PatchedProductRequest?: PatchedProductRequest } +/** + * Request parameters for metaProductPreloadRetrieve operation in MetaApi. + * @export + * @interface MetaApiMetaProductPreloadRetrieveRequest + */ +export interface MetaApiMetaProductPreloadRetrieveRequest { + /** + * + * @type {string} + * @memberof MetaApiMetaProductPreloadRetrieve + */ + readonly sku: string + + /** + * + * @type {string} + * @memberof MetaApiMetaProductPreloadRetrieve + */ + readonly system_slug: string +} + /** * Request parameters for metaProductRetrieve operation in MetaApi. * @export @@ -3871,6 +3993,26 @@ export class MetaApi extends BaseAPI { .then((request) => request(this.axios, this.basePath)) } + /** + * Pre-loads the product metadata for a given SKU, even if the SKU doesn\'t exist yet. + * @param {MetaApiMetaProductPreloadRetrieveRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof MetaApi + */ + public metaProductPreloadRetrieve( + requestParameters: MetaApiMetaProductPreloadRetrieveRequest, + options?: RawAxiosRequestConfig, + ) { + return MetaApiFp(this.configuration) + .metaProductPreloadRetrieve( + requestParameters.sku, + requestParameters.system_slug, + options, + ) + .then((request) => request(this.axios, this.basePath)) + } + /** * Viewset for Product model. * @param {MetaApiMetaProductRetrieveRequest} requestParameters Request parameters. diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 0038d24c..a08088eb 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -304,6 +304,33 @@ paths: responses: '204': description: No response body + /api/v0/meta/product/preload/{system_slug}/{sku}/: + get: + operationId: meta_product_preload_retrieve + description: Pre-loads the product metadata for a given SKU, even if the SKU + doesn't exist yet. + parameters: + - in: path + name: sku + schema: + type: string + pattern: ^[^/]+$ + required: true + - in: path + name: system_slug + schema: + type: string + pattern: ^[^/]+$ + required: true + tags: + - meta + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/Product' + description: '' /api/v0/payments/baskets/: get: operationId: payments_baskets_list diff --git a/system_meta/api.py b/system_meta/api.py new file mode 100644 index 00000000..d4312950 --- /dev/null +++ b/system_meta/api.py @@ -0,0 +1,118 @@ +"""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 get_product_metadata( + platform: str, readable_id: str, *, all_data: bool = False +) -> dict | None: + """ + Get product metadata from the Learn API. + + Args: + platform: The platform slug. + readable_id: The readable ID of the product. + all_data: Whether to return all the data returned or the minimal amount + to bootstrap a product. + + Returns: + The product metadata from the Learn API. + """ + + def _format_output(data: dict, *, all_data: bool) -> dict: + """Format the Learn API data accordingly.""" + + if all_data: + return data.get("results", [])[0] + + course_data = data.get("results")[0] + image_data = course_data.get("image", {}) + prices = course_data.get("prices", []) + prices.sort() + price = prices[-1] if len(prices) else 0 + + runs = course_data.get("runs", []) + run = next((r for r in runs if r.get("run_id") == readable_id), None) + if run: + run_prices = run.get("prices", []) + run_prices.sort() + run_price = run_prices[-1] if len(run_prices) else 0 + + return { + "sku": run.get("run_id") if run else course_data.get("readable_id"), + "title": course_data.get("title"), + "description": course_data.get("description"), + "image": { + "image_url": image_data.get("url"), + "alt_text": image_data.get("alt"), + "description": image_data.get("description"), + } + if image_data + else None, + "price": run_price if run and run_price > price else price, + } + + try: + split_readable_id, split_run = parse_readable_id(readable_id) + response = requests.get( + f"{settings.MITOL_LEARN_API_URL}learning_resources/", + params={"platform": platform, "readable_id": split_readable_id}, + timeout=10, + ) + response.raise_for_status() + raw_response = response.json() + + if raw_response.get("count", 0) > 0: + course_data = raw_response.get("results")[0] + if split_run and course_data.get("runs"): + test_run = next( + ( + r + for r in course_data.get("runs") + if r.get("run_id") == readable_id + ), + None, + ) + if test_run: + return _format_output(raw_response, all_data=all_data) + + return None + + return _format_output(raw_response, all_data=all_data) + else: + return None + except requests.RequestException: + log.exception("Failed to get product metadata for %s", readable_id) + return None + + +def update_product_metadata(product_id: int) -> None: + """Get product metadata from the Learn API.""" + + try: + product = Product.objects.get(id=product_id) + fetched_metadata = get_product_metadata(product.system.slug, product.sku) + + if not fetched_metadata: + log.warning("No Learn results found for product %s", product) + return + + product.image_metadata = ( + fetched_metadata.get("image", None) or product.image_metadata + ) + + product.name = fetched_metadata.get("title", product.name) + product.description = fetched_metadata.get("description", product.description) + product.price = fetched_metadata.get("price", 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..fef88ae6 --- /dev/null +++ b/system_meta/api_test.py @@ -0,0 +1,70 @@ +"""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 + + +@pytest.mark.parametrize("valid_run", [True, False]) +def test_retrieve_product_metadata(mocker, valid_run): + """Test that the retrieve_product_metadata function works.""" + + mocked_requests = mocker.patch("requests.get") + mocked_requests.return_value.json.return_value = { + "count": 1, + "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": "course-v1:MITx+12.345x+2T2099", + "prices": [150, 250], + } + ] + if valid_run + else [], + } + ], + } + + 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 if valid_run else 200 + + +def test_retrieve_product_metadata_no_match(mocker): + """Test that the retrieve_product_metadata function works when no data exists in Learn..""" + + mocked_requests = mocker.patch("requests.get") + mocked_requests.return_value.json.return_value = {"count": 0, "results": []} + + 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.description == "This is the wrong description." + assert product.price == 50 diff --git a/system_meta/management/commands/import_product.py b/system_meta/management/commands/import_product.py new file mode 100644 index 00000000..60f96f0b --- /dev/null +++ b/system_meta/management/commands/import_product.py @@ -0,0 +1,48 @@ +"""Import a product from the Learn API.""" + +import logging + +from django.core.management.base import BaseCommand + +from system_meta.api import get_product_metadata +from system_meta.models import IntegratedSystem, Product + +log = logging.getLogger(__name__) + + +class Command(BaseCommand): + """Import a product from the Learn API.""" + + help = "Import a product from the Learn API." + + def add_arguments(self, parser): + """Add arguments to the command.""" + parser.add_argument( + "system_slug", + type=str, + help="The slug of the system to import the product from", + ) + + parser.add_argument( + "readable_id", + type=str, + help="The readable ID of the product to import", + ) + + def handle(self, *args, **kwargs): # noqa: ARG002 + """Handle the command.""" + metadata = get_product_metadata(kwargs["system_slug"], kwargs["readable_id"]) + + system = IntegratedSystem.objects.get(slug=kwargs["system_slug"]) + product = Product.objects.create( + sku=metadata["sku"], + name=metadata["title"], + description=metadata["description"], + image_metadata=metadata["image"], + price=metadata["price"], + system=system, + ) + + self.stdout.write( + self.style.SUCCESS(f"Successfully imported product {product}") + ) diff --git a/system_meta/management/commands/manage_product.py b/system_meta/management/commands/manage_product.py index 4b56421b..d0d5f983 100644 --- a/system_meta/management/commands/manage_product.py +++ b/system_meta/management/commands/manage_product.py @@ -42,7 +42,7 @@ def add_arguments(self, parser): "-s", type=str, required=True, - help="The system to add the product to.", + help="The system slug to add the product to.", metavar="system", ) @@ -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/management/commands/update_product_image_data.py b/system_meta/management/commands/update_product_data.py similarity index 96% rename from system_meta/management/commands/update_product_image_data.py rename to system_meta/management/commands/update_product_data.py index 45f7bdc2..c8db712f 100644 --- a/system_meta/management/commands/update_product_image_data.py +++ b/system_meta/management/commands/update_product_data.py @@ -10,7 +10,7 @@ class Command(BaseCommand): Example usage: python manage.py update_product_image_data --product_id 1 """ - help = "Update image_metadata for all Product objects" + help = "Update product data for all Product objects from Learn." def add_arguments(self, parser): parser.add_argument( diff --git a/system_meta/management/tests/manage_product_test.py b/system_meta/management/tests/manage_product_test.py index 3e14f1f1..928ec9ce 100644 --- a/system_meta/management/tests/manage_product_test.py +++ b/system_meta/management/tests/manage_product_test.py @@ -26,7 +26,7 @@ def test_add_product(): "--sku", "test_sku", "--system", - system.name, + system.slug, "--price", "1.00", "--description", diff --git a/system_meta/private_urls.py b/system_meta/private_urls.py index 2e240c9a..0fc01f03 100644 --- a/system_meta/private_urls.py +++ b/system_meta/private_urls.py @@ -5,7 +5,6 @@ from system_meta.views import ( apisix_test_request, authed_traefik_test_request, - traefik_test_request, ) urlpatterns = [ @@ -14,11 +13,6 @@ apisix_test_request, name="apisix_test_request", ), - re_path( - r"^traefik_test_request/$", - traefik_test_request, - name="traefik_test_request", - ), re_path( r"^authed_traefik_test_request/$", authed_traefik_test_request, 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/system_meta/urls.py b/system_meta/urls.py index dac3f399..7524318f 100644 --- a/system_meta/urls.py +++ b/system_meta/urls.py @@ -7,6 +7,7 @@ from system_meta.views import ( IntegratedSystemViewSet, ProductViewSet, + preload_sku, ) v0_router = routers.DefaultRouter() @@ -18,7 +19,12 @@ ) v0_router.register(r"^meta/product", ProductViewSet, basename="meta_products_api") +app_name = "v0" urlpatterns = [ path("admin/", admin.site.urls), + re_path( + r"^api/v0/meta/product/preload/(?P[^/]+)/(?P[^/]+)/$", + preload_sku, + ), re_path("^api/v0/", include((v0_router.urls, "v0"))), ] diff --git a/system_meta/views.py b/system_meta/views.py index 1d4940ca..c226a882 100644 --- a/system_meta/views.py +++ b/system_meta/views.py @@ -3,18 +3,22 @@ import logging from django_filters.rest_framework import DjangoFilterBackend +from drf_spectacular.utils import extend_schema from rest_framework import status from rest_framework.authentication import SessionAuthentication from rest_framework.decorators import ( api_view, authentication_classes, permission_classes, + throttle_classes, ) from rest_framework.permissions import ( IsAuthenticated, ) from rest_framework.response import Response +from rest_framework.throttling import AnonRateThrottle +from system_meta.api import get_product_metadata from system_meta.models import IntegratedSystem, Product from system_meta.serializers import ( AdminIntegratedSystemSerializer, @@ -59,6 +63,73 @@ class ProductViewSet(AuthVariegatedModelViewSet): ] +@extend_schema( + description=( + "Pre-loads the product metadata for a given SKU, even if the " + "SKU doesn't exist yet." + ), + methods=["GET"], + request=None, + responses=ProductSerializer, +) +@api_view(["GET"]) +@permission_classes([]) +@authentication_classes([SessionAuthentication]) +@throttle_classes( + [ + AnonRateThrottle, + ] +) +def preload_sku(request, system_slug, sku): # noqa: ARG001 + """ + Preload the SKU for the product. + + If we have this product, then we just return the product info (serialized + like you'd have gotten otherwise). If we don't, we'll create a skeleton + product, make the call to update it from Learn, and then return the new + product. + + Integrated systems should be able to use this to ensure the resource they're + displaying to the user has a corresponding product in UE before the user + tries to buy it. (This of course requires that the resource has also been + loaded into Learn too.) + + This is throttled for unauthenticated users to prevent abuse - this is + pretty likely to spawn a request to the Learn API, so we don't want to kill + it by accident. + """ + + if not IntegratedSystem.objects.filter(slug=system_slug).exists(): + return Response( + {"error": "System not found"}, + status=status.HTTP_404_NOT_FOUND, + ) + + existing_product = Product.objects.filter(sku=sku, system__slug=system_slug) + + if existing_product.exists(): + return Response(ProductSerializer(existing_product.first()).data) + + product_metadata = get_product_metadata(system_slug, sku) + if not product_metadata: + return Response( + {"error": "Resource not found"}, + status=status.HTTP_404_NOT_FOUND, + ) + + product = Product.objects.create( + name=product_metadata.get("title"), + sku=product_metadata.get("sku"), + description=product_metadata.get("description"), + price=product_metadata.get("price"), + system=IntegratedSystem.objects.get(slug=system_slug), + ) + product.save() + product.refresh_from_db() + + return Response(ProductSerializer(product).data) + + @api_view(["GET"]) @permission_classes([]) @authentication_classes([SessionAuthentication]) @@ -78,21 +149,6 @@ def apisix_test_request(request): return Response(response, status=status.HTTP_200_OK) -@api_view(["GET"]) -@permission_classes([]) -@authentication_classes([SessionAuthentication]) -def traefik_test_request(request): - """Test API request so we can see how the Traefik integration works.""" - - response = { - "name": "Response ok!", - "user": request.user.username if request.user is not None else None, - "metas": [f"{key}: {value}" for key, value in request.META.items()], - } - - return Response(response, status=status.HTTP_200_OK) - - @api_view(["GET"]) @permission_classes([IsAuthenticated]) @authentication_classes([SessionAuthentication]) diff --git a/unified_ecommerce/settings.py b/unified_ecommerce/settings.py index 40bb0914..26db1866 100644 --- a/unified_ecommerce/settings.py +++ b/unified_ecommerce/settings.py @@ -451,6 +451,7 @@ "ALLOWED_VERSIONS": [ "v0", ], + "DEFAULT_THROTTLE_RATES": {"anon": "100/hour", "user": "1000/day"}, } USE_X_FORWARDED_PORT = get_bool("USE_X_FORWARDED_PORT", False) # noqa: FBT003 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