Commit 875f7bff authored by Miguel Rincon's avatar Miguel Rincon Committed by Mike Greiling

Fixes issues with alert modal

- Prevent triggering a premature modal dismissal on errors
- Selects a default operator to help the user
- Places the error message with correct spacing
parent b6e01b7a
...@@ -54,9 +54,6 @@ export default { ...@@ -54,9 +54,6 @@ export default {
.map(this.formatAlertSummary) .map(this.formatAlertSummary)
.join(', '); .join(', ');
}, },
formDisabled() {
return Boolean(this.errorMessage || this.isLoading);
},
supportsComputedAlerts() { supportsComputedAlerts() {
return gon.features && gon.features.prometheusComputedAlerts; return gon.features && gon.features.prometheusComputedAlerts;
}, },
...@@ -103,6 +100,7 @@ export default { ...@@ -103,6 +100,7 @@ export default {
return `${alertQuery.label} ${alert.operator} ${alert.threshold}`; return `${alertQuery.label} ${alert.operator} ${alert.threshold}`;
}, },
hideModal() { hideModal() {
this.errorMessage = null;
this.$root.$emit('bv::hide::modal', this.modalId); this.$root.$emit('bv::hide::modal', this.modalId);
}, },
handleSetApiAction(apiAction) { handleSetApiAction(apiAction) {
...@@ -165,7 +163,7 @@ export default { ...@@ -165,7 +163,7 @@ export default {
</span> </span>
<alert-widget-form <alert-widget-form
ref="widgetForm" ref="widgetForm"
:disabled="formDisabled" :disabled="isLoading"
:alerts-to-manage="alertsToManage" :alerts-to-manage="alertsToManage"
:relevant-queries="relevantQueries" :relevant-queries="relevantQueries"
:error-message="errorMessage" :error-message="errorMessage"
......
...@@ -152,17 +152,18 @@ export default { ...@@ -152,17 +152,18 @@ export default {
this.threshold = existingAlert.threshold; this.threshold = existingAlert.threshold;
} else { } else {
this.selectedAlert = {}; this.selectedAlert = {};
this.operator = null; this.operator = this.operators.greaterThan;
this.threshold = null; this.threshold = null;
} }
this.prometheusMetricId = queryId; this.prometheusMetricId = queryId;
}, },
handleCancel() { handleHidden() {
this.resetAlertData(); this.resetAlertData();
this.$emit('cancel'); this.$emit('cancel');
}, },
handleSubmit() { handleSubmit(e) {
e.preventDefault();
this.$emit(this.submitAction, { this.$emit(this.submitAction, {
alert: this.selectedAlert.alert_path, alert: this.selectedAlert.alert_path,
operator: this.operator, operator: this.operator,
...@@ -196,19 +197,20 @@ export default { ...@@ -196,19 +197,20 @@ export default {
:ok-variant="submitAction === 'delete' ? 'danger' : 'success'" :ok-variant="submitAction === 'delete' ? 'danger' : 'success'"
:ok-title="submitActionText" :ok-title="submitActionText"
:ok-disabled="formDisabled" :ok-disabled="formDisabled"
class="prometheus-alert-widget d-flex align-items-center"
@ok="handleSubmit" @ok="handleSubmit"
@hidden="handleHidden"
> >
<span v-if="errorMessage" class="alert-error-message"> {{ errorMessage }} </span> <div v-if="errorMessage" class="alert-modal-message danger_message">{{ errorMessage }}</div>
<div class="alert-form"> <div class="alert-form">
<gl-form-group <gl-form-group
v-if="supportsComputedAlerts" v-if="supportsComputedAlerts"
:label="$options.alertQueryText.label" :label="$options.alertQueryText.label"
label-for="alert-query-input"
:valid-feedback="$options.alertQueryText.validFeedback" :valid-feedback="$options.alertQueryText.validFeedback"
:invalid-feedback="$options.alertQueryText.invalidFeedback" :invalid-feedback="$options.alertQueryText.invalidFeedback"
:state="isValidQuery" :state="isValidQuery"
> >
<gl-form-input v-model.trim="alertQuery" :state="isValidQuery" /> <gl-form-input id="alert-query-input" v-model.trim="alertQuery" :state="isValidQuery" />
<template #description> <template #description>
<div class="d-flex align-items-center"> <div class="d-flex align-items-center">
{{ __('Single or combined queries') }} {{ __('Single or combined queries') }}
...@@ -220,20 +222,21 @@ export default { ...@@ -220,20 +222,21 @@ export default {
</div> </div>
</template> </template>
</gl-form-group> </gl-form-group>
<gl-dropdown <gl-form-group v-else label-for="alert-query-dropdown" :label="$options.alertQueryText.label">
v-else <gl-dropdown
:text="queryDropdownLabel" id="alert-query-dropdown"
class="form-group" :text="queryDropdownLabel"
toggle-class="dropdown-menu-toggle" toggle-class="dropdown-menu-toggle"
>
<gl-dropdown-item
v-for="query in relevantQueries"
:key="query.metricId"
@click="selectQuery(query.metricId)"
> >
{{ query.label }} <gl-dropdown-item
</gl-dropdown-item> v-for="query in relevantQueries"
</gl-dropdown> :key="query.metricId"
@click="selectQuery(query.metricId)"
>
{{ query.label }}
</gl-dropdown-item>
</gl-dropdown>
</gl-form-group>
<gl-button-group class="mb-2" :label="s__('PrometheusAlerts|Operator')"> <gl-button-group class="mb-2" :label="s__('PrometheusAlerts|Operator')">
<gl-button <gl-button
:class="{ active: operator === operators.greaterThan }" :class="{ active: operator === operators.greaterThan }"
......
...@@ -117,13 +117,13 @@ ...@@ -117,13 +117,13 @@
vertical-align: middle; vertical-align: middle;
} }
.alert-form { .alert-modal-message {
padding: $gl-padding $gl-padding $gl-padding-8; margin-left: -1rem;
margin-right: -3rem;
label { margin-top: -1rem;
font-weight: normal; }
}
.alert-form {
.btn-group, .btn-group,
.action-group { .action-group {
display: flex; display: flex;
......
---
title: 'Monitor charts: Validate form for creating an alert before submitting'
merge_request: 17109
author:
type: fixed
...@@ -36,6 +36,13 @@ describe('AlertWidgetForm', () => { ...@@ -36,6 +36,13 @@ describe('AlertWidgetForm', () => {
const modal = () => wrapper.find(GlModal); const modal = () => wrapper.find(GlModal);
const modalTitle = () => modal().attributes('title'); const modalTitle = () => modal().attributes('title');
const submitText = () => modal().attributes('ok-title'); const submitText = () => modal().attributes('ok-title');
const e = {
preventDefault: jest.fn(),
};
beforeEach(() => {
e.preventDefault.mockReset();
});
afterEach(() => { afterEach(() => {
if (wrapper) wrapper.destroy(); if (wrapper) wrapper.destroy();
...@@ -62,10 +69,9 @@ describe('AlertWidgetForm', () => { ...@@ -62,10 +69,9 @@ describe('AlertWidgetForm', () => {
createComponent(); createComponent();
wrapper.vm.selectQuery('9'); wrapper.vm.selectQuery('9');
wrapper.vm.operator = '>';
wrapper.vm.threshold = 900; wrapper.vm.threshold = 900;
wrapper.vm.handleSubmit(); wrapper.vm.handleSubmit(e);
expect(wrapper.emitted().create[0]).toEqual([ expect(wrapper.emitted().create[0]).toEqual([
{ {
...@@ -75,6 +81,22 @@ describe('AlertWidgetForm', () => { ...@@ -75,6 +81,22 @@ describe('AlertWidgetForm', () => {
prometheus_metric_id: '9', prometheus_metric_id: '9',
}, },
]); ]);
expect(e.preventDefault).toHaveBeenCalledTimes(1);
});
it('resets form when modal is dismissed (hidden)', () => {
createComponent();
wrapper.vm.selectQuery('9');
wrapper.vm.selectQuery('>');
wrapper.vm.threshold = 800;
wrapper.find(GlModal).vm.$emit('hidden');
expect(wrapper.vm.selectedAlert).toEqual({});
expect(wrapper.vm.operator).toBe(null);
expect(wrapper.vm.threshold).toBe(null);
expect(wrapper.vm.prometheusMetricId).toBe(null);
}); });
describe('with existing alert', () => { describe('with existing alert', () => {
...@@ -90,7 +112,7 @@ describe('AlertWidgetForm', () => { ...@@ -90,7 +112,7 @@ describe('AlertWidgetForm', () => {
}); });
it('emits "delete" event when form values unchanged', () => { it('emits "delete" event when form values unchanged', () => {
wrapper.vm.handleSubmit(); wrapper.vm.handleSubmit(e);
expect(wrapper.emitted().delete[0]).toEqual([ expect(wrapper.emitted().delete[0]).toEqual([
{ {
...@@ -100,12 +122,13 @@ describe('AlertWidgetForm', () => { ...@@ -100,12 +122,13 @@ describe('AlertWidgetForm', () => {
prometheus_metric_id: '8', prometheus_metric_id: '8',
}, },
]); ]);
expect(e.preventDefault).toHaveBeenCalledTimes(1);
}); });
it('emits "update" event when form changed', () => { it('emits "update" event when form changed', () => {
wrapper.vm.threshold = 11; wrapper.vm.threshold = 11;
wrapper.vm.handleSubmit(); wrapper.vm.handleSubmit(e);
expect(wrapper.emitted().update[0]).toEqual([ expect(wrapper.emitted().update[0]).toEqual([
{ {
...@@ -115,6 +138,7 @@ describe('AlertWidgetForm', () => { ...@@ -115,6 +138,7 @@ describe('AlertWidgetForm', () => {
prometheus_metric_id: '8', prometheus_metric_id: '8',
}, },
]); ]);
expect(e.preventDefault).toHaveBeenCalledTimes(1);
}); });
}); });
}); });
...@@ -61,6 +61,8 @@ describe('AlertWidget', () => { ...@@ -61,6 +61,8 @@ describe('AlertWidget', () => {
// expect loading spinner to exist during fetch // expect loading spinner to exist during fetch
expect(vm.isLoading).toBeTruthy(); expect(vm.isLoading).toBeTruthy();
expect(vm.$refs.widgetForm.$props.disabled).toBe(true);
expect(vm.$el.querySelector('.loading-container')).toBeVisible(); expect(vm.$el.querySelector('.loading-container')).toBeVisible();
resolveReadAlert({ operator: '=', threshold: 42 }); resolveReadAlert({ operator: '=', threshold: 42 });
...@@ -70,6 +72,7 @@ describe('AlertWidget', () => { ...@@ -70,6 +72,7 @@ describe('AlertWidget', () => {
vm.$nextTick(() => { vm.$nextTick(() => {
expect(vm.isLoading).toEqual(false); expect(vm.isLoading).toEqual(false);
expect(vm.$el.querySelector('.loading-container')).toBeHidden(); expect(vm.$el.querySelector('.loading-container')).toBeHidden();
expect(vm.$refs.widgetForm.$props.disabled).toBe(false);
done(); done();
}), }),
); );
...@@ -155,6 +158,17 @@ describe('AlertWidget', () => { ...@@ -155,6 +158,17 @@ describe('AlertWidget', () => {
.catch(done.fail); .catch(done.fail);
}); });
it('dismisses error message when action is cancelled', () => {
vm = mountComponent(AlertWidgetComponent, props);
vm.$on('setAlerts', mockSetAlerts);
vm.errorMessage = 'Mock error message.';
// widget modal is dismissed
vm.$refs.widgetForm.$emit('cancel');
expect(vm.errorMessage).toBe(null);
});
it('updates an alert with an appropriate handler', done => { it('updates an alert with an appropriate handler', done => {
const alertParams = { operator: '<', threshold: 4, alert_path: alertPath }; const alertParams = { operator: '<', threshold: 4, alert_path: alertPath };
const newAlertParams = { operator: '=', threshold: 12 }; const newAlertParams = { operator: '=', threshold: 12 };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment