Repository: algolia/algoliasearch-client-go
Pull request #702 (2022-08-31 09:35:50)

fix(rules): Omitempty the facet values rules ordering

clemfromspace: @samouss Should have been a draft, my bad, was testing things around.

To answer your question: We rely heavily on the Marshal / Unmarshal on the CLI side, and I was trying to copy the rules from one index to another and I got an error like the following:
> Algolia API error [400] Invalid query parameter in consequence: Invalid value for "renderingContent" parameter, expected Error in renderingContent parsing: Unexpected value type: Expecting an array (near 1:647) (in [0]) (near 1:908)
My understanding is that for the rules which do not have an `order` present in the Algolia response, the Go client is setting the `order` to `nil` during the unmarshalling. I will then got a corresponding `"order": null` if during the marshal If I try to import back the rules to Algolia, since the API is expecting either an array for the `order` (even an empty one), or no `order` at all.

The reasoning was: Omit the `order` field if it's`nil`, so it won't be send to Algolia as a `null` JSON value.

Happy to to have your thoughts on this.
samouss: > My understanding is that for the rules which do not have an `order` present in the Algolia response, the Go client is setting the `order` to `nil` during the unmarshalling. I will then got a corresponding `"order": null` during the marshalling (when I try to import back the rules to Algolia). ...And have the above error, since the API is expecting either an array for the `order` (even an empty one), or no `order` at all (not sure about that though...)

Well, if that's the issue I guess it's easy to test. You can run the CLI with a patched version of the client to see if it works. We could even write an integration tests with the scenario you described. It would ensure that we'll never reproduce the issue.

I don't know what's the process in this case @VladislavFitz: the integration tests are driven by the common test suite. Is it fine to add new integration tests specifically to fix an issue found in the client. I guess it is but prefer to double check!
clemfromspace: I tested my patch using the Algolia CLI, and it seems to work well with the `omitempty`, although I needed toi add a second one on the `Values` field of the `FacetOrdering` struct because I had a similar error:

> failed to save rules for batch 1398: Algolia API error [400] Invalid query parameter in consequence: Invalid value for "renderingContent" parameter, expected Error in renderingContent parsing: Unexpected value type: Expecting an object.

@VladislavFitz let me know regarding the question of @samouss above, I can add an integration test if necessary.
clemfromspace: @VladislavFitz @samouss I added one unit test to check the `Marshal` behaviour following my changes. IMHO, it's enough but I can add integrations tests as well if you feel it's necessary.
VladislavFitz: @clemfromspace it's ok to add some custom integration tests. The more tests the better 🙂 
samouss: Is there anything retaining you from merging the PR? If not please go ahead.

Patch:
diff --git a/DOCKER_README.MD b/DOCKER_README.MD
index a28485c3..9f78fa70 100644
--- a/DOCKER_README.MD
+++ b/DOCKER_README.MD
@@ -18,7 +18,7 @@ $ brew install docker-machine
 
 Then install [VirtualBox](https://www.virtualbox.org/) using [Homebrew Cask](https://github.com/Homebrew/homebrew-cask) to get a driver for your Docker machine
 ```
-$ brew cask install virtualbox
+$ brew install --cask virtualbox
 ```
 
 You may need to enter your password and authorize the application through `System Settings` > `Security & Privacy`.
diff --git a/algolia/search/rendering_content.go b/algolia/search/rendering_content.go
index 2d624c5d..aa84ed21 100644
--- a/algolia/search/rendering_content.go
+++ b/algolia/search/rendering_content.go
@@ -12,7 +12,7 @@ type FacetOrdering struct {
 	Facets *FacetsOrder `json:"facets"`
 
 	// The ordering of facet values, within an individual list.
-	Values map[string]FacetValuesOrder `json:"values"`
+	Values map[string]FacetValuesOrder `json:"values,omitempty"`
 }
 
 // Facets ordering rule container
@@ -24,7 +24,7 @@ type FacetsOrder struct {
 // Facet values ordering rule container
 type FacetValuesOrder struct {
 	// Pinned order of facet values.
-	Order []string `json:"order"`
+	Order []string `json:"order,omitempty"`
 
 	// How to display the remaining items.
 	SortRemainingBy *SortRule `json:"sortRemainingBy"`


Test patch:
diff --git a/algolia/search/rendering_content_test.go b/algolia/search/rendering_content_test.go
index 35fe384c..50eac4c5 100644
--- a/algolia/search/rendering_content_test.go
+++ b/algolia/search/rendering_content_test.go
@@ -21,6 +21,9 @@ func TestUnmarshalRenderingContent(t *testing.T) {
 				},
 				"color": {
 					"order": ["red", "green"]
+				},
+				"size": {
+					"sortRemainingBy": "alpha"
 				}
 			}
 		}
@@ -29,10 +32,91 @@ func TestUnmarshalRenderingContent(t *testing.T) {
 	err := json.Unmarshal([]byte(payload), &r)
 	require.NoError(t, err)
 	require.Equal(t, r.FacetOrdering.Facets.Order, []string{"brand", "size", "color"})
+
 	require.Equal(t, r.FacetOrdering.Values["brand"].Order, []string{"Apple", "Sony", "Samsung"})
 	require.Equal(t, *r.FacetOrdering.Values["brand"].SortRemainingBy, Alpha)
+
 	require.Equal(t, r.FacetOrdering.Values["color"].Order, []string{"red", "green"})
 	require.Nil(t, r.FacetOrdering.Values["color"].SortRemainingBy)
+
+	require.Nil(t, r.FacetOrdering.Values["size"].Order)
+}
+
+func TestMarshalRenderingContent(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    RenderingContent
+		expected string
+	}{
+		{
+			name: "empty values",
+			input: RenderingContent{
+				FacetOrdering: &FacetOrdering{
+					Facets: &FacetsOrder{
+						Order: []string{"brand", "size", "color"},
+					},
+					Values: nil,
+				},
+			},
+			expected: `{
+				"facetOrdering": {
+					"facets": {
+						"order": [
+							"brand",
+							"size",
+							"color"
+						]
+					}
+				}
+			}`,
+		},
+		{
+			name: "values with one empty facet value `order`",
+			input: RenderingContent{
+				FacetOrdering: &FacetOrdering{
+					Facets: &FacetsOrder{
+						Order: []string{"brand", "size", "color"},
+					},
+					Values: map[string]FacetValuesOrder{
+						"brand": NewFacetValuesOrder([]string{"Apple", "Sony", "Samsung"}, Alpha),
+						"color": NewFacetValuesOrder(nil, Hidden),
+					},
+				},
+			},
+			expected: `{
+				"facetOrdering": {
+					"facets": {
+						"order": [
+							"brand",
+							"size",
+							"color"
+						]
+					},
+					"values": {
+						"brand": {
+							"order": [
+								"Apple",
+								"Sony",
+								"Samsung"
+							],
+							"sortRemainingBy": "alpha"
+						},
+						"color": {
+							"sortRemainingBy": "hidden"
+						}
+					}
+				}
+			}`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			b, err := json.Marshal(test.input)
+			require.NoError(t, err)
+			require.JSONEq(t, test.expected, string(b))
+		})
+	}
 }
 
 func TestBuildRenderingContent(t *testing.T) {