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.

Sign up for GitHub

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

[indexPatterns/create] refactor time field options #11996

Merged
merged 15 commits into from
May 26, 2017
Merged

[indexPatterns/create] refactor time field options #11996

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
942e937
[indexPatterns/create] timeFields -> timeFieldOptions
spalger May 25, 2017
f8caf83
Merge branch 'master' of github.com:elastic/kibana into index-pattern…
spalger May 25, 2017
4cb39d3
[indexPatterns/create] populate `this.timeFieldOptions`
spalger May 25, 2017
c3c3829
[indexPatterns/create] unify fetching and errors for timeFieldOptions
spalger May 25, 2017
ca93050
[indexPatterns/create] refreshFieldList() -> refreshTimeFieldOptions()
spalger May 25, 2017
1cf49c0
[indexPatterns/create] unify "No time fields available" messages
spalger May 25, 2017
b3f7d83
[indexPatterns/create] disable the timeField select when loading
spalger May 25, 2017
8ea9813
[indexPatterns/create] only require a time field is there are options
spalger May 25, 2017
8d1df86
[indexPatterns/create] isolate side-effects
spalger May 25, 2017
b8a7f4f
[indexPatterns/create] let refreshTimeFieldOptions() control model
spalger May 25, 2017
6ad5ff8
[indexPatterns/create] attempt to make default field selection clearer
spalger May 25, 2017
cebb6a6
[indexPatterns/create] explain why we set timeFieldOption
spalger May 25, 2017
7e33e0f
[indexPatterns/create] make timeFieldName `undefined` when not set
spalger May 25, 2017
4eab34b
[indexPatterns/create] Don't pass two args to .then() unless needed
spalger May 25, 2017
54a80f6
💅
spalger May 25, 2017
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
22 changes: 8 additions & 14 deletions .../kibana/public/management/sections/indices/create_index_pattern/create_index_pattern.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@
<!-- Input error text -->
<div
class="kuiVerticalRhythm"
ng-if="controller.fetchFieldsError"
ng-if="controller.timeFieldOptionsError"
>
<p class="kuiText">
<span class="kuiStatusText kuiStatusText--error">
<span class="kuiStatusText__icon kuiIcon fa-warning"></span>
{{controller.fetchFieldsError}}
{{controller.timeFieldOptionsError}}
</span>
</p>
</div>
Expand Down Expand Up @@ -105,7 +105,7 @@
<small>
<a
class="kuiLink"
ng-click="controller.refreshFieldList();"
ng-click="controller.refreshTimeFieldOptions();"
translate="KIBANA-REFRESH_FIELDS"
></a>
</small>
Expand All @@ -115,17 +115,11 @@
<select
class="kuiSelect kuiSelect--large kuiVerticalRhythmSmall"
data-test-subj="createIndexPatternTimeFieldSelect"
ng-disabled="controller.fetchFieldsError || controller.dateFields.length === 1"
ng-required="!controller.fetchFieldsError"
ng-options="field.name for field in controller.dateFields"
ng-model="controller.formValues.timeField"
ng-disabled="controller.isLoading() || controller.timeFieldOptionsError || controller.timeFieldOptions.length === 1"
ng-required="controller.timeFieldOptions.length"
ng-options="option.display for option in controller.timeFieldOptions"
ng-model="controller.formValues.timeFieldOption"
></select>

<p
class="kuiSubText kuiVerticalRhythmSmall"
ng-if="!controller.fetchFieldsError && !controller.indexHasDateFields"
translate="KIBANA-INDICES_DONT_CONTAIN_TIME_FIELDS"
></p>
</div>
</div>

Expand Down Expand Up @@ -347,7 +341,7 @@
<!-- Form actions -->
<button
data-test-subj="createIndexPatternCreateButton"
ng-disabled="form.$invalid || controller.fetchFieldsError || controller.isLoading()"
ng-disabled="form.$invalid || controller.timeFieldOptionsError || controller.isLoading()"
class="kuiButton kuiButton--primary kuiVerticalRhythm"
type="submit"
>
Expand Down
205 changes: 101 additions & 104 deletions ...ns/kibana/public/management/sections/indices/create_index_pattern/create_index_pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,37 +29,23 @@ uiModules.get('apps/management')
nameIsPattern: false,
expandable: false,
nameInterval: _.find(intervals, { name: 'daily' }),
timeField: null,
timeFieldOption: null,
};

// UI state.
this.dateFields = null;
this.timeFieldOptions = [];
this.timeFieldOptionsError = null;
this.sampleCount = 5;
this.samples = null;
this.existing = null;
this.nameIntervalOptions = intervals;
this.patternErrors = [];
this.fetchFieldsError = null;

