From f97e26ab69e0337827b6d49b1f5b4754184b4a2a Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 13:58:48 -0400 Subject: [PATCH 01/12] feat: add soft purge support to call_fastly_purge_api Co-Authored-By: Claude Fable 5 --- main/utils.py | 7 ++++++- main/utils_test.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/main/utils.py b/main/utils.py index db209a800f..64301d5af4 100644 --- a/main/utils.py +++ b/main/utils.py @@ -212,7 +212,7 @@ def inner_function(request, *args, **kwargs): return inner_decorator -def call_fastly_purge_api(relative_url, timeout=30): +def call_fastly_purge_api(relative_url, timeout=30, *, soft=False): """ Call the Fastly purge API. @@ -223,6 +223,8 @@ def call_fastly_purge_api(relative_url, timeout=30): Args: - relative_url The relative URL to purge. - timeout Timeout in seconds for the request (default: 30) + - soft If True, send a soft purge (Fastly-Soft-Purge: 1) so + Fastly marks the object stale instead of evicting it Returns: - Dict of the response (resp.json) Raises: @@ -242,6 +244,9 @@ def call_fastly_purge_api(relative_url, timeout=30): headers = {} + if soft: + headers["Fastly-Soft-Purge"] = "1" + if settings.FASTLY_API_KEY: headers["fastly-key"] = settings.FASTLY_API_KEY diff --git a/main/utils_test.py b/main/utils_test.py index adeb91f757..27a9399364 100644 --- a/main/utils_test.py +++ b/main/utils_test.py @@ -26,6 +26,7 @@ _sorted_query_string, cache_page_for_all_users, cache_page_for_anonymous_users, + call_fastly_purge_api, chunks, clean_data, clear_views_cache, @@ -678,3 +679,16 @@ def test_clear_views_cache_uses_large_itersize(mock_caches): mock_cache.delete_pattern.assert_called_once_with("views.*", itersize=1000) assert result == 3 + + +@pytest.mark.parametrize("soft", [True, False]) +def test_call_fastly_purge_api_soft_header(mocker, settings, soft): + """soft=True sends the Fastly-Soft-Purge: 1 header; default sends none""" + settings.FASTLY_API_KEY = "fake-key" + mock_request = mocker.patch("main.utils.requests.request") + mock_request.return_value.json.return_value = {"status": "ok"} + + call_fastly_purge_api("/c/unit/mitx", soft=soft) + + headers = mock_request.call_args.kwargs["headers"] + assert headers.get("Fastly-Soft-Purge") == ("1" if soft else None) From 41a8f77e3cc58c3e4bac64fddf68158007620f9b Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 14:03:45 -0400 Subject: [PATCH 02/12] feat: add clear_featured_caches task for featured-list cache invalidation Co-Authored-By: Claude Fable 5 --- learning_resources/tasks.py | 18 ++++++++++++++- learning_resources/tasks_test.py | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/learning_resources/tasks.py b/learning_resources/tasks.py index 6bd4b0a37e..e747a554db 100644 --- a/learning_resources/tasks.py +++ b/learning_resources/tasks.py @@ -13,6 +13,7 @@ from django.db import OperationalError from django.db.models import Q from django.utils import timezone +from requests.exceptions import RequestException from learning_resources.constants import LearningResourceType from learning_resources.etl import ovs, pipelines, youtube @@ -57,7 +58,7 @@ from main.celery import app from main.constants import ISOFORMAT from main.decorators import cooldown_task -from main.utils import chunks, clear_views_cache, now_in_utc +from main.utils import call_fastly_purge_api, chunks, clear_views_cache, now_in_utc log = logging.getLogger(__name__) @@ -78,6 +79,21 @@ def update_next_start_date_and_prices(): return len(resources) +@app.task(autoretry_for=(RequestException,), retry_backoff=True, max_retries=3) +def clear_featured_caches(channel_names): + """ + Clear cached featured-list data for the given unit channels: the Redis + view cache first, then Fastly pages so re-renders fetch fresh API data. + Channel pages are hard-purged (the editor's refresh must be decisively + fresh); the homepage is soft-purged to keep its stale-while-revalidate + grace for visitors. + """ + clear_views_cache(key_prefix="featured_resources") + for name in channel_names: + call_fastly_purge_api(f"/c/unit/{name}", timeout=5) + call_fastly_purge_api("/", timeout=5, soft=True) + + @app.task(acks_late=True, reject_on_worker_lost=True) @cooldown_task( wait_time=3600, diff --git a/learning_resources/tasks_test.py b/learning_resources/tasks_test.py index bfd9d591a0..59d0101cb9 100644 --- a/learning_resources/tasks_test.py +++ b/learning_resources/tasks_test.py @@ -1187,3 +1187,41 @@ def test_cleanup_deleted_content_files_returns_error_on_unexpected_exception(moc result = cleanup_deleted_content_files() assert result == "cleanup_deleted_content_files threw an error" + + +def test_clear_featured_caches(mocker): + """Clears the Redis prefix first, then purges channel pages hard and homepage soft""" + manager = mocker.Mock() + manager.attach_mock( + mocker.patch("learning_resources.tasks.clear_views_cache"), + "clear_views_cache", + ) + manager.attach_mock( + mocker.patch("learning_resources.tasks.call_fastly_purge_api"), "purge" + ) + + tasks.clear_featured_caches.run(["mitx", "ocw"]) + + assert manager.mock_calls == [ + mocker.call.clear_views_cache(key_prefix="featured_resources"), + mocker.call.purge("/c/unit/mitx", timeout=5), + mocker.call.purge("/c/unit/ocw", timeout=5), + mocker.call.purge("/", timeout=5, soft=True), + ] + + +def test_clear_featured_caches_retry_config(): + """Autoretries on network errors with backoff""" + from requests.exceptions import RequestException + + task = tasks.clear_featured_caches + assert RequestException in task.autoretry_for + assert task.max_retries == 3 + assert task.retry_backoff is True + + +def test_clear_featured_caches_unconfigured_fastly(mocker, settings): + """With no Fastly API key the task completes without error""" + settings.FASTLY_API_KEY = "" + mocker.patch("learning_resources.tasks.clear_views_cache") + tasks.clear_featured_caches.run(["mitx"]) From 247f58166d3c5720da0c99bbb7ed5bbf340da2d8 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 14:13:55 -0400 Subject: [PATCH 03/12] feat: clear featured caches on learning path update/delete Co-Authored-By: Claude Fable 5 --- learning_resources/views.py | 42 ++++++++ learning_resources/views_learningpath_test.py | 95 +++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/learning_resources/views.py b/learning_resources/views.py index 589b5482c3..5d918d5f08 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -479,6 +479,36 @@ def get_queryset(self): ).filter(published=True) +def _enqueue_featured_cache_clear(path_resource_ids): + """ + If any of the given learning paths is a unit channel's featured list, + enqueue a post-commit task to clear the featured-list caches. + + Never raises into the caller's request (best-effort, per hq#11979). + """ + channel_names = list( + Channel.objects.filter( + featured_list_id__in=path_resource_ids, + channel_type=ChannelType.unit.name, + ).values_list("name", flat=True) + ) + if not channel_names: + return + + def _delay_clear(): + from learning_resources import tasks + + try: + tasks.clear_featured_caches.delay(channel_names) + except Exception: + log.exception( + "Failed to enqueue featured cache clear for channels %s", + channel_names, + ) + + transaction.on_commit(_delay_clear) + + @extend_schema_view( list=extend_schema( summary="List", description="Get a paginated list of learning paths" @@ -543,6 +573,18 @@ def update(self, request, *_args, **kwargs): ) return Response(serializer.data) + def perform_update(self, serializer): + super().perform_update(serializer) + _enqueue_featured_cache_clear([serializer.instance.id]) + + def perform_destroy(self, instance): + # Resolve channel names before the delete (Channel.featured_list is + # on_delete=SET_NULL); the atomic block defers the on_commit enqueue + # until after the delete commits. + with transaction.atomic(): + _enqueue_featured_cache_clear([instance.id]) + super().perform_destroy(instance) + @extend_schema_view( list=extend_schema( diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index 0ff92afab3..4ad7e2390d 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -6,6 +6,7 @@ from django.db.models import Max from django.urls import reverse +from channels.factories import ChannelFactory from learning_resources import factories, models from learning_resources.constants import ( LearningResourceRelationTypes, @@ -565,3 +566,97 @@ def test_adding_to_learning_path_not_effect_existing_membership(client, staff_us new_additional_parent_count + 1 == new_additional_parent.learning_resource.resources.count() ) + + +@pytest.fixture +def mock_featured_clear(mocker): + """Mock the clear_featured_caches task""" + return mocker.patch("learning_resources.tasks.clear_featured_caches") + + +def test_learning_path_update_clears_featured_caches( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """PATCHing a featured learning path enqueues the cache-clear task on commit""" + update_editor_group(user, True) # noqa: FBT003 + path_resource = factories.LearningResourceFactory.create( + is_learning_path=True, learning_path__author=user, published=True + ) + channel = ChannelFactory.create(is_unit=True, featured_list=path_resource) + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.patch( + reverse("lr:v1:learningpaths_api-detail", args=[path_resource.id]), + data={"title": "New title"}, + format="json", + ) + + assert resp.status_code == 200 + mock_featured_clear.delay.assert_called_once_with([channel.name]) + + +def test_learning_path_update_not_featured_no_clear( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """PATCHing a learning path that is no channel's featured list enqueues nothing""" + update_editor_group(user, True) # noqa: FBT003 + path_resource = factories.LearningResourceFactory.create( + is_learning_path=True, learning_path__author=user, published=True + ) + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.patch( + reverse("lr:v1:learningpaths_api-detail", args=[path_resource.id]), + data={"title": "New title"}, + format="json", + ) + + assert resp.status_code == 200 + mock_featured_clear.delay.assert_not_called() + + +def test_learning_path_delete_clears_featured_caches( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """Deleting a featured path resolves channel names before the delete and enqueues""" + update_editor_group(user, True) # noqa: FBT003 + learning_path = factories.LearningPathFactory.create() + channel = ChannelFactory.create( + is_unit=True, featured_list=learning_path.learning_resource + ) + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.delete( + reverse( + "lr:v1:learningpaths_api-detail", + args=[learning_path.learning_resource.id], + ) + ) + + assert resp.status_code == 204 + mock_featured_clear.delay.assert_called_once_with([channel.name]) + + +def test_featured_cache_clear_enqueue_failure_does_not_break_save( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """A failing task enqueue (broker down) must not break the API response""" + mock_featured_clear.delay.side_effect = Exception("broker down") + update_editor_group(user, True) # noqa: FBT003 + path_resource = factories.LearningResourceFactory.create( + is_learning_path=True, learning_path__author=user, published=True + ) + ChannelFactory.create(is_unit=True, featured_list=path_resource) + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.patch( + reverse("lr:v1:learningpaths_api-detail", args=[path_resource.id]), + data={"title": "New title"}, + format="json", + ) + + assert resp.status_code == 200 From f0adc1278edc0372d6b1e9319855c7f74c111e04 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 14:22:12 -0400 Subject: [PATCH 04/12] feat: clear featured caches on learning path item edits Co-Authored-By: Claude Fable 5 --- learning_resources/views.py | 6 +- learning_resources/views_learningpath_test.py | 85 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/learning_resources/views.py b/learning_resources/views.py index 5d918d5f08..4711410588 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -887,6 +887,7 @@ def create(self, request, *args, **kwargs): # noqa: ARG002 serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) serializer.save(parent_id=self.kwargs.get("learning_resource_id")) + _enqueue_featured_cache_clear([self.kwargs.get("learning_resource_id")]) relationship = LearningResourceRelationship.objects.prefetch_related( Prefetch("child", queryset=LearningResource.objects.for_serialization()) @@ -899,7 +900,9 @@ def create(self, request, *args, **kwargs): # noqa: ARG002 return Response(response_serializer.data, status=201, headers=headers) def update(self, request, *args, **kwargs): - return super().update(request, *args, **kwargs) + response = super().update(request, *args, **kwargs) + _enqueue_featured_cache_clear([self.kwargs.get("learning_resource_id")]) + return response def perform_destroy(self, instance): """Delete the relationship and update the positions of the remaining items""" @@ -910,6 +913,7 @@ def perform_destroy(self, instance): position__gt=instance.position, ).update(position=F("position") - 1) instance.delete() + _enqueue_featured_cache_clear([instance.parent_id]) @extend_schema_view( diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index 4ad7e2390d..5ae9a28883 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -660,3 +660,88 @@ def test_featured_cache_clear_enqueue_failure_does_not_break_save( ) assert resp.status_code == 200 + + +def test_learning_path_item_create_clears_featured_caches( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """Adding an item to a featured path enqueues the cache-clear task""" + update_editor_group(user, True) # noqa: FBT003 + learning_path = factories.LearningPathFactory.create() + channel = ChannelFactory.create( + is_unit=True, featured_list=learning_path.learning_resource + ) + course = factories.CourseFactory.create() + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.post( + reverse( + "lr:v1:learningpathitems_api-list", + args=[learning_path.learning_resource.id], + ), + data={"child": course.learning_resource.id}, + format="json", + ) + + assert resp.status_code == 201 + mock_featured_clear.delay.assert_called_once_with([channel.name]) + + +def test_learning_path_item_update_clears_featured_caches( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """Reordering an item in a featured path enqueues the cache-clear task""" + update_editor_group(user, True) # noqa: FBT003 + learning_path = factories.LearningPathFactory.create() + learning_path.learning_resource.children.all().delete() + items = sorted( + factories.LearningPathRelationshipFactory.create_batch( + 2, parent=learning_path.learning_resource + ), + key=lambda item: item.position, + ) + channel = ChannelFactory.create( + is_unit=True, featured_list=learning_path.learning_resource + ) + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.patch( + reverse( + "lr:v1:learningpathitems_api-detail", + args=[learning_path.learning_resource.id, items[0].id], + ), + data={"position": items[1].position}, + format="json", + ) + + assert resp.status_code == 200 + mock_featured_clear.delay.assert_called_once_with([channel.name]) + + +def test_learning_path_item_delete_clears_featured_caches( + client, user, mock_featured_clear, django_capture_on_commit_callbacks +): + """Removing an item from a featured path enqueues the cache-clear task""" + update_editor_group(user, True) # noqa: FBT003 + learning_path = factories.LearningPathFactory.create() + learning_path.learning_resource.children.all().delete() + items = factories.LearningPathRelationshipFactory.create_batch( + 2, parent=learning_path.learning_resource + ) + channel = ChannelFactory.create( + is_unit=True, featured_list=learning_path.learning_resource + ) + client.force_login(user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.delete( + reverse( + "lr:v1:learningpathitems_api-detail", + args=[learning_path.learning_resource.id, items[0].id], + ) + ) + + assert resp.status_code == 204 + mock_featured_clear.delay.assert_called_once_with([channel.name]) From cfc8cde896026bbd55de652e8bcc0e27b45a2e59 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 14:31:55 -0400 Subject: [PATCH 05/12] feat: clear featured caches on bulk learning-path membership changes Co-Authored-By: Claude Fable 5 --- learning_resources/views.py | 6 ++++ learning_resources/views_learningpath_test.py | 31 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/learning_resources/views.py b/learning_resources/views.py index 4711410588..1e1efa0c8b 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -787,6 +787,9 @@ def learning_paths(self, request, *args, **kwargs): # noqa: ARG002 relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, parent__resource_type=LearningResourceType.learning_path.name, ) + previous_parent_ids = list( + current_relationships.values_list("parent_id", flat=True) + ) # Remove the resource from lists it WAS in before but is not in now current_relationships.exclude(parent_id__in=learning_path_ids).delete() current_parent_lists = current_relationships.values_list("parent_id", flat=True) @@ -813,6 +816,9 @@ def learning_paths(self, request, *args, **kwargs): # noqa: ARG002 relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, position=last_index + 1, ) + _enqueue_featured_cache_clear( + {*previous_parent_ids, *(int(pk) for pk in learning_path_ids)} + ) current_relationships = LearningResourceRelationship.objects.prefetch_related( Prefetch( "child", diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index 5ae9a28883..720a004569 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -745,3 +745,34 @@ def test_learning_path_item_delete_clears_featured_caches( assert resp.status_code == 204 mock_featured_clear.delay.assert_called_once_with([channel.name]) + + +def test_set_learning_path_relationships_clears_featured_caches( + client, staff_user, mock_featured_clear, django_capture_on_commit_callbacks +): + """Bulk membership set enqueues clears for both added and removed featured paths""" + course = factories.CourseFactory.create() + added_path = factories.LearningPathFactory.create(author=staff_user) + removed_path = factories.LearningPathFactory.create(author=staff_user) + factories.LearningPathRelationshipFactory.create( + parent=removed_path.learning_resource, child=course.learning_resource + ) + added_channel = ChannelFactory.create( + is_unit=True, featured_list=added_path.learning_resource + ) + removed_channel = ChannelFactory.create( + is_unit=True, featured_list=removed_path.learning_resource + ) + url = reverse( + "lr:v1:learning_resource_relationships_api-learning-paths", + args=[course.learning_resource.id], + ) + client.force_login(staff_user) + + with django_capture_on_commit_callbacks(execute=True): + resp = client.patch(f"{url}?learning_path_id={added_path.learning_resource.id}") + + assert resp.status_code == 200 + mock_featured_clear.delay.assert_called_once() + (names,) = mock_featured_clear.delay.call_args.args + assert sorted(names) == sorted([added_channel.name, removed_channel.name]) From 02f0038e6a153e7244d9dc78ede48c4e4220dd2d Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 15:05:25 -0400 Subject: [PATCH 06/12] fix: invalidate featured query on learning path edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The browser query client's 30-minute staleTime (matched to the CDN TTL) means the editor's own session never refetches the featured list after they edit a learning path — the backend purge only helps on a full page reload. Invalidate the featured query key in the mutations that map to the backend's cache-clearing write hooks so the editor sees their change immediately on client-side navigation too. Co-Authored-By: Claude Fable 5 --- .../api/src/hooks/learningPaths/index.test.ts | 30 +++++++++++++++++ .../api/src/hooks/learningPaths/index.ts | 10 ++++++ .../src/hooks/learningResources/index.test.ts | 32 +++++++++++++++++++ .../api/src/hooks/learningResources/index.ts | 3 ++ 4 files changed, 75 insertions(+) diff --git a/frontends/api/src/hooks/learningPaths/index.test.ts b/frontends/api/src/hooks/learningPaths/index.test.ts index 2cb774c371..ddfe2ab584 100644 --- a/frontends/api/src/hooks/learningPaths/index.test.ts +++ b/frontends/api/src/hooks/learningPaths/index.test.ts @@ -15,6 +15,7 @@ import { useLearningPathCreate, useLearningPathDestroy, useLearningPathUpdate, + useLearningPathListItemMove, } from "./index" import { learningPathKeys } from "./queries" @@ -163,6 +164,9 @@ describe("LearningPath CRUD", () => { expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: ["learningPaths", "membershipList"], }) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: learningResourceKeys.featuredRoot(), + }) }) test("useLearningPathUpdate calls correct API", async () => { @@ -190,5 +194,31 @@ describe("LearningPath CRUD", () => { expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: ["learningPaths", "detail", path.id], }) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: learningResourceKeys.featuredRoot(), + }) + }) + + test("useLearningPathListItemMove invalidates featured and items queries", async () => { + const { path, relationship, pathUrls } = makeData() + setMockResponse.patch(pathUrls.relationshipDetails, relationship) + + const { wrapper, queryClient } = setupReactQueryTest() + jest.spyOn(queryClient, "invalidateQueries") + + const { result } = renderHook(useLearningPathListItemMove, { wrapper }) + result.current.mutate({ + parent: path.id, + id: relationship.id, + position: relationship.position, + }) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: learningPathKeys.infiniteItemsRoot(path.id), + }) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: learningResourceKeys.featuredRoot(), + }) }) }) diff --git a/frontends/api/src/hooks/learningPaths/index.ts b/frontends/api/src/hooks/learningPaths/index.ts index 75c5912c1d..ada723a6eb 100644 --- a/frontends/api/src/hooks/learningPaths/index.ts +++ b/frontends/api/src/hooks/learningPaths/index.ts @@ -13,6 +13,7 @@ import type { } from "../../generated/v1" import { learningPathsApi } from "../../clients" import { learningPathQueries, learningPathKeys } from "./queries" +import { learningResourceKeys } from "../learningResources/queries" import { useUserHasPermission, Permission } from "api/hooks/user" const useLearningPathsList = ( @@ -71,6 +72,9 @@ const useLearningPathUpdate = () => { queryClient.invalidateQueries({ queryKey: learningPathKeys.detail(vars.id), }) + queryClient.invalidateQueries({ + queryKey: learningResourceKeys.featuredRoot(), + }) }, }) } @@ -85,6 +89,9 @@ const useLearningPathDestroy = () => { queryClient.invalidateQueries({ queryKey: learningPathKeys.membershipList(), }) + queryClient.invalidateQueries({ + queryKey: learningResourceKeys.featuredRoot(), + }) }, }) } @@ -109,6 +116,9 @@ const useLearningPathListItemMove = () => { queryClient.invalidateQueries({ queryKey: learningPathKeys.infiniteItemsRoot(vars.parent), }) + queryClient.invalidateQueries({ + queryKey: learningResourceKeys.featuredRoot(), + }) }, }) } diff --git a/frontends/api/src/hooks/learningResources/index.test.ts b/frontends/api/src/hooks/learningResources/index.test.ts index 8b7ae81098..9191b39659 100644 --- a/frontends/api/src/hooks/learningResources/index.test.ts +++ b/frontends/api/src/hooks/learningResources/index.test.ts @@ -9,7 +9,10 @@ import { useInfiniteLearningResourceItems, useLearningResourcesList, useLearningResourceTopics, + useLearningResourceSetLearningPathRelationships, } from "./index" +import { learningResourceKeys } from "./queries" +import { learningPathKeys } from "../learningPaths/queries" import { setMockResponse, urls, makeRequest } from "../../test-utils" import * as factories from "../../test-utils/factories" import { UseQueryResult } from "@tanstack/react-query" @@ -142,3 +145,32 @@ describe("useLearningResourceTopics", () => { }, ) }) + +describe("useLearningResourceSetLearningPathRelationships", () => { + it("invalidates learning path and featured queries", async () => { + const resource = factory.resource() + const url = urls.learningResources.setLearningPathRelationships({ + id: resource.id, + }) + setMockResponse.patch(url, resource) + + const { wrapper, queryClient } = setupReactQueryTest() + jest.spyOn(queryClient, "invalidateQueries") + + const { result } = renderHook( + useLearningResourceSetLearningPathRelationships, + { + wrapper, + }, + ) + result.current.mutate({ id: resource.id }) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: learningPathKeys.root, + }) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: learningResourceKeys.featuredRoot(), + }) + }) +}) diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts index 5144938a44..3c9d42d343 100644 --- a/frontends/api/src/hooks/learningResources/index.ts +++ b/frontends/api/src/hooks/learningResources/index.ts @@ -165,6 +165,9 @@ const useLearningResourceSetLearningPathRelationships = () => { * Additionally, the lists we've removed from the resource are not easily available. */ queryClient.invalidateQueries({ queryKey: learningPathKeys.root }) + queryClient.invalidateQueries({ + queryKey: learningResourceKeys.featuredRoot(), + }) }, }) } From 3c220e886858f6ad37fec3ab2ebbf55bb47a9f63 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 15:18:12 -0400 Subject: [PATCH 07/12] refactor: simplify featured cache clearing per review Hoist the tasks import to module level (tasks was already imported at module scope, so the function-local import bought nothing), shrink the bulk-action id set (featured_list_id__in coerces strings), and delete the tautological retry-config test. Co-Authored-By: Claude Fable 5 --- learning_resources/tasks_test.py | 10 ---------- learning_resources/views.py | 8 ++------ 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/learning_resources/tasks_test.py b/learning_resources/tasks_test.py index 59d0101cb9..4eed262858 100644 --- a/learning_resources/tasks_test.py +++ b/learning_resources/tasks_test.py @@ -1210,16 +1210,6 @@ def test_clear_featured_caches(mocker): ] -def test_clear_featured_caches_retry_config(): - """Autoretries on network errors with backoff""" - from requests.exceptions import RequestException - - task = tasks.clear_featured_caches - assert RequestException in task.autoretry_for - assert task.max_retries == 3 - assert task.retry_backoff is True - - def test_clear_featured_caches_unconfigured_fastly(mocker, settings): """With no Fastly API key the task completes without error""" settings.FASTLY_API_KEY = "" diff --git a/learning_resources/views.py b/learning_resources/views.py index 1e1efa0c8b..aedc9b20a9 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -30,7 +30,7 @@ from authentication.decorators import blocked_ip_exempt from channels.constants import ChannelType from channels.models import Channel -from learning_resources import permissions +from learning_resources import permissions, tasks from learning_resources.constants import ( GROUP_CONTENT_FILE_CONTENT_VIEWERS, LearningResourceRelationTypes, @@ -496,8 +496,6 @@ def _enqueue_featured_cache_clear(path_resource_ids): return def _delay_clear(): - from learning_resources import tasks - try: tasks.clear_featured_caches.delay(channel_names) except Exception: @@ -816,9 +814,7 @@ def learning_paths(self, request, *args, **kwargs): # noqa: ARG002 relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, position=last_index + 1, ) - _enqueue_featured_cache_clear( - {*previous_parent_ids, *(int(pk) for pk in learning_path_ids)} - ) + _enqueue_featured_cache_clear({*previous_parent_ids, *learning_path_ids}) current_relationships = LearningResourceRelationship.objects.prefetch_related( Prefetch( "child", From 7c7363c1c14a0244d9d94a5d8822c7c1b04bb1d4 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 11 Aug 2026 15:32:17 -0400 Subject: [PATCH 08/12] refactor: condense featured cache-clear tests via fixture and parametrize Co-Authored-By: Claude Fable 5 --- learning_resources/views_learningpath_test.py | 124 +++++++----------- 1 file changed, 48 insertions(+), 76 deletions(-) diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index 720a004569..cb78f3bbd2 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -574,25 +574,43 @@ def mock_featured_clear(mocker): return mocker.patch("learning_resources.tasks.clear_featured_caches") -def test_learning_path_update_clears_featured_caches( - client, user, mock_featured_clear, django_capture_on_commit_callbacks -): - """PATCHing a featured learning path enqueues the cache-clear task on commit""" +@pytest.fixture +def featured_path(client, user): + """Create a learning path featured by a unit channel and log in an editor""" update_editor_group(user, True) # noqa: FBT003 - path_resource = factories.LearningResourceFactory.create( - is_learning_path=True, learning_path__author=user, published=True - ) - channel = ChannelFactory.create(is_unit=True, featured_list=path_resource) + path = factories.LearningPathFactory.create(author=user) + channel = ChannelFactory.create(is_unit=True, featured_list=path.learning_resource) client.force_login(user) + return path, channel + + +@pytest.mark.parametrize( + ("method", "data", "expected_status"), + [("patch", {"title": "New title"}, 200), ("delete", None, 204)], +) +def test_learning_path_write_clears_featured_caches( # noqa: PLR0913 + client, + featured_path, + mock_featured_clear, + django_capture_on_commit_callbacks, + method, + data, + expected_status, +): + """ + Updating or deleting a featured learning path enqueues the cache-clear + task on commit (delete resolves channel names before the row is gone) + """ + path, channel = featured_path with django_capture_on_commit_callbacks(execute=True): - resp = client.patch( - reverse("lr:v1:learningpaths_api-detail", args=[path_resource.id]), - data={"title": "New title"}, + resp = getattr(client, method)( + reverse("lr:v1:learningpaths_api-detail", args=[path.learning_resource.id]), + data=data, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == expected_status mock_featured_clear.delay.assert_called_once_with([channel.name]) @@ -601,14 +619,12 @@ def test_learning_path_update_not_featured_no_clear( ): """PATCHing a learning path that is no channel's featured list enqueues nothing""" update_editor_group(user, True) # noqa: FBT003 - path_resource = factories.LearningResourceFactory.create( - is_learning_path=True, learning_path__author=user, published=True - ) + path = factories.LearningPathFactory.create(author=user) client.force_login(user) with django_capture_on_commit_callbacks(execute=True): resp = client.patch( - reverse("lr:v1:learningpaths_api-detail", args=[path_resource.id]), + reverse("lr:v1:learningpaths_api-detail", args=[path.learning_resource.id]), data={"title": "New title"}, format="json", ) @@ -617,44 +633,16 @@ def test_learning_path_update_not_featured_no_clear( mock_featured_clear.delay.assert_not_called() -def test_learning_path_delete_clears_featured_caches( - client, user, mock_featured_clear, django_capture_on_commit_callbacks -): - """Deleting a featured path resolves channel names before the delete and enqueues""" - update_editor_group(user, True) # noqa: FBT003 - learning_path = factories.LearningPathFactory.create() - channel = ChannelFactory.create( - is_unit=True, featured_list=learning_path.learning_resource - ) - client.force_login(user) - - with django_capture_on_commit_callbacks(execute=True): - resp = client.delete( - reverse( - "lr:v1:learningpaths_api-detail", - args=[learning_path.learning_resource.id], - ) - ) - - assert resp.status_code == 204 - mock_featured_clear.delay.assert_called_once_with([channel.name]) - - def test_featured_cache_clear_enqueue_failure_does_not_break_save( - client, user, mock_featured_clear, django_capture_on_commit_callbacks + client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): """A failing task enqueue (broker down) must not break the API response""" mock_featured_clear.delay.side_effect = Exception("broker down") - update_editor_group(user, True) # noqa: FBT003 - path_resource = factories.LearningResourceFactory.create( - is_learning_path=True, learning_path__author=user, published=True - ) - ChannelFactory.create(is_unit=True, featured_list=path_resource) - client.force_login(user) + path, _ = featured_path with django_capture_on_commit_callbacks(execute=True): resp = client.patch( - reverse("lr:v1:learningpaths_api-detail", args=[path_resource.id]), + reverse("lr:v1:learningpaths_api-detail", args=[path.learning_resource.id]), data={"title": "New title"}, format="json", ) @@ -663,22 +651,16 @@ def test_featured_cache_clear_enqueue_failure_does_not_break_save( def test_learning_path_item_create_clears_featured_caches( - client, user, mock_featured_clear, django_capture_on_commit_callbacks + client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): """Adding an item to a featured path enqueues the cache-clear task""" - update_editor_group(user, True) # noqa: FBT003 - learning_path = factories.LearningPathFactory.create() - channel = ChannelFactory.create( - is_unit=True, featured_list=learning_path.learning_resource - ) + path, channel = featured_path course = factories.CourseFactory.create() - client.force_login(user) with django_capture_on_commit_callbacks(execute=True): resp = client.post( reverse( - "lr:v1:learningpathitems_api-list", - args=[learning_path.learning_resource.id], + "lr:v1:learningpathitems_api-list", args=[path.learning_resource.id] ), data={"child": course.learning_resource.id}, format="json", @@ -689,28 +671,23 @@ def test_learning_path_item_create_clears_featured_caches( def test_learning_path_item_update_clears_featured_caches( - client, user, mock_featured_clear, django_capture_on_commit_callbacks + client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): """Reordering an item in a featured path enqueues the cache-clear task""" - update_editor_group(user, True) # noqa: FBT003 - learning_path = factories.LearningPathFactory.create() - learning_path.learning_resource.children.all().delete() + path, channel = featured_path + path.learning_resource.children.all().delete() items = sorted( factories.LearningPathRelationshipFactory.create_batch( - 2, parent=learning_path.learning_resource + 2, parent=path.learning_resource ), key=lambda item: item.position, ) - channel = ChannelFactory.create( - is_unit=True, featured_list=learning_path.learning_resource - ) - client.force_login(user) with django_capture_on_commit_callbacks(execute=True): resp = client.patch( reverse( "lr:v1:learningpathitems_api-detail", - args=[learning_path.learning_resource.id, items[0].id], + args=[path.learning_resource.id, items[0].id], ), data={"position": items[1].position}, format="json", @@ -721,25 +698,20 @@ def test_learning_path_item_update_clears_featured_caches( def test_learning_path_item_delete_clears_featured_caches( - client, user, mock_featured_clear, django_capture_on_commit_callbacks + client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): """Removing an item from a featured path enqueues the cache-clear task""" - update_editor_group(user, True) # noqa: FBT003 - learning_path = factories.LearningPathFactory.create() - learning_path.learning_resource.children.all().delete() + path, channel = featured_path + path.learning_resource.children.all().delete() items = factories.LearningPathRelationshipFactory.create_batch( - 2, parent=learning_path.learning_resource - ) - channel = ChannelFactory.create( - is_unit=True, featured_list=learning_path.learning_resource + 2, parent=path.learning_resource ) - client.force_login(user) with django_capture_on_commit_callbacks(execute=True): resp = client.delete( reverse( "lr:v1:learningpathitems_api-detail", - args=[learning_path.learning_resource.id, items[0].id], + args=[path.learning_resource.id, items[0].id], ) ) From dac6fa992d991d47642fc4f03e152280d67ccedb Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 12 Aug 2026 08:47:38 -0400 Subject: [PATCH 09/12] fix: guard against null learning_path in list card count Co-Authored-By: Claude Fable 5 --- .../LearningResourceCard/LearningResourceListCard.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 802c24d4e6..7132187540 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -91,7 +91,10 @@ export const Count = ({ resource }: { resource: LearningResource }) => { if (resource.resource_type !== ResourceTypeEnum.LearningPath) { return null } - const count = resource.learning_path.item_count + const count = resource.learning_path?.item_count + if (count === undefined) { + return null + } return (
{count} {pluralize("item", count)} From ae862050fc3a872ce5755a839daba11f83519253 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 12 Aug 2026 09:00:56 -0400 Subject: [PATCH 10/12] refactor: trim docstrings and drop redundant fastly no-key test Co-Authored-By: Claude Fable 5 --- learning_resources/tasks.py | 7 ++----- learning_resources/tasks_test.py | 7 ------- learning_resources/views.py | 6 ++---- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/learning_resources/tasks.py b/learning_resources/tasks.py index e747a554db..e473a7c7c8 100644 --- a/learning_resources/tasks.py +++ b/learning_resources/tasks.py @@ -82,11 +82,8 @@ def update_next_start_date_and_prices(): @app.task(autoretry_for=(RequestException,), retry_backoff=True, max_retries=3) def clear_featured_caches(channel_names): """ - Clear cached featured-list data for the given unit channels: the Redis - view cache first, then Fastly pages so re-renders fetch fresh API data. - Channel pages are hard-purged (the editor's refresh must be decisively - fresh); the homepage is soft-purged to keep its stale-while-revalidate - grace for visitors. + Clear the Redis featured-list cache, hard-purge channel pages from + Fastly, and soft-purge the homepage. """ clear_views_cache(key_prefix="featured_resources") for name in channel_names: diff --git a/learning_resources/tasks_test.py b/learning_resources/tasks_test.py index 4eed262858..23a17433bf 100644 --- a/learning_resources/tasks_test.py +++ b/learning_resources/tasks_test.py @@ -1208,10 +1208,3 @@ def test_clear_featured_caches(mocker): mocker.call.purge("/c/unit/ocw", timeout=5), mocker.call.purge("/", timeout=5, soft=True), ] - - -def test_clear_featured_caches_unconfigured_fastly(mocker, settings): - """With no Fastly API key the task completes without error""" - settings.FASTLY_API_KEY = "" - mocker.patch("learning_resources.tasks.clear_views_cache") - tasks.clear_featured_caches.run(["mitx"]) diff --git a/learning_resources/views.py b/learning_resources/views.py index aedc9b20a9..fbb9f9e79f 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -481,10 +481,8 @@ def get_queryset(self): def _enqueue_featured_cache_clear(path_resource_ids): """ - If any of the given learning paths is a unit channel's featured list, - enqueue a post-commit task to clear the featured-list caches. - - Never raises into the caller's request (best-effort, per hq#11979). + Enqueue a post-commit featured-cache clear if any of the given paths is + a unit channel's featured list; best-effort, never raises. """ channel_names = list( Channel.objects.filter( From 274d725c770e1a8e5fbfa79466796f5040fbaab9 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 12 Aug 2026 11:14:52 -0400 Subject: [PATCH 11/12] refactor: clear featured caches synchronously instead of via Celery The cache-clear task landed on the default Celery queue, which also runs ETL backpopulates and bulk indexing, so a purge could sit behind minutes of queued work -- defeating the point of the feature (editor sees changes within seconds). The purge work is cheap (tuned Redis pattern delete plus two Fastly calls with 5s timeouts) and only triggered by rare human saves, so run it best-effort in the request via transaction.on_commit. Co-Authored-By: Claude Fable 5 --- learning_resources/tasks.py | 15 +---- learning_resources/tasks_test.py | 21 ------- learning_resources/views.py | 52 +++++++++++----- learning_resources/views_learningpath_test.py | 61 +++++++++++++------ 4 files changed, 78 insertions(+), 71 deletions(-) diff --git a/learning_resources/tasks.py b/learning_resources/tasks.py index e473a7c7c8..6bd4b0a37e 100644 --- a/learning_resources/tasks.py +++ b/learning_resources/tasks.py @@ -13,7 +13,6 @@ from django.db import OperationalError from django.db.models import Q from django.utils import timezone -from requests.exceptions import RequestException from learning_resources.constants import LearningResourceType from learning_resources.etl import ovs, pipelines, youtube @@ -58,7 +57,7 @@ from main.celery import app from main.constants import ISOFORMAT from main.decorators import cooldown_task -from main.utils import call_fastly_purge_api, chunks, clear_views_cache, now_in_utc +from main.utils import chunks, clear_views_cache, now_in_utc log = logging.getLogger(__name__) @@ -79,18 +78,6 @@ def update_next_start_date_and_prices(): return len(resources) -@app.task(autoretry_for=(RequestException,), retry_backoff=True, max_retries=3) -def clear_featured_caches(channel_names): - """ - Clear the Redis featured-list cache, hard-purge channel pages from - Fastly, and soft-purge the homepage. - """ - clear_views_cache(key_prefix="featured_resources") - for name in channel_names: - call_fastly_purge_api(f"/c/unit/{name}", timeout=5) - call_fastly_purge_api("/", timeout=5, soft=True) - - @app.task(acks_late=True, reject_on_worker_lost=True) @cooldown_task( wait_time=3600, diff --git a/learning_resources/tasks_test.py b/learning_resources/tasks_test.py index 23a17433bf..bfd9d591a0 100644 --- a/learning_resources/tasks_test.py +++ b/learning_resources/tasks_test.py @@ -1187,24 +1187,3 @@ def test_cleanup_deleted_content_files_returns_error_on_unexpected_exception(moc result = cleanup_deleted_content_files() assert result == "cleanup_deleted_content_files threw an error" - - -def test_clear_featured_caches(mocker): - """Clears the Redis prefix first, then purges channel pages hard and homepage soft""" - manager = mocker.Mock() - manager.attach_mock( - mocker.patch("learning_resources.tasks.clear_views_cache"), - "clear_views_cache", - ) - manager.attach_mock( - mocker.patch("learning_resources.tasks.call_fastly_purge_api"), "purge" - ) - - tasks.clear_featured_caches.run(["mitx", "ocw"]) - - assert manager.mock_calls == [ - mocker.call.clear_views_cache(key_prefix="featured_resources"), - mocker.call.purge("/c/unit/mitx", timeout=5), - mocker.call.purge("/c/unit/ocw", timeout=5), - mocker.call.purge("/", timeout=5, soft=True), - ] diff --git a/learning_resources/views.py b/learning_resources/views.py index fbb9f9e79f..5b0cb8622e 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -30,7 +30,7 @@ from authentication.decorators import blocked_ip_exempt from channels.constants import ChannelType from channels.models import Channel -from learning_resources import permissions, tasks +from learning_resources import permissions from learning_resources.constants import ( GROUP_CONTENT_FILE_CONTENT_VIEWERS, LearningResourceRelationTypes, @@ -109,7 +109,13 @@ AnonymousAccessReadonlyPermission, is_admin_user, ) -from main.utils import cache_page_for_all_users, cache_page_for_anonymous_users, chunks +from main.utils import ( + cache_page_for_all_users, + cache_page_for_anonymous_users, + call_fastly_purge_api, + chunks, + clear_views_cache, +) from vector_search.serializers import LearningResourcesSearchFiltersSerializer @@ -479,10 +485,24 @@ def get_queryset(self): ).filter(published=True) -def _enqueue_featured_cache_clear(path_resource_ids): +def clear_featured_caches(channel_names): + """ + Clear the Redis featured-list cache, hard-purge channel pages from + Fastly, and soft-purge the homepage. + """ + clear_views_cache(key_prefix="featured_resources") + for name in channel_names: + call_fastly_purge_api(f"/c/unit/{name}", timeout=5) + call_fastly_purge_api("/", timeout=5, soft=True) + + +def _clear_featured_caches_on_commit(path_resource_ids): """ - Enqueue a post-commit featured-cache clear if any of the given paths is - a unit channel's featured list; best-effort, never raises. + Clear the featured caches after commit if any of the given paths is a + unit channel's featured list; best-effort, never raises. + + Runs synchronously in the request (not via Celery) so the purge is done + by the time the editor's save returns, regardless of worker backlog. """ channel_names = list( Channel.objects.filter( @@ -493,16 +513,16 @@ def _enqueue_featured_cache_clear(path_resource_ids): if not channel_names: return - def _delay_clear(): + def _clear(): try: - tasks.clear_featured_caches.delay(channel_names) + clear_featured_caches(channel_names) except Exception: log.exception( - "Failed to enqueue featured cache clear for channels %s", + "Failed to clear featured caches for channels %s", channel_names, ) - transaction.on_commit(_delay_clear) + transaction.on_commit(_clear) @extend_schema_view( @@ -571,14 +591,14 @@ def update(self, request, *_args, **kwargs): def perform_update(self, serializer): super().perform_update(serializer) - _enqueue_featured_cache_clear([serializer.instance.id]) + _clear_featured_caches_on_commit([serializer.instance.id]) def perform_destroy(self, instance): # Resolve channel names before the delete (Channel.featured_list is - # on_delete=SET_NULL); the atomic block defers the on_commit enqueue + # on_delete=SET_NULL); the atomic block defers the on_commit clear # until after the delete commits. with transaction.atomic(): - _enqueue_featured_cache_clear([instance.id]) + _clear_featured_caches_on_commit([instance.id]) super().perform_destroy(instance) @@ -812,7 +832,7 @@ def learning_paths(self, request, *args, **kwargs): # noqa: ARG002 relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, position=last_index + 1, ) - _enqueue_featured_cache_clear({*previous_parent_ids, *learning_path_ids}) + _clear_featured_caches_on_commit({*previous_parent_ids, *learning_path_ids}) current_relationships = LearningResourceRelationship.objects.prefetch_related( Prefetch( "child", @@ -887,7 +907,7 @@ def create(self, request, *args, **kwargs): # noqa: ARG002 serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) serializer.save(parent_id=self.kwargs.get("learning_resource_id")) - _enqueue_featured_cache_clear([self.kwargs.get("learning_resource_id")]) + _clear_featured_caches_on_commit([self.kwargs.get("learning_resource_id")]) relationship = LearningResourceRelationship.objects.prefetch_related( Prefetch("child", queryset=LearningResource.objects.for_serialization()) @@ -901,7 +921,7 @@ def create(self, request, *args, **kwargs): # noqa: ARG002 def update(self, request, *args, **kwargs): response = super().update(request, *args, **kwargs) - _enqueue_featured_cache_clear([self.kwargs.get("learning_resource_id")]) + _clear_featured_caches_on_commit([self.kwargs.get("learning_resource_id")]) return response def perform_destroy(self, instance): @@ -913,7 +933,7 @@ def perform_destroy(self, instance): position__gt=instance.position, ).update(position=F("position") - 1) instance.delete() - _enqueue_featured_cache_clear([instance.parent_id]) + _clear_featured_caches_on_commit([instance.parent_id]) @extend_schema_view( diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index cb78f3bbd2..e139af6ccc 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -7,7 +7,7 @@ from django.urls import reverse from channels.factories import ChannelFactory -from learning_resources import factories, models +from learning_resources import factories, models, views from learning_resources.constants import ( LearningResourceRelationTypes, ) @@ -570,8 +570,8 @@ def test_adding_to_learning_path_not_effect_existing_membership(client, staff_us @pytest.fixture def mock_featured_clear(mocker): - """Mock the clear_featured_caches task""" - return mocker.patch("learning_resources.tasks.clear_featured_caches") + """Mock the synchronous clear_featured_caches function""" + return mocker.patch("learning_resources.views.clear_featured_caches") @pytest.fixture @@ -598,8 +598,8 @@ def test_learning_path_write_clears_featured_caches( # noqa: PLR0913 expected_status, ): """ - Updating or deleting a featured learning path enqueues the cache-clear - task on commit (delete resolves channel names before the row is gone) + Updating or deleting a featured learning path clears the featured caches + on commit (delete resolves channel names before the row is gone) """ path, channel = featured_path @@ -611,13 +611,13 @@ def test_learning_path_write_clears_featured_caches( # noqa: PLR0913 ) assert resp.status_code == expected_status - mock_featured_clear.delay.assert_called_once_with([channel.name]) + mock_featured_clear.assert_called_once_with([channel.name]) def test_learning_path_update_not_featured_no_clear( client, user, mock_featured_clear, django_capture_on_commit_callbacks ): - """PATCHing a learning path that is no channel's featured list enqueues nothing""" + """PATCHing a learning path that is no channel's featured list clears nothing""" update_editor_group(user, True) # noqa: FBT003 path = factories.LearningPathFactory.create(author=user) client.force_login(user) @@ -630,14 +630,14 @@ def test_learning_path_update_not_featured_no_clear( ) assert resp.status_code == 200 - mock_featured_clear.delay.assert_not_called() + mock_featured_clear.assert_not_called() -def test_featured_cache_clear_enqueue_failure_does_not_break_save( +def test_featured_cache_clear_failure_does_not_break_save( client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): - """A failing task enqueue (broker down) must not break the API response""" - mock_featured_clear.delay.side_effect = Exception("broker down") + """A failing cache clear (Redis/Fastly down) must not break the API response""" + mock_featured_clear.side_effect = Exception("broker down") path, _ = featured_path with django_capture_on_commit_callbacks(execute=True): @@ -653,7 +653,7 @@ def test_featured_cache_clear_enqueue_failure_does_not_break_save( def test_learning_path_item_create_clears_featured_caches( client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): - """Adding an item to a featured path enqueues the cache-clear task""" + """Adding an item to a featured path clears the featured caches""" path, channel = featured_path course = factories.CourseFactory.create() @@ -667,13 +667,13 @@ def test_learning_path_item_create_clears_featured_caches( ) assert resp.status_code == 201 - mock_featured_clear.delay.assert_called_once_with([channel.name]) + mock_featured_clear.assert_called_once_with([channel.name]) def test_learning_path_item_update_clears_featured_caches( client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): - """Reordering an item in a featured path enqueues the cache-clear task""" + """Reordering an item in a featured path clears the featured caches""" path, channel = featured_path path.learning_resource.children.all().delete() items = sorted( @@ -694,13 +694,13 @@ def test_learning_path_item_update_clears_featured_caches( ) assert resp.status_code == 200 - mock_featured_clear.delay.assert_called_once_with([channel.name]) + mock_featured_clear.assert_called_once_with([channel.name]) def test_learning_path_item_delete_clears_featured_caches( client, featured_path, mock_featured_clear, django_capture_on_commit_callbacks ): - """Removing an item from a featured path enqueues the cache-clear task""" + """Removing an item from a featured path clears the featured caches""" path, channel = featured_path path.learning_resource.children.all().delete() items = factories.LearningPathRelationshipFactory.create_batch( @@ -716,13 +716,13 @@ def test_learning_path_item_delete_clears_featured_caches( ) assert resp.status_code == 204 - mock_featured_clear.delay.assert_called_once_with([channel.name]) + mock_featured_clear.assert_called_once_with([channel.name]) def test_set_learning_path_relationships_clears_featured_caches( client, staff_user, mock_featured_clear, django_capture_on_commit_callbacks ): - """Bulk membership set enqueues clears for both added and removed featured paths""" + """Bulk membership set clears caches for both added and removed featured paths""" course = factories.CourseFactory.create() added_path = factories.LearningPathFactory.create(author=staff_user) removed_path = factories.LearningPathFactory.create(author=staff_user) @@ -745,6 +745,27 @@ def test_set_learning_path_relationships_clears_featured_caches( resp = client.patch(f"{url}?learning_path_id={added_path.learning_resource.id}") assert resp.status_code == 200 - mock_featured_clear.delay.assert_called_once() - (names,) = mock_featured_clear.delay.call_args.args + mock_featured_clear.assert_called_once() + (names,) = mock_featured_clear.call_args.args assert sorted(names) == sorted([added_channel.name, removed_channel.name]) + + +def test_clear_featured_caches(mocker): + """Clears the Redis prefix first, then purges channel pages hard and homepage soft""" + manager = mocker.Mock() + manager.attach_mock( + mocker.patch("learning_resources.views.clear_views_cache"), + "clear_views_cache", + ) + manager.attach_mock( + mocker.patch("learning_resources.views.call_fastly_purge_api"), "purge" + ) + + views.clear_featured_caches(["mitx", "ocw"]) + + assert manager.mock_calls == [ + mocker.call.clear_views_cache(key_prefix="featured_resources"), + mocker.call.purge("/c/unit/mitx", timeout=5), + mocker.call.purge("/c/unit/ocw", timeout=5), + mocker.call.purge("/", timeout=5, soft=True), + ] From 8f153490e43aa17b1c3126c1af5f075423df5326 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 12 Aug 2026 11:26:57 -0400 Subject: [PATCH 12/12] fix: make each Fastly purge independently best-effort A failed channel-page purge no longer aborts the homepage purge (or other channel purges); each failure is logged and the loop continues. Co-Authored-By: Claude Fable 5 --- learning_resources/views.py | 13 +++++++++---- learning_resources/views_learningpath_test.py | 18 ++++++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/learning_resources/views.py b/learning_resources/views.py index 5b0cb8622e..f659a87a15 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -19,6 +19,7 @@ inline_serializer, ) from grpc._channel import _InactiveRpcError +from requests.exceptions import RequestException from rest_framework import serializers, views, viewsets from rest_framework.decorators import action from rest_framework.filters import OrderingFilter @@ -488,12 +489,16 @@ def get_queryset(self): def clear_featured_caches(channel_names): """ Clear the Redis featured-list cache, hard-purge channel pages from - Fastly, and soft-purge the homepage. + Fastly, and soft-purge the homepage. Each Fastly purge is independently + best-effort so one failure doesn't leave the remaining pages stale. """ clear_views_cache(key_prefix="featured_resources") - for name in channel_names: - call_fastly_purge_api(f"/c/unit/{name}", timeout=5) - call_fastly_purge_api("/", timeout=5, soft=True) + purges = [(f"/c/unit/{name}", False) for name in channel_names] + [("/", True)] + for relative_url, soft in purges: + try: + call_fastly_purge_api(relative_url, timeout=5, soft=soft) + except RequestException: + log.exception("Featured cache Fastly purge failed for %s", relative_url) def _clear_featured_caches_on_commit(path_resource_ids): diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index e139af6ccc..9835bb7d50 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -5,6 +5,7 @@ import pytest from django.db.models import Max from django.urls import reverse +from requests.exceptions import RequestException from channels.factories import ChannelFactory from learning_resources import factories, models, views @@ -765,7 +766,20 @@ def test_clear_featured_caches(mocker): assert manager.mock_calls == [ mocker.call.clear_views_cache(key_prefix="featured_resources"), - mocker.call.purge("/c/unit/mitx", timeout=5), - mocker.call.purge("/c/unit/ocw", timeout=5), + mocker.call.purge("/c/unit/mitx", timeout=5, soft=False), + mocker.call.purge("/c/unit/ocw", timeout=5, soft=False), mocker.call.purge("/", timeout=5, soft=True), ] + + +def test_clear_featured_caches_continues_after_purge_failure(mocker): + """A failed channel purge must not skip the remaining Fastly purges""" + mocker.patch("learning_resources.views.clear_views_cache") + mock_purge = mocker.patch( + "learning_resources.views.call_fastly_purge_api", + side_effect=[RequestException("fastly down"), None, None], + ) + + views.clear_featured_caches(["mitx", "ocw"]) + + assert mock_purge.call_count == 3