From 960f8432594753f2b4062744ba0bf4e79d0fc546 Mon Sep 17 00:00:00 2001
From: WithoutPants <53250216+WithoutPants@users.noreply.github.com>
Date: Thu, 18 Mar 2021 21:45:18 +1100
Subject: [PATCH] Fix filter building with sub-filters (#1212)

* Fix bracketing on sub-filters
* Add vscode to gitignore
---
 .gitignore                         |  1 +
 pkg/sqlite/filter.go               |  9 ++++++---
 pkg/sqlite/filter_internal_test.go | 31 ++++++++++++++++++++----------
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/.gitignore b/.gitignore
index 822b8ce50..d6c2eea2c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,6 +35,7 @@ ui/v2.5/src/core/generated-*.tsx
 .idea/**/usage.statistics.xml
 .idea/**/dictionaries
 .idea/**/shelf
+.vscode
 
 # Generated files
 .idea/**/contentModel.xml
diff --git a/pkg/sqlite/filter.go b/pkg/sqlite/filter.go
index 8b29c9a2f..bae383825 100644
--- a/pkg/sqlite/filter.go
+++ b/pkg/sqlite/filter.go
@@ -182,7 +182,7 @@ func (f *filterBuilder) getSubFilterClause(clause, subFilterClause string) strin
 			}
 		}
 
-		ret += op + subFilterClause
+		ret += op + "(" + subFilterClause + ")"
 	}
 
 	return ret
@@ -216,7 +216,7 @@ func (f *filterBuilder) generateHavingClauses() (string, []interface{}) {
 	if f.subFilter != nil {
 		c, a := f.subFilter.generateHavingClauses()
 		if c != "" {
-			clause += " " + f.subFilterOp + " " + c
+			clause = f.getSubFilterClause(clause, c)
 			if len(a) > 0 {
 				args = append(args, a...)
 			}
@@ -284,7 +284,10 @@ func (f *filterBuilder) andClauses(input []sqlClause) (string, []interface{}) {
 	}
 
 	if len(clauses) > 0 {
-		c := "(" + strings.Join(clauses, " AND ") + ")"
+		c := strings.Join(clauses, " AND ")
+		if len(clauses) > 1 {
+			c = "(" + c + ")"
+		}
 		return c, args
 	}
 
diff --git a/pkg/sqlite/filter_internal_test.go b/pkg/sqlite/filter_internal_test.go
index 5677074d7..302aff0db 100644
--- a/pkg/sqlite/filter_internal_test.go
+++ b/pkg/sqlite/filter_internal_test.go
@@ -227,14 +227,14 @@ func TestGenerateWhereClauses(t *testing.T) {
 	// ensure single where clause is generated correctly
 	f.addWhere(clause1)
 	r, rArgs := f.generateWhereClauses()
-	assert.Equal("("+clause1+")", r)
+	assert.Equal(clause1, r)
 	assert.Len(rArgs, 0)
 
 	// ensure multiple where clauses are surrounded with parenthesis and
 	// ANDed together
 	f.addWhere(clause2, arg1, arg2)
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause1+" AND "+clause2+")", r)
+	assert.Equal(fmt.Sprintf("(%s AND %s)", clause1, clause2), r)
 	assert.Len(rArgs, 2)
 
 	// ensure empty subfilter is not added to generated where clause
@@ -242,13 +242,13 @@ func TestGenerateWhereClauses(t *testing.T) {
 	f.and(sf)
 
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause1+" AND "+clause2+")", r)
+	assert.Equal(fmt.Sprintf("(%s AND %s)", clause1, clause2), r)
 	assert.Len(rArgs, 2)
 
 	// ensure sub-filter is generated correctly
 	sf.addWhere(clause3, arg3)
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause1+" AND "+clause2+") AND ("+clause3+")", r)
+	assert.Equal(fmt.Sprintf("(%s AND %s) AND (%s)", clause1, clause2, clause3), r)
 	assert.Len(rArgs, 3)
 
 	// ensure OR sub-filter is generated correctly
@@ -258,7 +258,7 @@ func TestGenerateWhereClauses(t *testing.T) {
 	f.or(sf)
 
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause1+" AND "+clause2+") OR ("+clause3+")", r)
+	assert.Equal(fmt.Sprintf("(%s AND %s) OR (%s)", clause1, clause2, clause3), r)
 	assert.Len(rArgs, 3)
 
 	// ensure NOT sub-filter is generated correctly
@@ -268,7 +268,7 @@ func TestGenerateWhereClauses(t *testing.T) {
 	f.not(sf)
 
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause1+" AND "+clause2+") AND NOT ("+clause3+")", r)
+	assert.Equal(fmt.Sprintf("(%s AND %s) AND NOT (%s)", clause1, clause2, clause3), r)
 	assert.Len(rArgs, 3)
 
 	// ensure empty filter with ANDed sub-filter does not include AND
@@ -276,7 +276,7 @@ func TestGenerateWhereClauses(t *testing.T) {
 	f.and(sf)
 
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause3+")", r)
+	assert.Equal(fmt.Sprintf("(%s)", clause3), r)
 	assert.Len(rArgs, 1)
 
 	// ensure empty filter with ORed sub-filter does not include OR
@@ -284,7 +284,7 @@ func TestGenerateWhereClauses(t *testing.T) {
 	f.or(sf)
 
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("("+clause3+")", r)
+	assert.Equal(fmt.Sprintf("(%s)", clause3), r)
 	assert.Len(rArgs, 1)
 
 	// ensure empty filter with NOTed sub-filter does not include AND
@@ -292,8 +292,19 @@ func TestGenerateWhereClauses(t *testing.T) {
 	f.not(sf)
 
 	r, rArgs = f.generateWhereClauses()
-	assert.Equal("NOT ("+clause3+")", r)
+	assert.Equal(fmt.Sprintf("NOT (%s)", clause3), r)
 	assert.Len(rArgs, 1)
+
+	// (clause1) AND ((clause2) OR (clause3))
+	f = &filterBuilder{}
+	f.addWhere(clause1)
+	sf2 := &filterBuilder{}
+	sf2.addWhere(clause2, arg1, arg2)
+	f.and(sf2)
+	sf2.or(sf)
+	r, rArgs = f.generateWhereClauses()
+	assert.Equal(fmt.Sprintf("%s AND (%s OR (%s))", clause1, clause2, clause3), r)
+	assert.Len(rArgs, 3)
 }
 
 func TestGenerateHavingClauses(t *testing.T) {
@@ -312,7 +323,7 @@ func TestGenerateHavingClauses(t *testing.T) {
 	// ensure single Having clause is generated correctly
 	f.addHaving(clause1)
 	r, rArgs := f.generateHavingClauses()
-	assert.Equal("("+clause1+")", r)
+	assert.Equal(clause1, r)
 	assert.Len(rArgs, 0)
 
 	// ensure multiple Having clauses are surrounded with parenthesis and