Skip to content

Commit

Permalink
Fix: Strip AST from duplicated elements (elastic#266)
Browse files Browse the repository at this point in the history
* fix: strip ast property from elements on duplication

whitelist object properties instead of blacklisting id and ast

* fix: remove unused duplicate download action

downloadWorkpad and downloadWorkpadById were exactly the same code, but only downloadWorkpadById was used

* fix: remove element ast using the selector

this ensures that any previously incorrectly duplicated elements will now work correctly and the ast removed once they are persisted
  • Loading branch information
w33ble authored Dec 12, 2017
1 parent 0c6ccb2 commit 7f9cc48
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 17 deletions.
3 changes: 2 additions & 1 deletion public/lib/workpad_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ export function remove(id) {
}

export function find(searchTerm) {
return fetch.get(`${apiPath}/find?name=${_get(searchTerm, 'length') ? searchTerm : '*'}`).then(resp => resp.data);
return fetch.get(`${apiPath}/find?name=${_get(searchTerm, 'length') ? searchTerm : '*'}`)
.then(resp => resp.data);
}
12 changes: 9 additions & 3 deletions public/state/actions/elements.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createAction } from 'redux-actions';
import { get, omit } from 'lodash';
import { get, pick } from 'lodash';
import { set, del } from 'object-path-immutable';
import { createThunk } from 'redux-thunks';
import * as args from './resolved_args';
Expand Down Expand Up @@ -51,6 +51,12 @@ function getSiblingContext(state, elementId, checkIndex) {
return getSiblingContext(state, elementId, prevContextIndex);
}

function getBareElement(el, includeId = false) {
const props = ['position', 'expression', 'filters'];
if (includeId) return pick(el, props.concat('id'));
return pick(el, props);
}

export const elementLayer = createAction('elementLayer');

export const setPosition = createAction('setPosition', (elementId, pageId, position) => ({ pageId, elementId, position }));
Expand Down Expand Up @@ -136,7 +142,7 @@ export const fetchAllRenderables = createThunk('fetchAllRenderables', ({ dispatc
});

export const duplicateElement = createThunk('duplicateElement', ({ dispatch, type }, element, pageId) => {
const newElement = Object.assign({}, getDefaultElement(), omit(element, 'id'));
const newElement = Object.assign({}, getDefaultElement(), getBareElement(element));
// move the element so users can see that it was added
newElement.position.top = newElement.position.top + 10;
newElement.position.left = newElement.position.left + 10;
Expand Down Expand Up @@ -270,7 +276,7 @@ export const deleteArgumentAtIndex = createThunk('deleteArgumentAtIndex', ({ dis
payload: element defaults. Eg {expression: 'foo'}
*/
export const addElement = createThunk('addElement', ({ dispatch }, pageId, element) => {
const newElement = Object.assign({}, getDefaultElement(), omit(element, 'id'));
const newElement = Object.assign({}, getDefaultElement(), getBareElement(element));
const _addElement = createAction('addElement');
dispatch(_addElement({ pageId, element: newElement }));

Expand Down
15 changes: 4 additions & 11 deletions public/state/actions/workpad.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,10 @@ export const loadWorkpadById = createThunk('loadWorkpadById', ({ dispatch }, wor

export const downloadWorkpadById = createThunk('downloadWorkpadbyId', ({ dispatch }, workpadId) => {
// TODO: handle the failed loading state
workpadService.get(workpadId).then(resp => {
console.log(resp);
fileSaver.saveAs(new Blob([JSON.stringify(resp)], { type: 'application/json' }), `canvas-workpad-${resp.name}-${resp.id}.json`);
});
});

export const downloadWorkpad = createThunk('downloadWorkpad', ({ dispatch }, workpadId) => {
// TODO: handle the failed loading state
workpadService.get(workpadId).then(resp => {
console.log(resp);
fileSaver.saveAs(new Blob([JSON.stringify(resp)], { type: 'application/json' }), `canvas-workpad-${resp.name}-${resp.id}.json`);
workpadService.get(workpadId)
.then(resp => {
const jsonBlob = new Blob([JSON.stringify(resp)], { type: 'application/json' });
fileSaver.saveAs(jsonBlob, `canvas-workpad-${resp.name}-${resp.id}.json`);
});
});

Expand Down
8 changes: 6 additions & 2 deletions public/state/selectors/workpad.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { get, find, findIndex, map } from 'lodash';
import { get, find, findIndex, map, omit } from 'lodash';
import { safeElementFromExpression } from '../../../common/lib/ast';
import { append } from '../../lib/modify_path';
import { getAssets } from './assets';
Expand Down Expand Up @@ -76,8 +76,12 @@ export function getElements(state, pageId, withAst = true) {

const page = getPageById(state, id);
const elements = get(page, 'elements');

if (!elements) return [];
if (!withAst) return elements;

// explicitely strip the ast, basically a fix for corrupted workpads
// due to https://github.com/elastic/kibana-canvas/issues/260
if (!withAst) return elements.map(el => omit(el, ['ast']));

return elements.map(appendAst);
}
Expand Down

0 comments on commit 7f9cc48

Please sign in to comment.