Add comments #5825

wellcomecollection · Nov 26, 2024 · e7f763c · e7f763c
1 parent ad48316
commit e7f763c
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 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
@@ -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
@@ -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,