Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce inheritance to the Parameter structure #4049

Merged
merged 30 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c81f46e
Start draft for new Parameter structure
gabrieldutra Aug 7, 2019
20a2f83
Add the rest of the methods
gabrieldutra Aug 7, 2019
5c7e745
EnumParameter
gabrieldutra Aug 7, 2019
a05d4b2
QueryBasedDropdownParameter
gabrieldutra Aug 7, 2019
3c52dcb
DateParameter
gabrieldutra Aug 7, 2019
db009df
DateRangeParameter
gabrieldutra Aug 8, 2019
8cccf8b
Update Parameter usage on code
gabrieldutra Aug 8, 2019
a1a3421
Merge dynamicValue into normalizedValue
gabrieldutra Aug 11, 2019
6b98f3d
Merge branch 'master' into parameters-structure
gabrieldutra Sep 4, 2019
622240d
Add updateLocals and omit unwanted props
gabrieldutra Sep 5, 2019
5dcacbe
Allow null NumberParameter and omit parentQueryId
gabrieldutra Sep 7, 2019
f7a382b
Merge branch 'master' into parameters-structure
gabrieldutra Sep 11, 2019
3279e88
Rename parameter getValue to getExecutionValue
gabrieldutra Sep 11, 2019
c4c0418
Update $$value to normalizedValue + omit on save
gabrieldutra Sep 11, 2019
bd09b34
Merge branch 'master' into parameters-structure
gabrieldutra Sep 16, 2019
a6df8b3
Add a few comments
gabrieldutra Sep 16, 2019
8e0a684
Remove ngModel property from Parameter
gabrieldutra Sep 17, 2019
6b09a84
Use value directly in DateRangeParameter
gabrieldutra Sep 18, 2019
0374841
Use simpler separator for DateRange url param
gabrieldutra Sep 18, 2019
0e5782e
Add backward compatibility
gabrieldutra Sep 19, 2019
3718047
Merge branch 'master' into parameters-structure
gabrieldutra Sep 23, 2019
b63b584
Merge branch 'master' into parameters-structure
gabrieldutra Sep 29, 2019
da7255e
Use normalizeValue null value for isEmpty
gabrieldutra Sep 29, 2019
927783d
Start creating jest tests
gabrieldutra Oct 5, 2019
3a3c08f
Add more tests
gabrieldutra Oct 6, 2019
b970fb0
Merge branch 'master' into parameters-structure
gabrieldutra Oct 6, 2019
567536d
Merge branch 'master' into parameters-structure
gabrieldutra Oct 10, 2019
d269a38
Normalize null value for multi mode in Enum
gabrieldutra Oct 10, 2019
82c1104
Use saved value for param isEmpty
gabrieldutra Oct 11, 2019
56268b3
Merge branch 'master' into parameters-structure
arikfr Oct 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions client/app/services/parameters/NumberParameter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { toNumber, isNull, isUndefined } from 'lodash';
import { Parameter } from '.';

class NumberParameter extends Parameter {
constructor(parameter, parentQueryId) {
super(parameter, parentQueryId);
this.setValue(parameter.value);
}

static normalizeValue(value) {
if (isNull(value) || isUndefined(value)) {
return null;
}

const normalizedValue = toNumber(value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toNumber converts empty strings to 0, but I think it should be normalized to null instead.
How about:

import { toNumber, trim } from 'lodash';

normalizeValue(value) {
    if (!trim(value)) {
      return null;
    }
    ...
}

return !isNaN(normalizedValue) ? normalizedValue : 0;
}
}

export default NumberParameter;
130 changes: 130 additions & 0 deletions client/app/services/parameters/Parameter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { isNull, isObject, isFunction, isUndefined, isEqual, has } from 'lodash';
import { TextParameter, NumberParameter } from '.';

class Parameter {
constructor(parameter, parentQueryId) {
this.title = parameter.title;
this.name = parameter.name;
this.type = parameter.type;
this.global = parameter.global; // backward compatibility in Widget service
this.parentQueryId = parentQueryId;

// Used for meta-parameters (i.e. dashboard-level params)
this.locals = [];

// Used for URL serialization
Object.defineProperty(this, 'urlPrefix', {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Cypress pointed out, this doesn't work with inheritance 🙂

configurable: true,
enumerable: false, // don't save it
writable: true,
value: 'p_',
});
}

static create(param, parentQueryId) {
switch (param.type) {
case 'number':
return new NumberParameter(param, parentQueryId);
default:
return new TextParameter({ ...param, type: 'text' }, parentQueryId);
}
}
ranbena marked this conversation as resolved.
Show resolved Hide resolved

static normalizeValue(value) {
if (isUndefined(value)) {
return null;
}
return value;
}

static getValue(param, extra = {}) {
if (!isObject(param) || !isFunction(param.getValue)) {
return null;
}

return param.getValue(extra);
}

static setValue(param, value) {
if (!isObject(param) || !isFunction(param.setValue)) {
return null;
}

return param.setValue(value);
}

get isEmpty() {
return isNull(this.getValue());
}

get hasPendingValue() {
return this.pendingValue !== undefined && !isEqual(this.pendingValue, this.normalizedValue);
}

get normalizedValue() {
return this.$$value;
gabrieldutra marked this conversation as resolved.
Show resolved Hide resolved
}

// TODO: Remove this property when finally moved to React
get ngModel() {
return this.normalizedValue;
}
gabrieldutra marked this conversation as resolved.
Show resolved Hide resolved

set ngModel(value) {
this.setValue(value);
}

clone() {
return Parameter.create(this);
}

setValue(value) {
const normalizedValue = this.constructor.normalizeValue(value);
this.value = normalizedValue;
this.$$value = normalizedValue;

this.clearPendingValue();
return this;
}

getValue() {
return this.value;
}

setPendingValue(value) {
this.pendingValue = this.constructor.normalizeValue(value);
}

applyPendingValue() {
if (this.hasPendingValue) {
this.setValue(this.pendingValue);
}
}

clearPendingValue() {
this.pendingValue = undefined;
}

toUrlParams() {
const prefix = this.urlPrefix;
return {
[`${prefix}${this.name}`]: !this.isEmpty ? this.value : null,
[`${prefix}${this.name}.start`]: null,
[`${prefix}${this.name}.end`]: null,
};
}

fromUrlParams(query) {
const prefix = this.urlPrefix;
const key = `${prefix}${this.name}`;
if (has(query, key)) {
this.setValue(query[key]);
}
}

toQueryTextFragment() {
return `{{ ${this.name} }}`;
}
}

export default Parameter;
19 changes: 19 additions & 0 deletions client/app/services/parameters/TextParameter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { toString, isEmpty } from 'lodash';
import { Parameter } from '.';

class TextParameter extends Parameter {
constructor(parameter, parentQueryId) {
super(parameter, parentQueryId);
this.setValue(parameter.value);
}

static normalizeValue(value) {
const normalizedValue = toString(value);
if (isEmpty(normalizedValue)) {
return null;
}
return normalizedValue;
}
}
gabrieldutra marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggesting addition:

isEmptyValue(value) {
  return !trim(value);
}

export default TextParameter;
3 changes: 3 additions & 0 deletions client/app/services/parameters/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { default as Parameter } from './Parameter';
export { default as TextParameter } from './TextParameter';
export { default as NumberParameter } from './NumberParameter';