Skip to content

Commit

Permalink
[Canvas] Clean up expression function definitions (#37305)
Browse files Browse the repository at this point in the history
* Cleaned up common function defs

    fixed broken tests

    Remove boolean and null as context/arg valid types in lt, lte, gt, and gte

    Added 'function' alias

    Updated comparator tests

    Added neq to options in compare function

    null audit

* Deleted obsolete 'browser' and 'server' functions

* More clean up

* Removed server and browser function i18n strings

* Fixed broken tests

* Addressing feedback

* Removed browser and server from function_help

* Added ts-ignore for constant import

* Fixed help text label

* Fixed font
  • Loading branch information
cqliu1 authored Jun 7, 2019
1 parent 26a98b5 commit 3c9182f
Show file tree
Hide file tree
Showing 64 changed files with 237 additions and 252 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
*/

import { functions as commonFunctions } from '../common';
import { browser } from './browser';
import { location } from './location';
import { markdown } from './markdown';
import { urlparam } from './urlparam';

export const functions = [browser, location, markdown, urlparam, ...commonFunctions];
export const functions = [location, markdown, urlparam, ...commonFunctions];
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getFunctionHelp } from '../../strings';
type Context = Datatable | null;

interface Arguments {
expression: string[];
content: string[];
font: Style;
}

