Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const defaultProps = {
vizType: null,
};

class ChartControl extends React.Component {
export class ChartControl extends React.Component {
componentWillMount() {
if (this.props.datasourceId) {
this.props.actions.setFormOpts(this.props.datasourceId, this.props.datasourceType);
Expand Down
3 changes: 1 addition & 2 deletions caravel/assets/javascripts/explorev2/components/Filters.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
// import { Tab, Row, Col, Nav, NavItem } from 'react-bootstrap';
import Select from 'react-select';
import { Button } from 'react-bootstrap';
import { connect } from 'react-redux';
Expand All @@ -18,7 +17,7 @@ const defaultProps = {
filters: [],
};

class Filters extends React.Component {
export class Filters extends React.Component {
constructor(props) {
super(props);
this.state = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const defaultProps = {
groupByColumns: [],
};

class GroupBy extends React.Component {
export class GroupBy extends React.Component {
changeColumns(groupByColumnOpts) {
this.props.actions.setGroupByColumns(groupByColumnOpts);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const propTypes = {
actions: React.PropTypes.object,
};

class SqlClause extends React.Component {
export class SqlClause extends React.Component {
changeWhere(whereClause) {
this.props.actions.setWhereClause(whereClause);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const defaultProps = {
until: null,
};

class TimeFilter extends React.Component {
export class TimeFilter extends React.Component {
changeTimeColumn(timeColumnOpt) {
const val = (timeColumnOpt) ? timeColumnOpt.value : null;
this.props.actions.setTimeColumn(val);
Expand Down
2 changes: 2 additions & 0 deletions caravel/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@
"less": "^2.6.1",
"less-loader": "^2.2.2",
"mocha": "^2.4.5",
"nock": "^8.0.0",
"react-addons-test-utils": "^15.3.2",
"redux-mock-store": "^1.2.1",
"style-loader": "^0.13.0",
"transform-loader": "^0.2.3",
"url-loader": "^0.5.7",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import Select from 'react-select';
import { ChartControl } from '../../../../javascripts/explorev2/components/ChartControl';
import { shallow } from 'enzyme';
import { describe, it } from 'mocha';
import { expect } from 'chai';

describe('QuerySearch', () => {
it('should render', () => {
expect(
React.isValidElement(<ChartControl />)
).to.equal(true);
});

const wrapper = shallow(<ChartControl />);
it('should have two Select', () => {
expect(wrapper.find(Select)).to.have.length(2);
});
});

45 changes: 45 additions & 0 deletions caravel/assets/spec/javascripts/explore/components/Filter_spec.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React from 'react';
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { shallow } from 'enzyme';
import { Button } from 'react-bootstrap';
import Select from 'react-select';
import { Filters } from '../../../../javascripts/explorev2/components/Filters';
import shortid from 'shortid';

function setup() {
const props = {
filters: [
{
id: shortid.generate(),
field: null,
op: null,
value: null,
},
]
}
const wrapper = shallow(<Filters {...props} />);
return {
props,
wrapper,
};
}

describe('Filters', () => {
it('renders', () => {
expect(React.isValidElement(<Filters />)).to.equal(true);
});

it('should have one button', () => {
const wrapper = shallow(<Filters />);
expect(wrapper.find(Button)).to.have.length(1);
expect(wrapper.find(Button).contains('Add Filter')).to.eql(true);
});

it('should have Select and button for filters', () => {
const { wrapper, props } = setup();
expect(wrapper.find(Button)).to.have.length(2);
expect(wrapper.find(Select)).to.have.length(2);
expect(wrapper.find('input')).to.have.length(1);
});
});
17 changes: 17 additions & 0 deletions caravel/assets/spec/javascripts/explore/components/GroupBy_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { shallow } from 'enzyme';
import Select from 'react-select';
import { GroupBy } from '../../../../javascripts/explorev2/components/GroupBy';

describe('GroupBy', () => {
it('renders', () => {
expect(React.isValidElement(<GroupBy />)).to.equal(true);
});

it('should have two Select', () => {
const wrapper = shallow(<GroupBy />);
expect(wrapper.find(Select)).to.have.length(2);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react';
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { shallow } from 'enzyme';
import { SqlClause } from '../../../../javascripts/explorev2/components/SqlClause';

describe('SqlClause', () => {
it('renders', () => {
expect(React.isValidElement(<SqlClause />)).to.equal(true);
});

it('should have two input fields', () => {
const wrapper = shallow(<SqlClause />);
expect(wrapper.find('input')).to.have.length(2);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import Select from 'react-select';
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { shallow } from 'enzyme';
import { TimeFilter } from '../../../../javascripts/explorev2/components/TimeFilter';

describe('TimeFilter', () => {
it('renders', () => {
expect(React.isValidElement(<TimeFilter />)).to.equal(true);
});

it('should have four Select', () => {
const wrapper = shallow(<TimeFilter />);
expect(wrapper.find(Select)).to.have.length(4);
});
});
142 changes: 141 additions & 1 deletion caravel/assets/spec/javascripts/explore/components/actions_spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,111 @@
import { it, describe } from 'mocha';
import { expect } from 'chai';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import nock from 'nock';
import { expect, assert } from 'chai';
import sinon from 'sinon';
import shortid from 'shortid';
import * as actions from '../../../../javascripts/explorev2/actions/exploreActions';
import { initialState } from '../../../../javascripts/explorev2/stores/store';
import { exploreReducer } from '../../../../javascripts/explorev2/reducers/exploreReducer';

const middlewares = [thunk];
const mockStore = configureMockStore(middlewares);

describe('ajax call for datasource metadata', () => {
afterEach(() => {
nock.cleanAll();
});

it('should return a function', () => {
expect(actions.setFormOpts(999, 'test')).to.be.function;
});

it('should dispatch clearAllOpts', () => {
const dispatch = sinon.spy();
actions.setFormOpts(null, null)(dispatch);
assert(dispatch.withArgs(actions.clearAllOpts()).calledOnce);
});

it('should dispatch new opts', () => {
nock('/caravel')
.get('/fetch_datasource_metadata')
.query({ datasource_id: 999, datasource_type: 'test' })
.reply(200, {
datasource_class: 'SqlaTable',
time_columns: ['col'],
time_grains: [],
groupby_cols: [],
metrics: [],
filter_cols: [],
});

const store = mockStore(initialState);
store.dispatch = sinon.spy();
store.dispatch(actions.setFormOpts(999, 'test'));
expect(store.dispatch.callCount).to.equal(5);
Copy link
Contributor Author

@vera-liu vera-liu Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble figuring this one out. I looked at multiple docs for testing ajax action creators, sinon fakeServer, nock etc. I think http://redux.js.org/docs/recipes/WritingTests.html seems to make more sense. I kind of modified some of it, but this test keeps failing. Saying the actual callCount of dispatch is 1 instead of 5. I figure the dispatch callcount may not take callbacks into account. Still trying to figure this out.

Copy link

@ascott ascott Oct 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if we test that the expected actions were called and not how many times dispatch was called? it looks like that's what the example is doing:

nock('http://example.com/')
      .get('/todos')
      .reply(200, { body: { todos: ['do something'] }})

    const expectedActions = [
      { type: types.FETCH_TODOS_REQUEST },
      { type: types.FETCH_TODOS_SUCCESS, body: { todos: ['do something']  } }
    ]
    const store = mockStore({ todos: [] })

    return store.dispatch(actions.fetchTodos())
      .then(() => { // return of async actions
        expect(store.getActions()).toEqual(expectedActions)
      })
  })
const expectedActions = [
  { type: types.SET_TIME_COLUMN_OPTS },
  { type: types.SET_TIME_GRAIN_OPTS },
  {...},
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it but it says 'Cannot read property 'then' of undefined'. It seems that the ajax call never returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it but it didn't work, it says 'then' can't be called on undefined. I think somehow the ajax request never returned on the nock

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's an example spec that tests fetching from an API that the redux best practices doc references...
https://git.musta.ch/airbnb/airbnb/pull/44253/files#diff-1e402f9922b2b08b8ef90a58502d4c6bR56

maybe we can use that pattern?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asking in #frontend-goalie might be helpful too...

expect(store.getState().timeColumnOpts).to.eql(['col']);
});
});

describe('reducers', () => {
it('should return new state with time column options', () => {
const newState = exploreReducer(initialState, actions.setTimeColumnOpts(['col1', 'col2']));
expect(newState.timeColumnOpts).to.eql(['col1', 'col2']);
});
it('should return new state with time grain options', () => {
const newState = exploreReducer(initialState, actions.setTimeGrainOpts(['day', 'week']));
expect(newState.timeGrainOpts).to.eql(['day', 'week']);
});

it('should return new state with groupby column options', () => {
const newState = exploreReducer(initialState, actions.setGroupByColumnOpts(['col1', 'col2']));
expect(newState.groupByColumnOpts).to.eql(['col1', 'col2']);
});

it('should return new state with metrics options', () => {
const newState = exploreReducer(initialState, actions.setMetricsOpts(['metric1', 'metric2']));
expect(newState.metricsOpts).to.eql(['metric1', 'metric2']);
});

it('should return new state with filter column options', () => {
const newState = exploreReducer(initialState, actions.setFilterColumnOpts(['col1', 'col2']));
expect(newState.filterColumnOpts).to.eql(['col1', 'col2']);
});

it('should return new state with all form data reset', () => {
const newState = exploreReducer(initialState, actions.resetFormData());
expect(newState.vizType).to.not.exist;
expect(newState.timeColumn).to.not.exist;
expect(newState.timeGrain).to.not.exist;
expect(newState.since).to.not.exist;
expect(newState.until).to.not.exist;
expect(newState.groupByColumns).to.be.empty;
expect(newState.metrics).to.be.empty;
expect(newState.columns).to.be.empty;
expect(newState.orderings).to.be.empty;
expect(newState.timeStampFormat).to.not.exist;
expect(newState.rowLimit).to.not.exist;
expect(newState.searchBox).to.be.false;
expect(newState.whereClause).to.be.empty;
expect(newState.havingClause).to.be.empty;
expect(newState.filters).to.be.empty;
});

it('should clear all options in store', () => {
const newState = exploreReducer(initialState, actions.clearAllOpts());
expect(newState.timeColumnOpts).to.be.empty;
expect(newState.timeGrainOpts).to.be.empty;
expect(newState.groupByColumnOpts).to.be.empty;
expect(newState.metricsOpts).to.be.empty;
expect(newState.filterColumnOpts).to.be.empty;
});

// it('should return new state with datasource class', () => {
// const newState = exploreReducer(initialState, actions.setDatasourceClass('SqlaTable'));
// expect(newState.datasourceClass).to.equal('SqlaTable');
// });

it('should return new state with datasource id', () => {
const newState = exploreReducer(initialState, actions.setDatasource(1));
expect(newState.datasourceId).to.equal(1);
Expand All @@ -16,6 +116,36 @@ describe('reducers', () => {
expect(newState.vizType).to.equal('bar');
});

it('should return new state with time column', () => {
const newState = exploreReducer(initialState, actions.setTimeColumn('ds'));
expect(newState.timeColumn).to.equal('ds');
});

it('should return new state with time grain', () => {
const newState = exploreReducer(initialState, actions.setTimeGrain('day'));
expect(newState.timeGrain).to.equal('day');
});

it('should return new state with since', () => {
const newState = exploreReducer(initialState, actions.setSince('1 day ago'));
expect(newState.since).to.equal('1 day ago');
});

it('should return new state with until', () => {
const newState = exploreReducer(initialState, actions.setUntil('now'));
expect(newState.until).to.equal('now');
});

it('should return new state with groupby columns', () => {
const newState = exploreReducer(initialState, actions.setGroupByColumns(['col1', 'col2']));
expect(newState.groupByColumns).to.eql(['col1', 'col2']);
});

it('should return new state with metrics', () => {
const newState = exploreReducer(initialState, actions.setMetrics(['sum', 'count']));
expect(newState.metrics).to.eql(['sum', 'count']);
});

it('should return new state with added column', () => {
const newColumn = 'col';
const newState = exploreReducer(initialState, actions.addColumn(newColumn));
Expand Down Expand Up @@ -57,6 +187,16 @@ describe('reducers', () => {
expect(newState.searchBox).to.equal(true);
});

it('should return new state with where clause', () => {
const newState = exploreReducer(initialState, actions.setWhereClause('where'));
expect(newState.whereClause).to.equal('where');
});

it('should return new state with having clause', () => {
const newState = exploreReducer(initialState, actions.setHavingClause('having'));
expect(newState.havingClause).to.equal('having');
});

it('should return new state with added filter', () => {
const newFilter = {
id: shortid.generate(),
Expand Down
1 change: 1 addition & 0 deletions caravel/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class ListWidgetWithCheckboxes(ListWidget):
ACCESS_REQUEST_MISSING_ERR = __(
"The access requests seem to have been deleted")
USER_MISSING_ERR = __("The user seems to have been deleted")
DATASOURCE_ACCESS_ERR = __("User does not have access to this datasource")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry forgot to add this for last PR, adding it here



def get_database_access_error_msg(database_name):
Expand Down
13 changes: 13 additions & 0 deletions tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,19 @@ def test_csv_endpoint(self):
self.assertEqual(list(expected_data), list(data))
self.logout()

def test_datasource_metadata_endpoint(self):
params = [
'datasource_id=1',
'datasource_type=table'
]
resp = self.client.get('/caravel/fetch_datasource_metadata?'+'&'.join(params))
self.assertEquals(500, resp.status_code)

self.login('admin')
resp = self.client.get('/caravel/fetch_datasource_metadata?'+'&'.join(params))
self.assertEquals(200, resp.status_code)
self.logout()

def test_queries_endpoint(self):
resp = self.client.get('/caravel/queries/{}'.format(0))
self.assertEquals(403, resp.status_code)
Expand Down