From a6ad7c746d8df982874c3e88da48a4890a14f074 Mon Sep 17 00:00:00 2001 From: Anastasia Beglova Date: Tue, 18 Aug 2026 11:29:16 -0400 Subject: [PATCH 1/6] make healthcheck resistant to k8 pod culling (#3760) --- main/settings.py | 12 + vector_search/tasks.py | 333 +++++++++++++++++------ vector_search/tasks_test.py | 529 +++++++++++++++++++++++++++++++++--- vector_search/utils.py | 6 +- 4 files changed, 754 insertions(+), 126 deletions(-) diff --git a/main/settings.py b/main/settings.py index 407bf9da59..5929d75976 100644 --- a/main/settings.py +++ b/main/settings.py @@ -800,6 +800,18 @@ def get_all_config_keys(): name="QDRANT_ENCODER", default="vector_search.encoders.gensim.GensimEncoder" ) +# Max Sentry alerts the embeddings healthcheck sends per alert type per run. The +# healthcheck reports per resource, so an environment that is simply behind on +# embedding (e.g. RC) would otherwise burn thousands of events in one run. +# Production should set this high (a large backlog there is a real incident, not +# expected drift); 0 or less disables the cap entirely. The default is deliberately +# low so an unconfigured environment can't spend the quota, and the cap sends one +# explicit notice when it engages, so a capped run is never mistaken for a clean one. +EMBEDDINGS_HEALTHCHECK_ALERT_CAP = get_int( + name="EMBEDDINGS_HEALTHCHECK_ALERT_CAP", + default=20, +) + QDRANT_POINT_UPLOAD_BATCH_SIZE = get_int( name="QDRANT_POINT_UPLOAD_BATCH_SIZE", default=1000 ) diff --git a/vector_search/tasks.py b/vector_search/tasks.py index 1d933c37c1..c465c6283a 100644 --- a/vector_search/tasks.py +++ b/vector_search/tasks.py @@ -1,5 +1,6 @@ import datetime import logging +from uuid import uuid4 import celery import grpc @@ -18,7 +19,6 @@ ) from learning_resources.serializers import ( ContentFileSerializer, - LearningResourceSerializer, ) from learning_resources.utils import load_course_blocklist from learning_resources_search.constants import ( @@ -60,6 +60,23 @@ EMBED_FAILURE_TTL = 60 * 60 * 24 # 24h defensive cleanup for the per-run counter +# point ids per Qdrant existence lookup in the healthcheck tasks +HEALTHCHECK_POINT_BATCH_SIZE = 200 + +# resources per batched resource-embedding check task. Checking a resource is one +# payload-free point, so batching trades task count for Qdrant round trips: at 1 per +# task a full run costs one round trip per resource. +HEALTHCHECK_RESOURCE_BATCH_SIZE = 200 + +# content files per batched content-file check task. Lower than the resource batch: +# each one is serialized with its chunked text, so this bounds the text a worker +# holds at once +HEALTHCHECK_CONTENT_FILE_BATCH_SIZE = 100 + +# TTL for the per-run Sentry alert counters; long enough to outlive a healthcheck +# run (including redelivered tasks) and short enough to not accumulate in redis +HEALTHCHECK_ALERT_TTL = 60 * 60 * 24 + def _record_embedding_failure(failure_key: str) -> None: """Bump the per-invocation embedding-failure counter in the shared redis cache.""" @@ -603,113 +620,197 @@ def remove_unpublished_run_content_files(self, run_id): return _replace_with_chain(self, tasks) -@app.task -def embeddings_healthcheck(): +@app.task(bind=True, acks_late=True, reject_on_worker_lost=True) +def embeddings_healthcheck(self): """ - Check for missing embeddings and summaries in Qdrant and log warnings to Sentry + Dispatch the embeddings healthcheck as a group of independent, self-reporting + tasks: batches of resources, batches of content files, and the summaries check. + + Both checks are dispatched over their own rows, so nothing is queued for a + resource with no content files, and neither list is built from the other. """ + resources = LearningResource.objects.filter(Q(published=True) | Q(test_mode=True)) + resource_ids = list(resources.order_by("id").values_list("id", flat=True)) + + # streamed with iterator(): there are far more content files than resources, and + # only one batch of ids needs to be in memory at a time to build the signatures + content_file_ids = ( + ContentFile.objects.filter(published=True) + .exclude(Q(content="") | Q(content__isnull=True)) + .filter( + Q(run__learning_resource__in=resources) | Q(learning_resource__in=resources) + ) + .order_by("id") + .values_list("id", flat=True) + .iterator(chunk_size=HEALTHCHECK_CONTENT_FILE_BATCH_SIZE) + ) - remaining_content_file_ids = [] - remaining_resources = [] - resource_point_ids = {} - all_resources = LearningResource.objects.filter( - Q(published=True) | Q(test_mode=True) + # scopes the per-run Sentry alert cap: every task dispatched here shares one + # budget. Redelivery reuses the same task id, so a culled and re-dispatched run + # keeps its already-spent budget rather than paying for the same alerts twice. + # A direct call (a shell invocation rather than delay()) has no task id, and + # falling back to a generated key keeps the cap on: without one, this would hand + # every dispatched task run_key=None and uncap the entire run. + run_key = self.request.id or str(uuid4()) + + content_file_tasks = [ + embeddings_healthcheck_content_files.si(batch, run_key=run_key) + for batch in chunks( + content_file_ids, chunk_size=HEALTHCHECK_CONTENT_FILE_BATCH_SIZE + ) + ] + resource_tasks = [ + embeddings_healthcheck_resource_embeddings.si(batch, run_key=run_key) + for batch in chunks(resource_ids, chunk_size=HEALTHCHECK_RESOURCE_BATCH_SIZE) + ] + log.info( + "Embeddings healthcheck dispatching %d resource batches for %d resources " + "and %d content file batches", + len(resource_tasks), + len(resource_ids), + len(content_file_tasks), ) - for lr in all_resources: - serialized = LearningResourceSerializer(lr).data - point_id = vector_point_id(vector_point_key(serialized)) - resource_point_ids[point_id] = {"resource_id": lr.readable_id, "id": lr.id} - content_file_point_ids = {} - # All runs are embedded in Qdrant, not just best_run. - content_files = ContentFile.objects.for_serialization().filter( - Q(run__learning_resource=lr) | Q(learning_resource=lr), - published=True, + return self.replace( + celery.group( + [ + summaries_healthcheck.si(run_key=run_key), + *resource_tasks, + *content_file_tasks, + ] ) - for cf in content_files: - if cf and cf.content: - serialized_cf = ContentFileSerializer(cf).data - point_id = vector_point_id( - vector_point_key( - serialized_cf, chunk_number=0, document_type="content_file" - ) - ) - content_file_point_ids[point_id] = {"key": cf.key, "id": cf.id} - for batch in chunks(content_file_point_ids.keys(), chunk_size=200): - remaining_content_files = filter_existing_qdrant_points_by_ids( - batch, collection_name=CONTENT_FILES_COLLECTION_NAME - ) - remaining_content_file_ids.extend( - [ - content_file_point_ids.get(p, {}).get("id") - for p in remaining_content_files - ] - ) + ) + +@app.task(acks_late=True, reject_on_worker_lost=True) +def embeddings_healthcheck_resource_embeddings(resource_ids, run_key=None): + """ + Check a batch of learning resources for their own missing embeddings in Qdrant + and report the findings to Sentry. + + Batched: each resource contributes one point and no payload, so the cost here is + Qdrant round trips rather than memory. + + Read-only, so re-running after a worker is culled mid-check is safe. + """ + # Build the point ids from the same bulk serialization the embedding pipeline + # uses, so the healthcheck can't disagree with it about a resource's point key. + resource_point_ids = { + vector_point_id(vector_point_key(serialized)): serialized["_id"] + for serialized in serialize_bulk_learning_resources(resource_ids) + } + + missing_resource_ids = [] for batch in chunks( - all_resources.values_list("id", flat=True), - chunk_size=200, + resource_point_ids.keys(), chunk_size=HEALTHCHECK_POINT_BATCH_SIZE ): - remaining_resources.extend( - filter_existing_qdrant_points_by_ids( - [ - vector_point_id(vector_point_key(serialized_resource)) - for serialized_resource in serialize_bulk_learning_resources(batch) - ], - collection_name=RESOURCES_COLLECTION_NAME, + missing_resource_ids.extend( + resource_point_ids[p] + for p in filter_existing_qdrant_points_by_ids( + batch, collection_name=RESOURCES_COLLECTION_NAME ) + if p in resource_point_ids ) - remaining_resource_ids = [ - resource_point_ids.get(p, {}).get("id") for p in remaining_resources - ] - missing_summaries = _missing_summaries() - log.info( - "Embeddings healthcheck found %d missing content file embeddings", - len(remaining_content_file_ids), - ) - log.info( - "Embeddings healthcheck found %d missing resource embeddings", - len(remaining_resources), - ) log.info( - "Embeddings healthcheck found %d missing summaries and flashcards", - len(missing_summaries), + "Embeddings healthcheck: %d of %d resources missing their embedding", + len(missing_resource_ids), + len(resource_ids), ) - if len(remaining_content_file_ids) > 0: + # one alert per batch rather than per resource: the ids identify the resources + # without spending an alert (and a slice of the per-run cap) on each one + if missing_resource_ids: _sentry_healthcheck_log( "embeddings", - "missing_content_file_embeddings", + "missing_learning_resource_embeddings", { - "count": len(remaining_content_file_ids), - "ids": remaining_content_file_ids, - "run_ids": set( - ContentFile.objects.filter( - id__in=remaining_content_file_ids - ).values_list("run__run_id", flat=True)[:100] + "count": len(missing_resource_ids), + "ids": missing_resource_ids, + "readable_ids": list( + LearningResource.objects.filter( + id__in=missing_resource_ids + ).values_list("readable_id", flat=True)[:100] ), }, - f"Warning: {len(remaining_content_file_ids)} missing content file " - "embeddings detected", + "Warning: learning resources are missing their embeddings", + run_key=run_key, + ) + + +@app.task(acks_late=True, reject_on_worker_lost=True) +def embeddings_healthcheck_content_files(content_file_ids, run_key=None): + """ + Check a batch of content files for missing embeddings in Qdrant and report the + findings to Sentry. + + Batched over content files rather than over the resources that own them: the + batch size then bounds how much serialized text a worker holds at once, + regardless of how many content files any one resource has. + + Read-only, so re-running after a worker is culled mid-check is safe. + """ + missing_content_file_ids = [] + + content_file_point_ids = {} + # All runs are embedded in Qdrant, not just best_run, so this batch is drawn from + # content files directly rather than from any one run. + for cf in ContentFile.objects.for_serialization().filter(id__in=content_file_ids): + if cf and cf.content: + serialized_cf = ContentFileSerializer(cf).data + point_id = vector_point_id( + vector_point_key( + serialized_cf, chunk_number=0, document_type="content_file" + ) + ) + content_file_point_ids[point_id] = cf.id + for batch in chunks( + content_file_point_ids.keys(), chunk_size=HEALTHCHECK_POINT_BATCH_SIZE + ): + missing_content_file_ids.extend( + content_file_point_ids[p] + for p in filter_existing_qdrant_points_by_ids( + batch, collection_name=CONTENT_FILES_COLLECTION_NAME + ) + if p in content_file_point_ids ) - if len(remaining_resources) > 0: + log.info( + "Embeddings healthcheck: %d of %d content files missing embeddings", + len(missing_content_file_ids), + len(content_file_ids), + ) + + if missing_content_file_ids: _sentry_healthcheck_log( "embeddings", - "missing_learning_resource_embeddings", + "missing_content_file_embeddings", { - "count": len(remaining_resource_ids), - "ids": remaining_resource_ids, - "titles": list( - LearningResource.objects.filter( - id__in=remaining_resource_ids - ).values_list("title", flat=True) + "count": len(missing_content_file_ids), + "ids": missing_content_file_ids, + "run_ids": set( + ContentFile.objects.filter( + id__in=missing_content_file_ids + ).values_list("run__run_id", flat=True)[:100] ), }, - f"Warning: {len(remaining_resource_ids)} missing learning resource " - "embeddings detected", + "Warning: content files are missing embeddings", + run_key=run_key, ) + + +@app.task(acks_late=True, reject_on_worker_lost=True) +def summaries_healthcheck(run_key=None): + """ + Check for content files missing summaries/flashcards and report to Sentry. + + Read-only, so re-running after a worker is culled is safe. + """ + missing_summaries = _missing_summaries() + log.info( + "Embeddings healthcheck found %d missing summaries and flashcards", + len(missing_summaries), + ) if len(missing_summaries) > 0: _sentry_healthcheck_log( "embeddings", @@ -723,8 +824,8 @@ def embeddings_healthcheck(): )[:100] ), }, - f"Warning: {len(missing_summaries)} missing content file summaries " - "detected", + "Warning: missing content file summaries detected", + run_key=run_key, ) @@ -749,14 +850,78 @@ def _missing_summaries(): ) -def _sentry_healthcheck_log(healthcheck, alert_type, context, message): +def _capture_healthcheck_message(healthcheck, alert_type, context, message): + """Send one healthcheck message to Sentry, uncapped.""" with sentry_sdk.new_scope() as scope: scope.set_tag("healthcheck", healthcheck) scope.set_tag("alert_type", alert_type) - scope.set_context("missing_content_file_embeddings", context) + # key the context off the alert so resource/summary alerts don't file their + # payload under a content-file key + scope.set_context(alert_type, context) sentry_sdk.capture_message(message) +def _healthcheck_alert_count(run_key, alert_type): + """ + Atomically count this run's alerts of one type, so the per-run cap holds across + every worker running the healthcheck's tasks. + """ + cache = caches["redis"] + key = f"healthcheck_alerts:{run_key}:{alert_type}" + try: + return cache.incr(key) + except ValueError: # key absent + # add() is atomic, so exactly one of the workers racing to create the counter + # wins it; the losers fall through to incr instead of each resetting it to 1 + # and handing every racer the same count of 1 + if cache.add(key, 1, HEALTHCHECK_ALERT_TTL): + return 1 + try: + return cache.incr(key) + except ValueError: + # expired between add() and incr(): only reachable if this run outlives + # HEALTHCHECK_ALERT_TTL, in which case counting from 1 again is right + return 1 + + +def _sentry_healthcheck_log(healthcheck, alert_type, context, message, run_key=None): + """ + Report a healthcheck finding to Sentry, capped per run per alert type. + + The healthcheck reports per resource, so an environment that is merely behind on + embedding produces one alert per affected resource. Without a cap that is + thousands of events for a single expected condition. run_key scopes the counter + to one healthcheck run; callers without one (direct calls, tests) are uncapped. + + Each alert type gets its own budget: the checks are independent signals, and a + run works through resources for hours, so a shared budget would be spent by + whichever check happens to find something first. + """ + cap = settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP + if run_key and cap > 0: + count = _healthcheck_alert_count(run_key, alert_type) + if count > cap: + # the count is atomic, so exactly one worker sees cap + 1 and the notice + # is sent once: a capped run must never look like a run that only found + # `cap` problems + if count == cap + 1: + log.warning( + "Embeddings healthcheck reached the Sentry alert cap (%d) for %s; " + "further alerts of this type are suppressed for this run", + cap, + alert_type, + ) + _capture_healthcheck_message( + healthcheck, + f"{alert_type}_suppressed", + {"suppressed_alert_type": alert_type, "cap": cap}, + "Warning: healthcheck alerts suppressed after reaching the " + "per-run cap", + ) + return + _capture_healthcheck_message(healthcheck, alert_type, context, message) + + @app.task(acks_late=True, reject_on_worker_lost=True) def sync_topics(): """ diff --git a/vector_search/tasks_test.py b/vector_search/tasks_test.py index e821d5bc16..ccf67a3a1d 100644 --- a/vector_search/tasks_test.py +++ b/vector_search/tasks_test.py @@ -6,6 +6,7 @@ from celery.exceptions import Retry from django.conf import settings from django.core.cache.backends.locmem import LocMemCache +from django.db.models import Q from learning_resources.etl.constants import ( RESOURCE_FILE_ETL_SOURCES, @@ -28,23 +29,31 @@ PROGRAM_TYPE, ) from learning_resources_search.exceptions import RetryError -from learning_resources_search.serializers import serialize_bulk_content_files +from learning_resources_search.serializers import ( + serialize_bulk_content_files, + serialize_bulk_learning_resources, +) from main.utils import now_in_utc from vector_search.constants import CONTENT_FILE_PREPASS_PAYLOAD_FIELDS from vector_search.tasks import ( + _healthcheck_alert_count, _record_embedding_failure, _retry_countdown, + _sentry_healthcheck_log, embed_learning_resources_by_id, embed_new_content_files, embed_new_learning_resources, embed_run_content_files, embeddings_healthcheck, + embeddings_healthcheck_content_files, + embeddings_healthcheck_resource_embeddings, finalize_embeddings, generate_embeddings, remove_embeddings, remove_run_content_files, remove_unpublished_run_content_files, start_embed_resources, + summaries_healthcheck, ) from vector_search.utils import vector_point_id, vector_point_key @@ -998,80 +1007,518 @@ def test_embed_run_content_files_all_unchanged_dispatches_nothing( mocked_celery.chain.assert_not_called() -def test_embeddings_healthcheck_no_missing_embeddings(mocker): +def _missing_everything(batch, collection_name=None): + """Report every point in the batch as absent from Qdrant.""" + return list(batch) + + +def _missing_only(collection): + """Report every point absent for one collection, nothing missing elsewhere.""" + + def fake_filter(batch, collection_name=None): + return list(batch) if collection_name == collection else [] + + return fake_filter + + +def test_embeddings_healthcheck_dispatches_batches_of_each_kind(mocker, mocked_celery): + """ + embeddings_healthcheck should dispatch batches of resources and batches of content + files, each drawn from its own rows, plus the standalone summaries task """ - Test embeddings_healthcheck when there are no missing embeddings + resources = LearningResourceFactory.create_batch(5, published=True) + content_files = [ + ContentFileFactory.create( + run=None, learning_resource=resource, content="test", published=True + ) + for resource in resources + ] + LearningResourceFactory.create_batch(2, published=False, test_mode=False) + expected_ids = sorted( + LearningResource.objects.filter( + Q(published=True) | Q(test_mode=True) + ).values_list("id", flat=True) + ) + mocker.patch("vector_search.tasks.HEALTHCHECK_RESOURCE_BATCH_SIZE", 2) + mocker.patch("vector_search.tasks.HEALTHCHECK_CONTENT_FILE_BATCH_SIZE", 2) + mock_resource_check = mocker.patch( + "vector_search.tasks.embeddings_healthcheck_resource_embeddings", autospec=True + ) + mock_content_check = mocker.patch( + "vector_search.tasks.embeddings_healthcheck_content_files", autospec=True + ) + mock_summaries = mocker.patch( + "vector_search.tasks.summaries_healthcheck", autospec=True + ) + + with pytest.raises(mocked_celery.replace_exception_class): + embeddings_healthcheck.delay() + + # the resource check is batched, and every eligible resource lands in a batch + resource_batches = [call.args[0] for call in mock_resource_check.si.mock_calls] + assert all(len(batch) <= 2 for batch in resource_batches) + assert sorted(rid for batch in resource_batches for rid in batch) == expected_ids + # the content-file check is batched over content files, not over their resources + content_batches = [call.args[0] for call in mock_content_check.si.mock_calls] + assert all(len(batch) <= 2 for batch in content_batches) + assert sorted(cid for batch in content_batches for cid in batch) == sorted( + cf.id for cf in content_files + ) + assert mocked_celery.group.call_count == 1 + assert mocked_celery.replace.call_args[0][1] == mocked_celery.group.return_value + assert mocked_celery.group.call_args[0][0][0] is mock_summaries.si.return_value + # every dispatched task shares one run_key, so they share one Sentry alert budget + run_keys = ( + {call.kwargs["run_key"] for call in mock_resource_check.si.mock_calls} + | {call.kwargs["run_key"] for call in mock_content_check.si.mock_calls} + | {call.kwargs["run_key"] for call in mock_summaries.si.mock_calls} + ) + assert len(run_keys) == 1 + assert next(iter(run_keys)) is not None + + +def test_embeddings_healthcheck_dispatch_only_queues_checkable_content_files( + mocker, mocked_celery +): + """ + Only content files the check could report on are queued -- published, with + content, belonging to an eligible resource. Anything else is a batch slot spent on + a file the check would skip, and a resource with no content files at all never + costs a task. + """ + published_resource = LearningResourceFactory.create( + published=True, create_runs=False + ) + run = LearningResourceRunFactory.create( + published=True, learning_resource=published_resource + ) + by_run = ContentFileFactory.create(run=run, content="test", published=True) + by_resource = ContentFileFactory.create( + run=None, learning_resource=published_resource, content="test", published=True + ) + + # none of these should reach a content-file task + ContentFileFactory.create( + run=None, learning_resource=published_resource, content="test", published=False + ) + ContentFileFactory.create( + run=None, learning_resource=published_resource, content="", published=True + ) + ContentFileFactory.create( + run=None, learning_resource=published_resource, content=None, published=True + ) + unpublished_resource = LearningResourceFactory.create( + published=False, test_mode=False, create_runs=False + ) + ContentFileFactory.create( + run=None, learning_resource=unpublished_resource, content="test", published=True + ) + # a resource with no content files at all costs no content-file task + LearningResourceFactory.create(published=True, create_runs=False) + + mock_content_check = mocker.patch( + "vector_search.tasks.embeddings_healthcheck_content_files", autospec=True + ) + mocker.patch( + "vector_search.tasks.embeddings_healthcheck_resource_embeddings", autospec=True + ) + mocker.patch("vector_search.tasks.summaries_healthcheck", autospec=True) + + with pytest.raises(mocked_celery.replace_exception_class): + embeddings_healthcheck.delay() + + queued_ids = sorted( + cid for call in mock_content_check.si.mock_calls for cid in call.args[0] + ) + assert queued_ids == sorted([by_run.id, by_resource.id]) + + +def test_embeddings_healthcheck_caps_a_direct_invocation(mocker, mocked_celery): + """ + A direct call -- embeddings_healthcheck() in a shell rather than delay() -- has no + task id. Without a generated fallback every dispatched task would get + run_key=None, and one manual run would send an uncapped alert per affected + resource. + """ + LearningResourceFactory.create_batch(3, published=True) + mock_resource_check = mocker.patch( + "vector_search.tasks.embeddings_healthcheck_resource_embeddings", autospec=True + ) + mocker.patch( + "vector_search.tasks.embeddings_healthcheck_content_files", autospec=True + ) + mocker.patch("vector_search.tasks.summaries_healthcheck", autospec=True) + + with pytest.raises(mocked_celery.replace_exception_class): + embeddings_healthcheck() + + run_keys = {call.kwargs["run_key"] for call in mock_resource_check.si.mock_calls} + assert len(run_keys) == 1 + assert next(iter(run_keys)) is not None + + +def test_embeddings_healthcheck_content_files_no_missing_embeddings(mocker): + """ + A content-file check should stay silent when Qdrant has every point it checks """ lr = LearningResourceFactory.create(published=True) LearningResourceRunFactory.create(published=True, learning_resource=lr) - ContentFileFactory.create(run=lr.runs.first(), content="test", published=True) + cf = ContentFileFactory.create(run=lr.runs.first(), content="test", published=True) mock_sentry = mocker.patch("vector_search.tasks.sentry_sdk", autospec=True) mocker.patch( "vector_search.tasks.filter_existing_qdrant_points_by_ids", return_value=[] ) - embeddings_healthcheck() + embeddings_healthcheck_content_files([cf.id]) assert mock_sentry.capture_message.call_count == 0 -def test_embeddings_healthcheck_missing_both(mocker): +def test_embeddings_healthcheck_resource_embeddings_no_missing(mocker): + """ + A batched resource check should stay silent when Qdrant has every point + """ + resources = LearningResourceFactory.create_batch(3, published=True) + mock_log = mocker.patch("vector_search.tasks._sentry_healthcheck_log") + mocker.patch( + "vector_search.tasks.filter_existing_qdrant_points_by_ids", return_value=[] + ) + + embeddings_healthcheck_resource_embeddings([lr.id for lr in resources]) + assert mock_log.call_count == 0 + + +def test_embeddings_healthcheck_resource_embeddings_batches_qdrant_lookups(mocker): + """ + A batch must cost one Qdrant lookup per HEALTHCHECK_POINT_BATCH_SIZE points rather + than one per resource: checking resources one at a time is what turned a full run + into thousands of round trips. + """ + resources = LearningResourceFactory.create_batch(5, published=True) + mocker.patch("vector_search.tasks.HEALTHCHECK_POINT_BATCH_SIZE", 2) + mock_filter = mocker.patch( + "vector_search.tasks.filter_existing_qdrant_points_by_ids", return_value=[] + ) + + embeddings_healthcheck_resource_embeddings([lr.id for lr in resources]) + + # 5 points at 2 per lookup is 3 lookups, not 5 + assert mock_filter.call_count == 3 + + +def test_embeddings_healthcheck_resource_embeddings_reports_the_batch_once(mocker): + """ + A batch sends one alert listing the resources it found, rather than one alert per + resource, so a batch costs a single slice of the per-run cap + """ + resources = LearningResourceFactory.create_batch(3, published=True) + mocker.patch( + "vector_search.tasks.filter_existing_qdrant_points_by_ids", + side_effect=_missing_everything, + ) + mock_log = mocker.patch("vector_search.tasks._sentry_healthcheck_log") + + embeddings_healthcheck_resource_embeddings([lr.id for lr in resources]) + + assert mock_log.call_count == 1 + assert mock_log.mock_calls[0].args[1] == "missing_learning_resource_embeddings" + context = mock_log.mock_calls[0].args[2] + assert context["count"] == len(resources) + assert sorted(context["ids"]) == sorted(lr.id for lr in resources) + # the readable ids identify the resources without needing a lookup from Sentry + assert sorted(context["readable_ids"]) == sorted(lr.readable_id for lr in resources) + + +def test_embeddings_healthcheck_resource_embeddings_ignores_present_resources(mocker): + """ + Only the resources Qdrant is actually missing are reported, so a batch can't + implicate the resources it merely shared a task with + """ + present, missing = LearningResourceFactory.create_batch(2, published=True) + missing_point_ids = [ + vector_point_id(vector_point_key(serialized)) + for serialized in serialize_bulk_learning_resources([missing.id]) + ] + mocker.patch( + "vector_search.tasks.filter_existing_qdrant_points_by_ids", + side_effect=lambda point_ids, **_: [ + p for p in point_ids if p in missing_point_ids + ], + ) + mock_log = mocker.patch("vector_search.tasks._sentry_healthcheck_log") + + embeddings_healthcheck_resource_embeddings([present.id, missing.id]) + + assert mock_log.call_count == 1 + assert mock_log.mock_calls[0].args[2]["ids"] == [missing.id] + + +def test_embeddings_healthcheck_reports_both_kinds_of_gap(mocker): """ - Test embeddings_healthcheck when there are missing content files and learning resources + The two checks together report both the missing content files and the missing + resource embeddings, each under its own alert type carrying its own ids """ lr = LearningResourceFactory.create(published=True, create_runs=False) LearningResourceRunFactory.create(published=True, learning_resource=lr) cf = ContentFileFactory.create(run=lr.runs.first(), content="test", published=True) mocker.patch( "vector_search.tasks.filter_existing_qdrant_points_by_ids", - side_effect=[ - [vector_point_id(lr.readable_id)], - [ - vector_point_id( - f"{cf.run.learning_resource.id}.{cf.run.run_id}.{cf.key}.0" - ) - ], - ], + side_effect=_missing_everything, ) - mock_sentry = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + mock_log = mocker.patch("vector_search.tasks._sentry_healthcheck_log") - embeddings_healthcheck() + embeddings_healthcheck_content_files([cf.id]) + embeddings_healthcheck_resource_embeddings([lr.id]) - assert mock_sentry.call_count == 2 + assert mock_log.call_count == 2 + by_alert_type = {call.args[1]: call.args[2] for call in mock_log.mock_calls} + assert by_alert_type["missing_content_file_embeddings"]["ids"] == [cf.id] + assert by_alert_type["missing_learning_resource_embeddings"]["ids"] == [lr.id] + # each alert carries the ids it found rather than a count alone + for context in by_alert_type.values(): + assert context["count"] == 1 -def test_embeddings_healthcheck_checks_all_runs(mocker): +def test_embeddings_healthcheck_content_files_checks_all_runs(mocker): """ - embeddings_healthcheck should check content files from every run, not just best_run + A content-file check should check content files from every run, not just best_run """ from vector_search.constants import CONTENT_FILES_COLLECTION_NAME lr = LearningResourceFactory.create(published=True, create_runs=False) run_a = LearningResourceRunFactory.create(published=True, learning_resource=lr) run_b = LearningResourceRunFactory.create(published=True, learning_resource=lr) - ContentFileFactory.create(run=run_a, content="test", published=True) - ContentFileFactory.create(run=run_b, content="test", published=True) + cf_a = ContentFileFactory.create(run=run_a, content="test", published=True) + cf_b = ContentFileFactory.create(run=run_b, content="test", published=True) - def fake_filter(batch, collection_name=None): - # report every content file point as missing, no missing resources - return list(batch) if collection_name == CONTENT_FILES_COLLECTION_NAME else [] + mocker.patch( + "vector_search.tasks.filter_existing_qdrant_points_by_ids", + side_effect=_missing_only(CONTENT_FILES_COLLECTION_NAME), + ) + mock_log = mocker.patch("vector_search.tasks._sentry_healthcheck_log") + + embeddings_healthcheck_content_files([cf_a.id, cf_b.id]) + + assert mock_log.call_count == 1 + assert mock_log.mock_calls[0].args[1] == "missing_content_file_embeddings" + context = mock_log.mock_calls[0].args[2] + assert context["count"] == 2 + assert sorted(context["ids"]) == sorted([cf_a.id, cf_b.id]) + + +def test_embeddings_healthcheck_content_files_ignores_files_outside_its_batch(mocker): + """ + A batch only reports the content files it was given, so sibling tasks can't + double-report the same content files + """ + mine = LearningResourceFactory.create(published=True, create_runs=False) + run_mine = LearningResourceRunFactory.create(published=True, learning_resource=mine) + my_cf = ContentFileFactory.create(run=run_mine, content="test", published=True) + + theirs = LearningResourceFactory.create(published=True, create_runs=False) + run_theirs = LearningResourceRunFactory.create( + published=True, learning_resource=theirs + ) + other_cf = ContentFileFactory.create(run=run_theirs, content="test", published=True) mocker.patch( "vector_search.tasks.filter_existing_qdrant_points_by_ids", - side_effect=fake_filter, + side_effect=_missing_everything, ) - mock_sentry = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + mock_log = mocker.patch("vector_search.tasks._sentry_healthcheck_log") - embeddings_healthcheck() + embeddings_healthcheck_content_files([my_cf.id]) - assert ( - mock_sentry.mock_calls[0].args[0] - == "Warning: 2 missing content file embeddings detected" + reported_file_ids = { + file_id + for call in mock_log.mock_calls + for file_id in call.args[2].get("ids", []) + } + assert reported_file_ids == {my_cf.id} + assert other_cf.id not in reported_file_ids + + +def test_embeddings_healthcheck_sentry_messages_are_count_free(mocker): + """ + Sentry messages must not interpolate counts or ids, or the per-resource + reports each become a separate Sentry issue instead of grouped occurrences + of one. Details live in the context, which is keyed by alert_type. + """ + lr = LearningResourceFactory.create(published=True, create_runs=False) + LearningResourceRunFactory.create(published=True, learning_resource=lr) + cf = ContentFileFactory.create(run=lr.runs.first(), content="test", published=True) + mocker.patch( + "vector_search.tasks.filter_existing_qdrant_points_by_ids", + side_effect=_missing_everything, ) + mock_scope = mocker.MagicMock() + mocker.patch( + "vector_search.tasks.sentry_sdk.new_scope" + ).return_value.__enter__.return_value = mock_scope + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + embeddings_healthcheck_content_files([cf.id]) + embeddings_healthcheck_resource_embeddings([lr.id]) + + messages = [call.args[0] for call in mock_capture.mock_calls] + assert set(messages) == { + "Warning: content files are missing embeddings", + "Warning: learning resources are missing their embeddings", + } + assert not any(char.isdigit() for message in messages for char in message) + # context key tracks the alert, so resource alerts aren't filed under a + # content-file key + context_keys = [call.args[0] for call in mock_scope.set_context.mock_calls] + assert sorted(context_keys) == [ + "missing_content_file_embeddings", + "missing_learning_resource_embeddings", + ] + + +def test_sentry_healthcheck_log_caps_alerts_per_run(mocker, embed_cache, settings): + """ + An environment that is simply behind on embedding produces one alert per + affected resource, so the per-run cap must stop sending after the limit rather + than spending the whole Sentry quota on one expected condition. + """ + settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP = 3 + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + for _ in range(10): + _sentry_healthcheck_log( + "embeddings", "missing_thing", {"a": 1}, "Warning: thing", run_key="run-1" + ) + + messages = [call.args[0] for call in mock_capture.mock_calls] + # 3 real alerts, then exactly one notice that the cap engaged, then silence + assert messages == [ + "Warning: thing", + "Warning: thing", + "Warning: thing", + "Warning: healthcheck alerts suppressed after reaching the per-run cap", + ] + + +def test_sentry_healthcheck_log_cap_is_per_alert_type(mocker, embed_cache, settings): + """ + Each alert type gets its own budget, so a flood of one kind can't hide a + different kind of failure entirely. A run works through resources for hours, so a + shared budget would go to whichever check found something first. + """ + settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP = 1 + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + for _ in range(50): + _sentry_healthcheck_log( + "embeddings", "type_a", {}, "Warning: a", run_key="run-1" + ) + _sentry_healthcheck_log("embeddings", "type_b", {}, "Warning: b", run_key="run-1") + + messages = [call.args[0] for call in mock_capture.mock_calls] + assert messages.count("Warning: a") == 1 + assert messages.count("Warning: b") == 1 + + +def test_sentry_healthcheck_log_notifies_once_per_capped_type( + mocker, embed_cache, settings +): + """ + Each capped type reports that it was capped exactly once, so a capped run is + never mistaken for a clean one and isn't itself a flood + """ + settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP = 1 + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + for alert_type in ("type_a", "type_b"): + for _ in range(5): + _sentry_healthcheck_log( + "embeddings", alert_type, {}, f"Warning: {alert_type}", run_key="run-1" + ) + + messages = [call.args[0] for call in mock_capture.mock_calls] + suppressed = "Warning: healthcheck alerts suppressed after reaching the per-run cap" + assert messages.count(suppressed) == 2 + + +def test_healthcheck_alert_count_does_not_reset_on_concurrent_create( + mocker, embed_cache +): + """ + Workers racing to create a run's counter must not each be handed a count of 1: + the healthcheck fans out across workers, so a lost count means the cap overshoots + by however many raced. + """ + real_incr = embed_cache.incr + raises_left = 2 + + def racing_incr(*args, **kwargs): + # both racers check the counter before either has created it, so both see it + # as absent; afterwards the key really does exist + nonlocal raises_left + if raises_left: + raises_left -= 1 + raise ValueError + return real_incr(*args, **kwargs) + + mocker.patch.object(embed_cache, "incr", side_effect=racing_incr) + + counts = [_healthcheck_alert_count("run-1", "type_a") for _ in range(2)] + + # the racer that won add() counts 1; the loser falls through to a real incr + # rather than resetting the counter and counting 1 as well + assert counts == [1, 2] + + +def test_sentry_healthcheck_log_cap_is_per_run(mocker, embed_cache, settings): + """ + The budget resets between runs, so a capped run doesn't silence the next one + """ + settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP = 1 + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + for run_key in ("run-1", "run-2"): + for _ in range(3): + _sentry_healthcheck_log( + "embeddings", "missing_thing", {}, "Warning: thing", run_key=run_key + ) + + messages = [call.args[0] for call in mock_capture.mock_calls] + assert messages.count("Warning: thing") == 2 + + +@pytest.mark.parametrize("cap", [0, -1]) +def test_sentry_healthcheck_log_cap_disabled(mocker, embed_cache, settings, cap): + """ + A cap of 0 or less disables capping, for environments (production) where a + large backlog is a real incident rather than expected drift + """ + settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP = cap + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + for _ in range(10): + _sentry_healthcheck_log( + "embeddings", "missing_thing", {}, "Warning: thing", run_key="run-1" + ) + + assert mock_capture.call_count == 10 + + +def test_sentry_healthcheck_log_uncapped_without_run_key(mocker, embed_cache, settings): + """ + Callers with no run_key (direct invocations) are uncapped, so the cap can't + silently swallow one-off checks + """ + settings.EMBEDDINGS_HEALTHCHECK_ALERT_CAP = 1 + mock_capture = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") + + for _ in range(5): + _sentry_healthcheck_log("embeddings", "missing_thing", {}, "Warning: thing") + + assert mock_capture.call_count == 5 -def test_embeddings_healthcheck_missing_summaries(mocker): +def test_summaries_healthcheck_missing_summaries(mocker): """ - Test embeddings_healthcheck for missing contentfile summaries/flashcards + Test summaries_healthcheck for missing contentfile summaries/flashcards """ content_extension = [".srt"] content_type = ["file"] @@ -1107,17 +1554,17 @@ def test_embeddings_healthcheck_missing_summaries(mocker): ) mock_sentry = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") - embeddings_healthcheck() + summaries_healthcheck() assert mock_sentry.call_count == 1 assert ( mock_sentry.mock_calls[0].args[0] - == "Warning: 1 missing content file summaries detected" + == "Warning: missing content file summaries detected" ) -def test_embeddings_healthcheck_excludes_already_summarized(mocker): +def test_summaries_healthcheck_excludes_already_summarized(mocker): """ - embeddings_healthcheck should not count content files that already have + summaries_healthcheck should not count content files that already have a summary as missing (regression test for passing overwrite=True implicitly by mis-ordering get_unprocessed_content_file_ids arguments) """ @@ -1156,13 +1603,13 @@ def test_embeddings_healthcheck_excludes_already_summarized(mocker): ) mock_sentry = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") - embeddings_healthcheck() + summaries_healthcheck() assert mock_sentry.call_count == 0 -def test_embeddings_healthcheck_summaries_scoped_to_require_summaries(mocker): +def test_summaries_healthcheck_scoped_to_require_summaries(mocker): """ - embeddings_healthcheck should only count missing summaries for learning + summaries_healthcheck should only count missing summaries for learning resources that require them, not every learning resource (regression test for get_unprocessed_content_file_ids never receiving learning_resource_ids) @@ -1201,7 +1648,7 @@ def test_embeddings_healthcheck_summaries_scoped_to_require_summaries(mocker): ) mock_sentry = mocker.patch("vector_search.tasks.sentry_sdk.capture_message") - embeddings_healthcheck() + summaries_healthcheck() assert mock_sentry.call_count == 0 diff --git a/vector_search/utils.py b/vector_search/utils.py index ba3d574944..b61ecc1247 100644 --- a/vector_search/utils.py +++ b/vector_search/utils.py @@ -1587,11 +1587,15 @@ def filter_existing_qdrant_points_by_ids( Return only points that dont exist in qdrant """ client = qdrant_client() + # existence check only: payloads/vectors would be fetched and discarded, and for + # content files the payload carries the chunked text response = client.retrieve( collection_name=collection_name, ids=point_ids, + with_payload=False, + with_vectors=False, ) - existing = [record.id for record in response] + existing = {record.id for record in response} return [point_id for point_id in point_ids if point_id not in existing] From 93a7bbe78db09c135a0a62c5826c6b5317a205a7 Mon Sep 17 00:00:00 2001 From: Shankar Ambady Date: Tue, 18 Aug 2026 11:36:32 -0400 Subject: [PATCH 2/6] Allow for null course ids in webhook payload (#3776) * allow null in webhook serializer * serializer and spec update * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- frontends/api/src/generated/v1/api.ts | 28 ++++++++++++------------- openapi/specs/v1.yaml | 6 ++++++ webhooks/serializers.py | 6 ++++-- webhooks/views_test.py | 30 +++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 186b779090..1c4f7a43b6 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -595,13 +595,13 @@ export interface ContentFileWebHookRequest { * @type {string} * @memberof ContentFileWebHookRequest */ - course_id?: string + course_id?: string | null /** * * @type {string} * @memberof ContentFileWebHookRequest */ - course_readable_id?: string + course_readable_id?: string | null } /** @@ -627,13 +627,13 @@ export interface ContentFileWebHookRequestRequest { * @type {string} * @memberof ContentFileWebHookRequestRequest */ - course_id?: string + course_id?: string | null /** * * @type {string} * @memberof ContentFileWebHookRequestRequest */ - course_readable_id?: string + course_readable_id?: string | null } /** @@ -33656,8 +33656,8 @@ export const WebhooksApiAxiosParamCreator = function ( * @param {WebhooksContentFilesCreateSourceEnum} source * `mit_edx` - mit_edx * `mitpe` - mitpe * `mitxonline` - mitxonline * `oll` - oll * `ocw` - ocw * `podcast` - podcast * `mit_climate` - mit_climate * `see` - see * `xpro` - xpro * `youtube` - youtube * `canvas` - canvas * `ovs` - ovs * @param {ContentFileWebHookRequestRequest} ContentFileWebHookRequestRequest * @param {string} [content_path] - * @param {string} [course_id] - * @param {string} [course_readable_id] + * @param {string | null} [course_id] + * @param {string | null} [course_readable_id] * @param {*} [options] Override http request option. * @throws {RequiredError} */ @@ -33665,8 +33665,8 @@ export const WebhooksApiAxiosParamCreator = function ( source: WebhooksContentFilesCreateSourceEnum, ContentFileWebHookRequestRequest: ContentFileWebHookRequestRequest, content_path?: string, - course_id?: string, - course_readable_id?: string, + course_id?: string | null, + course_readable_id?: string | null, options: RawAxiosRequestConfig = {}, ): Promise => { // verify required parameter 'source' is not null or undefined @@ -33845,8 +33845,8 @@ export const WebhooksApiFp = function (configuration?: Configuration) { * @param {WebhooksContentFilesCreateSourceEnum} source * `mit_edx` - mit_edx * `mitpe` - mitpe * `mitxonline` - mitxonline * `oll` - oll * `ocw` - ocw * `podcast` - podcast * `mit_climate` - mit_climate * `see` - see * `xpro` - xpro * `youtube` - youtube * `canvas` - canvas * `ovs` - ovs * @param {ContentFileWebHookRequestRequest} ContentFileWebHookRequestRequest * @param {string} [content_path] - * @param {string} [course_id] - * @param {string} [course_readable_id] + * @param {string | null} [course_id] + * @param {string | null} [course_readable_id] * @param {*} [options] Override http request option. * @throws {RequiredError} */ @@ -33854,8 +33854,8 @@ export const WebhooksApiFp = function (configuration?: Configuration) { source: WebhooksContentFilesCreateSourceEnum, ContentFileWebHookRequestRequest: ContentFileWebHookRequestRequest, content_path?: string, - course_id?: string, - course_readable_id?: string, + course_id?: string | null, + course_readable_id?: string | null, options?: RawAxiosRequestConfig, ): Promise< ( @@ -34052,14 +34052,14 @@ export interface WebhooksApiWebhooksContentFilesCreateRequest { * @type {string} * @memberof WebhooksApiWebhooksContentFilesCreate */ - readonly course_id?: string + readonly course_id?: string | null /** * * @type {string} * @memberof WebhooksApiWebhooksContentFilesCreate */ - readonly course_readable_id?: string + readonly course_readable_id?: string | null } /** diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index a052fcef65..b972580b41 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -9864,10 +9864,12 @@ paths: name: course_id schema: type: string + nullable: true - in: query name: course_readable_id schema: type: string + nullable: true - in: query name: source schema: @@ -10445,8 +10447,10 @@ components: $ref: '#/components/schemas/SourceEnum' course_id: type: string + nullable: true course_readable_id: type: string + nullable: true required: - source ContentFileWebHookRequestRequest: @@ -10459,8 +10463,10 @@ components: $ref: '#/components/schemas/SourceEnum' course_id: type: string + nullable: true course_readable_id: type: string + nullable: true required: - source Course: diff --git a/webhooks/serializers.py b/webhooks/serializers.py index 9438d77c7e..ecbb272849 100644 --- a/webhooks/serializers.py +++ b/webhooks/serializers.py @@ -18,8 +18,10 @@ class ContentFileWebHookRequestSerializer(serializers.Serializer): content_path = serializers.CharField(required=False, allow_blank=True) source_choices = [(e.name.lower(), e.value) for e in ETLSource] source = serializers.ChoiceField(choices=source_choices) - course_id = serializers.CharField(required=False, allow_blank=True) - course_readable_id = serializers.CharField(required=False, allow_blank=True) + course_id = serializers.CharField(required=False, allow_blank=True, allow_null=True) + course_readable_id = serializers.CharField( + required=False, allow_blank=True, allow_null=True + ) class OVSVideoWebhookRequestSerializer(serializers.Serializer): diff --git a/webhooks/views_test.py b/webhooks/views_test.py index cb24298e40..f40b5da1af 100644 --- a/webhooks/views_test.py +++ b/webhooks/views_test.py @@ -185,6 +185,36 @@ def test_content_file_webhook_view_canvas_success(settings, client, mocker): mock_ingest.assert_called_once_with(["/path/to/canvas/course.tar.gz", False]) +@pytest.mark.django_db +@pytest.mark.parametrize( + ("course_id", "course_readable_id"), + [(2198, None), (None, "2198")], +) +def test_content_file_webhook_view_canvas_null_id( + settings, client, mocker, course_id, course_readable_id +): + """ + Test ContentFileWebhookView accepts Canvas create webhooks with null IDs + """ + url = reverse("webhooks:v1:content_file_webhook") + mock_ingest = mocker.patch("webhooks.views.ingest_canvas_course.apply_async") + + data = { + "source": ETLSource.canvas.name, + "content_path": "/path/to/canvas/course.tar.gz", + "course_id": course_id, + "course_readable_id": course_readable_id, + } + response = client.post( + url, + data=json.dumps(data), + content_type="application/json", + headers={"X-MITLearn-Signature": get_secret(data, settings)}, + ) + assert response.status_code == 200 + mock_ingest.assert_called_once_with(["/path/to/canvas/course.tar.gz", False]) + + @pytest.mark.django_db @pytest.mark.parametrize( ("etl_source", "content_path", "readable_id"), From 71701ab8b0ba28654e96bf0dadc7d8786cf6d9b2 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Tue, 18 Aug 2026 14:53:21 -0400 Subject: [PATCH 3/6] receipt page (#3717) * add reciept page inital implementation * remove hallucinated "reciept not found" page in favor of traditional 404, more concise and accurate comments * design updates * fix row flattening issue * copilot feedback * add auth gate * handle additional cases where order can be null in receiptMenuItem. * show refunds * accessibility changes * remove accidentally committed memory limit config change * fix back button * update mitxonline api package * fix typecheck --- frontends/api/package.json | 2 +- .../src/mitxonline/hooks/orders/queries.ts | 29 +- .../mitxonline/test-utils/factories/orders.ts | 138 ++++- .../api/src/mitxonline/test-utils/urls.ts | 3 + frontends/main/package.json | 2 +- .../DashboardPage/ContractContent.test.tsx | 8 +- .../DashboardDialogs.test.tsx | 11 +- .../EnrolledCourseCard.test.tsx | 79 ++- .../CoursewareDisplay/EnrolledCourseCard.tsx | 17 +- .../HomeEnrollmentsDisplay.test.tsx | 7 +- .../ProgramEnrollmentCard.test.tsx | 46 +- .../ProgramEnrollmentCard.tsx | 17 +- .../ProgramEnrollmentDisplay.test.tsx | 6 + .../CoursewareDisplay/receiptMenuItem.test.ts | 81 ++- .../CoursewareDisplay/receiptMenuItem.ts | 44 +- .../CoursewareDisplay/test-utils.ts | 62 ++ .../src/app-pages/ReceiptPage/ReceiptCard.tsx | 48 ++ .../ReceiptPage/ReceiptDetailList.tsx | 86 +++ .../ReceiptPage/ReceiptOrderSummary.tsx | 165 ++++++ .../ReceiptPage/ReceiptPage.test.tsx | 552 ++++++++++++++++++ .../src/app-pages/ReceiptPage/ReceiptPage.tsx | 359 ++++++++++++ .../ReceiptPage/ReceiptRedirect.test.tsx | 266 +++++++++ .../app-pages/ReceiptPage/ReceiptRedirect.tsx | 72 +++ .../src/app-pages/ReceiptPage/receiptUtils.ts | 70 +++ .../src/app/(site)/receipt/[orderId]/page.tsx | 26 + .../receipt/by-program/[programId]/page.tsx | 28 + .../(site)/receipt/by-run/[runId]/page.tsx | 28 + .../main/src/app/(site)/receipt/layout.tsx | 22 + .../mitxonline/useOrderIdForResource.ts | 91 +++ frontends/main/src/common/urls.ts | 14 + yarn.lock | 12 +- 31 files changed, 2331 insertions(+), 60 deletions(-) create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptCard.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptDetailList.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptOrderSummary.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptPage.test.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptPage.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.test.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.tsx create mode 100644 frontends/main/src/app-pages/ReceiptPage/receiptUtils.ts create mode 100644 frontends/main/src/app/(site)/receipt/[orderId]/page.tsx create mode 100644 frontends/main/src/app/(site)/receipt/by-program/[programId]/page.tsx create mode 100644 frontends/main/src/app/(site)/receipt/by-run/[runId]/page.tsx create mode 100644 frontends/main/src/app/(site)/receipt/layout.tsx create mode 100644 frontends/main/src/common/mitxonline/useOrderIdForResource.ts diff --git a/frontends/api/package.json b/frontends/api/package.json index 83612e3dac..391af5b986 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -35,7 +35,7 @@ }, "dependencies": { "@mitodl/mit-learn-api-axios": "2026.7.22", - "@mitodl/mitxonline-api-axios": "2026.8.6", + "@mitodl/mitxonline-api-axios": "2026.8.18", "@tanstack/react-query": "^5.66.0", "axios": "^1.12.2", "tiny-invariant": "^1.3.3" diff --git a/frontends/api/src/mitxonline/hooks/orders/queries.ts b/frontends/api/src/mitxonline/hooks/orders/queries.ts index 87304be885..351e65bdbb 100644 --- a/frontends/api/src/mitxonline/hooks/orders/queries.ts +++ b/frontends/api/src/mitxonline/hooks/orders/queries.ts @@ -1,10 +1,19 @@ import { queryOptions } from "@tanstack/react-query" import { ordersApi } from "../../clients" -import type { Order } from "@mitodl/mitxonline-api-axios/v2" +import type { + Order, + OrdersApiOrdersHistoryListRequest, + PaginatedOrderHistoryList, +} from "@mitodl/mitxonline-api-axios/v2" const orderKeys = { root: ["mitxonline", "orders"], receipt: (orderId: number) => [...orderKeys.root, "receipt", orderId], + historyList: (opts: OrdersApiOrdersHistoryListRequest) => [ + ...orderKeys.root, + "history", + opts, + ], } const orderQueries = { @@ -19,6 +28,24 @@ const orderQueries = { .then((res) => res.data) }, }), + /** + * Fulfilled and refunded orders, most recent first. + * + * Enrollments carry no reference to their order, so getting from a run or + * program to its receipt means searching these lines — see + * `useOrderIdForRun` / `useOrderIdForProgram`. + * + * `opts` is required rather than defaulted: older deployments drop the + * pagination envelope when `limit` is absent and return a bare array, which + * would not match the declared return type. + */ + historyList: (opts: OrdersApiOrdersHistoryListRequest) => + queryOptions({ + queryKey: orderKeys.historyList(opts), + queryFn: async (): Promise => { + return ordersApi.ordersHistoryList(opts).then((res) => res.data) + }, + }), } export { orderQueries, orderKeys } diff --git a/frontends/api/src/mitxonline/test-utils/factories/orders.ts b/frontends/api/src/mitxonline/test-utils/factories/orders.ts index b5b004e4da..54051fe95b 100644 --- a/frontends/api/src/mitxonline/test-utils/factories/orders.ts +++ b/frontends/api/src/mitxonline/test-utils/factories/orders.ts @@ -1,5 +1,17 @@ import { faker } from "@faker-js/faker/locale/en" -import type { Order, TransactionLine } from "@mitodl/mitxonline-api-axios/v2" +import type { + Line, + Nested, + Order, + OrderHistory, + OrderRefundsInner, + OrderStreetAddress, + OrderTransactions, + PaginatedOrderHistoryList, + Product, + RedeemedDiscount, + TransactionLine, +} from "@mitodl/mitxonline-api-axios/v2" const transactionLine = ( overrides: Partial = {}, @@ -17,19 +29,135 @@ const transactionLine = ( ...overrides, }) +const orderTransactions = ( + overrides: Partial = {}, +): OrderTransactions => ({ + card_number: `xxxxxxxxxxxx${faker.string.numeric(4)}`, + card_type: "Visa", + name: faker.person.fullName(), + bill_to_email: faker.internet.email(), + payment_method: "card", + ...overrides, +}) + +const orderStreetAddress = ( + overrides: Partial = {}, +): OrderStreetAddress => ({ + line: [faker.location.streetAddress()], + postal_code: faker.location.zipCode(), + state: faker.location.state({ abbreviated: true }), + city: faker.location.city(), + country: "US", + ...overrides, +}) + +const redeemedDiscount = ( + overrides: Partial = {}, +): RedeemedDiscount => ({ + redeemed_discount: { + id: faker.number.int(), + created_on: faker.date.past().toISOString(), + updated_on: faker.date.past().toISOString(), + amount: faker.commerce.price({ min: 5, max: 50 }), + discount_type: "dollars-off", + redemption_type: "one-time", + discount_code: faker.string.alphanumeric(12), + ...overrides, + }, +}) + +const orderRefund = ( + overrides: Partial = {}, +): OrderRefundsInner => ({ + amount: Number(faker.commerce.price({ min: 5, max: 500 })), + date: faker.date.past().toISOString(), + ...overrides, +}) + const order = (overrides: Partial = {}): Order => ({ id: faker.number.int(), state: "fulfilled", - purchaser: [], + purchaser: { + country: "US", + email: faker.internet.email(), + }, total_price_paid: faker.commerce.price({ min: 50, max: 500 }), lines: [transactionLine()], discounts: [], refunds: [], reference_number: faker.string.alphanumeric(10), created_on: faker.date.past().toISOString(), - transactions: {}, - street_address: {}, + transactions: orderTransactions(), + street_address: orderStreetAddress(), + refund_eligible: false, ...overrides, }) -export { order, transactionLine } +/** + * The default `purchasable_object` has only an `id`, which matches no variant — + * pass a shaped object (with `course`, or neither `course` nor `run_tag`) when the + * test needs it to resolve. + */ +const product = (overrides: Partial = {}): Product => ({ + id: faker.number.int(), + price: faker.commerce.price({ min: 50, max: 500 }), + description: faker.commerce.productDescription(), + is_active: true, + purchasable_object: { id: faker.number.int() }, + ...overrides, +}) + +const line = (overrides: Partial = {}): Line => { + const unitPrice = faker.commerce.price({ min: 50, max: 500 }) + return { + id: faker.number.int(), + quantity: 1, + item_description: faker.commerce.productName(), + unit_price: unitPrice, + total_price: unitPrice, + product: product(), + ...overrides, + } +} + +const orderHistory = (overrides: Partial = {}): OrderHistory => ({ + id: faker.number.int(), + state: "fulfilled", + reference_number: faker.string.alphanumeric(10), + purchaser: { + id: faker.number.int(), + name: faker.person.fullName(), + created_on: faker.date.past().toISOString(), + updated_on: faker.date.past().toISOString(), + }, + total_price_paid: faker.commerce.price({ min: 50, max: 500 }), + lines: [line()], + created_on: faker.date.past().toISOString(), + titles: [], + updated_on: faker.date.past().toISOString(), + refund_eligible: false, + ...overrides, +}) + +const orderHistoryList = ( + results: OrderHistory[], + opts: { count?: number; next?: string | null; previous?: string | null } = {}, +): PaginatedOrderHistoryList => ({ + count: opts.count ?? results.length, + next: opts.next ?? null, + previous: opts.previous ?? null, + results, +}) + +export { + order, + orderHistory, + orderHistoryList, + orderRefund, + orderStreetAddress, + orderTransactions, + line, + product, + redeemedDiscount, + transactionLine, +} diff --git a/frontends/api/src/mitxonline/test-utils/urls.ts b/frontends/api/src/mitxonline/test-utils/urls.ts index 3983dc05b6..6009de6460 100644 --- a/frontends/api/src/mitxonline/test-utils/urls.ts +++ b/frontends/api/src/mitxonline/test-utils/urls.ts @@ -3,6 +3,7 @@ import type { CoursesApiCourseVariantRunsV3Request, CourseCertificatesApiCourseCertificatesRetrieveRequest, ProgramCertificatesApiProgramCertificatesRetrieveRequest, + OrdersApiOrdersHistoryListRequest, ProgramCollectionsApiProgramCollectionsListRequest, ProgramsApiProgramsListV2Request, } from "@mitodl/mitxonline-api-axios/v2" @@ -155,6 +156,8 @@ const baskets = { const orders = { receipt: (orderId: number) => `${getApiBaseUrl()}/api/v0/orders/receipt/${orderId}/`, + historyList: (params?: OrdersApiOrdersHistoryListRequest) => + `${getApiBaseUrl()}/api/v0/orders/history/${queryify(params)}`, } const verifiedProgramEnrollments = { diff --git a/frontends/main/package.json b/frontends/main/package.json index 152a687380..d4fe006d6b 100644 --- a/frontends/main/package.json +++ b/frontends/main/package.json @@ -18,7 +18,7 @@ "@mitodl/arithmix": "^0.2.3", "@mitodl/course-search-utils": "^3.5.2", "@mitodl/hacksnack": "^0.1.1", - "@mitodl/mitxonline-api-axios": "2026.8.6", + "@mitodl/mitxonline-api-axios": "2026.8.18", "@mitodl/smoot-design": "6.31.1", "@mui/base": "5.0.0-beta.70", "@mui/material": "^6.4.5", diff --git a/frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx b/frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx index 9b321fa855..5cdeca9724 100644 --- a/frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/ContractContent.test.tsx @@ -12,9 +12,10 @@ import { urls, factories } from "api/mitxonline-test-utils" import { createCoursesWithContractRuns, createTestContracts, + setupOrderHistory, setupOrgAndUser, - setupProgramsAndCourses, setupOrgDashboardMocks, + setupProgramsAndCourses, } from "./CoursewareDisplay/test-utils" import { CourseWithCourseRunsSerializerV2, @@ -26,6 +27,11 @@ import { useFeatureFlagEnabled } from "posthog-js/react" import { FeatureFlags } from "@/common/feature_flags" import { contractAdminView } from "@/common/urls" +// Verified cards look up their order; default to none, tests override. +beforeEach(() => { + setupOrderHistory() +}) + jest.mock("posthog-js/react", () => ({ ...jest.requireActual("posthog-js/react"), useFeatureFlagEnabled: jest.fn(), diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx index c46cbb4a4b..87e6a0eab6 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx @@ -11,7 +11,11 @@ import { import { HomeEnrollmentsDisplay } from "./HomeEnrollmentsDisplay" import { CoursewareCard } from "./CoursewareCard" import { buildCourseEntry } from "./model/dashboardViewModel" -import { dashboardCourse, setupEnrollments } from "./test-utils" +import { + dashboardCourse, + setupEnrollments, + setupOrderHistory, +} from "./test-utils" import * as mitxonline from "api/mitxonline-test-utils" import { urls as testUrls, @@ -29,6 +33,11 @@ import { trackProgramUnenrolled, } from "@/common/analytics/gtm" +// Verified cards look up their order; default to none, tests override. +beforeEach(() => { + setupOrderHistory() +}) + jest.mock("posthog-js/react") jest.mock("@/common/analytics/gtm", () => ({ trackCourseUnenrolled: jest.fn(), diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.test.tsx index 0f829c1d10..01a5438629 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.test.tsx @@ -10,10 +10,17 @@ import { } from "@/test-utils" import * as mitxonline from "api/mitxonline-test-utils" import { mitxonlineLegacyUrl } from "@/common/mitxonline" +import { receiptByRunView, receiptView } from "@/common/urls" import { makeRequest } from "api/test-utils" import { faker } from "@faker-js/faker/locale/en" import moment from "moment" import { EnrolledCourseCard } from "./EnrolledCourseCard" +import { setupOrderHistory } from "./test-utils" + +// Verified cards look up their order; default to none, tests override. +beforeEach(() => { + setupOrderHistory() +}) const EnrollmentMode = { Audit: "audit", @@ -784,12 +791,13 @@ describe.each([ enrollment_mode: EnrollmentMode.Verified, grades: [mitxonline.factories.enrollment.grade({ passed: true })], }) + setupOrderHistory({ runId: enrollment.run.id }) renderWithProviders() await user.click( within(getCard()).getByRole("button", { name: "More options" }), ) expect( - screen.getByRole("menuitem", { name: "Receipt" }), + await screen.findByRole("menuitem", { name: "Receipt" }), ).toBeInTheDocument() }) @@ -809,30 +817,77 @@ describe.each([ ).not.toBeInTheDocument() }) - test("Receipt links to correct MITx Online URL for verified enrollment", async () => { + test("Receipt links to the receipt for the order that paid for the run", async () => { + setupUserApis() + const runId = faker.number.int({ min: 1 }) + setupOrderHistory({ runId, orderId: 87 }) + const enrollment = mitxonline.factories.enrollment.courseEnrollment({ + enrollment_mode: EnrollmentMode.Verified, + grades: [mitxonline.factories.enrollment.grade({ passed: true })], + run: { id: runId }, + }) + + renderWithProviders() + await user.click( + within(getCard()).getByRole("button", { name: "More options" }), + ) + + expect( + await screen.findByRole("menuitem", { name: "Receipt" }), + ).toHaveAttribute("href", receiptView(87)) + }) + + // Verified does not imply a run-level order, so the item must stay hidden. + test("Receipt is hidden for a verified enrollment with no order behind it", async () => { setupUserApis() - const runId = faker.number.int() + const runId = faker.number.int({ min: 1 }) + // An order exists, but for a different run. + setupOrderHistory({ runId: runId + 1, orderId: 87 }) const enrollment = mitxonline.factories.enrollment.courseEnrollment({ enrollment_mode: EnrollmentMode.Verified, grades: [mitxonline.factories.enrollment.grade({ passed: true })], run: { id: runId }, }) - const windowOpenSpy = jest - .spyOn(window, "open") - .mockImplementation(() => null) renderWithProviders() await user.click( within(getCard()).getByRole("button", { name: "More options" }), ) - await user.click(screen.getByRole("menuitem", { name: "Receipt" })) + // Wait for a sibling item so we know the menu has rendered. + await screen.findByRole("menuitem", { name: "Unenroll" }) + + expect( + screen.queryByRole("menuitem", { name: "Receipt" }), + ).not.toBeInTheDocument() + }) + + /** + * An `orders/history` outage must not look the same as "you never paid for this". + * The item stays, pointing at the resolver route, which refetches and renders its + * own skeleton or 404 instead of the option silently vanishing from every card. + */ + test("Receipt falls back to the resolver route when the order lookup fails", async () => { + setupUserApis() + const runId = faker.number.int({ min: 1 }) + setMockResponse.get( + mitxonline.urls.orders.historyList({ limit: 100 }), + "Server error", + { code: 500 }, + ) + const enrollment = mitxonline.factories.enrollment.courseEnrollment({ + enrollment_mode: EnrollmentMode.Verified, + grades: [mitxonline.factories.enrollment.grade({ passed: true })], + run: { id: runId }, + }) - expect(windowOpenSpy).toHaveBeenCalledWith( - mitxonlineLegacyUrl(`/orders/receipt/by-run/${runId}/`), - "_blank", - "noopener,noreferrer", + renderWithProviders() + await user.click( + within(getCard()).getByRole("button", { name: "More options" }), ) - windowOpenSpy.mockRestore() + + expect( + await screen.findByRole("menuitem", { name: "Receipt" }), + ).toHaveAttribute("href", receiptByRunView(runId)) }) }) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.tsx index ef5e89fa63..c137f62973 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.tsx @@ -33,10 +33,11 @@ import { SiblingRunsPanel, SiblingRunsToggle } from "./SiblingRunsAccordion" import { EnrollmentStatusIcon } from "./EnrollmentStatus" import { mitxUserQueries } from "api/mitxonline-hooks/user" import { useQuery } from "@tanstack/react-query" -import { coursePageView } from "@/common/urls" +import { coursePageView, receiptByRunView } from "@/common/urls" import NiceModal from "@ebay/nice-modal-react" import { EmailSettingsDialog, UnenrollDialog } from "./DashboardDialogs" import { getReceiptMenuItem } from "./receiptMenuItem" +import { useOrderIdForRun } from "@/common/mitxonline/useOrderIdForResource" import { CourseRunEnrollmentV3, V3UserProgramEnrollment, @@ -251,6 +252,17 @@ export const EnrolledCourseCard = ({ ) : null const mitxOnlineUser = useQuery(mitxUserQueries.me()) const isStaff = mitxOnlineUser.data?.is_staff + /** + * Only verified enrollments can have a receipt, so the lookup is skipped + * entirely for audit ones. Every card shares one `orders/history` query (same + * cache key), so this is a single request for the whole dashboard rather than + * one per card. + */ + const receiptResolution = useOrderIdForRun( + isVerifiedEnrollmentMode(enrollment?.enrollment_mode) + ? (run?.id ?? null) + : null, + ) const title = isCompact ? course.title : run?.title || course.title const coursewareUrl = run?.courseware_url const certificateLink = getCertificateLink( @@ -455,7 +467,8 @@ export const EnrolledCourseCard = ({ const receiptMenuItem = getReceiptMenuItem( enrollment?.enrollment_mode, - `/orders/receipt/by-run/${enrollment?.run.id}/`, + receiptResolution, + receiptByRunView(run.id), ) if (receiptMenuItem) menuItems.push(receiptMenuItem) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx index 3cc426c761..6ddcd1d085 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx @@ -27,9 +27,14 @@ import { import { HomeEnrollmentsDisplay } from "./HomeEnrollmentsDisplay" import * as mitxonline from "api/mitxonline-test-utils" import { useFeatureFlagEnabled } from "posthog-js/react" -import { setupEnrollments } from "./test-utils" +import { setupEnrollments, setupOrderHistory } from "./test-utils" import { faker } from "@faker-js/faker/locale/en" +// Verified cards look up their order; default to none, tests override. +beforeEach(() => { + setupOrderHistory() +}) + jest.mock("posthog-js/react") const mockedUseFeatureFlagEnabled = jest .mocked(useFeatureFlagEnabled) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.test.tsx index fe38d3abd9..aa2b43178e 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.test.tsx @@ -2,8 +2,15 @@ import React from "react" import { renderWithProviders, screen, user, within } from "@/test-utils" import * as mitxonline from "api/mitxonline-test-utils" import { mitxonlineLegacyUrl } from "@/common/mitxonline" +import { receiptView } from "@/common/urls" import { DisplayModeEnum } from "@mitodl/mitxonline-api-axios/v2" import { ProgramEnrollmentCard } from "./ProgramEnrollmentCard" +import { setupOrderHistory } from "./test-utils" + +// Verified cards look up their order; default to none, tests override. +beforeEach(() => { + setupOrderHistory() +}) describe.each([ { display: "desktop", testId: "enrollment-card-desktop" }, @@ -227,6 +234,7 @@ describe.each([ mitxonline.factories.enrollment.programEnrollmentV3({ enrollment_mode: "verified", }) + setupOrderHistory({ programId: programEnrollment.program.id }) renderWithProviders( , ) @@ -234,7 +242,7 @@ describe.each([ within(getCard()).getByRole("button", { name: "More options" }), ) expect( - screen.getByRole("menuitem", { name: "Receipt" }), + await screen.findByRole("menuitem", { name: "Receipt" }), ).toBeInTheDocument() }) @@ -254,29 +262,45 @@ describe.each([ ).not.toBeInTheDocument() }) - test("Receipt links to correct MITx Online URL for verified program enrollment", async () => { + test("Receipt links to the receipt for the order that paid for the program", async () => { const program = mitxonline.factories.programs.simpleProgram({ id: 99 }) + setupOrderHistory({ programId: 99, orderId: 23 }) const programEnrollment = mitxonline.factories.enrollment.programEnrollmentV3({ program, enrollment_mode: "verified", }) - const windowOpenSpy = jest - .spyOn(window, "open") - .mockImplementation(() => null) renderWithProviders( , ) await user.click( within(getCard()).getByRole("button", { name: "More options" }), ) - await user.click(screen.getByRole("menuitem", { name: "Receipt" })) - expect(windowOpenSpy).toHaveBeenCalledWith( - mitxonlineLegacyUrl("/orders/receipt/by-program/99/"), - "_blank", - "noopener,noreferrer", + expect( + await screen.findByRole("menuitem", { name: "Receipt" }), + ).toHaveAttribute("href", receiptView(23)) + }) + + test("Receipt is hidden for a verified program enrollment with no order", async () => { + const program = mitxonline.factories.programs.simpleProgram({ id: 99 }) + // An order exists, but for a different program. + setupOrderHistory({ programId: 100, orderId: 23 }) + const programEnrollment = + mitxonline.factories.enrollment.programEnrollmentV3({ + program, + enrollment_mode: "verified", + }) + renderWithProviders( + , + ) + await user.click( + within(getCard()).getByRole("button", { name: "More options" }), ) - windowOpenSpy.mockRestore() + await screen.findByRole("menuitem", { name: "Program Record" }) + + expect( + screen.queryByRole("menuitem", { name: "Receipt" }), + ).not.toBeInTheDocument() }) }) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.tsx index 6c3ccc026c..a55713539e 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentCard.tsx @@ -1,5 +1,9 @@ import React from "react" -import { programPageView, programView } from "@/common/urls" +import { + programPageView, + programView, + receiptByProgramView, +} from "@/common/urls" import { DisplayModeEnum, V3UserProgramEnrollment, @@ -25,6 +29,7 @@ import { getCertificateLink } from "./model/dashboardViewModel" import NiceModal from "@ebay/nice-modal-react" import { UnenrollProgramDialog } from "./DashboardDialogs" import { getReceiptMenuItem } from "./receiptMenuItem" +import { useOrderIdForProgram } from "@/common/mitxonline/useOrderIdForResource" import { SimpleMenu, Stack } from "ol-components" import { EnrollmentStatus } from "./helpers" import { ProgressBadge } from "./ProgressBadge" @@ -55,6 +60,13 @@ export const ProgramEnrollmentCard = ({ const upgradedAndIncomplete = isVerifiedEnrollmentMode( programEnrollment.enrollment_mode, ) + /** + * Skipped for audit enrollments, which never have a receipt. Shares one + * `orders/history` query with every other card on the dashboard. + */ + const receiptResolution = useOrderIdForProgram( + upgradedAndIncomplete ? programId : null, + ) const displayMode = program.display_mode const titleSection = ( @@ -114,7 +126,8 @@ export const ProgramEnrollmentCard = ({ } const receiptMenuItem = getReceiptMenuItem( programEnrollment.enrollment_mode, - `/orders/receipt/by-program/${program.id}/`, + receiptResolution, + receiptByProgramView(programId), ) if (receiptMenuItem) menuItems.push(receiptMenuItem) const contextMenu = ( diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx index b60b41b513..b83b05025b 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx @@ -28,10 +28,16 @@ import { import { ProgramEnrollmentDisplay } from "./ProgramEnrollmentDisplay" import * as mitxonline from "api/mitxonline-test-utils" import { makeRequest } from "api/test-utils" +import { setupOrderHistory } from "./test-utils" import { useFeatureFlagEnabled } from "posthog-js/react" import { faker } from "@faker-js/faker/locale/en" import invariant from "tiny-invariant" +// Verified cards look up their order; default to none, tests override. +beforeEach(() => { + setupOrderHistory() +}) + jest.mock("posthog-js/react") const mockedUseFeatureFlagEnabled = jest .mocked(useFeatureFlagEnabled) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.test.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.test.ts index ac90a57595..2df1ab94ba 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.test.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.test.ts @@ -1,11 +1,84 @@ import { getReceiptMenuItem } from "./receiptMenuItem" +import type { OrderIdResolution } from "@/common/mitxonline/useOrderIdForResource" + +const RESOLVER_HREF = "/receipt/by-program/99" + +const resolution = ( + overrides: Partial = {}, +): OrderIdResolution => ({ + isPending: false, + isError: false, + orderId: null, + ...overrides, +}) describe("getReceiptMenuItem", () => { test("returns null when enrollment mode is undefined", () => { - const menuItem = getReceiptMenuItem( - undefined, - "/orders/receipt/by-program/99/", + expect( + getReceiptMenuItem(undefined, resolution({ orderId: 87 }), RESOLVER_HREF), + ).toBeNull() + }) + + test("returns null for audit enrollments, since auditing is free", () => { + expect( + getReceiptMenuItem("audit", resolution({ orderId: 87 }), RESOLVER_HREF), + ).toBeNull() + }) + + test("returns null while the order lookup is still pending", () => { + expect( + getReceiptMenuItem( + "verified", + resolution({ isPending: true }), + RESOLVER_HREF, + ), + ).toBeNull() + }) + + test("links straight to the resolved receipt", () => { + expect( + getReceiptMenuItem( + "verified", + resolution({ orderId: 87 }), + RESOLVER_HREF, + ), + ).toEqual( + expect.objectContaining({ + key: "receipt", + label: "Receipt", + href: "/receipt/87", + }), + ) + }) + + /** + * The lookup succeeded and found nothing — e.g. verified via a program purchase + * that upgraded an existing audit enrollment, which creates no order. + */ + test("hides the item when the lookup found no order", () => { + expect( + getReceiptMenuItem("verified", resolution(), RESOLVER_HREF), + ).toBeNull() + }) + + /** + * A failed lookup must not look like "no receipt", or an `orders/history` outage + * would silently drop Receipt from every card at once. Link to the resolver + * route, which refetches and renders its own skeleton or 404. + */ + test("falls back to the resolver route when the lookup failed", () => { + expect( + getReceiptMenuItem( + "verified", + resolution({ isError: true }), + RESOLVER_HREF, + ), + ).toEqual( + expect.objectContaining({ + key: "receipt", + label: "Receipt", + href: RESOLVER_HREF, + }), ) - expect(menuItem).toBeNull() }) }) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.ts index 3b3e77244d..730d609562 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/receiptMenuItem.ts @@ -1,26 +1,46 @@ import { SimpleMenuItem } from "ol-components" -import { - isVerifiedEnrollmentMode, - mitxonlineLegacyUrl, -} from "@/common/mitxonline" +import { isVerifiedEnrollmentMode } from "@/common/mitxonline" +import { receiptView } from "@/common/urls" +import type { OrderIdResolution } from "@/common/mitxonline/useOrderIdForResource" +/** + * The "Receipt" item for a dashboard card, or null when there is nothing to link + * to. + * + * Verified track alone is not enough — a program purchase can upgrade an existing + * audit enrollment without creating any order, leaving no receipt. + * + * The three reasons `orderId` can be null are deliberately not equivalent: + * + * - still loading — show nothing yet rather than a link that may not work + * - lookup failed — we do not know whether a receipt exists, so link to + * `resolverHref`, which refetches and shows its own skeleton or 404. Hiding the + * item here would make a failing request indistinguishable from "you never paid + * for this", and would silently drop Receipt from every card at once. + * - looked up and found nothing — genuinely no receipt, so hide the item + */ const getReceiptMenuItem = ( enrollmentMode: string | null | undefined, - receiptPath: string, + resolution: OrderIdResolution, + resolverHref: string, ): SimpleMenuItem | null => { if (!enrollmentMode || !isVerifiedEnrollmentMode(enrollmentMode)) return null + if (resolution.isPending) return null + + const href = + resolution.orderId !== null + ? receiptView(resolution.orderId) + : resolution.isError + ? resolverHref + : null + + if (href === null) return null return { className: "dashboard-card-menu-item", key: "receipt", label: "Receipt", - onClick: () => { - window.open( - mitxonlineLegacyUrl(receiptPath), - "_blank", - "noopener,noreferrer", - ) - }, + href, } } diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts index a9269a57a9..ec4cfb8cd4 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts @@ -25,6 +25,67 @@ const makeCourseEnrollment = factories.enrollment.courseEnrollment const makeGrade = factories.enrollment.grade const makeContract = factories.contracts.contract +/** + * Mock the order history that verified enrollment cards fetch to decide whether to + * show a "Receipt" item. Required in any suite rendering a verified enrollment, + * or the unmocked request fails the test. Defaults to an empty history (no + * receipt); pass `runId`/`programId` to make one resolve. + */ +const setupOrderHistory = ({ + runId, + programId, + orderId = faker.number.int({ min: 1 }), +}: { + runId?: number + programId?: number + orderId?: number +} = {}) => { + const lines = [] + if (runId !== undefined) { + lines.push( + factories.orders.line({ + product: factories.orders.product({ + purchasable_object: { + id: runId, + title: "Some Run", + course: { id: faker.number.int(), title: "Some Course" }, + }, + }), + }), + ) + } + if (programId !== undefined) { + lines.push( + factories.orders.line({ + product: factories.orders.product({ + purchasable_object: { + id: programId, + title: "Some Program", + readable_id: "program-v1:MITxT+SysEng", + }, + }), + }), + ) + } + + setMockResponse.get( + urls.orders.historyList({ limit: 100 }), + factories.orders.orderHistoryList( + lines.length > 0 + ? [ + factories.orders.orderHistory({ + id: orderId, + state: "fulfilled", + lines, + }), + ] + : [], + ), + ) + + return { orderId } +} + const dashboardCourse: PartialFactory = ( ...overrides ) => { @@ -563,6 +624,7 @@ const buildProgramScenario = ( export { dashboardCourse, dashboardProgram, + setupOrderHistory, setupEnrollments, setupProgramsAndCourses, setupOrgAndUser, diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptCard.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptCard.tsx new file mode 100644 index 0000000000..2150475fa3 --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptCard.tsx @@ -0,0 +1,48 @@ +import { styled } from "ol-components" + +/** + * The white card the receipt is built from. Shared so the summary, the detail + * sections and the issuer block stay in step. + * + * `cardshadow` in the design system is `0 4px 8px #13141514`. + */ +const ReceiptCard = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "column", + gap: "24px", + padding: "32px", + borderRadius: "4px", + backgroundColor: theme.custom.colors.white, + boxShadow: "0 4px 8px 0 rgba(19, 20, 21, 0.08)", + [theme.breakpoints.down("sm")]: { + gap: "12px", + padding: "16px", + }, +})) + +/** + * The Order / Customer / Payment cards read as one block: a 1px gap shows the + * seams while only the outer corners are rounded. + * + * Uses `:first-of-type` / `:last-of-type` rather than the child index because a + * section is omitted entirely when the API supplied none of its fields. + */ +const ReceiptCardStack = styled.div({ + display: "flex", + flexDirection: "column", + gap: "1px", + width: "100%", + "> *": { + borderRadius: 0, + }, + "> *:first-of-type": { + borderTopLeftRadius: "4px", + borderTopRightRadius: "4px", + }, + "> *:last-of-type": { + borderBottomLeftRadius: "4px", + borderBottomRightRadius: "4px", + }, +}) + +export { ReceiptCard, ReceiptCardStack } diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptDetailList.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptDetailList.tsx new file mode 100644 index 0000000000..5e71945b67 --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptDetailList.tsx @@ -0,0 +1,86 @@ +import React from "react" +import { styled } from "ol-components" + +/** A label/value pair. A nullish `value` means the row is dropped. */ +type ReceiptDetail = { + label: string + value: React.ReactNode +} + +const Rows = styled.dl({ + display: "flex", + flexDirection: "column", + gap: "16px", + width: "100%", + margin: 0, +}) + +/** `dl > div` wrapping a dt/dd pair is the standard grouping form. */ +const Row = styled.div({ + display: "flex", + gap: "10px", + alignItems: "flex-start", +}) + +/** + * Fixed-width label column, so values line up down the card without needing a + * grid. It narrows on mobile to leave the value room to wrap rather than truncate. + */ +const Label = styled.dt(({ theme }) => ({ + ...theme.typography.body2, + fontWeight: theme.typography.fontWeightBold, + flexShrink: 0, + width: "200px", + margin: 0, + [theme.breakpoints.down("sm")]: { + ...theme.typography.body3, + fontWeight: theme.typography.fontWeightBold, + width: "104px", + }, +})) + +const Value = styled.dd(({ theme }) => ({ + ...theme.typography.body2, + flex: "1 0 0", + minWidth: 0, + margin: 0, + wordBreak: "break-word", + [theme.breakpoints.down("sm")]: { + ...theme.typography.body3, + }, +})) + +/** + * Drop rows the API did not supply, so callers can tell whether a section has any + * content left and skip rendering it along with its heading. MITx Online leaves + * optional receipt fields unset rather than blank, and an order with no payment + * transaction has every payment and address field null. + */ +const populatedRows = (rows: ReceiptDetail[]): ReceiptDetail[] => + rows.filter( + ({ value }) => value !== null && value !== undefined && value !== "", + ) + +/** + * The label/value rows inside a receipt card. Renders nothing when every row was + * filtered out; see {@link populatedRows}. + */ +const ReceiptDetailList: React.FC<{ + rows: ReceiptDetail[] + className?: string +}> = ({ rows, className }) => ( + + {populatedRows(rows).map(({ label, value }, index) => ( + // Labels repeat when an order has more than one line, so position is the + // only unique key available. + // eslint-disable-next-line react/no-array-index-key + + + {value} + + ))} + +) + +export { ReceiptDetailList, populatedRows } +export type { ReceiptDetail } diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptOrderSummary.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptOrderSummary.tsx new file mode 100644 index 0000000000..efeed02953 --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptOrderSummary.tsx @@ -0,0 +1,165 @@ +import React from "react" +import { styled } from "ol-components" +import { StateEnum } from "@mitodl/mitxonline-api-axios/v2" +import type { Order } from "@mitodl/mitxonline-api-axios/v2" +import { ReceiptCard } from "./ReceiptCard" +import { formatMoney, formatReceiptDate } from "./receiptUtils" + +const SummaryCard = styled(ReceiptCard)(({ theme }) => ({ + gap: "20px", + [theme.breakpoints.down("sm")]: { + gap: "12px", + }, +})) + +/** The heading and the total share a size; only the section headings are red. */ +const Heading = styled.h2(({ theme }) => ({ + ...theme.typography.h5, + color: theme.custom.colors.darkGray2, + margin: 0, + [theme.breakpoints.down("sm")]: theme.typography.subtitle1, +})) + +const Rows = styled.div({ + display: "flex", + flexDirection: "column", + gap: "16px", +}) + +const Row = styled.div({ + display: "flex", + gap: "16px", + alignItems: "flex-start", + justifyContent: "space-between", +}) + +const RowLabel = styled.span(({ theme }) => ({ + ...theme.typography.body2, + fontWeight: theme.typography.fontWeightBold, + color: theme.custom.colors.darkGray2, + [theme.breakpoints.down("sm")]: { + ...theme.typography.body3, + fontWeight: theme.typography.fontWeightBold, + }, +})) + +const RowValue = styled.span(({ theme }) => ({ + ...theme.typography.body2, + color: theme.custom.colors.darkGray2, + textAlign: "right", + whiteSpace: "nowrap", + [theme.breakpoints.down("sm")]: { + ...theme.typography.body3, + }, +})) + +const DiscountValue = styled(RowValue)(({ theme }) => ({ + color: theme.custom.colors.green, +})) + +const RefundLabel = styled(RowLabel)(({ theme }) => ({ + color: theme.custom.colors.red, +})) + +const RefundValue = styled(RowValue)(({ theme }) => ({ + color: theme.custom.colors.red, +})) + +const TotalRow = styled(Row)(({ theme }) => ({ + ...theme.typography.h5, + color: theme.custom.colors.darkGray2, + borderTop: `1px solid ${theme.custom.colors.lightGray2}`, + paddingTop: "24px", + [theme.breakpoints.down("sm")]: theme.typography.subtitle1, +})) + +/** `line.discount` is per unit, hence the multiply. */ +const getTotalDiscount = (order: Order): number => + order.lines.reduce( + (total, line) => total + Number(line.discount) * line.quantity, + 0, + ) + +const getTotalQuantity = (order: Order): number => + order.lines.reduce((total, line) => total + line.quantity, 0) + +const isRefundedState = (state: Order["state"]): boolean => + state === StateEnum.Refunded || state === StateEnum.PartiallyRefunded + +/** + * One row per refund, so a refunded order does not read as a plain paid receipt. + * `total_price_paid` is stamped at fulfillment and a refund never reduces it, so + * the total below stays at the amount charged. + * + * Refunds normally come from refund transactions, but the + * `refund_fulfilled_order` management command flips an order's state without + * writing one — hence the state fallback, which has no amount to show. + */ +const getRefundRows = (order: Order): { label: string; amount?: number }[] => { + if (order.refunds.length > 0) { + return order.refunds.map((refund) => ({ + label: refund.date + ? `Refund applied (${formatReceiptDate(refund.date)})` + : "Refund applied", + amount: refund.amount, + })) + } + return isRefundedState(order.state) ? [{ label: "Refund applied" }] : [] +} + +/** + * Per-item prices, discount and quantity totals, and the amount paid. The + * design's "Tax" and "Total Before Tax" rows are omitted — the payload has no tax + * data, so "Total Before Tax" would just restate the total. + */ +const ReceiptOrderSummary: React.FC<{ order: Order; className?: string }> = ({ + order, + className, +}) => { + const totalDiscount = getTotalDiscount(order) + const totalQuantity = getTotalQuantity(order) + + return ( + + Order Summary + + {order.lines.map((line, index) => ( + // Receipt lines carry no id, and the same product can legitimately + // appear twice, so position is the only stable key available. + // eslint-disable-next-line react/no-array-index-key + + {line.content_title} + {formatMoney(line.price)} + + ))} + {totalDiscount > 0 ? ( + + Discount + {`- ${formatMoney(totalDiscount)}`} + + ) : null} + + Quantity + {`x ${totalQuantity}`} + + {getRefundRows(order).map(({ label, amount }, index) => ( + // An order can carry several refunds and nothing identifies them, so + // position is the only stable key available. + // eslint-disable-next-line react/no-array-index-key + + {label} + {amount === undefined ? null : ( + {`- ${formatMoney(amount)}`} + )} + + ))} + + + Total: + {formatMoney(order.total_price_paid)} + + + ) +} + +export { ReceiptOrderSummary, getTotalDiscount, getTotalQuantity } diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptPage.test.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptPage.test.tsx new file mode 100644 index 0000000000..158bda8180 --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptPage.test.tsx @@ -0,0 +1,552 @@ +import React from "react" +import { + renderWithProviders, + screen, + setMockResponse, + user, + within, +} from "@/test-utils" +import * as mitxonline from "api/mitxonline-test-utils" +import ReceiptPage from "./ReceiptPage" +import * as urls from "@/common/urls" + +const ORDER_ID = 4242 + +const setupApis = ({ + order, + user, +}: { + order?: ReturnType + user?: ReturnType +} = {}) => { + const mitxUser = user ?? mitxonline.factories.user.user() + const receipt = order ?? mitxonline.factories.orders.order({ id: ORDER_ID }) + setMockResponse.get(mitxonline.urls.userMe.get(), mitxUser) + setMockResponse.get(mitxonline.urls.orders.receipt(ORDER_ID), receipt) + return { mitxUser, receipt } +} + +/** Reads a label's value cell within one section, since labels repeat. */ +const findValueFor = async (sectionName: string, label: string) => { + const heading = await screen.findByRole("heading", { name: sectionName }) + // The heading and its detail list are siblings inside the section element. + const section = heading.closest("section") + if (!section) throw new Error(`No section found for "${sectionName}"`) + const term = within(section).getByText(label) + const value = term.closest("dt")?.nextElementSibling + if (!value) throw new Error(`No value cell found for "${label}"`) + return value +} + +describe("ReceiptPage", () => { + test("renders order information from the receipt", async () => { + const line = mitxonline.factories.orders.transactionLine({ + content_title: "The Iterative Innovation Process", + readable_id: "program-v1:xPRO+SysEngx", + start_date: "2024-09-01T00:00:00Z", + end_date: "2025-12-24T00:00:00Z", + price: "1524.60", + quantity: 1, + discount: "0.00", + CEUs: "20", + }) + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + lines: [line], + reference_number: "xpro-b2c-production-66238", + created_on: "2024-06-14T00:00:00Z", + total_price_paid: "1524.60", + }), + }) + + renderWithProviders() + + expect( + await screen.findByRole("heading", { name: "Receipt", level: 1 }), + ).toBeInTheDocument() + + expect( + await findValueFor("Order Information", "Order Item:"), + ).toHaveTextContent("The Iterative Innovation Process") + expect(await findValueFor("Order Information", "Dates:")).toHaveTextContent( + "September 01, 2024 - December 24, 2025", + ) + expect( + await findValueFor("Order Information", "Order Number:"), + ).toHaveTextContent("xpro-b2c-production-66238") + expect( + await findValueFor("Order Information", "Order Date:"), + ).toHaveTextContent("June 14, 2024") + expect( + await findValueFor("Order Information", "Unit Price:"), + ).toHaveTextContent("$1,524.60") + expect( + await findValueFor("Order Information", "Total Paid:"), + ).toHaveTextContent("$1,524.60") + expect( + await findValueFor("Order Information", "Product Number:"), + ).toHaveTextContent("program-v1:xPRO+SysEngx") + expect(await findValueFor("Order Information", "CEUs:")).toHaveTextContent( + "20", + ) + }) + + test("formats dates in UTC so they match the recorded order", async () => { + // Local formatting would render these a day early. + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + created_on: "2026-07-09T00:00:00Z", + lines: [ + mitxonline.factories.orders.transactionLine({ + start_date: "2026-07-08T00:00:00Z", + end_date: "2027-07-09T00:00:00Z", + }), + ], + }), + }) + + renderWithProviders() + + expect(await findValueFor("Order Information", "Dates:")).toHaveTextContent( + "July 08, 2026 - July 09, 2027", + ) + expect( + await findValueFor("Order Information", "Order Date:"), + ).toHaveTextContent("July 09, 2026") + }) + + test("shows only the known end of the range when a run has no end date", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + lines: [ + mitxonline.factories.orders.transactionLine({ + start_date: "2026-06-22T00:00:00Z", + end_date: null as unknown as string, + }), + ], + }), + }) + + renderWithProviders() + + expect(await findValueFor("Order Information", "Dates:")).toHaveTextContent( + "June 22, 2026", + ) + }) + + test("shows cents on whole-dollar amounts", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + lines: [ + mitxonline.factories.orders.transactionLine({ price: "500.00" }), + ], + total_price_paid: "500.00", + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Order Information", "Unit Price:"), + ).toHaveTextContent("$500.00") + }) + + test("takes the customer name and email from the MITx Online user", async () => { + const { mitxUser } = setupApis({ + user: mitxonline.factories.user.user({ + name: "Peter Pinch", + email: "pdpinch@mit.edu", + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Customer Information", "Name:"), + ).toHaveTextContent(mitxUser.name!) + expect( + await findValueFor("Customer Information", "Email:"), + ).toHaveTextContent("pdpinch@mit.edu") + }) + + test("renders the billing address as a single line", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + street_address: { + line: ["123 Main Street"], + city: "Danvers", + state: "MA", + postal_code: "01923", + country: "US", + }, + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Customer Information", "Address:"), + ).toHaveTextContent("123 Main Street, Danvers MA, 01923, US") + }) + + test("renders card payment details", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + transactions: { + payment_method: "card", + card_type: "Visa", + card_number: "xxxxxxxxxxxx1111", + name: "Peter Pinch", + }, + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Payment Information", "Payment Method:"), + ).toHaveTextContent("Visa | xxxxxxxxxxxx1111") + expect( + await findValueFor("Payment Information", "Name:"), + ).toHaveTextContent("Peter Pinch") + }) + + test("renders Paypal payments without card details", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + transactions: { + payment_method: "paypal", + bill_to_email: "pdpinch@mit.edu", + name: "Peter Pinch", + }, + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Payment Information", "Payment Method:"), + ).toHaveTextContent("PayPal") + }) + + // Parity with the MITx Online receipt, which shows the payer's email for PayPal + // orders in place of card details. + test("shows the payer email for Paypal orders", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + transactions: { + payment_method: "paypal", + bill_to_email: "payer@example.com", + name: "Peter Pinch", + }, + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Payment Information", "Email:"), + ).toHaveTextContent("payer@example.com") + }) + + // Parity: MITx Online shows a per-line total. It is redundant on a single-line + // order, where it equals the order total shown below. + test("omits the per-line total on a single-line order", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + lines: [mitxonline.factories.orders.transactionLine()], + }), + }) + + renderWithProviders() + + await screen.findByRole("heading", { name: "Order Information" }) + expect(screen.queryByText("Line Total:")).not.toBeInTheDocument() + }) + + test("shows a per-line total when the order has multiple lines", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + lines: [ + mitxonline.factories.orders.transactionLine({ total_paid: "100.00" }), + mitxonline.factories.orders.transactionLine({ total_paid: "250.00" }), + ], + }), + }) + + renderWithProviders() + + await screen.findByRole("heading", { name: "Order Information" }) + expect(screen.getAllByText("Line Total:")).toHaveLength(2) + expect(screen.getByText("$100.00")).toBeInTheDocument() + expect(screen.getByText("$250.00")).toBeInTheDocument() + }) + + test("shows the discount code when one was redeemed", async () => { + const discountCode = "30468acf5a4e4c3c9c31a262caf984c9" // pragma: allowlist secret + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + discounts: [ + mitxonline.factories.orders.redeemedDiscount({ + discount_code: discountCode, + }), + ], + }), + }) + + renderWithProviders() + + expect( + await findValueFor("Order Information", "Discount Code:"), + ).toHaveTextContent(discountCode) // + }) + + test("omits rows the receipt payload does not provide", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + // No discount redeemed and no per-line discount. + discounts: [], + lines: [ + mitxonline.factories.orders.transactionLine({ + discount: "0.00", + // MITx Online hardcodes CEUs to null on receipts today. + CEUs: null as unknown as string, + }), + ], + street_address: {}, + }), + }) + + renderWithProviders() + + await screen.findByRole("heading", { name: "Order Information" }) + + expect(screen.queryByText("Discount Code:")).not.toBeInTheDocument() + expect(screen.queryByText("Discount:")).not.toBeInTheDocument() + expect(screen.queryByText("CEUs:")).not.toBeInTheDocument() + expect(screen.getByText("Address:")).toBeInTheDocument() // MIT Learn's own + expect( + within( + ( + await screen.findByRole("heading", { name: "Customer Information" }) + ).closest("section")!, + ).queryByText("Address:"), + ).not.toBeInTheDocument() + }) + + test("drops the Payment Information section when there is no transaction", async () => { + // Matches a real zero-value order: MITx Online returns the + // transaction and address objects with every field null. + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + transactions: { + card_number: undefined, + card_type: undefined, + name: undefined, + bill_to_email: undefined, + payment_method: undefined, + }, + }), + }) + + renderWithProviders() + + await screen.findByRole("heading", { name: "Order Information" }) + + expect( + screen.queryByRole("heading", { name: "Payment Information" }), + ).not.toBeInTheDocument() + }) + + test("renders the order summary with total and quantity", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + lines: [ + mitxonline.factories.orders.transactionLine({ + content_title: "The Iterative Innovation Process", + price: "1524.60", + quantity: 2, + discount: "10.00", + }), + ], + total_price_paid: "3029.20", + }), + }) + + renderWithProviders() + + const summary = ( + await screen.findByRole("heading", { name: "Order Summary" }) + ).closest("div")! + + expect(summary).toHaveTextContent("The Iterative Innovation Process") + expect(summary).toHaveTextContent("x 2") + // 10.00 per unit × 2 units + expect(summary).toHaveTextContent("- $20.00") + expect(summary).toHaveTextContent("$3,029.20") + }) + + test("shows a row per refund, leaving the total at the amount charged", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + state: "partially_refunded", + total_price_paid: "1524.60", + refunds: [ + mitxonline.factories.orders.orderRefund({ + amount: 500, + date: "2024-07-02T00:00:00Z", + }), + mitxonline.factories.orders.orderRefund({ + amount: 24.6, + date: "2024-08-15T00:00:00Z", + }), + ], + }), + }) + + renderWithProviders() + + const summary = ( + await screen.findByRole("heading", { name: "Order Summary" }) + ).closest("div")! + + expect(summary).toHaveTextContent("Refund applied (July 02, 2024)") + expect(summary).toHaveTextContent("- $500.00") + expect(summary).toHaveTextContent("Refund applied (August 15, 2024)") + expect(summary).toHaveTextContent("- $24.60") + // total_price_paid is stamped at fulfillment; a refund never reduces it. + expect(summary).toHaveTextContent("$1,524.60") + }) + + // `refund_fulfilled_order` flips the state without writing a refund transaction. + test("notes a refund when the state says refunded but no refunds are recorded", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + state: "refunded", + refunds: [], + }), + }) + + renderWithProviders() + + const summary = ( + await screen.findByRole("heading", { name: "Order Summary" }) + ).closest("div")! + + expect(summary).toHaveTextContent("Refund applied") + }) + + test("shows no refund row on a fulfilled order", async () => { + setupApis({ + order: mitxonline.factories.orders.order({ + id: ORDER_ID, + state: "fulfilled", + refunds: [], + }), + }) + + renderWithProviders() + + await screen.findByRole("heading", { name: "Order Summary" }) + + expect(screen.queryByText(/Refund applied/)).not.toBeInTheDocument() + }) + + // Someone else's order 404s like a missing one; both get the generic 404. + test("renders the generic 404 when the order is not found or not yours", async () => { + setMockResponse.get( + mitxonline.urls.userMe.get(), + mitxonline.factories.user.user(), + ) + setMockResponse.get(mitxonline.urls.orders.receipt(ORDER_ID), "Not found", { + code: 404, + }) + + renderWithProviders() + + expect( + await screen.findByText(/couldn't find what you were looking for/i), + ).toBeInTheDocument() + expect( + screen.queryByText(/We could not load this receipt/i), + ).not.toBeInTheDocument() + }) + + // A 500 says nothing about existence, so it keeps an actionable message. + test("keeps an actionable message when the request fails for another reason", async () => { + setMockResponse.get( + mitxonline.urls.userMe.get(), + mitxonline.factories.user.user(), + ) + setMockResponse.get( + mitxonline.urls.orders.receipt(ORDER_ID), + "Server error", + { code: 500 }, + ) + + renderWithProviders() + + // An alert, so the failure is read out rather than silently replacing the + // skeleton. + expect(await screen.findByRole("alert")).toHaveTextContent( + /We could not load this receipt/i, + ) + expect( + screen.getByRole("link", { name: "Back to Dashboard" }), + ).toHaveAttribute("href", "/dashboard") + }) + + /** + * jsdom starts each test file with a single history entry, matching a receipt + * opened directly in a fresh tab. + */ + test("Back goes to the dashboard when there is no history to return to", async () => { + setupApis() + + const { location } = renderWithProviders() + await screen.findByRole("heading", { name: "Order Summary" }) + await user.click(screen.getByRole("button", { name: "Back" })) + + expect(location.current.pathname).toBe("/dashboard") + }) + + test("Back uses history when there is somewhere to return to", async () => { + setupApis() + // A second entry, so history.back() has an effect. + window.history.pushState({}, "", urls.receiptView(ORDER_ID)) + + const { location } = renderWithProviders() + await screen.findByRole("heading", { name: "Order Summary" }) + await user.click(screen.getByRole("button", { name: "Back" })) + + expect(location.current.pathname).not.toBe("/dashboard") + }) + + test("announces loading and then the loaded receipt", async () => { + setupApis() + + renderWithProviders() + + expect(screen.getByRole("status")).toHaveTextContent("Loading receipt.") + + await screen.findByRole("heading", { name: "Order Summary" }) + + expect(screen.getByRole("status")).toHaveTextContent("Receipt loaded.") + }) +}) diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptPage.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptPage.tsx new file mode 100644 index 0000000000..0bcb19490a --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptPage.tsx @@ -0,0 +1,359 @@ +"use client" + +import React from "react" +import Image from "next/image" +import { useRouter } from "next-nprogress-bar" +import { useQuery } from "@tanstack/react-query" +import { RiArrowLeftLine, RiPrinterLine } from "@remixicon/react" +import { Container, Skeleton, Typography, styled } from "ol-components" +import { Button, ButtonLink, VisuallyHidden } from "@mitodl/smoot-design" +import { orderQueries } from "api/mitxonline-hooks/orders" +import { mitxUserQueries } from "api/mitxonline-hooks/user" +import type { Order } from "@mitodl/mitxonline-api-axios/v2" +import type { AxiosError } from "axios" +import NotFoundPage from "@/app-pages/ErrorPage/NotFoundPage" +import mitLearnLogo from "@/public/images/mit-learn-logo-black.svg" +import { env } from "@/env" +import * as urls from "@/common/urls" +import { ReceiptCard, ReceiptCardStack } from "./ReceiptCard" +import { ReceiptDetailList, populatedRows } from "./ReceiptDetailList" +import type { ReceiptDetail } from "./ReceiptDetailList" +import { ReceiptOrderSummary } from "./ReceiptOrderSummary" +import { + formatDateRange, + formatMoney, + formatPaymentMethod, + formatReceiptDate, + formatStreetAddress, + getDiscountCode, +} from "./receiptUtils" + +const SUPPORT_EMAIL = env("NEXT_PUBLIC_MITOL_SUPPORT_EMAIL") || "" + +/** MIT Learn's own details — not learner data, not from the API. */ +const MIT_LEARN_ADDRESS = + "600 Technology Square, NE49-2000, Cambridge, MA 02139 USA" + +const Background = styled.div(({ theme }) => ({ + backgroundColor: theme.custom.colors.lightGray1, + minHeight: "100%", +})) + +const PageContainer = styled(Container)(({ theme }) => ({ + display: "flex", + flexDirection: "column", + gap: "24px", + paddingTop: "64px", + paddingBottom: "64px", + [theme.breakpoints.down("sm")]: { + paddingTop: "32px", + paddingBottom: "32px", + }, +})) + +const TitleRow = styled.div({ + display: "flex", + alignItems: "center", + justifyContent: "space-between", + gap: "16px", +}) + +const PageHeading = styled.h1(({ theme }) => ({ + ...theme.typography.h3, + color: theme.custom.colors.black, + margin: 0, + [theme.breakpoints.down("sm")]: theme.typography.h5, +})) + +const Actions = styled.div(({ theme }) => ({ + display: "flex", + gap: "32px", + flexShrink: 0, + [theme.breakpoints.down("sm")]: { + gap: "16px", + }, + // Neither button belongs in a printed receipt. + "@media print": { + display: "none", + }, +})) + +/** + * Two columns on desktop with the summary on the right; one column on mobile with + * the summary first. The summary leads in the DOM so mobile needs no reordering, + * and desktop places it explicitly in the second column. + */ +const Columns = styled.div(({ theme }) => ({ + display: "grid", + gap: "40px", + gridTemplateColumns: "minmax(0, 1fr)", + alignItems: "start", + [theme.breakpoints.up("md")]: { + gridTemplateColumns: "minmax(0, 825fr) minmax(0, 411fr)", + }, +})) + +const SummaryColumn = styled.div(({ theme }) => ({ + [theme.breakpoints.up("md")]: { + gridColumn: 2, + gridRow: 1, + }, +})) + +const DetailColumn = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "column", + gap: "16px", + [theme.breakpoints.up("md")]: { + gridColumn: 1, + gridRow: 1, + }, +})) + +const SectionHeading = styled.h2(({ theme }) => ({ + ...theme.typography.h5, + color: theme.custom.colors.red, + margin: 0, + [theme.breakpoints.down("sm")]: theme.typography.subtitle1, +})) + +const IssuerCard = styled(ReceiptCard)({ + padding: "32px", +}) + +const IssuerLogo = styled(Image)(({ theme }) => ({ + display: "block", + alignSelf: "flex-start", + width: "auto", + height: "34px", + [theme.breakpoints.down("sm")]: { + height: "24px", + }, +})) + +const SupportLink = styled.a(({ theme }) => ({ + color: theme.custom.colors.red, + textDecoration: "none", + ":hover": { + textDecoration: "underline", + }, +})) + +const ErrorState = styled.div({ + display: "flex", + flexDirection: "column", + alignItems: "flex-start", + gap: "16px", +}) + +/** + * One group of rows per line item, then the order-level rows. The design's "Tax" + * and "HSN" rows are omitted — the payload has no such fields. "CEUs" is wired up + * but MITx Online always returns null for it today. + */ +const getOrderDetailRows = (order: Order): ReceiptDetail[] => [ + ...order.lines.flatMap((line) => [ + { label: "Order Item:", value: line.content_title }, + { label: "Dates:", value: formatDateRange(line.start_date, line.end_date) }, + { label: "Product Number:", value: line.readable_id }, + { label: "CEUs:", value: line.CEUs }, + { label: "Unit Price:", value: formatMoney(line.price) }, + { label: "Quantity:", value: line.quantity }, + { + label: "Discount:", + value: + Number(line.discount) > 0 ? `-${formatMoney(line.discount)}` : null, + }, + { + // Per-line total, as the MITx Online receipt shows. Omitted for a + // single-line order, where it just restates the order total below. + label: "Line Total:", + value: order.lines.length > 1 ? formatMoney(line.total_paid) : null, + }, + ]), + { label: "Order Number:", value: order.reference_number }, + { label: "Order Date:", value: formatReceiptDate(order.created_on) }, + { label: "Discount Code:", value: getDiscountCode(order) }, + { label: "Total Paid:", value: formatMoney(order.total_price_paid) }, +] + +const ReceiptSkeleton: React.FC = () => ( + + + + +) + +const ReceiptPage: React.FC<{ orderId: number }> = ({ orderId }) => { + const router = useRouter() + const orderQuery = useQuery(orderQueries.receipt(orderId)) + /** + * `Order.purchaser` has no name field. The endpoint only returns the requester's + * own orders, so the logged-in user is the purchaser. + */ + const userQuery = useQuery(mitxUserQueries.me()) + + const order = orderQuery.data + const user = userQuery.data + + /** + * Filtered up front so a section with no rows can be dropped along with its + * heading — zero-value orders have no payment or billing details at all. + */ + const customerRows = order + ? populatedRows([ + { label: "Name:", value: user?.name }, + { label: "Email:", value: user?.email }, + { label: "Address:", value: formatStreetAddress(order.street_address) }, + ]) + : [] + + const paymentRows = order + ? populatedRows([ + { label: "Name:", value: order.transactions?.name }, + // PayPal orders carry the payer's email instead of card details. + { label: "Email:", value: order.transactions?.bill_to_email }, + { label: "Payment Method:", value: formatPaymentMethod(order) }, + ]) + : [] + + /** + * The endpoint is purchaser-scoped, so someone else's order 404s just like a + * missing one. Both get the generic 404 so a prober cannot tell them apart. + * Other failures keep the message below, since they say nothing about whether + * the order exists. + */ + const isNotFound = + !orderQuery.isPending && + (orderQuery.error as AxiosError | null)?.response?.status === 404 + + if (isNotFound) { + return + } + + /** + * `router.back()` is `history.back()`, which does nothing when the receipt is the + * tab's only entry — a bookmarked or newly opened link. Real history is still + * preferred, since it returns to the dashboard with its scroll position intact. + */ + const handleBack = () => { + if (window.history.length > 1) { + router.back() + } else { + router.push(urls.DASHBOARD_HOME) + } + } + + /** + * The heading and buttons render before the receipt does, and skeletons carry no + * ARIA, so the swap needs announcing. Failure is left to `ErrorState`, which is + * an assertive live region of its own. + */ + const statusMessage = orderQuery.isPending + ? "Loading receipt." + : order + ? "Receipt loaded." + : null + + return ( + + + + {statusMessage} + + + + Receipt + + + + + + + {orderQuery.isPending ? ( + + ) : orderQuery.isError || !order ? ( + + + We could not load this receipt.{" "} + {SUPPORT_EMAIL ? ( + <> + + Contact support + {" "} + if the problem persists. + + ) : ( + "Please try again in a moment." + )} + + + Back to Dashboard + + + ) : ( + + + + + + + + + Order Information + + + + {customerRows.length > 0 ? ( + + Customer Information + + + ) : null} + + {paymentRows.length > 0 ? ( + + Payment Information + + + ) : null} + + + + + + {SUPPORT_EMAIL} + + ) : null, + }, + ]} + /> + + + + )} + + + ) +} + +export default ReceiptPage +export { getOrderDetailRows } diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.test.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.test.tsx new file mode 100644 index 0000000000..a2800e2298 --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.test.tsx @@ -0,0 +1,266 @@ +import React from "react" +import { + renderWithProviders, + screen, + setMockResponse, + waitFor, +} from "@/test-utils" +import * as mitxonline from "api/mitxonline-test-utils" +import { receiptView } from "@/common/urls" +import { + ReceiptByProgramRedirect, + ReceiptByRunRedirect, +} from "./ReceiptRedirect" + +const RUN_ID = 500 +const PROGRAM_ID = 99 +const ORDER_ID = 4242 + +/** Must match the hook's request exactly, params included. */ +const HISTORY_URL = mitxonline.urls.orders.historyList({ limit: 100 }) + +/** Course run: the only variant with `course`. */ +const courseRunLine = (runId: number) => + mitxonline.factories.orders.line({ + product: mitxonline.factories.orders.product({ + purchasable_object: { + id: runId, + title: "Some Run", + readable_id: "course-v1:MITxT+1.234", + course: { id: 7, title: "Some Course" }, + }, + }), + }) + +/** Program: neither `course` nor `run_tag`. */ +const programLine = (programId: number) => + mitxonline.factories.orders.line({ + product: mitxonline.factories.orders.product({ + purchasable_object: { + id: programId, + title: "Some Program", + readable_id: "program-v1:MITxT+SysEng", + }, + }), + }) + +/** Program run: has `run_tag`, and its `id` is the run's, not the program's. */ +const programRunLine = (programRunId: number) => + mitxonline.factories.orders.line({ + product: mitxonline.factories.orders.product({ + purchasable_object: { + id: programRunId, + run_tag: "R1", + start_date: "2024-09-01T00:00:00Z", + end_date: "2025-12-24T00:00:00Z", + }, + }), + }) + +describe("ReceiptByRunRedirect", () => { + test("redirects to the receipt for the order that paid for the run", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [courseRunLine(RUN_ID)], + }), + ]), + ) + + const { location } = renderWithProviders( + , + ) + + await waitFor(() => { + expect(location.current.pathname).toBe(receiptView(ORDER_ID)) + }) + }) + + test("announces loading and then the redirect", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [courseRunLine(RUN_ID)], + }), + ]), + ) + + renderWithProviders() + + expect(screen.getByRole("status")).toHaveTextContent("Loading receipt.") + + await waitFor(() => { + expect(screen.getByRole("status")).toHaveTextContent( + "Receipt found. Redirecting.", + ) + }) + }) + + test("picks the most recent matching order", async () => { + setMockResponse.get( + HISTORY_URL, + // MITx Online returns order history most-recent-first. + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: 999, + state: "fulfilled", + lines: [courseRunLine(RUN_ID)], + }), + mitxonline.factories.orders.orderHistory({ + id: 111, + state: "fulfilled", + lines: [courseRunLine(RUN_ID)], + }), + ]), + ) + + const { location } = renderWithProviders( + , + ) + + await waitFor(() => { + expect(location.current.pathname).toBe(receiptView(999)) + }) + }) + + test("ignores orders that were not fulfilled", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: 999, + state: "canceled", + lines: [courseRunLine(RUN_ID)], + }), + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [courseRunLine(RUN_ID)], + }), + ]), + ) + + const { location } = renderWithProviders( + , + ) + + await waitFor(() => { + expect(location.current.pathname).toBe(receiptView(ORDER_ID)) + }) + }) + + test("does not match a program whose id happens to equal the run id", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [programLine(RUN_ID)], + }), + ]), + ) + + renderWithProviders() + + expect( + await screen.findByText(/couldn't find what you were looking for/i), + ).toBeInTheDocument() + }) + + test("shows not found when no order references the run", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + state: "fulfilled", + lines: [courseRunLine(RUN_ID + 1)], + }), + ]), + ) + + renderWithProviders() + + expect( + await screen.findByText(/couldn't find what you were looking for/i), + ).toBeInTheDocument() + }) + + // Indistinguishable from an absent receipt, on purpose. + test("shows the generic 404 when the order lookup fails", async () => { + setMockResponse.get(HISTORY_URL, "Server error", { code: 500 }) + + renderWithProviders() + + expect( + await screen.findByText(/couldn't find what you were looking for/i), + ).toBeInTheDocument() + }) +}) + +describe("ReceiptByProgramRedirect", () => { + test("redirects to the receipt for the order that paid for the program", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [programLine(PROGRAM_ID)], + }), + ]), + ) + + const { location } = renderWithProviders( + , + ) + + await waitFor(() => { + expect(location.current.pathname).toBe(receiptView(ORDER_ID)) + }) + }) + + test("does not match a program run whose id happens to equal the program id", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [programRunLine(PROGRAM_ID)], + }), + ]), + ) + + renderWithProviders() + + expect( + await screen.findByText(/couldn't find what you were looking for/i), + ).toBeInTheDocument() + }) + + test("does not match a course run whose id happens to equal the program id", async () => { + setMockResponse.get( + HISTORY_URL, + mitxonline.factories.orders.orderHistoryList([ + mitxonline.factories.orders.orderHistory({ + id: ORDER_ID, + state: "fulfilled", + lines: [courseRunLine(PROGRAM_ID)], + }), + ]), + ) + + renderWithProviders() + + expect( + await screen.findByText(/couldn't find what you were looking for/i), + ).toBeInTheDocument() + }) +}) diff --git a/frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.tsx b/frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.tsx new file mode 100644 index 0000000000..3a8c2977bd --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/ReceiptRedirect.tsx @@ -0,0 +1,72 @@ +"use client" + +import React from "react" +import { useRouter } from "next-nprogress-bar" +import { Container, Skeleton, styled } from "ol-components" +import { VisuallyHidden } from "@mitodl/smoot-design" +import * as urls from "@/common/urls" +import NotFoundPage from "@/app-pages/ErrorPage/NotFoundPage" +import { + useOrderIdForProgram, + useOrderIdForRun, +} from "@/common/mitxonline/useOrderIdForResource" +import type { OrderIdResolution } from "@/common/mitxonline/useOrderIdForResource" + +const PageContainer = styled(Container)({ + paddingTop: "40px", + paddingBottom: "80px", + display: "flex", + flexDirection: "column", + gap: "16px", +}) + +/** + * Resolves a course run or program to its order, then replaces the history entry + * with that order's receipt. `replace`, not `push`, so "Back" from the receipt + * skips this route. Mirrors MITx Online's `ReceiptByRunView`. + */ +const ReceiptRedirect: React.FC<{ resolution: OrderIdResolution }> = ({ + resolution, +}) => { + const router = useRouter() + const { isPending, orderId } = resolution + + React.useEffect(() => { + if (orderId !== null) { + router.replace(urls.receiptView(orderId)) + } + }, [orderId, router]) + + if (isPending || orderId !== null) { + return ( + + {/* Skeletons carry no ARIA, and this route only ever shows them. */} + + {isPending ? "Loading receipt." : "Receipt found. Redirecting."} + + + + + ) + } + + /** + * No order covers this run/program, or the lookup failed. Generic 404 for both, + * so a prober cannot tell a well-formed id from an unresolvable one. + */ + return +} + +const ReceiptByRunRedirect: React.FC<{ runId: number }> = ({ runId }) => { + const resolution = useOrderIdForRun(runId) + return +} + +const ReceiptByProgramRedirect: React.FC<{ programId: number }> = ({ + programId, +}) => { + const resolution = useOrderIdForProgram(programId) + return +} + +export { ReceiptRedirect, ReceiptByRunRedirect, ReceiptByProgramRedirect } diff --git a/frontends/main/src/app-pages/ReceiptPage/receiptUtils.ts b/frontends/main/src/app-pages/ReceiptPage/receiptUtils.ts new file mode 100644 index 0000000000..8bb3a45862 --- /dev/null +++ b/frontends/main/src/app-pages/ReceiptPage/receiptUtils.ts @@ -0,0 +1,70 @@ +import moment from "moment" +import type { Order, OrderStreetAddress } from "@mitodl/mitxonline-api-axios/v2" +import { formatPrice } from "@/common/mitxonline" + +/** Receipt amounts always show cents, unlike catalog prices. */ +const formatMoney = (amount: number | string): string => + formatPrice(amount, { avoidCents: false }) + +/** + * Long-form receipt dates, e.g. "June 14, 2024". UTC, not the viewer's zone: + * course dates are stored at UTC midnight and would otherwise render a day early + * west of Greenwich. + */ +const formatReceiptDate = (date: string): string => + moment.utc(date).format("MMMM DD, YYYY") + +/** A line's dates as one range; either end may be absent. */ +const formatDateRange = ( + startDate?: string | null, + endDate?: string | null, +): string | null => { + const start = startDate ? formatReceiptDate(startDate) : null + const end = endDate ? formatReceiptDate(endDate) : null + if (start && end) return `${start} - ${end}` + return start ?? end +} + +/** + * Billing address as one line, e.g. "123 Main Street, Danvers MA, 01923". Every + * part is optional, so only present ones are joined. + */ +const formatStreetAddress = ( + address: OrderStreetAddress | undefined, +): string | null => { + if (!address) return null + const cityAndState = [address.city, address.state] + .filter(Boolean) + .join(" ") + .trim() + const parts = [ + ...(address.line ?? []), + cityAndState, + address.postal_code, + address.country, + ].filter((part): part is string => Boolean(part && part.trim())) + + return parts.length > 0 ? parts.join(", ") : null +} + +/** e.g. "Visa | xxxxxxxxxxxx1111", or null when there was no payment. */ +const formatPaymentMethod = (order: Order): string | null => { + const transaction = order.transactions + if (!transaction) return null + if (transaction.payment_method === "paypal") return "PayPal" + const parts = [transaction.card_type, transaction.card_number].filter(Boolean) + return parts.length > 0 ? parts.join(" | ") : null +} + +/** The discount code redeemed on the order, if any. */ +const getDiscountCode = (order: Order): string | null => + order.discounts[0]?.redeemed_discount?.discount_code ?? null + +export { + formatDateRange, + formatMoney, + formatPaymentMethod, + formatReceiptDate, + formatStreetAddress, + getDiscountCode, +} diff --git a/frontends/main/src/app/(site)/receipt/[orderId]/page.tsx b/frontends/main/src/app/(site)/receipt/[orderId]/page.tsx new file mode 100644 index 0000000000..6ca5a91e76 --- /dev/null +++ b/frontends/main/src/app/(site)/receipt/[orderId]/page.tsx @@ -0,0 +1,26 @@ +import React from "react" +import type { Metadata } from "next" +import { notFound } from "next/navigation" +import { standardizeMetadata } from "@/common/metadata" +import ReceiptPage from "@/app-pages/ReceiptPage/ReceiptPage" + +export const metadata: Metadata = standardizeMetadata({ + title: "Receipt", + robots: { index: false }, +}) + +/** + * Fetched client-side: MITx Online's session cookie is not forwarded on + * server-side requests, so the order cannot be prefetched here. + */ +const Page: React.FC> = async ({ params }) => { + const { orderId } = await params + const id = Number(orderId) + if (!Number.isInteger(id) || id <= 0) { + notFound() + } + + return +} + +export default Page diff --git a/frontends/main/src/app/(site)/receipt/by-program/[programId]/page.tsx b/frontends/main/src/app/(site)/receipt/by-program/[programId]/page.tsx new file mode 100644 index 0000000000..42b597af6f --- /dev/null +++ b/frontends/main/src/app/(site)/receipt/by-program/[programId]/page.tsx @@ -0,0 +1,28 @@ +import React from "react" +import type { Metadata } from "next" +import { notFound } from "next/navigation" +import { standardizeMetadata } from "@/common/metadata" +import { ReceiptByProgramRedirect } from "@/app-pages/ReceiptPage/ReceiptRedirect" + +export const metadata: Metadata = standardizeMetadata({ + title: "Receipt", + robots: { index: false }, +}) + +/** + * Resolves a program to the order covering it and redirects to that order's + * receipt. See `ReceiptRedirect` for why this route exists. + */ +const Page: React.FC> = async ({ + params, +}) => { + const { programId } = await params + const id = Number(programId) + if (!Number.isInteger(id) || id <= 0) { + notFound() + } + + return +} + +export default Page diff --git a/frontends/main/src/app/(site)/receipt/by-run/[runId]/page.tsx b/frontends/main/src/app/(site)/receipt/by-run/[runId]/page.tsx new file mode 100644 index 0000000000..020ac0e2a8 --- /dev/null +++ b/frontends/main/src/app/(site)/receipt/by-run/[runId]/page.tsx @@ -0,0 +1,28 @@ +import React from "react" +import type { Metadata } from "next" +import { notFound } from "next/navigation" +import { standardizeMetadata } from "@/common/metadata" +import { ReceiptByRunRedirect } from "@/app-pages/ReceiptPage/ReceiptRedirect" + +export const metadata: Metadata = standardizeMetadata({ + title: "Receipt", + robots: { index: false }, +}) + +/** + * Resolves a course run to the order covering it and redirects to that order's + * receipt. See `ReceiptRedirect` for why this route exists. + */ +const Page: React.FC> = async ({ + params, +}) => { + const { runId } = await params + const id = Number(runId) + if (!Number.isInteger(id) || id <= 0) { + notFound() + } + + return +} + +export default Page diff --git a/frontends/main/src/app/(site)/receipt/layout.tsx b/frontends/main/src/app/(site)/receipt/layout.tsx new file mode 100644 index 0000000000..13e7d0584f --- /dev/null +++ b/frontends/main/src/app/(site)/receipt/layout.tsx @@ -0,0 +1,22 @@ +import React from "react" +import RestrictedRoute from "@/components/RestrictedRoute/RestrictedRoute" +import { Permission } from "api/hooks/user" + +/** + * Gates all three receipt routes behind authentication. + * + * Receipts are bookmark-prone — saved for expense reports and reopened days + * later, or opened from a dashboard tab left overnight — so a stale cookie is a + * routine case rather than an edge one. Without this, the receipt query fails and + * the learner dead-ends on the generic error page. `RestrictedRoute` instead sends + * them through Keycloak and back to the receipt they asked for. + */ +const Layout: React.FC<{ children: React.ReactNode }> = ({ children }) => { + return ( + + {children} + + ) +} + +export default Layout diff --git a/frontends/main/src/common/mitxonline/useOrderIdForResource.ts b/frontends/main/src/common/mitxonline/useOrderIdForResource.ts new file mode 100644 index 0000000000..6d1b3a3250 --- /dev/null +++ b/frontends/main/src/common/mitxonline/useOrderIdForResource.ts @@ -0,0 +1,91 @@ +import { useQuery } from "@tanstack/react-query" +import { orderQueries } from "api/mitxonline-hooks/orders" +import type { + Line, + OrderHistory, + ProductPurchasableObject, +} from "@mitodl/mitxonline-api-axios/v2" +import { StateEnum } from "@mitodl/mitxonline-api-axios/v2" + +/** + * An explicit limit is required: without one, `orders/history` bypasses + * pagination and returns a bare array instead of `{count, results}`. We do not + * follow `next`, so orders past this many are not found. + */ +const ORDER_HISTORY_LIMIT = 100 + +type OrderIdResolution = { + isPending: boolean + /** + * True when the history could not be fetched, so whether a receipt exists is + * unknown. Distinct from `orderId === null`, which means we looked and there is + * genuinely no order — callers must not treat the two the same, or a failing + * request looks identical to "you never paid for this". + */ + isError: boolean + /** Most recent fulfilled order covering the resource. May be zero-value. */ + orderId: number | null +} + +/** + * `purchasable_object` is an untagged union whose variants all expose a bare `id`, + * and those ids come from different tables — so match on shape, not id alone. + * Program-run products match neither guard, which is fine: their id is the run's, + * not the program's. + */ +const isCourseRun = (obj: ProductPurchasableObject): boolean => + "course" in obj && obj.course !== undefined + +const isProgram = (obj: ProductPurchasableObject): boolean => + !isCourseRun(obj) && !("run_tag" in obj && obj.run_tag !== undefined) + +const matchesLine = ( + line: Line, + resourceId: number, + isVariant: (obj: ProductPurchasableObject) => boolean, +): boolean => { + const purchased = line.product.purchasable_object + return purchased?.id === resourceId && isVariant(purchased) +} + +/** + * Most recent fulfilled order covering a resource. History comes back + * newest-first. Refunded orders are excluded, matching `ReceiptByRunView` — + * revisit when the refund section is built. + */ +const useOrderIdForResource = ( + resourceId: number | null, + isVariant: (obj: ProductPurchasableObject) => boolean, +): OrderIdResolution => { + const history = useQuery({ + ...orderQueries.historyList({ limit: ORDER_HISTORY_LIMIT }), + enabled: resourceId !== null, + }) + + if (resourceId === null) { + return { isPending: false, isError: false, orderId: null } + } + if (history.isPending) { + return { isPending: true, isError: false, orderId: null } + } + if (history.isError || !history.data) { + return { isPending: false, isError: true, orderId: null } + } + + const match = history.data.results.find( + (order: OrderHistory) => + order.state === StateEnum.Fulfilled && + order.lines.some((line) => matchesLine(line, resourceId, isVariant)), + ) + + return { isPending: false, isError: false, orderId: match?.id ?? null } +} + +const useOrderIdForRun = (runId: number | null): OrderIdResolution => + useOrderIdForResource(runId, isCourseRun) + +const useOrderIdForProgram = (programId: number | null): OrderIdResolution => + useOrderIdForResource(programId, isProgram) + +export { useOrderIdForRun, useOrderIdForProgram } +export type { OrderIdResolution } diff --git a/frontends/main/src/common/urls.ts b/frontends/main/src/common/urls.ts index 2badba9ee3..2b61c9ba61 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -115,6 +115,20 @@ export const PROGRAM_VIEW = "/dashboard/program/[id]" export const programView = (id: number) => generatePath(PROGRAM_VIEW, { id: String(id) }) +export const RECEIPT_VIEW = "/receipt/[orderId]" +export const receiptView = (orderId: number) => + generatePath(RECEIPT_VIEW, { orderId: String(orderId) }) +/** + * Enrollments carry no order reference, so these routes resolve the order from the + * run/program before redirecting to `RECEIPT_VIEW`. + */ +export const RECEIPT_BY_RUN_VIEW = "/receipt/by-run/[runId]" +export const receiptByRunView = (runId: number) => + generatePath(RECEIPT_BY_RUN_VIEW, { runId: String(runId) }) +export const RECEIPT_BY_PROGRAM_VIEW = "/receipt/by-program/[programId]" +export const receiptByProgramView = (programId: number) => + generatePath(RECEIPT_BY_PROGRAM_VIEW, { programId: String(programId) }) + export const SEARCH = "/search" export const ABOUT = "/about" diff --git a/yarn.lock b/yarn.lock index 7680650f4e..6a60e76cf0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3631,13 +3631,13 @@ __metadata: languageName: node linkType: hard -"@mitodl/mitxonline-api-axios@npm:2026.8.6": - version: 2026.8.6 - resolution: "@mitodl/mitxonline-api-axios@npm:2026.8.6" +"@mitodl/mitxonline-api-axios@npm:2026.8.18": + version: 2026.8.18 + resolution: "@mitodl/mitxonline-api-axios@npm:2026.8.18" dependencies: "@types/node": "npm:^20.11.19" axios: "npm:^1.6.5" - checksum: 10/88e68269e1b9e5fb2e79d97ea7232e131dfc4a73d0cb6b0193b2a5cbc24af5eef8f3dbd4f9a63347fbcbd715364bf54b48a2eb9d32b71c7c18186d96ac4d3e8c + checksum: 10/5efd646218d5da28446b06da59327041a6604926828de8ea896847a37b96159a66f899482031888e2f4303af23fa4b684687b6bda893e14297da6541b2699a2d languageName: node linkType: hard @@ -9550,7 +9550,7 @@ __metadata: dependencies: "@faker-js/faker": "npm:^10.0.0" "@mitodl/mit-learn-api-axios": "npm:2026.7.22" - "@mitodl/mitxonline-api-axios": "npm:2026.8.6" + "@mitodl/mitxonline-api-axios": "npm:2026.8.18" "@tanstack/react-query": "npm:^5.66.0" "@testing-library/react": "npm:^16.3.0" axios: "npm:^1.12.2" @@ -16898,7 +16898,7 @@ __metadata: "@mitodl/arithmix": "npm:^0.2.3" "@mitodl/course-search-utils": "npm:^3.5.2" "@mitodl/hacksnack": "npm:^0.1.1" - "@mitodl/mitxonline-api-axios": "npm:2026.8.6" + "@mitodl/mitxonline-api-axios": "npm:2026.8.18" "@mitodl/smoot-design": "npm:6.31.1" "@mui/base": "npm:5.0.0-beta.70" "@mui/material": "npm:^6.4.5" From d5c892185d611b1afbacfe30d62e1c8110909a82 Mon Sep 17 00:00:00 2001 From: Danielle Frappier Date: Tue, 18 Aug 2026 15:15:57 -0400 Subject: [PATCH 4/6] feat: Collect required compliance fields before any enrollment or checkout (#3766) --- .../mitxonline/test-utils/factories/orders.ts | 2 +- .../mitxonline/test-utils/factories/user.ts | 12 +- .../DashboardDialogs.test.tsx | 369 +-------------- .../CoursewareDisplay/DashboardDialogs.tsx | 156 +------ .../CoursewareDisplay/EnrolledCourseCard.tsx | 7 + .../UnenrolledCourseCard.compliance.test.tsx | 97 ++++ .../UnenrolledCourseCard.test.tsx | 5 +- .../hooks/useEnrollmentHandler.ts | 50 +-- .../ProductPages/CourseEnrollArea.test.tsx | 3 + .../ProductPages/CoursePage.test.tsx | 3 + .../ProductPages/ProgramAsCoursePage.test.tsx | 3 + .../ProgramHeaderEnrollButton.test.tsx | 3 + .../ProductPages/useCourseEnrollment.test.tsx | 4 + .../ProductPages/useCourseEnrollment.ts | 7 +- .../useProgramEnrollment.test.tsx | 4 + .../ProductPages/useProgramEnrollment.ts | 7 +- frontends/main/src/app/providers.tsx | 5 +- .../mitxonline/useComplianceGate.test.tsx | 191 ++++++++ .../common/mitxonline/useComplianceGate.tsx | 115 +++++ .../mitxonline/useReplaceBasketItem.test.tsx | 35 +- .../common/mitxonline/useReplaceBasketItem.ts | 29 +- .../CourseEnrollmentDialog.test.tsx | 7 + .../CourseEnrollmentDialog.tsx | 5 + .../JustInTimeDialog.test.tsx | 424 ++++++++++++++++++ .../EnrollmentDialogs/JustInTimeDialog.tsx | 291 ++++++++++++ .../complianceFields.test.ts | 323 +++++++++++++ .../EnrollmentDialogs/complianceFields.ts | 312 +++++++++++++ frontends/main/src/test-utils/index.tsx | 5 +- 28 files changed, 1900 insertions(+), 574 deletions(-) create mode 100644 frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/UnenrolledCourseCard.compliance.test.tsx create mode 100644 frontends/main/src/common/mitxonline/useComplianceGate.test.tsx create mode 100644 frontends/main/src/common/mitxonline/useComplianceGate.tsx create mode 100644 frontends/main/src/page-components/EnrollmentDialogs/JustInTimeDialog.test.tsx create mode 100644 frontends/main/src/page-components/EnrollmentDialogs/JustInTimeDialog.tsx create mode 100644 frontends/main/src/page-components/EnrollmentDialogs/complianceFields.test.ts create mode 100644 frontends/main/src/page-components/EnrollmentDialogs/complianceFields.ts diff --git a/frontends/api/src/mitxonline/test-utils/factories/orders.ts b/frontends/api/src/mitxonline/test-utils/factories/orders.ts index 54051fe95b..e9ee249935 100644 --- a/frontends/api/src/mitxonline/test-utils/factories/orders.ts +++ b/frontends/api/src/mitxonline/test-utils/factories/orders.ts @@ -85,11 +85,11 @@ const order = (overrides: Partial = {}): Order => ({ lines: [transactionLine()], discounts: [], refunds: [], + refund_eligible: false, reference_number: faker.string.alphanumeric(10), created_on: faker.date.past().toISOString(), transactions: orderTransactions(), street_address: orderStreetAddress(), - refund_eligible: false, ...overrides, }) diff --git a/frontends/api/src/mitxonline/test-utils/factories/user.ts b/frontends/api/src/mitxonline/test-utils/factories/user.ts index 4e3614374a..3b16027837 100644 --- a/frontends/api/src/mitxonline/test-utils/factories/user.ts +++ b/frontends/api/src/mitxonline/test-utils/factories/user.ts @@ -12,7 +12,13 @@ const enforcerId = new UniqueEnforcer() const legalAddress = (): LegalAddress => ({ country: faker.location.countryCode(), - state: faker.datatype.boolean() ? faker.location.state() : null, + // Real values are ISO-3166-2 codes (e.g. "US-MA"), not the full names + // faker.location.state() returns, and only a handful of countries even + // have subdivisions. A random name for a random country is never a value + // a state