Skip to content

Commit

Permalink
[ML] Fix slow calendar creation UI (#104248) (#104331)
Browse files Browse the repository at this point in the history
* [ML] Fix slow calendar creation UI

* updating jest snapshots

* updaing jest tests

* fixing functional tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: James Gowdy <jgowdy@elastic.co>
  • Loading branch information
kibanamachine and jgowdyelastic authored Jul 5, 2021
1 parent a65dee9 commit ff2daed
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 17 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const CalendarForm = ({
showImportModal,
onJobSelection,
saving,
loading,
selectedGroupOptions,
selectedJobOptions,
showNewEventModal,
Expand All @@ -83,7 +84,11 @@ export const CalendarForm = ({
const helpText = isNewCalendarIdValid === true && !isEdit ? msg : undefined;
const error = isNewCalendarIdValid === false && !isEdit ? [msg] : undefined;
const saveButtonDisabled =
canCreateCalendar === false || saving || !isNewCalendarIdValid || calendarId === '';
canCreateCalendar === false ||
saving ||
!isNewCalendarIdValid ||
calendarId === '' ||
loading === true;
const redirectToCalendarsManagementPage = useCreateAndNavigateToMlLink(ML_PAGES.CALENDARS_MANAGE);

return (
Expand Down Expand Up @@ -116,7 +121,7 @@ export const CalendarForm = ({
name="calendarId"
value={calendarId}
onChange={onCalendarIdChange}
disabled={isEdit === true || saving === true}
disabled={isEdit === true || saving === true || loading === true}
data-test-subj="mlCalendarIdInput"
/>
</EuiFormRow>
Expand All @@ -133,7 +138,7 @@ export const CalendarForm = ({
name="description"
value={description}
onChange={onDescriptionChange}
disabled={isEdit === true || saving === true}
disabled={isEdit === true || saving === true || loading === true}
data-test-subj="mlCalendarDescriptionInput"
/>
</EuiFormRow>
Expand All @@ -152,7 +157,7 @@ export const CalendarForm = ({
}
checked={isGlobalCalendar}
onChange={onGlobalCalendarChange}
disabled={saving === true || canCreateCalendar === false}
disabled={saving === true || canCreateCalendar === false || loading === true}
data-test-subj="mlCalendarApplyToAllJobsSwitch"
/>

Expand All @@ -172,7 +177,7 @@ export const CalendarForm = ({
options={jobIds}
selectedOptions={selectedJobOptions}
onChange={onJobSelection}
isDisabled={saving === true || canCreateCalendar === false}
isDisabled={saving === true || canCreateCalendar === false || loading === true}
data-test-subj="mlCalendarJobSelection"
/>
</EuiFormRow>
Expand All @@ -190,7 +195,7 @@ export const CalendarForm = ({
options={groupIds}
selectedOptions={selectedGroupOptions}
onChange={onGroupSelection}
isDisabled={saving === true || canCreateCalendar === false}
isDisabled={saving === true || canCreateCalendar === false || loading === true}
data-test-subj="mlCalendarJobGroupSelection"
/>
</EuiFormRow>
Expand All @@ -215,6 +220,8 @@ export const CalendarForm = ({
onDeleteClick={onEventDelete}
showImportModal={showImportModal}
showNewEventModal={showNewEventModal}
loading={loading}
saving={saving}
showSearchBar
/>
</EuiFormRow>
Expand Down Expand Up @@ -272,6 +279,7 @@ CalendarForm.propTypes = {
showImportModal: PropTypes.func.isRequired,
onJobSelection: PropTypes.func.isRequired,
saving: PropTypes.bool.isRequired,
loading: PropTypes.bool.isRequired,
selectedGroupOptions: PropTypes.array.isRequired,
selectedJobOptions: PropTypes.array.isRequired,
showNewEventModal: PropTypes.func.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { TIME_FORMAT } from '../../../../../../common/constants/time_format';

function DeleteButton({ onClick, canDeleteCalendar, testSubj }) {
function DeleteButton({ onClick, testSubj, disabled }) {
return (
<Fragment>
<EuiButtonEmpty
size="xs"
color="danger"
onClick={onClick}
isDisabled={canDeleteCalendar === false}
isDisabled={disabled}
data-test-subj={testSubj}
>
<FormattedMessage
Expand All @@ -42,6 +42,8 @@ export const EventsTable = ({
showSearchBar,
showImportModal,
showNewEventModal,
loading,
saving,
}) => {
const sorting = {
sort: {
Expand Down Expand Up @@ -93,7 +95,7 @@ export const EventsTable = ({
render: (event) => (
<DeleteButton
testSubj="mlCalendarEventDeleteButton"
canDeleteCalendar={canDeleteCalendar}
disabled={canDeleteCalendar === false || saving === true || loading === true}
onClick={() => {
onDeleteClick(event.event_id);
}}
Expand All @@ -105,7 +107,7 @@ export const EventsTable = ({
const search = {
toolsRight: [
<EuiButton
isDisabled={canCreateCalendar === false}
isDisabled={canCreateCalendar === false || saving === true || loading === true}
key="ml_new_event"
data-test-subj="mlCalendarNewEventButton"
size="s"
Expand All @@ -118,7 +120,7 @@ export const EventsTable = ({
/>
</EuiButton>,
<EuiButton
isDisabled={canCreateCalendar === false}
isDisabled={canCreateCalendar === false || saving === true || loading === true}
key="ml_import_event"
data-test-subj="mlCalendarImportEventsButton"
size="s"
Expand Down Expand Up @@ -164,6 +166,8 @@ EventsTable.propTypes = {
showImportModal: PropTypes.func,
showNewEventModal: PropTypes.func,
showSearchBar: PropTypes.bool,
loading: PropTypes.bool,
saving: PropTypes.bool,
};

EventsTable.defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ class NewCalendarUI extends Component {
groupIdOptions,
jobIdOptions,
saving,
loading,
selectedCalendar,
selectedJobOptions,
selectedGroupOptions,
Expand Down Expand Up @@ -377,6 +378,7 @@ class NewCalendarUI extends Component {
showImportModal={this.showImportModal}
onJobSelection={this.onJobSelection}
saving={saving}
loading={loading}
selectedGroupOptions={selectedGroupOptions}
selectedJobOptions={selectedJobOptions}
onCreateGroupOption={this.onCreateGroupOption}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,24 +132,22 @@ describe('NewCalendar', () => {
expect(wrapper).toMatchSnapshot();
});

test('Import modal shown on Import Events button click', () => {
test('Import modal button is disabled', () => {
const wrapper = mountWithIntl(<NewCalendar {...props} />);

const importButton = wrapper.find('[data-test-subj="mlCalendarImportEventsButton"]');
const button = importButton.find('EuiButton');
button.simulate('click');

expect(wrapper.state('isImportModalVisible')).toBe(true);
expect(button.prop('isDisabled')).toBe(true);
});

test('New event modal shown on New event button click', () => {
test('New event modal button is disabled', () => {
const wrapper = mountWithIntl(<NewCalendar {...props} />);

const importButton = wrapper.find('[data-test-subj="mlCalendarNewEventButton"]');
const button = importButton.find('EuiButton');
button.simulate('click');

expect(wrapper.state('isNewEventModalVisible')).toBe(true);
expect(button.prop('isDisabled')).toBe(true);
});

test('isDuplicateId returns true if form calendar id already exists in calendars', () => {
Expand Down
4 changes: 4 additions & 0 deletions x-pack/test/functional/apps/ml/settings/calendar_creation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ export default function ({ getService }: FtrProviderContext) {
await ml.settingsCalendar.assertCreateCalendarButtonEnabled(true);
await ml.settingsCalendar.navigateToCalendarCreationPage();

await ml.settingsCalendar.waitForFormEnabled();

await ml.testExecution.logTestStep('calendar creation sets calendar to apply to all jobs');
await ml.settingsCalendar.toggleApplyToAllJobsSwitch(true);
await ml.settingsCalendar.assertJobSelectionNotExists();
Expand Down Expand Up @@ -79,6 +81,8 @@ export default function ({ getService }: FtrProviderContext) {
await ml.settingsCalendar.assertCreateCalendarButtonEnabled(true);
await ml.settingsCalendar.navigateToCalendarCreationPage();

await ml.settingsCalendar.waitForFormEnabled();

await ml.testExecution.logTestStep(
'calendar creation verifies the job selection and job group section are displayed'
);
Expand Down
5 changes: 5 additions & 0 deletions x-pack/test/functional/services/ml/settings_calendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@ export function MachineLearningSettingsCalendarProvider(
);
},

async waitForFormEnabled() {
// @ts-expect-error null is acceptable for a disabled attribute that no longer exists.
await testSubjects.waitForAttributeToChange('mlCalendarIdInput', 'disabled', null);
},

async assertCalendarRowExists(calendarId: string) {
await testSubjects.existOrFail(this.calendarRowSelector(calendarId));
},
Expand Down

0 comments on commit ff2daed

Please sign in to comment.