const TIME_FILTER_FIELD_OPTIONS = {
NO_DATE_FIELD_DESIRED: {
name: $translate.instant('KIBANA-NO_DATE_FIELD_DESIRED')
},
NO_DATE_FIELDS_IN_INDICES: {
name: $translate.instant('KIBANA-NO_DATE_FIELDS_IN_INDICES')
}
};

const fetchFieldList = () => {
this.dateFields = null;
this.formValues.timeField = null;
let fetchFieldsError;
let dateFields;

const getTimeFieldOptions = () => {
const missingPattern = !this.formValues.name;
const missingInterval = this.formValues.nameIsPattern && !this.formValues.nameInterval;
if (missingPattern || missingInterval) {
return;
return Promise.resolve({ options: [] });
}

loadingCount += 1;
Expand All @@ -69,79 +55,81 @@ uiModules.get('apps/management')

return indexPatterns.mapper.getFieldsForIndexPattern(pattern, {
skipIndexPatternCache: true,
})
.catch((err) => {
// TODO: we should probably display a message of some kind
});
})
.then(
fields => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this is on a newline and not:

})
.then(fields =>

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it lines up with the errorback below. With this version of the implementation it's not necessary to use a single .then(), so I'll switch it to .then().catch()

const dateFields = fields.filter(field => field.type === 'date');

if (dateFields.length === 0) {
return {
options: [
{
display: $translate.instant('KIBANA-INDICES_DONT_CONTAIN_TIME_FIELDS')
}
]
};
}

return {
options: [
{
display: $translate.instant('KIBANA-NO_DATE_FIELD_DESIRED')
},
...dateFields.map(field => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

display: field.name,
fieldName: field.name
})),
]
};
},
err => {
if (err instanceof IndexPatternMissingIndices) {
fetchFieldsError = $translate.instant('KIBANA-INDICES_MATCH_PATTERN');
return [];
return {
error: $translate.instant('KIBANA-INDICES_MATCH_PATTERN')
};
}

throw err;
});
})
.then(fields => {
if (fields.length > 0) {
fetchFieldsError = null;
dateFields = fields.filter(field => field.type === 'date');
}

return {
fetchFieldsError,
dateFields,
};
}, notify.fatal)
)
.finally(() => {
loadingCount -= 1;
});
};

