Skip to content

Commit

Permalink
[KED-824][KED-788] Use new JSON API in app, and calculate transiti… (#8)
Browse files Browse the repository at this point in the history
* Migrate to a new data file/endpoint and format

* Fix random-data generator

* Remove unnecessary extra filter loop in linked-nodes

This should improve the efficiency of the search

* Calculate transitive links on the fly in Edge selector

* Fix tags, by ensuring nodeTags use generated tag IDs

* Update mock data, separate data.mock from state.mock, and fix tests

* Remove unused variable

* In format-data, validate input before anything else

* Fix node sorting by name in node-list

* Change min node-list height to 400px

* Fix getNodes test for sort direction

Sort by name, not ID

* Add new tests for transitive edge calculations

* Add missing addNewEdge test

* Use 'is_parameters' boolean flag for params instead of string text search

* Fix parameters in random data generator

* Remove getRandomMatch util

It's an unnecessary abstraction that can be covered by getRandom

* Add ability to load and run mock data

Also, reformat CONTRIBUTING.md (Automatically reformatted with Prettier or whatever), and add note about it to the docs

* Fix typo in comment
  • Loading branch information
richardwestenra authored Jul 15, 2019
1 parent 305ca10 commit 243fd1b
Show file tree
Hide file tree
Showing 35 changed files with 907 additions and 404 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
# CSS
/src/**/*.css

# JSON viz data (generated from logs)
# JSON viz data
/public/data
/public/logs
/public/api

# dependencies
/node_modules
Expand Down
43 changes: 25 additions & 18 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,36 +33,42 @@ Typically, small contributions to Kedro-Viz are more preferable due to an easier
## Your first contribution

Working on your first pull request? You can learn how from these resources:
* [First timers only](https://www.firsttimersonly.com/)
* [How to contribute to an open source project on GitHub](https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github)

- [First timers only](https://www.firsttimersonly.com/)
- [How to contribute to an open source project on GitHub](https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github)

### Guidelines
> *Note:* We only accept contributions under the [Apache 2.0](https://opensource.org/licenses/Apache-2.0) license and you should have permission to share the submitted code.
* Aim for cross-platform compatibility on Windows, macOS and Linux, and support recent versions of major browsers
* We use [SemVer](https://semver.org/) for versioning
* Our code is designed to be compatible with Python 3.5 onwards
* We use [Anaconda](https://www.anaconda.com/distribution/) as a preferred virtual environment
* We use [PEP 8 conventions](https://www.python.org/dev/peps/pep-0008/) for all Python code
* We use [Google docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) for code comments
* We use [PEP 484 type hints](https://www.python.org/dev/peps/pep-0484/) for all user-facing functions / class methods e.g.

> _Note:_ We only accept contributions under the [Apache 2.0](https://opensource.org/licenses/Apache-2.0) license and you should have permission to share the submitted code.
- Aim for cross-platform compatibility on Windows, macOS and Linux, and support recent versions of major browsers
- We use [SemVer](https://semver.org/) for versioning
- Our code is designed to be compatible with Python 3.5 onwards
- We use [Anaconda](https://www.anaconda.com/distribution/) as a preferred virtual environment
- We use [PEP 8 conventions](https://www.python.org/dev/peps/pep-0008/) for all Python code
- We use [Google docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) for code comments
- We use [PEP 484 type hints](https://www.python.org/dev/peps/pep-0484/) for all user-facing functions / class methods e.g.

```
def count_truthy(elements: List[Any]) -> int:
return sum(1 for elem in elements if elem)
```
* Keep UI elements fully keyboard accessible, and aim to support screen-readers where possible
* Maintain a high level of animation performance, and minimise page-load time

- Keep UI elements fully keyboard accessible, and aim to support screen-readers where possible
- Maintain a high level of animation performance, and minimise page-load time

### JavaScript Development

First, clone this repo. Then install dependencies (`npm i`). Now you're ready to begin development.

If you want to use a particular dataset, you'll first need to place it in `/public/logs/nodes.json`. Otherwise, you can serve randomly-generated data.

Kedro-Viz uses an environment variable to configure the data source. You can set it when starting up the dev server, e.g. `DATA=random npm start` will serve random procedurally-generated data (refreshed on each page-load). When random data is enabled, certain other features like snapshot history are also enabled by default to help with browser testing. This is usually the most useful setting for local development.
Kedro-Viz uses an environment variable to configure the data source. You can set it when starting up the dev server, e.g. `DATA=random npm start` will serve random procedurally-generated data (refreshed on each page-load). When random or mock data is enabled, certain other features like snapshot history are also enabled by default to help with browser testing. This is usually the most useful setting for local development.

In other words, to run the app in development mode on a local server, use one of the following:

- `DATA=random npm start` --> Serve randomly-generated data
- `DATA=mock npm start` --> Serve example test data, from `/src/utils/data.mock.js`
- `npm start` --> Serve data loaded from `/public/logs/nodes.json`

This will serve the app at [localhost:4141](http://localhost:4141/), and watch files in `/src` for changes. It will also update the `/lib` directory, which contains a Babel-compiled copy of the source. This directory is exported to `npm`, and is used when importing as a React component into another application. It is updated automatically on save in case you need to test/debug it locally (e.g. with `npm link`). You can also update it manually, by running
Expand All @@ -84,10 +90,10 @@ We use a branching model that helps us keep track of branches in a logical, cons

## Plugin contribution process

1. Fork the project
2. Develop your contribution in a new branch and open a PR against the `develop` branch
3. Make sure the CI builds are green (have a look at the section [Running checks locally](#running-checks-locally) below)
4. Update the PR according to the reviewer's comments
1. Fork the project
2. Develop your contribution in a new branch and open a PR against the `develop` branch
3. Make sure the CI builds are green (have a look at the section [Running checks locally](#running-checks-locally) below)
4. Update the PR according to the reviewer's comments

### JavaScript application tests

Expand All @@ -112,6 +118,7 @@ npm run test:coverage
See the [Create-React-App docs](https://github.com/facebook/create-react-app) for further information on JS testing.

## Python web server tests

To run E2E tests you need to install the test requirements which includes `behave`, do this using the following command:

```bash
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ As a JavaScript React component, the project is designed to be used in two diffe

1. **Standalone application**

Run `npm run build` to generate a production build as a full-page app. The built app will be placed in the `/build` directory. Data for the chart should be placed in `/public/logs/nodes.json` because this directory is marked `gitignore`.
Run `npm run build` to generate a production build as a full-page app. The built app will be placed in the `/build` directory. Data for the chart should be placed in `/public/api/nodes.json` because this directory is marked `gitignore`.

2. **React component**

Expand Down
2 changes: 1 addition & 1 deletion src/actions/actions.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mockData } from '../utils/data.mock';
import mockData from '../utils/data.mock';
import {
CHANGE_ACTIVE_SNAPSHOT,
CHANGE_VIEW,
Expand Down
12 changes: 8 additions & 4 deletions src/components/app/app.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { shallow } from 'enzyme';
import App from './index';
import { mockData } from '../../utils/data.mock';
import mockData from '../../utils/data.mock';

describe('App', () => {
describe('renders without crashing', () => {
Expand Down Expand Up @@ -30,14 +30,18 @@ describe('App', () => {

it('when data prop is set on first load', () => {
const wrapper = shallow(<App data={mockData} />);
expect(getSnapshotIDs(wrapper)).toHaveLength(mockData.length);
expect(getSnapshotIDs(wrapper)).toHaveLength(mockData.snapshots.length);
});

it('when data prop is updated', () => {
const wrapper = shallow(<App data={mockData} />);
const newMockData = Object.assign([], mockData.concat(mockData[0]));
const newMockData = Object.assign({}, mockData, {
snapshots: [...mockData.snapshots, mockData.snapshots[0]]
});
wrapper.setProps({ data: newMockData });
expect(getSnapshotIDs(wrapper)).toHaveLength(newMockData.length);
expect(getSnapshotIDs(wrapper)).toHaveLength(
newMockData.snapshots.length
);
});
});

Expand Down
21 changes: 12 additions & 9 deletions src/components/app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class App extends React.Component {
*/
dataWasUpdated(prevData, newData) {
// Check just the schema IDs of incoming data updates
const dataID = snapshots =>
const dataID = ({ snapshots }) =>
Array.isArray(snapshots) && snapshots.map(d => d.schema_id).join('');

return dataID(prevData) !== dataID(newData);
Expand All @@ -60,14 +60,17 @@ App.propTypes = {
allowHistoryDeletion: PropTypes.bool,
data: PropTypes.oneOfType([
PropTypes.string,
PropTypes.arrayOf(
PropTypes.shape({
created_ts: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
json_schema: PropTypes.array.isRequired,
schema_id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
message: PropTypes.string
})
)
PropTypes.shape({
snapshots: PropTypes.arrayOf(
PropTypes.shape({
created_ts: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
edges: PropTypes.array.isRequired,
message: PropTypes.string,
nodes: PropTypes.array.isRequired,
tags: PropTypes.array
})
)
})
]),
onDeleteSnapshot: PropTypes.func,
showHistory: PropTypes.bool
Expand Down
9 changes: 6 additions & 3 deletions src/components/app/load-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { json } from 'd3-fetch';
import config from '../../config';
import getRandomHistory from '../../utils/random-data';
import formatSnapshots from '../../utils/format-data';
import mockData from '../../utils/data.mock';
import { loadState } from '../../utils';

/**
Expand Down Expand Up @@ -48,9 +49,11 @@ export const loadData = (data, onLoadData) => {
switch (data) {
case 'random':
return formatSnapshots(getRandomHistory());
case 'mock':
return formatSnapshots(mockData);
case 'json':
loadJsonData().then(onLoadData);
return formatSnapshots([]);
return formatSnapshots({ snapshots: [] });
case null:
throw new Error('No data was provided to App component via props');
default:
Expand All @@ -64,10 +67,10 @@ export const loadData = (data, onLoadData) => {
export const loadJsonData = () => {
const { dataPath } = config();
return json(dataPath)
.then(json_schema => formatSnapshots([{ json_schema }]))
.catch(() => {
throw new Error(
`Unable to load pipeline data. Please check that you have placed a file at ${dataPath}`
);
});
})
.then(formatSnapshots);
};
2 changes: 1 addition & 1 deletion src/components/chart-ui/chart-ui.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ChartUI, {
mapStateToProps,
mapDispatchToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

describe('ChartUI', () => {
it('renders without crashing', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/description/description.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Description, mapStateToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import {
getActiveSnapshotMessage,
getActiveSnapshotTimestamp
Expand Down
2 changes: 1 addition & 1 deletion src/components/flowchart/flowchart.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import $ from 'cheerio';
import FlowChart, { mapStateToProps, mapDispatchToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import { getActiveSnapshotNodes } from '../../selectors/nodes';

describe('FlowChart', () => {
Expand Down
16 changes: 8 additions & 8 deletions src/components/flowchart/linked-nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ export const getLinkedNodes = (edges, nodeID) => {
const linkedNodes = [];

(function getParents(id) {
edges
.filter(d => d.target === id)
.forEach(d => {
edges.forEach(d => {
if (d.target === id) {
linkedNodes.push(d.source);
getParents(d.source);
});
}
});
})(nodeID);

(function getChildren(id) {
edges
.filter(d => d.source === id)
.forEach(d => {
edges.forEach(d => {
if (d.source === id) {
linkedNodes.push(d.target);
getChildren(d.target);
});
}
});
})(nodeID);

return linkedNodes;
Expand Down
2 changes: 1 addition & 1 deletion src/components/flowchart/linked-nodes.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { getLinkedNodes } from './linked-nodes';
import { getLayout } from '../../selectors/layout';
import { mockState } from '../../utils/data.mock';
import { mockState } from '../../utils/state.mock';

describe('getLinkedNodes function', () => {
it('should search through edges for ancestor and descendant nodes', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/history/history.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import History, {
mapStateToProps,
mapDispatchToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import { getSnapshotHistory } from '../../selectors';

const props = {
Expand Down
2 changes: 1 addition & 1 deletion src/components/node-list/node-list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
}

.pipeline-node-list-scrollbars {
min-height: 150px;
min-height: 400px;

.kui-theme--light & {
background: $color-bg-light-alt;
Expand Down
2 changes: 1 addition & 1 deletion src/components/node-list/node-list.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import NodeList, { mapStateToProps, mapDispatchToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import { getNodes } from '../../selectors/nodes';

describe('NodeList', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/sidebar-tabs/sidebar-tabs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import SidebarTabs, {
SidebarTabs as UnconnectedSidebarTabs,
mapStateToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

describe('SidebarTabs', () => {
it('renders without crashing', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/tag-list/tag-list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import TagList, {
mapStateToProps,
mapDispatchToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

describe('TagList', () => {
it('renders without crashing', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/wrapper/wrapper.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Wrapper, mapStateToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

const { showHistory, theme } = mockState;
const mockProps = { showHistory, theme };
Expand Down
2 changes: 1 addition & 1 deletion src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Generate a configuration object for use across the application
*/
const config = () => ({
dataPath: './logs/nodes.json',
dataPath: './api/nodes.json',
dataSource: process.env.REACT_APP_DATA_SOURCE || 'json',
localStorageName: 'KedroViz'
});
Expand Down
6 changes: 3 additions & 3 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import config from './config';
import './styles/index.css';

const { dataSource } = config();
const useRandomData = dataSource === 'random';
const showHistory = dataSource === 'random' || dataSource === 'mock';

ReactDOM.render(
<App
allowHistoryDeletion={useRandomData}
allowHistoryDeletion={showHistory}
data={dataSource}
showHistory={useRandomData}
showHistory={showHistory}
/>,
document.getElementById('root')
);
4 changes: 2 additions & 2 deletions src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ function reducer(state = {}, action) {
}

case TOGGLE_PARAMETERS: {
const paramIDs = state.snapshotNodes[state.activeSnapshot].filter(id =>
state.nodeName[id].includes('param')
const paramIDs = state.snapshotNodes[state.activeSnapshot].filter(
id => state.nodeIsParam[id]
);
return Object.assign({}, state, {
nodeDisabled: paramIDs.reduce(
Expand Down
Loading

0 comments on commit 243fd1b

Please sign in to comment.