From e7f763c38941270fc4f33e9bbbc8abfc6f62844c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Brychta?= <stepan.brychta@me.com>
Date: Tue, 26 Nov 2024 13:55:04 +0000
Subject: [PATCH] Add comments #5825

---
 .../api/search/services/WorksRequestBuilder.scala | 15 +++++++++++----
 .../search/models/AggregationResultsTest.scala    |  2 +-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala b/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala
index 7c40a542e..d19931021 100644
--- a/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala
+++ b/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala
@@ -72,6 +72,12 @@ object WorksRequestBuilder
       filterToQuery = buildWorkFilterQuery
     )
 
+  /**
+   * Each aggregatable field is indexed as a nested field with an `id` value and a `label` value. All aggregations
+   * are `id`-based, with a `label`-based sub-aggregation to get Elasticsearch to return all `label` values associated
+   * with each `id` bucket. (Usually each `id` value only has one one `label` value associated with it, but not always.
+   * For example, different Works can use different labels for a given LoC Subject Heading.)
+   */
   private def toIdBasedAggregation(
     aggregationName: String,
     nestedFieldPath: String,
@@ -82,12 +88,13 @@ object WorksRequestBuilder
       .field(s"$nestedFieldPath.id")
       .subAggregations(termsAgg("labels", s"$nestedFieldPath.label").size(1))
 
-  private def toLabelBasedAggregation(
+  @deprecated("This method is included for backward compatibility reasons and will be removed soon.", "25-10-2024")
+  private def toLegacyAggregation(
     aggregationName: String,
-    idFieldPath: String,
+    aggregatedFieldPath: String,
     size: Int
   ) =
-    TermsAggregation(aggregationName).size(size).field(idFieldPath)
+    TermsAggregation(aggregationName).size(size).field(aggregatedFieldPath)
 
   private def toAggregation(aggReq: WorkAggregationRequest) = aggReq match {
     // Note: we want these aggregations to return every possible value, so we
@@ -99,7 +106,7 @@ object WorksRequestBuilder
       toIdBasedAggregation("format", "aggregatableValues.workType", size = 30)
 
     case WorkAggregationRequest.ProductionDate =>
-      toLabelBasedAggregation(
+      toLegacyAggregation(
         "productionDates",
         "aggregatableValues.production.dates.label",
         size = 10
diff --git a/search/src/test/scala/weco/api/search/models/AggregationResultsTest.scala b/search/src/test/scala/weco/api/search/models/AggregationResultsTest.scala
index 4d62df1a2..20f10ed15 100644
--- a/search/src/test/scala/weco/api/search/models/AggregationResultsTest.scala
+++ b/search/src/test/scala/weco/api/search/models/AggregationResultsTest.scala
@@ -109,7 +109,7 @@ class AggregationResultsTest extends AnyFunSpec with Matchers {
     )
   }
 
-  it("correctly populates AggregationBucketData with IDs and labels if a nested 'labels' bucket is provided for each ID") {
+  it("correctly populates AggregationBucketData with IDs and labels if a nested 'labels' bucket is provided for each ID bucket") {
     val searchResponse = SearchResponse(
       took = 1234,
       isTimedOut = false,