const updateFieldList = results => {
this.fetchFieldsError = results.fetchFieldsError;
if (this.fetchFieldsError) {
return;
}
const findTimeFieldOption = match => {
if (!match) return;

this.dateFields = results.dateFields || [];
this.indexHasDateFields = this.dateFields.length > 0;
const moreThanOneDateField = this.dateFields.length > 1;
if (this.indexHasDateFields) {
this.dateFields.unshift(TIME_FILTER_FIELD_OPTIONS.NO_DATE_FIELD_DESIRED);
} else {
this.dateFields.unshift(TIME_FILTER_FIELD_OPTIONS.NO_DATE_FIELDS_IN_INDICES);
}

if (!moreThanOneDateField) {
// At this point the `dateFields` array contains the date fields and the "no selection"
// option. When we have less than two date fields we choose the last option, which will
// be the "no date fields available" option if there are zero date fields, or the only
// date field if there is one.
this.formValues.timeField = this.dateFields[this.dateFields.length - 1];
}
return this.timeFieldOptions.find(option => (
// comparison is not done with _.isEqual() because options get a unique
// `$$hashKey` tag attached to them by ng-repeat
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ this comment

option.fieldName === match.fieldName &&
option.display === match.display
));
};

const updateFieldListAndSetTimeField = (results, timeFieldName) => {
updateFieldList(results);
const pickDefaultTimeFieldOption = () => {
const noOptions = this.timeFieldOptions.length === 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should try and use pure functions as much as possible, so in this case, can we simply use an argument timeFieldOptions instead of relying on this.?

Copy link
Contributor Author

@spalger spalger May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This is another thing I tried and reverted, mostly because of the ambiguity caused by the existence of multiple timeFieldOptions in scope, and it's strong dependence on the structure of the shape of timeFieldOptions. If we moved this function out of the controller module along with the rest of the timeFieldOptions methods I don't think it would be ambiguous, but that seems like a much larger change than necessary.

That said, this function intentionally doesn't produce side effects so that it's pure by some definitions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd actually be in favor of that decision (to move it outside of the controller). Yes it doesn't produce side effects and that might be the more important aspect of pure functions but it still does rely on scoped state when it doesn't need to. I have no desire to hold up the review for this particular topic, as I'm just trying to understand how everyone feels about this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

strong dependence on the structure of the shape of timeFieldOptions

By this I mean to say that every time this.timeFieldOptions is modified in the future this function and its logic needs to be considered. Unfortunately, passing the timeFieldOptions in as an argument doesn't change that but, IMO, creates an illusion of decoupling.

Example:
Say I wanted to add a property to each timeFieldOption I would probably track down where they are created, then where they are stored, and then where they are used. In order to know that this function uses the this.timeFieldOptions array I would first have to find the location that reads this.timeFieldOption and then passes it to this function because pickDefaultTimeFieldOption()'s dependence on this.timeFieldOptions is essentially hidden. If instead I leave this function accessing this.timeFieldOption directly it's obvious that it is dependent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea that makes sense, and I don't think there is any decoupling happening here. There is and will always be a hard dependence, but I don't think the changes I'm suggesting make that coupling any less obvious, as when you're doing a search for this.timeFieldOptions, you'd see it passed into the function.

But as a pure function, you can:

  • Easily unit test as you're not dependent on any scoped state, or instance data to setup or tear down
  • Easily refactor the internals with zero risk of side effects
  • Easily migrate to a new file
  • Easily refactor/rename outside scoped/instanced variables without worrying about them affecting the internals of the function (you just might need to update the name of the provided parameter)
  • Easily document/explain the purpose of the function in the context of input and output

IMO, it seems like an easy win to strive for pure as much as possible. It's very possible that I'm not seeing a part of this that makes it harder or undesired and open to listening to other perspectives but the more codebases that I've worked in and the longer I've been writing code, the more I'm thinking that pure functions are a win/win across the board.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @chrisronline's reasoning here. I did some similar extraction as part of https://github.com/elastic/kibana/pull/11285/files#diff-053c72f0b419832d5edb39bd81422e06R1 and it made the controller logic easier to follow because it was higher level.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with this not going in right now. I think I'm going to open a discussion issue about this in general to get some general thoughts and best practices that others have learned. I do think it's an important conversation but it doesn't have to happen in this PR

const fieldOptions = this.timeFieldOptions.filter(option => !!option.fieldName);
const infoOptions = this.timeFieldOptions.filter(option => !option.fieldName);
Copy link
Contributor

Choose a reason for hiding this comment

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

What are infoOptions? Maybe it's better to describe these as dummyOptions or nonSelectableOptions to make their role clearer in this context?

And instead of deriving this state based on fieldName, can we just add a isSelectable: true property to the options on line 82? This would make the logic clearer:

const selectableOptions = this.timeFieldOptions.filter(option => option.isSelectable);
const nonSelectableOptions = this.timeFieldOptions.filter(option => !option.isSelectable);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, they're all selectable, they are really just "non-field" options. I could use that as the description rather than "info" if you think that's better.

const tooManyOptions = fieldOptions.length > 1 || infoOptions.length > 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it mean for there to be too many options? Too many to do what? Wondering if there's a name that can communicate the intent more clearly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Too many options to choose the default. If there are multiple field or nonField options then we can't pick the default, the user has to choose something... open to suggestions for better names.


if (!results.dateFields.length) {
return;
if (noOptions || tooManyOptions) {
return null;
}

const matchingTimeField = results.dateFields.find(field => field.name === timeFieldName);

//assign the field from the results-list
//angular recreates a new timefield instance, each time the list is refreshed.
//This ensures the selected field matches one of the instances in the list.
if (matchingTimeField) {
this.formValues.timeField = matchingTimeField;
if (fieldOptions.length === 1) {
return fieldOptions[0];
}

return infoOptions[0];
};

const resetIndex = () => {
this.patternErrors = [];
this.samples = null;
this.existing = null;
this.fetchFieldsError = null;
};

function mockIndexPattern(index) {
Expand Down Expand Up @@ -207,39 +195,54 @@ uiModules.get('apps/management')
return loadingCount > 0;
};

this.refreshFieldList = () => {
const timeField = this.formValues.timeField;
this.refreshTimeFieldOptions = () => {
const prevOption = this.formValues.timeFieldOption;

loadingCount += 1;
fetchFieldList().then(results => {
if (timeField) {
updateFieldListAndSetTimeField(results, timeField.name);
} else {
updateFieldList(results);
}
}).finally(() => {
loadingCount -= 1;
});
this.timeFieldOptions = [];
this.timeFieldOptionsError = null;
this.formValues.timeFieldOption = null;
getTimeFieldOptions()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we aren't opting for the async/await pattern here and using promises directly?

Copy link
Contributor Author

@spalger spalger May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, async/await don't work with angular for the same reason we can't use the global Promise contrustor :/ See #11632 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Why doesn't a simple $scope.$apply() not work in those scenarios? Or is that not recommended for other reasons?

Copy link
Contributor Author

@spalger spalger May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Manually triggering digest cycles is really a last resort option and can lead to all sorts of issues even in simple apps.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's interesting. I'd love to hear more about this. Do you have any resources and/or past github issues around this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm very interested in understanding the concerns around explicit $scope.$apply(). I've always considered it a valid option and the best option when dealing with async operations outside of angular.

I agree that sometimes $scope.$apply() is the only way to inter-op between angular and non-angular async contexts. My concern is that using $scope.$apply() when not absolutely necessary leads to trouble. We did this in Kibana 3 and ended up with more than a couple PR's that just converted a $scope.$apply() to if (!$scope.$$phase) $scope.$apply() because things started to get out of hand. I'm just saying that if we can avoid using $scope.$apply() (like by using native angular promises) then I think we should.

Copy link
Contributor

@cjcenizal cjcenizal May 25, 2017 •
edited by spalger
Loading

Choose a reason for hiding this comment

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

I agree, we should avoid using $apply where possible. I think the way to do that is to follow a few simple rules:

  1. Prefer Angular DOM event handlers like ng-click over event handlers (like $(element).on('click', () => {}) and element.addEventListener('click', () => {}).'
  2. Prefer Angular wrappers around global objects like $document and $window.
  3. Prefer Angular wrappers around async APIs like $q, $timeout, and $interval.

There are probably a few others we can dig up by Googling but the general theme is to just use whatever tools Angular makes available which will tie into the $digest cycle under the hood. These are situations where $apply is inappropriate.

This leaves us with the situations where $apply is appropriate: anywhere these tools can't be used. I think if we can confidently assess a call stack and state that it will never touch any of these tools, and therefore never trigger a $digest cycle on its own, then we can safely use $apply. Thoughts @spalger and @chrisronline ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cjcenizal by that logic there are no function in this PR that qualify unless we reimplement some core services. All of the asynchronicity in this controller is introduced by the indexPattern.mapper, savedObjects, and esAdmin services, all of which use angular async/http libraries.

If you're suggesting that we write more code that is completely disconnected from angular, that only inter-ops with angular at the last minute using something like $scope.$apply() or wrapping a non angular promise with $q.resolve() then I misunderstood you because I am totally on board with that. Unfortunately we have to adapt a lot of services to not use angular async/http first, or do something (module, app, component) greenfield and use non-angular libraries for everything from async to http.

My stance in this discussion so far was against using angular services in a non-angular context (like an async function() {} that uses the indexPattern.mapper, or esAdmin).

This specific pr is even worse in my opinion, converting any of the helpers to async function(){}s would be creating non-angular contexts between angular services and other angular contexts (Angular controller -> non-angular async function() {} -> angular services). As you stated, this is something we should probably avoid.

Copy link
Contributor

Choose a reason for hiding this comment

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

All of the asynchronicity in this controller is introduced by the indexPattern.mapper, savedObjects, and esAdmin services, all of which use angular async/http libraries.

Then I guess we can't use $apply. :)

My stance in this discussion so far was against using angular services in a non-angular context (like an async function() {} that uses the indexPattern.mapper, or esAdmin).

I see, sorry, I didn't pick up on that earlier. I thought we were just talking about how to use $apply correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@spalger I think that's an excellent point and something I wasn't considering fully. You're right in that I'm advising to add a layer from angular that needs to go back into angular and that's probably not a good idea unless there is a really good reason. I'm happy with the discussion around this and I 100% agree that we should try and decouple our business logic away from our angular implementation. With that said, I'm fine with the code as-is and appreciate the thoughts!

.then(results => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels like a good place for object destructuring: .then({ options, error } =>

this.timeFieldOptions = results.options;
this.timeFieldOptionsError = results.error;
if (!this.timeFieldOptions) {
return;
}

const restoredOption = findTimeFieldOption(prevOption);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a brief comment to describe these 3 lines?

// Persist the selected state in the UI.

const defaultOption = pickDefaultTimeFieldOption();
this.formValues.timeFieldOption = restoredOption || defaultOption;
})
.catch(notify.error)
.finally(() => {
loadingCount -= 1;
});
};

this.createIndexPattern = () => {
const id = this.formValues.name;
let timeFieldName;
if ((this.formValues.timeField !== TIME_FILTER_FIELD_OPTIONS.NO_DATE_FIELD_DESIRED)
&& (this.formValues.timeField !== TIME_FILTER_FIELD_OPTIONS.NO_DATE_FIELDS_IN_INDICES)) {
timeFieldName = this.formValues.timeField.name;
}
const {
name,
timeFieldOption,
nameIsPattern,
nameInterval,
expandable
} = this.formValues;

// Only event-time-based index patterns set an intervalName.
const intervalName =
this.formValues.nameIsPattern
? this.formValues.nameInterval.name
: undefined;
const id = name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just rename the variable in the above destructing assignment instead?

const {
  name: id,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I had it like that at one point, but found it easier to read when all of the values passed to sendCreateIndexPatternRequest() were defined in the same way (rather than using destructuring-rename for one and not others)


const notExpandable =
!this.formValues.expandable && this.canExpandIndices()
const timeFieldName = timeFieldOption && timeFieldOption.fieldName;
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable name makes me think it's a string but the code indicates it's a boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ends up being the field name or false, which isn't what I want... good catch


// this seems wrong, but it's the original logic... https://git.io/vHYFo
const notExpandable = (!expandable && this.canExpandIndices())
? true
: undefined;

// Only event-time-based index patterns set an intervalName.
const intervalName = (nameIsPattern && nameInterval)
? nameInterval.name
: undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why undefined instead of false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my eyes false represents "no", and undefined/null represent "no value" (with slightly different semantics depending on who you ask).

This is trying to represent "no intervalName", so I don't think false is appropriate.


loadingCount += 1;
sendCreateIndexPatternRequest(indexPatterns, {
id,
Expand Down Expand Up @@ -287,7 +290,6 @@ uiModules.get('apps/management')

if (!nameIsPattern) {
delete this.formValues.nameInterval;
delete this.formValues.timeField;
} else {
this.formValues.nameInterval = this.formValues.nameInterval || intervals.byName.days;
this.formValues.name = this.formValues.name || getDefaultPatternForInterval(this.formValues.nameInterval);
Expand Down Expand Up @@ -330,28 +332,23 @@ uiModules.get('apps/management')
.finally(() => {
// prevent running when no change happened (ie, first watcher call)
if (!_.isEqual(newVal, oldVal)) {
fetchFieldList().then(results => {
if (lastPromise === samplePromise) {
updateFieldList(results);
samplePromise = null;
}
});
this.refreshTimeFieldOptions();
}
});
});

$scope.$watchMulti([
'controller.sampleCount'
], () => {
this.refreshFieldList();
this.refreshTimeFieldOptions();
});

$scope.$watchMulti([
'controller.isLoading()',
'form.name.$error.indexNameInput',
'controller.formValues.timeField'
], ([loading, invalidIndexName, timeField]) => {
const state = { loading, invalidIndexName, timeField };
'controller.formValues.timeFieldOption'
], ([loading, invalidIndexName, timeFieldOption]) => {
const state = { loading, invalidIndexName, timeFieldOption };
this.createButtonText = pickCreateButtonText($translate, state);
});
});
4 changes: 2 additions & 2 deletions ...kibana/public/management/sections/indices/create_index_pattern/pick_create_button_text.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ export function pickCreateButtonText($translate, state) {
const {
loading,
invalidIndexName,
timeField
timeFieldOption
} = state;

if (loading) {
Expand All @@ -13,7 +13,7 @@ export function pickCreateButtonText($translate, state) {
return $translate.instant('KIBANA-INVALID_INDEX_PATTERN');
}

if (!timeField) {
if (!timeFieldOption) {
return $translate.instant('KIBANA-FIELD_IS_REQUIRED', {
fieldName: $translate.instant('KIBANA-TIME_FILTER_FIELD_NAME')
});
Expand Down
1 change: 0 additions & 1 deletion src/core_plugins/kibana/translations/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
"KIBANA-MORE": "more",
"KIBANA-TIME_FILTER_FIELD_NAME": "Time Filter field name",
"KIBANA-NO_DATE_FIELD_DESIRED": "I don't want to use the Time Filter",
"KIBANA-NO_DATE_FIELDS_IN_INDICES": "None available",
"KIBANA-REFRESH_FIELDS": "refresh fields",
"KIBANA-INDICES_DONT_CONTAIN_TIME_FIELDS": "The indices which match this index pattern don't contain any time fields.",
"KIBANA-INVALID_INDEX_PATTERN": "Invalid index name pattern.",
Expand Down