Add support for searching by group #243
Conversation
| // } | ||
| type parseResponse struct { | ||
| Query string `json:"query"` | ||
| Fields []string `json:"fields"` | ||
| ElasticQuery interface{} `json:"elastic_query"` | ||
| AllowAsGroup bool `json:"allow_as_group"` |
There was a problem hiding this comment.
seems like we shouldn't trust RP on this.. and maybe even push this decision down to goflow
| @@ -28,11 +28,11 @@ | |||
| "path": "/mr/contact/parse_query", | |||
| "body": { | |||
| "org_id": 1, | |||
| "query": "age \u003e 10" | |||
| "query": "AGE>10" | |||
There was a problem hiding this comment.
make sure we're sending back a rewritten query
rowanseymour
force-pushed
the
latest_goflow
branch
from
ed1bcca to
ae07523
Compare
Codecov Report
@@ Coverage Diff @@
## master #243 +/- ##
==========================================
- Coverage 48.08% 48.04% -0.05%
==========================================
Files 83 83
Lines 7603 7608 +5
==========================================
- Hits 3656 3655 -1
- Misses 3368 3371 +3
- Partials 579 582 +3
Continue to review full report at Codecov.
|
rowanseymour
force-pushed
the
latest_goflow
branch
from
ae07523 to
676ac76
Compare
search/search.go
Outdated
| @@ -14,6 +14,9 @@ import ( | |||
| "github.com/shopspring/decimal" | |||
| ) | |||
|
|
|||
| // GroupResolverFunc resolves a group name to a UUID because ES indexes groups by the latter | |||
| type GroupResolverFunc func(name string) assets.Group | |||
There was a problem hiding this comment.
What about having a single interface that has a ResolveField and ResolveGroup function on it? Just seems we are building lots of anonymous functions to represent something instead of just having an interface that would be clearer.
There was a problem hiding this comment.
That's fine but note that you can't use an interface - golang doesn't let you implement an interface anonymously 😐
There was a problem hiding this comment.
Not sure I understand. Yes, we'd need to create a struct, but then OrgAssets could just satisfy that as would a test version.
There was a problem hiding this comment.
That's what I'm doing - just meant you need a struct.. can't just define the funcs on the fly like we're doing currently
There was a problem hiding this comment.
Oh, well I don't much love on the fly funcs anyways so that's fine by me!
|
@nicpottier wanna take another peek? |
rowanseymour
changed the title
Update to latest goflow
Pausing to go learn ES..