Expand All @@ -34,10 +34,10 @@ export function markdown(): ExpressionFunction<'markdown', Context, Arguments, R
types: ['datatable', 'null'],
},
args: {
expression: {
aliases: ['_'],
content: {
aliases: ['_', 'expression'],
types: ['string'],
help: argHelp.expression,
help: argHelp.content,
default: '""',
multi: true,
},
Expand All @@ -48,7 +48,7 @@ export function markdown(): ExpressionFunction<'markdown', Context, Arguments, R
},
},
fn: (context, args) => {
const compileFunctions = args.expression.map(str =>
const compileFunctions = args.content.map(str =>
Handlebars.compile(String(str), { knownHelpersOnly: true })
);
const ctx = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export function urlparam(): ExpressionFunction<'urlparam', null, Arguments, stri
aliases: ['_', 'var', 'variable'],
help: argHelp.param,
multi: false,
required: true,
},
default: {
types: ['string'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('axisConfig', () => {

it('defaults to "left" if not provided', () => {
const result = fn(testTable);
expect(result).to.have.property('position', '');
expect(result).to.have.property('position', 'left');
});

it('throws when given an invalid position', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ describe('containerStyle', () => {
expect(result).to.have.property('type', 'containerStyle');
});

it('all style properties are omitted if args not provided', () => {
expect(result).to.only.have.key('type');
it('all style properties except `overflow` are omitted if args not provided', () => {
expect(result).to.have.keys('type', 'overflow');
expect(result).to.have.property('overflow', 'hidden');
});
});

Expand Down Expand Up @@ -131,6 +132,10 @@ describe('containerStyle', () => {
result = fn(null, { overflow: 'hidden' });
expect(result).to.have.property('overflow', 'hidden');
});
it(`defaults to 'hidden'`, () => {
const result = fn(null);
expect(result).to.have.property('overflow', 'hidden');
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,20 @@ describe('gt', () => {

it('should return false when the types are different', () => {
expect(fn(1, { value: '1' })).to.be(false);
expect(fn(true, { value: 'true' })).to.be(false);
expect(fn(null, { value: 'null' })).to.be(false);
expect(fn('3', { value: 3 })).to.be(false);
});

it('should return true when greater than', () => {
expect(fn(2, { value: 1 })).to.be(true);
expect(fn('b', { value: 'a' })).to.be(true);
expect(fn('foo', { value: 'bar' })).to.be(true);
expect(fn(true, { value: false })).to.be(true);
});

it('should return false when less than or equal to', () => {
expect(fn(1, { value: 2 })).to.be(false);
expect(fn(2, { value: 2 })).to.be(false);
expect(fn('a', { value: 'b' })).to.be(false);
expect(fn('bar', { value: 'foo' })).to.be(false);
expect(fn('foo', { value: 'foo' })).to.be(false);
expect(fn(false, { value: true })).to.be(false);
expect(fn(true, { value: true })).to.be(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,21 @@ describe('gte', () => {

it('should return false when the types are different', () => {
expect(fn(1, { value: '1' })).to.be(false);
expect(fn(true, { value: 'true' })).to.be(false);
expect(fn(null, { value: 'null' })).to.be(false);
expect(fn(3, { value: '3' })).to.be(false);
});

it('should return true when greater than or equal to', () => {
expect(fn(2, { value: 1 })).to.be(true);
expect(fn(2, { value: 2 })).to.be(true);
expect(fn('b', { value: 'a' })).to.be(true);
expect(fn('b', { value: 'b' })).to.be(true);
expect(fn('foo', { value: 'bar' })).to.be(true);
expect(fn('foo', { value: 'foo' })).to.be(true);
expect(fn(true, { value: false })).to.be(true);
expect(fn(true, { value: true })).to.be(true);
});

it('should return false when less than', () => {
expect(fn(1, { value: 2 })).to.be(false);
expect(fn('a', { value: 'b' })).to.be(false);
expect(fn('bar', { value: 'foo' })).to.be(false);
expect(fn(false, { value: true })).to.be(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,21 @@ describe('lt', () => {

it('should return false when the types are different', () => {
expect(fn(1, { value: '1' })).to.be(false);
expect(fn(true, { value: 'true' })).to.be(false);
expect(fn(null, { value: 'null' })).to.be(false);
expect(fn('3', { value: 3 })).to.be(false);
});

it('should return false when greater than or equal to', () => {
expect(fn(2, { value: 1 })).to.be(false);
expect(fn(2, { value: 2 })).to.be(false);
expect(fn('b', { value: 'a' })).to.be(false);
expect(fn('b', { value: 'b' })).to.be(false);
expect(fn('foo', { value: 'bar' })).to.be(false);
expect(fn('foo', { value: 'foo' })).to.be(false);
expect(fn(true, { value: false })).to.be(false);
expect(fn(true, { value: true })).to.be(false);
});

it('should return true when less than', () => {
expect(fn(1, { value: 2 })).to.be(true);
expect(fn('a', { value: 'b' })).to.be(true);
expect(fn('bar', { value: 'foo' })).to.be(true);
expect(fn(false, { value: true })).to.be(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,21 @@ describe('lte', () => {

it('should return false when the types are different', () => {
expect(fn(1, { value: '1' })).to.be(false);
expect(fn(true, { value: 'true' })).to.be(false);
expect(fn(null, { value: 'null' })).to.be(false);
expect(fn('3', { value: 3 })).to.be(false);
});

it('should return false when greater than', () => {
expect(fn(2, { value: 1 })).to.be(false);
expect(fn('b', { value: 'a' })).to.be(false);
expect(fn('foo', { value: 'bar' })).to.be(false);
expect(fn(true, { value: false })).to.be(false);
});

it('should return true when less than or equal to', () => {
expect(fn(1, { value: 2 })).to.be(true);
expect(fn(2, { value: 2 })).to.be(true);
expect(fn('a', { value: 'b' })).to.be(true);
expect(fn('a', { value: 'a' })).to.be(true);
expect(fn('bar', { value: 'foo' })).to.be(true);
expect(fn('foo', { value: 'foo' })).to.be(true);
expect(fn(false, { value: true })).to.be(true);
expect(fn(true, { value: true })).to.be(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import expect from '@kbn/expect';
import { render } from '../render';
import { functionWrapper } from '../../../../__tests__/helpers/function_wrapper';
import { DEFAULT_ELEMENT_CSS } from '../../../../common/lib/constants';
import { testTable } from './fixtures/test_tables';
import { fontStyle, containerStyle } from './fixtures/test_styles';

Expand Down Expand Up @@ -56,9 +57,9 @@ describe('render', () => {
expect(result).to.have.property('css', '".canvasRenderEl { background-color: red; }"');
});

it("defaults to '* > * {}'", () => {
it(`defaults to '${DEFAULT_ELEMENT_CSS}'`, () => {
const result = fn(renderTable);
expect(result).to.have.property('css', '"* > * {}"');
expect(result).to.have.property('css', `"${DEFAULT_ELEMENT_CSS}"`);
});
});

Expand Down
11 changes: 7 additions & 4 deletions x-pack/plugins/canvas/canvas_plugin_src/functions/common/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,26 @@ import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/public';
import { getFunctionHelp } from '../../strings';

interface Arguments {
condition: boolean[] | null;
condition: boolean[];
}

export function all(): ExpressionFunction<'all', any, Arguments, boolean> {
export function all(): ExpressionFunction<'all', null, Arguments, boolean> {
const { help, args: argHelp } = getFunctionHelp().all;

return {
name: 'all',
type: 'boolean',
help,
context: {
types: ['null'],
},
args: {
condition: {
aliases: ['_'],
types: ['boolean', 'null'],
types: ['boolean'],
help: argHelp.condition,
required: true,
multi: true,
help: argHelp.condition,
},
},
fn: (_context, args) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { getFunctionHelp, getFunctionErrors } from '../../strings';
interface Arguments {
column: string;
type: DatatableColumnType | null;
name: string | null;
name: string;
}

export function alterColumn(): ExpressionFunction<'alterColumn', Datatable, Arguments, Datatable> {
Expand All @@ -30,18 +30,17 @@ export function alterColumn(): ExpressionFunction<'alterColumn', Datatable, Argu
column: {
aliases: ['_'],
types: ['string'],
required: true,
help: argHelp.column,
},
type: {
types: ['string'],
help: argHelp.type,
default: null,
options: ['null', 'boolean', 'number', 'string'],
options: ['null', 'boolean', 'number', 'string', 'date'],
},
name: {
types: ['string', 'null'],
types: ['string'],
help: argHelp.name,
default: null,
},
},
fn: (context, args) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,23 @@ import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/public';
import { getFunctionHelp } from '../../strings';

interface Arguments {
condition: boolean[] | null;
condition: boolean[];
}

export function any(): ExpressionFunction<'any', any, Arguments, boolean> {
export function any(): ExpressionFunction<'any', null, Arguments, boolean> {
const { help, args: argHelp } = getFunctionHelp().any;

return {
name: 'any',
type: 'boolean',
context: {
types: ['null'],
},
help,
args: {
condition: {
aliases: ['_'],
types: ['boolean', 'null'],
types: ['boolean'],
required: true,
multi: true,
help: argHelp.condition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import moment from 'moment';
import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/public';
import { Datatable, Position } from '../types';
import { Position } from '../types';
import { getFunctionHelp, getFunctionErrors } from '../../strings';

interface Arguments {
Expand All @@ -21,7 +21,7 @@ interface AxisConfig extends Arguments {
type: 'axisConfig';
}

export function axisConfig(): ExpressionFunction<'axisConfig', Datatable, Arguments, AxisConfig> {
export function axisConfig(): ExpressionFunction<'axisConfig', null, Arguments, AxisConfig> {
const { help, args: argHelp } = getFunctionHelp().axisConfig;
const errors = getFunctionErrors().axisConfig;

Expand All @@ -30,7 +30,7 @@ export function axisConfig(): ExpressionFunction<'axisConfig', Datatable, Argume
aliases: [],
type: 'axisConfig',
context: {
types: ['datatable'],
types: ['null'],
},
help,
args: {
Expand All @@ -46,11 +46,11 @@ export function axisConfig(): ExpressionFunction<'axisConfig', Datatable, Argume
default: 'left',
},
min: {
types: ['number', 'date', 'string', 'null'],
types: ['number', 'string', 'null'],
help: argHelp.min,
},
max: {
types: ['number', 'date', 'string', 'null'],
types: ['number', 'string', 'null'],
help: argHelp.max,
},
tickSize: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export function caseFn(): ExpressionFunction<'case', any, Arguments, Promise<Cas
},
then: {
resolve: false,
required: true,
help: argHelp.then,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ export function clear(): ExpressionFunction<'clear', any, {}, null> {
name: 'clear',
type: 'null',
help,
context: {
types: ['null'],
},
args: {},
fn: () => null,
};
Expand Down
Loading

0 comments on commit 3c9182f

Please sign in to comment.