Skip to content

Commit

Permalink
Merge pull request #85 from nyaruka/preview_total_only
Browse files Browse the repository at this point in the history 
Stop returning sample contacts on preview endpoints
  • Loading branch information
@rowanseymour
rowanseymour authored May 25, 2023
2 parents b755e98 + da3afe0 commit e39a74c
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 263 deletions.
33 changes: 33 additions & 0 deletions core/search/search.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,39 @@ func BuildElasticQuery(oa *models.OrgAssets, group *models.Group, status models.
return eq
}

// GetContactTotal returns the total count of matching contacts for the given query
func GetContactTotal(ctx context.Context, rt *runtime.Runtime, oa *models.OrgAssets, query string) (*contactql.ContactQuery, int64, error) {
env := oa.Env()
var parsed *contactql.ContactQuery
var err error

if rt.ES == nil {
return nil, 0, errors.Errorf("no elastic client available, check your configuration")
}

if query != "" {
parsed, err = contactql.ParseQuery(env, query, oa.SessionAssets())
if err != nil {
return nil, 0, errors.Wrapf(err, "error parsing query: %s", query)
}
}

eq := BuildElasticQuery(oa, nil, models.NilContactStatus, nil, parsed)

count, err := rt.ES.Count(rt.Config.ElasticContactsIndex).Routing(strconv.FormatInt(int64(oa.OrgID()), 10)).Query(eq).Do(ctx)
if err != nil {
// Get *elastic.Error which contains additional information
ee, ok := err.(*elastic.Error)
if !ok {
return nil, 0, errors.Wrap(err, "error performing query")
}

return nil, 0, errors.Wrapf(err, "error performing query: %s", ee.Details.Reason)
}

return parsed, count, nil
}

