Skip to content

Commit

Permalink
fix: backport latest single value fixes in master (#322)
Browse files Browse the repository at this point in the history
* v33.1.0

* chore: yarn lock

* Fix error handling when switching viz type (#317)

Layout validation is run when the update
button is clicked.
This should avoid to load the plugin when there is an invalid layout
which will likely cause an error due to missing expected data/config.

Also, in the reducers we cannot always assume the layout has all the
required dimensions/items, ie. when starting a viz from scratch and
switch viz type or click the update button before selecting some
required dimension item.

* fix: single value without dx items (#313)

* fix: single value without dx items

* fix: pass array instead of object

* fix: fix prettier

* fix: code style

* @dhis2/analytics@2.4.0

* v33.1.1

* fix: update plugin dependency (#318)

* fix: update @dhis2/analytics dependency (#319)

* fix: update plugin version (#320)

v33.1.2

* remove flex-1 add flex-grow-1 (#321)

* chore: update analytics dep

* v33.1.3

* chore: update plugin dep
  • Loading branch information
janhenrikoverland committed Sep 27, 2019
1 parent e667134 commit ab968a0
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 107 deletions.
7 changes: 5 additions & 2 deletions packages/app/i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1)\n"
"POT-Creation-Date: 2019-08-08T13:09:25.099Z\n"
"PO-Revision-Date: 2019-08-08T13:09:25.099Z\n"
"POT-Creation-Date: 2019-09-16T07:28:39.022Z\n"
"PO-Revision-Date: 2019-09-16T07:28:39.022Z\n"

msgid "Rename successful"
msgstr ""
Expand Down Expand Up @@ -129,6 +129,9 @@ msgstr ""
msgid "Viewing interpretation from {{interpretationDate}}"
msgstr ""

msgid "Error validating layout"
msgstr ""

msgid "Create a new visualization by adding dimensions to the layout"
msgstr ""

Expand Down
4 changes: 2 additions & 2 deletions packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
"webpack-bundle-analyzer": "^3.0.3"
},
"dependencies": {
"@dhis2/analytics": "2.3.4",
"@dhis2/analytics": "^2.4.2",
"@dhis2/d2-i18n": "^1.0.3",
"@dhis2/d2-ui-core": "^6.2.1",
"@dhis2/d2-ui-file-menu": "^6.2.1",
"@dhis2/d2-ui-interpretations": "^6.2.1",
"@dhis2/data-visualizer-plugin": "33.0.3",
"@dhis2/data-visualizer-plugin": "^33.1.3",
"@dhis2/ui": "^1.0.0-beta.11",
"@dhis2/ui-core": "^3.4.0",
"@material-ui/core": "^3.1.2",
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/components/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ body {
flex-direction: column;
}

.flex-1 {
flex: 1 1 0%;
.flex-grow-1 {
flex-grow: 1;
}

/* Headerbar */
Expand Down
17 changes: 5 additions & 12 deletions packages/app/src/components/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import VisualizationTypeSelector from './VisualizationTypeSelector/Visualization
import DimensionsPanel from './DimensionsPanel/DimensionsPanel';
import Interpretations from './Interpretations/Interpretations';
import Visualization from './Visualization/Visualization';
import BlankCanvas from './Visualization/BlankCanvas';
import Layout from './Layout/Layout';
import * as fromReducers from '../reducers';
import * as fromActions from '../actions';
Expand Down Expand Up @@ -159,11 +158,6 @@ export class App extends Component {
}

render() {
const showVis =
this.props.current &&
Object.keys(this.props.current).length > 0 &&
!this.props.loadError;

return (
<FatalErrorBoundary>
<AxisSetup />
Expand All @@ -175,23 +169,23 @@ export class App extends Component {
<div className="toolbar-type">
<VisualizationTypeSelector />
</div>
<div className="toolbar-menubar flex-1">
<div className="toolbar-menubar flex-grow-1">
<MenuBar apiObjectName={this.props.apiObjectName} />
</div>
</div>
<div className="section-main flex-1 flex-ct">
<div className="section-main flex-grow-1 flex-ct">
<div className="main-left">
<DimensionsPanel />
</div>
<div className="main-center flex-1 flex-ct flex-dir-col">
<div className="main-center flex-grow-1 flex-ct flex-dir-col">
<div className="main-center-layout">
<Layout />
</div>
<div className="main-center-titlebar">
<TitleBar />
</div>
<div className="main-center-canvas flex-1">
{showVis ? <Visualization /> : <BlankCanvas />}
<div className="main-center-canvas flex-grow-1">
<Visualization />
</div>
</div>
{this.props.ui.rightSidebarOpen && this.props.current && (
Expand All @@ -214,7 +208,6 @@ const mapStateToProps = state => ({
settings: fromReducers.fromSettings.sGetSettings(state),
current: fromReducers.fromCurrent.sGetCurrent(state),
interpretations: fromReducers.fromVisualization.sGetInterpretations(state),
loadError: fromReducers.fromLoader.sGetLoadError(state),
ui: fromReducers.fromUi.sGetUi(state),
});

Expand Down
27 changes: 23 additions & 4 deletions packages/app/src/components/UpdateButton/UpdateButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,41 @@ import Button from '@material-ui/core/Button';
import i18n from '@dhis2/d2-i18n';

import { sGetUi } from '../../reducers/ui';
import { sGetCurrent } from '../../reducers/current';
import { sGetCurrent, sGetCurrentFromUi } from '../../reducers/current';
import * as fromActions from '../../actions';
import history from '../../modules/history';
import styles from './styles/UpdateButton.style';
import { CURRENT_AO_KEY } from '../../api/userDataStore';

import { acSetLoadError, acClearLoadError } from '../../actions/loader';
import { validateLayout } from '../../modules/layoutValidation';

const UpdateButton = ({
classes,
clearLoadError,
acClearLoadError,
acSetLoadError,
onUpdate,
ui,
current,
currentFromUi,
onClick,
flat,
...props
}) => {
const wrappedOnClick = () => {
clearLoadError();
// validate layout on update
// validation error message will be shown without loading the plugin first
try {
validateLayout(currentFromUi);

acClearLoadError();
} catch (err) {
acSetLoadError(
err && err.message
? err.message
: i18n.t('Error validating layout')
);
}
onUpdate(ui);

const urlContainsCurrentAOKey =
Expand Down Expand Up @@ -63,11 +80,13 @@ const UpdateButton = ({
const mapStateToProps = state => ({
ui: sGetUi(state),
current: sGetCurrent(state),
currentFromUi: sGetCurrentFromUi(state),
});

const mapDispatchToProps = {
onUpdate: fromActions.fromCurrent.acSetCurrentFromUi,
clearLoadError: fromActions.fromLoader.acClearLoadError,
acSetLoadError,
acClearLoadError,
};

UpdateButton.propTypes = {
Expand Down
31 changes: 2 additions & 29 deletions packages/app/src/components/Visualization/Visualization.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,7 @@ import { sGetLoadError } from '../../reducers/loader';

import { acAddMetadata } from '../../actions/metadata';
import { acSetChart } from '../../actions/chart';
import {
acSetLoadError,
acSetLoading,
acClearLoadError,
} from '../../actions/loader';

import { validateLayout } from '../../modules/layoutValidation';
import { acSetLoadError } from '../../actions/loader';

import BlankCanvas from './BlankCanvas';

Expand All @@ -35,22 +29,8 @@ export class Visualization extends Component {
this.state = {
renderId: null,
};

if (props.chartConfig) {
this.validate(props.chartConfig);
}
}

validate = visualization => {
try {
validateLayout(visualization);

this.props.acClearLoadError();
} catch (err) {
this.onError(err);
}
};

onError = err => {
const error =
(err && err.message) ||
Expand Down Expand Up @@ -99,11 +79,6 @@ export class Visualization extends Component {
}

componentDidUpdate(prevProps) {
if (this.props.chartConfig !== prevProps.chartConfig) {
this.validate(this.props.chartConfig);
return;
}

// open sidebar
if (this.props.rightSidebarOpen !== prevProps.rightSidebarOpen) {
this.getNewRenderId();
Expand All @@ -114,7 +89,7 @@ export class Visualization extends Component {
const { chartConfig, chartFilters, error } = this.props;
const { renderId } = this.state;

return error ? (
return Boolean(!chartConfig || error) ? (
<BlankCanvas />
) : (
<ChartPlugin
Expand Down Expand Up @@ -162,7 +137,5 @@ export default connect(
acAddMetadata,
acSetChart,
acSetLoadError,
acSetLoading,
acClearLoadError,
}
)(Visualization);
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
chartFiltersSelector,
} from '../Visualization';
import BlankCanvas from '../BlankCanvas';
import * as validator from '../../../modules/layoutValidation';

jest.mock('@dhis2/data-visualizer-plugin', () => () => <div />);

Expand All @@ -24,7 +23,7 @@ describe('Visualization', () => {

beforeEach(() => {
props = {
chartConfig: null,
chartConfig: {},
chartFilters: null,
error: null,
rightSidebarOpen: false,
Expand All @@ -43,7 +42,7 @@ describe('Visualization', () => {
expect(vis().find(BlankCanvas).length).toEqual(1);
});

it('renders a ChartPlugin when no error', () => {
it('renders a ChartPlugin when no error and chartConfig available', () => {
expect(vis().find(ChartPlugin).length).toEqual(1);
});

Expand Down Expand Up @@ -78,23 +77,6 @@ describe('Visualization', () => {
expect(props.acSetLoadError).toHaveBeenCalledWith(errorMsg);
});

it('triggers clearLoadError when chart config has valid layout', () => {
props.chartConfig = { name: 'rainbowDash' };
validator.validateLayout = () => 'valid';
vis();
expect(props.acClearLoadError).toHaveBeenCalled();
});

it('triggers setLoadError when chart config has invalid layout', () => {
props.chartConfig = { name: 'non-valid rainbowDash' };
validator.validateLayout = () => {
throw new Error('not valid');
};
vis();

expect(props.acSetLoadError).toHaveBeenCalled();
});

it('renders chart with new id when rightSidebarOpen prop changes', () => {
const wrapper = vis();

Expand All @@ -104,18 +86,6 @@ describe('Visualization', () => {

expect(initialId).not.toEqual(updatedId);
});

it('triggers clearLoadError when chart changed to a different, valid chart', () => {
validator.validateLayout = () => 'valid';
const wrapper = vis();

wrapper.setProps({
...props,
chartConfig: { name: 'rainbowDash' },
});

expect(props.acClearLoadError).toHaveBeenCalledTimes(1);
});
});

describe('reselectors', () => {
Expand Down
8 changes: 1 addition & 7 deletions packages/app/src/components/__tests__/App.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,7 @@ describe('App', () => {
expect(app().find('div').length).toBeGreaterThan(0);
});

it('does not render a Visualization component when current is not populated', () => {
expect(app().find(Visualization).length).toEqual(0);
});

it('renders Visualization component when current is populated', () => {
props.current = { visProp: {} };

it('renders Visualization component', () => {
expect(app().find(Visualization).length).toBeGreaterThan(0);
});

Expand Down
14 changes: 11 additions & 3 deletions packages/app/src/modules/current.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
DIMENSION_ID_DATA,
DIMENSION_ID_PERIOD,
dimensionCreate,
layoutGetDimensionItems,
layoutReplaceDimension,
} from '@dhis2/analytics';

import options from './options';
Expand Down Expand Up @@ -71,14 +73,20 @@ export const getSingleValueCurrentFromUi = (state, action) => {
},
};

// only save the first dx item
const axesFromUi = getAxesFromUi(ui);
axesFromUi.columns[0].items = [axesFromUi.columns[0].items[0]];

// only save the first dx item
const dxItems = layoutGetDimensionItems(axesFromUi, DIMENSION_ID_DATA);
const singleValueAxesFromUi = layoutReplaceDimension(
axesFromUi,
DIMENSION_ID_DATA,
[dxItems[0]]
);

return {
...state,
[BASE_FIELD_TYPE]: ui.type,
...axesFromUi,
...singleValueAxesFromUi,
...getOptionsFromUi(ui),
};
};
Expand Down
17 changes: 17 additions & 0 deletions packages/app/src/reducers/current.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,20 @@ export default (state = DEFAULT_CURRENT, action) => {
// Selectors

export const sGetCurrent = state => state.current;
export const sGetCurrentFromUi = state => {
const ui = state.ui;

switch (ui.type) {
case PIE:
case GAUGE:
return getPieCurrentFromUi(state, { value: ui });
case SINGLE_VALUE:
return getSingleValueCurrentFromUi(state, { value: ui });
case YEAR_OVER_YEAR_LINE:
case YEAR_OVER_YEAR_COLUMN:
return getYearOverYearCurrentFromUi(state, { value: ui });
default: {
return getDefaultFromUi(state, { value: ui });
}
}
};
4 changes: 2 additions & 2 deletions packages/plugin/package.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"name": "@dhis2/data-visualizer-plugin",
"version": "33.0.3",
"version": "33.1.3",
"description": "DHIS2 Data Visualizer plugin",
"main": "./build/index.js",
"license": "BSD-3-Clause",
"dependencies": {
"@dhis2/analytics": "2.3.4",
"@dhis2/analytics": "^2.4.2",
"@material-ui/core": "^3.1.2",
"lodash-es": "^4.17.11",
"react": "^16.6.0",
Expand Down
Loading

0 comments on commit ab968a0

Please sign in to comment.