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.

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

fix: Validate feature values before saving #4043

Merged
merged 6 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 20 additions & 13 deletions frontend/web/components/ValueEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import Highlight from './Highlight'
import ConfigProvider from 'common/providers/ConfigProvider'
import { Clipboard } from 'polyfill-react-native'
import Icon from './Icon'
import { IonIcon } from '@ionic/react'
import { checkmarkCircle, warning } from 'ionicons/icons'

const toml = require('toml')
const yaml = require('yaml')
Expand Down Expand Up @@ -91,24 +93,25 @@ class Validation extends Component {
render() {
const displayLanguage =
this.props.language === 'ini' ? 'toml' : this.props.language
return (
return this.state.error ? (
<Tooltip
position='top'
title={
!this.state.error ? (
<span className='language-icon ion-ios-checkmark-circle' />
) : (
<span
id='language-validation-error'
className='language-icon ion-ios-warning'
/>
)
<IonIcon
id='language-validation-error'
className='language-icon text-danger'
icon={warning}
/>
}
>
{!this.state.error
? `${displayLanguage} validation passed`
: `${displayLanguage} validation error, please check your value.<br/>Error: ${this.state.error}`}
{`${displayLanguage} validation error, please check your value.<br/>Error: ${this.state.error}`}
</Tooltip>
) : (
<IonIcon
id='language-validation-success'
className='language-icon text-success'
icon={checkmarkCircle}
/>
)
}
}
Expand All @@ -127,7 +130,11 @@ class ValueEditor extends Component {
}

renderValidation = () => (
<Validation language={this.state.language} value={this.props.value} />
<Validation
key={this.state.language}
language={this.state.language}
value={this.props.value}
/>
)

render() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import EnvironmentSelect from 'components/EnvironmentSelect'
import SegmentOverrideLimit from 'components/SegmentOverrideLimit'
import { getStore } from 'common/store'
import { getEnvironment } from 'common/services/useEnvironment'
import { saveFeatureWithValidation } from 'components/saveFeatureWithValidation'

class TheComponent extends Component {
state = {
Expand Down Expand Up @@ -306,7 +307,7 @@ export default class SegmentOverridesInner extends Component {
return (
<FeatureListProvider>
{({}, { editFeatureSegments, isSaving }) => {
const save = () => {
const save = saveFeatureWithValidation(() => {
FeatureListStore.isSaving = true
FeatureListStore.trigger('change')
!isSaving &&
Expand All @@ -324,7 +325,7 @@ export default class SegmentOverridesInner extends Component {
},
)
this.setState({ isSaving: true })
}
})
const segmentOverride =
segmentOverrides && segmentOverrides.filter((v) => v.segment === id)
if (!segmentOverrides) return null
Expand Down
25 changes: 7 additions & 18 deletions frontend/web/components/modals/CreateFlag.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { getSupportedContentType } from 'common/services/useSupportedContentType
import { getGithubIntegration } from 'common/services/useGithubIntegration'
import { removeUserOverride } from 'components/RemoveUserOverride'
import ExternalResourcesLinkTab from 'components/ExternalResourcesLinkTab'
import { saveFeatureWithValidation } from 'components/saveFeatureWithValidation'

const CreateFlag = class extends Component {
static displayName = 'CreateFlag'
Expand Down Expand Up @@ -855,7 +856,7 @@ const CreateFlag = class extends Component {
editFeatureValue,
},
) => {
const saveFeatureValue = (schedule) => {
const saveFeatureValue = saveFeatureWithValidation((schedule) => {
this.setState({ valueChanged: false })
if ((is4Eyes || schedule) && !identity) {
openModal2(
Expand Down Expand Up @@ -926,29 +927,17 @@ const CreateFlag = class extends Component {
}}
/>,
)
} else if (
document.getElementById('language-validation-error')
) {
openConfirm({
body: 'Your remote config value does not pass validation for the language you have selected. Are you sure you wish to save?',
noText: 'Cancel',
onYes: () => {
this.save(editFeatureValue, isSaving)
},
title: 'Validation error',
yesText: 'Save',
})
} else {
this.save(editFeatureValue, isSaving)
}
}
})

const saveSettings = () => {
this.setState({ settingsChanged: false })
this.save(editFeatureSettings, isSaving)
}

const saveFeatureSegments = () => {
const saveFeatureSegments = saveFeatureWithValidation(() => {
this.setState({ segmentsChanged: false })

if (is4Eyes && isVersioned && !identity) {
Expand Down Expand Up @@ -1021,11 +1010,11 @@ const CreateFlag = class extends Component {
} else {
this.save(editFeatureSegments, isSaving)
}
}
})

const onCreateFeature = () => {
const onCreateFeature = saveFeatureWithValidation(() => {
this.save(createFlag, isSaving)
}
})

const featureLimitAlert =
Utils.calculateRemainingLimitsPercentage(
Expand Down
15 changes: 15 additions & 0 deletions frontend/web/components/saveFeatureWithValidation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export const saveFeatureWithValidation = (cb: () => void) => {
return () => {
if (document.getElementById('language-validation-error')) {
openConfirm({
body: 'Your remote config value does not pass validation for the language you have selected. Are you sure you wish to save?',
noText: 'Cancel',
onYes: () => cb(),
title: 'Validation error',
yesText: 'Save',
})
} else {
cb()
}
}
}
3 changes: 3 additions & 0 deletions frontend/web/styles/3rdParty/_hljs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@
}
}
span {
display: flex;
align-items: center;
gap: 2px;
color: $text-icon-light-grey;
cursor: pointer;
font-size: $font-caption-sm;
Expand Down
Loading