Updates from code review comments

* Removed comment about default TranslateCategories value * Changed translateCat to translateCategories in tests * Combined helper functions in exchange_test related to TranslateCategories
prebid · Nov 5, 2019 · ce9c17a · ce9c17a
1 parent a23dfb2
commit ce9c17a
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 64 deletions.
diff --git a/endpoints/openrtb2/video_auction_test.go b/endpoints/openrtb2/video_auction_test.go
@@ -106,12 +106,12 @@ func TestCreateBidExtension(t *testing.T) {
 		Increment: 0.1,
 	})
 
-	translateCat := true
+	translateCategories := true
 	videoRequest := openrtb_ext.BidRequestVideo{
 		IncludeBrandCategory: &openrtb_ext.IncludeBrandCategory{
 			PrimaryAdserver:     1,
 			Publisher:           "",
-			TranslateCategories: &translateCat,
+			TranslateCategories: &translateCategories,
 		},
 		PodConfig: openrtb_ext.PodConfig{
 			DurationRangeSec:     durationRange,
@@ -140,12 +140,12 @@ func TestCreateBidExtensionExactDurTrueNoPriceRange(t *testing.T) {
 	durationRange = append(durationRange, 15)
 	durationRange = append(durationRange, 30)
 
-	translateCat := false
+	translateCategories := false
 	videoRequest := openrtb_ext.BidRequestVideo{
 		IncludeBrandCategory: &openrtb_ext.IncludeBrandCategory{
 			PrimaryAdserver:     1,
 			Publisher:           "",
-			TranslateCategories: &translateCat,
+			TranslateCategories: &translateCategories,
 		},
 		PodConfig: openrtb_ext.PodConfig{
 			DurationRangeSec:     durationRange,

diff --git a/exchange/exchange.go b/exchange/exchange.go
@@ -358,7 +358,6 @@ func applyCategoryMapping(ctx context.Context, requestExt openrtb_ext.ExtRequest
 	var translateCategories = true
 
 	if includeBrandCategory && brandCatExt.WithCategory {
-		//if TranslateCategories is nil, default category translation to enabled
 		if brandCatExt.TranslateCategories != nil {
 			translateCategories = *brandCatExt.TranslateCategories
 		}

diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go
@@ -526,8 +526,8 @@ func newExtRequest() openrtb_ext.ExtRequest {
 		},
 	}
 
-	translateCat := true
-	brandCat := openrtb_ext.ExtIncludeBrandCategory{PrimaryAdServer: 1, WithCategory: true, TranslateCategories: &translateCat}
+	translateCategories := true
+	brandCat := openrtb_ext.ExtIncludeBrandCategory{PrimaryAdServer: 1, WithCategory: true, TranslateCategories: &translateCategories}
 
 	reqExt := openrtb_ext.ExtRequestTargeting{
 		PriceGranularity:     priceGran,
@@ -569,61 +569,6 @@ func newExtRequestNoBrandCat() openrtb_ext.ExtRequest {
 	}
 }
 
-func newExtRequestTranslateCatNil() openrtb_ext.ExtRequest {
-	priceGran := openrtb_ext.PriceGranularity{
-		Precision: 2,
-		Ranges: []openrtb_ext.GranularityRange{
-			{
-				Min:       0.0,
-				Max:       20.0,
-				Increment: 2.0,
-			},
-		},
-	}
-
-	brandCat := openrtb_ext.ExtIncludeBrandCategory{WithCategory: true, PrimaryAdServer: 1}
-
-	reqExt := openrtb_ext.ExtRequestTargeting{
-		PriceGranularity:     priceGran,
-		IncludeWinners:       true,
-		IncludeBrandCategory: &brandCat,
-	}
-
-	return openrtb_ext.ExtRequest{
-		Prebid: openrtb_ext.ExtRequestPrebid{
-			Targeting: &reqExt,
-		},
-	}
-}
-
-func newExtRequestTranslateCatFalse() openrtb_ext.ExtRequest {
-	priceGran := openrtb_ext.PriceGranularity{
-		Precision: 2,
-		Ranges: []openrtb_ext.GranularityRange{
-			{
-				Min:       0.0,
-				Max:       20.0,
-				Increment: 2.0,
-			},
-		},
-	}
-
-	translateCat := false
-	brandCat := openrtb_ext.ExtIncludeBrandCategory{WithCategory: true, TranslateCategories: &translateCat}
-
-	reqExt := openrtb_ext.ExtRequestTargeting{
-		PriceGranularity:     priceGran,
-		IncludeWinners:       true,
-		IncludeBrandCategory: &brandCat,
-	}
-
-	return openrtb_ext.ExtRequest{
-		Prebid: openrtb_ext.ExtRequestPrebid{
-			Targeting: &reqExt,
-		},
-	}
-}
-
 func TestCategoryMapping(t *testing.T) {
 
 	categoriesFetcher, error := newCategoryFetcher("./test/category-mapping")
@@ -739,7 +684,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) {
 		t.Errorf("Failed to create a category Fetcher: %v", error)
 	}
 
-	requestExt := newExtRequestTranslateCatNil()
+	requestExt := newExtRequestTranslateCategories(nil)
 
 	targData := &targetData{
 		priceGranularity: requestExt.Prebid.Targeting.PriceGranularity,
@@ -781,14 +726,45 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) {
 	assert.Equal(t, 2, len(bidCategory), "Bidders category mapping doesn't match")
 }
 
+func newExtRequestTranslateCategories(translateCategories *bool) openrtb_ext.ExtRequest {
+	priceGran := openrtb_ext.PriceGranularity{
+		Precision: 2,
+		Ranges: []openrtb_ext.GranularityRange{
+			{
+				Min:       0.0,
+				Max:       20.0,
+				Increment: 2.0,
+			},
+		},
+	}
+
+	brandCat := openrtb_ext.ExtIncludeBrandCategory{WithCategory: true, PrimaryAdServer: 1}
+	if translateCategories != nil {
+		brandCat.TranslateCategories = translateCategories
+	}
+
+	reqExt := openrtb_ext.ExtRequestTargeting{
+		PriceGranularity:     priceGran,
+		IncludeWinners:       true,
+		IncludeBrandCategory: &brandCat,
+	}
+
+	return openrtb_ext.ExtRequest{
+		Prebid: openrtb_ext.ExtRequestPrebid{
+			Targeting: &reqExt,
+		},
+	}
+}
+
 func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) {
 
 	categoriesFetcher, error := newCategoryFetcher("./test/category-mapping")
 	if error != nil {
 		t.Errorf("Failed to create a category Fetcher: %v", error)
 	}
 
-	requestExt := newExtRequestTranslateCatFalse()
+	translateCategories := false
+	requestExt := newExtRequestTranslateCategories(&translateCategories)
 
 	targData := &targetData{
 		priceGranularity: requestExt.Prebid.Targeting.PriceGranularity,