// GetContactIDsForQueryPage returns a page of contact ids for the given query and sort
func GetContactIDsForQueryPage(ctx context.Context, rt *runtime.Runtime, oa *models.OrgAssets, group *models.Group, excludeIDs []models.ContactID, query string, sort string, offset int, pageSize int) (*contactql.ContactQuery, []models.ContactID, int64, error) {
env := oa.Env()
Expand Down
48 changes: 38 additions & 10 deletions core/search/search_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,40 @@ import (
"github.com/stretchr/testify/require"
)

func TestGetContactIDsForQueryPage(t *testing.T) {
func TestGetContactTotal(t *testing.T) {
ctx, rt := testsuite.Runtime()

defer testsuite.Reset(testsuite.ResetElastic)
oa, err := models.GetOrgAssets(ctx, rt, testdata.Org1.ID)
require.NoError(t, err)

tcs := []struct {
query string
expectedTotal int64
expectedError string
}{
{query: "george OR bob", expectedTotal: 2},
{query: "george", expectedTotal: 1},
{query: "age >= 30", expectedTotal: 1},
{
query: "goats > 2", // no such contact field
expectedError: "error parsing query: goats > 2: can't resolve 'goats' to attribute, scheme or field",
},
}

for i, tc := range tcs {
_, total, err := search.GetContactTotal(ctx, rt, oa, tc.query)

if tc.expectedError != "" {
assert.EqualError(t, err, tc.expectedError)
} else {
assert.NoError(t, err, "%d: error encountered performing query", i)
assert.Equal(t, tc.expectedTotal, total, "%d: total mismatch", i)
}
}
}

func TestGetContactIDsForQueryPage(t *testing.T) {
ctx, rt := testsuite.Runtime()

oa, err := models.GetOrgAssets(ctx, rt, testdata.Org1.ID)
require.NoError(t, err)
Expand All @@ -30,9 +60,9 @@ func TestGetContactIDsForQueryPage(t *testing.T) {
}{
{ // 0
group: testdata.ActiveGroup,
query: "george",
expectedContacts: []models.ContactID{testdata.George.ID},
expectedTotal: 1,
query: "george OR bob",
expectedContacts: []models.ContactID{testdata.George.ID, testdata.Bob.ID},
expectedTotal: 2,
},
{ // 1
group: testdata.BlockedGroup,
Expand Down Expand Up @@ -80,8 +110,6 @@ func TestGetContactIDsForQueryPage(t *testing.T) {
func TestGetContactIDsForQuery(t *testing.T) {
ctx, rt := testsuite.Runtime()

defer testsuite.Reset(testsuite.ResetElastic)

oa, err := models.GetOrgAssets(ctx, rt, 1)
require.NoError(t, err)

Expand All @@ -92,9 +120,9 @@ func TestGetContactIDsForQuery(t *testing.T) {
expectedError string
}{
{
query: "george",
query: "george OR bob",
limit: -1,
expectedContacts: []models.ContactID{testdata.George.ID},
expectedContacts: []models.ContactID{testdata.George.ID, testdata.Bob.ID},
}, {
query: "nobody",
limit: -1,
Expand All @@ -119,7 +147,7 @@ func TestGetContactIDsForQuery(t *testing.T) {
assert.EqualError(t, err, tc.expectedError)
} else {
assert.NoError(t, err, "%d: error encountered performing query", i)
assert.Equal(t, tc.expectedContacts, ids, "%d: ids mismatch", i)
assert.ElementsMatch(t, tc.expectedContacts, ids, "%d: ids mismatch", i)
}
}
}
36 changes: 9 additions & 27 deletions web/flow/preview_start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,12 @@ func init() {
// "in_a_flow": false,
// "started_previously": true,
// "not_seen_since_days": 90
// },
// "sample_size": 5
// }
// }
//
// {
// "query": "(group = "No Age" OR group = "No Name" OR uuid = "e5bb9e6f-7703-4ba1-afba-0b12791de38b") AND history != \"Registration\"",
// "total": 567,
// "sample": [12, 34, 56, 67, 78],
// "metadata": {
// "fields": [
// {"key": "age", "name": "Age"}
// ],
// "allow_as_group": true
// }
// "query": "(group = \"No Age\" OR group = \"No Name\" OR uuid = \"e5bb9e6f-7703-4ba1-afba-0b12791de38b\") AND history != \"Registration\"",
// "total": 567
// }
type previewRequest struct {
OrgID models.OrgID `json:"org_id" validate:"required"`
Expand All @@ -56,15 +48,12 @@ type previewRequest struct {
ContactUUIDs []flows.ContactUUID `json:"contact_uuids"`
Query string `json:"query"`
} `json:"include" validate:"required"`
Exclude models.Exclusions `json:"exclude"`
SampleSize int `json:"sample_size" validate:"required"`
Exclude models.Exclusions `json:"exclude"`
}

type previewResponse struct {
Query string `json:"query"`
Total int `json:"total"`
SampleIDs []models.ContactID `json:"sample_ids"`
Metadata *contactql.Inspection `json:"metadata,omitempty"`
Query string `json:"query"`
Total int `json:"total"`
}

func handlePreviewStart(ctx context.Context, rt *runtime.Runtime, r *previewRequest) (any, int, error) {
Expand Down Expand Up @@ -95,20 +84,13 @@ func handlePreviewStart(ctx context.Context, rt *runtime.Runtime, r *previewRequ
return nil, 0, err
}
if query == "" {
return &previewResponse{SampleIDs: []models.ContactID{}}, http.StatusOK, nil
return &previewResponse{Query: "", Total: 0}, http.StatusOK, nil
}

parsedQuery, sampleIDs, total, err := search.GetContactIDsForQueryPage(ctx, rt, oa, nil, nil, query, "", 0, r.SampleSize)
parsedQuery, total, err := search.GetContactTotal(ctx, rt, oa, query)
if err != nil {
return nil, 0, errors.Wrap(err, "error querying preview")
}

inspection := contactql.Inspect(parsedQuery)

return &previewResponse{
Query: parsedQuery.String(),
Total: int(total),
SampleIDs: sampleIDs,
Metadata: inspection,
}, http.StatusOK, nil
return &previewResponse{Query: parsedQuery.String(), Total: int(total)}, http.StatusOK, nil
}
113 changes: 13 additions & 100 deletions web/flow/testdata/preview_start.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"body": {},
"status": 400,
"response": {
"error": "request failed validation: field 'org_id' is required, field 'flow_id' is required, field 'sample_size' is required"
"error": "request failed validation: field 'org_id' is required, field 'flow_id' is required"
}
},
{
Expand All @@ -25,14 +25,12 @@
"body": {
"org_id": 1,
"flow_id": 10001,
"include": {},
"sample_size": 3
"include": {}
},
"status": 200,
"response": {
"query": "",
"total": 0,
"sample_ids": []
"total": 0
}
},
{
Expand All @@ -52,37 +50,12 @@
"bd2aab59-5e28-4db4-b6e8-bbdb75fd7a0a"
],
"query": ""
},
"sample_size": 3
}
},
"status": 200,
"response": {
"query": "group = \"Doctors\" OR group = \"Testers\" OR uuid = \"5a8345c1-514a-4d1b-aee5-6f39b2f53cfa\" OR uuid = \"bd2aab59-5e28-4db4-b6e8-bbdb75fd7a0a\"",
"total": 121,
"sample_ids": [
10123,
10122,
10121
],
"metadata": {
"attributes": [
"group",
"uuid"
],
"schemes": [],
"fields": [],
"groups": [
{
"uuid": "c153e265-f7c9-4539-9dbc-9b358714b638",
"name": "Doctors"
},
{
"uuid": "5e9d8fab-5e7e-4f51-b533-261af5dea70d",
"name": "Testers"
}
],
"allow_as_group": false
}
"total": 121
}
},
{
Expand All @@ -96,26 +69,12 @@
"group_ids": [],
"contact_ids": [],
"query": "gender = M"
},
"sample_size": 3
}
},
"status": 200,
"response": {
"query": "gender = \"M\"",
"total": 0,
"sample_ids": [],
"metadata": {
"attributes": [],
"schemes": [],
"fields": [
{
"key": "gender",
"name": "Gender"
}
],
"groups": [],
"allow_as_group": true
}
"total": 0
}
},
{
Expand All @@ -141,37 +100,12 @@
"in_a_flow": true,
"started_previously": true,
"not_seen_since_days": 90
},
"sample_size": 3
}
},
"status": 200,
"response": {
"query": "(group = \"Doctors\" OR group = \"Testers\" OR uuid = \"5a8345c1-514a-4d1b-aee5-6f39b2f53cfa\" OR uuid = \"bd2aab59-5e28-4db4-b6e8-bbdb75fd7a0a\") AND status = \"active\" AND flow = \"\" AND history != \"Pick a Number\" AND last_seen_on > \"07-04-2018\"",
"total": 0,
"sample_ids": [],
"metadata": {
"attributes": [
"flow",
"group",
"history",
"last_seen_on",
"status",
"uuid"
],
"schemes": [],
"fields": [],
"groups": [
{
"uuid": "c153e265-f7c9-4539-9dbc-9b358714b638",
"name": "Doctors"
},
{
"uuid": "5e9d8fab-5e7e-4f51-b533-261af5dea70d",
"name": "Testers"
}
],
"allow_as_group": false
}
"total": 0
}
},
{
Expand All @@ -189,31 +123,12 @@
"in_a_flow": true,
"started_previously": true,
"not_seen_since_days": 90
},
"sample_size": 3
}
},
"status": 200,
"response": {
"query": "gender = \"M\" AND status = \"active\" AND flow = \"\" AND history != \"Pick a Number\" AND last_seen_on > \"07-04-2018\"",
"total": 0,
"sample_ids": [],
"metadata": {
"attributes": [
"flow",
"history",
"last_seen_on",
"status"
],
"schemes": [],
"fields": [
{
"key": "gender",
"name": "Gender"
}
],
"groups": [],
"allow_as_group": false
}
"total": 0
}
},
{
Expand All @@ -226,8 +141,7 @@
"include": {
"query": "gender ="
},
"exclude": {},
"sample_size": 3
"exclude": {}
},
"status": 400,
"response": {
Expand All @@ -248,8 +162,7 @@
"include": {
"query": "goats > 10"
},
"exclude": {},
"sample_size": 3
"exclude": {}
},
"status": 400,
"response": {
Expand Down
Loading

0 comments on commit e39a74c

Please sign in to comment.