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
4 changes: 2 additions & 2 deletions frontend/common/types/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,8 @@ export type AuditLogItem = {
export type AuditLogDetail = AuditLogItem & {
change_details: {
field: string
old: FlagsmithValue
new: FlagsmithValue
old: FlagsmithValue | object
new: FlagsmithValue | object
}[]
}
export type PaymentMethod = 'CHARGEBEE' | 'XERO' | 'AWS_MARKETPLACE'
Expand Down
27 changes: 27 additions & 0 deletions frontend/common/utils/__tests__/diffValue.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { stringifyDiffValue } from 'common/utils/diffValue'

describe('stringifyDiffValue', () => {
it('returns an empty string for null and undefined', () => {
expect(stringifyDiffValue(null)).toBe('')
expect(stringifyDiffValue(undefined)).toBe('')
})

it('quotes the empty string', () => {
expect(stringifyDiffValue('')).toBe('""')
})

it('stringifies primitives', () => {
expect(stringifyDiffValue('abc')).toBe('abc')
expect(stringifyDiffValue(3)).toBe('3')
expect(stringifyDiffValue(false)).toBe('false')
})

it('pretty-prints arrays of objects instead of [object Object]', () => {
const rules = [
{ conditions: [{ operator: 'EQUAL', property: 'a', value: '1' }] },
]
const result = stringifyDiffValue(rules)
expect(result).not.toContain('[object Object]')
expect(JSON.parse(result)).toEqual(rules)
})
})
17 changes: 17 additions & 0 deletions frontend/common/utils/diffValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Turns an audit-log / diff value into the string shown in the diff viewer.
* Objects and arrays (e.g. a segment's `rules_data`) are pretty-printed as JSON,
* otherwise template interpolation would render them as `[object Object]`.
*/
export const stringifyDiffValue = (value: unknown): string => {
if (value === undefined || value === null) {
return ''
}
if (value === '') {
return '""'
}
if (typeof value === 'object') {
return JSON.stringify(value, null, 2)
}
return `${value}`
}
27 changes: 11 additions & 16 deletions frontend/web/components/diff/DiffString.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,15 @@ import Diff, { DiffMethod } from 'react-diff-viewer-continued'
import Prism from 'prismjs'
import 'prismjs/components/prism-json'
import { FlagsmithValue } from 'common/types/responses'
import { stringifyDiffValue } from 'common/utils/diffValue'
import DiffRow from './DiffRow'

type DiffType = {
oldValue: FlagsmithValue
newValue: FlagsmithValue
oldValue: FlagsmithValue | object
newValue: FlagsmithValue | object
Comment on lines +10 to +11

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Major · Functional Correctness · Quick win

Supported by static analysis

Script executed:

sed -n '1,75p' frontend/web/components/diff/DiffString.tsx; sed -n '88,113p' frontend/web/components/pages/AuditLogItemPage.tsx; rg -n -m 3 '\bE2E\b' frontend/common frontend/web/components/diff frontend/rspack | head -60

Repository: Flagsmith/flagsmith

Length of output: 4187


Script executed:

git diff 1ec1049e6e40ab539242417c645b0ed5e2181f48 ebbc64c137f85dcb6b262591930bf572779a630c -- frontend/web/components/diff/DiffString.tsx frontend/common/types/responses.ts frontend/common/utils/diffValue.ts frontend/common/utils/__tests__/diffValue.test.ts
nl -ba frontend/web/components/diff/DiffString.tsx
nl -ba frontend/web/components/pages/AuditLogItemPage.tsx | sed -n '1,180p'
nl -ba frontend/common/utils/diffValue.ts
rg -n -F -e 'rules_data' -e 'E2E:' -e 'E2E =' -e 'process.env.E2E' frontend --glob '!yarn.lock' --glob '!package-lock.json' --glob '!pnpm-lock.yaml'

Repository: Flagsmith/flagsmith

Length of output: 15456


Script executed:

rg -n -F -e 'rules_data' -e 'change_details' api | sed -n '1,220p'
rg -n -F -e 'useGetAuditLogItemQuery' -e 'getAuditLogItem' frontend/common/services frontend/e2e
nl -ba frontend/common/services/useAuditLogItem.ts
nl -ba frontend/web/project/polyfil.js | sed -n '1,40p'
nl -ba frontend/api/index.js | sed -n '105,145p'
nl -ba frontend/api/dev-routes.js | sed -n '45,75p'
nl -ba frontend/playwright.config.ts | sed -n '85,115p'
nl -ba frontend/package.json | sed -n '1,180p'
rg -n -F -e 'FlagsmithValue =' -e '@types/react' -e 'typecheck' frontend/common/types/responses.ts frontend/package.json frontend/tsconfig.json frontend/yarn.lock frontend/package-lock.json frontend/pnpm-lock.yaml 2>/dev/null

Repository: Flagsmith/flagsmith

Length of output: 37496


Web query:

React 19 documentation React children plain JavaScript objects invalid as a React child arrays objects

Result:

Sources consulted:
- Children – React — https://react.dev/reference/react/Children
- JavaScript in JSX with Curly Braces – React — https://react.dev/learn/javascript-in-jsx-with-curly-braces
- Minified React error #31 – React — https://react.dev/errors/31?args%5B%5D=object+with+keys+%7Bmessage%2C+name%2C+code%2C+config%2C+request%7D&invariant=31
- Minified React error #0 – React — https://react.dev/errors/0
- Legacy React APIs – React — https://react.dev/reference/react/legacy
- …and 7 more

Script executed:

nl -ba api/audit/serializers.py | sed -n '1,110p'
nl -ba api/core/models.py | sed -n '65,130p;200,240p'
nl -ba api/segments/models.py | sed -n '45,160p'
nl -ba api/segments/serializers.py | sed -n '195,250p;295,325p'
nl -ba frontend/tsconfig.json
nl -ba frontend/package-lock.json | sed -n '7874,7888p'
rg -n -F -e 'old-value' -e 'new-value' frontend/e2e frontend/common frontend/web/components/diff
rg -n -F -e 'AuditLogChangeDetail' -e 'AuditLogChangeDetailSerializer' api/audit api/core

Repository: Flagsmith/flagsmith

Length of output: 20887


Web query:

@types/react 19.3.0 ReactNode type React HTMLAttributes children ReactNode declaration index.d.ts

Result:

Sources consulted:
- @types/react@19.3.0 - jsDocs.io — https://www.jsdocs.io/package/%40types/react
- @types/react - npm — https://www.npmjs.com/package/%40types/react?activeTab=versions
- @types/react | Snyk — https://security.snyk.io/package/npm/%40types%2Freact
- …and 26 more

Script executed:

curl -fsSL --max-time 20 https://unpkg.com/@types/react@19.3.0/index.d.ts | awk '/type ReactNode =/{show=1; count=0} show {print NR ":" $0; count++; if(count==18)show=0} /interface DOMAttributes<T>/{show2=1; count2=0} show2 {print NR ":" $0; count2++; if(count2==12)show2=0} /div: React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>/{print NR ":" $0}'
nl -ba frontend/e2e/helpers/e2e-helpers.playwright.ts | sed -n '795,835p'

Repository: Flagsmith/flagsmith

Length of output: 3365


Serialise objects in the E2E branch.

When rules_data contains rule objects, React cannot render the audit-log detail in E2E mode. The widened types also cause typechecking to fail. Serialise objects in this branch while preserving primitive values.

Suggested fix

-        <div data-test={'old-value'}>{oldValue}</div>
-        <div data-test={'new-value'}>{newValue}</div>
+        <div data-test={'old-value'}>
+          {oldValue !== null && typeof oldValue === 'object'
+            ? stringifyDiffValue(oldValue)
+            : oldValue}
+        </div>
+        <div data-test={'new-value'}>
+          {newValue !== null && typeof newValue === 'object'
+            ? stringifyDiffValue(newValue)
+            : newValue}
+        </div>

compareMethod?: DiffMethod
}

const sanitiseDiffString = (value: FlagsmithValue) => {
if (value === undefined || value === null) {
return ''
}
if (value === '') {
return '""'
}
return `${value}`
}
const DiffString: FC<DiffType> = ({
compareMethod = DiffMethod.WORDS_WITH_SPACE,
newValue,
Expand All @@ -34,7 +26,7 @@ const DiffString: FC<DiffType> = ({
)
}

if (oldValue === newValue) {
if (stringifyDiffValue(oldValue) === stringifyDiffValue(newValue)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor · Functional Correctness · Quick win

Supported by static analysis

Script executed:

sed -n '545,572p' frontend/common/types/responses.ts; sed -n '1,72p' frontend/web/components/diff/DiffString.tsx; sed -n '88,113p' frontend/web/components/pages/AuditLogItemPage.tsx; rg -n -m 3 'change_details|DiffString' frontend/common frontend/web/components/pages | head -75

Repository: Flagsmith/flagsmith

Length of output: 4313


Script executed:

git diff 1ec1049e6e40ab539242417c645b0ed5e2181f48 ebbc64c137f85dcb6b262591930bf572779a630c -- frontend/web/components/diff/DiffString.tsx frontend/common/utils/diffValue.ts frontend/common/types/responses.ts frontend/common/utils/__tests__/diffValue.test.ts
nl -ba frontend/common/utils/diffValue.ts
nl -ba frontend/web/components/diff/DiffString.tsx
nl -ba frontend/web/components/pages/AuditLogItemPage.tsx | sed -n '1,145p'
rg -n -F -e 'change_details' -e 'ChangeDetail' -- api frontend | head -160

Repository: Flagsmith/flagsmith

Length of output: 19644


Script executed:

nl -ba api/audit/serializers.py
nl -ba api/audit/types.py
nl -ba api/core/models.py | sed -n '1,150p'
nl -ba api/tests/integration/audit/test_audit_logs.py | sed -n '215,270p;388,475p'
rg -n -e 'class FeatureStateValue' -e 'string_value' -e 'integer_value' -e 'boolean_value' -e 'float_value' api/features api/tests/integration/audit
rg -n -F 'FlagsmithValue' frontend/common/types/responses.ts

Repository: Flagsmith/flagsmith

Length of output: 26140


Script executed:

nl -ba api/features/feature_states/models.py | sed -n '1,105p'
nl -ba api/features/models.py | sed -n '1150,1245p'
rg -n -e 'JSONField' -e 'AbstractBaseAuditableModel' -e 'auditable_model' -- api/{features,segments,environments,projects,organisations,identities,core} --glob '*.py' --glob '!*/migrations/*'
nl -ba api/core/models.py | sed -n '150,255p'
rg -n -F -e 'rules_data' -e 'HistoricalRecords' api/segments --glob '*.py' --glob '!*/migrations/*'

Repository: Flagsmith/flagsmith

Length of output: 19240


Script executed:

rg -n --glob '*.py' --glob '!**/migrations/**' -e 'JSONField' -e 'abstract_base_auditable_model_factory' api
nl -ba api/segments/models.py | sed -n '85,160p'
nl -ba api/segments/serializers.py | sed -n '130,245p;300,370p'
nl -ba api/features/release_pipelines/core/models.py | sed -n '40,90p;180,245p'
nl -ba api/core/signals.py | sed -n '1,170p'

Repository: Flagsmith/flagsmith

Length of output: 26747


Script executed:

nl -ba api/integrations/launch_darkly/models.py
rg -n -F -e 'LaunchDarklyImportStatus' -e '.status =' -e '"status"' api/integrations/launch_darkly --glob '*.py' --glob '!**/migrations/**'
rg -n -F -e 'DiffString' -e 'AuditLogDetail' frontend --glob '*.{ts,tsx}'
nl -ba frontend/common/services/useAuditLogItem.ts
rg -n -F -e 'rules_data =' -e '"rules_data"' api/segments api --glob '*.py' --glob '!**/migrations/**'

Repository: Flagsmith/flagsmith

Length of output: 12646


Script executed:

nl -ba frontend/web/components/diff/DiffFeatureStateValues.tsx
nl -ba frontend/web/components/diff/DiffVariations.tsx
rg -n -F -e 'DiffFeatureStateValues' -e 'DiffVariations' frontend/web --glob '*.{ts,tsx}'
nl -ba frontend/web/components/diff/DiffSegmentOverrides.tsx
rg -n -F -e 'AuditLogRetrieveSerializer' -e 'audit-detail' api/audit api/projects --glob '*.py'
nl -ba api/integrations/launch_darkly/serializers.py | sed -n '1,85p'

Repository: Flagsmith/flagsmith

Length of output: 17359


Script executed:

nl -ba frontend/web/components/diff/DiffFeature.tsx | sed -n '1,245p'
nl -ba frontend/web/components/diff/CompareSegmentOverride.tsx
nl -ba frontend/web/components/diff/diff-utils.ts | sed -n '1,260p'
rg -n -F -e 'featureStateToValue' -e 'getFeatureStateValue' frontend/common/utils/utils.tsx frontend/web/components/diff
rg -n -F -e 'DiffFeature' -e 'CompareSegmentOverride' frontend/web/components --glob '*.tsx'

Repository: Flagsmith/flagsmith

Length of output: 26711


Script executed:

rg -n -F 'getTypedValue' frontend/common/utils
nl -ba frontend/common/utils/featureStateToValue.ts
rg -n -F -C 10 'getTypedValue:' frontend/common/utils/utils.tsx
nl -ba frontend/web/components/diff/DiffChangeRequest.tsx | sed -n '30,105p'
nl -ba frontend/web/components/CompareEnvironments/ExpandedRow.tsx | sed -n '1,90p'
nl -ba frontend/documentation/pages/DiffString.stories.tsx

Repository: Flagsmith/flagsmith

Length of output: 8921


Script executed:

nl -ba frontend/common/utils/utils.tsx | sed -n '570,650p'
nl -ba api/audit/views.py | sed -n '25,90p'
nl -ba frontend/web/components/diff/DiffRow.tsx

Repository: Flagsmith/flagsmith

Length of output: 5647


Script executed:

nl -ba api/segments/admin.py
nl -ba api/segments/models.py | sed -n '153,225p'
rg -n -F -e 'SegmentAdmin' -e 'admin.site.register(Segment' -e '@admin.register(Segment' api --glob '*.py'
rg -n -F -e 'Segment.objects.create(' -e 'Segment.objects.update_or_create(' -e 'rules_data=' api --glob '*.py' --glob '!**/migrations/**' --glob '!**/tests/**'

Repository: Flagsmith/flagsmith

Length of output: 5634


Script executed:

nl -ba frontend/web/components/diff/useCompareTargets.ts | sed -n '1,200p'
nl -ba frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx | sed -n '300,375p'
rg -n -F -e 'sourceDescriptor' -e 'value: ' frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx
nl -ba frontend/common/types/responses.ts | sed -n '10,25p;560,575p'

Repository: Flagsmith/flagsmith

Length of output: 11850


Script executed:

rg -n -i -e 'djangorestframework' -e 'django-simple-history' api/pyproject.toml api/uv.lock api/poetry.lock api/requirements* pyproject.toml --glob '*'

Repository: Flagsmith/flagsmith

Length of output: 7069


Web query:

Django REST framework 3.18.3 ReadOnlyField to_representation returns value unchanged github

Result:

Sources consulted:
- django-rest-framework/rest_framework/fields.py at 3.18.3 · encode/django-rest-framework · GitHub — https://github.com/encode/django-rest-framework/blob/3.18.3/rest_framework/fields.py
- Release Notes - Django REST framework — https://www.django-rest-framework.org/community/release-notes/
- django-rest-framework/rest_framework/fields.py at main · encode/django-rest-framework · GitHub — https://github.com/encode/django-rest-framework/blob/main/rest_framework/fields.py
- django-rest-framework/docs/api-guide/serializers.md at main · encode/django-rest-framework · GitHub — https://github.com/encode/django-rest-framework/blob/main/docs/api-guide/serializers.md
- …and 33 more

Script executed:

curl -fsSL --max-time 15 https://raw.githubusercontent.com/encode/django-rest-framework/3.18.3/rest_framework/fields.py | nl -ba | sed -n '/class ReadOnlyField(/,/class HiddenField(/p'

Repository: Flagsmith/flagsmith

Length of output: 920


Script executed:

rg -n -F -e 'react-diff-viewer-continued@' -e '"react-diff-viewer-continued"' frontend/package.json frontend/yarn.lock frontend/package-lock.json frontend/pnpm-lock.yaml

Repository: Flagsmith/flagsmith

Length of output: 438


Preserve value types in the diff.

If an audit detail changes from 1 to "1", this comparison renders an unchanged row. Compare and display values in a way that preserves their types, while retaining JSON rendering for objects and arrays.

if (oldValue === null || oldValue === '') {
return <div className='text-muted'>No Value.</div>
}
Expand All @@ -44,7 +36,7 @@ const DiffString: FC<DiffType> = ({
className='d-inline mb-0'
dangerouslySetInnerHTML={{
__html: Prism.highlight(
sanitiseDiffString(newValue),
stringifyDiffValue(newValue),
Prism.languages.json,
'json',
),
Expand All @@ -55,11 +47,14 @@ const DiffString: FC<DiffType> = ({
}
return (
<Diff
oldValue={sanitiseDiffString(oldValue)}
newValue={sanitiseDiffString(newValue)}
oldValue={stringifyDiffValue(oldValue)}
newValue={stringifyDiffValue(newValue)}
compareMethod={compareMethod}
hideLineNumbers={
!(`${oldValue}`.includes?.('\n') || `${newValue}`.includes?.('\n'))
!(
stringifyDiffValue(oldValue).includes('\n') ||
stringifyDiffValue(newValue).includes('\n')
Comment on lines +50 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor · Performance & Scalability · Quick win

Supported by static analysis

Script executed:

sed -n '15,69p' frontend/web/components/diff/DiffString.tsx; sed -n '88,113p' frontend/web/components/pages/AuditLogItemPage.tsx; rg -n -m 3 'rules_data|change_details' frontend/common/types frontend/web/components/pages | head -70

Repository: Flagsmith/flagsmith

Length of output: 3028


Script executed:

git diff 1ec1049e6e40ab539242417c645b0ed5e2181f48 ebbc64c137f85dcb6b262591930bf572779a630c -- frontend/web/components/diff/DiffString.tsx frontend/common/utils/diffValue.ts frontend/common/types/responses.ts
nl -ba frontend/web/components/diff/DiffString.tsx
nl -ba frontend/common/utils/diffValue.ts
nl -ba frontend/web/components/pages/AuditLogItemPage.tsx | sed -n '1,150p'
rg -n -F -e 'rules_data' -e 'change_details' api | head -160

Repository: Flagsmith/flagsmith

Length of output: 28153


Script executed:

nl -ba api/segments/models.py | sed -n '35,145p'
nl -ba api/segments/serializers.py | sed -n '1,210p'
nl -ba api/segments/serializers.py | sed -n '210,370p'
nl -ba api/core/models.py | sed -n '65,110p'
nl -ba api/audit/serializers.py | sed -n '1,115p'
nl -ba api/tests/unit/segments/test_unit_segments_views.py | sed -n '2010,2105p'
rg -n -i -e 'max.*(rule|condition|segment)' -e '(rule|condition).*(limit|max)' api/segments api/core api/organisations frontend/web/components/segments frontend/common 2>/dev/null | head -100

Repository: Flagsmith/flagsmith

Length of output: 34537


Script executed:

nl -ba api/core/models.py | sed -n '110,130p;205,245p'
nl -ba api/segments/models.py | sed -n '145,190p;215,315p;335,370p'
rg -n -F -e 'SEGMENT_RULES_CONDITIONS_LIMIT' -e 'SEGMENT_CONDITION_VALUE_LIMIT' api --glob '*settings*' --glob '*.py' | head -60
rg -n -F -e 'E2E =' -e 'E2E:' -e 'E2E:' frontend --glob '*config*' --glob '*.d.ts' | head -40

Repository: Flagsmith/flagsmith

Length of output: 14564


Script executed:

rg -n -F -e '<DiffString' -e "from 'components/diff/DiffString'" frontend --glob '*.{ts,tsx,js,jsx}'
nl -ba frontend/common/utils/__tests__/diffValue.test.ts
nl -ba api/app/settings/common.py | sed -n '1428,1449p'

Repository: Flagsmith/flagsmith

Length of output: 3639


Serialise each diff value once per render.

For large rules_data values, repeated serialisation can delay the audit detail view. Reuse each rendered value for the comparison, diff props and newline check.

Suggested fix

-  if (stringifyDiffValue(oldValue) === stringifyDiffValue(newValue)) {
+  const oldValueString = stringifyDiffValue(oldValue)
+  const newValueString = stringifyDiffValue(newValue)
+
+  if (oldValueString === newValueString) {
@@
-              stringifyDiffValue(newValue),
+              newValueString,
@@
-      oldValue={stringifyDiffValue(oldValue)}
-      newValue={stringifyDiffValue(newValue)}
+      oldValue={oldValueString}
+      newValue={newValueString}
@@
-          stringifyDiffValue(oldValue).includes('\n') ||
-          stringifyDiffValue(newValue).includes('\n')
+          oldValueString.includes('\n') ||
+          newValueString.includes('\n')

)
}
renderContent={(str) => (
<pre
Expand Down
Loading