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.

Sign up for GitHub

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: incorrect negative value conversion #4316

Merged
merged 6 commits into from
Aug 21, 2024
Merged

fix: incorrect negative value conversion #4316

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
820aae8
fix incorrect negative value conversion
kyle-ssg Jul 9, 2024
d3bd37e
Add negative value warning
kyle-ssg Aug 14, 2024
de8c3c8
Unused import
kyle-ssg Aug 14, 2024
1ba7a30
Fix space
kyle-ssg Aug 14, 2024
6053d7e
Improve copy
kyle-ssg Aug 14, 2024
17b45b3
copy
kyle-ssg Aug 21, 2024
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
2 changes: 1 addition & 1 deletion frontend/common/utils/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ const Utils = Object.assign({}, require('./base/_utils'), {
}

const typedValue = testWithTrim ? str.trim() : str
const isNum = /^\d+$/.test(typedValue)
const isNum = /^-?\d+$/.test(typedValue)

if (isNum && parseInt(typedValue) > Number.MAX_SAFE_INTEGER) {
return `${str}`
Expand Down
40 changes: 39 additions & 1 deletion frontend/web/components/Feature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,31 @@ import AddVariationButton from './mv/AddVariationButton'
import ErrorMessage from './ErrorMessage'
import Tooltip from './Tooltip'
import Icon from './Icon'
import InputGroup from './base/forms/InputGroup'
import WarningMessage from './WarningMessage'

function isNegativeNumberString(str) {
if (typeof Utils.getTypedValue(str) !== 'number') {
return false
}
if (typeof str !== 'string') {
return false
}
const num = parseFloat(str)
return !isNaN(num) && num < 0
}

export default class Feature extends PureComponent {
static displayName = 'Feature'

constructor(props) {
super(props)
this.state = {
isNegativeNumberString: isNegativeNumberString(
props.environmentFlag?.feature_state_value,
),
}
}
removeVariation = (i) => {
const idToRemove = this.props.multivariate_options[i].id

Expand Down Expand Up @@ -102,13 +123,30 @@ export default class Feature extends PureComponent {
}
tooltip={`${Constants.strings.REMOTE_CONFIG_DESCRIPTION}${
!isEdit
? '<br/>Setting this when creating a feature will set the value for all environments. You can edit the this individually for each environment once the feature is created.'
? '<br/>Setting this when creating a feature will set the value for all environments. You can edit this individually for each environment once the feature is created.'
: ''
}`}
title={`${valueString}`}
/>
</FormGroup>
)}
{this.state.isNegativeNumberString && (
<WarningMessage
warningMessage={
<div>
This feature currently has the value of{' '}
<strong>"{environmentFlag?.feature_state_value}"</strong>.
Saving this feature will convert its value from a string to a
number. If you wish to preserve this value as a string, please
save it via our{' '}
<a href='https://api.flagsmith.com/api/v1/docs/#/api/api_v1_environments_featurestates_update'>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit - suggest wording this as please save it using the Admin API for consistency with other docs. LGTM, good stuff 🙏

API
</a>
.
</div>
}
/>
)}

{!!error && (
<div className='mx-2 mt-2'>
Expand Down
Loading