From 07e2bdb08f464dda895bc6c2a5a3aa2c66339ae1 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Thu, 11 Jun 2026 20:01:11 -0400 Subject: [PATCH 01/20] feat: add recently-published Algolia replica for "newest courses first" sort Indexes recently_published_timestamp (the course's earliest published run start, sharing the is_course_new_content signal) and configures a desc()-sorted Algolia replica so the Learner Portal search can offer a "newest courses first" sort. The replica is only declared/configured when RECENTLY_PUBLISHED_REPLICA_INDEX_NAME is set (safe no-op until ops provisions it); the name is also added to the secured-API-key restrictIndices so the MFE's scoped key can query it. ENT-11384 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../algolia-search-sort-replicas.md | 67 +++++++++++++++++ enterprise_catalog/apps/api_client/algolia.py | 34 +++++++++ .../apps/api_client/tests/test_algolia.py | 32 +++++++++ .../apps/catalog/algolia_utils.py | 72 +++++++++++++++++-- .../apps/catalog/tests/test_algolia_utils.py | 51 +++++++++++++ enterprise_catalog/settings/base.py | 1 + 6 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 docs/references/algolia-search-sort-replicas.md diff --git a/docs/references/algolia-search-sort-replicas.md b/docs/references/algolia-search-sort-replicas.md new file mode 100644 index 00000000..b8ba42bd --- /dev/null +++ b/docs/references/algolia-search-sort-replicas.md @@ -0,0 +1,67 @@ +# Algolia search sort replicas ("newest courses first") + +How sort order is implemented for the Learner Portal search page, and how the +"newest courses first" sort is wired end-to-end. + +## How sorting works in Algolia here + +The enterprise catalog is indexed into a **primary** Algolia index whose name comes +from `settings.ALGOLIA['INDEX_NAME']`. Algolia does not re-sort a single index at query +time; instead each sort order is a **replica** index with its own `customRanking`. The +Learner Portal MFE switches sort by pointing its search at a different index name. + +Replicas are declared on the primary index's settings (`ALGOLIA_INDEX_SETTINGS['replicas']`) +and each replica's ranking is set with its own settings call. All of this lives in +[`enterprise_catalog/apps/catalog/algolia_utils.py`](../../enterprise_catalog/apps/catalog/algolia_utils.py) +and is applied by `configure_algolia_index()` during a full reindex. + +| Index | Setting key (`settings.ALGOLIA[...]`) | Leading `customRanking` | Used by | +|-------|----------------------------------------|--------------------------|---------| +| primary (relevance) | `INDEX_NAME` | text relevance + `desc(course_bayesian_average)` | default search | +| duration replica | `REPLICA_INDEX_NAME` | `desc(duration)` | video search | +| recently-published replica | `RECENTLY_PUBLISHED_REPLICA_INDEX_NAME` | `desc(recently_published_timestamp)` | "newest courses first" | + +## "Newest courses first" + +- **What "newest" means:** a course's *earliest published course-run start* — i.e. when the + course first became available. This is the same signal as the `is_new_content` flag + (ENT-11384); both are derived from `_earliest_published_course_run_start()`, which ignores + unpublished/draft runs so backfilled drafts can't change a course's recency. +- **The sort attribute:** `recently_published_timestamp` — a Unix timestamp (int) computed by + `get_course_recently_published_timestamp()` and added to each course Algolia object. + Courses with no published run start get `0` so they sort **last** under the `desc` ranking. + > Do not use `ALGOLIA_DEFAULT_TIMESTAMP` for the "missing" case — it is a far-future + > sentinel (year 3000) and would float undated courses to the *top* of a newest-first sort. +- **The replica:** `ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS` leads with + `desc(recently_published_timestamp)` and then keeps the primary index's tie-breakers. + +## Deployment / ops dependency + +`ALGOLIA` is **not** in `DICT_UPDATE_KEYS` in +[`settings/production.py`](../../enterprise_catalog/settings/production.py), so the deployment +YAML *replaces* the entire `ALGOLIA` dict rather than merging into the `base.py` default. +Consequences: + +1. The `base.py` default (`'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ''`) only applies in + local/test. **Enabling the replica in stage/prod requires ops to add + `RECENTLY_PUBLISHED_REPLICA_INDEX_NAME` to the `ALGOLIA` block in `edx-internal`.** +2. The code guards on the name being set: the replica is only declared on the primary index + and only configured when a name is present, so deploying this code *before* ops adds the + name is a safe no-op (no `virtual(None)` replica is created). +3. After the name is configured, a **full reindex** (`reindex_algolia`) must run so the + replica is created and `recently_published_timestamp` is populated on every object. + +The replica index name is also added to the secured-API-key `restrictIndices` in +[`api_client/algolia.py`](../../enterprise_catalog/apps/api_client/algolia.py) so the MFE's +scoped search key is permitted to query it. + +## End-to-end rollout (cross-repo) + +This service only produces the sorted replica. The user-facing sort is gated and measured in +the other two repos: + +- **`edx-enterprise`** — a `search_default_sort_newest` waffle flag surfaced via + `enterprise_features`, used as the eligibility gate / kill-switch. +- **`frontend-app-learner-portal-enterprise`** — points the course `` at the + recently-published replica when the flag is on **and** the Optimizely Web experiment buckets + the user into the "newest" variant; control keeps the relevance index. diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index 65dfc422..85788195 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -46,6 +46,10 @@ def algolia_index_name(self): def algolia_replica_index_name(self): return settings.ALGOLIA.get('REPLICA_INDEX_NAME') + @property + def algolia_recently_published_replica_index_name(self): + return settings.ALGOLIA.get('RECENTLY_PUBLISHED_REPLICA_INDEX_NAME') + def init_index(self): """ Initializes an index within Algolia. Initializing an index will create it if it doesn't exist. @@ -111,6 +115,34 @@ def set_index_settings(self, index_settings, primary_index=True): ) raise exc + def set_replica_index_settings(self, index_settings, replica_index_name): + """ + Set settings on a specific replica index by name. + + Unlike ``set_index_settings(..., primary_index=False)`` (which targets the single + ``REPLICA_INDEX_NAME`` replica), this configures any replica the primary index has + declared in its ``replicas`` setting -- used for the additional "recently published" + sort replica. The replica must already be declared on the primary index (Algolia + creates replica indices when the primary index's settings are saved). + + Arguments: + index_settings (dict): A dictionary of Algolia settings. + replica_index_name (str): The name of the replica index to configure. + """ + if not self.algolia_index: + logger.error('Algolia index does not exist. Did you initialize it?') + return + + try: + replica_index = self._get_index(replica_index_name) + replica_index.set_settings(index_settings) + except AlgoliaException as exc: + logger.exception( + 'Unable to set settings for Algolia replica index %s due to an exception.', + replica_index_name, + ) + raise exc + def index_exists(self): """ Returns whether the index exists in Algolia. @@ -424,6 +456,8 @@ def generate_secured_api_key(self, user_id, enterprise_catalog_query_uuids): indices.append(self.algolia_index_name) if self.algolia_replica_index_name: indices.append(self.algolia_replica_index_name) + if self.algolia_recently_published_replica_index_name: + indices.append(self.algolia_recently_published_replica_index_name) if indices: restrictions |= {'restrictIndices': indices} diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index 93f32d8d..98f9da3c 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -87,6 +87,38 @@ def test_get_index_raises_when_primary_uninitialized(self): with self.assertRaises(ImproperlyConfigured): client._get_index() + def test_set_replica_index_settings_targets_named_replica(self): + """ + ``set_replica_index_settings`` applies settings to the replica resolved by name. + """ + client = self._build_client() + replica = mock.MagicMock(name='recently_published_replica') + client._client.init_index.return_value = replica + settings_payload = {'customRanking': ['desc(recently_published_timestamp)']} + + client.set_replica_index_settings(settings_payload, 'enterprise_catalog_recently_published_desc') + + client._client.init_index.assert_called_once_with('enterprise_catalog_recently_published_desc') + replica.set_settings.assert_called_once_with(settings_payload) + + def test_set_replica_index_settings_noop_when_primary_uninitialized(self): + """ + With no primary index initialized, the call logs and returns without touching Algolia. + """ + client = AlgoliaSearchClient() # nothing initialized + client.set_replica_index_settings({'customRanking': []}, 'some_replica') # must not raise + + def test_set_replica_index_settings_reraises_algolia_exception(self): + """ + An AlgoliaException from set_settings propagates to the caller. + """ + client = self._build_client() + replica = mock.MagicMock(name='recently_published_replica') + replica.set_settings.side_effect = AlgoliaException('boom') + client._client.init_index.return_value = replica + with self.assertRaises(AlgoliaException): + client.set_replica_index_settings({'customRanking': []}, 'some_replica') + def test_save_objects_batch_calls_save_on_primary(self): """ ``save_objects_batch`` delegates to ``save_objects`` on the primary index by default. diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index a18c1e42..1527ded4 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -65,8 +65,20 @@ ALGOLIA_JSON_METADATA_MAX_SIZE = 100000 ALGOLIA_REPLICA_INDEX_NAME = settings.ALGOLIA.get('REPLICA_INDEX_NAME') +# Optional second replica that sorts courses by recency (newest-first). Only declared when a +# name is configured (see settings.ALGOLIA); absent in local/test until ops sets it in the +# deployment config. +ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME = settings.ALGOLIA.get('RECENTLY_PUBLISHED_REPLICA_INDEX_NAME') algolia_replica_index = f'virtual({ALGOLIA_REPLICA_INDEX_NAME})' +algolia_recently_published_replica_index = f'virtual({ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME})' + +# Replicas declared on the primary index. The recency replica is only included when its name +# is configured, so deploying this code before ops adds the name won't declare a +# ``virtual(None)`` replica on the primary index. +ALGOLIA_REPLICAS = [algolia_replica_index] +if ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME: + ALGOLIA_REPLICAS.append(algolia_recently_published_replica_index) # keep attributes from content objects that we explicitly want in Algolia ALGOLIA_FIELDS = [ @@ -142,6 +154,7 @@ 'transcript_languages', 'translation_languages', 'is_new_content', + 'recently_published_timestamp', ] # default configuration for the index @@ -206,9 +219,7 @@ 'desc(course_bayesian_average)', 'desc(recent_enrollment_count)', ], - 'replicas': [ - algolia_replica_index - ], + 'replicas': ALGOLIA_REPLICAS, } ALGOLIA_REPLICA_INDEX_SETTINGS = { @@ -222,6 +233,21 @@ ], } +# Replica that sorts courses by recency (newest-first). Leads with the precomputed +# ``recently_published_timestamp`` (the course's earliest published run start) descending, +# then falls back to the primary index's tie-breakers. Surfaced in the Learner Portal search +# page as the "newest courses first" sort. +ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS = { + 'customRanking': [ + 'desc(recently_published_timestamp)', + 'asc(metadata_language)', + 'asc(visible_via_association)', + 'asc(created)', + 'desc(course_bayesian_average)', + 'desc(recent_enrollment_count)', + ], +} + def _should_index_course(course_metadata): """ @@ -417,6 +443,11 @@ def configure_algolia_index(algolia_client): """ algolia_client.set_index_settings(ALGOLIA_INDEX_SETTINGS) algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, primary_index=False) + if ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME: + algolia_client.set_replica_index_settings( + ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME, + ) def get_algolia_object_id(content_type, uuid): @@ -632,18 +663,44 @@ def is_course_archived(course): return len(availability_list) == 0 or 'Archived' in availability_list -def is_course_new_content(course): - """True if the course's earliest run start (any status) is within the last 12 months (ENT-11386).""" +def _earliest_published_course_run_start(course): + """ + The earliest course-run start as a timezone-aware datetime, or None. + + Shared by the "new content" facet and the "recently published" sort timestamp so both key off + the same signal: a course's earliest run start across runs of *any* status, matching the + Discovery course release date (ENT-11386). Earlier runs that were later unpublished/archived + still count, so a recent re-run can't make a long-standing course look new. + """ starts = [] for run in course.get('course_runs') or []: if run.get('start'): parsed = parse_datetime(run['start']) if parsed: starts.append(parsed) - if not starts: + return min(starts) if starts else None + + +def is_course_new_content(course): + """True if the course's earliest run start (any status) is within the last 12 months (ENT-11386).""" + earliest_start = _earliest_published_course_run_start(course) + if earliest_start is None: return False now = localized_utcnow() - return now - relativedelta(months=12) <= min(starts) <= now + return now - relativedelta(months=12) <= earliest_start <= now + + +def get_course_recently_published_timestamp(course): + """ + Unix timestamp (int) of the course's earliest course-run start (any status), used as the + custom-ranking attribute for the "newest courses first" Algolia replica. + + Sorting this descending surfaces courses released most recently. Courses with no run start + default to 0 so they sort last under a desc ranking -- ``ALGOLIA_DEFAULT_TIMESTAMP`` (a + far-future sentinel) is deliberately NOT used, since it would float undated courses to the top. + """ + earliest_start = _earliest_published_course_run_start(course) + return int(earliest_start.timestamp()) if earliest_start else 0 def get_course_partners(course): @@ -1674,6 +1731,7 @@ def _algolia_object_from_product(product, algolia_fields): 'translation_languages': get_course_translation_languages(searchable_product), 'metadata_language': searchable_product.get('metadata_language', 'en'), 'is_new_content': is_course_new_content(searchable_product), + 'recently_published_timestamp': get_course_recently_published_timestamp(searchable_product), }) elif searchable_product.get('content_type') == PROGRAM: # Build course metadata cache once for all program functions that need it diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index d76e5a29..29cdb2ae 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -247,6 +247,41 @@ def test_is_course_new_content(self): {'status': 'published', 'start': 'not-a-date'}, ]}) is False + def test_get_course_recently_published_timestamp(self): + """ + Verify the "newest courses" sort timestamp uses the earliest run start (any status). + """ + now = localized_utcnow() + recent = now - timedelta(days=30) + old = now - timedelta(days=500) + + # No run start -> 0 so the course sorts last under a desc ranking. + assert utils.get_course_recently_published_timestamp({'course_runs': []}) == 0 + # Malformed ISO strings are silently ignored (treated as no date). + assert utils.get_course_recently_published_timestamp( + {'course_runs': [{'status': 'published', 'start': 'not-a-date'}]} + ) == 0 + # Run status is irrelevant: an unpublished run's start still counts. + assert utils.get_course_recently_published_timestamp( + {'course_runs': [{'status': 'unpublished', 'start': recent.isoformat()}]} + ) == int(recent.timestamp()) + # A run start -> its Unix timestamp, regardless of age (unlike is_new_content's 12-month window). + assert utils.get_course_recently_published_timestamp( + {'course_runs': [{'status': 'published', 'start': recent.isoformat()}]} + ) == int(recent.timestamp()) + assert utils.get_course_recently_published_timestamp( + {'course_runs': [{'status': 'published', 'start': old.isoformat()}]} + ) == int(old.timestamp()) + # Earliest start wins across any status: an old unpublished run is the release date. + assert utils.get_course_recently_published_timestamp({'course_runs': [ + {'status': 'unpublished', 'start': old.isoformat()}, + {'status': 'published', 'start': recent.isoformat()}, + ]}) == int(old.timestamp()) + assert utils.get_course_recently_published_timestamp({'course_runs': [ + {'status': 'published', 'start': old.isoformat()}, + {'status': 'published', 'start': recent.isoformat()}, + ]}) == int(old.timestamp()) + @ddt.data( ( { @@ -997,6 +1032,22 @@ def test_configure_algolia_index(self, mock_search_client): utils.ALGOLIA_REPLICA_INDEX_SETTINGS, primary_index=False ) + # No recently-published replica name is configured by default, so that replica is skipped. + mock_search_client.return_value.set_replica_index_settings.assert_not_called() + + @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') + def test_configure_algolia_index_configures_recently_published_replica(self, mock_search_client): + """ + When a recently-published replica name is configured, its settings are applied too. + """ + algolia_client = utils.get_initialized_algolia_client() + replica_name = 'enterprise_catalog_recently_published_desc' + with mock.patch.object(utils, 'ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', replica_name): + utils.configure_algolia_index(algolia_client) + mock_search_client.return_value.set_replica_index_settings.assert_called_once_with( + utils.ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + replica_name, + ) @ddt.data( ( diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index 02aba971..54cd8234 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -435,6 +435,7 @@ ALGOLIA = { 'INDEX_NAME': '', 'REPLICA_INDEX_NAME': '', + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': '', 'APPLICATION_ID': '', 'API_KEY': '', } From 2ddbfd6ac8460a611122e44cf6eddb4225cfbf46 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Thu, 11 Jun 2026 20:28:28 -0400 Subject: [PATCH 02/20] docs: convert newest-sort reference doc to ADR 0014 Reframes the docs/references note as docs/decisions/0014, an ADR whose central (proposed) decision is how to handle the recency replica being unavailable: rely on config-gating (covered today) + rollout order + the waffle-flag kill-switch, rather than runtime index-existence detection. Records the runtime-fallback alternative as the escape hatch. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 107 ++++++++++++++++++ .../algolia-search-sort-replicas.md | 67 ----------- 2 files changed, 107 insertions(+), 67 deletions(-) create mode 100644 docs/decisions/0014-newest-courses-sort-replica.rst delete mode 100644 docs/references/algolia-search-sort-replicas.md diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst new file mode 100644 index 00000000..ec55b8af --- /dev/null +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -0,0 +1,107 @@ +Newest-Courses-First Search Sort via a Recency-Sorted Algolia Replica +===================================================================== + +Status +------ + +Proposed + +Context +------- + +The enterprise Learner Portal search page sorts a single Algolia index by +relevance. Algolia does not re-sort an index at query time, so each alternate +sort order is a separate *replica* index with its own ``customRanking``; the +consumer switches sort by pointing its search at a different index name. To +offer a "newest courses first" sort we add a recency-sorted replica. + +* The primary index (``ALGOLIA['INDEX_NAME']``) keeps the relevance ranking. +* A new replica (``ALGOLIA['RECENTLY_PUBLISHED_REPLICA_INDEX_NAME']``) leads its + ranking with ``desc(recently_published_timestamp)`` — a per-course Unix + timestamp of the *earliest published course-run start* (the same signal as the + ``is_new_content`` flag, via the shared ``_earliest_published_course_run_start`` + helper). Courses with no published run start get ``0`` so they sort last under + a descending ranking — deliberately not the far-future ``ALGOLIA_DEFAULT_TIMESTAMP``, + which would float undated courses to the top. + +The sort is rolled out across three repositories: + +#. **enterprise-catalog** (this service) builds and configures the replica. +#. **edx-enterprise** exposes the ``enterprise.search_default_sort_newest`` + waffle flag via ``enterprise_features`` — the eligibility gate / kill-switch. +#. **frontend-app-learner-portal-enterprise** points the course ```` at + the replica when the flag is on *and* the Optimizely "newest" experiment + variant is active for the user. + +Two facts shape the failure modes: + +* ``ALGOLIA`` is *replaced* (not merged) from the deployment YAML, so the replica + is only live once ops sets ``RECENTLY_PUBLISHED_REPLICA_INDEX_NAME`` in + ``edx-internal`` and a ``reindex_algolia`` run declares it on the primary. +* An Algolia *virtual* replica exists as soon as it is declared on the primary + index's settings (it mirrors the primary's records); it does not wait for a + populated record set. So once this service is deployed and ``reindex_algolia`` + has run, the replica exists. + +Decision +-------- + +The replica is **config-gated on both sides** and is never queried unless its +name is configured: + +* Backend: the replica is declared on the primary index and its settings are + applied **only** when ``RECENTLY_PUBLISHED_REPLICA_INDEX_NAME`` is set; + otherwise it is a no-op (no ``virtual(None)`` replica is created). +* MFE: the course search uses the replica only when its index-name config var is + non-empty (and the flag + experiment gates pass); otherwise it falls back to + the primary (relevance) index. + +**The open question this ADR records:** how should we handle the case where the +replica *name is configured* but the Algolia index does **not yet exist** (e.g. +the MFE env var is set before this service has been deployed and reindexed)? + +The proposed answer is to **rely on operational guarantees rather than runtime +index-existence detection**: + +#. the documented rollout order — deploy + ``reindex_algolia`` here *before* + pointing the MFE at the replica; and +#. the ``enterprise.search_default_sort_newest`` waffle flag, which doubles as a + readiness gate and an instant kill-switch — it should not be enabled until the + replica is live, and flipping it off immediately reverts every learner to the + relevance index. + +We deliberately do **not** add code that detects a missing Algolia index at +search time and silently falls back to the primary index. + +Consequences +------------ + +* **Covered:** "replica name not configured" → the base (relevance) index is + used. This is handled explicitly in both the backend (conditional replica + declaration) and the MFE (the ``&& recentlyPublishedIndexName`` guard), so the + default-to-base behavior is guaranteed for the unconfigured case. +* **Not covered in code:** "replica name configured but the Algolia index does + not exist yet" → the MFE would query a missing index and surface an error / + empty results rather than falling back. This is a transient, + operator-controlled window mitigated by the rollout order and the kill-switch + flag, not by code. +* **The waffle flag is the readiness contract:** enabling it asserts "the replica + is live." This keeps the safe path a single, instantly reversible toggle + rather than per-request defensive logic in the search hot path. +* **Escape hatch is recorded:** if the operational mitigation proves too fragile + in practice, the documented next step is an ``onError``/try-primary fallback in + the MFE search path (see *Alternatives*). Capturing the question here lets us + revisit it without re-discovering the trade-off. + +Alternatives considered +------------------------ + +* **Runtime index-existence detection / fall back to base on Algolia error.** + Rejected for now: react-instantsearch would need an error path that re-renders + against the primary index, adding per-search complexity and an extra failure + mode, to protect a transient window the kill-switch flag already guards. It + remains the documented escape hatch if the operational approach proves + insufficient. +* **Always declare the replica (no config gate).** Rejected: would create a + ``virtual(None)`` replica on the primary index in environments where the name + is unset, and would couple every environment to the rollout. diff --git a/docs/references/algolia-search-sort-replicas.md b/docs/references/algolia-search-sort-replicas.md deleted file mode 100644 index b8ba42bd..00000000 --- a/docs/references/algolia-search-sort-replicas.md +++ /dev/null @@ -1,67 +0,0 @@ -# Algolia search sort replicas ("newest courses first") - -How sort order is implemented for the Learner Portal search page, and how the -"newest courses first" sort is wired end-to-end. - -## How sorting works in Algolia here - -The enterprise catalog is indexed into a **primary** Algolia index whose name comes -from `settings.ALGOLIA['INDEX_NAME']`. Algolia does not re-sort a single index at query -time; instead each sort order is a **replica** index with its own `customRanking`. The -Learner Portal MFE switches sort by pointing its search at a different index name. - -Replicas are declared on the primary index's settings (`ALGOLIA_INDEX_SETTINGS['replicas']`) -and each replica's ranking is set with its own settings call. All of this lives in -[`enterprise_catalog/apps/catalog/algolia_utils.py`](../../enterprise_catalog/apps/catalog/algolia_utils.py) -and is applied by `configure_algolia_index()` during a full reindex. - -| Index | Setting key (`settings.ALGOLIA[...]`) | Leading `customRanking` | Used by | -|-------|----------------------------------------|--------------------------|---------| -| primary (relevance) | `INDEX_NAME` | text relevance + `desc(course_bayesian_average)` | default search | -| duration replica | `REPLICA_INDEX_NAME` | `desc(duration)` | video search | -| recently-published replica | `RECENTLY_PUBLISHED_REPLICA_INDEX_NAME` | `desc(recently_published_timestamp)` | "newest courses first" | - -## "Newest courses first" - -- **What "newest" means:** a course's *earliest published course-run start* — i.e. when the - course first became available. This is the same signal as the `is_new_content` flag - (ENT-11384); both are derived from `_earliest_published_course_run_start()`, which ignores - unpublished/draft runs so backfilled drafts can't change a course's recency. -- **The sort attribute:** `recently_published_timestamp` — a Unix timestamp (int) computed by - `get_course_recently_published_timestamp()` and added to each course Algolia object. - Courses with no published run start get `0` so they sort **last** under the `desc` ranking. - > Do not use `ALGOLIA_DEFAULT_TIMESTAMP` for the "missing" case — it is a far-future - > sentinel (year 3000) and would float undated courses to the *top* of a newest-first sort. -- **The replica:** `ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS` leads with - `desc(recently_published_timestamp)` and then keeps the primary index's tie-breakers. - -## Deployment / ops dependency - -`ALGOLIA` is **not** in `DICT_UPDATE_KEYS` in -[`settings/production.py`](../../enterprise_catalog/settings/production.py), so the deployment -YAML *replaces* the entire `ALGOLIA` dict rather than merging into the `base.py` default. -Consequences: - -1. The `base.py` default (`'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ''`) only applies in - local/test. **Enabling the replica in stage/prod requires ops to add - `RECENTLY_PUBLISHED_REPLICA_INDEX_NAME` to the `ALGOLIA` block in `edx-internal`.** -2. The code guards on the name being set: the replica is only declared on the primary index - and only configured when a name is present, so deploying this code *before* ops adds the - name is a safe no-op (no `virtual(None)` replica is created). -3. After the name is configured, a **full reindex** (`reindex_algolia`) must run so the - replica is created and `recently_published_timestamp` is populated on every object. - -The replica index name is also added to the secured-API-key `restrictIndices` in -[`api_client/algolia.py`](../../enterprise_catalog/apps/api_client/algolia.py) so the MFE's -scoped search key is permitted to query it. - -## End-to-end rollout (cross-repo) - -This service only produces the sorted replica. The user-facing sort is gated and measured in -the other two repos: - -- **`edx-enterprise`** — a `search_default_sort_newest` waffle flag surfaced via - `enterprise_features`, used as the eligibility gate / kill-switch. -- **`frontend-app-learner-portal-enterprise`** — points the course `` at the - recently-published replica when the flag is on **and** the Optimizely Web experiment buckets - the user into the "newest" variant; control keeps the relevance index. From 91cd95b9adedf8c59ad2e841f19d2b761ffd188c Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Thu, 11 Jun 2026 21:10:43 -0400 Subject: [PATCH 03/20] test: cover recency replica build, secured-key restrictIndices, and course-object field Raises patch coverage on the newest-sort change: refactors the module-level replica-list construction into a testable _build_algolia_replicas() (covers both the configured and unconfigured branches); adds a _algolia_object_from_product course test asserting recently_published_timestamp; and adds generate_secured_api_key tests covering the recency replica being included/omitted from restrictIndices (the property and both branches). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../apps/api_client/tests/test_algolia.py | 43 ++++++++++++++++++- .../apps/catalog/algolia_utils.py | 25 +++++++---- .../apps/catalog/tests/test_algolia_utils.py | 20 +++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index 98f9da3c..54a482af 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -6,7 +6,7 @@ import ddt from algoliasearch.exceptions import AlgoliaException from django.core.exceptions import ImproperlyConfigured -from django.test import TestCase +from django.test import TestCase, override_settings from enterprise_catalog.apps.api_client.algolia import AlgoliaSearchClient @@ -119,6 +119,47 @@ def test_set_replica_index_settings_reraises_algolia_exception(self): with self.assertRaises(AlgoliaException): client.set_replica_index_settings({'customRanking': []}, 'some_replica') + @override_settings(ALGOLIA={ + 'INDEX_NAME': 'enterprise_catalog', + 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_published_desc', + 'SEARCH_API_KEY': 'fake-search-key', + }) + @mock.patch('enterprise_catalog.apps.api_client.algolia.SearchClient.generate_secured_api_key') + def test_generate_secured_api_key_restricts_to_all_indices(self, mock_generate): + """The secured key's restrictIndices covers the primary, duration, and recency replicas.""" + mock_generate.return_value = 'secured-key' + client = AlgoliaSearchClient() + + result = client.generate_secured_api_key('user-1', ['query-uuid-1']) + + assert result['secured_api_key'] == 'secured-key' + _api_key, restrictions = mock_generate.call_args[0] + assert restrictions['restrictIndices'] == [ + 'enterprise_catalog', + 'enterprise_catalog_duration_desc', + 'enterprise_catalog_recently_published_desc', + ] + + @override_settings(ALGOLIA={ + 'INDEX_NAME': 'enterprise_catalog', + 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', + 'SEARCH_API_KEY': 'fake-search-key', + }) + @mock.patch('enterprise_catalog.apps.api_client.algolia.SearchClient.generate_secured_api_key') + def test_generate_secured_api_key_omits_recency_replica_when_unset(self, mock_generate): + """When no recency replica is configured, it is excluded from restrictIndices.""" + mock_generate.return_value = 'secured-key' + client = AlgoliaSearchClient() + + client.generate_secured_api_key('user-1', ['query-uuid-1']) + + _api_key, restrictions = mock_generate.call_args[0] + assert restrictions['restrictIndices'] == [ + 'enterprise_catalog', + 'enterprise_catalog_duration_desc', + ] + def test_save_objects_batch_calls_save_on_primary(self): """ ``save_objects_batch`` delegates to ``save_objects`` on the primary index by default. diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 1527ded4..196c841b 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -71,14 +71,23 @@ ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME = settings.ALGOLIA.get('RECENTLY_PUBLISHED_REPLICA_INDEX_NAME') algolia_replica_index = f'virtual({ALGOLIA_REPLICA_INDEX_NAME})' -algolia_recently_published_replica_index = f'virtual({ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME})' - -# Replicas declared on the primary index. The recency replica is only included when its name -# is configured, so deploying this code before ops adds the name won't declare a -# ``virtual(None)`` replica on the primary index. -ALGOLIA_REPLICAS = [algolia_replica_index] -if ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME: - ALGOLIA_REPLICAS.append(algolia_recently_published_replica_index) + + +def _build_algolia_replicas(recently_published_replica_index_name): + """ + Build the list of replica indexes to declare on the primary index. + + The recency replica is only included when its name is configured, so deploying this code + before ops sets the name won't declare a ``virtual(None)`` replica on the primary index. + """ + replicas = [algolia_replica_index] + if recently_published_replica_index_name: + replicas.append(f'virtual({recently_published_replica_index_name})') + return replicas + + +# Replicas declared on the primary index (see ``_build_algolia_replicas``). +ALGOLIA_REPLICAS = _build_algolia_replicas(ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME) # keep attributes from content objects that we explicitly want in Algolia ALGOLIA_FIELDS = [ diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 29cdb2ae..f24d888a 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -282,6 +282,26 @@ def test_get_course_recently_published_timestamp(self): {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) + def test_build_algolia_replicas_includes_recency_replica_when_configured(self): + """The recency replica is declared only when its index name is configured.""" + assert utils._build_algolia_replicas('enterprise_catalog_recently_published_desc') == [ + utils.algolia_replica_index, + 'virtual(enterprise_catalog_recently_published_desc)', + ] + # Unconfigured (empty / None) -> only the base replica, never virtual(None). + assert utils._build_algolia_replicas('') == [utils.algolia_replica_index] + assert utils._build_algolia_replicas(None) == [utils.algolia_replica_index] + + def test_algolia_object_includes_recently_published_timestamp(self): + """The course Algolia object carries recently_published_timestamp (and is_new_content).""" + course_metadata = ContentMetadataFactory(content_type=COURSE) + algolia_object = utils._algolia_object_from_product( + course_metadata.json_metadata, utils.ALGOLIA_FIELDS, + ) + expected = utils.get_course_recently_published_timestamp(course_metadata.json_metadata) + assert algolia_object['recently_published_timestamp'] == expected + assert 'is_new_content' in algolia_object + @ddt.data( ( { From 4e0f2a5f75295cfdb5e85df30333d80a6abf81d3 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Thu, 11 Jun 2026 21:47:48 -0400 Subject: [PATCH 04/20] style: silence pylint protected-access in new algolia replica tests The recency-replica coverage tests call the module-level _build_algolia_replicas and _algolia_object_from_product helpers directly. Add the method-scoped # pylint: disable=protected-access used elsewhere in this file so CI lint passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- enterprise_catalog/apps/catalog/tests/test_algolia_utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index f24d888a..b7a7ff45 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -284,6 +284,7 @@ def test_get_course_recently_published_timestamp(self): def test_build_algolia_replicas_includes_recency_replica_when_configured(self): """The recency replica is declared only when its index name is configured.""" + # pylint: disable=protected-access assert utils._build_algolia_replicas('enterprise_catalog_recently_published_desc') == [ utils.algolia_replica_index, 'virtual(enterprise_catalog_recently_published_desc)', @@ -294,6 +295,7 @@ def test_build_algolia_replicas_includes_recency_replica_when_configured(self): def test_algolia_object_includes_recently_published_timestamp(self): """The course Algolia object carries recently_published_timestamp (and is_new_content).""" + # pylint: disable=protected-access course_metadata = ContentMetadataFactory(content_type=COURSE) algolia_object = utils._algolia_object_from_product( course_metadata.json_metadata, utils.ALGOLIA_FIELDS, From f02d2d67b2211eb0802a37ea7900f76b19e1c577 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Fri, 12 Jun 2026 11:42:21 -0400 Subject: [PATCH 05/20] refactor: unify set_index_settings to target any index by name Replace the two-valued primary_index boolean with an optional index_name that routes through the existing _get_index() helper (which already defaults to the primary index). This folds the separate set_replica_index_settings method back into set_index_settings: one method now configures the primary, the base REPLICA_INDEX_NAME replica, and the recently-published replica uniformly. Removes the near-duplicate method (same guard + try/except/log) and the docstring cross-reference that signalled the overlap. All three call sites live in configure_algolia_index; tests updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) --- enterprise_catalog/apps/api_client/algolia.py | 47 +++++-------------- .../apps/api_client/tests/test_algolia.py | 26 +++++++--- .../apps/catalog/algolia_utils.py | 6 +-- .../apps/catalog/tests/test_algolia_utils.py | 16 ++++--- 4 files changed, 42 insertions(+), 53 deletions(-) diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index 85788195..98c8467a 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -89,57 +89,32 @@ def init_index(self): ) raise exc - def set_index_settings(self, index_settings, primary_index=True): + def set_index_settings(self, index_settings, index_name=None): """ - Set default settings to use for the Algolia index. + Set settings on an Algolia index, defaulting to the primary index. + + Pass ``index_name`` to target a replica instead -- the base ``REPLICA_INDEX_NAME`` + replica, the "recently published" sort replica, etc. A replica must already be + declared on the primary index's ``replicas`` setting (Algolia creates replica + indices when the primary index's settings are saved). Note: This will override manual updates to the index configuration on the Algolia dashboard but ensures consistent settings (configuration as code). - Arguments: - settings (dict): A dictionary of Algolia settings. - """ - if not self.algolia_index: - logger.error('Algolia index does not exist. Did you initialize it?') - return - - try: - if primary_index: - self.algolia_index.set_settings(index_settings) - else: - self.replica_index.set_settings(index_settings) - except AlgoliaException as exc: - logger.exception( - 'Unable to set settings for Algolia\'s %s index due to an exception.', - self.algolia_index_name, - ) - raise exc - - def set_replica_index_settings(self, index_settings, replica_index_name): - """ - Set settings on a specific replica index by name. - - Unlike ``set_index_settings(..., primary_index=False)`` (which targets the single - ``REPLICA_INDEX_NAME`` replica), this configures any replica the primary index has - declared in its ``replicas`` setting -- used for the additional "recently published" - sort replica. The replica must already be declared on the primary index (Algolia - creates replica indices when the primary index's settings are saved). - Arguments: index_settings (dict): A dictionary of Algolia settings. - replica_index_name (str): The name of the replica index to configure. + index_name (str): Optional index to target; defaults to the primary index. """ if not self.algolia_index: logger.error('Algolia index does not exist. Did you initialize it?') return try: - replica_index = self._get_index(replica_index_name) - replica_index.set_settings(index_settings) + self._get_index(index_name).set_settings(index_settings) except AlgoliaException as exc: logger.exception( - 'Unable to set settings for Algolia replica index %s due to an exception.', - replica_index_name, + 'Unable to set settings for Algolia\'s %s index due to an exception.', + index_name or self.algolia_index_name, ) raise exc diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index 54a482af..4ae00888 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -87,28 +87,40 @@ def test_get_index_raises_when_primary_uninitialized(self): with self.assertRaises(ImproperlyConfigured): client._get_index() - def test_set_replica_index_settings_targets_named_replica(self): + def test_set_index_settings_targets_primary_by_default(self): """ - ``set_replica_index_settings`` applies settings to the replica resolved by name. + ``set_index_settings()`` with no name applies settings to the cached primary index. + """ + client = self._build_client() + settings_payload = {'customRanking': ['asc(created)']} + + client.set_index_settings(settings_payload) + + client.algolia_index.set_settings.assert_called_once_with(settings_payload) + client._client.init_index.assert_not_called() # primary is cached, not re-initialized + + def test_set_index_settings_targets_named_replica(self): + """ + ``set_index_settings(index_name=...)`` applies settings to the replica resolved by name. """ client = self._build_client() replica = mock.MagicMock(name='recently_published_replica') client._client.init_index.return_value = replica settings_payload = {'customRanking': ['desc(recently_published_timestamp)']} - client.set_replica_index_settings(settings_payload, 'enterprise_catalog_recently_published_desc') + client.set_index_settings(settings_payload, index_name='enterprise_catalog_recently_published_desc') client._client.init_index.assert_called_once_with('enterprise_catalog_recently_published_desc') replica.set_settings.assert_called_once_with(settings_payload) - def test_set_replica_index_settings_noop_when_primary_uninitialized(self): + def test_set_index_settings_noop_when_primary_uninitialized(self): """ With no primary index initialized, the call logs and returns without touching Algolia. """ client = AlgoliaSearchClient() # nothing initialized - client.set_replica_index_settings({'customRanking': []}, 'some_replica') # must not raise + client.set_index_settings({'customRanking': []}, index_name='some_replica') # must not raise - def test_set_replica_index_settings_reraises_algolia_exception(self): + def test_set_index_settings_reraises_algolia_exception(self): """ An AlgoliaException from set_settings propagates to the caller. """ @@ -117,7 +129,7 @@ def test_set_replica_index_settings_reraises_algolia_exception(self): replica.set_settings.side_effect = AlgoliaException('boom') client._client.init_index.return_value = replica with self.assertRaises(AlgoliaException): - client.set_replica_index_settings({'customRanking': []}, 'some_replica') + client.set_index_settings({'customRanking': []}, index_name='some_replica') @override_settings(ALGOLIA={ 'INDEX_NAME': 'enterprise_catalog', diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 196c841b..da3b199f 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -451,11 +451,11 @@ def configure_algolia_index(algolia_client): Configures the settings for an Algolia index. """ algolia_client.set_index_settings(ALGOLIA_INDEX_SETTINGS) - algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, primary_index=False) + algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=ALGOLIA_REPLICA_INDEX_NAME) if ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME: - algolia_client.set_replica_index_settings( + algolia_client.set_index_settings( ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, - ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME, + index_name=ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME, ) diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index b7a7ff45..6da41740 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -1049,13 +1049,15 @@ def test_configure_algolia_index(self, mock_search_client): """ algolia_client = utils.get_initialized_algolia_client() utils.configure_algolia_index(algolia_client) - mock_search_client.return_value.set_index_settings.assert_any_call(utils.ALGOLIA_INDEX_SETTINGS) - mock_search_client.return_value.set_index_settings.assert_called_with( + set_index_settings = mock_search_client.return_value.set_index_settings + set_index_settings.assert_any_call(utils.ALGOLIA_INDEX_SETTINGS) + set_index_settings.assert_any_call( utils.ALGOLIA_REPLICA_INDEX_SETTINGS, - primary_index=False + index_name=utils.ALGOLIA_REPLICA_INDEX_NAME, ) - # No recently-published replica name is configured by default, so that replica is skipped. - mock_search_client.return_value.set_replica_index_settings.assert_not_called() + # No recently-published replica name is configured by default, so only the primary and + # base replica are configured -- the recency replica settings are never applied. + assert set_index_settings.call_count == 2 @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') def test_configure_algolia_index_configures_recently_published_replica(self, mock_search_client): @@ -1066,9 +1068,9 @@ def test_configure_algolia_index_configures_recently_published_replica(self, moc replica_name = 'enterprise_catalog_recently_published_desc' with mock.patch.object(utils, 'ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', replica_name): utils.configure_algolia_index(algolia_client) - mock_search_client.return_value.set_replica_index_settings.assert_called_once_with( + mock_search_client.return_value.set_index_settings.assert_any_call( utils.ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, - replica_name, + index_name=replica_name, ) @ddt.data( From a0170255b41e19ab46484bf64b35ecc81cfcb8c2 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Fri, 12 Jun 2026 12:00:49 -0400 Subject: [PATCH 06/20] fix: update incremental_reindex_algolia for unified set_index_settings The unify refactor removed set_index_settings' primary_index kwarg, breaking this caller (E1123 unexpected-keyword-arg). Configure the replica by name instead, and wire the SDK client onto the AlgoliaSearchClient so the index_name lookup (_get_index) resolves the replica -- the old primary_index=False path used the cached replica_index handle and didn't need the client set. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../management/commands/incremental_reindex_algolia.py | 7 ++++++- .../commands/tests/test_incremental_reindex_algolia.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py b/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py index 605c48a9..c80c0397 100644 --- a/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py +++ b/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py @@ -107,11 +107,16 @@ def handle(self, *args, **options): replica_name = replica_index_name or f'{index_name}_repl' sdk_client = new_search_client_or_error() algolia_client = AlgoliaSearchClient() + # Target a non-primary index: wire the SDK client and the primary handle directly so + # set_index_settings() resolves to this alternate index. The replica is configured by + # name, which goes through _get_index() -> self._client.init_index(replica_name), so + # the underlying client must be set (init_index() would target the production index). + algolia_client._client = sdk_client # pylint: disable=protected-access algolia_client.algolia_index = sdk_client.init_index(index_name) algolia_client.replica_index = sdk_client.init_index(replica_name) primary_settings = {**ALGOLIA_INDEX_SETTINGS, 'replicas': [f'virtual({replica_name})']} algolia_client.set_index_settings(primary_settings) - algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, primary_index=False) + algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=replica_name) self.stdout.write(f'Content types: {", ".join(content_types) if content_types else "all"}') self.stdout.write(f'Target index: {resolved_index or "(not configured)"}') diff --git a/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py b/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py index 47f44140..074b7f84 100644 --- a/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py +++ b/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py @@ -198,7 +198,7 @@ def test_configure_index_called_before_dispatch(self, mock_task): primary_settings = primary_call_kwargs[0][0] assert primary_settings['replicas'] == ['virtual(enterprise_catalog_v2_repl)'] replica_call = algolia_instance.set_index_settings.call_args_list[1] - assert replica_call[1].get('primary_index') is False + assert replica_call[1].get('index_name') == 'enterprise_catalog_v2_repl' @mock.patch(TASK_PATH) def test_explicit_replica_name_used(self, mock_task): From 34af3cb712ebdbaedbd6ef68d8689b5c219d3d4c Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 08:40:59 -0400 Subject: [PATCH 07/20] refactor: drive optional Algolia sort replicas from a config registry Generalize the single recency-replica gate into a registry of optional sort replicas (OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS in api_client/constants, the shared source of truth for which replicas exist). The indexer (_build_algolia_replicas, configure_algolia_index) and the secured-API-key restriction now loop over the registry: each replica is declared, configured, and made queryable ONLY when its settings.ALGOLIA index-name key is non-empty. This makes "not in config -> not used" structural and obvious (inert by default, the safe thing to check in), and reduces a future sort order to a registry entry plus the field its customRanking sorts on -- no change to the gating logic. The waffle flag remains the instant, scoped runtime on/off; the config gate only governs whether the replica infrastructure exists. Updates ADR 0014. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 10 ++-- enterprise_catalog/apps/api_client/algolia.py | 21 ++++++-- .../apps/api_client/constants.py | 12 +++++ .../apps/catalog/algolia_utils.py | 53 +++++++++++++------ .../apps/catalog/tests/test_algolia_utils.py | 26 +++++---- 5 files changed, 88 insertions(+), 34 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index ec55b8af..109589d4 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -49,9 +49,13 @@ Decision The replica is **config-gated on both sides** and is never queried unless its name is configured: -* Backend: the replica is declared on the primary index and its settings are - applied **only** when ``RECENTLY_PUBLISHED_REPLICA_INDEX_NAME`` is set; - otherwise it is a no-op (no ``virtual(None)`` replica is created). +* Backend: optional sort replicas are driven by a registry + (``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS`` — the recency replica is the first + entry). Each is declared on the primary index and configured **only** when its + ``settings.ALGOLIA`` index-name key is set; an unconfigured replica is skipped + entirely (no ``virtual(None)`` replica is created), so the code is inert until + ops provides a name. Adding a future sort is a registry entry plus the field + its ``customRanking`` sorts on — not a change to the gating logic. * MFE: the course search uses the replica only when its index-name config var is non-empty (and the flag + experiment gates pass); otherwise it falls back to the primary (relevance) index. diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index 98c8467a..e927bbed 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -10,6 +10,9 @@ from django.conf import settings from django.core.exceptions import ImproperlyConfigured +from enterprise_catalog.apps.api_client.constants import ( + OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS, +) from enterprise_catalog.apps.catalog.utils import batch, localized_utcnow @@ -47,8 +50,19 @@ def algolia_replica_index_name(self): return settings.ALGOLIA.get('REPLICA_INDEX_NAME') @property - def algolia_recently_published_replica_index_name(self): - return settings.ALGOLIA.get('RECENTLY_PUBLISHED_REPLICA_INDEX_NAME') + def optional_replica_index_names(self): + """ + Configured index names of all optional sort replicas (empty when none are set). + + Driven by ``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS``; an unconfigured replica is omitted, + so the secured API key only grants access to replicas that actually exist. + """ + names = [] + for config_key in OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS: + index_name = settings.ALGOLIA.get(config_key) + if index_name: + names.append(index_name) + return names def init_index(self): """ @@ -431,8 +445,7 @@ def generate_secured_api_key(self, user_id, enterprise_catalog_query_uuids): indices.append(self.algolia_index_name) if self.algolia_replica_index_name: indices.append(self.algolia_replica_index_name) - if self.algolia_recently_published_replica_index_name: - indices.append(self.algolia_recently_published_replica_index_name) + indices.extend(self.optional_replica_index_names) if indices: restrictions |= {'restrictIndices': indices} diff --git a/enterprise_catalog/apps/api_client/constants.py b/enterprise_catalog/apps/api_client/constants.py index bc53bf19..b30012be 100644 --- a/enterprise_catalog/apps/api_client/constants.py +++ b/enterprise_catalog/apps/api_client/constants.py @@ -18,6 +18,18 @@ DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_KEY = 'average_course_review' DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_TTL = 60 * 120 # 2 hours +# Algolia API Client Constants +# settings.ALGOLIA keys that name each *optional* sort replica index (the primary index and +# its base ``REPLICA_INDEX_NAME`` replica are always present; these are additive sort orders +# such as "newest first"). A replica is only declared, configured, and made queryable when its +# key holds a non-empty index name, so listing a key here is inert until ops sets the name. +# Adding a new sort = add its key here, map it to index settings in ``algolia_utils``, and +# compute the field its ``customRanking`` sorts on. This is the single source of truth for +# *which* optional replicas exist, shared by the indexer and the secured-API-key restriction. +OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS = ( + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', +) + COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER = 15 # As of 1/26/24 this is calculated from Snowflake: diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index da3b199f..e0d2d066 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -20,6 +20,7 @@ COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER, DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_KEY, DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_TTL, + OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS, ) from enterprise_catalog.apps.catalog.constants import ( ALGOLIA_DEFAULT_TIMESTAMP, @@ -65,29 +66,29 @@ ALGOLIA_JSON_METADATA_MAX_SIZE = 100000 ALGOLIA_REPLICA_INDEX_NAME = settings.ALGOLIA.get('REPLICA_INDEX_NAME') -# Optional second replica that sorts courses by recency (newest-first). Only declared when a -# name is configured (see settings.ALGOLIA); absent in local/test until ops sets it in the -# deployment config. -ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME = settings.ALGOLIA.get('RECENTLY_PUBLISHED_REPLICA_INDEX_NAME') algolia_replica_index = f'virtual({ALGOLIA_REPLICA_INDEX_NAME})' -def _build_algolia_replicas(recently_published_replica_index_name): +def _build_algolia_replicas(): """ Build the list of replica indexes to declare on the primary index. - The recency replica is only included when its name is configured, so deploying this code - before ops sets the name won't declare a ``virtual(None)`` replica on the primary index. + Always includes the base (duration) replica, plus a ``virtual(name)`` for each optional sort + replica (see ``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS``) whose index name is configured. + Unconfigured optional replicas are omitted, so deploying this code before ops sets a name + won't declare a ``virtual(None)`` replica on the primary index. """ replicas = [algolia_replica_index] - if recently_published_replica_index_name: - replicas.append(f'virtual({recently_published_replica_index_name})') + for config_key in OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS: + index_name = settings.ALGOLIA.get(config_key) + if index_name: + replicas.append(f'virtual({index_name})') return replicas # Replicas declared on the primary index (see ``_build_algolia_replicas``). -ALGOLIA_REPLICAS = _build_algolia_replicas(ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME) +ALGOLIA_REPLICAS = _build_algolia_replicas() # keep attributes from content objects that we explicitly want in Algolia ALGOLIA_FIELDS = [ @@ -257,6 +258,29 @@ def _build_algolia_replicas(recently_published_replica_index_name): ], } +# Maps each optional-replica config key (see ``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS``) to the +# index settings applied to that replica. Adding a new sort replica means adding its key to the +# shared tuple and its ``customRanking`` settings here (and computing the field it sorts on). +OPTIONAL_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, +} + + +def _configured_optional_replicas(): + """ + Return ``(index_name, index_settings)`` for each optional sort replica whose name is set. + + Optional replicas are additive sort orders declared on the primary index only when ops sets + their index name in ``settings.ALGOLIA``. An unconfigured replica is skipped, so this is + inert until its name is provided. + """ + configured = [] + for config_key in OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS: + index_name = settings.ALGOLIA.get(config_key) + if index_name: + configured.append((index_name, OPTIONAL_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY[config_key])) + return configured + def _should_index_course(course_metadata): """ @@ -448,15 +472,12 @@ def new_search_client_or_error(): def configure_algolia_index(algolia_client): """ - Configures the settings for an Algolia index. + Configures the settings for the primary Algolia index and its replicas. """ algolia_client.set_index_settings(ALGOLIA_INDEX_SETTINGS) algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=ALGOLIA_REPLICA_INDEX_NAME) - if ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME: - algolia_client.set_index_settings( - ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, - index_name=ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME, - ) + for index_name, index_settings in _configured_optional_replicas(): + algolia_client.set_index_settings(index_settings, index_name=index_name) def get_algolia_object_id(content_type, uuid): diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 6da41740..40cdcdc7 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -4,7 +4,7 @@ import ddt from dateutil.relativedelta import relativedelta -from django.test import TestCase +from django.test import TestCase, override_settings from enterprise_catalog.apps.catalog import algolia_utils as utils from enterprise_catalog.apps.catalog.algolia_utils import _get_course_run @@ -282,16 +282,20 @@ def test_get_course_recently_published_timestamp(self): {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) - def test_build_algolia_replicas_includes_recency_replica_when_configured(self): - """The recency replica is declared only when its index name is configured.""" + def test_build_algolia_replicas_includes_optional_replica_when_configured(self): + """An optional replica is declared only when its index name is configured.""" # pylint: disable=protected-access - assert utils._build_algolia_replicas('enterprise_catalog_recently_published_desc') == [ - utils.algolia_replica_index, - 'virtual(enterprise_catalog_recently_published_desc)', - ] - # Unconfigured (empty / None) -> only the base replica, never virtual(None). - assert utils._build_algolia_replicas('') == [utils.algolia_replica_index] - assert utils._build_algolia_replicas(None) == [utils.algolia_replica_index] + replica_name = 'enterprise_catalog_recently_published_desc' + with override_settings(ALGOLIA={'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': replica_name}): + assert utils._build_algolia_replicas() == [ + utils.algolia_replica_index, + f'virtual({replica_name})', + ] + # Unconfigured (empty) -> only the base replica, never virtual(None). + with override_settings(ALGOLIA={'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ''}): + assert utils._build_algolia_replicas() == [utils.algolia_replica_index] + with override_settings(ALGOLIA={}): + assert utils._build_algolia_replicas() == [utils.algolia_replica_index] def test_algolia_object_includes_recently_published_timestamp(self): """The course Algolia object carries recently_published_timestamp (and is_new_content).""" @@ -1066,7 +1070,7 @@ def test_configure_algolia_index_configures_recently_published_replica(self, moc """ algolia_client = utils.get_initialized_algolia_client() replica_name = 'enterprise_catalog_recently_published_desc' - with mock.patch.object(utils, 'ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', replica_name): + with override_settings(ALGOLIA={'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': replica_name}): utils.configure_algolia_index(algolia_client) mock_search_client.return_value.set_index_settings.assert_any_call( utils.ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, From 7b9964f32c7d502fe1941bf6b033d2c28e8a8b8c Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 10:40:48 -0400 Subject: [PATCH 08/20] refactor: fold the base replica into the unified replica registry The registry previously covered only the optional sort replicas; the base duration replica was still special-cased (hardcoded algolia_replica_index, a dedicated configure call, a separate secured-key append). Generalize so ALL replicas flow through one registry, base first: - ALGOLIA_REPLICA_CONFIG_KEYS (api_client/constants) now lists every replica (REPLICA_INDEX_NAME, then RECENTLY_PUBLISHED_REPLICA_INDEX_NAME), in declaration order, as the single source of truth. - _build_algolia_replicas / _configured_replicas / configure_algolia_index and the secured-key restrictIndices all loop the registry; each replica is declared/configured/restricted only when its index name is set. This also fixes a latent virtual(None) for the base replica when REPLICA_INDEX_NAME is unset. - The client's init_index/index_exists still eagerly manage the primary + base-replica handles -- the required-core-pair lifecycle, intentionally left separate from "which replicas get declared/configured." Updates ADR 0014 and tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 19 +++++--- enterprise_catalog/apps/api_client/algolia.py | 16 +++---- .../apps/api_client/constants.py | 15 ++++--- .../apps/catalog/algolia_utils.py | 43 +++++++++---------- .../apps/catalog/tests/test_algolia_utils.py | 32 ++++++++------ 5 files changed, 65 insertions(+), 60 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index 109589d4..8e5e31ce 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -49,13 +49,18 @@ Decision The replica is **config-gated on both sides** and is never queried unless its name is configured: -* Backend: optional sort replicas are driven by a registry - (``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS`` — the recency replica is the first - entry). Each is declared on the primary index and configured **only** when its - ``settings.ALGOLIA`` index-name key is set; an unconfigured replica is skipped - entirely (no ``virtual(None)`` replica is created), so the code is inert until - ops provides a name. Adding a future sort is a registry entry plus the field - its ``customRanking`` sorts on — not a change to the gating logic. +* Backend: **all** sort replicas are driven by one registry + (``ALGOLIA_REPLICA_CONFIG_KEYS`` — the base duration replica first, then additive + sorts like recency). Each is declared on the primary index, configured, and + added to the secured-key ``restrictIndices`` **only** when its ``settings.ALGOLIA`` + index-name key is set; an unconfigured replica is skipped entirely (no + ``virtual(None)`` replica is created), so the code is inert until ops provides a + name. This is the single source of truth for which replicas exist — adding a + future sort is a registry entry plus the field its ``customRanking`` sorts on, + not a change to the gating logic. (The client's ``init_index`` / + ``index_exists`` still eagerly manage the primary + base-replica *handles* — that + is the required-core-pair lifecycle, a separate concern from which replicas get + declared/configured.) * MFE: the course search uses the replica only when its index-name config var is non-empty (and the flag + experiment gates pass); otherwise it falls back to the primary (relevance) index. diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index e927bbed..c796f4b8 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -11,7 +11,7 @@ from django.core.exceptions import ImproperlyConfigured from enterprise_catalog.apps.api_client.constants import ( - OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS, + ALGOLIA_REPLICA_CONFIG_KEYS, ) from enterprise_catalog.apps.catalog.utils import batch, localized_utcnow @@ -50,15 +50,15 @@ def algolia_replica_index_name(self): return settings.ALGOLIA.get('REPLICA_INDEX_NAME') @property - def optional_replica_index_names(self): + def replica_index_names(self): """ - Configured index names of all optional sort replicas (empty when none are set). + Configured index names of every sort replica (base + additive), empty when none are set. - Driven by ``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS``; an unconfigured replica is omitted, - so the secured API key only grants access to replicas that actually exist. + Driven by ``ALGOLIA_REPLICA_CONFIG_KEYS``; an unconfigured replica is omitted, so the + secured API key only grants access to replicas that actually exist. """ names = [] - for config_key in OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS: + for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: index_name = settings.ALGOLIA.get(config_key) if index_name: names.append(index_name) @@ -443,9 +443,7 @@ def generate_secured_api_key(self, user_id, enterprise_catalog_query_uuids): indices = [] if self.algolia_index_name: indices.append(self.algolia_index_name) - if self.algolia_replica_index_name: - indices.append(self.algolia_replica_index_name) - indices.extend(self.optional_replica_index_names) + indices.extend(self.replica_index_names) if indices: restrictions |= {'restrictIndices': indices} diff --git a/enterprise_catalog/apps/api_client/constants.py b/enterprise_catalog/apps/api_client/constants.py index b30012be..f979df33 100644 --- a/enterprise_catalog/apps/api_client/constants.py +++ b/enterprise_catalog/apps/api_client/constants.py @@ -19,15 +19,16 @@ DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_TTL = 60 * 120 # 2 hours # Algolia API Client Constants -# settings.ALGOLIA keys that name each *optional* sort replica index (the primary index and -# its base ``REPLICA_INDEX_NAME`` replica are always present; these are additive sort orders -# such as "newest first"). A replica is only declared, configured, and made queryable when its -# key holds a non-empty index name, so listing a key here is inert until ops sets the name. +# settings.ALGOLIA keys that name each sort replica index, in declaration order. The primary +# index keeps the relevance ranking; every replica -- the base "duration" replica and any +# additive sort such as "newest first" -- is declared, configured, and made queryable ONLY when +# its key holds a non-empty index name, so listing a key here is inert until ops sets the name. # Adding a new sort = add its key here, map it to index settings in ``algolia_utils``, and # compute the field its ``customRanking`` sorts on. This is the single source of truth for -# *which* optional replicas exist, shared by the indexer and the secured-API-key restriction. -OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS = ( - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', +# *which* replicas exist, shared by the indexer and the secured-API-key restriction. +ALGOLIA_REPLICA_CONFIG_KEYS = ( + 'REPLICA_INDEX_NAME', # base replica (relevance + duration tie-breaks) + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', # "newest courses first" ) COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER = 15 diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index e0d2d066..0faadd08 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -16,11 +16,11 @@ from enterprise_catalog.apps.api_client.algolia import AlgoliaSearchClient from enterprise_catalog.apps.api_client.constants import ( + ALGOLIA_REPLICA_CONFIG_KEYS, COURSE_REVIEW_BASE_AVG_REVIEW_SCORE, COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER, DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_KEY, DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_TTL, - OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS, ) from enterprise_catalog.apps.catalog.constants import ( ALGOLIA_DEFAULT_TIMESTAMP, @@ -65,22 +65,19 @@ ALGOLIA_UUID_BATCH_SIZE = 100 ALGOLIA_JSON_METADATA_MAX_SIZE = 100000 -ALGOLIA_REPLICA_INDEX_NAME = settings.ALGOLIA.get('REPLICA_INDEX_NAME') - -algolia_replica_index = f'virtual({ALGOLIA_REPLICA_INDEX_NAME})' def _build_algolia_replicas(): """ Build the list of replica indexes to declare on the primary index. - Always includes the base (duration) replica, plus a ``virtual(name)`` for each optional sort - replica (see ``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS``) whose index name is configured. - Unconfigured optional replicas are omitted, so deploying this code before ops sets a name - won't declare a ``virtual(None)`` replica on the primary index. + Declares a ``virtual(name)`` for each replica in the registry (``ALGOLIA_REPLICA_CONFIG_KEYS`` + -- the base duration replica plus any additive sort replica) whose index name is configured. + Unconfigured replicas are omitted, so deploying this code before ops sets a name won't declare + a ``virtual(None)`` replica on the primary index. """ - replicas = [algolia_replica_index] - for config_key in OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS: + replicas = [] + for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: index_name = settings.ALGOLIA.get(config_key) if index_name: replicas.append(f'virtual({index_name})') @@ -258,27 +255,28 @@ def _build_algolia_replicas(): ], } -# Maps each optional-replica config key (see ``OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS``) to the -# index settings applied to that replica. Adding a new sort replica means adding its key to the -# shared tuple and its ``customRanking`` settings here (and computing the field it sorts on). -OPTIONAL_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { +# Maps each replica config key (see ``ALGOLIA_REPLICA_CONFIG_KEYS``) to the index settings +# applied to that replica. Adding a new sort replica means adding its key to the shared tuple +# and its ``customRanking`` settings here (and computing the field its ``customRanking`` sorts on). +ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { + 'REPLICA_INDEX_NAME': ALGOLIA_REPLICA_INDEX_SETTINGS, 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, } -def _configured_optional_replicas(): +def _configured_replicas(): """ - Return ``(index_name, index_settings)`` for each optional sort replica whose name is set. + Return ``(index_name, index_settings)`` for each replica whose index name is configured. - Optional replicas are additive sort orders declared on the primary index only when ops sets - their index name in ``settings.ALGOLIA``. An unconfigured replica is skipped, so this is - inert until its name is provided. + Covers every replica in the registry -- the base duration replica and any additive sort + replica (``ALGOLIA_REPLICA_CONFIG_KEYS``). An unconfigured replica is skipped, so this is + inert until its name is provided in ``settings.ALGOLIA``. """ configured = [] - for config_key in OPTIONAL_ALGOLIA_REPLICA_CONFIG_KEYS: + for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: index_name = settings.ALGOLIA.get(config_key) if index_name: - configured.append((index_name, OPTIONAL_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY[config_key])) + configured.append((index_name, ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY[config_key])) return configured @@ -475,8 +473,7 @@ def configure_algolia_index(algolia_client): Configures the settings for the primary Algolia index and its replicas. """ algolia_client.set_index_settings(ALGOLIA_INDEX_SETTINGS) - algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=ALGOLIA_REPLICA_INDEX_NAME) - for index_name, index_settings in _configured_optional_replicas(): + for index_name, index_settings in _configured_replicas(): algolia_client.set_index_settings(index_settings, index_name=index_name) diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 40cdcdc7..e31a17d8 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -282,20 +282,23 @@ def test_get_course_recently_published_timestamp(self): {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) - def test_build_algolia_replicas_includes_optional_replica_when_configured(self): - """An optional replica is declared only when its index name is configured.""" + def test_build_algolia_replicas_only_includes_configured_replicas(self): + """Each replica (base + optional) is declared only when its index name is configured.""" # pylint: disable=protected-access - replica_name = 'enterprise_catalog_recently_published_desc' - with override_settings(ALGOLIA={'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': replica_name}): + with override_settings(ALGOLIA={ + 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_published_desc', + }): assert utils._build_algolia_replicas() == [ - utils.algolia_replica_index, - f'virtual({replica_name})', + 'virtual(enterprise_catalog_duration_desc)', + 'virtual(enterprise_catalog_recently_published_desc)', ] - # Unconfigured (empty) -> only the base replica, never virtual(None). - with override_settings(ALGOLIA={'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ''}): - assert utils._build_algolia_replicas() == [utils.algolia_replica_index] + # Only the base replica configured -> only it is declared. + with override_settings(ALGOLIA={'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc'}): + assert utils._build_algolia_replicas() == ['virtual(enterprise_catalog_duration_desc)'] + # Nothing configured -> no replicas at all, never virtual(None). with override_settings(ALGOLIA={}): - assert utils._build_algolia_replicas() == [utils.algolia_replica_index] + assert not utils._build_algolia_replicas() def test_algolia_object_includes_recently_published_timestamp(self): """The course Algolia object carries recently_published_timestamp (and is_new_content).""" @@ -1052,15 +1055,16 @@ def test_configure_algolia_index(self, mock_search_client): Verify that `configure_algolia_index_settings` makes call to configure index settings. """ algolia_client = utils.get_initialized_algolia_client() - utils.configure_algolia_index(algolia_client) + with override_settings(ALGOLIA={'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc'}): + utils.configure_algolia_index(algolia_client) set_index_settings = mock_search_client.return_value.set_index_settings set_index_settings.assert_any_call(utils.ALGOLIA_INDEX_SETTINGS) set_index_settings.assert_any_call( utils.ALGOLIA_REPLICA_INDEX_SETTINGS, - index_name=utils.ALGOLIA_REPLICA_INDEX_NAME, + index_name='enterprise_catalog_duration_desc', ) - # No recently-published replica name is configured by default, so only the primary and - # base replica are configured -- the recency replica settings are never applied. + # Only the base replica is configured here (no optional replica name set), so the primary + # plus the single base replica are the only two set_index_settings calls. assert set_index_settings.call_count == 2 @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') From d7ded39bc12644d290fd317543acec0ab1841d63 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 14:10:31 -0400 Subject: [PATCH 09/20] docs: clarify what the base REPLICA_INDEX_NAME replica does REPLICA_INDEX_NAME is the base replica sorted by desc(duration); the learner-portal MFE points its video search (SEARCH_INDEX_IDS.VIDEOS) at this index (its ALGOLIA_REPLICA_INDEX_NAME env var). Document this at the settings default and in the replica registry, and note that the ALGOLIA dict is replaced (not merged) from deployment config. No rename -- the key is an established, ops-facing config name. Co-Authored-By: Claude Opus 4.8 (1M context) --- enterprise_catalog/apps/api_client/constants.py | 4 ++-- enterprise_catalog/settings/base.py | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/enterprise_catalog/apps/api_client/constants.py b/enterprise_catalog/apps/api_client/constants.py index f979df33..d1404f1d 100644 --- a/enterprise_catalog/apps/api_client/constants.py +++ b/enterprise_catalog/apps/api_client/constants.py @@ -27,8 +27,8 @@ # compute the field its ``customRanking`` sorts on. This is the single source of truth for # *which* replicas exist, shared by the indexer and the secured-API-key restriction. ALGOLIA_REPLICA_CONFIG_KEYS = ( - 'REPLICA_INDEX_NAME', # base replica (relevance + duration tie-breaks) - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', # "newest courses first" + 'REPLICA_INDEX_NAME', # base replica, desc(duration); MFE video search uses it + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', # "newest courses first", desc(recently_published_timestamp) ) COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER = 15 diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index 54cd8234..005924b1 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -432,9 +432,16 @@ STUDIO_BASE_URL = os.environ.get('STUDIO_BASE_URL', '') # Algolia +# Index-name keys are populated per-environment from the deployment config (edx-internal); the +# whole ALGOLIA dict is *replaced* there, not merged. Each replica is declared/configured only +# when its key holds a non-empty name (see ALGOLIA_REPLICA_CONFIG_KEYS). ALGOLIA = { 'INDEX_NAME': '', + # Base replica, sorted by duration descending (desc(duration)). The learner-portal MFE points + # its video search (SEARCH_INDEX_IDS.VIDEOS) at this index, so it is also the MFE's + # ALGOLIA_REPLICA_INDEX_NAME env var. 'REPLICA_INDEX_NAME': '', + # "Newest courses first" replica, sorted by desc(recently_published_timestamp). 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': '', 'APPLICATION_ID': '', 'API_KEY': '', From c449a447e01390fa653c1b5febdb2435dea47ab4 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 10:45:19 -0400 Subject: [PATCH 10/20] feat: fail safe when configuring sort replicas Configuring a sort replica is additive and must never abort the reindex. Wrap each per-replica set_index_settings(index_name=...) call in the unified registry loop so an AlgoliaException is logged and skipped, leaving the primary index and the other replicas configured. The degraded state is always the safe base (relevance) sort; the failed replica catches up on the next successful run. Record the fail-safe in ADR 0014 and cover it with a test (base replica succeeds while recency fails). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 11 ++++++ .../apps/catalog/algolia_utils.py | 13 ++++++- .../apps/catalog/tests/test_algolia_utils.py | 35 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index 8e5e31ce..52ed86e9 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -61,6 +61,12 @@ name is configured: ``index_exists`` still eagerly manage the primary + base-replica *handles* — that is the required-core-pair lifecycle, a separate concern from which replicas get declared/configured.) +* Backend (fail-safe): configuring each replica in ``configure_algolia_index`` is + wrapped so that any ``AlgoliaException`` is logged and skipped. One replica + failing to configure never aborts the reindex — the primary (relevance) index + and the other replicas are still configured. The failed replica keeps its prior + settings (or, when brand new, mirrors the primary's relevance ranking) until the + next successful run, so the degraded state is still the safe base sort. * MFE: the course search uses the replica only when its index-name config var is non-empty (and the flag + experiment gates pass); otherwise it falls back to the primary (relevance) index. @@ -94,6 +100,11 @@ Consequences empty results rather than falling back. This is a transient, operator-controlled window mitigated by the rollout order and the kill-switch flag, not by code. +* **Reindex degrades gracefully:** if configuring a replica fails, the error is + logged and the reindex continues with the primary index and the other replicas + intact, so a problem with one sort can never take down core search indexing. + The worst case is that one replica lags its ranking until the next run — never a + broken or empty primary index. * **The waffle flag is the readiness contract:** enabling it asserts "the replica is live." This keeps the safe path a single, instantly reversible toggle rather than per-request defensive logic in the search hot path. diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 0faadd08..c9219f02 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -3,6 +3,7 @@ import logging import time +from algoliasearch.exceptions import AlgoliaException from algoliasearch.search_client import SearchClient from dateutil import parser from dateutil.relativedelta import relativedelta @@ -474,7 +475,17 @@ def configure_algolia_index(algolia_client): """ algolia_client.set_index_settings(ALGOLIA_INDEX_SETTINGS) for index_name, index_settings in _configured_replicas(): - algolia_client.set_index_settings(index_settings, index_name=index_name) + # A replica's settings failing to apply must never abort the reindex: log and continue so + # the primary (relevance) index and the other replicas stay configured. The failed replica + # keeps its prior settings (or, if brand new, mirrors the primary's relevance ranking) + # until the next successful run, so the safe fallback is the base sort. See ADR 0014. + try: + algolia_client.set_index_settings(index_settings, index_name=index_name) + except AlgoliaException: + LOGGER.exception( + 'Failed to configure replica index "%s"; continuing with the remaining indexes.', + index_name, + ) def get_algolia_object_id(content_type, uuid): diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index e31a17d8..2e63c470 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -3,6 +3,7 @@ from uuid import uuid4 import ddt +from algoliasearch.exceptions import AlgoliaException from dateutil.relativedelta import relativedelta from django.test import TestCase, override_settings @@ -1081,6 +1082,40 @@ def test_configure_algolia_index_configures_recently_published_replica(self, moc index_name=replica_name, ) + @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') + def test_configure_algolia_index_replica_failure_is_safe(self, mock_search_client): + """ + One replica failing to configure must not abort the reindex: the primary and the other + replicas are still configured, the error is logged, and nothing propagates. + """ + algolia_client = utils.get_initialized_algolia_client() + base_name = 'enterprise_catalog_duration_desc' + recency_name = 'enterprise_catalog_recently_published_desc' + + def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=unused-argument + # Primary and base-replica calls succeed; only the recency replica raises. + if index_name == recency_name: + raise AlgoliaException('boom') + + mock_search_client.return_value.set_index_settings.side_effect = fail_only_for_recency + with override_settings(ALGOLIA={ + 'REPLICA_INDEX_NAME': base_name, + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': recency_name, + }): + with self.assertLogs(utils.LOGGER, level='ERROR') as error_logs: + # Does not raise, even though the recency replica fails. + utils.configure_algolia_index(algolia_client) + set_index_settings = mock_search_client.return_value.set_index_settings + # The primary (relevance) index and the base replica were still configured. + set_index_settings.assert_any_call(utils.ALGOLIA_INDEX_SETTINGS) + set_index_settings.assert_any_call(utils.ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=base_name) + # The recency replica was attempted (and failed) -- logged, not swallowed silently. + set_index_settings.assert_any_call( + utils.ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + index_name=recency_name, + ) + assert any(recency_name in message for message in error_logs.output) + @ddt.data( ( {'courses': [{'key': 'program_course_key'}, {'key': 'other_course_key'}]}, From 771bb1ba380cfa62c86b55e7e959c7bc03ec7c80 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 14:42:41 -0400 Subject: [PATCH 11/20] docs: how-to guide for adding a new Algolia sort replica Step-by-step engineer guide: declare the settings key, register it in ALGOLIA_REPLICA_CONFIG_KEYS, index the field the sort needs, define the replica's customRanking, and map key -> settings. Covers the names-in-settings vs definitions-in-code split, the deploy/reindex + ALGOLIA-is-replaced caveat, the optional MFE wiring, and the inert-by-default / fail-safe properties. Cross-refs ADR 0014 and uses a worked "price: low to high" example. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/how_to/add_an_algolia_sort_replica.rst | 167 ++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 docs/how_to/add_an_algolia_sort_replica.rst diff --git a/docs/how_to/add_an_algolia_sort_replica.rst b/docs/how_to/add_an_algolia_sort_replica.rst new file mode 100644 index 00000000..10079405 --- /dev/null +++ b/docs/how_to/add_an_algolia_sort_replica.rst @@ -0,0 +1,167 @@ +Add a new Algolia sort replica +============================== + +The Learner Portal search page sorts a single Algolia index by relevance. Algolia does +not re-sort an index at query time, so every alternate sort order is a separate *replica* +index with its own ``customRanking``; the consumer (the MFE) switches sort by pointing its +search at a different index name. + +All replicas are driven by **one registry**, so adding a sort is mostly additive. This guide +walks through it end to end. See ``docs/decisions/0014-newest-courses-sort-replica.rst`` for +the design rationale, and treat the **recently-published ("newest first") replica** as the +canonical example to copy. + +The mental model: names vs. definitions +--------------------------------------- + +There are two layers, and the split matters: + +* **Index *names* live in Django settings** (``settings.ALGOLIA``), populated per-environment + from the deployment config (edx-internal). These vary by environment and are owned by ops. +* **Replica *definitions* live in code** — which replicas exist (the registry) and how each is + ranked (``customRanking``). A replica's ranking sorts on a field that the indexing code must + compute, so the definition is intrinsically code, not config. + +A replica is declared, configured, and made queryable **only when its index-name key holds a +non-empty value**. With the name unset (the default everywhere until ops sets it), the new +replica is completely inert — nothing is declared, no ``virtual(None)`` is created, and the +secured API key does not reference it. This is what makes it safe to merge and deploy the code +*before* ops turns it on. + +Worked example +-------------- + +Suppose we want a **"price: low to high"** sort. We'll use the settings key +``PRICE_ASC_REPLICA_INDEX_NAME`` and (in a given environment) an index named +``enterprise_catalog_price_asc``. + +Backend steps (this service) +---------------------------- + +**1. Declare the settings key.** In ``enterprise_catalog/settings/base.py``, add the key to the +``ALGOLIA`` dict with an empty default and a one-line comment describing the sort: + +.. code-block:: python + + ALGOLIA = { + 'INDEX_NAME': '', + 'REPLICA_INDEX_NAME': '', # base replica, desc(duration); MFE video search + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': '', # "newest first", desc(recently_published_timestamp) + # "price: low to high", asc(first_enrollable_paid_seat_price) + 'PRICE_ASC_REPLICA_INDEX_NAME': '', + 'APPLICATION_ID': '', + 'API_KEY': '', + } + +The empty default keeps it inert until ops provides a real index name. + +**2. Register the key.** Add it to ``ALGOLIA_REPLICA_CONFIG_KEYS`` in +``enterprise_catalog/apps/api_client/constants.py`` — the single source of truth for *which* +replicas exist, shared by the indexer and the secured-key restriction. (It lives here, not in +``settings``/``algolia_utils``, so both ``api_client.algolia`` and ``catalog.algolia_utils`` can +import it without a circular dependency.) + +.. code-block:: python + + ALGOLIA_REPLICA_CONFIG_KEYS = ( + 'REPLICA_INDEX_NAME', + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', + 'PRICE_ASC_REPLICA_INDEX_NAME', + ) + +**3. Make sure the field you sort on is indexed.** A ``customRanking`` can only sort on a numeric +attribute that exists on the records. If your sort uses a field that is *already* indexed (the +price example reuses ``first_enrollable_paid_seat_price``), skip this step. If it needs a new +signal, in ``enterprise_catalog/apps/catalog/algolia_utils.py``: + +* write a ``get_course_(course)`` helper that returns the numeric value (see + ``get_course_recently_published_timestamp`` — note it returns ``0`` for "no value" so those + records sort last under a ``desc`` ranking; pick a sentinel that sorts your missing values to + the *end* of *your* order); +* add the field name to ``ALGOLIA_FIELDS``; +* set it on the course object in ``_algolia_object_from_product`` (the ``content_type == COURSE`` + branch). + +**4. Define the replica's ranking.** Still in ``algolia_utils.py``, add a settings dict. Lead with +your sort criterion, then append the primary index's shared tie-breakers so records that tie on +your criterion (and any "missing value" bucket) fall back to the relevance ordering and +pagination stays deterministic: + +.. code-block:: python + + ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS = { + 'customRanking': [ + 'asc(first_enrollable_paid_seat_price)', + # shared tie-breakers (same as the primary index) -- keep ties stable + 'asc(metadata_language)', + 'asc(visible_via_association)', + 'asc(created)', + 'desc(course_bayesian_average)', + 'desc(recent_enrollment_count)', + ], + } + +**5. Map the key to its settings.** Add an entry to +``ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY`` in ``algolia_utils.py``: + +.. code-block:: python + + ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { + 'REPLICA_INDEX_NAME': ALGOLIA_REPLICA_INDEX_SETTINGS, + 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + 'PRICE_ASC_REPLICA_INDEX_NAME': ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS, + } + +**That is all the wiring.** You do **not** touch ``_build_algolia_replicas``, +``_configured_replicas``, ``configure_algolia_index``, or the secured-key +``replica_index_names`` — they all loop the registry, so the new replica is automatically +declared on the primary index, has its settings applied during a reindex, and is added to the +secured API key's ``restrictIndices`` once its name is configured. + +Tests +----- + +* Extend the registry / configure tests in + ``enterprise_catalog/apps/catalog/tests/test_algolia_utils.py`` (e.g. + ``test_build_algolia_replicas_only_includes_configured_replicas`` and + ``test_configure_algolia_index_configures_*``) to cover the new key, using + ``override_settings(ALGOLIA={...})``. +* If you added a field computation, unit-test the ``get_course_`` helper. +* The secured-key tests in ``api_client/tests/test_algolia.py`` already loop the registry; add + the new index name to the "all indices" expectation if you want explicit coverage. + +Deploy / ops +------------ + +Once the code is merged and deployed: + +#. Ops sets ``PRICE_ASC_REPLICA_INDEX_NAME`` (to e.g. ``enterprise_catalog_price_asc``) in the + ``ALGOLIA`` config in edx-internal. **The whole** ``ALGOLIA`` **dict is replaced, not merged**, + so the entire dict must be restated with the new key included. +#. Run ``./manage.py reindex_algolia``. A *virtual* replica exists as soon as it is declared on + the primary index's settings (it mirrors the primary's records), so the replica is live after + one reindex — no separate population step. + +Frontend (only if the MFE will use the sort) +-------------------------------------------- + +The backend builds the replica regardless; a sort is only *user-visible* once the MFE points a +search at it. In ``frontend-app-learner-portal-enterprise``: + +* add an ``ALGOLIA__REPLICA_INDEX_NAME`` env var in ``src/index.tsx`` and + ``src/types/types.d.ts`` (mirror of the backend settings key); +* point the relevant ```` at it (see ``SearchVideo.jsx``, which uses the + base replica, or ``SearchCourse.jsx`` for the recency replica); +* if the sort changes default behavior, gate it behind a waffle flag and/or an Optimizely + experiment, exactly as the recency sort does (the flag doubles as a kill-switch — see ADR 0014). + +Safety properties you get for free +---------------------------------- + +* **Inert by default.** No configured name → the replica is never declared, configured, or + queryable. Merge and deploy ahead of ops with no effect. +* **Fail-safe configuration.** ``configure_algolia_index`` wraps each replica's settings call so an + ``AlgoliaException`` is logged and skipped — one replica failing to configure never aborts the + reindex, and the primary index plus the other replicas stay configured. +* **No** ``virtual(None)``. Unconfigured keys are skipped entirely, never declared as a broken + ``virtual(None)`` replica on the primary index. From da3bfa491b1280c5eae0cc99602194388a667b71 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 15:23:39 -0400 Subject: [PATCH 12/20] refactor: rename recency sort to "recently released" (earliest run start, any status) Align the newest-courses sort with master's ENT-11386: is_new_content and the sort now share _earliest_course_run_start, which keys off the earliest course-run start of ANY status (the Discovery release date) rather than published runs only -- so an old course with a recent re-publish no longer looks new or sorts as newest. Rename to match the new semantics (the index-name key is brand-new and unreleased, so no ops migration is needed): recently_published_timestamp -> recently_released_timestamp get_course_recently_published_timestamp -> get_course_recently_released_timestamp RECENTLY_PUBLISHED_REPLICA_INDEX_NAME -> RECENTLY_RELEASED_REPLICA_INDEX_NAME ALGOLIA_RECENTLY_PUBLISHED_..._SETTINGS -> ALGOLIA_RECENTLY_RELEASED_..._SETTINGS _earliest_published_course_run_start -> _earliest_course_run_start Updates tests, ADR 0014, and the how-to guide. Matching MFE rename follows in #1511. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 15 ++++--- docs/how_to/add_an_algolia_sort_replica.rst | 10 ++--- enterprise_catalog/apps/api_client/algolia.py | 2 +- .../apps/api_client/constants.py | 2 +- .../apps/api_client/tests/test_algolia.py | 14 +++--- .../apps/catalog/algolia_utils.py | 22 +++++----- .../apps/catalog/tests/test_algolia_utils.py | 44 +++++++++---------- enterprise_catalog/settings/base.py | 4 +- 8 files changed, 57 insertions(+), 56 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index 52ed86e9..436ab080 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -16,11 +16,12 @@ consumer switches sort by pointing its search at a different index name. To offer a "newest courses first" sort we add a recency-sorted replica. * The primary index (``ALGOLIA['INDEX_NAME']``) keeps the relevance ranking. -* A new replica (``ALGOLIA['RECENTLY_PUBLISHED_REPLICA_INDEX_NAME']``) leads its - ranking with ``desc(recently_published_timestamp)`` — a per-course Unix - timestamp of the *earliest published course-run start* (the same signal as the - ``is_new_content`` flag, via the shared ``_earliest_published_course_run_start`` - helper). Courses with no published run start get ``0`` so they sort last under +* A new replica (``ALGOLIA['RECENTLY_RELEASED_REPLICA_INDEX_NAME']``) leads its + ranking with ``desc(recently_released_timestamp)`` — a per-course Unix + timestamp of the *earliest course-run start of any status* (the discovery course + release date, the same signal as the ``is_new_content`` flag, via the shared + ``_earliest_course_run_start`` helper — ENT-11386). Courses with no run start + get ``0`` so they sort last under a descending ranking — deliberately not the far-future ``ALGOLIA_DEFAULT_TIMESTAMP``, which would float undated courses to the top. @@ -36,7 +37,7 @@ The sort is rolled out across three repositories: Two facts shape the failure modes: * ``ALGOLIA`` is *replaced* (not merged) from the deployment YAML, so the replica - is only live once ops sets ``RECENTLY_PUBLISHED_REPLICA_INDEX_NAME`` in + is only live once ops sets ``RECENTLY_RELEASED_REPLICA_INDEX_NAME`` in ``edx-internal`` and a ``reindex_algolia`` run declares it on the primary. * An Algolia *virtual* replica exists as soon as it is declared on the primary index's settings (it mirrors the primary's records); it does not wait for a @@ -93,7 +94,7 @@ Consequences * **Covered:** "replica name not configured" → the base (relevance) index is used. This is handled explicitly in both the backend (conditional replica - declaration) and the MFE (the ``&& recentlyPublishedIndexName`` guard), so the + declaration) and the MFE (the ``&& recentlyReleasedIndexName`` guard), so the default-to-base behavior is guaranteed for the unconfigured case. * **Not covered in code:** "replica name configured but the Algolia index does not exist yet" → the MFE would query a missing index and surface an error / diff --git a/docs/how_to/add_an_algolia_sort_replica.rst b/docs/how_to/add_an_algolia_sort_replica.rst index 10079405..7cad10b7 100644 --- a/docs/how_to/add_an_algolia_sort_replica.rst +++ b/docs/how_to/add_an_algolia_sort_replica.rst @@ -8,7 +8,7 @@ search at a different index name. All replicas are driven by **one registry**, so adding a sort is mostly additive. This guide walks through it end to end. See ``docs/decisions/0014-newest-courses-sort-replica.rst`` for -the design rationale, and treat the **recently-published ("newest first") replica** as the +the design rationale, and treat the **recently-released ("newest first") replica** as the canonical example to copy. The mental model: names vs. definitions @@ -46,7 +46,7 @@ Backend steps (this service) ALGOLIA = { 'INDEX_NAME': '', 'REPLICA_INDEX_NAME': '', # base replica, desc(duration); MFE video search - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': '', # "newest first", desc(recently_published_timestamp) + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': '', # "newest first", desc(recently_released_timestamp) # "price: low to high", asc(first_enrollable_paid_seat_price) 'PRICE_ASC_REPLICA_INDEX_NAME': '', 'APPLICATION_ID': '', @@ -65,7 +65,7 @@ import it without a circular dependency.) ALGOLIA_REPLICA_CONFIG_KEYS = ( 'REPLICA_INDEX_NAME', - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME', 'PRICE_ASC_REPLICA_INDEX_NAME', ) @@ -75,7 +75,7 @@ price example reuses ``first_enrollable_paid_seat_price``), skip this step. If i signal, in ``enterprise_catalog/apps/catalog/algolia_utils.py``: * write a ``get_course_(course)`` helper that returns the numeric value (see - ``get_course_recently_published_timestamp`` — note it returns ``0`` for "no value" so those + ``get_course_recently_released_timestamp`` — note it returns ``0`` for "no value" so those records sort last under a ``desc`` ranking; pick a sentinel that sorts your missing values to the *end* of *your* order); * add the field name to ``ALGOLIA_FIELDS``; @@ -108,7 +108,7 @@ pagination stays deterministic: ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { 'REPLICA_INDEX_NAME': ALGOLIA_REPLICA_INDEX_SETTINGS, - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, 'PRICE_ASC_REPLICA_INDEX_NAME': ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS, } diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index c796f4b8..579b34af 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -108,7 +108,7 @@ def set_index_settings(self, index_settings, index_name=None): Set settings on an Algolia index, defaulting to the primary index. Pass ``index_name`` to target a replica instead -- the base ``REPLICA_INDEX_NAME`` - replica, the "recently published" sort replica, etc. A replica must already be + replica, the "newest courses" sort replica, etc. A replica must already be declared on the primary index's ``replicas`` setting (Algolia creates replica indices when the primary index's settings are saved). diff --git a/enterprise_catalog/apps/api_client/constants.py b/enterprise_catalog/apps/api_client/constants.py index d1404f1d..03f16162 100644 --- a/enterprise_catalog/apps/api_client/constants.py +++ b/enterprise_catalog/apps/api_client/constants.py @@ -28,7 +28,7 @@ # *which* replicas exist, shared by the indexer and the secured-API-key restriction. ALGOLIA_REPLICA_CONFIG_KEYS = ( 'REPLICA_INDEX_NAME', # base replica, desc(duration); MFE video search uses it - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME', # "newest courses first", desc(recently_published_timestamp) + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME', # "newest courses first", desc(recently_released_timestamp) ) COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER = 15 diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index 4ae00888..f3b4826a 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -104,13 +104,13 @@ def test_set_index_settings_targets_named_replica(self): ``set_index_settings(index_name=...)`` applies settings to the replica resolved by name. """ client = self._build_client() - replica = mock.MagicMock(name='recently_published_replica') + replica = mock.MagicMock(name='recently_released_replica') client._client.init_index.return_value = replica - settings_payload = {'customRanking': ['desc(recently_published_timestamp)']} + settings_payload = {'customRanking': ['desc(recently_released_timestamp)']} - client.set_index_settings(settings_payload, index_name='enterprise_catalog_recently_published_desc') + client.set_index_settings(settings_payload, index_name='enterprise_catalog_recently_released_desc') - client._client.init_index.assert_called_once_with('enterprise_catalog_recently_published_desc') + client._client.init_index.assert_called_once_with('enterprise_catalog_recently_released_desc') replica.set_settings.assert_called_once_with(settings_payload) def test_set_index_settings_noop_when_primary_uninitialized(self): @@ -125,7 +125,7 @@ def test_set_index_settings_reraises_algolia_exception(self): An AlgoliaException from set_settings propagates to the caller. """ client = self._build_client() - replica = mock.MagicMock(name='recently_published_replica') + replica = mock.MagicMock(name='recently_released_replica') replica.set_settings.side_effect = AlgoliaException('boom') client._client.init_index.return_value = replica with self.assertRaises(AlgoliaException): @@ -134,7 +134,7 @@ def test_set_index_settings_reraises_algolia_exception(self): @override_settings(ALGOLIA={ 'INDEX_NAME': 'enterprise_catalog', 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_published_desc', + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_released_desc', 'SEARCH_API_KEY': 'fake-search-key', }) @mock.patch('enterprise_catalog.apps.api_client.algolia.SearchClient.generate_secured_api_key') @@ -150,7 +150,7 @@ def test_generate_secured_api_key_restricts_to_all_indices(self, mock_generate): assert restrictions['restrictIndices'] == [ 'enterprise_catalog', 'enterprise_catalog_duration_desc', - 'enterprise_catalog_recently_published_desc', + 'enterprise_catalog_recently_released_desc', ] @override_settings(ALGOLIA={ diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index c9219f02..2ec6ae9f 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -162,7 +162,7 @@ def _build_algolia_replicas(): 'transcript_languages', 'translation_languages', 'is_new_content', - 'recently_published_timestamp', + 'recently_released_timestamp', ] # default configuration for the index @@ -242,12 +242,12 @@ def _build_algolia_replicas(): } # Replica that sorts courses by recency (newest-first). Leads with the precomputed -# ``recently_published_timestamp`` (the course's earliest published run start) descending, +# ``recently_released_timestamp`` (the course's earliest course-run start, any status) descending, # then falls back to the primary index's tie-breakers. Surfaced in the Learner Portal search # page as the "newest courses first" sort. -ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS = { +ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS = { 'customRanking': [ - 'desc(recently_published_timestamp)', + 'desc(recently_released_timestamp)', 'asc(metadata_language)', 'asc(visible_via_association)', 'asc(created)', @@ -261,7 +261,7 @@ def _build_algolia_replicas(): # and its ``customRanking`` settings here (and computing the field its ``customRanking`` sorts on). ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { 'REPLICA_INDEX_NAME': ALGOLIA_REPLICA_INDEX_SETTINGS, - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, } @@ -701,11 +701,11 @@ def is_course_archived(course): return len(availability_list) == 0 or 'Archived' in availability_list -def _earliest_published_course_run_start(course): +def _earliest_course_run_start(course): """ The earliest course-run start as a timezone-aware datetime, or None. - Shared by the "new content" facet and the "recently published" sort timestamp so both key off + Shared by the "new content" facet and the "newest courses" sort timestamp so both key off the same signal: a course's earliest run start across runs of *any* status, matching the Discovery course release date (ENT-11386). Earlier runs that were later unpublished/archived still count, so a recent re-run can't make a long-standing course look new. @@ -721,14 +721,14 @@ def _earliest_published_course_run_start(course): def is_course_new_content(course): """True if the course's earliest run start (any status) is within the last 12 months (ENT-11386).""" - earliest_start = _earliest_published_course_run_start(course) + earliest_start = _earliest_course_run_start(course) if earliest_start is None: return False now = localized_utcnow() return now - relativedelta(months=12) <= earliest_start <= now -def get_course_recently_published_timestamp(course): +def get_course_recently_released_timestamp(course): """ Unix timestamp (int) of the course's earliest course-run start (any status), used as the custom-ranking attribute for the "newest courses first" Algolia replica. @@ -737,7 +737,7 @@ def get_course_recently_published_timestamp(course): default to 0 so they sort last under a desc ranking -- ``ALGOLIA_DEFAULT_TIMESTAMP`` (a far-future sentinel) is deliberately NOT used, since it would float undated courses to the top. """ - earliest_start = _earliest_published_course_run_start(course) + earliest_start = _earliest_course_run_start(course) return int(earliest_start.timestamp()) if earliest_start else 0 @@ -1769,7 +1769,7 @@ def _algolia_object_from_product(product, algolia_fields): 'translation_languages': get_course_translation_languages(searchable_product), 'metadata_language': searchable_product.get('metadata_language', 'en'), 'is_new_content': is_course_new_content(searchable_product), - 'recently_published_timestamp': get_course_recently_published_timestamp(searchable_product), + 'recently_released_timestamp': get_course_recently_released_timestamp(searchable_product), }) elif searchable_product.get('content_type') == PROGRAM: # Build course metadata cache once for all program functions that need it diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 2e63c470..936014fc 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -248,7 +248,7 @@ def test_is_course_new_content(self): {'status': 'published', 'start': 'not-a-date'}, ]}) is False - def test_get_course_recently_published_timestamp(self): + def test_get_course_recently_released_timestamp(self): """ Verify the "newest courses" sort timestamp uses the earliest run start (any status). """ @@ -257,28 +257,28 @@ def test_get_course_recently_published_timestamp(self): old = now - timedelta(days=500) # No run start -> 0 so the course sorts last under a desc ranking. - assert utils.get_course_recently_published_timestamp({'course_runs': []}) == 0 + assert utils.get_course_recently_released_timestamp({'course_runs': []}) == 0 # Malformed ISO strings are silently ignored (treated as no date). - assert utils.get_course_recently_published_timestamp( + assert utils.get_course_recently_released_timestamp( {'course_runs': [{'status': 'published', 'start': 'not-a-date'}]} ) == 0 # Run status is irrelevant: an unpublished run's start still counts. - assert utils.get_course_recently_published_timestamp( + assert utils.get_course_recently_released_timestamp( {'course_runs': [{'status': 'unpublished', 'start': recent.isoformat()}]} ) == int(recent.timestamp()) # A run start -> its Unix timestamp, regardless of age (unlike is_new_content's 12-month window). - assert utils.get_course_recently_published_timestamp( + assert utils.get_course_recently_released_timestamp( {'course_runs': [{'status': 'published', 'start': recent.isoformat()}]} ) == int(recent.timestamp()) - assert utils.get_course_recently_published_timestamp( + assert utils.get_course_recently_released_timestamp( {'course_runs': [{'status': 'published', 'start': old.isoformat()}]} ) == int(old.timestamp()) # Earliest start wins across any status: an old unpublished run is the release date. - assert utils.get_course_recently_published_timestamp({'course_runs': [ + assert utils.get_course_recently_released_timestamp({'course_runs': [ {'status': 'unpublished', 'start': old.isoformat()}, {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) - assert utils.get_course_recently_published_timestamp({'course_runs': [ + assert utils.get_course_recently_released_timestamp({'course_runs': [ {'status': 'published', 'start': old.isoformat()}, {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) @@ -288,11 +288,11 @@ def test_build_algolia_replicas_only_includes_configured_replicas(self): # pylint: disable=protected-access with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_published_desc', + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_released_desc', }): assert utils._build_algolia_replicas() == [ 'virtual(enterprise_catalog_duration_desc)', - 'virtual(enterprise_catalog_recently_published_desc)', + 'virtual(enterprise_catalog_recently_released_desc)', ] # Only the base replica configured -> only it is declared. with override_settings(ALGOLIA={'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc'}): @@ -301,15 +301,15 @@ def test_build_algolia_replicas_only_includes_configured_replicas(self): with override_settings(ALGOLIA={}): assert not utils._build_algolia_replicas() - def test_algolia_object_includes_recently_published_timestamp(self): - """The course Algolia object carries recently_published_timestamp (and is_new_content).""" + def test_algolia_object_includes_recently_released_timestamp(self): + """The course Algolia object carries recently_released_timestamp (and is_new_content).""" # pylint: disable=protected-access course_metadata = ContentMetadataFactory(content_type=COURSE) algolia_object = utils._algolia_object_from_product( course_metadata.json_metadata, utils.ALGOLIA_FIELDS, ) - expected = utils.get_course_recently_published_timestamp(course_metadata.json_metadata) - assert algolia_object['recently_published_timestamp'] == expected + expected = utils.get_course_recently_released_timestamp(course_metadata.json_metadata) + assert algolia_object['recently_released_timestamp'] == expected assert 'is_new_content' in algolia_object @ddt.data( @@ -1069,16 +1069,16 @@ def test_configure_algolia_index(self, mock_search_client): assert set_index_settings.call_count == 2 @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') - def test_configure_algolia_index_configures_recently_published_replica(self, mock_search_client): + def test_configure_algolia_index_configures_recently_released_replica(self, mock_search_client): """ - When a recently-published replica name is configured, its settings are applied too. + When a recently-released replica name is configured, its settings are applied too. """ algolia_client = utils.get_initialized_algolia_client() - replica_name = 'enterprise_catalog_recently_published_desc' - with override_settings(ALGOLIA={'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': replica_name}): + replica_name = 'enterprise_catalog_recently_released_desc' + with override_settings(ALGOLIA={'RECENTLY_RELEASED_REPLICA_INDEX_NAME': replica_name}): utils.configure_algolia_index(algolia_client) mock_search_client.return_value.set_index_settings.assert_any_call( - utils.ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + utils.ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, index_name=replica_name, ) @@ -1090,7 +1090,7 @@ def test_configure_algolia_index_replica_failure_is_safe(self, mock_search_clien """ algolia_client = utils.get_initialized_algolia_client() base_name = 'enterprise_catalog_duration_desc' - recency_name = 'enterprise_catalog_recently_published_desc' + recency_name = 'enterprise_catalog_recently_released_desc' def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=unused-argument # Primary and base-replica calls succeed; only the recency replica raises. @@ -1100,7 +1100,7 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u mock_search_client.return_value.set_index_settings.side_effect = fail_only_for_recency with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': base_name, - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': recency_name, + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': recency_name, }): with self.assertLogs(utils.LOGGER, level='ERROR') as error_logs: # Does not raise, even though the recency replica fails. @@ -1111,7 +1111,7 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u set_index_settings.assert_any_call(utils.ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=base_name) # The recency replica was attempted (and failed) -- logged, not swallowed silently. set_index_settings.assert_any_call( - utils.ALGOLIA_RECENTLY_PUBLISHED_REPLICA_INDEX_SETTINGS, + utils.ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, index_name=recency_name, ) assert any(recency_name in message for message in error_logs.output) diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index 005924b1..91d90b11 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -441,8 +441,8 @@ # its video search (SEARCH_INDEX_IDS.VIDEOS) at this index, so it is also the MFE's # ALGOLIA_REPLICA_INDEX_NAME env var. 'REPLICA_INDEX_NAME': '', - # "Newest courses first" replica, sorted by desc(recently_published_timestamp). - 'RECENTLY_PUBLISHED_REPLICA_INDEX_NAME': '', + # "Newest courses first" replica, sorted by desc(recently_released_timestamp). + 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': '', 'APPLICATION_ID': '', 'API_KEY': '', } From de56e128197fe94b8228dfac8830197523412ee1 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 15:35:49 -0400 Subject: [PATCH 13/20] refactor: address review feedback on recency-sort replica - incremental_reindex_algolia: drop the now-dead replica_index init; the replica is configured by name via set_index_settings(index_name=...), which initializes it lazily through _get_index(). - configure_algolia_index: log a single-line WARNING when a replica fails to configure (set_index_settings already logs the traceback before re-raising), so a failed replica no longer emits two stack traces. - configure_algolia_index: build the primary's declared replicas list dynamically from settings.ALGOLIA rather than an import-time global, so it stays consistent with the replicas actually configured and can be varied in tests via override_settings. Drop the ALGOLIA_REPLICAS global. - ADR 0014: document that the virtual replica mirrors non-course records, which have no recently_released_timestamp and so sort last by design. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 10 +++++++++ .../apps/catalog/algolia_utils.py | 15 +++++++------ .../apps/catalog/tests/test_algolia_utils.py | 21 ++++++++++++++----- .../commands/incremental_reindex_algolia.py | 1 - .../tests/test_incremental_reindex_algolia.py | 8 ++++--- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index 436ab080..70d022bf 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -106,6 +106,16 @@ Consequences intact, so a problem with one sort can never take down core search indexing. The worst case is that one replica lags its ranking until the next run — never a broken or empty primary index. +* **Non-course records sort last, by design:** the replica is *virtual* over the + primary index, so it mirrors every record — programs, executive education, videos, + etc. — not just courses. ``recently_released_timestamp`` is only computed in the + ``content_type == COURSE`` branch, so non-course records have no such attribute and + Algolia ranks them last under ``desc(recently_released_timestamp)``. This is fine + because the consumer (the Learner Portal) points only its course ```` at the + replica and filters by content type; the "newest courses first" sort is, by + contract, a course sort. Were a future caller to query this replica for non-course + content, those records would all tie at the bottom — that caller would need its own + recency field. * **The waffle flag is the readiness contract:** enabling it asserts "the replica is live." This keeps the safe path a single, instantly reversible toggle rather than per-request defensive logic in the search hot path. diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 2ec6ae9f..3c3a6180 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -85,9 +85,6 @@ def _build_algolia_replicas(): return replicas -# Replicas declared on the primary index (see ``_build_algolia_replicas``). -ALGOLIA_REPLICAS = _build_algolia_replicas() - # keep attributes from content objects that we explicitly want in Algolia ALGOLIA_FIELDS = [ 'additional_information', @@ -227,7 +224,6 @@ def _build_algolia_replicas(): 'desc(course_bayesian_average)', 'desc(recent_enrollment_count)', ], - 'replicas': ALGOLIA_REPLICAS, } ALGOLIA_REPLICA_INDEX_SETTINGS = { @@ -473,7 +469,12 @@ def configure_algolia_index(algolia_client): """ Configures the settings for the primary Algolia index and its replicas. """ - algolia_client.set_index_settings(ALGOLIA_INDEX_SETTINGS) + # Declare the replicas dynamically (from the current settings.ALGOLIA) rather than from an + # import-time global, so the set the primary declares stays consistent with the replicas + # _configured_replicas() actually configures below -- and so tests can vary it via + # override_settings. + primary_settings = {**ALGOLIA_INDEX_SETTINGS, 'replicas': _build_algolia_replicas()} + algolia_client.set_index_settings(primary_settings) for index_name, index_settings in _configured_replicas(): # A replica's settings failing to apply must never abort the reindex: log and continue so # the primary (relevance) index and the other replicas stay configured. The failed replica @@ -482,7 +483,9 @@ def configure_algolia_index(algolia_client): try: algolia_client.set_index_settings(index_settings, index_name=index_name) except AlgoliaException: - LOGGER.exception( + # set_index_settings() already logged the traceback before re-raising, so log a + # single-line warning here rather than a second stack trace. + LOGGER.warning( 'Failed to configure replica index "%s"; continuing with the remaining indexes.', index_name, ) diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 936014fc..80911efc 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -1059,7 +1059,12 @@ def test_configure_algolia_index(self, mock_search_client): with override_settings(ALGOLIA={'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc'}): utils.configure_algolia_index(algolia_client) set_index_settings = mock_search_client.return_value.set_index_settings - set_index_settings.assert_any_call(utils.ALGOLIA_INDEX_SETTINGS) + # The primary index is declared with the dynamically-built replicas list (the single + # configured base replica here), not the bare ALGOLIA_INDEX_SETTINGS. + set_index_settings.assert_any_call({ + **utils.ALGOLIA_INDEX_SETTINGS, + 'replicas': ['virtual(enterprise_catalog_duration_desc)'], + }) set_index_settings.assert_any_call( utils.ALGOLIA_REPLICA_INDEX_SETTINGS, index_name='enterprise_catalog_duration_desc', @@ -1102,19 +1107,25 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u 'REPLICA_INDEX_NAME': base_name, 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': recency_name, }): - with self.assertLogs(utils.LOGGER, level='ERROR') as error_logs: + # The per-replica handler logs a single-line WARNING (set_index_settings already logged + # the traceback before re-raising), so we don't double up stack traces. + with self.assertLogs(utils.LOGGER, level='WARNING') as warning_logs: # Does not raise, even though the recency replica fails. utils.configure_algolia_index(algolia_client) set_index_settings = mock_search_client.return_value.set_index_settings - # The primary (relevance) index and the base replica were still configured. - set_index_settings.assert_any_call(utils.ALGOLIA_INDEX_SETTINGS) + # The primary (relevance) index is declared with both replicas, and the base replica + # was still configured. + set_index_settings.assert_any_call({ + **utils.ALGOLIA_INDEX_SETTINGS, + 'replicas': [f'virtual({base_name})', f'virtual({recency_name})'], + }) set_index_settings.assert_any_call(utils.ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=base_name) # The recency replica was attempted (and failed) -- logged, not swallowed silently. set_index_settings.assert_any_call( utils.ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, index_name=recency_name, ) - assert any(recency_name in message for message in error_logs.output) + assert any(recency_name in message for message in warning_logs.output) @ddt.data( ( diff --git a/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py b/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py index c80c0397..3b01f7b2 100644 --- a/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py +++ b/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py @@ -113,7 +113,6 @@ def handle(self, *args, **options): # the underlying client must be set (init_index() would target the production index). algolia_client._client = sdk_client # pylint: disable=protected-access algolia_client.algolia_index = sdk_client.init_index(index_name) - algolia_client.replica_index = sdk_client.init_index(replica_name) primary_settings = {**ALGOLIA_INDEX_SETTINGS, 'replicas': [f'virtual({replica_name})']} algolia_client.set_index_settings(primary_settings) algolia_client.set_index_settings(ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=replica_name) diff --git a/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py b/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py index 074b7f84..51360671 100644 --- a/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py +++ b/enterprise_catalog/apps/search/management/commands/tests/test_incremental_reindex_algolia.py @@ -189,9 +189,9 @@ def test_configure_index_called_before_dispatch(self, mock_task): self._call('--index-name', 'enterprise_catalog_v2') sdk = self.mock_new_sdk_client.return_value algolia_instance = self.mock_algolia_cls.return_value - # Both primary and replica indices are initialized + # The primary handle is initialized directly; the replica is configured by name via + # set_index_settings(index_name=...), which initializes it lazily inside the client. sdk.init_index.assert_any_call('enterprise_catalog_v2') - sdk.init_index.assert_any_call('enterprise_catalog_v2_repl') # set_index_settings called twice: primary (with replicas overridden) then replica assert algolia_instance.set_index_settings.call_count == 2 primary_call_kwargs = algolia_instance.set_index_settings.call_args_list[0] @@ -206,10 +206,12 @@ def test_explicit_replica_name_used(self, mock_task): self._call('--index-name', 'enterprise_catalog_v2', '--replica-name', 'my_replica') sdk = self.mock_new_sdk_client.return_value sdk.init_index.assert_any_call('enterprise_catalog_v2') - sdk.init_index.assert_any_call('my_replica') algolia_instance = self.mock_algolia_cls.return_value primary_settings = algolia_instance.set_index_settings.call_args_list[0][0][0] assert primary_settings['replicas'] == ['virtual(my_replica)'] + # The replica is configured by name (which initializes it lazily inside the client). + replica_call = algolia_instance.set_index_settings.call_args_list[1] + assert replica_call[1].get('index_name') == 'my_replica' @mock.patch(TASK_PATH) def test_configure_index_skipped_on_dry_run(self, mock_task): From c5644d36e33646178c05f60d97f4a6fe81425b92 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 21:15:08 -0400 Subject: [PATCH 14/20] refactor: address Troy's review comments on recency-sort replica - Rename _build_algolia_replicas -> _get_algolia_replica_names (it returns a list of replica names, not replicas) and the local replicas -> replica_names; add a list[str] return annotation. Update references in tests and the how-to guide. - Add an int return annotation to get_course_recently_released_timestamp so the int return reads clearly despite the datetime-sounding name. - settings/base.py: document each replica's purpose (the MFE video-search sort; the newest-courses sort) rather than the volatile customRanking detail, which now lives only in the *_REPLICA_INDEX_SETTINGS dicts. - ADR 0014: move the design (what the replica does + the rollout) out of Context into Decision, leaving Context to background and the problem statement. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 56 ++++++++++--------- docs/how_to/add_an_algolia_sort_replica.rst | 4 +- .../apps/catalog/algolia_utils.py | 22 ++++---- .../apps/catalog/tests/test_algolia_utils.py | 8 +-- enterprise_catalog/settings/base.py | 9 +-- 5 files changed, 53 insertions(+), 46 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index 70d022bf..b70bd68f 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -12,41 +12,47 @@ Context The enterprise Learner Portal search page sorts a single Algolia index by relevance. Algolia does not re-sort an index at query time, so each alternate sort order is a separate *replica* index with its own ``customRanking``; the -consumer switches sort by pointing its search at a different index name. To -offer a "newest courses first" sort we add a recency-sorted replica. - -* The primary index (``ALGOLIA['INDEX_NAME']``) keeps the relevance ranking. -* A new replica (``ALGOLIA['RECENTLY_RELEASED_REPLICA_INDEX_NAME']``) leads its - ranking with ``desc(recently_released_timestamp)`` — a per-course Unix - timestamp of the *earliest course-run start of any status* (the discovery course - release date, the same signal as the ``is_new_content`` flag, via the shared - ``_earliest_course_run_start`` helper — ENT-11386). Courses with no run start - get ``0`` so they sort last under - a descending ranking — deliberately not the far-future ``ALGOLIA_DEFAULT_TIMESTAMP``, - which would float undated courses to the top. +consumer switches sort by pointing its search at a different index name. We +already maintain one such replica — the base ``ALGOLIA['REPLICA_INDEX_NAME']``, +which the Learner Portal points its video search at. -The sort is rolled out across three repositories: - -#. **enterprise-catalog** (this service) builds and configures the replica. -#. **edx-enterprise** exposes the ``enterprise.search_default_sort_newest`` - waffle flag via ``enterprise_features`` — the eligibility gate / kill-switch. -#. **frontend-app-learner-portal-enterprise** points the course ```` at - the replica when the flag is on *and* the Optimizely "newest" experiment - variant is active for the user. +Two facts about this machinery constrain how any new replica can be rolled out: -Two facts shape the failure modes: - -* ``ALGOLIA`` is *replaced* (not merged) from the deployment YAML, so the replica - is only live once ops sets ``RECENTLY_RELEASED_REPLICA_INDEX_NAME`` in - ``edx-internal`` and a ``reindex_algolia`` run declares it on the primary. +* ``ALGOLIA`` is *replaced* (not merged) from the deployment YAML, so a replica + is only live once ops sets its index-name key in ``edx-internal`` and a + ``reindex_algolia`` run declares it on the primary. * An Algolia *virtual* replica exists as soon as it is declared on the primary index's settings (it mirrors the primary's records); it does not wait for a populated record set. So once this service is deployed and ``reindex_algolia`` has run, the replica exists. +We want to offer learners a "newest courses first" sort. The problem is how to +add that sort — and roll it out across the repositories that together own +enterprise search — without a partially-configured replica degrading or breaking +the existing relevance search. + Decision -------- +Add a recency-sorted replica (``ALGOLIA['RECENTLY_RELEASED_REPLICA_INDEX_NAME']``) +that leads its ranking with ``desc(recently_released_timestamp)`` — a per-course +Unix timestamp of the *earliest course-run start of any status* (the Discovery +course release date, the same signal as the ``is_new_content`` flag, via the +shared ``_earliest_course_run_start`` helper — ENT-11386). Courses with no run +start get ``0`` so they sort last under a descending ranking — deliberately not +the far-future ``ALGOLIA_DEFAULT_TIMESTAMP``, which would float undated courses to +the top. The primary index (``ALGOLIA['INDEX_NAME']``) keeps the relevance +ranking. + +The sort is rolled out across three repositories: + +#. **enterprise-catalog** (this service) builds and configures the replica. +#. **edx-enterprise** exposes the ``enterprise.search_default_sort_newest`` + waffle flag via ``enterprise_features`` — the eligibility gate / kill-switch. +#. **frontend-app-learner-portal-enterprise** points the course ```` at + the replica when the flag is on *and* the Optimizely "newest" experiment + variant is active for the user. + The replica is **config-gated on both sides** and is never queried unless its name is configured: diff --git a/docs/how_to/add_an_algolia_sort_replica.rst b/docs/how_to/add_an_algolia_sort_replica.rst index 7cad10b7..8114af80 100644 --- a/docs/how_to/add_an_algolia_sort_replica.rst +++ b/docs/how_to/add_an_algolia_sort_replica.rst @@ -112,7 +112,7 @@ pagination stays deterministic: 'PRICE_ASC_REPLICA_INDEX_NAME': ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS, } -**That is all the wiring.** You do **not** touch ``_build_algolia_replicas``, +**That is all the wiring.** You do **not** touch ``_get_algolia_replica_names``, ``_configured_replicas``, ``configure_algolia_index``, or the secured-key ``replica_index_names`` — they all loop the registry, so the new replica is automatically declared on the primary index, has its settings applied during a reindex, and is added to the @@ -123,7 +123,7 @@ Tests * Extend the registry / configure tests in ``enterprise_catalog/apps/catalog/tests/test_algolia_utils.py`` (e.g. - ``test_build_algolia_replicas_only_includes_configured_replicas`` and + ``test_get_algolia_replica_names_only_includes_configured_replicas`` and ``test_configure_algolia_index_configures_*``) to cover the new key, using ``override_settings(ALGOLIA={...})``. * If you added a field computation, unit-test the ``get_course_`` helper. diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 3c3a6180..3331bd8a 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -68,21 +68,21 @@ ALGOLIA_JSON_METADATA_MAX_SIZE = 100000 -def _build_algolia_replicas(): +def _get_algolia_replica_names() -> list[str]: """ - Build the list of replica indexes to declare on the primary index. + Build the list of replica index names to declare on the primary index. - Declares a ``virtual(name)`` for each replica in the registry (``ALGOLIA_REPLICA_CONFIG_KEYS`` - -- the base duration replica plus any additive sort replica) whose index name is configured. - Unconfigured replicas are omitted, so deploying this code before ops sets a name won't declare - a ``virtual(None)`` replica on the primary index. + Returns a ``virtual(name)`` entry for each replica in the registry + (``ALGOLIA_REPLICA_CONFIG_KEYS`` -- the base duration replica plus any additive sort replica) + whose index name is configured. Unconfigured replicas are omitted, so deploying this code + before ops sets a name won't declare a ``virtual(None)`` replica on the primary index. """ - replicas = [] + replica_names = [] for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: index_name = settings.ALGOLIA.get(config_key) if index_name: - replicas.append(f'virtual({index_name})') - return replicas + replica_names.append(f'virtual({index_name})') + return replica_names # keep attributes from content objects that we explicitly want in Algolia @@ -473,7 +473,7 @@ def configure_algolia_index(algolia_client): # import-time global, so the set the primary declares stays consistent with the replicas # _configured_replicas() actually configures below -- and so tests can vary it via # override_settings. - primary_settings = {**ALGOLIA_INDEX_SETTINGS, 'replicas': _build_algolia_replicas()} + primary_settings = {**ALGOLIA_INDEX_SETTINGS, 'replicas': _get_algolia_replica_names()} algolia_client.set_index_settings(primary_settings) for index_name, index_settings in _configured_replicas(): # A replica's settings failing to apply must never abort the reindex: log and continue so @@ -731,7 +731,7 @@ def is_course_new_content(course): return now - relativedelta(months=12) <= earliest_start <= now -def get_course_recently_released_timestamp(course): +def get_course_recently_released_timestamp(course) -> int: """ Unix timestamp (int) of the course's earliest course-run start (any status), used as the custom-ranking attribute for the "newest courses first" Algolia replica. diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 80911efc..6ccaa509 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -283,23 +283,23 @@ def test_get_course_recently_released_timestamp(self): {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) - def test_build_algolia_replicas_only_includes_configured_replicas(self): + def test_get_algolia_replica_names_only_includes_configured_replicas(self): """Each replica (base + optional) is declared only when its index name is configured.""" # pylint: disable=protected-access with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_released_desc', }): - assert utils._build_algolia_replicas() == [ + assert utils._get_algolia_replica_names() == [ 'virtual(enterprise_catalog_duration_desc)', 'virtual(enterprise_catalog_recently_released_desc)', ] # Only the base replica configured -> only it is declared. with override_settings(ALGOLIA={'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc'}): - assert utils._build_algolia_replicas() == ['virtual(enterprise_catalog_duration_desc)'] + assert utils._get_algolia_replica_names() == ['virtual(enterprise_catalog_duration_desc)'] # Nothing configured -> no replicas at all, never virtual(None). with override_settings(ALGOLIA={}): - assert not utils._build_algolia_replicas() + assert not utils._get_algolia_replica_names() def test_algolia_object_includes_recently_released_timestamp(self): """The course Algolia object carries recently_released_timestamp (and is_new_content).""" diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index 91d90b11..70bd2911 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -437,11 +437,12 @@ # when its key holds a non-empty name (see ALGOLIA_REPLICA_CONFIG_KEYS). ALGOLIA = { 'INDEX_NAME': '', - # Base replica, sorted by duration descending (desc(duration)). The learner-portal MFE points - # its video search (SEARCH_INDEX_IDS.VIDEOS) at this index, so it is also the MFE's - # ALGOLIA_REPLICA_INDEX_NAME env var. + # Base replica: the long-standing sort the learner-portal MFE points its video search + # (SEARCH_INDEX_IDS.VIDEOS) at -- the MFE's ALGOLIA_REPLICA_INDEX_NAME env var. Its + # customRanking lives in ALGOLIA_REPLICA_INDEX_SETTINGS. 'REPLICA_INDEX_NAME': '', - # "Newest courses first" replica, sorted by desc(recently_released_timestamp). + # "Newest courses first" replica, surfaced as a sort option on the learner-portal course + # search. Its customRanking lives in ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS. 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': '', 'APPLICATION_ID': '', 'API_KEY': '', From 0a14b05dff6dead396d335f81ef131b5b08d8731 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 21:56:47 -0400 Subject: [PATCH 15/20] fix: report the effective Algolia index name on set_index_settings failure When set_index_settings() is called without index_name, it targets the cached handle (self.algolia_index), which a caller may have wired to an alternate index (the incremental reindex command's --index-name path). The failure log used self.algolia_index_name (the configured primary name), which is stale or empty for that caller. Derive the effective name from the handle instead, falling back to the configured name. Addresses Copilot review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) --- enterprise_catalog/apps/api_client/algolia.py | 7 +++++- .../apps/api_client/tests/test_algolia.py | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index 579b34af..56b655ae 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -126,9 +126,14 @@ def set_index_settings(self, index_settings, index_name=None): try: self._get_index(index_name).set_settings(index_settings) except AlgoliaException as exc: + # With no index_name, the target is the cached handle (self.algolia_index), which a + # caller may have wired to an alternate index (e.g. the incremental reindex command's + # --index-name path). Report that handle's actual name rather than the configured + # primary name (algolia_index_name), which could be stale or empty for that caller. + effective_index_name = index_name or getattr(self.algolia_index, 'name', None) or self.algolia_index_name logger.exception( 'Unable to set settings for Algolia\'s %s index due to an exception.', - index_name or self.algolia_index_name, + effective_index_name, ) raise exc diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index f3b4826a..1263f747 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -131,6 +131,29 @@ def test_set_index_settings_reraises_algolia_exception(self): with self.assertRaises(AlgoliaException): client.set_index_settings({'customRanking': []}, index_name='some_replica') + def test_set_index_settings_failure_logs_effective_index_name(self): + """ + When no index_name is given but the cached handle is wired to an alternate index, the + failure log reports that handle's actual name -- not the (possibly stale/empty) configured + primary name. Mirrors the incremental reindex command's --index-name path. + """ + client = self._build_client() + # The cached handle is wired to an alternate index, as the incremental command does. Use a + # name with no substring overlap with PRIMARY_INDEX_NAME so the assertions can't false-pass. + wired_index_name = 'some_alternate_index' + client.algolia_index.name = wired_index_name + client.algolia_index.set_settings.side_effect = AlgoliaException('boom') + + with self.assertLogs('enterprise_catalog.apps.api_client.algolia', level='ERROR') as logs: + with self.assertRaises(AlgoliaException): + client.set_index_settings({'customRanking': []}) # no index_name -> targets the handle + + # Assert on the formatted message only; the captured traceback contains the package path + # ("enterprise_catalog/...") which would otherwise collide with PRIMARY_INDEX_NAME. + message = logs.records[0].getMessage() + self.assertIn(wired_index_name, message) + self.assertNotIn(self.PRIMARY_INDEX_NAME, message) + @override_settings(ALGOLIA={ 'INDEX_NAME': 'enterprise_catalog', 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', From 90790d3db57d712be68797b19c056dc754c6128a Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Mon, 15 Jun 2026 22:15:50 -0400 Subject: [PATCH 16/20] fix: harden Algolia index configuration against registry drift and uninitialized client Addresses two Copilot review comments: - _configured_replicas(): a configured replica key with no entry in ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY (registry drift) is now skipped with an error log instead of raising KeyError and aborting the reindex -- consistent with the per-replica fail-safe behavior. - configure_algolia_index(): fail fast with ImproperlyConfigured when the primary index handle is missing (init_index() bailed on missing INDEX_NAME / credentials). Since ALGOLIA is replaced per-environment, this prevents a misconfigured run from silently no-op'ing and reporting success. Adds tests for both paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../apps/catalog/algolia_utils.py | 29 +++++++++++++-- .../apps/catalog/tests/test_algolia_utils.py | 35 +++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 3331bd8a..f7b11f2b 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -8,6 +8,7 @@ from dateutil import parser from dateutil.relativedelta import relativedelta from django.conf import settings +from django.core.exceptions import ImproperlyConfigured from django.db.models import Q from django.utils.dateparse import parse_datetime from django.utils.translation import gettext as _ @@ -267,13 +268,26 @@ def _configured_replicas(): Covers every replica in the registry -- the base duration replica and any additive sort replica (``ALGOLIA_REPLICA_CONFIG_KEYS``). An unconfigured replica is skipped, so this is - inert until its name is provided in ``settings.ALGOLIA``. + inert until its name is provided in ``settings.ALGOLIA``. A configured replica whose key has + no settings mapped (registry drift -- a key added to ``ALGOLIA_REPLICA_CONFIG_KEYS`` without a + matching entry in ``ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY``) is likewise skipped, with + an error logged, rather than raising a ``KeyError`` that would abort the whole reindex. """ configured = [] for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: index_name = settings.ALGOLIA.get(config_key) - if index_name: - configured.append((index_name, ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY[config_key])) + if not index_name: + continue + index_settings = ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY.get(config_key) + if index_settings is None: + LOGGER.error( + 'Algolia replica config key "%s" is configured (index "%s") but has no settings ' + 'mapped in ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY; skipping it.', + config_key, + index_name, + ) + continue + configured.append((index_name, index_settings)) return configured @@ -469,6 +483,15 @@ def configure_algolia_index(algolia_client): """ Configures the settings for the primary Algolia index and its replicas. """ + if not algolia_client.algolia_index: + # init_index() logs and returns without setting algolia_index when a required name + # (INDEX_NAME / REPLICA_INDEX_NAME) or credential is missing. set_index_settings() would + # then silently no-op for every index, making a misconfigured reindex look successful. + # Since ALGOLIA is replaced (not merged) per-environment, fail loudly here instead. + raise ImproperlyConfigured( + 'Cannot configure Algolia index: the primary index is not initialized. Check the ' + 'ALGOLIA settings (INDEX_NAME, REPLICA_INDEX_NAME, APPLICATION_ID, API_KEY).' + ) # Declare the replicas dynamically (from the current settings.ALGOLIA) rather than from an # import-time global, so the set the primary declares stays consistent with the replicas # _configured_replicas() actually configures below -- and so tests can vary it via diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 6ccaa509..b46398a6 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -5,6 +5,7 @@ import ddt from algoliasearch.exceptions import AlgoliaException from dateutil.relativedelta import relativedelta +from django.core.exceptions import ImproperlyConfigured from django.test import TestCase, override_settings from enterprise_catalog.apps.catalog import algolia_utils as utils @@ -1127,6 +1128,40 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u ) assert any(recency_name in message for message in warning_logs.output) + def test_configured_replicas_skips_config_key_without_settings_mapping(self): + """ + A configured replica whose key has no settings mapped (registry drift) is skipped with an + error logged -- never a KeyError that would abort the reindex. The well-formed replicas + are still returned. + """ + # pylint: disable=protected-access + bogus_key = 'BOGUS_REPLICA_INDEX_NAME' # in the registry tuple, but not in the settings map + registry = utils.ALGOLIA_REPLICA_CONFIG_KEYS + (bogus_key,) + with mock.patch.object(utils, 'ALGOLIA_REPLICA_CONFIG_KEYS', registry): + with override_settings(ALGOLIA={ + 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', + bogus_key: 'enterprise_catalog_bogus_desc', + }): + with self.assertLogs(utils.LOGGER, level='ERROR') as error_logs: + configured = utils._configured_replicas() + # The well-formed base replica is returned; the unmapped key is skipped (not raised on). + assert ('enterprise_catalog_duration_desc', utils.ALGOLIA_REPLICA_INDEX_SETTINGS) in configured + assert all(index_name != 'enterprise_catalog_bogus_desc' for index_name, _ in configured) + assert any(bogus_key in message for message in error_logs.output) + + @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') + def test_configure_algolia_index_raises_when_primary_uninitialized(self, mock_search_client): + """ + If the primary index was never initialized (e.g. init_index() bailed on a missing + INDEX_NAME), configuring fails fast instead of silently applying no settings. + """ + algolia_client = utils.get_initialized_algolia_client() + algolia_client.algolia_index = None # simulate init_index() bailing on missing config + with self.assertRaises(ImproperlyConfigured): + utils.configure_algolia_index(algolia_client) + # Nothing was configured -- we refuse rather than report a no-op reindex as successful. + mock_search_client.return_value.set_index_settings.assert_not_called() + @ddt.data( ( {'courses': [{'key': 'program_course_key'}, {'key': 'other_course_key'}]}, From 672f1e481663f20fe837a3050207693cb411de10 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Tue, 16 Jun 2026 10:04:16 -0400 Subject: [PATCH 17/20] docs: refine ADR 0014 per review (virtual-replica tradeoff, cost, context) Addresses Troy's ADR comments: - Context: state the pre-existing single-replica design assumption (one setting, one provisioning function) as historical background, and frame the problem as adding a *second* replica. - Decision: state explicitly that we declare a *virtual* replica. - Alternatives considered: add 'standard (non-virtual) replica' -- deterministic newest-first but ~doubles record count/cost; we accept virtual's relevance-bias tradeoff to avoid that. Documents the escape hatch. - Consequences: add the 'no added record cost' consequence (a standard replica would roughly double the indexed record count). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index b70bd68f..e13f8ead 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -12,24 +12,22 @@ Context The enterprise Learner Portal search page sorts a single Algolia index by relevance. Algolia does not re-sort an index at query time, so each alternate sort order is a separate *replica* index with its own ``customRanking``; the -consumer switches sort by pointing its search at a different index name. We -already maintain one such replica — the base ``ALGOLIA['REPLICA_INDEX_NAME']``, -which the Learner Portal points its video search at. +consumer switches sort by pointing its search at a different index name. -Two facts about this machinery constrain how any new replica can be rolled out: +The fundamental design of the enterprise-catalog logic assumes we will only ever +provision *one* replica: there is a single setting +(``ALGOLIA['REPLICA_INDEX_NAME']`` — the base "duration" replica the Learner +Portal points its video search at) and a single Python function that provisions +that one replica. -* ``ALGOLIA`` is *replaced* (not merged) from the deployment YAML, so a replica - is only live once ops sets its index-name key in ``edx-internal`` and a - ``reindex_algolia`` run declares it on the primary. -* An Algolia *virtual* replica exists as soon as it is declared on the primary - index's settings (it mirrors the primary's records); it does not wait for a - populated record set. So once this service is deployed and ``reindex_algolia`` - has run, the replica exists. +``ALGOLIA`` is also *replaced* (not merged) from the deployment YAML, so a replica +is only live once ops sets its index-name key in ``edx-internal`` and a +``reindex_algolia`` run declares it on the primary. We want to offer learners a "newest courses first" sort. The problem is how to -add that sort — and roll it out across the repositories that together own -enterprise search — without a partially-configured replica degrading or breaking -the existing relevance search. +add a *second* replica — and roll it out across the repositories that together +own enterprise search — without a partially-configured replica degrading or +breaking the existing relevance search. Decision -------- @@ -44,6 +42,11 @@ the far-future ``ALGOLIA_DEFAULT_TIMESTAMP``, which would float undated courses the top. The primary index (``ALGOLIA['INDEX_NAME']``) keeps the relevance ranking. +The replica is declared as an Algolia *virtual* replica (``virtual(name)``), so it +mirrors the primary's records instead of duplicating them. This is a deliberate +cost/precision tradeoff versus a standard replica — see *Alternatives considered* +and *Consequences*. + The sort is rolled out across three repositories: #. **enterprise-catalog** (this service) builds and configures the replica. @@ -112,6 +115,10 @@ Consequences intact, so a problem with one sort can never take down core search indexing. The worst case is that one replica lags its ranking until the next run — never a broken or empty primary index. +* **No added record cost:** because the replica is *virtual*, it mirrors the + primary's records rather than duplicating them, so adding it does not grow our + Algolia record count. A standard (non-virtual) replica would roughly double the + indexed record count — and its cost — for each sort we add (see *Alternatives*). * **Non-course records sort last, by design:** the replica is *virtual* over the primary index, so it mirrors every record — programs, executive education, videos, etc. — not just courses. ``recently_released_timestamp`` is only computed in the @@ -142,3 +149,15 @@ Alternatives considered * **Always declare the replica (no config gate).** Rejected: would create a ``virtual(None)`` replica on the primary index in environments where the name is unset, and would couple every environment to the rollout. +* **A standard (non-virtual) replica instead of a virtual one.** A standard + replica is a full, independent copy of the index that sorts strictly by its own + ``ranking``/``customRanking`` — a fully deterministic newest-first order + regardless of the search query. We chose a *virtual* replica instead: a virtual + replica reuses the primary's records, so it adds no record count or cost (see + *Consequences*), whereas a standard replica roughly doubles our indexed record + count and its associated cost. The accepted tradeoff is that a virtual replica + always keeps textual relevance as the top-priority sort factor, so under a text + query "newest first" is relevance-biased rather than strictly chronological (it + *is* strictly chronological when browsing with no query). If a strictly + deterministic order under query later proves necessary, switching this one + replica to a standard replica is the documented escape hatch. From 43c6165727fafd39f5f8c845682c63f5faf16d5f Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Tue, 16 Jun 2026 11:07:11 -0400 Subject: [PATCH 18/20] refactor: settings-driven Algolia replica registry (ADDITIONAL_REPLICA_INDEX_SETTINGS) Per iloveagent57's review, collapse the replica registry's indirection into a single settings-driven map: - settings/base.py: define ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS'], an index_name -> index settings map (config-as-code), and move the recency customRanking constant (ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS) here. - settings/production.py: add 'ALGOLIA' to DICT_UPDATE_KEYS so the deployment YAML is merged (not replaced) -- ops can override per-env index names/credentials while the code-defined replica settings are preserved. - Remove ALGOLIA_REPLICA_CONFIG_KEYS (constants.py) and ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY (algolia_utils.py); _get_algolia_replica_names, _configured_replicas, and replica_index_names now read settings.ALGOLIA directly (base REPLICA_INDEX_NAME + ADDITIONAL_REPLICA_INDEX_SETTINGS keys). No more cross-module registry, so the constants.py circular-import workaround is gone and the settings-map drift (a KeyError risk Copilot flagged) is structurally impossible. Behavior change: the recency replica now ships declared in code (always declared in every env once reindexed), rather than being inert until ops set an index name. The replica is virtual (no record cost), and user-facing exposure is still gated by the enterprise.search_default_sort_newest waffle flag + Optimizely experiment. Updates ADR 0014 and the how-to guide; rewrites the affected tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 62 ++++--- docs/how_to/add_an_algolia_sort_replica.rst | 167 ++++++++---------- enterprise_catalog/apps/api_client/algolia.py | 18 +- .../apps/api_client/constants.py | 13 -- .../apps/api_client/tests/test_algolia.py | 4 +- .../apps/catalog/algolia_utils.py | 72 ++------ .../apps/catalog/tests/test_algolia_utils.py | 56 +++--- enterprise_catalog/settings/base.py | 38 +++- enterprise_catalog/settings/production.py | 5 +- 9 files changed, 190 insertions(+), 245 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index e13f8ead..0356210a 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -20,9 +20,9 @@ provision *one* replica: there is a single setting Portal points its video search at) and a single Python function that provisions that one replica. -``ALGOLIA`` is also *replaced* (not merged) from the deployment YAML, so a replica -is only live once ops sets its index-name key in ``edx-internal`` and a -``reindex_algolia`` run declares it on the primary. +Historically the whole ``ALGOLIA`` dict has been *replaced* (not merged) from the +deployment YAML, so anything code wants to keep in it — like a replica's ranking +definition — would be lost unless ops restated it in ``edx-internal``. We want to offer learners a "newest courses first" sort. The problem is how to add a *second* replica — and roll it out across the repositories that together @@ -32,8 +32,9 @@ breaking the existing relevance search. Decision -------- -Add a recency-sorted replica (``ALGOLIA['RECENTLY_RELEASED_REPLICA_INDEX_NAME']``) -that leads its ranking with ``desc(recently_released_timestamp)`` — a per-course +Add a recency-sorted replica (the ``enterprise_catalog_recently_released_desc`` +entry in ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']``) that leads its ranking +with ``desc(recently_released_timestamp)`` — a per-course Unix timestamp of the *earliest course-run start of any status* (the Discovery course release date, the same signal as the ``is_new_content`` flag, via the shared ``_earliest_course_run_start`` helper — ENT-11386). Courses with no run @@ -56,21 +57,21 @@ The sort is rolled out across three repositories: the replica when the flag is on *and* the Optimizely "newest" experiment variant is active for the user. -The replica is **config-gated on both sides** and is never queried unless its -name is configured: - -* Backend: **all** sort replicas are driven by one registry - (``ALGOLIA_REPLICA_CONFIG_KEYS`` — the base duration replica first, then additive - sorts like recency). Each is declared on the primary index, configured, and - added to the secured-key ``restrictIndices`` **only** when its ``settings.ALGOLIA`` - index-name key is set; an unconfigured replica is skipped entirely (no - ``virtual(None)`` replica is created), so the code is inert until ops provides a - name. This is the single source of truth for which replicas exist — adding a - future sort is a registry entry plus the field its ``customRanking`` sorts on, - not a change to the gating logic. (The client's ``init_index`` / - ``index_exists`` still eagerly manage the primary + base-replica *handles* — that - is the required-core-pair lifecycle, a separate concern from which replicas get - declared/configured.) +The design generalizes the old one-replica assumption to a settings-driven map, and +gates *user exposure* with the waffle flag rather than gating *declaration* on ops: + +* Backend: additional sort replicas are declared in + ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` — an ``index_name -> index settings`` + map defined in ``settings/base.py`` as config-as-code (the ``customRanking`` is code, + since it sorts on a field the indexer computes). ``ALGOLIA`` is added to + ``DICT_UPDATE_KEYS`` so the deployment YAML is now *merged*, not replaced: ops can + override per-environment index names and credentials while these code-defined replica + settings are preserved. One map is the single source of truth — its entries are + declared on the primary index, configured during a reindex, and added to the secured-key + ``restrictIndices`` — so adding a future sort is one new entry plus the field its + ``customRanking`` sorts on. (The base ``REPLICA_INDEX_NAME`` keeps its own + per-environment key and its required-core-pair lifecycle in ``init_index`` / + ``index_exists``.) * Backend (fail-safe): configuring each replica in ``configure_algolia_index`` is wrapped so that any ``AlgoliaException`` is logged and skipped. One replica failing to configure never aborts the reindex — the primary (relevance) index @@ -101,15 +102,18 @@ search time and silently falls back to the primary index. Consequences ------------ -* **Covered:** "replica name not configured" → the base (relevance) index is - used. This is handled explicitly in both the backend (conditional replica - declaration) and the MFE (the ``&& recentlyReleasedIndexName`` guard), so the - default-to-base behavior is guaranteed for the unconfigured case. -* **Not covered in code:** "replica name configured but the Algolia index does - not exist yet" → the MFE would query a missing index and surface an error / - empty results rather than falling back. This is a transient, - operator-controlled window mitigated by the rollout order and the kill-switch - flag, not by code. +* **Exposure is MFE/flag-gated, not declaration-gated:** the backend now declares the + replica in every environment (it ships in ``ADDITIONAL_REPLICA_INDEX_SETTINGS``), so + "is it declared" is no longer the gate. The course search falls back to the primary + (relevance) index whenever the MFE's replica env var is unset or the flag/experiment is + off (the ``&& recentlyReleasedIndexName`` guard), so user exposure is controlled by the + MFE env var + waffle flag. Because the replica is *virtual* (no extra records), always + declaring it costs nothing. +* **Not covered in code:** "the replica is pointed at by the MFE but its Algolia index + does not exist yet" (e.g. the MFE env var is set before this service has been deployed + and reindexed) → the MFE would query a missing index and surface an error / empty + results rather than falling back. This is a transient, operator-controlled window + mitigated by the rollout order and the kill-switch flag, not by code. * **Reindex degrades gracefully:** if configuring a replica fails, the error is logged and the reindex continues with the primary index and the other replicas intact, so a problem with one sort can never take down core search indexing. diff --git a/docs/how_to/add_an_algolia_sort_replica.rst b/docs/how_to/add_an_algolia_sort_replica.rst index 8114af80..70ec05f8 100644 --- a/docs/how_to/add_an_algolia_sort_replica.rst +++ b/docs/how_to/add_an_algolia_sort_replica.rst @@ -6,86 +6,58 @@ not re-sort an index at query time, so every alternate sort order is a separate index with its own ``customRanking``; the consumer (the MFE) switches sort by pointing its search at a different index name. -All replicas are driven by **one registry**, so adding a sort is mostly additive. This guide -walks through it end to end. See ``docs/decisions/0014-newest-courses-sort-replica.rst`` for -the design rationale, and treat the **recently-released ("newest first") replica** as the -canonical example to copy. - -The mental model: names vs. definitions ---------------------------------------- - -There are two layers, and the split matters: - -* **Index *names* live in Django settings** (``settings.ALGOLIA``), populated per-environment - from the deployment config (edx-internal). These vary by environment and are owned by ops. -* **Replica *definitions* live in code** — which replicas exist (the registry) and how each is - ranked (``customRanking``). A replica's ranking sorts on a field that the indexing code must - compute, so the definition is intrinsically code, not config. - -A replica is declared, configured, and made queryable **only when its index-name key holds a -non-empty value**. With the name unset (the default everywhere until ops sets it), the new -replica is completely inert — nothing is declared, no ``virtual(None)`` is created, and the -secured API key does not reference it. This is what makes it safe to merge and deploy the code -*before* ops turns it on. +Beyond the base replica (``ALGOLIA['REPLICA_INDEX_NAME']``, used by the MFE video search), +every additional sort replica is declared in **one place** -- the +``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` map -- so adding a sort is mostly additive. +This guide walks through it end to end. See +``docs/decisions/0014-newest-courses-sort-replica.rst`` for the design rationale, and treat the +**recently-released ("newest first") replica** as the canonical example to copy. + +The mental model: one settings-driven map +------------------------------------------ + +``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` maps an **index name** to that replica's +**Algolia index settings** (its ``customRanking``). It is defined in ``settings/base.py`` as +config-as-code: a replica's ranking sorts on a field the indexing code must compute, so the +definition is intrinsically code, not deployment config. + +``ALGOLIA`` is *merged* (not replaced) from the deployment config -- it is listed in +``DICT_UPDATE_KEYS`` (``settings/production.py``) -- so an environment can override the per-env +index names / credentials while the code-defined ``ADDITIONAL_REPLICA_INDEX_SETTINGS`` defaults are +preserved. + +The backend declares and configures every replica in this map on each reindex, and the secured API +key grants access to them. A sort is only *user-visible* once the MFE points a search at it, gated +by a waffle flag / Optimizely experiment -- so the flag, not the map, controls exposure (see +*Frontend* below and ADR 0014). Worked example -------------- -Suppose we want a **"price: low to high"** sort. We'll use the settings key -``PRICE_ASC_REPLICA_INDEX_NAME`` and (in a given environment) an index named +Suppose we want a **"price: low to high"** sort, using an index named ``enterprise_catalog_price_asc``. Backend steps (this service) ---------------------------- -**1. Declare the settings key.** In ``enterprise_catalog/settings/base.py``, add the key to the -``ALGOLIA`` dict with an empty default and a one-line comment describing the sort: - -.. code-block:: python - - ALGOLIA = { - 'INDEX_NAME': '', - 'REPLICA_INDEX_NAME': '', # base replica, desc(duration); MFE video search - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': '', # "newest first", desc(recently_released_timestamp) - # "price: low to high", asc(first_enrollable_paid_seat_price) - 'PRICE_ASC_REPLICA_INDEX_NAME': '', - 'APPLICATION_ID': '', - 'API_KEY': '', - } - -The empty default keeps it inert until ops provides a real index name. - -**2. Register the key.** Add it to ``ALGOLIA_REPLICA_CONFIG_KEYS`` in -``enterprise_catalog/apps/api_client/constants.py`` — the single source of truth for *which* -replicas exist, shared by the indexer and the secured-key restriction. (It lives here, not in -``settings``/``algolia_utils``, so both ``api_client.algolia`` and ``catalog.algolia_utils`` can -import it without a circular dependency.) - -.. code-block:: python - - ALGOLIA_REPLICA_CONFIG_KEYS = ( - 'REPLICA_INDEX_NAME', - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME', - 'PRICE_ASC_REPLICA_INDEX_NAME', - ) - -**3. Make sure the field you sort on is indexed.** A ``customRanking`` can only sort on a numeric +**1. Make sure the field you sort on is indexed.** A ``customRanking`` can only sort on a numeric attribute that exists on the records. If your sort uses a field that is *already* indexed (the price example reuses ``first_enrollable_paid_seat_price``), skip this step. If it needs a new signal, in ``enterprise_catalog/apps/catalog/algolia_utils.py``: * write a ``get_course_(course)`` helper that returns the numeric value (see - ``get_course_recently_released_timestamp`` — note it returns ``0`` for "no value" so those - records sort last under a ``desc`` ranking; pick a sentinel that sorts your missing values to - the *end* of *your* order); + ``get_course_recently_released_timestamp`` -- note it returns ``0`` for "no value" so those + records sort last under a ``desc`` ranking; pick a sentinel that sorts your missing values to the + *end* of *your* order); * add the field name to ``ALGOLIA_FIELDS``; * set it on the course object in ``_algolia_object_from_product`` (the ``content_type == COURSE`` branch). -**4. Define the replica's ranking.** Still in ``algolia_utils.py``, add a settings dict. Lead with -your sort criterion, then append the primary index's shared tie-breakers so records that tie on -your criterion (and any "missing value" bucket) fall back to the relevance ordering and -pagination stays deterministic: +**2. Define the replica's ranking and register it.** In ``enterprise_catalog/settings/base.py``, +add a settings constant and an entry in ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` keyed by +the index name. Lead the ``customRanking`` with your sort criterion, then append the primary +index's shared tie-breakers so records that tie on your criterion (and any "missing value" bucket) +fall back to the relevance ordering and pagination stays deterministic: .. code-block:: python @@ -101,33 +73,33 @@ pagination stays deterministic: ], } -**5. Map the key to its settings.** Add an entry to -``ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY`` in ``algolia_utils.py``: - -.. code-block:: python - - ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { - 'REPLICA_INDEX_NAME': ALGOLIA_REPLICA_INDEX_SETTINGS, - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, - 'PRICE_ASC_REPLICA_INDEX_NAME': ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS, + ALGOLIA = { + 'INDEX_NAME': '', + 'REPLICA_INDEX_NAME': '', + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'enterprise_catalog_recently_released_desc': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, + 'enterprise_catalog_price_asc': ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS, + }, + 'APPLICATION_ID': '', + 'API_KEY': '', } **That is all the wiring.** You do **not** touch ``_get_algolia_replica_names``, -``_configured_replicas``, ``configure_algolia_index``, or the secured-key -``replica_index_names`` — they all loop the registry, so the new replica is automatically -declared on the primary index, has its settings applied during a reindex, and is added to the -secured API key's ``restrictIndices`` once its name is configured. +``_configured_replicas``, ``configure_algolia_index``, or the secured-key ``replica_index_names`` +-- they all read ``ADDITIONAL_REPLICA_INDEX_SETTINGS``, so the new replica is automatically declared +on the primary index, has its settings applied during a reindex, and is added to the secured API +key's ``restrictIndices``. Tests ----- -* Extend the registry / configure tests in +* Extend the configure / registry tests in ``enterprise_catalog/apps/catalog/tests/test_algolia_utils.py`` (e.g. - ``test_get_algolia_replica_names_only_includes_configured_replicas`` and - ``test_configure_algolia_index_configures_*``) to cover the new key, using + ``test_get_algolia_replica_names_combines_base_and_additional_replicas`` and + ``test_configure_algolia_index_configures_additional_replica``) to cover the new replica, using ``override_settings(ALGOLIA={...})``. * If you added a field computation, unit-test the ``get_course_`` helper. -* The secured-key tests in ``api_client/tests/test_algolia.py`` already loop the registry; add +* The secured-key tests in ``api_client/tests/test_algolia.py`` exercise ``restrictIndices``; add the new index name to the "all indices" expectation if you want explicit coverage. Deploy / ops @@ -135,12 +107,13 @@ Deploy / ops Once the code is merged and deployed: -#. Ops sets ``PRICE_ASC_REPLICA_INDEX_NAME`` (to e.g. ``enterprise_catalog_price_asc``) in the - ``ALGOLIA`` config in edx-internal. **The whole** ``ALGOLIA`` **dict is replaced, not merged**, - so the entire dict must be restated with the new key included. -#. Run ``./manage.py reindex_algolia``. A *virtual* replica exists as soon as it is declared on - the primary index's settings (it mirrors the primary's records), so the replica is live after - one reindex — no separate population step. +#. The replica's name and settings ship in code (``ADDITIONAL_REPLICA_INDEX_SETTINGS``), so no + edx-internal change is required to *declare* it. If an environment needs a different index name, + ops overrides ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` in edx-internal -- because + ``ALGOLIA`` is merged, only the keys ops sets are overridden. +#. Run ``./manage.py reindex_algolia``. A *virtual* replica exists as soon as it is declared on the + primary index's settings (it mirrors the primary's records), so the replica is live after one + reindex -- no separate population step. Frontend (only if the MFE will use the sort) -------------------------------------------- @@ -149,19 +122,19 @@ The backend builds the replica regardless; a sort is only *user-visible* once th search at it. In ``frontend-app-learner-portal-enterprise``: * add an ``ALGOLIA__REPLICA_INDEX_NAME`` env var in ``src/index.tsx`` and - ``src/types/types.d.ts`` (mirror of the backend settings key); -* point the relevant ```` at it (see ``SearchVideo.jsx``, which uses the - base replica, or ``SearchCourse.jsx`` for the recency replica); -* if the sort changes default behavior, gate it behind a waffle flag and/or an Optimizely - experiment, exactly as the recency sort does (the flag doubles as a kill-switch — see ADR 0014). + ``src/types/types.d.ts`` whose value matches the backend index name; +* point the relevant ```` at it (see ``SearchVideo.jsx``, which uses the base + replica, or ``SearchCourse.jsx`` for the recency replica); +* gate it behind a waffle flag and/or an Optimizely experiment, exactly as the recency sort does + (the flag doubles as a kill-switch -- see ADR 0014). -Safety properties you get for free ----------------------------------- +Safety properties +----------------- -* **Inert by default.** No configured name → the replica is never declared, configured, or - queryable. Merge and deploy ahead of ops with no effect. +* **Virtual replicas, no extra records.** Each replica is declared ``virtual(name)``, so it mirrors + the primary's records rather than duplicating them -- no added Algolia record count or cost. * **Fail-safe configuration.** ``configure_algolia_index`` wraps each replica's settings call so an - ``AlgoliaException`` is logged and skipped — one replica failing to configure never aborts the + ``AlgoliaException`` is logged and skipped -- one replica failing to configure never aborts the reindex, and the primary index plus the other replicas stay configured. -* **No** ``virtual(None)``. Unconfigured keys are skipped entirely, never declared as a broken - ``virtual(None)`` replica on the primary index. +* **Flag-gated exposure.** Declaring a replica does not make it user-visible; the MFE only queries + it when its waffle flag / experiment is on, so the flag is the kill-switch. diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index 56b655ae..c97955cb 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -10,9 +10,6 @@ from django.conf import settings from django.core.exceptions import ImproperlyConfigured -from enterprise_catalog.apps.api_client.constants import ( - ALGOLIA_REPLICA_CONFIG_KEYS, -) from enterprise_catalog.apps.catalog.utils import batch, localized_utcnow @@ -52,16 +49,17 @@ def algolia_replica_index_name(self): @property def replica_index_names(self): """ - Configured index names of every sort replica (base + additive), empty when none are set. + Index names of every configured sort replica (base + additional), empty when none are set. - Driven by ``ALGOLIA_REPLICA_CONFIG_KEYS``; an unconfigured replica is omitted, so the - secured API key only grants access to replicas that actually exist. + The base replica is ``ALGOLIA['REPLICA_INDEX_NAME']`` (when set); the additional sort + replicas are the keys of ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']``. Used to scope the + secured API key to exactly the replicas that exist. """ names = [] - for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: - index_name = settings.ALGOLIA.get(config_key) - if index_name: - names.append(index_name) + base_replica = settings.ALGOLIA.get('REPLICA_INDEX_NAME') + if base_replica: + names.append(base_replica) + names.extend(settings.ALGOLIA.get('ADDITIONAL_REPLICA_INDEX_SETTINGS', {}).keys()) return names def init_index(self): diff --git a/enterprise_catalog/apps/api_client/constants.py b/enterprise_catalog/apps/api_client/constants.py index 03f16162..bc53bf19 100644 --- a/enterprise_catalog/apps/api_client/constants.py +++ b/enterprise_catalog/apps/api_client/constants.py @@ -18,19 +18,6 @@ DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_KEY = 'average_course_review' DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_TTL = 60 * 120 # 2 hours -# Algolia API Client Constants -# settings.ALGOLIA keys that name each sort replica index, in declaration order. The primary -# index keeps the relevance ranking; every replica -- the base "duration" replica and any -# additive sort such as "newest first" -- is declared, configured, and made queryable ONLY when -# its key holds a non-empty index name, so listing a key here is inert until ops sets the name. -# Adding a new sort = add its key here, map it to index settings in ``algolia_utils``, and -# compute the field its ``customRanking`` sorts on. This is the single source of truth for -# *which* replicas exist, shared by the indexer and the secured-API-key restriction. -ALGOLIA_REPLICA_CONFIG_KEYS = ( - 'REPLICA_INDEX_NAME', # base replica, desc(duration); MFE video search uses it - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME', # "newest courses first", desc(recently_released_timestamp) -) - COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER = 15 # As of 1/26/24 this is calculated from Snowflake: diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index 1263f747..78cec60c 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -157,7 +157,9 @@ def test_set_index_settings_failure_logs_effective_index_name(self): @override_settings(ALGOLIA={ 'INDEX_NAME': 'enterprise_catalog', 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_released_desc', + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'enterprise_catalog_recently_released_desc': {'customRanking': ['desc(recently_released_timestamp)']}, + }, 'SEARCH_API_KEY': 'fake-search-key', }) @mock.patch('enterprise_catalog.apps.api_client.algolia.SearchClient.generate_secured_api_key') diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index f7b11f2b..deaacc54 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -18,7 +18,6 @@ from enterprise_catalog.apps.api_client.algolia import AlgoliaSearchClient from enterprise_catalog.apps.api_client.constants import ( - ALGOLIA_REPLICA_CONFIG_KEYS, COURSE_REVIEW_BASE_AVG_REVIEW_SCORE, COURSE_REVIEW_BAYESIAN_CONFIDENCE_NUMBER, DISCOVERY_AVERAGE_COURSE_REVIEW_CACHE_KEY, @@ -73,16 +72,17 @@ def _get_algolia_replica_names() -> list[str]: """ Build the list of replica index names to declare on the primary index. - Returns a ``virtual(name)`` entry for each replica in the registry - (``ALGOLIA_REPLICA_CONFIG_KEYS`` -- the base duration replica plus any additive sort replica) - whose index name is configured. Unconfigured replicas are omitted, so deploying this code - before ops sets a name won't declare a ``virtual(None)`` replica on the primary index. + Returns a ``virtual(name)`` entry for the base replica (``ALGOLIA['REPLICA_INDEX_NAME']``, when + set) and for each additional sort replica (the keys of + ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']``). An unset base replica is omitted, so we never + declare a ``virtual(None)`` replica on the primary index. """ replica_names = [] - for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: - index_name = settings.ALGOLIA.get(config_key) - if index_name: - replica_names.append(f'virtual({index_name})') + base_replica = settings.ALGOLIA.get('REPLICA_INDEX_NAME') + if base_replica: + replica_names.append(f'virtual({base_replica})') + for index_name in settings.ALGOLIA.get('ADDITIONAL_REPLICA_INDEX_SETTINGS', {}): + replica_names.append(f'virtual({index_name})') return replica_names @@ -238,55 +238,21 @@ def _get_algolia_replica_names() -> list[str]: ], } -# Replica that sorts courses by recency (newest-first). Leads with the precomputed -# ``recently_released_timestamp`` (the course's earliest course-run start, any status) descending, -# then falls back to the primary index's tie-breakers. Surfaced in the Learner Portal search -# page as the "newest courses first" sort. -ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS = { - 'customRanking': [ - 'desc(recently_released_timestamp)', - 'asc(metadata_language)', - 'asc(visible_via_association)', - 'asc(created)', - 'desc(course_bayesian_average)', - 'desc(recent_enrollment_count)', - ], -} - -# Maps each replica config key (see ``ALGOLIA_REPLICA_CONFIG_KEYS``) to the index settings -# applied to that replica. Adding a new sort replica means adding its key to the shared tuple -# and its ``customRanking`` settings here (and computing the field its ``customRanking`` sorts on). -ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY = { - 'REPLICA_INDEX_NAME': ALGOLIA_REPLICA_INDEX_SETTINGS, - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, -} - - def _configured_replicas(): """ - Return ``(index_name, index_settings)`` for each replica whose index name is configured. + Return ``(index_name, index_settings)`` for each replica to configure. - Covers every replica in the registry -- the base duration replica and any additive sort - replica (``ALGOLIA_REPLICA_CONFIG_KEYS``). An unconfigured replica is skipped, so this is - inert until its name is provided in ``settings.ALGOLIA``. A configured replica whose key has - no settings mapped (registry drift -- a key added to ``ALGOLIA_REPLICA_CONFIG_KEYS`` without a - matching entry in ``ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY``) is likewise skipped, with - an error logged, rather than raising a ``KeyError`` that would abort the whole reindex. + Covers the base replica (``ALGOLIA['REPLICA_INDEX_NAME']`` with ``ALGOLIA_REPLICA_INDEX_SETTINGS``, + when set) and every additional sort replica declared in + ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` (an ``index_name -> settings`` map). Because each + additional replica carries its own settings, there is no separate settings lookup that can drift + out of sync; this is inert until at least one replica is configured. """ configured = [] - for config_key in ALGOLIA_REPLICA_CONFIG_KEYS: - index_name = settings.ALGOLIA.get(config_key) - if not index_name: - continue - index_settings = ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY.get(config_key) - if index_settings is None: - LOGGER.error( - 'Algolia replica config key "%s" is configured (index "%s") but has no settings ' - 'mapped in ALGOLIA_REPLICA_INDEX_SETTINGS_BY_CONFIG_KEY; skipping it.', - config_key, - index_name, - ) - continue + base_replica = settings.ALGOLIA.get('REPLICA_INDEX_NAME') + if base_replica: + configured.append((base_replica, ALGOLIA_REPLICA_INDEX_SETTINGS)) + for index_name, index_settings in settings.ALGOLIA.get('ADDITIONAL_REPLICA_INDEX_SETTINGS', {}).items(): configured.append((index_name, index_settings)) return configured diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index b46398a6..58571b23 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -284,12 +284,14 @@ def test_get_course_recently_released_timestamp(self): {'status': 'published', 'start': recent.isoformat()}, ]}) == int(old.timestamp()) - def test_get_algolia_replica_names_only_includes_configured_replicas(self): - """Each replica (base + optional) is declared only when its index name is configured.""" + def test_get_algolia_replica_names_combines_base_and_additional_replicas(self): + """The base replica and every additional sort replica are declared as virtual replicas.""" # pylint: disable=protected-access with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': 'enterprise_catalog_recently_released_desc', + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'enterprise_catalog_recently_released_desc': {'customRanking': []}, + }, }): assert utils._get_algolia_replica_names() == [ 'virtual(enterprise_catalog_duration_desc)', @@ -298,7 +300,13 @@ def test_get_algolia_replica_names_only_includes_configured_replicas(self): # Only the base replica configured -> only it is declared. with override_settings(ALGOLIA={'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc'}): assert utils._get_algolia_replica_names() == ['virtual(enterprise_catalog_duration_desc)'] - # Nothing configured -> no replicas at all, never virtual(None). + # An unset base replica is omitted (never virtual(None)); additional replicas still declare. + with override_settings(ALGOLIA={ + 'REPLICA_INDEX_NAME': '', + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': {'enterprise_catalog_recently_released_desc': {}}, + }): + assert utils._get_algolia_replica_names() == ['virtual(enterprise_catalog_recently_released_desc)'] + # Nothing configured -> no replicas at all. with override_settings(ALGOLIA={}): assert not utils._get_algolia_replica_names() @@ -1075,16 +1083,19 @@ def test_configure_algolia_index(self, mock_search_client): assert set_index_settings.call_count == 2 @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') - def test_configure_algolia_index_configures_recently_released_replica(self, mock_search_client): + def test_configure_algolia_index_configures_additional_replica(self, mock_search_client): """ - When a recently-released replica name is configured, its settings are applied too. + Each entry in ADDITIONAL_REPLICA_INDEX_SETTINGS has its settings applied, keyed by index name. """ algolia_client = utils.get_initialized_algolia_client() replica_name = 'enterprise_catalog_recently_released_desc' - with override_settings(ALGOLIA={'RECENTLY_RELEASED_REPLICA_INDEX_NAME': replica_name}): + recency_settings = {'customRanking': ['desc(recently_released_timestamp)']} + with override_settings(ALGOLIA={ + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': {replica_name: recency_settings}, + }): utils.configure_algolia_index(algolia_client) mock_search_client.return_value.set_index_settings.assert_any_call( - utils.ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, + recency_settings, index_name=replica_name, ) @@ -1097,6 +1108,7 @@ def test_configure_algolia_index_replica_failure_is_safe(self, mock_search_clien algolia_client = utils.get_initialized_algolia_client() base_name = 'enterprise_catalog_duration_desc' recency_name = 'enterprise_catalog_recently_released_desc' + recency_settings = {'customRanking': ['desc(recently_released_timestamp)']} def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=unused-argument # Primary and base-replica calls succeed; only the recency replica raises. @@ -1106,7 +1118,7 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u mock_search_client.return_value.set_index_settings.side_effect = fail_only_for_recency with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': base_name, - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': recency_name, + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': {recency_name: recency_settings}, }): # The per-replica handler logs a single-line WARNING (set_index_settings already logged # the traceback before re-raising), so we don't double up stack traces. @@ -1122,33 +1134,9 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u }) set_index_settings.assert_any_call(utils.ALGOLIA_REPLICA_INDEX_SETTINGS, index_name=base_name) # The recency replica was attempted (and failed) -- logged, not swallowed silently. - set_index_settings.assert_any_call( - utils.ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, - index_name=recency_name, - ) + set_index_settings.assert_any_call(recency_settings, index_name=recency_name) assert any(recency_name in message for message in warning_logs.output) - def test_configured_replicas_skips_config_key_without_settings_mapping(self): - """ - A configured replica whose key has no settings mapped (registry drift) is skipped with an - error logged -- never a KeyError that would abort the reindex. The well-formed replicas - are still returned. - """ - # pylint: disable=protected-access - bogus_key = 'BOGUS_REPLICA_INDEX_NAME' # in the registry tuple, but not in the settings map - registry = utils.ALGOLIA_REPLICA_CONFIG_KEYS + (bogus_key,) - with mock.patch.object(utils, 'ALGOLIA_REPLICA_CONFIG_KEYS', registry): - with override_settings(ALGOLIA={ - 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - bogus_key: 'enterprise_catalog_bogus_desc', - }): - with self.assertLogs(utils.LOGGER, level='ERROR') as error_logs: - configured = utils._configured_replicas() - # The well-formed base replica is returned; the unmapped key is skipped (not raised on). - assert ('enterprise_catalog_duration_desc', utils.ALGOLIA_REPLICA_INDEX_SETTINGS) in configured - assert all(index_name != 'enterprise_catalog_bogus_desc' for index_name, _ in configured) - assert any(bogus_key in message for message in error_logs.output) - @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') def test_configure_algolia_index_raises_when_primary_uninitialized(self, mock_search_client): """ diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index 70bd2911..dec904e0 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -432,18 +432,42 @@ STUDIO_BASE_URL = os.environ.get('STUDIO_BASE_URL', '') # Algolia -# Index-name keys are populated per-environment from the deployment config (edx-internal); the -# whole ALGOLIA dict is *replaced* there, not merged. Each replica is declared/configured only -# when its key holds a non-empty name (see ALGOLIA_REPLICA_CONFIG_KEYS). +# The per-environment index names and credentials (INDEX_NAME, REPLICA_INDEX_NAME, APPLICATION_ID, +# API_KEY) are populated from the deployment config (edx-internal). ALGOLIA is listed in +# DICT_UPDATE_KEYS (see settings/production.py), so the deployment YAML is *merged* into this dict +# rather than replacing it -- per-environment keys override the defaults below while code-defined +# defaults like ADDITIONAL_REPLICA_INDEX_SETTINGS are preserved. + +# customRanking for the "newest courses first" sort replica. Defined here as config-as-code and +# referenced from ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS'] below. Leads with the precomputed +# recently_released_timestamp (a course's earliest course-run start, any status) descending, then +# the primary index's relevance tie-breakers. +ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS = { + 'customRanking': [ + 'desc(recently_released_timestamp)', + 'asc(metadata_language)', + 'asc(visible_via_association)', + 'asc(created)', + 'desc(course_bayesian_average)', + 'desc(recent_enrollment_count)', + ], +} + ALGOLIA = { 'INDEX_NAME': '', # Base replica: the long-standing sort the learner-portal MFE points its video search # (SEARCH_INDEX_IDS.VIDEOS) at -- the MFE's ALGOLIA_REPLICA_INDEX_NAME env var. Its - # customRanking lives in ALGOLIA_REPLICA_INDEX_SETTINGS. + # customRanking lives in ALGOLIA_REPLICA_INDEX_SETTINGS (apps/catalog/algolia_utils.py). 'REPLICA_INDEX_NAME': '', - # "Newest courses first" replica, surfaced as a sort option on the learner-portal course - # search. Its customRanking lives in ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS. - 'RECENTLY_RELEASED_REPLICA_INDEX_NAME': '', + # Additional sort replicas beyond the base one, mapped index_name -> Algolia index settings. + # This is the single source of truth for which extra replicas exist: each entry is declared on + # the primary index, has its settings applied during a reindex, and is added to the secured API + # key. The learner-portal only points its course at the "newest first" replica when the + # enterprise.search_default_sort_newest waffle flag + Optimizely experiment are on, so the flag + # -- not the mere presence of an entry here -- gates user-facing exposure. + 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'enterprise_catalog_recently_released_desc': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, + }, 'APPLICATION_ID': '', 'API_KEY': '', } diff --git a/enterprise_catalog/settings/production.py b/enterprise_catalog/settings/production.py index d5067751..45e095bf 100644 --- a/enterprise_catalog/settings/production.py +++ b/enterprise_catalog/settings/production.py @@ -21,7 +21,10 @@ # Keep track of the names of settings that represent dicts. Instead of overriding the values in base.py, # the values read from disk should UPDATE the pre-configured dicts. -DICT_UPDATE_KEYS = ('JWT_AUTH', 'REST_FRAMEWORK') +# ALGOLIA is merged (not replaced) so the deployment YAML can set per-environment index names and +# credentials while code-defined defaults such as ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS'] +# (the sort-replica customRanking definitions) are preserved. +DICT_UPDATE_KEYS = ('JWT_AUTH', 'REST_FRAMEWORK', 'ALGOLIA') # This may be overridden by the YAML in catalog_CFG, # but it should be here as a default. From 004ee0679c17027686324aa11fb1293851cc0cff Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Thu, 18 Jun 2026 10:29:04 -0400 Subject: [PATCH 19/20] style: fix E302 (two blank lines before _configured_replicas) The settings-driven refactor removed the block that sat above _configured_replicas, leaving a single blank line before the def. Restore the second blank line so pycodestyle (make style) passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- enterprise_catalog/apps/catalog/algolia_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index deaacc54..3b037913 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -238,6 +238,7 @@ def _get_algolia_replica_names() -> list[str]: ], } + def _configured_replicas(): """ Return ``(index_name, index_settings)`` for each replica to configure. From 9303a2239577cc43375a3cd5a041e2d57b8ede1c Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Thu, 18 Jun 2026 13:31:30 -0400 Subject: [PATCH 20/20] refactor: address PR #118 review feedback (VIRTUAL rename, ADR + doc fixes) Addresses outstanding review comments from pwnage101 and Copilot: - Rename ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS'] -> ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS'] (the replicas are declared as virtual()) across settings, client, utils, tests, and docs. - configure_algolia_index: drop the now-incorrect "ALGOLIA is replaced (not merged)" justification -- ALGOLIA is merged via DICT_UPDATE_KEYS. - how-to: clarify the ALGOLIA merge is shallow, so overriding the replica map replaces it wholesale rather than per-key. - settings/base.py: describe the base replica accurately as a "by duration" sort (its customRanking leads with desc(duration)), not "relevance". - ADR 0014: status Proposed -> Accepted (matches 0012/0013); drop two Consequences bullets that were implementation details, not consequences. - incremental_reindex_algolia: add TODO (ENT-11982) flagging that the command still configures only the base replica and re-plumbs client internals as a temporary workaround. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../0014-newest-courses-sort-replica.rst | 18 ++++---------- docs/how_to/add_an_algolia_sort_replica.rst | 24 +++++++++++-------- enterprise_catalog/apps/api_client/algolia.py | 4 ++-- .../apps/api_client/tests/test_algolia.py | 2 +- .../apps/catalog/algolia_utils.py | 10 ++++---- .../apps/catalog/tests/test_algolia_utils.py | 10 ++++---- .../commands/incremental_reindex_algolia.py | 12 ++++++++++ enterprise_catalog/settings/base.py | 13 +++++----- enterprise_catalog/settings/production.py | 2 +- 9 files changed, 51 insertions(+), 44 deletions(-) diff --git a/docs/decisions/0014-newest-courses-sort-replica.rst b/docs/decisions/0014-newest-courses-sort-replica.rst index 0356210a..b41ad885 100644 --- a/docs/decisions/0014-newest-courses-sort-replica.rst +++ b/docs/decisions/0014-newest-courses-sort-replica.rst @@ -4,7 +4,7 @@ Newest-Courses-First Search Sort via a Recency-Sorted Algolia Replica Status ------ -Proposed +Accepted Context ------- @@ -33,7 +33,7 @@ Decision -------- Add a recency-sorted replica (the ``enterprise_catalog_recently_released_desc`` -entry in ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']``) that leads its ranking +entry in ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']``) that leads its ranking with ``desc(recently_released_timestamp)`` — a per-course Unix timestamp of the *earliest course-run start of any status* (the Discovery course release date, the same signal as the ``is_new_content`` flag, via the @@ -61,7 +61,7 @@ The design generalizes the old one-replica assumption to a settings-driven map, gates *user exposure* with the waffle flag rather than gating *declaration* on ops: * Backend: additional sort replicas are declared in - ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` — an ``index_name -> index settings`` + ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']`` — an ``index_name -> index settings`` map defined in ``settings/base.py`` as config-as-code (the ``customRanking`` is code, since it sorts on a field the indexer computes). ``ALGOLIA`` is added to ``DICT_UPDATE_KEYS`` so the deployment YAML is now *merged*, not replaced: ops can @@ -103,22 +103,12 @@ Consequences ------------ * **Exposure is MFE/flag-gated, not declaration-gated:** the backend now declares the - replica in every environment (it ships in ``ADDITIONAL_REPLICA_INDEX_SETTINGS``), so + replica in every environment (it ships in ``ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS``), so "is it declared" is no longer the gate. The course search falls back to the primary (relevance) index whenever the MFE's replica env var is unset or the flag/experiment is off (the ``&& recentlyReleasedIndexName`` guard), so user exposure is controlled by the MFE env var + waffle flag. Because the replica is *virtual* (no extra records), always declaring it costs nothing. -* **Not covered in code:** "the replica is pointed at by the MFE but its Algolia index - does not exist yet" (e.g. the MFE env var is set before this service has been deployed - and reindexed) → the MFE would query a missing index and surface an error / empty - results rather than falling back. This is a transient, operator-controlled window - mitigated by the rollout order and the kill-switch flag, not by code. -* **Reindex degrades gracefully:** if configuring a replica fails, the error is - logged and the reindex continues with the primary index and the other replicas - intact, so a problem with one sort can never take down core search indexing. - The worst case is that one replica lags its ranking until the next run — never a - broken or empty primary index. * **No added record cost:** because the replica is *virtual*, it mirrors the primary's records rather than duplicating them, so adding it does not grow our Algolia record count. A standard (non-virtual) replica would roughly double the diff --git a/docs/how_to/add_an_algolia_sort_replica.rst b/docs/how_to/add_an_algolia_sort_replica.rst index 70ec05f8..7ec5165f 100644 --- a/docs/how_to/add_an_algolia_sort_replica.rst +++ b/docs/how_to/add_an_algolia_sort_replica.rst @@ -8,7 +8,7 @@ search at a different index name. Beyond the base replica (``ALGOLIA['REPLICA_INDEX_NAME']``, used by the MFE video search), every additional sort replica is declared in **one place** -- the -``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` map -- so adding a sort is mostly additive. +``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']`` map -- so adding a sort is mostly additive. This guide walks through it end to end. See ``docs/decisions/0014-newest-courses-sort-replica.rst`` for the design rationale, and treat the **recently-released ("newest first") replica** as the canonical example to copy. @@ -16,14 +16,14 @@ This guide walks through it end to end. See The mental model: one settings-driven map ------------------------------------------ -``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` maps an **index name** to that replica's +``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']`` maps an **index name** to that replica's **Algolia index settings** (its ``customRanking``). It is defined in ``settings/base.py`` as config-as-code: a replica's ranking sorts on a field the indexing code must compute, so the definition is intrinsically code, not deployment config. ``ALGOLIA`` is *merged* (not replaced) from the deployment config -- it is listed in ``DICT_UPDATE_KEYS`` (``settings/production.py``) -- so an environment can override the per-env -index names / credentials while the code-defined ``ADDITIONAL_REPLICA_INDEX_SETTINGS`` defaults are +index names / credentials while the code-defined ``ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS`` defaults are preserved. The backend declares and configures every replica in this map on each reindex, and the secured API @@ -54,7 +54,7 @@ signal, in ``enterprise_catalog/apps/catalog/algolia_utils.py``: branch). **2. Define the replica's ranking and register it.** In ``enterprise_catalog/settings/base.py``, -add a settings constant and an entry in ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` keyed by +add a settings constant and an entry in ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']`` keyed by the index name. Lead the ``customRanking`` with your sort criterion, then append the primary index's shared tie-breakers so records that tie on your criterion (and any "missing value" bucket) fall back to the relevance ordering and pagination stays deterministic: @@ -76,7 +76,7 @@ fall back to the relevance ordering and pagination stays deterministic: ALGOLIA = { 'INDEX_NAME': '', 'REPLICA_INDEX_NAME': '', - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': { 'enterprise_catalog_recently_released_desc': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, 'enterprise_catalog_price_asc': ALGOLIA_PRICE_ASC_REPLICA_INDEX_SETTINGS, }, @@ -86,7 +86,7 @@ fall back to the relevance ordering and pagination stays deterministic: **That is all the wiring.** You do **not** touch ``_get_algolia_replica_names``, ``_configured_replicas``, ``configure_algolia_index``, or the secured-key ``replica_index_names`` --- they all read ``ADDITIONAL_REPLICA_INDEX_SETTINGS``, so the new replica is automatically declared +-- they all read ``ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS``, so the new replica is automatically declared on the primary index, has its settings applied during a reindex, and is added to the secured API key's ``restrictIndices``. @@ -107,10 +107,14 @@ Deploy / ops Once the code is merged and deployed: -#. The replica's name and settings ship in code (``ADDITIONAL_REPLICA_INDEX_SETTINGS``), so no - edx-internal change is required to *declare* it. If an environment needs a different index name, - ops overrides ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` in edx-internal -- because - ``ALGOLIA`` is merged, only the keys ops sets are overridden. +#. The replica's name and settings ship in code (``ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS``), so no + edx-internal change is required to *declare* it -- and normally ops should not override it at all. + The ``ALGOLIA`` merge is *shallow*: the deployment YAML's top-level ``ALGOLIA`` keys override the + code defaults, but nested dicts are **not** deep-merged. So setting + ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']`` in edx-internal **replaces the entire map** + for that environment (dropping every code-defined replica it does not restate) rather than + overriding individual entries. Prefer setting the index name in code; override the map only when + you intend to fully restate it. #. Run ``./manage.py reindex_algolia``. A *virtual* replica exists as soon as it is declared on the primary index's settings (it mirrors the primary's records), so the replica is live after one reindex -- no separate population step. diff --git a/enterprise_catalog/apps/api_client/algolia.py b/enterprise_catalog/apps/api_client/algolia.py index c97955cb..8f209e30 100644 --- a/enterprise_catalog/apps/api_client/algolia.py +++ b/enterprise_catalog/apps/api_client/algolia.py @@ -52,14 +52,14 @@ def replica_index_names(self): Index names of every configured sort replica (base + additional), empty when none are set. The base replica is ``ALGOLIA['REPLICA_INDEX_NAME']`` (when set); the additional sort - replicas are the keys of ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']``. Used to scope the + replicas are the keys of ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']``. Used to scope the secured API key to exactly the replicas that exist. """ names = [] base_replica = settings.ALGOLIA.get('REPLICA_INDEX_NAME') if base_replica: names.append(base_replica) - names.extend(settings.ALGOLIA.get('ADDITIONAL_REPLICA_INDEX_SETTINGS', {}).keys()) + names.extend(settings.ALGOLIA.get('ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS', {}).keys()) return names def init_index(self): diff --git a/enterprise_catalog/apps/api_client/tests/test_algolia.py b/enterprise_catalog/apps/api_client/tests/test_algolia.py index 78cec60c..f949bc59 100644 --- a/enterprise_catalog/apps/api_client/tests/test_algolia.py +++ b/enterprise_catalog/apps/api_client/tests/test_algolia.py @@ -157,7 +157,7 @@ def test_set_index_settings_failure_logs_effective_index_name(self): @override_settings(ALGOLIA={ 'INDEX_NAME': 'enterprise_catalog', 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': { 'enterprise_catalog_recently_released_desc': {'customRanking': ['desc(recently_released_timestamp)']}, }, 'SEARCH_API_KEY': 'fake-search-key', diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 3b037913..9a071857 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -74,14 +74,14 @@ def _get_algolia_replica_names() -> list[str]: Returns a ``virtual(name)`` entry for the base replica (``ALGOLIA['REPLICA_INDEX_NAME']``, when set) and for each additional sort replica (the keys of - ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']``). An unset base replica is omitted, so we never + ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']``). An unset base replica is omitted, so we never declare a ``virtual(None)`` replica on the primary index. """ replica_names = [] base_replica = settings.ALGOLIA.get('REPLICA_INDEX_NAME') if base_replica: replica_names.append(f'virtual({base_replica})') - for index_name in settings.ALGOLIA.get('ADDITIONAL_REPLICA_INDEX_SETTINGS', {}): + for index_name in settings.ALGOLIA.get('ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS', {}): replica_names.append(f'virtual({index_name})') return replica_names @@ -245,7 +245,7 @@ def _configured_replicas(): Covers the base replica (``ALGOLIA['REPLICA_INDEX_NAME']`` with ``ALGOLIA_REPLICA_INDEX_SETTINGS``, when set) and every additional sort replica declared in - ``ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS']`` (an ``index_name -> settings`` map). Because each + ``ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS']`` (an ``index_name -> settings`` map). Because each additional replica carries its own settings, there is no separate settings lookup that can drift out of sync; this is inert until at least one replica is configured. """ @@ -253,7 +253,7 @@ def _configured_replicas(): base_replica = settings.ALGOLIA.get('REPLICA_INDEX_NAME') if base_replica: configured.append((base_replica, ALGOLIA_REPLICA_INDEX_SETTINGS)) - for index_name, index_settings in settings.ALGOLIA.get('ADDITIONAL_REPLICA_INDEX_SETTINGS', {}).items(): + for index_name, index_settings in settings.ALGOLIA.get('ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS', {}).items(): configured.append((index_name, index_settings)) return configured @@ -454,7 +454,7 @@ def configure_algolia_index(algolia_client): # init_index() logs and returns without setting algolia_index when a required name # (INDEX_NAME / REPLICA_INDEX_NAME) or credential is missing. set_index_settings() would # then silently no-op for every index, making a misconfigured reindex look successful. - # Since ALGOLIA is replaced (not merged) per-environment, fail loudly here instead. + # Fail loudly here instead so a missing/empty primary index name can't pass as success. raise ImproperlyConfigured( 'Cannot configure Algolia index: the primary index is not initialized. Check the ' 'ALGOLIA settings (INDEX_NAME, REPLICA_INDEX_NAME, APPLICATION_ID, API_KEY).' diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index 58571b23..69d4f099 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -289,7 +289,7 @@ def test_get_algolia_replica_names_combines_base_and_additional_replicas(self): # pylint: disable=protected-access with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': 'enterprise_catalog_duration_desc', - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': { 'enterprise_catalog_recently_released_desc': {'customRanking': []}, }, }): @@ -303,7 +303,7 @@ def test_get_algolia_replica_names_combines_base_and_additional_replicas(self): # An unset base replica is omitted (never virtual(None)); additional replicas still declare. with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': '', - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': {'enterprise_catalog_recently_released_desc': {}}, + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': {'enterprise_catalog_recently_released_desc': {}}, }): assert utils._get_algolia_replica_names() == ['virtual(enterprise_catalog_recently_released_desc)'] # Nothing configured -> no replicas at all. @@ -1085,13 +1085,13 @@ def test_configure_algolia_index(self, mock_search_client): @mock.patch('enterprise_catalog.apps.catalog.algolia_utils.AlgoliaSearchClient') def test_configure_algolia_index_configures_additional_replica(self, mock_search_client): """ - Each entry in ADDITIONAL_REPLICA_INDEX_SETTINGS has its settings applied, keyed by index name. + Each entry in ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS has its settings applied, keyed by index name. """ algolia_client = utils.get_initialized_algolia_client() replica_name = 'enterprise_catalog_recently_released_desc' recency_settings = {'customRanking': ['desc(recently_released_timestamp)']} with override_settings(ALGOLIA={ - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': {replica_name: recency_settings}, + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': {replica_name: recency_settings}, }): utils.configure_algolia_index(algolia_client) mock_search_client.return_value.set_index_settings.assert_any_call( @@ -1118,7 +1118,7 @@ def fail_only_for_recency(index_settings, index_name=None): # pylint: disable=u mock_search_client.return_value.set_index_settings.side_effect = fail_only_for_recency with override_settings(ALGOLIA={ 'REPLICA_INDEX_NAME': base_name, - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': {recency_name: recency_settings}, + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': {recency_name: recency_settings}, }): # The per-replica handler logs a single-line WARNING (set_index_settings already logged # the traceback before re-raising), so we don't double up stack traces. diff --git a/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py b/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py index c6ee8a57..9308a69b 100644 --- a/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py +++ b/enterprise_catalog/apps/search/management/commands/incremental_reindex_algolia.py @@ -117,6 +117,18 @@ def handle(self, *args, **options): else: self.stdout.write('Configuring Algolia index settings...') replica_name = replica_index_name or f'{index_name}_repl' + # TODO (ENT-11982): this block is a temporary workaround and should be refactored before + # this command runs in production (the incremental-reindexing cutover) on two fronts: + # 1. It configures only the single base ("by duration") replica -- it does NOT loop the + # unified sort-replica registry the way configure_algolia_index() does (via + # _get_algolia_replica_names() / _configured_replicas()), so the "newest courses + # first" sort and any future ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS'] + # replicas are not configured by this path. + # 2. It re-plumbs AlgoliaSearchClient internals (assigning _client / algolia_index + # directly below) and spins up a second client via new_search_client_or_error() -- + # both are hacks that work around AlgoliaSearchClient.init_index() not accepting an + # arbitrary index name. The real fix is to parameterize init_index() so this command + # can target a non-primary index without reaching into the client's internals. sdk_client = new_search_client_or_error() algolia_client = AlgoliaSearchClient() # Target a non-primary index: wire the SDK client and the primary handle directly so diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index dec904e0..6bafcf3b 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -436,10 +436,10 @@ # API_KEY) are populated from the deployment config (edx-internal). ALGOLIA is listed in # DICT_UPDATE_KEYS (see settings/production.py), so the deployment YAML is *merged* into this dict # rather than replacing it -- per-environment keys override the defaults below while code-defined -# defaults like ADDITIONAL_REPLICA_INDEX_SETTINGS are preserved. +# defaults like ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS are preserved. # customRanking for the "newest courses first" sort replica. Defined here as config-as-code and -# referenced from ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS'] below. Leads with the precomputed +# referenced from ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS'] below. Leads with the precomputed # recently_released_timestamp (a course's earliest course-run start, any status) descending, then # the primary index's relevance tie-breakers. ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS = { @@ -455,9 +455,10 @@ ALGOLIA = { 'INDEX_NAME': '', - # Base replica: the long-standing sort the learner-portal MFE points its video search - # (SEARCH_INDEX_IDS.VIDEOS) at -- the MFE's ALGOLIA_REPLICA_INDEX_NAME env var. Its - # customRanking lives in ALGOLIA_REPLICA_INDEX_SETTINGS (apps/catalog/algolia_utils.py). + # Base replica: a long-standing "by duration" sort (its customRanking leads with desc(duration), + # defined in ALGOLIA_REPLICA_INDEX_SETTINGS in apps/catalog/algolia_utils.py) that the + # learner-portal MFE points its video search (SEARCH_INDEX_IDS.VIDEOS) at, via the MFE's + # ALGOLIA_REPLICA_INDEX_NAME env var. 'REPLICA_INDEX_NAME': '', # Additional sort replicas beyond the base one, mapped index_name -> Algolia index settings. # This is the single source of truth for which extra replicas exist: each entry is declared on @@ -465,7 +466,7 @@ # key. The learner-portal only points its course at the "newest first" replica when the # enterprise.search_default_sort_newest waffle flag + Optimizely experiment are on, so the flag # -- not the mere presence of an entry here -- gates user-facing exposure. - 'ADDITIONAL_REPLICA_INDEX_SETTINGS': { + 'ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS': { 'enterprise_catalog_recently_released_desc': ALGOLIA_RECENTLY_RELEASED_REPLICA_INDEX_SETTINGS, }, 'APPLICATION_ID': '', diff --git a/enterprise_catalog/settings/production.py b/enterprise_catalog/settings/production.py index 45e095bf..f182bd56 100644 --- a/enterprise_catalog/settings/production.py +++ b/enterprise_catalog/settings/production.py @@ -22,7 +22,7 @@ # Keep track of the names of settings that represent dicts. Instead of overriding the values in base.py, # the values read from disk should UPDATE the pre-configured dicts. # ALGOLIA is merged (not replaced) so the deployment YAML can set per-environment index names and -# credentials while code-defined defaults such as ALGOLIA['ADDITIONAL_REPLICA_INDEX_SETTINGS'] +# credentials while code-defined defaults such as ALGOLIA['ADDITIONAL_VIRTUAL_REPLICA_INDEX_SETTINGS'] # (the sort-replica customRanking definitions) are preserved. DICT_UPDATE_KEYS = ('JWT_AUTH', 'REST_FRAMEWORK', 'ALGOLIA')