Skip to content

Commit

Permalink
Create the concept of embeddableHandlers (elastic#12146)
Browse files Browse the repository at this point in the history
* Move dashboard panel rendering logic to each registered type.

* Remove dashboard knowledge of click and brush handlers for visualizations

Move it to the VisualizeEmbeddableHandler.

* merge master with manual changes

* No need to use lodash

* Add EmbeddableHandler base class

* Use correct path to embeddable_handlers_registry

* clean up

* Set visualize scope uiState that is of type PersistedState, otherwise it won't actually be set.

* add retry to loading saved search data

* Fix handleError param and remove unnecessary private param

* Rename savePanelState updatePanel and return the new object rather than mutating the original

* Make ContainerAPI a base class and move the dashboard specific functionality into a new class

* Make api's async and clean up documentation

* Fix panel tests

* Fix bug which broke tests - need to pass container-api to dashboard-panel

* Address code comments

- Rename onFilter to addFilter
- Use angular promises instead of async/await
- fix jsdoc
- rename createChildUiState to getInitialState

* changed the wrong variable name

* no need for async or Promise.reject on interface

* add tests that will fail due to spy pane issue in this PR

* Fix logic with spy pane toggle

There is still a bit of a bug here as mentioned in
elastic#13340 but it will be fixed
separately as it’s also an issue in master

* Fix failing test
  • Loading branch information
stacey-gammon committed Aug 8, 2017
1 parent 0698088 commit b8350ea
Show file tree
Hide file tree
Showing 25 changed files with 448 additions and 289 deletions.
3 changes: 2 additions & 1 deletion src/core_plugins/kibana/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ export default function (kibana) {
'navbarExtensions',
'managementSections',
'devTools',
'docViews'
'docViews',
'embeddableHandlers',
],
injectVars,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@ import angular from 'angular';
import expect from 'expect.js';
import ngMock from 'ng_mock';
import 'plugins/kibana/dashboard/saved_dashboard/saved_dashboard';
import { DashboardContainerAPI } from '../dashboard_container_api';
import { DashboardState } from '../dashboard_state';
import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from 'plugins/kibana/dashboard/panel/panel_state';

describe('dashboard panels', function () {
let $scope;
let $el;
let AppState;

function compile(dashboard) {
ngMock.inject(($rootScope, $controller, $compile, $route) => {
ngMock.inject(($injector, $rootScope, $controller, $compile, $route) => {
AppState = $injector.get('AppState');
$scope = $rootScope.$new();
$route.current = {
locals: {
Expand All @@ -18,19 +22,18 @@ describe('dashboard panels', function () {
params: {}
};

const dashboardState = new DashboardState(dashboard, AppState, false);
$scope.containerApi = new DashboardContainerAPI(dashboardState);
$el = angular.element(`
<dashboard-app>
<dashboard-grid
style="width: 600px; height: 600px;"
ng-if="!hasExpandedPanel()"
on-panel-removed="onPanelRemoved"
panels="panels"
get-vis-click-handler="filterBarClickHandler"
get-vis-brush-handler="brushEvent"
save-state="saveState"
toggle-expand="toggleExpandPanel"
create-child-ui-state="createChildUiState"
toggle-expand="toggleExpandPanel"
container-api="containerApi"
></dashboard-grid>
</dashboard-app>`);
$compile($el)($scope);
Expand Down
38 changes: 19 additions & 19 deletions src/core_plugins/kibana/public/dashboard/__tests__/panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,30 @@ import ngMock from 'ng_mock';
import Promise from 'bluebird';
import sinon from 'sinon';
import noDigestPromise from 'test_utils/no_digest_promises';
import mockUiState from 'fixtures/mock_ui_state';
import { SavedObjectsClientProvider } from 'ui/saved_objects';
import { DashboardContainerAPI } from '../dashboard_container_api';
import { DashboardState } from '../dashboard_state';
import { SavedObjectsClient } from 'ui/saved_objects';

describe('dashboard panel', function () {
let $scope;
let $el;
let parentScope;
let savedDashboard;
let AppState;

noDigestPromise.activateForSuite();

function init(mockDocResponse) {
ngMock.module('kibana');
ngMock.inject(($rootScope, $compile, Private) => {
Private.swap(SavedObjectsClientProvider, () => {
return {
get: sinon.stub().returns(Promise.resolve(mockDocResponse))
};
});

ngMock.inject(($rootScope, $compile, Private, $injector) => {
const SavedDashboard = $injector.get('SavedDashboard');
AppState = $injector.get('AppState');
savedDashboard = new SavedDashboard();
sinon.stub(SavedObjectsClient.prototype, 'get').returns(Promise.resolve(mockDocResponse));
parentScope = $rootScope.$new();
parentScope.saveState = sinon.stub();
parentScope.createChildUiState = sinon.stub().returns(mockUiState);
const dashboardState = new DashboardState(savedDashboard, AppState, false);
parentScope.containerApi = new DashboardContainerAPI(dashboardState);
parentScope.getVisClickHandler = sinon.stub();
parentScope.getVisBrushHandler = sinon.stub();
parentScope.registerPanelIndexPattern = sinon.stub();
Expand All @@ -41,39 +43,37 @@ describe('dashboard panel', function () {
panel="panel"
is-full-screen-mode="false"
is-expanded="false"
get-vis-click-handler="getVisClickHandler"
get-vis-brush-handler="getVisBrushHandler"
save-state="saveState"
register-panel-index-pattern="registerPanelIndexPattern"
create-child-ui-state="createChildUiState">
container-api="containerApi"
>
</dashboard-panel>`)(parentScope);
$scope = $el.isolateScope();
parentScope.$digest();
});
}

afterEach(() => {
SavedObjectsClient.prototype.get.restore();
$scope.$destroy();
$el.remove();
});

it('should not visualize the visualization if it does not exist', function () {
init({ found: false });
return $scope.loadedPanel.then(() => {
return $scope.renderPromise.then(() => {
expect($scope.error).to.be('Could not locate that visualization (id: foo1)');
parentScope.$digest();
const content = $el.find('.panel-content');
expect(content).to.have.length(0);
expect(content.children().length).to.be(0);
});
});

it('should try to visualize the visualization if found', function () {
init({ id: 'foo1', type: 'visualization', _version: 2, attributes: {} });
return $scope.loadedPanel.then(() => {
return $scope.renderPromise.then(() => {
expect($scope.error).not.to.be.ok();
parentScope.$digest();
const content = $el.find('.panel-content');
expect(content).to.have.length(1);
expect(content.children().length).to.be.greaterThan(0);
});
});
});
15 changes: 2 additions & 13 deletions src/core_plugins/kibana/public/dashboard/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,10 @@ <h2 class="kuiTitle kuiVerticalRhythm">
on-panel-removed="onPanelRemoved"
dashboard-view-mode="dashboardViewMode"
panels="panels"
get-vis-click-handler="getFilterBarClickHandler"
get-vis-brush-handler="getBrushEvent"
save-state="saveState"
app-state="appState"
toggle-expand="toggleExpandPanel"
create-child-ui-state="createChildUiState"
toggle-expand="toggleExpandPanel"
register-panel-index-pattern="registerPanelIndexPattern"
data-shared-items-count="{{panels.length}}"
on-filter="filter"
container-api="containerApi"
></dashboard-grid>

<dashboard-panel
Expand All @@ -98,12 +92,7 @@ <h2 class="kuiTitle kuiVerticalRhythm">
is-full-screen-mode="!chrome.getVisible()"
is-expanded="true"
dashboard-view-mode="dashboardViewMode"
get-vis-click-handler="getFilterBarClickHandler"
get-vis-brush-handler="getBrushEvent"
save-state="saveState"
app-state="appState"
register-panel-index-pattern="registerPanelIndexPattern"
create-child-ui-state="createChildUiState"
container-api="containerApi"
toggle-expand="toggleExpandPanel(expandedPanel.panelIndex)"
></dashboard-panel>

Expand Down
31 changes: 5 additions & 26 deletions src/core_plugins/kibana/public/dashboard/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@ import { DocTitleProvider } from 'ui/doc_title';
import { getTopNavConfig } from './top_nav/get_top_nav_config';
import { DashboardConstants, createDashboardEditUrl } from './dashboard_constants';
import { VisualizeConstants } from 'plugins/kibana/visualize/visualize_constants';
import { UtilsBrushEventProvider } from 'ui/utils/brush_event';
import { FilterBarClickHandlerProvider } from 'ui/filter_bar/filter_bar_click_handler';
import { DashboardState } from './dashboard_state';
import { notify } from 'ui/notify';
import './panel/get_object_loaders_for_dashboard';
import { documentationLinks } from 'ui/documentation_links/documentation_links';
import { showCloneModal } from './top_nav/show_clone_modal';
import { migrateLegacyQuery } from 'ui/utils/migrateLegacyQuery';
import { QueryManagerProvider } from 'ui/query_manager';
import { ESC_KEY_CODE } from 'ui_framework/services';
import { DashboardContainerAPI } from './dashboard_container_api';

const app = uiModules.get('app/dashboard', [
'elasticsearch',
Expand Down Expand Up @@ -86,8 +84,6 @@ app.directive('dashboardApp', function ($injector) {
const confirmModal = $injector.get('confirmModal');
const config = $injector.get('config');
const Private = $injector.get('Private');
const brushEvent = Private(UtilsBrushEventProvider);
const filterBarClickHandler = Private(FilterBarClickHandlerProvider);

return {
restrict: 'E',
Expand All @@ -103,8 +99,10 @@ app.directive('dashboardApp', function ($injector) {
docTitle.change(dash.title);
}

const dashboardState = new DashboardState(dash, AppState, dashboardConfig);
const dashboardState = new DashboardState(dash, AppState, dashboardConfig.getHideWriteControls());
$scope.appState = dashboardState.getAppState();
const queryManager = Private(QueryManagerProvider)(dashboardState.getAppState());
$scope.containerApi = new DashboardContainerAPI(dashboardState, queryManager);

// The 'previouslyStored' check is so we only update the time filter on dashboard open, not during
// normal cross app navigation.
Expand All @@ -124,6 +122,7 @@ app.directive('dashboardApp', function ($injector) {
};
$scope.panels = dashboardState.getPanels();
$scope.fullScreenMode = dashboardState.getFullScreenMode();
$scope.indexPatterns = dashboardState.getPanelIndexPatterns();
};

// Part of the exposed plugin API - do not remove without careful consideration.
Expand Down Expand Up @@ -155,11 +154,8 @@ app.directive('dashboardApp', function ($injector) {
$scope.timefilter = timefilter;
$scope.expandedPanel = null;
$scope.dashboardViewMode = dashboardState.getViewMode();
$scope.appState = dashboardState.getAppState();

$scope.landingPageUrl = () => `#${DashboardConstants.LANDING_PAGE_PATH}`;
$scope.getBrushEvent = () => brushEvent(dashboardState.getAppState());
$scope.getFilterBarClickHandler = () => filterBarClickHandler(dashboardState.getAppState());
$scope.hasExpandedPanel = () => $scope.expandedPanel !== null;
$scope.getDashTitle = () => getDashboardTitle(
dashboardState.getTitle(),
Expand Down Expand Up @@ -212,17 +208,6 @@ app.directive('dashboardApp', function ($injector) {
notify.info(`Search successfully added to your dashboard`);
};

/**
* Creates a child ui state for the panel. It's passed the ui state to use, but needs to
* be generated from the parent (why, I don't know yet).
* @param path {String} - the unique path for this ui state.
* @param uiState {Object} - the uiState for the child.
* @returns {Object}
*/
$scope.createChildUiState = function createChildUiState(path, uiState) {
return dashboardState.uiState.createChild(path, uiState, true);
};

$scope.$watch('model.darkTheme', () => {
dashboardState.setDarkTheme($scope.model.darkTheme);
updateTheme();
Expand All @@ -242,12 +227,6 @@ app.directive('dashboardApp', function ($injector) {
$scope.indexPatterns = dashboardState.getPanelIndexPatterns();
};

$scope.filter = function (field, value, operator, index) {
queryManager.add(field, value, operator, index);
updateState();
};


$scope.$watch('model.query', (newQuery) => {
$scope.model.query = migrateLegacyQuery(newQuery);
dashboardState.applyFilters($scope.model.query, filterBar.getFilters());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { ContainerAPI } from 'ui/embeddable';

export class DashboardContainerAPI extends ContainerAPI {
constructor(dashboardState, queryManager) {
super();
this.dashboardState = dashboardState;
this.queryManager = queryManager;
}

addFilter(field, value, operator, index) {
this.queryManager.add(field, value, operator, index);
}

updatePanel(panelIndex, panelAttributes) {
const panelToUpdate = this.dashboardState.getPanels().find((panel) => panel.panelIndex === panelIndex);
Object.assign(panelToUpdate, panelAttributes);
this.dashboardState.saveState();
return panelToUpdate;
}

getAppState() {
return this.dashboardState.appState;
}

createChildUistate(path, initialState) {
return this.dashboardState.uiState.createChild(path, initialState, true);
}

registerPanelIndexPattern(panelIndex, pattern) {
this.dashboardState.registerPanelIndexPatternMap(panelIndex, pattern);
this.dashboardState.saveState();
}

}
12 changes: 6 additions & 6 deletions src/core_plugins/kibana/public/dashboard/dashboard_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ export class DashboardState {
*
* @param savedDashboard {SavedDashboard}
* @param AppState {AppState}
* @param dashboardConfig {DashboardConfigProvider}
* @param hideWriteControls {boolean} true if write controls should be hidden.
*/
constructor(savedDashboard, AppState, dashboardConfig) {
constructor(savedDashboard, AppState, hideWriteControls) {
this.savedDashboard = savedDashboard;
this.dashboardConfig = dashboardConfig;
this.hideWriteControls = hideWriteControls;

this.stateDefaults = getStateDefaults(this.savedDashboard, this.dashboardConfig.getHideWriteControls());
this.stateDefaults = getStateDefaults(this.savedDashboard, this.hideWriteControls);

this.appState = new AppState(this.stateDefaults);
this.uiState = this.appState.makeStateful('uiState');
Expand Down Expand Up @@ -117,7 +117,7 @@ export class DashboardState {
// The right way to fix this might be to ensure the defaults object stored on state is a deep
// clone, but given how much code uses the state object, I determined that to be too risky of a change for
// now. TODO: revisit this!
this.stateDefaults = getStateDefaults(this.savedDashboard, this.dashboardConfig.getHideWriteControls());
this.stateDefaults = getStateDefaults(this.savedDashboard, this.hideWriteControls);
// The original query won't be restored by the above because the query on this.savedDashboard is applied
// in place in order for it to affect the visualizations.
this.stateDefaults.query = this.lastSavedDashboardFilters.query;
Expand Down Expand Up @@ -259,7 +259,7 @@ export class DashboardState {
* @returns {DashboardViewMode}
*/
getViewMode() {
return this.dashboardConfig.getHideWriteControls() ? DashboardViewMode.VIEW : this.appState.viewMode;
return this.hideWriteControls ? DashboardViewMode.VIEW : this.appState.viewMode;
}

/**
Expand Down
Loading

0 comments on commit b8350ea

Please sign in to comment.