Skip to content

Commit

Permalink
[Canvas] Clean up expression function definitions (#37305) (#38476)
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 10, 2019
1 parent 92d9535 commit d976a24
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 d976a24

Please sign in to comment.