From c8c0b28e62a3272ba4ddcdd952b00ac1b3cb7b19 Mon Sep 17 00:00:00 2001 From: briandennis Date: Tue, 7 Aug 2018 08:33:27 -0500 Subject: [PATCH 01/25] set update scheduled query payload correctly --- app/actions/sessions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/actions/sessions.js b/app/actions/sessions.js index 8bce3ce06..9ccd938e5 100644 --- a/app/actions/sessions.js +++ b/app/actions/sessions.js @@ -203,7 +203,7 @@ export function updateScheduledQuery(connectionId, payload = {}) { if (!res.error) { dispatch({ type: 'UPDATE_SCHEDULED_QUERY', - payload: body + payload: res }); } return res; From 50ce5728c559f022d24435086ecc2d292b22537e Mon Sep 17 00:00:00 2001 From: briandennis Date: Tue, 7 Aug 2018 09:12:35 -0500 Subject: [PATCH 02/25] update backend to correctly set names and add/remove columns --- backend/persistent/QueryScheduler.js | 30 ++++++--- backend/persistent/plotly-api.js | 93 ++++++++++++++++++++-------- backend/routes.js | 17 ++++- 3 files changed, 102 insertions(+), 38 deletions(-) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 1313a6f6c..677305085 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -20,9 +20,12 @@ import { getGridMeta, newGrid, patchGrid, - updateGrid + updateGrid, + getGrid } from './plotly-api.js'; +const SUCCESS_CODES = [200, 201, 204]; + class QueryScheduler { constructor() { this.scheduleQuery = this.scheduleQuery.bind(this); @@ -281,8 +284,7 @@ class QueryScheduler { */ throw new Error(`QueryExecutionError: ${e.message}`); }); - }).then(({rows}) => { - + }).then(({rows, columnnames}) => { Logger.log(`Query "${query}" took ${process.hrtime(startTime)[0]} seconds`, 2); Logger.log(`Updating grid ${fid} with new data`, 2); Logger.log( @@ -294,6 +296,7 @@ class QueryScheduler { return updateGrid( rows, + columnnames, fid, uids, requestor @@ -305,7 +308,7 @@ class QueryScheduler { }); }).then(res => { Logger.log(`Request to Plotly for grid ${fid} took ${process.hrtime(startTime)[0]} seconds`, 2); - if (res.status !== 200) { + if (!SUCCESS_CODES.includes(res.status)) { Logger.log(`Error ${res.status} while updating grid ${fid}.`, 2); /* @@ -357,6 +360,11 @@ class QueryScheduler { this.clearQuery(fid); return deleteQuery(fid); } + /* + * Warning: The front end looks for "PlotlyApiError" in this error message. Don't change it! + */ + throw new Error(`PlotlyApiError: non 200 grid update response code (got: ${res.status})`); + }); }); @@ -381,11 +389,17 @@ class QueryScheduler { */ throw new Error(`MetadataError: ${e.message}`); }); - }).then((res) => { + }).then(() => { Logger.log(`Request to Plotly for creating a grid took ${process.hrtime(startTime)[0]} seconds`, 2); - return res.json().then(() => { - Logger.log(`Grid ${fid} has been updated.`, 2); - }); + Logger.log(`Grid ${fid} has been updated.`, 2); + + startTime = process.hrtime(); + + // fetch updated grid for returning + return getGrid(fid, requestor); + }).then((res) => { + Logger.log(`Request to Plotly for fetching updated grid took ${process.hrtime(startTime)[0]} seconds`, 2); + return res.json(); }); } } diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index c7dc76317..f2f5702b8 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -164,36 +164,75 @@ export function patchGrid(fid, requestor, body) { }); } -export function updateGrid(rows, fid, uids, requestor) { - const {username, apiKey, accessToken} = getCredentials(requestor); +export function updateGrid(rows, columnnames, fid, uids = [], requestor) { + const numColumns = columnnames.length; + const columns = getColumns(rows, numColumns); + const columnEntries = columns.map((column, columnIndex) => { + return { data: column, name: columnnames[columnIndex], order: columnIndex }; + }); - // TODO - Test case where no rows are returned. - if (uids.length !== (rows[0] || []).length) { - Logger.log(` - A different number of columns was returned in the - query than what was initially saved in the grid. - ${rows[0].length} columns were queried, - ${uids.length} columns were originally saved. - The connector does not create columns (yet), - and so we will only update the first ${uids.length} - columns. - `); + const {username, apiKey, accessToken} = getCredentials(requestor); + const baseUrl = `grids/${fid}/col`; + const baseParams = { username, apiKey, accessToken }; + + if (numColumns > uids.length) { + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.join(',')}`; + const putBody = { cols: columnEntries.slice(0, uids.length) }; + + // append new columns + const postUrl = baseUrl; + const postBody = { cols: columnEntries.slice(uids.length) }; + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }).then((res) => { + if (res.status !== 200) { + return res; + } + + return plotlyAPIRequest(postUrl, { + ...baseParams, + body: postBody, + method: 'POST' + }); + }); + } else if (numColumns < uids.length) { + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.slice(0, numColumns).join(',')}`; + const putBody = { cols: columnEntries }; + + // delete unused existing columns + const deleteUrl = `${baseUrl}?uid=${uids.slice(numColumns)}`; + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }).then((res) => { + if (res.status !== 200) { + return res; + } + + return plotlyAPIRequest(deleteUrl, { + ...baseParams, + method: 'DELETE' + }); + }); } - /* - * For now, only update up to the number of columns that are - * already saved. In the future, we should just create more - * columns. See error message above. - */ - const columns = getColumns(rows, uids.length); - - const url = `grids/${fid}/col?uid=${uids.join(',')}`; - const body = { - cols: JSON.stringify(columns.map(column => ({ - data: column - }))) - }; - return plotlyAPIRequest(url, {body, username, apiKey, accessToken, method: 'PUT'}); + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.join(',')}`; + const putBody = { cols: columnEntries }; + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }); + } function getColumns(rows, maxColumns) { diff --git a/backend/routes.js b/backend/routes.js index 5596084c4..895449d76 100644 --- a/backend/routes.js +++ b/backend/routes.js @@ -703,7 +703,18 @@ export default class Servers { fid, uids, query, connectionId, requestor, cronInterval, refreshInterval ); }) - .then(() => { + .then((updatedGridResponse) => { + const orderedUids = + Object.keys(updatedGridResponse.cols) + .map(key => updatedGridResponse.cols[key]) + .sort((a, b) => a.order - b.order) + .map(col => col.uid); + + const queryObject = { + ...req.params, + uids: orderedUids + }; + let status; if (getQuery(req.params.fid)) { // TODO - Technically, this should be @@ -712,8 +723,8 @@ export default class Servers { } else { status = 201; } - that.queryScheduler.scheduleQuery(req.params); - res.json(status, {}); + that.queryScheduler.scheduleQuery(queryObject); + res.json(status, queryObject); return next(); }) .catch(onError); From df31eecd990d3079dc8a9ad45f07a0054ef2d0ec Mon Sep 17 00:00:00 2001 From: briandennis Date: Tue, 7 Aug 2018 09:13:46 -0500 Subject: [PATCH 03/25] fix updateGrid signature in tests --- test/backend/QueryScheduler.spec.js | 3 ++- test/backend/plotly-api.spec.js | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index 14030a643..7e46fe3e0 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -622,7 +622,8 @@ describe('QueryScheduler', function() { } function resetAndVerifyGridContents(fid, uids) { - return updateGrid([[1, 2, 3, 4, 5, 6]], fid, uids, username, apiKey) + const placeholderColNames = Array(uids.length).fill('_'); + return updateGrid([[1, 2, 3, 4, 5, 6]], placeholderColNames, fid, uids, username, apiKey) .then(assertResponseStatus(200)).then(() => { return getGrid(fid, username); }) diff --git a/test/backend/plotly-api.spec.js b/test/backend/plotly-api.spec.js index eb14db8b9..012831cba 100644 --- a/test/backend/plotly-api.spec.js +++ b/test/backend/plotly-api.spec.js @@ -39,6 +39,7 @@ describe('Grid API Functions', function () { ['y', 20, 50, 80, 110, 140], ['z', 30, 60, 90, 120, 150] ], + Array(3).fill('_'), // placeholder column names fid, uids, username From 81381f27923ea218116aeffcc91590dfe298b49c Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Tue, 7 Aug 2018 13:01:06 -0700 Subject: [PATCH 04/25] update scheduler tests for new updateGrid API --- test/backend/QueryScheduler.spec.js | 3 +-- test/backend/plotly-api.spec.js | 16 ++++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index 7e46fe3e0..6f6d65ed1 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -622,8 +622,7 @@ describe('QueryScheduler', function() { } function resetAndVerifyGridContents(fid, uids) { - const placeholderColNames = Array(uids.length).fill('_'); - return updateGrid([[1, 2, 3, 4, 5, 6]], placeholderColNames, fid, uids, username, apiKey) + return updateGrid([[1, 2, 3, 4, 5, 6]], names, fid, uids, username, apiKey) .then(assertResponseStatus(200)).then(() => { return getGrid(fid, username); }) diff --git a/test/backend/plotly-api.spec.js b/test/backend/plotly-api.spec.js index 012831cba..c2e720247 100644 --- a/test/backend/plotly-api.spec.js +++ b/test/backend/plotly-api.spec.js @@ -11,7 +11,6 @@ import { assertResponseStatus, getResponseJson, initGrid, - names, username } from './utils.js'; @@ -28,6 +27,7 @@ describe('Grid API Functions', function () { // it works off of the assumption that a grid exists let fid; + const newColumnNames = Array(6).fill('_').map((v, i) => String(i)); // placeholder column names return initGrid('Test updateGrid') .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { @@ -39,7 +39,7 @@ describe('Grid API Functions', function () { ['y', 20, 50, 80, 110, 140], ['z', 30, 60, 90, 120, 150] ], - Array(3).fill('_'), // placeholder column names + newColumnNames, // placeholder column names fid, uids, username @@ -52,27 +52,27 @@ describe('Grid API Functions', function () { .then(assertResponseStatus(200)) .then(getResponseJson).then(json => { assert.deepEqual( - json.cols[names[0]].data, + json.cols[newColumnNames[0]].data, ['x', 'y', 'z'] ); assert.deepEqual( - json.cols[names[1]].data, + json.cols[newColumnNames[1]].data, [10, 20, 30] ); assert.deepEqual( - json.cols[names[2]].data, + json.cols[newColumnNames[2]].data, [40, 50, 60] ); assert.deepEqual( - json.cols[names[3]].data, + json.cols[newColumnNames[3]].data, [70, 80, 90] ); assert.deepEqual( - json.cols[names[4]].data, + json.cols[newColumnNames[4]].data, [100, 110, 120] ); assert.deepEqual( - json.cols[names[5]].data, + json.cols[newColumnNames[5]].data, [130, 140, 150] ); }) From 9c014ce8cea03ab2c08c42c8974950123d23f845 Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Tue, 7 Aug 2018 19:01:12 -0700 Subject: [PATCH 05/25] fix updateGrid to GET source or truth --- backend/persistent/plotly-api.js | 125 +++++++++++++++------------- test/backend/routes.queries.spec.js | 76 ++++++++++------- 2 files changed, 113 insertions(+), 88 deletions(-) diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index f2f5702b8..736cda587 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -164,7 +164,7 @@ export function patchGrid(fid, requestor, body) { }); } -export function updateGrid(rows, columnnames, fid, uids = [], requestor) { +export function updateGrid(rows, columnnames, fid, _unusedUIDs, requestor) { const numColumns = columnnames.length; const columns = getColumns(rows, numColumns); const columnEntries = columns.map((column, columnIndex) => { @@ -175,64 +175,73 @@ export function updateGrid(rows, columnnames, fid, uids = [], requestor) { const baseUrl = `grids/${fid}/col`; const baseParams = { username, apiKey, accessToken }; - if (numColumns > uids.length) { - // repopulate existing columns - const putUrl = `${baseUrl}?uid=${uids.join(',')}`; - const putBody = { cols: columnEntries.slice(0, uids.length) }; - - // append new columns - const postUrl = baseUrl; - const postBody = { cols: columnEntries.slice(uids.length) }; - - return plotlyAPIRequest(putUrl, { - ...baseParams, - body: putBody, - method: 'PUT' - }).then((res) => { - if (res.status !== 200) { - return res; - } - - return plotlyAPIRequest(postUrl, { - ...baseParams, - body: postBody, - method: 'POST' - }); - }); - } else if (numColumns < uids.length) { - // repopulate existing columns - const putUrl = `${baseUrl}?uid=${uids.slice(0, numColumns).join(',')}`; - const putBody = { cols: columnEntries }; - - // delete unused existing columns - const deleteUrl = `${baseUrl}?uid=${uids.slice(numColumns)}`; - - return plotlyAPIRequest(putUrl, { - ...baseParams, - body: putBody, - method: 'PUT' - }).then((res) => { - if (res.status !== 200) { - return res; - } - - return plotlyAPIRequest(deleteUrl, { - ...baseParams, - method: 'DELETE' - }); - }); - } - - // repopulate existing columns - const putUrl = `${baseUrl}?uid=${uids.join(',')}`; - const putBody = { cols: columnEntries }; - - return plotlyAPIRequest(putUrl, { - ...baseParams, - body: putBody, - method: 'PUT' + // Fetch latest Grid to get the source of truth + return getGrid(fid, requestor) + .then(res => res.json()) + .then(data => { + const uids = Object.keys(data.cols) + .map(key => data.cols[key]) + .sort((a, b) => a.order - b.order) + .map(col => col.uid); + + if (numColumns > uids.length) { + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.join(',')}`; + const putBody = { cols: columnEntries.slice(0, uids.length) }; + + // append new columns + const postUrl = baseUrl; + const postBody = { cols: columnEntries.slice(uids.length) }; + + return plotlyAPIRequest(postUrl, { + ...baseParams, + body: postBody, + method: 'POST' + }).then((res) => { + if (res.status !== 200) { + return res; + } + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }); + }); + } else if (numColumns < uids.length) { + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.slice(0, numColumns).join(',')}`; + const putBody = { cols: columnEntries }; + + // delete unused existing columns + const deleteUrl = `${baseUrl}?uid=${uids.slice(numColumns)}`; + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }).then((res) => { + if (res.status !== 200) { + return res; + } + + return plotlyAPIRequest(deleteUrl, { + ...baseParams, + method: 'DELETE' + }); + }); + } + + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.join(',')}`; + const putBody = { cols: columnEntries }; + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }); }); - } function getColumns(rows, maxColumns) { diff --git a/test/backend/routes.queries.spec.js b/test/backend/routes.queries.spec.js index 5f8683196..f32b4b2c1 100644 --- a/test/backend/routes.queries.spec.js +++ b/test/backend/routes.queries.spec.js @@ -1,5 +1,5 @@ import {assert} from 'chai'; -import {merge} from 'ramda'; +import {merge, omit} from 'ramda'; import uuid from 'uuid'; import {saveConnection} from '../../backend/persistent/Connections.js'; @@ -114,18 +114,18 @@ describe('Routes:', () => { query: 'SELECT * from ebola_2014 LIMIT 2' }; - return POST('queries', queryObject); - }) - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); + return POST('queries', queryObject) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json2 => { + assert.deepEqual(json2, queryObject); - return GET('queries'); - }) - .then(getResponseJson).then(json => { - assert.deepEqual(json, [queryObject]); + return GET('queries'); + }) + .then(getResponseJson).then(json3 => { + assert.deepEqual(json3, [queryObject]); - return deleteGrid(fid, username); + return deleteGrid(fid, username); + }); }); }); @@ -158,12 +158,13 @@ describe('Routes:', () => { return POST('queries', queryObject) .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); - return GET('queries'); - }) - .then(getResponseJson).then(json => { - assert.deepEqual(json, [queryObject]); + .then(getResponseJson).then((json) => { + assert.deepEqual(omit('uids', json), omit('uids', queryObject)); + assert.equal(json.uids.length, 6); + return GET('queries').then(getResponseJson) + .then((getResponse) => { + assert.deepEqual(getResponse, [json]); + }); }); }); @@ -236,19 +237,34 @@ describe('Routes:', () => { }); it('gets individual queries', function() { - return GET(`queries/${queryObject.fid}`) - .then(assertResponseStatus(404)).then(() => { - return POST('queries', queryObject); - }) - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, {}); - return GET(`queries/${queryObject.fid}`); - }) - .then(assertResponseStatus(200)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, queryObject); - }); + // uids are hardcoded + const uids = [ + '9bbb87', + '8d4dd0', + '3e40ef', + 'b169c0', + '55e326', + 'e69f70' + ]; + return GET(`queries/${queryObject.fid}`) + .then(assertResponseStatus(404)).then(() => { + return POST('queries', queryObject); + }) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, { + ...queryObject, + uids + }); + return GET(`queries/${queryObject.fid}`); + }) + .then(assertResponseStatus(200)) + .then(getResponseJson).then(json => { + assert.deepEqual(json, { + ...queryObject, + uids + }); + }); }); it('deletes queries', function() { From 5d3b55845c2092e1ec0a36e9d8ad65483751a9fe Mon Sep 17 00:00:00 2001 From: briandennis Date: Wed, 8 Aug 2018 03:53:25 -0400 Subject: [PATCH 06/25] if GET is not 200 in updateGrid, return the response --- backend/persistent/plotly-api.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index 736cda587..b38a0e460 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -175,10 +175,19 @@ export function updateGrid(rows, columnnames, fid, _unusedUIDs, requestor) { const baseUrl = `grids/${fid}/col`; const baseParams = { username, apiKey, accessToken }; - // Fetch latest Grid to get the source of truth + // fetch latest grid to get the source of truth return getGrid(fid, requestor) - .then(res => res.json()) - .then(data => { + .then(res => { + if (res.status !== 200) { + return res; + } + return res.json(); + }).then(data => { + if (data.status) { + // bad res was passed along, return it + return data; + } + const uids = Object.keys(data.cols) .map(key => data.cols[key]) .sort((a, b) => a.order - b.order) From dbdd6547b39e6ee34f09f496954ec370b60c146d Mon Sep 17 00:00:00 2001 From: briandennis Date: Wed, 8 Aug 2018 03:54:07 -0400 Subject: [PATCH 07/25] omit unnecessary info in tests --- test/backend/QueryScheduler.spec.js | 11 +++++--- test/backend/routes.queries.spec.js | 41 +++++++++-------------------- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index 6f6d65ed1..49cf4467e 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -1,7 +1,7 @@ import {assert} from 'chai'; import sinon from 'sinon'; -import {merge} from 'ramda'; +import {merge, omit} from 'ramda'; import {saveConnection} from '../../backend/persistent/Connections.js'; import { @@ -528,7 +528,7 @@ describe('QueryScheduler', function() { it('clears and deletes the query if its associated grid was deleted', function() { const refreshInterval = 5; - const cronInterval = `*/${refreshInterval} * * * * *`; + const cronInterval = `*/${refreshInterval} * * * * *`; // every 5 seconds /* * Save the sqlConnections to a file. @@ -550,7 +550,6 @@ describe('QueryScheduler', function() { queryObject = { fid, uids, - refreshInterval, cronInterval, connectionId, query: 'SELECT * from ebola_2014 LIMIT 2', @@ -562,7 +561,11 @@ describe('QueryScheduler', function() { queryScheduler.scheduleQuery(queryObject); - assert.deepEqual(getQueries(), [queryObject], 'Query has not been saved'); + assert.deepEqual( + getQueries().map(query => omit('refreshInterval', query)), + [queryObject], + 'Query has not been saved' + ); assert(Boolean(queryScheduler.queryJobs[fid]), 'Query has not been scheduled'); return deleteGrid(fid, username); diff --git a/test/backend/routes.queries.spec.js b/test/backend/routes.queries.spec.js index f32b4b2c1..d97fe3e92 100644 --- a/test/backend/routes.queries.spec.js +++ b/test/backend/routes.queries.spec.js @@ -237,34 +237,19 @@ describe('Routes:', () => { }); it('gets individual queries', function() { - // uids are hardcoded - const uids = [ - '9bbb87', - '8d4dd0', - '3e40ef', - 'b169c0', - '55e326', - 'e69f70' - ]; - return GET(`queries/${queryObject.fid}`) - .then(assertResponseStatus(404)).then(() => { - return POST('queries', queryObject); - }) - .then(assertResponseStatus(201)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, { - ...queryObject, - uids - }); - return GET(`queries/${queryObject.fid}`); - }) - .then(assertResponseStatus(200)) - .then(getResponseJson).then(json => { - assert.deepEqual(json, { - ...queryObject, - uids - }); - }); + return GET(`queries/${queryObject.fid}`) + .then(assertResponseStatus(404)).then(() => { + return POST('queries', queryObject); + }) + .then(assertResponseStatus(201)) + .then(getResponseJson).then(json => { + assert.deepEqual(omit('uids', json), omit('uids', queryObject)); + return GET(`queries/${queryObject.fid}`); + }) + .then(assertResponseStatus(200)) + .then(getResponseJson).then(json => { + assert.deepEqual(omit('uids', json), omit('uids', queryObject)); + }); }); it('deletes queries', function() { From 51ea8ccdf8cdd3aa7fb6b6c35e2d96e533149f55 Mon Sep 17 00:00:00 2001 From: briandennis Date: Wed, 8 Aug 2018 05:58:10 -0400 Subject: [PATCH 08/25] remove uid parameters where possible, maintain falcon specific query params if updated from Chart Studio --- backend/persistent/QueryScheduler.js | 7 +++---- backend/persistent/plotly-api.js | 8 +++---- backend/routes.js | 31 ++++++++++++++++------------ backend/utils/gridUtils.js | 10 +++++++++ test/backend/QueryScheduler.spec.js | 6 +++--- test/backend/plotly-api.spec.js | 2 -- 6 files changed, 37 insertions(+), 27 deletions(-) create mode 100644 backend/utils/gridUtils.js diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 677305085..9ba25010e 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -37,7 +37,7 @@ class QueryScheduler { // this.job wraps this.queryAndUpdateGrid to avoid concurrent runs of the same job this.runningJobs = {}; - this.job = (fid, uids, query, connectionId, requestor) => { + this.job = (fid, query, connectionId, requestor) => { try { if (this.runningJobs[fid]) { return; @@ -45,7 +45,7 @@ class QueryScheduler { this.runningJobs[fid] = true; - return this.queryAndUpdateGrid(fid, uids, query, connectionId, requestor) + return this.queryAndUpdateGrid(fid, query, connectionId, requestor) .catch(error => { Logger.log(error, 0); }).then(() => { @@ -239,7 +239,7 @@ class QueryScheduler { }); } - queryAndUpdateGrid(fid, uids, query, connectionId, requestor, cronInterval, refreshInterval) { + queryAndUpdateGrid(fid, query, connectionId, requestor, cronInterval, refreshInterval) { const requestedDBConnections = getConnectionById(connectionId); const formattedRefresh = refreshInterval || mapCronToRefresh(cronInterval); let startTime = process.hrtime(); @@ -298,7 +298,6 @@ class QueryScheduler { rows, columnnames, fid, - uids, requestor ).catch((e) => { /* diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index b38a0e460..a079fb5d0 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -6,6 +6,7 @@ import { getCredentials, getSetting } from '../settings.js'; +import {extractOrderedUids} from '../utils/gridUtils.js'; // Module to access Plot.ly REST API @@ -164,7 +165,7 @@ export function patchGrid(fid, requestor, body) { }); } -export function updateGrid(rows, columnnames, fid, _unusedUIDs, requestor) { +export function updateGrid(rows, columnnames, fid, requestor) { const numColumns = columnnames.length; const columns = getColumns(rows, numColumns); const columnEntries = columns.map((column, columnIndex) => { @@ -188,10 +189,7 @@ export function updateGrid(rows, columnnames, fid, _unusedUIDs, requestor) { return data; } - const uids = Object.keys(data.cols) - .map(key => data.cols[key]) - .sort((a, b) => a.order - b.order) - .map(col => col.uid); + const uids = extractOrderedUids(data); if (numColumns > uids.length) { // repopulate existing columns diff --git a/backend/routes.js b/backend/routes.js index 895449d76..17c64e5f2 100644 --- a/backend/routes.js +++ b/backend/routes.js @@ -8,6 +8,7 @@ import path from 'path'; import {PlotlyOAuth} from './plugins/authorization.js'; import {generateAndSaveAccessToken} from './utils/authUtils.js'; +import {extractOrderedUids} from './utils/gridUtils.js'; import { getAccessTokenCookieOptions, getCookieOptions, @@ -667,7 +668,6 @@ export default class Servers { const { filename, fid, - uids, query, connectionId, requestor, @@ -700,29 +700,34 @@ export default class Servers { if (fid) { return checkWritePermissions(fid, requestor).then(function () { return that.queryScheduler.queryAndUpdateGrid( - fid, uids, query, connectionId, requestor, cronInterval, refreshInterval + fid, query, connectionId, requestor, cronInterval, refreshInterval ); }) .then((updatedGridResponse) => { - const orderedUids = - Object.keys(updatedGridResponse.cols) - .map(key => updatedGridResponse.cols[key]) - .sort((a, b) => a.order - b.order) - .map(col => col.uid); - - const queryObject = { - ...req.params, - uids: orderedUids - }; + const orderedUids = extractOrderedUids(updatedGridResponse); + const previousQuery = getQuery(req.params.fid); let status; - if (getQuery(req.params.fid)) { + if (previousQuery) { // TODO - Technically, this should be // under the endpoint `/queries/:fid` status = 200; } else { status = 201; } + + let oldParamsToCarryOver = {}; + if (previousQuery) { + const {name} = previousQuery; + oldParamsToCarryOver = {name}; + } + + const queryObject = { + ...oldParamsToCarryOver, + ...req.params, + uids: orderedUids + }; + that.queryScheduler.scheduleQuery(queryObject); res.json(status, queryObject); return next(); diff --git a/backend/utils/gridUtils.js b/backend/utils/gridUtils.js new file mode 100644 index 000000000..25da243a4 --- /dev/null +++ b/backend/utils/gridUtils.js @@ -0,0 +1,10 @@ +export function extractOrderedUids (grid) { + try { + return Object.keys(grid.cols) + .map(key => grid.cols[key]) + .sort((a, b) => a.order - b.order) + .map(col => col.uid); + } catch (e) { + return null; + } +} \ No newline at end of file diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index 49cf4467e..b7acc59b3 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -624,8 +624,8 @@ describe('QueryScheduler', function() { }); } - function resetAndVerifyGridContents(fid, uids) { - return updateGrid([[1, 2, 3, 4, 5, 6]], names, fid, uids, username, apiKey) + function resetAndVerifyGridContents(fid) { + return updateGrid([[1, 2, 3, 4, 5, 6]], names, fid, username, apiKey) .then(assertResponseStatus(200)).then(() => { return getGrid(fid, username); }) @@ -673,7 +673,7 @@ describe('QueryScheduler', function() { }) .then(() => wait(1.5 * refreshInterval * 1000)) .then(() => checkGridAgainstQuery(fid, 'First check')) - .then(() => resetAndVerifyGridContents(fid, uids)) + .then(() => resetAndVerifyGridContents(fid)) .then(() => wait(1.5 * refreshInterval * 1000)) .then(() => checkGridAgainstQuery(fid, 'Second check')) .then(() => deleteGrid(fid, username)); diff --git a/test/backend/plotly-api.spec.js b/test/backend/plotly-api.spec.js index c2e720247..4c228e86b 100644 --- a/test/backend/plotly-api.spec.js +++ b/test/backend/plotly-api.spec.js @@ -32,7 +32,6 @@ describe('Grid API Functions', function () { .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { fid = json.file.fid; - const uids = json.file.cols.map(col => col.uid); return updateGrid( [ ['x', 10, 40, 70, 100, 130], @@ -41,7 +40,6 @@ describe('Grid API Functions', function () { ], newColumnNames, // placeholder column names fid, - uids, username ); }) From 61749cb09e66bb868cf16ec853e58d62ee22169e Mon Sep 17 00:00:00 2001 From: briandennis Date: Wed, 8 Aug 2018 06:53:04 -0400 Subject: [PATCH 09/25] call scheduled query job with correct parameters --- backend/persistent/QueryScheduler.js | 7 +++---- test/backend/QueryScheduler.spec.js | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 9ba25010e..7244ed0d4 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -37,15 +37,14 @@ class QueryScheduler { // this.job wraps this.queryAndUpdateGrid to avoid concurrent runs of the same job this.runningJobs = {}; - this.job = (fid, query, connectionId, requestor) => { + this.job = (fid, query, connectionId, requestor, cronInterval, refreshInterval) => { try { if (this.runningJobs[fid]) { return; } this.runningJobs[fid] = true; - - return this.queryAndUpdateGrid(fid, query, connectionId, requestor) + return this.queryAndUpdateGrid(fid, query, connectionId, requestor, cronInterval, refreshInterval) .catch(error => { Logger.log(error, 0); }).then(() => { @@ -116,7 +115,7 @@ class QueryScheduler { saveQuery(queryParams); // Schedule - const job = () => this.job(fid, uids, query, connectionId, requestor); + const job = () => this.job(fid, query, connectionId, requestor, cronInterval, refreshInterval); let jobInterval = cronInterval; if (!jobInterval) { // convert refresh interval to cron representation diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index b7acc59b3..eeedf0cdf 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -464,13 +464,13 @@ describe('QueryScheduler', function() { clock.tick(3.25 * refreshInterval * 1000); assert(spy1.calledThrice, 'job1 should have been called three times'); assert(spy1.alwaysCalledWith( - query.fid, query.uids, query.query, + query.fid, query.query, query.connectionId, query.requestor ), `job1 was called with unexpected args: ${spy1.args}`); assert(spy2.calledThrice, 'job2 should have been called three times'); assert(spy2.alwaysCalledWith( - query.fid, query.uids, 'query-2', - query.connectionId, query.requestor + query.fid, 'query-2', + query.connectionId, query.requestor, ), `job2 was called with unexpected args: ${spy2.args}`); clock.restore(); From a3037b6328dc4f1318b0d41a5e2d225968c8023f Mon Sep 17 00:00:00 2001 From: briandennis Date: Wed, 8 Aug 2018 07:40:08 -0400 Subject: [PATCH 10/25] move DELETE before PUT to minimize risk of column naming collision --- backend/persistent/plotly-api.js | 142 ++++++++++++++++--------------- 1 file changed, 72 insertions(+), 70 deletions(-) diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index a079fb5d0..1338f968f 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -178,76 +178,78 @@ export function updateGrid(rows, columnnames, fid, requestor) { // fetch latest grid to get the source of truth return getGrid(fid, requestor) - .then(res => { - if (res.status !== 200) { - return res; - } - return res.json(); - }).then(data => { - if (data.status) { - // bad res was passed along, return it - return data; - } - - const uids = extractOrderedUids(data); - - if (numColumns > uids.length) { - // repopulate existing columns - const putUrl = `${baseUrl}?uid=${uids.join(',')}`; - const putBody = { cols: columnEntries.slice(0, uids.length) }; - - // append new columns - const postUrl = baseUrl; - const postBody = { cols: columnEntries.slice(uids.length) }; - - return plotlyAPIRequest(postUrl, { - ...baseParams, - body: postBody, - method: 'POST' - }).then((res) => { - if (res.status !== 200) { - return res; - } - - return plotlyAPIRequest(putUrl, { - ...baseParams, - body: putBody, - method: 'PUT' - }); - }); - } else if (numColumns < uids.length) { - // repopulate existing columns - const putUrl = `${baseUrl}?uid=${uids.slice(0, numColumns).join(',')}`; - const putBody = { cols: columnEntries }; - - // delete unused existing columns - const deleteUrl = `${baseUrl}?uid=${uids.slice(numColumns)}`; - - return plotlyAPIRequest(putUrl, { - ...baseParams, - body: putBody, - method: 'PUT' - }).then((res) => { - if (res.status !== 200) { - return res; - } - - return plotlyAPIRequest(deleteUrl, { - ...baseParams, - method: 'DELETE' - }); - }); - } - - // repopulate existing columns - const putUrl = `${baseUrl}?uid=${uids.join(',')}`; - const putBody = { cols: columnEntries }; - - return plotlyAPIRequest(putUrl, { - ...baseParams, - body: putBody, - method: 'PUT' - }); + .then(res => { + if (res.status !== 200) { + return res; + } + return res.json(); + }).then(data => { + if (data.status) { + // bad res was passed along, return it + return data; + } + + const uids = extractOrderedUids(data); + + if (numColumns > uids.length) { + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.join(',')}`; + const putBody = { cols: columnEntries.slice(0, uids.length) }; + + // append new columns + const postUrl = baseUrl; + const postBody = { cols: columnEntries.slice(uids.length) }; + + return plotlyAPIRequest(postUrl, { + ...baseParams, + body: postBody, + method: 'POST' + }).then((res) => { + if (res.status !== 200) { + return res; + } + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }); + }); + } else if (numColumns < uids.length) { + // delete unused existing columns + const deleteUrl = `${baseUrl}?uid=${uids.slice(numColumns)}`; + + // repopulate used existing columns + const putUrl = `${baseUrl}?uid=${uids.slice(0, numColumns).join(',')}`; + const putBody = { cols: columnEntries }; + + + + return plotlyAPIRequest(deleteUrl, { + ...baseParams, + method: 'DELETE' + }).then((res) => { + if (res.status !== 204) { + return res; + } + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }); + }); + } + + // repopulate existing columns + const putUrl = `${baseUrl}?uid=${uids.join(',')}`; + const putBody = { cols: columnEntries }; + + return plotlyAPIRequest(putUrl, { + ...baseParams, + body: putBody, + method: 'PUT' + }); }); } From 02ab47c8f638399b6f45db4ec329cfee840fec79 Mon Sep 17 00:00:00 2001 From: briandennis Date: Wed, 8 Aug 2018 12:35:42 -0400 Subject: [PATCH 11/25] add uid tests for updateGrid --- test/backend/plotly-api.spec.js | 131 ++++++++++++++++++++++++++++---- 1 file changed, 118 insertions(+), 13 deletions(-) diff --git a/test/backend/plotly-api.spec.js b/test/backend/plotly-api.spec.js index 4c228e86b..ca9230889 100644 --- a/test/backend/plotly-api.spec.js +++ b/test/backend/plotly-api.spec.js @@ -13,7 +13,14 @@ import { initGrid, username } from './utils.js'; +import {extractOrderedUids} from '../../backend/utils/gridUtils.js'; +function genPlaceholderColumnNames (numColumns) { + return Array(numColumns).fill('_').map((_, index) => `placeholder-${index}`); +} + +// Note: in the following tests grids are created to begin with, but +// the actual backend never does this — it assumes a grid already exists. describe('Grid API Functions', function () { before(() => { saveSetting('USERS', [{username, apiKey}]); @@ -21,14 +28,112 @@ describe('Grid API Functions', function () { saveSetting('PLOTLY_API_SSL_ENABLED', true); }); - it('updateGrid overwrites a grid with new data', function () { - // First, create a new grid. - // Note that the app never actually does this, - // it works off of the assumption that a grid exists + it('updateGrid updates a grid but keeps the same uids', function () { + let fid, originalUids; + return initGrid('test grid') + .then(assertResponseStatus(201)) + .then(getResponseJson) + .then(json => { + originalUids = json.file.cols.map(col => col.uid); + fid = json.file.fid; + + return updateGrid( + [ + ['x', 10, 40, 70, 100, 130], + ['y', 20, 50, 80, 110, 140], + ['z', 30, 60, 90, 120, 150] + ], + genPlaceholderColumnNames(6), + fid, + username + ); + }) + .then(assertResponseStatus(200)) + .then(() => { + return getGrid(fid, username); + }) + .then(assertResponseStatus(200)) + .then(getResponseJson) + .then(json => { + const finalUids = extractOrderedUids(json); + assert.equal(finalUids.length, 6); + assert.deepEqual(originalUids, finalUids, 'original and final uids differ'); + }) + .then(() => deleteGrid(fid, username)); + }); + + it('updateGrid keeps all original uids when appending additional columns', function () { + let fid, originalUids; + return initGrid('test grid') + .then(assertResponseStatus(201)) + .then(getResponseJson) + .then(json => { + originalUids = json.file.cols.map(col => col.uid); + fid = json.file.fid; + + return updateGrid( + [ + ['x', 10, 40, 70, 100, 130, 160, 190], + ['y', 20, 50, 80, 110, 140, 170, 200], + ['z', 30, 60, 90, 120, 150, 180, 210] + ], + genPlaceholderColumnNames(8), + fid, + username + ); + }) + .then(assertResponseStatus(201)) + .then(() => { + return getGrid(fid, username); + }) + .then(assertResponseStatus(200)) + .then(getResponseJson) + .then(json => { + const finalUids = extractOrderedUids(json); + assert.equal(finalUids.length, 8); + assert.deepEqual(originalUids, finalUids.slice(0, 6), 'original and final uids differ'); + }) + .then(() => deleteGrid(fid, username)); + }); + + it('updateGrid keeps subset of original uids when deleting columns', function () { + let fid, originalUids; + return initGrid('test grid') + .then(assertResponseStatus(201)) + .then(getResponseJson) + .then(json => { + originalUids = json.file.cols.map(col => col.uid); + fid = json.file.fid; + + return updateGrid( + [ + ['x', 10, 40, 70], + ['y', 20, 50, 80], + ['z', 30, 60, 90] + ], + genPlaceholderColumnNames(4), + fid, + username + ); + }) + .then(assertResponseStatus(200)) + .then(() => { + return getGrid(fid, username); + }) + .then(assertResponseStatus(200)) + .then(getResponseJson) + .then(json => { + const finalUids = extractOrderedUids(json); + assert.equal(finalUids.length, 4); + assert.deepEqual(originalUids.slice(0, 4), finalUids, 'original and final uids differ'); + }) + .then(() => deleteGrid(fid, username)); + }); + it('updateGrid overwrites a grid with correct data and column names', function () { let fid; - const newColumnNames = Array(6).fill('_').map((v, i) => String(i)); // placeholder column names - return initGrid('Test updateGrid') + const placeholderColumnNames = genPlaceholderColumnNames(6); + return initGrid('test grid') .then(assertResponseStatus(201)) .then(getResponseJson).then(json => { fid = json.file.fid; @@ -38,7 +143,7 @@ describe('Grid API Functions', function () { ['y', 20, 50, 80, 110, 140], ['z', 30, 60, 90, 120, 150] ], - newColumnNames, // placeholder column names + placeholderColumnNames, fid, username ); @@ -50,27 +155,27 @@ describe('Grid API Functions', function () { .then(assertResponseStatus(200)) .then(getResponseJson).then(json => { assert.deepEqual( - json.cols[newColumnNames[0]].data, + json.cols[placeholderColumnNames[0]].data, ['x', 'y', 'z'] ); assert.deepEqual( - json.cols[newColumnNames[1]].data, + json.cols[placeholderColumnNames[1]].data, [10, 20, 30] ); assert.deepEqual( - json.cols[newColumnNames[2]].data, + json.cols[placeholderColumnNames[2]].data, [40, 50, 60] ); assert.deepEqual( - json.cols[newColumnNames[3]].data, + json.cols[placeholderColumnNames[3]].data, [70, 80, 90] ); assert.deepEqual( - json.cols[newColumnNames[4]].data, + json.cols[placeholderColumnNames[4]].data, [100, 110, 120] ); assert.deepEqual( - json.cols[newColumnNames[5]].data, + json.cols[placeholderColumnNames[5]].data, [130, 140, 150] ); }) From 1a2bfd56d405dc9eed8edb905bb07611c7f937aa Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Wed, 8 Aug 2018 10:11:56 -0700 Subject: [PATCH 12/25] GET /grids/:fid/col --- backend/persistent/plotly-api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index 1338f968f..a2ba2129d 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -177,7 +177,7 @@ export function updateGrid(rows, columnnames, fid, requestor) { const baseParams = { username, apiKey, accessToken }; // fetch latest grid to get the source of truth - return getGrid(fid, requestor) + return plotlyAPIRequest(baseUrl, { ...baseParams, method: 'GET' }) .then(res => { if (res.status !== 200) { return res; From dc4091249e48a10db12068abe1838900f36437a5 Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Wed, 8 Aug 2018 10:19:29 -0700 Subject: [PATCH 13/25] check for status as the end of `queryAndUpdateGrid` --- backend/persistent/QueryScheduler.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 7244ed0d4..31c45c6cc 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -397,6 +397,9 @@ class QueryScheduler { return getGrid(fid, requestor); }).then((res) => { Logger.log(`Request to Plotly for fetching updated grid took ${process.hrtime(startTime)[0]} seconds`, 2); + if (res.status !== 200) { + return res.text(); + } return res.json(); }); } From 9049db33461b1d23675236415c92813d41820862 Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Wed, 8 Aug 2018 10:46:27 -0700 Subject: [PATCH 14/25] fix syncing issue after updating query cronInterval --- app/components/Settings/scheduler/preview-modal.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/Settings/scheduler/preview-modal.jsx b/app/components/Settings/scheduler/preview-modal.jsx index 3703b704b..c1ff370d8 100644 --- a/app/components/Settings/scheduler/preview-modal.jsx +++ b/app/components/Settings/scheduler/preview-modal.jsx @@ -308,7 +308,7 @@ export class PreviewModal extends Component { ) : ( - {props.query.cronInterval ? ( + {this.state.cronInterval ? ( {cronstrue.toString(this.state.cronInterval)} ) : ( From b4f4f0a06f54b8db11bedb7db7e4f1ca9e489e7a Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Wed, 8 Aug 2018 10:55:23 -0700 Subject: [PATCH 15/25] extract getGridColumn into API function --- backend/persistent/QueryScheduler.js | 6 +++--- backend/persistent/plotly-api.js | 13 ++++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js index 31c45c6cc..a91db20b8 100644 --- a/backend/persistent/QueryScheduler.js +++ b/backend/persistent/QueryScheduler.js @@ -21,7 +21,7 @@ import { newGrid, patchGrid, updateGrid, - getGrid + getGridColumn } from './plotly-api.js'; const SUCCESS_CODES = [200, 201, 204]; @@ -393,8 +393,8 @@ class QueryScheduler { startTime = process.hrtime(); - // fetch updated grid for returning - return getGrid(fid, requestor); + // fetch updated grid column for returning + return getGridColumn(fid, requestor); }).then((res) => { Logger.log(`Request to Plotly for fetching updated grid took ${process.hrtime(startTime)[0]} seconds`, 2); if (res.status !== 200) { diff --git a/backend/persistent/plotly-api.js b/backend/persistent/plotly-api.js index a2ba2129d..56969bce1 100644 --- a/backend/persistent/plotly-api.js +++ b/backend/persistent/plotly-api.js @@ -165,6 +165,17 @@ export function patchGrid(fid, requestor, body) { }); } +export function getGridColumn(fid, requestor) { + const {username, apiKey, accessToken} = getCredentials(requestor); + + return plotlyAPIRequest(`grids/${fid}/col`, { + method: 'GET', + username, + apiKey, + accessToken + }); +} + export function updateGrid(rows, columnnames, fid, requestor) { const numColumns = columnnames.length; const columns = getColumns(rows, numColumns); @@ -177,7 +188,7 @@ export function updateGrid(rows, columnnames, fid, requestor) { const baseParams = { username, apiKey, accessToken }; // fetch latest grid to get the source of truth - return plotlyAPIRequest(baseUrl, { ...baseParams, method: 'GET' }) + return getGridColumn(fid, requestor) .then(res => { if (res.status !== 200) { return res; From 9351cc9a957f6016ca05c121db2fffa7c40fa64c Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Wed, 8 Aug 2018 10:58:45 -0700 Subject: [PATCH 16/25] rename preview-modal.jsx -> preview-modal.test.jsx --- .../scheduler/{preview-modal.jsx => preview-modal.test.jsx} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/app/components/Settings/scheduler/{preview-modal.jsx => preview-modal.test.jsx} (98%) diff --git a/test/app/components/Settings/scheduler/preview-modal.jsx b/test/app/components/Settings/scheduler/preview-modal.test.jsx similarity index 98% rename from test/app/components/Settings/scheduler/preview-modal.jsx rename to test/app/components/Settings/scheduler/preview-modal.test.jsx index 2268b7102..7bf3c523e 100644 --- a/test/app/components/Settings/scheduler/preview-modal.jsx +++ b/test/app/components/Settings/scheduler/preview-modal.test.jsx @@ -64,7 +64,7 @@ describe('Preview Modal Tests', () => { .find('button') .at(1) .text() - ).toEqual(expect.stringContaining('Log in')); + ).toEqual(expect.stringContaining('Switch users')); }); it('should render edit and delete buttons if logged in', () => { From a4cf0d4eba6a07070562bc99bd0d1a525da496a7 Mon Sep 17 00:00:00 2001 From: Mike Fix Date: Wed, 8 Aug 2018 11:54:56 -0700 Subject: [PATCH 17/25] revert removing refreshInterval from test --- test/backend/QueryScheduler.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/backend/QueryScheduler.spec.js b/test/backend/QueryScheduler.spec.js index eeedf0cdf..a6bf179b7 100644 --- a/test/backend/QueryScheduler.spec.js +++ b/test/backend/QueryScheduler.spec.js @@ -1,7 +1,7 @@ import {assert} from 'chai'; import sinon from 'sinon'; -import {merge, omit} from 'ramda'; +import {merge} from 'ramda'; import {saveConnection} from '../../backend/persistent/Connections.js'; import { @@ -550,6 +550,7 @@ describe('QueryScheduler', function() { queryObject = { fid, uids, + refreshInterval, cronInterval, connectionId, query: 'SELECT * from ebola_2014 LIMIT 2', @@ -562,7 +563,7 @@ describe('QueryScheduler', function() { queryScheduler.scheduleQuery(queryObject); assert.deepEqual( - getQueries().map(query => omit('refreshInterval', query)), + getQueries(), [queryObject], 'Query has not been saved' ); From 400b5c2488f20f56f7654347111be398628b9724 Mon Sep 17 00:00:00 2001 From: Jake Dexheimer Date: Wed, 8 Aug 2018 13:57:43 -0500 Subject: [PATCH 18/25] clean up SuccessMessage, show datasetUrl in create-modal --- .../Settings/scheduler/create-modal.jsx | 19 +++++++++++++++---- .../Settings/scheduler/preview-modal.jsx | 11 +++++++---- app/components/success.jsx | 10 ++++++---- app/utils/utils.js | 5 +++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/app/components/Settings/scheduler/create-modal.jsx b/app/components/Settings/scheduler/create-modal.jsx index c2cbf6e11..f6e249feb 100644 --- a/app/components/Settings/scheduler/create-modal.jsx +++ b/app/components/Settings/scheduler/create-modal.jsx @@ -6,12 +6,14 @@ import cronstrue from 'cronstrue'; import {Row, Column} from '../../layout.jsx'; import Modal from '../../modal.jsx'; +import {Link} from '../../Link.react'; import SuccessMessage from '../../success.jsx'; import RequestError from './request-error.jsx'; import TimedMessage from './timed-message.jsx'; import CronPicker from '../cron-picker/cron-picker.jsx'; import SQL from './sql.jsx'; +import {datasetUrl as getDatasetURL} from '../../../utils/utils'; import {getHighlightMode, WAITING_MESSAGE, SAVE_WARNING} from '../../../constants/constants.js'; import './create-modal.css'; @@ -62,7 +64,8 @@ class CreateModal extends Component { name: props.initialName, interval: '*/5 * * * *', error: null, - saving: false + saving: false, + datasetUrl: null }; this.options = { lineWrapping: true, @@ -108,8 +111,12 @@ class CreateModal extends Component { cronInterval: this.state.interval, name: this.state.name ? this.state.name.trim() : '' }) - .then(() => { - this.setState({successMessage: 'Scheduled query saved successfully!', saving: false}); + .then((res) => { + this.setState({ + successMessage: 'Scheduled query saved successfully!', + saving: false, + datasetUrl: getDatasetURL(res.fid) + }); }) .catch(error => this.setState({error: error.message, saving: false})); } @@ -193,7 +200,11 @@ class CreateModal extends Component { {this.state.successMessage ? ( - {this.state.successMessage} + + + View Live Dataset + + ) : (