Skip to content

Commit

Permalink
fix: boolean values are converted to 1 and 0 only on configuration pa…
Browse files Browse the repository at this point in the history
…ge (#1347)

**Issue number:**
https://splunk.atlassian.net/browse/ADDON-73559
## Summary
1. Boolean values are mapped only on configuration page. 
2. Do not map boolean values when typing inside single input select
(BUG)

1 justification:
Looks like splunk is using backend values mapping only on forms used on
configuration page.

By backend values mapping i mean converting
values like  ‘TRUE’, ‘T’, ‘Y’, ‘YES’, true will be converted into ‘1’
values like ‘FALSE’, ‘F’, ‘N’, ‘NO’, ‘NONE’, false will be converted
into ‘0’

values on inputs page are saved and retrieved correctly, so I assume
that on any different page than configuration that mapping is not
needed.

Thats why for function used to map values `getValueMapTruthyFalse` there
is added another parameter `currentPageName`. If the page is
configuration values are mappend into 1 and 0 if not values are left as
they are.

### Changes

> Please provide a summary of what's being changed

On Input page values are shared to backend correctly, only on
configuration page they are mapped.

### User experience

Users can use mapped till now words on inputs page.
> Please describe what the user experience looks like before and after
this change

## Checklist

If your change doesn't seem to apply, please leave them unchecked.

* [x] I have performed a self-review of this change
* [x] Changes have been tested
* [x] Changes are documented
* [x] PR title follows [conventional commit
semantics](https://www.conventionalcommits.org/en/v1.0.0/)

---------

Co-authored-by: srv-rr-github-token <[email protected]>
  • Loading branch information
soleksy-splunk and srv-rr-github-token authored Oct 1, 2024
1 parent d2ef3c0 commit d0d0c11
Show file tree
Hide file tree
Showing 17 changed files with 71 additions and 38 deletions.
4 changes: 2 additions & 2 deletions docs/entity/components.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ A clear button is visible to the right of the dropdown when this field is marked
| valueField | string | If you use endpointUrl and your data are not simple text data, you can specify here which property of retrieved object should be used as value for each item.```item.content?.[valueField]``` | - |
| [dependencies](../advanced/dependent_dropdown.md) | array | It is used to update the options via an API call when the value of any field in the dependencies list is updated. | - |

> When using [Boolean](https://docs.splunk.com/Documentation/Splunk/latest/SearchReference/ListOfDataTypes) values consider that inside splunk values like 'TRUE', 'T', 'Y', 'YES', true will be converted into '1' and values like 'FALSE', 'F', 'N', 'NO', 'NONE', false will be converted into '0'.
> When using [Boolean](https://docs.splunk.com/Documentation/Splunk/latest/SearchReference/ListOfDataTypes) values on any form inside configuration page, consider that inside splunk values like 'TRUE', 'T', 'Y', 'YES', true will be converted into '1' and values like 'FALSE', 'F', 'N', 'NO', 'NONE', false will be converted into '0'.
>
> Consider using values '0' and '1' as false and true values.
Expand Down Expand Up @@ -411,7 +411,7 @@ This is how it looks in the UI:

![image](../images/components/radio_component_example.png)

> When using [Boolean](https://docs.splunk.com/Documentation/Splunk/latest/SearchReference/ListOfDataTypes) values consider that inside splunk values like 'TRUE', 'T', 'Y', 'YES', true will be converted into '1' and values like 'FALSE', 'F', 'N', 'NO', 'NONE', false will be converted into '0'.
> When using [Boolean](https://docs.splunk.com/Documentation/Splunk/latest/SearchReference/ListOfDataTypes) values on any form inside configuration page, consider that inside splunk values like 'TRUE', 'T', 'Y', 'YES', true will be converted into '1' and values like 'FALSE', 'F', 'N', 'NO', 'NONE', false will be converted into '0'.
>
> Consider using values '0' and '1' as false and true values.
Expand Down
5 changes: 3 additions & 2 deletions ui/src/components/BaseFormView/BaseFormTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Mode } from '../../constants/modes';
import {
AcceptableFormValueOrNull,
AcceptableFormValueOrNullish,
StandardPages,
} from '../../types/components/shareableTypes';
import { MarkdownMessageProps } from '../MarkdownMessage/MarkdownMessage';
import {
Expand Down Expand Up @@ -66,7 +67,7 @@ export interface BaseFormProps {
currentServiceState?: Record<string, AcceptableFormValueOrNull>;
serviceName: string;
mode: Mode;
page: string;
page: StandardPages;
stanzaName: string;
groupName?: string;
handleFormSubmit: (isSubmitting: boolean, closeEntity: boolean) => void;
Expand All @@ -75,7 +76,7 @@ export interface BaseFormProps {
export interface BaseFormState {
serviceName?: string;
mode?: Mode;
page?: string;
page?: StandardPages;
stanzaName?: string;
data: BaseFormStateData;
errorMsg?: string;
Expand Down
8 changes: 6 additions & 2 deletions ui/src/components/BaseFormView/BaseFormView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ class BaseFormView extends PureComponent<BaseFormProps, BaseFormState> {
const stateWithModifications = getModifiedState(
{ data: temState },
this.props.mode,
this.fieldsWithModifications
this.fieldsWithModifications,
this.props.page
);
if (stateWithModifications.shouldUpdateState) {
temState = { ...stateWithModifications.newState.data };
Expand Down Expand Up @@ -936,7 +937,8 @@ class BaseFormView extends PureComponent<BaseFormProps, BaseFormState> {
const { newState } = getModifiedState(
tempState,
this.props.mode,
this.fieldsWithModifications.filter((entity) => entity.field === field)
this.fieldsWithModifications.filter((entity) => entity.field === field),
this.props.page
);

if (this.hookDeferred) {
Expand Down Expand Up @@ -1231,6 +1233,7 @@ class BaseFormView extends PureComponent<BaseFormProps, BaseFormState> {
disabled={temState?.disabled || false}
markdownMessage={temState?.markdownMessage}
dependencyValues={temState?.dependencyValues || null}
page={this.props.page}
/>
);
}
Expand Down Expand Up @@ -1326,6 +1329,7 @@ class BaseFormView extends PureComponent<BaseFormProps, BaseFormState> {
dependencyValues={temState.dependencyValues || null}
fileNameToDisplay={temState.fileNameToDisplay}
modifiedEntitiesData={temState.modifiedEntitiesData}
page={this.props.page}
/>
);
})}
Expand Down
2 changes: 2 additions & 0 deletions ui/src/components/ControlWrapper/ControlWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ interface ControlWrapperProps {
label?: string;
required?: boolean;
};
page?: string;
}

class ControlWrapper extends React.PureComponent<ControlWrapperProps> {
Expand Down Expand Up @@ -108,6 +109,7 @@ class ControlWrapper extends React.PureComponent<ControlWrapperProps> {
mode: this.props.mode,
...this?.props?.entity,
...this.props?.modifiedEntitiesData,
page: this.props.page,
}
)
: `No View Found for ${this?.props?.entity?.type} type`;
Expand Down
3 changes: 2 additions & 1 deletion ui/src/components/DeleteModal/DeleteModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import { axiosCallWrapper } from '../../util/axiosCallWrapper';
import TableContext from '../../context/TableContext';
import { parseErrorMsg, getFormattedMessage } from '../../util/messageUtil';
import { PAGE_INPUT } from '../../constants/pages';
import { StandardPages } from '../../types/components/shareableTypes';

const ModalWrapper = styled(Modal)`
width: 800px;
`;

interface DeleteModalProps {
page: string;
page: StandardPages;
handleRequestClose: () => void;
serviceName: string;
stanzaName: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const Base: Story = {
handleRequestClose: fn(),
serviceName: 'serviceName',
stanzaName: 'stanzaName',
page: '',
page: 'configuration',
open: true,
},
};
Expand Down
5 changes: 3 additions & 2 deletions ui/src/components/EntityModal/EntityModal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { ERROR_AUTH_PROCESS_TERMINATED_TRY_AGAIN } from '../../constants/oAuthErrorMessage';
import { Mode } from '../../constants/modes';
import * as axiosWrapper from '../../util/axiosCallWrapper';
import { StandardPages } from '../../types/components/shareableTypes';

describe('Oauth field disabled on edit - diableonEdit property', () => {
const handleRequestClose = jest.fn();
Expand Down Expand Up @@ -279,7 +280,7 @@ describe('EntityModal - custom warning', () => {
setUnifiedConfig(newConfig);
};

const renderModal = (inputMode: Mode, page: string) => {
const renderModal = (inputMode: Mode, page: StandardPages) => {
const props = {
serviceName: 'account',
mode: inputMode,
Expand All @@ -305,7 +306,7 @@ describe('EntityModal - custom warning', () => {
${'config'} | ${'input'}
`(
'display custom warning for $mode mode - $page tab',
({ mode, page }: { mode: keyof typeof WARNING_MESSAGES; page: string }) => {
({ mode, page }: { mode: keyof typeof WARNING_MESSAGES; page: StandardPages }) => {
if (page === 'configuration') {
setUpConfigWithWarningMessageForConfiguration();
} else {
Expand Down
3 changes: 2 additions & 1 deletion ui/src/components/EntityModal/EntityModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import { ButtonClickHandler } from '@splunk/react-ui/Button';
import { Mode, MODE_CLONE, MODE_CREATE, MODE_EDIT } from '../../constants/modes';
import { StyledButton } from '../../pages/EntryPageStyle';
import BaseFormView from '../BaseFormView/BaseFormView';
import { StandardPages } from '../../types/components/shareableTypes';

const ModalWrapper = styled(Modal)`
width: 800px;
`;

export interface EntityModalProps {
page: string;
page: StandardPages;
mode: Mode;
serviceName: string;
handleRequestClose: () => void;
Expand Down
3 changes: 2 additions & 1 deletion ui/src/components/EntityPage/EntityPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import BaseFormView from '../BaseFormView/BaseFormView';
import { SubTitleComponent } from '../../pages/Input/InputPageStyle';
import { PAGE_INPUT } from '../../constants/pages';
import { StyledButton } from '../../pages/EntryPageStyle';
import { StandardPages } from '../../types/components/shareableTypes';

interface EntityPageProps {
handleRequestClose: () => void;
serviceName: string;
mode: Mode;
page: string;
page: StandardPages;
stanzaName?: string;
formLabel?: string;
groupName?: string;
Expand Down
18 changes: 11 additions & 7 deletions ui/src/components/FormModifications/FormModifications.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Mode } from '../../constants/modes';
import { AcceptableFormValueOrNullish } from '../../types/components/shareableTypes';
import { AcceptableFormValueOrNullish, StandardPages } from '../../types/components/shareableTypes';
import { getValueMapTruthyFalse } from '../../util/considerFalseAndTruthy';
import {
BaseFormState,
Expand Down Expand Up @@ -80,17 +80,20 @@ export function getAllFieldsWithModifications(
const getModificationForEntity = (
entity: EntitiesAllowingModifications,
stateShallowCopy: BaseFormState,
mode: Mode
mode: Mode,
page: StandardPages
) => {
let modification = entity.modifyFieldsOnValue?.find((mod) => {
const currentFieldValue = stateShallowCopy.data?.[entity.field]?.value;
return (
// do not compare empty values for modifications
currentFieldValue !== undefined &&
currentFieldValue !== null &&
// here type convertion is needed as splunk keeps all data as string
// and users can put numbers or booleans inside global config
getValueMapTruthyFalse(currentFieldValue) === getValueMapTruthyFalse(mod.fieldValue) &&
// values are directly equal or they are equal after type conversion
(currentFieldValue === mod.fieldValue ||
// here conversion is needed as splunk keeps boolish data on configuration page as 1 and 0
getValueMapTruthyFalse(currentFieldValue, page) ===
getValueMapTruthyFalse(mod.fieldValue, page)) &&
(!mod.mode || mod.mode === mode)
);
});
Expand Down Expand Up @@ -151,12 +154,13 @@ const getStateAfterModification = (
export const getModifiedState = (
state: BaseFormState,
mode: Mode,
entitiesToModify: EntitiesAllowingModifications[]
entitiesToModify: EntitiesAllowingModifications[],
page: StandardPages
) => {
let stateShallowCopy = { ...state };
let shouldUpdateState = false;
entitiesToModify.forEach((entity: EntitiesAllowingModifications) => {
const modifications = getModificationForEntity(entity, stateShallowCopy, mode);
const modifications = getModificationForEntity(entity, stateShallowCopy, mode, page);

modifications?.fieldsToModify.forEach((modificationFields) => {
const { fieldId, ...fieldProps } = modificationFields;
Expand Down
8 changes: 6 additions & 2 deletions ui/src/components/RadioComponent/RadioComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { Component } from 'react';
import RadioBar from '@splunk/react-ui/RadioBar';
import styled from 'styled-components';
import { getValueMapTruthyFalse } from '../../util/considerFalseAndTruthy';
import { StandardPages } from '../../types/components/shareableTypes';

const RadioBarWrapper = styled(RadioBar)`
width: 320px;
Expand All @@ -23,6 +24,7 @@ interface RadioComponentProps {
}[];
};
disabled: boolean;
page?: StandardPages;
}

class RadioComponent extends Component<RadioComponentProps> {
Expand All @@ -37,14 +39,16 @@ class RadioComponent extends Component<RadioComponentProps> {
inline
onChange={this.handleChange}
value={
this.props.value ? getValueMapTruthyFalse(this.props.value) : this.props.value
this.props.value
? getValueMapTruthyFalse(this.props.value, this.props.page)
: this.props.value
}
key={this.props.field}
>
{this.props.controlOptions.items.map((item) => (
<RadioBarOption
key={item.value}
value={getValueMapTruthyFalse(item.value)}
value={getValueMapTruthyFalse(item.value, this.props.page)}
label={item.label}
disabled={this.props.disabled}
/>
Expand Down
20 changes: 10 additions & 10 deletions ui/src/components/SingleInputComponent/SingleInputComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { axiosCallWrapper } from '../../util/axiosCallWrapper';
import { SelectCommonOptions } from '../../types/globalConfig/entities';
import { filterResponse } from '../../util/util';
import { getValueMapTruthyFalse } from '../../util/considerFalseAndTruthy';
import { StandardPages } from '../../types/components/shareableTypes';

const SelectWrapper = styled(Select)`
width: 320px !important;
Expand Down Expand Up @@ -48,6 +49,7 @@ export interface SingleInputComponentProps {
hideClearBtn?: boolean;
};
required: boolean;
page?: StandardPages;
}

function SingleInputComponent(props: SingleInputComponentProps) {
Expand All @@ -73,9 +75,9 @@ function SingleInputComponent(props: SingleInputComponentProps) {
hideClearBtn,
} = controlOptions;

function handleChange(e: unknown, obj: { value: string | number | boolean }) {
const handleChange = (e: unknown, obj: { value: string | number | boolean }) => {
restProps.handleChange(field, obj.value);
}
};
const Option = createSearchChoice ? ComboBox.Option : Select.Option;
const Heading = createSearchChoice ? ComboBox.Heading : Select.Heading;

Expand All @@ -85,15 +87,15 @@ function SingleInputComponent(props: SingleInputComponentProps) {
if ('value' in item && item.value && item.label) {
// splunk will mape those when sending post form
// so worth doing it earlier to keep same state before and after post
const itemValue = getValueMapTruthyFalse(item.value);
const itemValue = getValueMapTruthyFalse(item.value, props.page);
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore JSX element type 'Option' does not have any construct or call signatures.
data.push(<Option label={item.label} value={itemValue} key={item.value} />);
}
if ('children' in item && item.children && item.label) {
data.push(<Heading key={item.label}>{item.label}</Heading>);
item.children.forEach((child) => {
const childValue = getValueMapTruthyFalse(child.value);
const childValue = getValueMapTruthyFalse(child.value, props.page);
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore JSX element type 'Option' does not have any construct or call signatures.
data.push(<Option label={child.label} value={childValue} key={childValue} />);
Expand Down Expand Up @@ -176,12 +178,11 @@ function SingleInputComponent(props: SingleInputComponentProps) {
return createSearchChoice ? (
<StyledDiv className="dropdownBox">
<ComboBox
// do not map empty values like '', null, undefined
value={props.value ? getValueMapTruthyFalse(props.value) : ''}
value={props.value}
name={field}
error={error}
disabled={effectiveDisabled}
onChange={handleChange} // eslint-disable-line react/jsx-no-bind
onChange={handleChange}
inline
>
{options && options.length > 0 && options}
Expand All @@ -194,12 +195,11 @@ function SingleInputComponent(props: SingleInputComponentProps) {
inputId={props.id}
className="dropdownBox"
data-test-loading={loading}
// do not map empty values like '', null, undefined
value={props.value ? getValueMapTruthyFalse(props.value) : props.value}
value={props.value}
name={field}
error={error}
disabled={effectiveDisabled}
onChange={handleChange} // eslint-disable-line react/jsx-no-bind
onChange={handleChange}
filter={!disableSearch}
inline
>
Expand Down
5 changes: 3 additions & 2 deletions ui/src/components/table/CustomTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import { isReadonlyRow } from './table.utils';
import { SortDirection } from './useTableSort';
import { GlobalConfig } from '../../types/globalConfig/globalConfig';
import { ITableConfig } from '../../types/globalConfig/pages';
import { StandardPages } from '../../types/components/shareableTypes';

interface CustomTableProps {
page: string;
page: StandardPages;
serviceName?: string;
data: RowDataFields[];
handleToggleActionClick: (row: RowDataFields) => void;
Expand All @@ -37,7 +38,7 @@ interface IEntityModal {
mode?: Mode;
}

const getServiceToStyleMap = (page: string, unifiedConfigs: GlobalConfig) => {
const getServiceToStyleMap = (page: StandardPages, unifiedConfigs: GlobalConfig) => {
const serviceToStyleMap: Record<string, typeof STYLE_PAGE | typeof STYLE_MODAL> = {};
if (page === PAGE_INPUT) {
const inputsPage = unifiedConfigs.pages.inputs;
Expand Down
3 changes: 2 additions & 1 deletion ui/src/components/table/TableWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useTableContext } from '../../context/useTableContext';
import { isFalse, isTrue } from '../../util/considerFalseAndTruthy';
import { isReadonlyRow } from './table.utils';
import { ITableConfig } from '../../types/globalConfig/pages';
import { StandardPages } from '../../types/components/shareableTypes';

export interface ITableWrapperProps {
page: typeof PAGE_INPUT | typeof PAGE_CONF;
Expand All @@ -34,7 +35,7 @@ const defaultTableConfig: ITableConfig = {
};

const getTableConfigAndServices = (
page: string,
page: StandardPages,
unifiedConfigs: GlobalConfig,
serviceName?: string
) => {
Expand Down
3 changes: 3 additions & 0 deletions ui/src/types/components/shareableTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ export type AcceptableFormValueOrNull = AcceptableFormValue | null;
export type AcceptableFormValueOrNullish = AcceptableFormValueOrNull | undefined;

export type AcceptableFormRecord = Record<string, AcceptableFormValueOrNull>;

export type StandardPages = 'configuration' | 'inputs';
export type AllPossiblePages = StandardPages | 'dashboard';
3 changes: 2 additions & 1 deletion ui/src/util/considerFalseAndTruthy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ describe('getValueMapTruthyFalse function', () => {
[{}, {}],
[[], '0'],
])('getValueMapTruthyFalse(%p) should return %p', (input, expected) => {
expect(getValueMapTruthyFalse(input)).toEqual(expected);
expect(getValueMapTruthyFalse(input, 'configuration')).toEqual(expected);
expect(getValueMapTruthyFalse(input, 'inputs')).toEqual(input);
});
});
Loading

0 comments on commit d0d0c11

Please sign in to comment.