Skip to content

Commit

Permalink
[Profiling] Add validations to settings (elastic#175972)
Browse files Browse the repository at this point in the history
## Summary

In elastic#174712, we integrated the new
settings field component into the Universal profiling settings. The new
setting field supports validation on the user input, based on the schema
of the given setting.

Since some of the Universal profiling settings have schema with
specified restrictions, which were not enforced in the UI before, this
PR adds schema validation so that they are enforced.

**Settings with `schema` restrictions:**

[observability:profilingCo2PerKWH](https://github.com/elastic/kibana/blob/9ee7b79d20e982eaae96c0b803ff2d17f98bbfcc/x-pack/plugins/observability/server/ui_settings.ts#L492):
>= 0

[observability:profilingDatacenterPUE](https://github.com/elastic/kibana/blob/9ee7b79d20e982eaae96c0b803ff2d17f98bbfcc/x-pack/plugins/observability/server/ui_settings.ts#L469):
>=0

[observability:profilingPerVCPUWattX86](https://github.com/elastic/kibana/blob/9ee7b79d20e982eaae96c0b803ff2d17f98bbfcc/x-pack/plugins/observability/server/ui_settings.ts#L425):
>=0

[observability:profilingPervCPUWattArm64](https://github.com/elastic/kibana/blob/9ee7b79d20e982eaae96c0b803ff2d17f98bbfcc/x-pack/plugins/observability/server/ui_settings.ts#L440):
>=0

[observability:profilingAWSCostDiscountRate](https://github.com/elastic/kibana/blob/9ee7b79d20e982eaae96c0b803ff2d17f98bbfcc/x-pack/plugins/observability/server/ui_settings.ts#L501):
>=0 and <=100

[observability:profilingCostPervCPUPerHour](https://github.com/elastic/kibana/blob/9ee7b79d20e982eaae96c0b803ff2d17f98bbfcc/x-pack/plugins/observability/server/ui_settings.ts#L523):
>=0 and <=100

<img width="1147" alt="Screenshot 2024-01-31 at 14 08 10"
src="https://github.com/elastic/kibana/assets/59341489/ace8da2e-ae0b-4d94-997b-bec3534a4bfe">

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
2 people authored and CoenWarmer committed Feb 15, 2024
1 parent 3eaa27e commit f1bd3ca
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
EuiFlexItem,
EuiHealth,
EuiText,
EuiToolTip,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
Expand All @@ -22,6 +23,7 @@ interface Props {
onDiscardChanges: () => void;
onSave: () => void;
saveLabel: string;
areChangesInvalid?: boolean;
}

export function BottomBarActions({
Expand All @@ -30,6 +32,7 @@ export function BottomBarActions({
onSave,
unsavedChangesCount,
saveLabel,
areChangesInvalid = false,
}: Props) {
return (
<EuiBottomBar paddingSize="s" data-test-subj="profilingBottomBarActions">
Expand Down Expand Up @@ -64,16 +67,29 @@ export function BottomBarActions({
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton
data-test-subj="profilingBottomBarActionsButton"
onClick={onSave}
fill
isLoading={isLoading}
color="success"
iconType="check"
<EuiToolTip
content={
areChangesInvalid &&
i18n.translate(
'xpack.profiling.bottomBarActions.saveButtonTooltipWithInvalidChanges',
{
defaultMessage: 'Fix invalid settings before saving.',
}
)
}
>
{saveLabel}
</EuiButton>
<EuiButton
disabled={areChangesInvalid}
data-test-subj="profilingBottomBarActionsButton"
onClick={onSave}
fill
isLoading={isLoading}
color="success"
iconType="check"
>
{saveLabel}
</EuiButton>
</EuiToolTip>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
Expand Down
13 changes: 5 additions & 8 deletions x-pack/plugins/profiling/public/views/settings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
import { useEditableSettings, useUiTracker } from '@kbn/observability-shared-plugin/public';
import { isEmpty } from 'lodash';
import React from 'react';
import { ValueValidation } from '@kbn/core-ui-settings-browser/src/types';
import { FieldRowProvider } from '@kbn/management-settings-components-field-row';
import { useProfilingDependencies } from '../../components/contexts/profiling_dependencies/use_profiling_dependencies';
import { ProfilingAppPageTemplate } from '../../components/profiling_app_page_template';
Expand All @@ -53,7 +52,7 @@ export function Settings() {
const trackProfilingEvent = useUiTracker({ app: 'profiling' });
const {
start: {
core: { docLinks, notifications },
core: { docLinks, notifications, settings },
},
} = useProfilingDependencies();

Expand Down Expand Up @@ -81,11 +80,7 @@ export function Settings() {
}
}

// We don't validate the user input on these settings
const settingsValidationResponse: ValueValidation = {
successfulValidation: true,
valid: true,
};
const hasInvalidChanges = Object.values(unsavedChanges).some(({ isInvalid }) => isInvalid);

return (
<ProfilingAppPageTemplate hideSearchBar>
Expand Down Expand Up @@ -215,7 +210,8 @@ export function Settings() {
{...{
links: docLinks.links.management,
showDanger: (message: string) => notifications.toasts.addDanger(message),
validateChange: async () => settingsValidationResponse,
validateChange: (key: string, value: any) =>
settings.client.validateValue(key, value),
}}
>
<FieldRow
Expand All @@ -242,6 +238,7 @@ export function Settings() {
defaultMessage: 'Save changes',
})}
unsavedChangesCount={Object.keys(unsavedChanges).length}
areChangesInvalid={hasInvalidChanges}
/>
)}
</>
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/profiling/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
"@kbn/profiling-utils",
"@kbn/security-plugin",
"@kbn/shared-ux-utility",
"@kbn/core-ui-settings-browser",
"@kbn/management-settings-components-field-row"
// add references to other TypeScript projects the plugin depends on

Expand Down

0 comments on commit f1bd3ca

Please sign in to comment.