Allow scheduling same query more than once in UI (#96)

Fixes #88
fleetdm · Dec 4, 2020 · bb921bc · bb921bc
1 parent 149b4ed
commit bb921bc
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 4 deletions.
diff --git a/...onents/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.jsx b/...onents/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.jsx
@@ -76,7 +76,7 @@ class ScheduledQueriesListItem extends Component {
 
   render () {
     const { checked, disabled, isSelected, scheduledQuery } = this.props;
-    const { id, name, interval, shard, version } = scheduledQuery;
+    const { id, query_name: name, interval, shard, version } = scheduledQuery;
     const { loggingTypeString, onDblClick, onCheck, onSelect, renderPlatformIcon } = this;
     const rowClassname = classnames(baseClass, {
       [`${baseClass}--selected`]: isSelected,

diff --git a/.../queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.tests.jsx b/.../queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.tests.jsx
@@ -15,7 +15,7 @@ const defaultProps = {
 describe('ScheduledQueriesListItem - component', () => {
   it('renders the scheduled query data', () => {
     const component = mount(<ScheduledQueriesListItem {...defaultProps} />);
-    expect(component.text()).toContain(scheduledQueryStub.name);
+    expect(component.text()).toContain(scheduledQueryStub.query_name);
     expect(component.text()).toContain(scheduledQueryStub.interval);
     expect(component.text()).toContain(scheduledQueryStub.shard);
     expect(component.find('PlatformIcon').length).toEqual(1);
@@ -24,7 +24,7 @@ describe('ScheduledQueriesListItem - component', () => {
   it('renders when the platform attribute is null', () => {
     const scheduledQuery = { ...scheduledQueryStub, platform: null };
     const component = mount(<ScheduledQueriesListItem checked={false} scheduledQuery={scheduledQuery} {...defaultProps} />);
-    expect(component.text()).toContain(scheduledQueryStub.name);
+    expect(component.text()).toContain(scheduledQueryStub.query_name);
     expect(component.text()).toContain(scheduledQueryStub.interval);
     expect(component.text()).toContain(scheduledQueryStub.shard);
     expect(component.find('PlatformIcon').length).toEqual(1);

diff --git a/frontend/test/stubs.js b/frontend/test/stubs.js
@@ -140,6 +140,7 @@ export const scheduledQueryStub = {
   id: 1,
   interval: 60,
   name: 'Get all users',
+  query_name: 'users',
   pack_id: 123,
   platform: 'darwin',
   query: 'SELECT * FROM users',

diff --git a/server/service/service_scheduled_queries.go b/server/service/service_scheduled_queries.go
@@ -23,12 +23,35 @@ func (svc service) ScheduleQuery(ctx context.Context, sq *kolide.ScheduledQuery)
 		if err != nil {
 			return nil, errors.Wrap(err, "lookup name for query")
 		}
-		sq.Name = query.Name
+
+		packQueries, err := svc.ds.ListScheduledQueriesInPack(sq.PackID, kolide.ListOptions{})
+		if err != nil {
+			return nil, errors.Wrap(err, "find existing scheduled queries")
+		}
+		_ = packQueries
+
+		sq.Name = findNextNameForQuery(query.Name, packQueries)
+		sq.QueryName = query.Name
+	} else if sq.QueryName == "" {
+		query, err := svc.ds.Query(sq.QueryID)
+		if err != nil {
+			return nil, errors.Wrap(err, "lookup name for query")
+		}
 		sq.QueryName = query.Name
 	}
 	return svc.ds.NewScheduledQuery(sq)
 }
 
+// Add "-1" suffixes to the query name until it is unique
+func findNextNameForQuery(name string, scheduled []*kolide.ScheduledQuery) string {
+	for _, q := range scheduled {
+		if name == q.Name {
+			return findNextNameForQuery(name+"-1", scheduled)
+		}
+	}
+	return name
+}
+
 func (svc service) ModifyScheduledQuery(ctx context.Context, id uint, p kolide.ScheduledQueryPayload) (*kolide.ScheduledQuery, error) {
 	sq, err := svc.GetScheduledQuery(ctx, id)
 	if err != nil {

diff --git a/server/service/service_scheduled_queries_test.go b/server/service/service_scheduled_queries_test.go
@@ -0,0 +1,144 @@
+package service
+
+import (
+	"context"
+	"testing"
+
+	"github.com/fleetdm/fleet/server/kolide"
+	"github.com/fleetdm/fleet/server/mock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestScheduleQuery(t *testing.T) {
+	ds := new(mock.Store)
+	svc, err := newTestService(ds, nil, nil)
+	require.Nil(t, err)
+
+	expectedQuery := &kolide.ScheduledQuery{
+		Name:      "foobar",
+		QueryName: "foobar",
+		QueryID:   3,
+	}
+
+	ds.NewScheduledQueryFunc = func(q *kolide.ScheduledQuery, opts ...kolide.OptionalArg) (*kolide.ScheduledQuery, error) {
+		assert.Equal(t, expectedQuery, q)
+		return expectedQuery, nil
+	}
+
+	_, err = svc.ScheduleQuery(context.Background(), expectedQuery)
+	assert.NoError(t, err)
+	assert.True(t, ds.NewScheduledQueryFuncInvoked)
+}
+
+func TestScheduleQueryNoName(t *testing.T) {
+	ds := new(mock.Store)
+	svc, err := newTestService(ds, nil, nil)
+	require.Nil(t, err)
+
+	expectedQuery := &kolide.ScheduledQuery{
+		Name:      "foobar",
+		QueryName: "foobar",
+		QueryID:   3,
+	}
+
+	ds.QueryFunc = func(qid uint) (*kolide.Query, error) {
+		require.Equal(t, expectedQuery.QueryID, qid)
+		return &kolide.Query{Name: expectedQuery.QueryName}, nil
+	}
+	ds.ListScheduledQueriesInPackFunc = func(id uint, opts kolide.ListOptions) ([]*kolide.ScheduledQuery, error) {
+		// No matching query
+		return []*kolide.ScheduledQuery{
+			&kolide.ScheduledQuery{
+				Name: "froobling",
+			},
+		}, nil
+	}
+	ds.NewScheduledQueryFunc = func(q *kolide.ScheduledQuery, opts ...kolide.OptionalArg) (*kolide.ScheduledQuery, error) {
+		assert.Equal(t, expectedQuery, q)
+		return expectedQuery, nil
+	}
+
+	_, err = svc.ScheduleQuery(
+		context.Background(),
+		&kolide.ScheduledQuery{QueryID: expectedQuery.QueryID},
+	)
+	assert.NoError(t, err)
+	assert.True(t, ds.NewScheduledQueryFuncInvoked)
+}
+
+func TestScheduleQueryNoNameMultiple(t *testing.T) {
+	ds := new(mock.Store)
+	svc, err := newTestService(ds, nil, nil)
+	require.Nil(t, err)
+
+	expectedQuery := &kolide.ScheduledQuery{
+		Name:      "foobar-1",
+		QueryName: "foobar",
+		QueryID:   3,
+	}
+
+	ds.QueryFunc = func(qid uint) (*kolide.Query, error) {
+		require.Equal(t, expectedQuery.QueryID, qid)
+		return &kolide.Query{Name: expectedQuery.QueryName}, nil
+	}
+	ds.ListScheduledQueriesInPackFunc = func(id uint, opts kolide.ListOptions) ([]*kolide.ScheduledQuery, error) {
+		// No matching query
+		return []*kolide.ScheduledQuery{
+			&kolide.ScheduledQuery{
+				Name: "foobar",
+			},
+		}, nil
+	}
+	ds.NewScheduledQueryFunc = func(q *kolide.ScheduledQuery, opts ...kolide.OptionalArg) (*kolide.ScheduledQuery, error) {
+		assert.Equal(t, expectedQuery, q)
+		return expectedQuery, nil
+	}
+
+	_, err = svc.ScheduleQuery(
+		context.Background(),
+		&kolide.ScheduledQuery{QueryID: expectedQuery.QueryID},
+	)
+	assert.NoError(t, err)
+	assert.True(t, ds.NewScheduledQueryFuncInvoked)
+}
+
+func TestFindNextNameForQuery(t *testing.T) {
+	var testCases = []struct {
+		name      string
+		scheduled []*kolide.ScheduledQuery
+		expected  string
+	}{
+		{
+			name:      "foobar",
+			scheduled: []*kolide.ScheduledQuery{},
+			expected:  "foobar",
+		},
+		{
+			name: "foobar",
+			scheduled: []*kolide.ScheduledQuery{
+				{
+					Name: "foobar",
+				},
+			},
+			expected: "foobar-1",
+		}, {
+			name: "foobar",
+			scheduled: []*kolide.ScheduledQuery{
+				{
+					Name: "foobar",
+				},
+				{
+					Name: "foobar-1",
+				},
+			},
+			expected: "foobar-1-1",
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, tt.expected, findNextNameForQuery(tt.name, tt.scheduled))
+		})
+	}
+}