feat: Redesign Compare Environments page#7965
Conversation
- Convert CompareEnvironments from JS to TypeScript - Add expandable rows with full diff view (segments, variations) - Add standard table filters (search, tags, owners, groups, sort) - Split results into "Changed features" and "Unchanged features" tables - Add column headers for source/target environments and segment changes - Show environment names in diff view dropdown instead of "Old/New Value" - Add environment name legend with +/- indicators in diff colours - Style improvements: proper table header styling, badge counts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe legacy environment comparison module is replaced by a TypeScript comparison flow with typed comparison data, paginated feature-state fetching, filtering and sorting, expandable feature rows, editing, and environment-specific diff rendering. The page compares changed and unchanged features across two selected environments, while Estimated code review effort: 4 (Complex) | ~60 minutes Comment |
github-actions
Bot
added
the
front-end
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d35e7802-5b0d-4d36-a807-ab7858d6883c
📒 Files selected for processing (4)
frontend/web/components/CompareEnvironments.jsfrontend/web/components/CompareEnvironments.tsxfrontend/web/components/diff/DiffFeature.tsxfrontend/web/styles/3rdParty/_react-diff.scss
💤 Files with no reviewable changes (1)
- frontend/web/components/CompareEnvironments.js
Visual Regression19 screenshots compared. See report for details. |
Add Edit buttons to each environment column that open the CreateFlagModal for editing features directly from the comparison view. The modal handles change requests automatically when required. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
github-actions
Bot
added
feature
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2e5c607a-0133-4beb-977e-e3136eda0457
📒 Files selected for processing (1)
frontend/web/components/CompareEnvironments.tsx
talissoncosta
left a comment
talissoncosta
left a comment
There was a problem hiding this comment.
Looks good @kyle-ssg ! Thanks for that!
I just left some suggestions.
If we can break it into small components and get rid of the _data import would be amazing.
Address PR review feedback on the Compare Environments redesign: - Move the single 607-line component into a CompareEnvironments/ folder with a barrel: types.ts, useEnvironmentComparison hook (fetch + diff derivation, now unit-testable), ExpandedRow, CompareFeatureRow, and the layout component. - Replace relative imports with components/ and common/ aliases. - Reuse the shared FlagsmithValue type instead of an inline union. - Surface a fetch error state with a retry action instead of silently swallowing failures and rendering "No differences found". - Guard against stale overlapping comparisons with a request-id check so a slow earlier response can't overwrite a newer one. - Make the row expand toggle keyboard accessible (role, tabIndex, aria-expanded, Enter/Space handling). - Pass the numeric projectId into CreateFlagModal. - Fall back to the left project flag for the right-side edit action so the modal edits the existing feature instead of switching to create mode. - Consolidate duplicated diff header styles into a shared base rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
feature
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9358fdf4-3fea-4572-8fe1-3a268386a44c
📒 Files selected for processing (7)
frontend/web/components/CompareEnvironments/CompareEnvironments.tsxfrontend/web/components/CompareEnvironments/CompareFeatureRow.tsxfrontend/web/components/CompareEnvironments/ExpandedRow.tsxfrontend/web/components/CompareEnvironments/index.tsfrontend/web/components/CompareEnvironments/types.tsfrontend/web/components/CompareEnvironments/useEnvironmentComparison.tsfrontend/web/styles/3rdParty/_react-diff.scss
…PageGet Export the existing recursivePageGet helper and reuse it in the comparison hook so every list is fetched across all pages, replacing the page_size=999 calls that silently capped results at 999. A small _data.get adapter bridges it to the baseQuery contract the helper expects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/common/services/useProjectFlag.ts (1)
12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake
recursivePageGetgeneric
It’s reused for bothProjectFlagandFeatureState, so parameterising the response type will remove the cast inuseEnvironmentComparisonand keep callers type-safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e7b13097-571d-4572-a88c-8cca942142a7
📒 Files selected for processing (2)
frontend/common/services/useProjectFlag.tsfrontend/web/components/CompareEnvironments/useEnvironmentComparison.ts
github-actions
Bot
added
feature
talissoncosta
left a comment
talissoncosta
left a comment
There was a problem hiding this comment.
Thanks for addressing the changes.
It looks good!
- Add getAllEnvironmentFeatureStates endpoint for bulk
environments/{key}/featurestates/ (default states only, all pages)
- Rewrite useEnvironmentComparison to use RTK Query hooks, dropping the
manual race guard in favour of currentData semantics
- Fix recursivePageGet: it read results/next off the RTK baseQuery
{data} wrapper, so pagination silently stopped after the first page
- Move recursivePageGet to common/utils as it is now shared by the
project flag and feature state services
- Surface feature state fetch errors in ExpandedRow instead of showing
"No differences between environments"
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ading UX - Remove Edit buttons; clicking the switch or value chip opens the edit modal (keyboard accessible via Enter/Space on the cell) - Share column width constants between header and rows, and clamp overflowing value chips so wide values no longer break alignment - Keep the table visible and dimmed during refreshes instead of replacing it with a loader - Refetch when the edit modal saves (FeatureListStore events) rather than on every modal close Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
feature
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/common/utils/recursivePageGet.ts (1)
1-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo unit tests for this new shared pagination helper.
recursivePageGetis a pure, easily testable function now shared by two service files; a couple of tests (single page, multi-page merge vianext, error passthrough) would guard against regressions.frontend/web/components/CompareEnvironments/CompareFeatureRow.tsx (1)
82-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a real segment-difference metric —
Math.max(leftOverrides, rightOverrides)shows the larger override count, not the number of changed segments, so the “Segment changes” chip can be non-zero even when both environments have the same overrides. Either compare the override sets or relabel the chip to match the metric.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d54db1e4-94fc-4c56-af47-fe10cf5e6c4b
📒 Files selected for processing (9)
frontend/common/services/useFeatureState.tsfrontend/common/services/useProjectFlag.tsfrontend/common/types/requests.tsfrontend/common/utils/recursivePageGet.tsfrontend/web/components/CompareEnvironments/CompareEnvironments.tsxfrontend/web/components/CompareEnvironments/CompareFeatureRow.tsxfrontend/web/components/CompareEnvironments/ExpandedRow.tsxfrontend/web/components/CompareEnvironments/constants.tsfrontend/web/components/CompareEnvironments/useEnvironmentComparison.ts
| getAllEnvironmentFeatureStates: builder.query< | ||
| Res['featureStates'], | ||
| Req['getAllEnvironmentFeatureStates'] | ||
| >({ | ||
| providesTags: [{ id: 'LIST', type: 'FeatureState' }], | ||
| queryFn: async (query, _baseQueryApi, _extraOptions, baseQuery) => | ||
| recursivePageGet<FeatureState>( | ||
| `environments/${query.environmentKey}/featurestates/?page_size=999`, | ||
| null, | ||
| baseQuery, | ||
| ), | ||
| }), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep outline frontend/common/services/useFeatureState.ts --view expandedRepository: Flagsmith/flagsmith
Length of output: 291
🏁 Script executed:
sed -n '1,180p' frontend/common/services/useFeatureState.tsRepository: Flagsmith/flagsmith
Length of output: 3609
🏁 Script executed:
rg -n "type FeatureState|interface FeatureState|featureStates|getAllEnvironmentFeatureStates|addFeatureSegmentsToFeatureStates|feature_segment" frontend/common/types frontend/common -g '!**/dist/**'Repository: Flagsmith/flagsmith
Length of output: 11938
🏁 Script executed:
sed -n '760,810p' frontend/common/types/responses.ts && printf '\n---\n' && sed -n '750,790p' frontend/common/types/requests.ts && printf '\n---\n' && rg -n "useGetAllEnvironmentFeatureStatesQuery|getAllEnvironmentFeatureStates" frontend/common -g '!**/dist/**'Repository: Flagsmith/flagsmith
Length of output: 2975
🏁 Script executed:
rg -n "environments/.*/featurestates|featurestates/\?page_size=999|getAllEnvironmentFeatureStatesQuery|useGetAllEnvironmentFeatureStatesQuery" . -g '!**/dist/**' -g '!**/node_modules/**'Repository: Flagsmith/flagsmith
Length of output: 4003
🏁 Script executed:
sed -n '1,180p' frontend/web/components/CompareEnvironments/useEnvironmentComparison.ts && printf '\n---\n' && rg -n "feature_segment.*Serializer|FeatureState.*Serializer|featurestates" api -g '!**/migrations/**' -g '!**/node_modules/**'Repository: Flagsmith/flagsmith
Length of output: 39867
🏁 Script executed:
sed -n '1,220p' api/environments/serializers.py && printf '\n---\n' && sed -n '1,240p' api/environments/views.py && printf '\n---\n' && sed -n '1,220p' api/features/serializers.pyRepository: Flagsmith/flagsmith
Length of output: 25232
🏁 Script executed:
rg -n "environment-featurestates|featurestates-list|FeatureStateSerializerFull|FeatureStateSerializerBasic|IdentityAllFeatureStatesSerializer" api/environments api/features -g '!**/migrations/**'Repository: Flagsmith/flagsmith
Length of output: 2077
🏁 Script executed:
sed -n '1,220p' api/environments/urls.py && printf '\n---\n' && sed -n '1,260p' api/features/views.pyRepository: Flagsmith/flagsmith
Length of output: 16009
🏁 Script executed:
sed -n '620,760p' api/features/views.py && printf '\n---\n' && sed -n '540,640p' api/features/serializers.pyRepository: Flagsmith/flagsmith
Length of output: 8873
🏁 Script executed:
sed -n '660,840p' api/features/serializers.py && printf '\n---\n' && sed -n '1,220p' api/features/features_service.pyRepository: Flagsmith/flagsmith
Length of output: 11729
getAllEnvironmentFeatureStates returns a different feature_segment shape than FeatureState expects. Either resolve segment refs here like getFeatureStates does, or narrow the endpoint type to the raw environment-feature-state shape so consumers don’t assume the nested object is present.
| type EnvironmentStateCellProps = { | ||
| enabled: boolean | ||
| value: FeatureChange['leftValue'] | ||
| onEdit: () => void | ||
| } | ||
|
|
||
| // The switch and value are read-only representations — clicking either opens | ||
| // the edit modal rather than toggling in place | ||
| const EnvironmentStateCell: FC<EnvironmentStateCellProps> = ({ | ||
| enabled, | ||
| onEdit, | ||
| value, | ||
| }) => ( | ||
| <div | ||
| className='clickable d-flex align-items-center gap-2 overflow-hidden' | ||
| role='button' | ||
| tabIndex={0} | ||
| onClick={(e: React.MouseEvent) => { | ||
| e.stopPropagation() | ||
| onEdit() | ||
| }} | ||
| onKeyDown={(e: React.KeyboardEvent) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| onEdit() | ||
| } | ||
| }} | ||
| > | ||
| <Switch className='flex-shrink-0 pe-none' checked={enabled} disabled /> | ||
| <FeatureValue className='overflow-hidden' value={value} /> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
EnvironmentStateCell lacks an accessible label describing its action.
The cell exposes role='button' but no aria-label; its accessible name falls back to the rendered FeatureValue text (e.g. a raw value), giving no indication that activating it opens the edit modal. Consider an aria-label such as Edit feature state.
♻️ Proposed fix
<div
className='clickable d-flex align-items-center gap-2 overflow-hidden'
role='button'
+ aria-label='Edit feature state'
tabIndex={0}📝 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.
| type EnvironmentStateCellProps = { | |
| enabled: boolean | |
| value: FeatureChange['leftValue'] | |
| onEdit: () => void | |
| } | |
| // The switch and value are read-only representations — clicking either opens | |
| // the edit modal rather than toggling in place | |
| const EnvironmentStateCell: FC<EnvironmentStateCellProps> = ({ | |
| enabled, | |
| onEdit, | |
| value, | |
| }) => ( | |
| <div | |
| className='clickable d-flex align-items-center gap-2 overflow-hidden' | |
| role='button' | |
| tabIndex={0} | |
| onClick={(e: React.MouseEvent) => { | |
| e.stopPropagation() | |
| onEdit() | |
| }} | |
| onKeyDown={(e: React.KeyboardEvent) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| onEdit() | |
| } | |
| }} | |
| > | |
| <Switch className='flex-shrink-0 pe-none' checked={enabled} disabled /> | |
| <FeatureValue className='overflow-hidden' value={value} /> | |
| </div> | |
| ) | |
| type EnvironmentStateCellProps = { | |
| enabled: boolean | |
| value: FeatureChange['leftValue'] | |
| onEdit: () => void | |
| } | |
| // The switch and value are read-only representations — clicking either opens | |
| // the edit modal rather than toggling in place | |
| const EnvironmentStateCell: FC<EnvironmentStateCellProps> = ({ | |
| enabled, | |
| onEdit, | |
| value, | |
| }) => ( | |
| <div | |
| className='clickable d-flex align-items-center gap-2 overflow-hidden' | |
| role='button' | |
| aria-label='Edit feature state' | |
| tabIndex={0} | |
| onClick={(e: React.MouseEvent) => { | |
| e.stopPropagation() | |
| onEdit() | |
| }} | |
| onKeyDown={(e: React.KeyboardEvent) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| onEdit() | |
| } | |
| }} | |
| > | |
| <Switch className='flex-shrink-0 pe-none' checked={enabled} disabled /> | |
| <FeatureValue className='overflow-hidden' value={value} /> | |
| </div> | |
| ) |
| // Shared column widths so the header and rows stay aligned. The environment | ||
| // column must fit a Switch, the widest possible FeatureValue chip (20 | ||
| // truncated characters plus quotes) and an Edit button. | ||
| export const ENV_COLUMN_WIDTH = 320 | ||
| export const SEGMENTS_COLUMN_WIDTH = 140 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Comment references a removed "Edit button".
The width rationale mentions fitting "an Edit button", but CompareFeatureRow.tsx replaced the standalone Edit button with the clickable EnvironmentStateCell (Switch + FeatureValue chip only). Update the comment to reflect the current layout so future width changes aren't based on stale assumptions.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
github-actions
Bot
added
feature
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Please describe.
How did you test this code?
Compared features in production