Skip to content
Open
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
10 changes: 10 additions & 0 deletions frontend/common/types/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,16 @@ export type FeatureStateValue = {
type: 'int' | 'unicode' | 'bool' | 'float'
}

// The trait shape from the core API, which keys its type as `value_type`
// where feature states use `type`.
export type TraitValue = {
boolean_value: boolean | null
float_value?: number | null
integer_value: number | null
string_value: string | null
value_type: 'int' | 'unicode' | 'bool' | 'float'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract the inline value_type union into a named type.

Line 605 adds an inline union type. The frontend TypeScript guideline requires union types to use named aliases. Define a named alias such as TraitValueType and use it for TraitValue.value_type. Reuse the alias for FeatureStateValue.type if both fields share the same contract.

As per coding guidelines: frontend/**/*.{ts,tsx} requires inline union types to be extracted into named types.

Proposed refactor
+export type TraitValueType = 'int' | 'unicode' | 'bool' | 'float'
+
 export type TraitValue = {
   boolean_value: boolean | null
   float_value?: number | null
   integer_value: number | null
   string_value: string | null
-  value_type: 'int' | 'unicode' | 'bool' | 'float'
+  value_type: TraitValueType
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
value_type: 'int' | 'unicode' | 'bool' | 'float'
export type TraitValueType = 'int' | 'unicode' | 'bool' | 'float'
export type TraitValue = {
boolean_value: boolean | null
float_value?: number | null
integer_value: number | null
string_value: string | null
value_type: TraitValueType
}

Source: Coding guidelines

}

export type MultivariateOption = {
id: number
uuid: string
Expand Down
50 changes: 46 additions & 4 deletions frontend/common/utils/__tests__/featureStateToValue.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { featureStateToValue } from 'common/utils/featureStateToValue'
import type { FlagsmithValue, TraitValue } from 'common/types/responses'

describe('featureStateToValue', () => {
it.each([
Expand All @@ -10,10 +11,51 @@ describe('featureStateToValue', () => {
expect(featureStateToValue(nested as never)).toBe(expected)
})

it('reads value_type when present (core traits)', () => {
expect(
featureStateToValue({ integer_value: 3, value_type: 'int' } as never),
).toBe(3)
// Typed rather than cast, so these fail to compile if TraitValue drifts.
it.each<[string, TraitValue, FlagsmithValue]>([
[
'int',
{
boolean_value: null,
integer_value: 3,
string_value: null,
value_type: 'int',
},
3,
],
[
'float',
{
boolean_value: null,
float_value: 2.5,
integer_value: null,
string_value: null,
value_type: 'float',
},
2.5,
],
[
'bool',
{
boolean_value: false,
integer_value: null,
string_value: null,
value_type: 'bool',
},
false,
],
[
'unicode',
{
boolean_value: null,
integer_value: null,
string_value: 'power_users',
value_type: 'unicode',
},
'power_users',
],
])('reads value_type on a core trait (%s)', (_label, trait, expected) => {
expect(featureStateToValue(trait)).toBe(expected)
})

it('normalises a missing int/float to null', () => {
Expand Down
12 changes: 7 additions & 5 deletions frontend/common/utils/featureStateToValue.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import type { FeatureStateValue, FlagsmithValue } from 'common/types/responses'
import type {
FeatureStateValue,
FlagsmithValue,
TraitValue,
} from 'common/types/responses'

/**
* Flattens a feature state (or core trait) value into its typed scalar.
Expand All @@ -10,7 +14,7 @@ import type { FeatureStateValue, FlagsmithValue } from 'common/types/responses'
* their unit tests) don't pull the Flux stores in through `utils.tsx`.
*/
export function featureStateToValue(
value: FlagsmithValue | FeatureStateValue | undefined,
value: FlagsmithValue | FeatureStateValue | TraitValue | undefined,
): FlagsmithValue {
if (value === null || value === undefined) {
return null
Expand All @@ -19,9 +23,7 @@ export function featureStateToValue(
return value
}
// `value_type` is the type key on core traits; `type` on feature states.
const type =
(value as { value_type?: FeatureStateValue['type'] }).value_type ??
value.type
const type = 'value_type' in value ? value.value_type : value.type
switch (type) {
case 'bool':
return value.boolean_value
Expand Down
3 changes: 2 additions & 1 deletion frontend/common/utils/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ProjectFlag,
SegmentCondition,
Tag,
TraitValue,
UserPermissions,
} from 'common/types/responses'
import flagsmith from '@flagsmith/flagsmith'
Expand Down Expand Up @@ -883,7 +884,7 @@ const Utils = Object.assign({}, BaseUtils, {
type: 'unicode',
}
},
valueToTrait(value: FlagsmithValue) {
valueToTrait(value: FlagsmithValue): TraitValue {
const val = Utils.getTypedValue(value)

if (typeof val === 'boolean') {
Expand Down
Loading