Skip to content

fix(Diff display): Segment rules diff showing up as [Object object] - #8910

Merged
khvn26 merged 4 commits into
mainfrom
refactor/diff-string-real-diff-in-e2e
Nov 9, 2026
Merged

khvn26 merged 4 commits into
mainfrom
refactor/diff-string-real-diff-in-e2e

Conversation

@khvn26

@khvn26 khvn26 commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Contributes to #8867

In this PR, we remove the if (E2E) branch from DiffString, add an assertDiffString test helper, and fix the [object Object] segment rules diff display.

How did you test this code?

Ran segment-test.pw.ts and versioning-tests.pw.ts locally.

DiffString rendered raw `old-value`/`new-value` divs when E2E was set,
so Playwright never exercised the diff users actually see.

Always render the real diff inside a `diff-string` wrapper, and have
`compareVersion` assert against the diff's removed and added lines.
@vercel

vercel Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
flagsmith-frontend-preview Ready Ready Preview Nov 9, 2026 4:02pm UTC
flagsmith-frontend-staging Ready Ready Preview Nov 9, 2026 4:02pm UTC
1 Skipped Deployment
Project Deployment Actions Updated
docs Ignored Ignored Preview Nov 9, 2026 4:02pm UTC

Request Review

@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Explain Change →

Walkthrough

Audit log diff values now support arrays and objects. The sanitiser formats object values as indented JSON. DiffString compares and renders sanitised values. End-to-end helpers check diff rows, and a segment test verifies that an updated rule displays 19 rather than [object Object].

Estimated code review effort: 2 (Simple) | ~12 minutes

Merge Risk: Low · up to a01d5

The structured audit display change has no established production blocker. The comparison helper needs follow-up for structured and multiline inputs; stronger segment and tab-visibility assertions would improve regression protection.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@github-actions github-actions Bot added front-end Issue related to the React Front End Dashboard refactor labels Nov 9, 2026
@khvn26 khvn26 changed the title refactor(Diff): Render the real diff in DiffString under E2E fix(Diff display): Segment rules diff showing up as [Object object] Nov 9, 2026
DiffString turned values into strings with a template literal, so JSON
fields like segment `rules_data` became `[object Object]` on both
sides of the diff, which then collapsed as unchanged.

Format arrays and objects as indented JSON via
`Utils.sanitiseDiffString`, which DiffString now uses instead of its
own copy, and compare the formatted text.

Contributes to #8867
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-8910 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api-test:pr-8910 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api:pr-8910 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-private-cloud:pr-8910 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-frontend:pr-8910 Finished ✅ Results ✅

@github-actions github-actions Bot added the fix label Nov 9, 2026
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor
✅ private-cloud · depot-ubuntu-latest-16 — run #22965 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  4 passed

Details

stats  4 tests across 4 suites
duration  19.4 seconds
commit  a01d57d
info  🔄 Run: #22965 (attempt 1)

🗂️ Previous results
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #22965 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  36.4 seconds
commit  a01d57d
info  🔄 Run: #22965 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #22965 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  33.5 seconds
commit  a01d57d
info  🔄 Run: #22965 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #22965 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  35.3 seconds
commit  a01d57d
info  🔄 Run: #22965 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-arm-16 — run #22951 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  3 passed

Details

stats  3 tests across 3 suites
duration  45.7 seconds
commit  b72d4d9
info  🔄 Run: #22951 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-16 — run #22951 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  3 passed

Details

stats  3 tests across 3 suites
duration  32.9 seconds
commit  b72d4d9
info  🔄 Run: #22951 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #22951 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  2 passed

Details

stats  2 tests across 2 suites
duration  32.6 seconds
commit  b72d4d9
info  🔄 Run: #22951 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #22951 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  21 passed
skipped  1 skipped

Details

stats  22 tests across 14 suites
duration  1 minute, 13 seconds
commit  b72d4d9
info  🔄 Run: #22951 (attempt 1)

Skipped tests

firefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss

@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Visual Regression

24 screenshots compared. See report for details.
View full report

talissoncosta
talissoncosta previously approved these changes Nov 9, 2026

@talissoncosta talissoncosta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good @khvn26. Just one small comment. Not blocking.

Use a `group` role labelled "Value diff" instead of a `data-test`
attribute, as E2E is moving to accessible selectors.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: bde222ee-7a55-48a5-a8bf-aa4de1fbbfda
Commits

Reviewing files that changed from the base of the PR and between b72d4d9 and a01d57d.

Files selected for processing: 3

  • frontend/e2e/helpers/e2e-helpers.playwright.ts
  • frontend/e2e/tests/segment-test.pw.ts
  • frontend/web/components/diff/DiffString.tsx

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Comment thread frontend/e2e/tests/segment-test.pw.ts Resolved
@khvn26
khvn26 merged commit 13539af into main Nov 9, 2026
29 checks passed
@khvn26
khvn26 deleted the refactor/diff-string-real-diff-in-e2e branch November 9, 2026 16:16

This branch was successfully deployed

2 active deployments
Preview – flagsmith-frontend-preview — a01d57d5 Deployed Nov 9, 2026 by vercel[bot]
Preview – flagsmith-frontend-staging — a01d57d5 Deployed Nov 9, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix front-end Issue related to the React Front End Dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants