Introduce inheritance to the Parameter structure #4049
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import { findKey, startsWith, has, includes, isNull, values } from 'lodash'; | ||
| import moment from 'moment'; | ||
| import PropTypes from 'prop-types'; | ||
| import { Parameter } from '.'; | ||
|
|
||
| const DATETIME_FORMATS = { | ||
| // eslint-disable-next-line quote-props | ||
| 'date': 'YYYY-MM-DD', | ||
| 'datetime-local': 'YYYY-MM-DD HH:mm', | ||
| 'datetime-with-seconds': 'YYYY-MM-DD HH:mm:ss', | ||
| }; | ||
|
|
||
| const DYNAMIC_PREFIX = 'd_'; | ||
|
|
||
| const DYNAMIC_DATES = { | ||
| now: { | ||
| name: 'Today/Now', | ||
| value: () => moment(), | ||
|
There was a problem hiding this comment. Just curious: shouldn't it be There was a problem hiding this comment. In this case we can and want to use the user's TZ (I believe it's less confusing and it reaches more use cases). There was a problem hiding this comment. When user selects a dynamic value for parameter - what do we pass to a server when query is executed: "magic" constant or computed value? There was a problem hiding this comment. Computed value (actual "dates/date ranges") There was a problem hiding this comment. Okay. I think, it may be an issue. Imagine that there is a query like So shouldn't we always pass date/time/range params in UTC to make them predictable and not dependent on DB config and user's locale settings? There was a problem hiding this comment. 🤔 you have a point, from my experience I've always used I still have the feeling that the user TZ is the less confusing when considering the user, so I believe it's not a bug, it's one acceptable behavior for this. But we could eventually offer more options to solve this. This is certainly not in the scope of this PR, so I say we should move this discussion to somewhere else with more visibility (forum or create an issue). There was a problem hiding this comment. I see your point about It's definitely not in the scope of this PR, but we have to discuss it somewhere. There was a problem hiding this comment. Let's have it as There was a problem hiding this comment. The current behaviour is |
||
| }, | ||
| yesterday: { | ||
| name: 'Yesterday', | ||
| value: () => moment().subtract(1, 'day'), | ||
| }, | ||
| }; | ||
|
|
||
| export const DynamicDateType = PropTypes.oneOf(values(DYNAMIC_DATES)); | ||
|
|
||
| function isDynamicDateString(value) { | ||
| return startsWith(value, DYNAMIC_PREFIX) && has(DYNAMIC_DATES, value.substring(DYNAMIC_PREFIX.length)); | ||
| } | ||
|
|
||
| export function isDynamicDate(value) { | ||
| return includes(DYNAMIC_DATES, value); | ||
| } | ||
|
|
||
| export function getDynamicDateFromString(value) { | ||
| if (!isDynamicDateString(value)) { | ||
| return null; | ||
| } | ||
| return DYNAMIC_DATES[value.substring(DYNAMIC_PREFIX.length)]; | ||
| } | ||
|
|
||
| class DateParameter extends Parameter { | ||
| constructor(parameter, parentQueryId) { | ||
| super(parameter, parentQueryId); | ||
| this.useCurrentDateTime = parameter.useCurrentDateTime; | ||
| this.setValue(parameter.value); | ||
| } | ||
|
|
||
| get hasDynamicValue() { | ||
| return isDynamicDate(this.normalizedValue); | ||
| } | ||
|
|
||
| // eslint-disable-next-line class-methods-use-this | ||
| normalizeValue(value) { | ||
| if (isDynamicDateString(value)) { | ||
| return getDynamicDateFromString(value); | ||
| } | ||
|
|
||
| if (isDynamicDate(value)) { | ||
| return value; | ||
| } | ||
|
|
||
| const normalizedValue = moment(value); | ||
| return normalizedValue.isValid() ? normalizedValue : null; | ||
| } | ||
|
|
||
| setValue(value) { | ||
| const normalizedValue = this.normalizeValue(value); | ||
| if (isDynamicDate(normalizedValue)) { | ||
| this.value = DYNAMIC_PREFIX + findKey(DYNAMIC_DATES, normalizedValue); | ||
| } else if (moment.isMoment(normalizedValue)) { | ||
| this.value = normalizedValue.format(DATETIME_FORMATS[this.type]); | ||
| } else { | ||
| this.value = normalizedValue; | ||
| } | ||
| this.$$value = normalizedValue; | ||
|
|
||
| this.updateLocals(); | ||
| this.clearPendingValue(); | ||
| return this; | ||
| } | ||
|
|
||
| getExecutionValue() { | ||
| if (this.hasDynamicValue) { | ||
| return this.normalizedValue.value().format(DATETIME_FORMATS[this.type]); | ||
| } | ||
| if (isNull(this.value) && this.useCurrentDateTime) { | ||
| return moment().format(DATETIME_FORMATS[this.type]); | ||
| } | ||
| return this.value; | ||
| } | ||
| } | ||
|
|
||
| export default DateParameter; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove this line and just use
value.