Skip to content

Commit

Permalink
Provide d2 as a prop rather than being a dependency (#227)
Browse files Browse the repository at this point in the history
The plugin should get d2 as a property rather than having its own version, as the app should determine which version of d2 to use.
  • Loading branch information
jenniferarnesen committed Mar 14, 2019
1 parent 9b215c8 commit f7423b3
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 20 deletions.
6 changes: 6 additions & 0 deletions packages/app/src/components/Visualization/Visualization.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { createSelector } from 'reselect';
import i18n from '@dhis2/d2-i18n';
Expand Down Expand Up @@ -107,6 +108,7 @@ export class Visualization extends Component {
) : (
<ChartPlugin
id={renderId}
d2={this.context.d2}
config={chartConfig}
filters={chartFilters}
onChartGenerated={this.onChartGenerated}
Expand All @@ -118,6 +120,10 @@ export class Visualization extends Component {
}
}

Visualization.contextTypes = {
d2: PropTypes.object,
};

export const chartConfigSelector = createSelector(
[sGetCurrent, sGetVisualization, sGetUiInterpretation],
(current, visualization, interpretation) =>
Expand Down
1 change: 0 additions & 1 deletion packages/plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"license": "BSD-3-Clause",
"dependencies": {
"@material-ui/core": "^3.1.2",
"d2": "31.2.1",
"d2-charts-api": "^31.0.12",
"lodash-es": "^4.17.11",
"react": "^16.6.0",
Expand Down
9 changes: 7 additions & 2 deletions packages/plugin/src/ChartPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class ChartPlugin extends Component {
};

getConfigById = id => {
return apiFetchVisualization('chart', id);
return apiFetchVisualization(this.props.d2, 'chart', id);
};

renderChart = async () => {
Expand Down Expand Up @@ -118,7 +118,11 @@ class ChartPlugin extends Component {
extraOptions[BASE_FIELD_YEARLY_SERIES] = yearlySeriesLabels;
extraOptions.xAxisLabels = computeGenericPeriodNames(responses);
} else {
responses = await apiFetchAnalytics(visualization, options);
responses = await apiFetchAnalytics(
this.props.d2,
visualization,
options
);
}

if (responses.length) {
Expand Down Expand Up @@ -175,6 +179,7 @@ ChartPlugin.defaultProps = {

ChartPlugin.propTypes = {
id: PropTypes.number,
d2: PropTypes.object.isRequired,
animation: PropTypes.number,
config: PropTypes.object.isRequired,
filters: PropTypes.object,
Expand Down
6 changes: 4 additions & 2 deletions packages/plugin/src/__tests__/ChartPlugin.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe('ChartPlugin', () => {
filters: {},
style: { height: 100 },
id: 1,
d2: {},
forDashboard: false,
onChartGenerated: jest.fn(),
onResponsesReceived: jest.fn(),
Expand Down Expand Up @@ -173,7 +174,7 @@ describe('ChartPlugin', () => {

setTimeout(() => {
expect(api.apiFetchAnalytics).toHaveBeenCalled();
expect(api.apiFetchAnalytics.mock.calls[0][1]).toEqual({
expect(api.apiFetchAnalytics.mock.calls[0][2]).toEqual({
option1: 'def',
});

Expand All @@ -193,6 +194,7 @@ describe('ChartPlugin', () => {
setTimeout(() => {
expect(apiViz.apiFetchVisualization).toHaveBeenCalled();
expect(apiViz.apiFetchVisualization).toHaveBeenCalledWith(
props.d2,
'chart',
'test1'
);
Expand Down Expand Up @@ -244,7 +246,7 @@ describe('ChartPlugin', () => {

setTimeout(() => {
expect(api.apiFetchAnalytics).toHaveBeenCalled();
expect(api.apiFetchAnalytics.mock.calls[0][1]).toHaveProperty(
expect(api.apiFetchAnalytics.mock.calls[0][2]).toHaveProperty(
'relativePeriodDate',
period
);
Expand Down
14 changes: 6 additions & 8 deletions packages/plugin/src/api/analytics.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import { getInstance } from 'd2';

const peId = 'pe';

export const apiFetchAnalytics = async (current, options) => {
const d2 = await getInstance();

export const apiFetchAnalytics = async (d2, current, options) => {
const req = new d2.analytics.request()
.fromModel(current)
.withParameters(options);
Expand All @@ -14,9 +10,11 @@ export const apiFetchAnalytics = async (current, options) => {
return [new d2.analytics.response(rawResponse)];
};

export const apiFetchAnalyticsForYearOverYear = async (current, options) => {
const d2 = await getInstance();

export const apiFetchAnalyticsForYearOverYear = async (
d2,
current,
options
) => {
let yearlySeriesReq = new d2.analytics.request()
.addPeriodDimension(current.yearlySeries)
.withSkipData(true)
Expand Down
11 changes: 4 additions & 7 deletions packages/plugin/src/api/visualization.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { getInstance } from 'd2';
import { getFieldsStringByType } from '../modules/fields';

export const apiFetchVisualization = (type, id) =>
getInstance().then(d2 =>
d2.models[type].get(id, {
fields: getFieldsStringByType(type),
})
);
export const apiFetchVisualization = (d2, type, id) =>
d2.models[type].get(id, {
fields: getFieldsStringByType(type),
});

0 comments on commit f7423b3

Please sign in to comment.