refactor(features): migrate FeaturesPage to RTK Query and eliminate legacy stores #6361
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Create reusable hook that wraps useUpdateProjectMutation with automatic toast notifications, supporting customizable messages and error callbacks.
Replace duplicated try/catch/toast patterns across 5 components with useUpdateProjectWithToast hook, eliminating ~70 lines of duplicated code.
Replace complex Omit/Partial approach with explicit Pick for better clarity and type safety. This makes it immediately clear which Project fields are updateable via the API.
Add RTK Query optimistic updates to updateProject mutation using onQueryStarted lifecycle hook. This immediately updates the cache when mutations are called and automatically rolls back on errors, providing instant UI feedback without manual rollback logic. Benefits: - Instant UI updates before server responds - Automatic error rollback via patchResult.undo() - Reduces component complexity by eliminating manual error handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove useEffect and manual rollback logic from 3 components that now benefit from mutation-level optimistic updates: - ProjectInformation: Remove useEffect syncing and onError rollback - AdditionalSettings: Remove useEffect syncing - FeatureNameValidation: Remove useEffect syncing and manual optimistic update Components now use local state only for form editing, with RTK Query handling all cache updates and automatic rollbacks on errors. Benefits: - Simpler components (~22 lines removed across 3 files) - No cursor jumps during typing when other settings update - No manual error handling needed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Create ChangeRequestsApprovalsSetting component following the same pattern as other settings (PreventFlagDefaultsSetting, CaseSensitivitySetting, FeatureNameValidation). Changes: - New ChangeRequestsApprovalsSetting.tsx component - AdditionalSettings simplified from 76 to 25 lines - All settings now follow consistent pattern (receive project prop) Benefits: - Better consistency across all setting components - Improved encapsulation (component owns its state) - Easier to test in isolation - Cleaner AdditionalSettings parent component 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove 9 unnecessary useCallback wrappers from project settings components. These callbacks had dependencies that change frequently (project properties, local state), causing recreation on every render and providing no memoization benefit. Changes across 7 files: - CaseSensitivitySetting: Remove useCallback from handleToggle - PreventFlagDefaultsSetting: Remove useCallback from handleToggle - FeatureNameValidation: Remove useCallback from handleToggle + handleSave - ProjectInformation: Remove useCallback from handleSubmit - ChangeRequestsApprovalsSetting: Remove useCallback from both functions - SDKSettingsTab: Remove useCallback from 2 toggle handlers - DeleteProject: Remove useCallback from handleDelete Benefits: - Simpler, more honest code (~36 lines removed) - Same performance (callbacks already recreated every render) - Removed unused useCallback imports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Call AppActions.refreshOrganisation() after project updates to ensure the legacy OrganisationStore stays in sync with RTK Query cache. This fixes the navbar not updating when project name changes, which was causing the E2E test (project-test.ts) to fail.
Fixed critical bug where the case sensitivity toggle had inverted behavior.
The backend field 'only_allow_lower_case_feature_names' has a negative
semantic (true = force lowercase), but the UI shows "Case sensitive features"
with a positive semantic (ON = allow uppercase).
Therefore we must invert the checked state to match the UI's meaning:
- checked={!project.only_allow_lower_case_feature_names}
Now when users enable "Case sensitive features", it correctly sets
only_allow_lower_case_feature_names=false, allowing uppercase characters.
This matches the original implementation and fixes the critical issue
documented in PROJECT_ORG_FIXES.md #1.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
…etting - Add ?? null to handle undefined initial values from backend - Fix stale closure bug in auto-save by passing value explicitly to saveChangeRequests - Fix manual save by using arrow function closure for onSave prop - Ensure both toggle (auto-save) and manual save work correctly This fixes issues where: - Initial value could be undefined instead of null - Auto-save on toggle used stale state value - Manual save button passed event object instead of current state 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add optional data-test prop to component type definition - Pass data-test to underlying Setting component - Follow kebab-case naming convention (js-change-request-approvals) This enables E2E testing by providing stable selectors that won't break with styling changes, following the pattern used by other settings in the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add E2E tests covering four critical scenarios: 1. Auto-save on toggle ON - verifies toggle saves immediately 2. Manual save after value change - verifies save button works 3. Auto-save on toggle OFF - verifies disable persists 4. Manual save with new value - verifies full workflow Each test includes navigation away and back to ensure values persist to backend, catching stale closure bugs that would otherwise pass with in-memory state. Test uses kebab-case data-test selector following codebase conventions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…lability Changed from early return pattern to wrapped conditional block following the pattern from initialise-tests.ts. This allows future tests added after the Change Requests tests to run regardless of the segment_change_requests feature flag status. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add UpdateOrganisationBody type with optional fields - Add getOrganisation, updateOrganisation, deleteOrganisation request types - Support for organisation name, force_2fa, and project creation restrictions Part of OrganisationSettingsPage TypeScript migration
- Add getOrganisation query with cache tags - Add updateOrganisation mutation with optimistic updates - Add deleteOrganisation mutation with cache invalidation - Implement automatic rollback on error for optimistic updates Part of OrganisationSettingsPage TypeScript migration
- Add useUpdateOrganisationWithToast hook for consistent update handling - Add useDeleteOrganisationWithToast hook with success callbacks - Include toast notifications and error handling - Support legacy store sync via AppActions.refreshOrganisation() Part of OrganisationSettingsPage TypeScript migration
Clean up redundant comments that were added during the service merge. The code is self-explanatory without these markers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed circular loading state issues where empty state would incorrectly appear or search input would lose focus during filter changes. Key changes: - Updated loading conditions to use `isFetching` for proper state transitions - Fixed filter parameter normalization (empty strings to null) - Cleaned API params to omit undefined/null values preventing empty query strings - Ensured search input stays mounted during filter changes This resolves the issue where value_search= was being sent as an empty string causing the API to return no results. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Cleaned up code by removing: - Redundant explanatory comments where code is self-documenting - Duplicate empty state fallback block in renderFeaturesList - Unnecessary comment lines in buildApiFilterParams This reduces ~25 lines while maintaining code clarity and functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add skeleton loading with proper type guards instead of type assertions - Simplify loading logic with clear shouldShowEmptyState condition - Fix empty state flash on initial load by checking !data - Prevent search input remounting during filter changes - Use TypeScript type guard pattern for better type safety 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove type definitions related to deleted multivariate CRUD endpoints: - CreateMultivariateOptionBody and UpdateMultivariateOptionBody - Req endpoint types: createMultivariateOption, updateMultivariateOption, deleteMultivariateOption - Res['multivariateOption'] response type Note: Core types MultivariateOption and MultivariateFeatureStateValue remain as they're used for reading multivariate data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
talissoncosta
force-pushed
the
refactor/migrate-critical-components-featurespagejs-2
branch
from
5343151 to
2299462
Compare
Remove legacy usePageTrackingWithContext wrapper and migrate FeaturesPage to use the new unified usePageTracking hook with options object pattern. Since FeaturesPage was the only consumer of the legacy wrapper and is already being refactored in this PR, there's no need to maintain backward compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove reference to FEATURES_MIGRATION_FOLLOWUP.md since documentation files are not included in this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
talissoncosta
force-pushed
the
refactor/migrate-critical-components-featurespagejs-2
branch
from
f502ae8 to
fe6a05e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Summary
This PR completes the migration of FeaturesPage from legacy Flux stores to RTK Query with comprehensive refactoring, improved TypeScript type safety, better error handling, and cleaner documentation.
Changes
1. Core RTK Query Migration
useFeatureListWithFiltershook for filtered feature fetching with automatic API key to ID conversionuseProjectEnvironmentshook to replace ProjectStore functionality2. Custom Hooks & Code Organization
useCallbackwrappers with empty dependenciesfeatureFilterParams.tsfor global reusefeatureFilters.ts3. UI/UX Improvements
isTogglingandisRemovingstates from mutation hooksisSavingbehavior4. Component Refactoring
useFeaturePageDisplayhookrenderFeaturesList()helper functionshowCreateButtonprop (was always true)5. Type Safety Improvements
projectIdtype tonumberacross all components6. Documentation Cleanup
useFeatureListWithFilters.ts: 81→34 linesfeatureFilterParams.ts: Simplified 4 JSDoc blocksuseFeatureFilters.ts: 26→3 linesuseRemoveFeatureWithToast.ts: 18→1 lineuseToggleFeatureWithToast.ts: 18→1 lineuseProjectEnvironments.ts: 25→1 linefeatureFilters.ts: Simplified type comments7. Performance Optimizations
8. Code Quality
Files Changed
Created:
common/hooks/useFeatureListWithFilters.tscommon/hooks/useProjectEnvironments.tscommon/types/featureFilters.tscommon/utils/featureFilterParams.tsweb/components/pages/features/hooks/useFeatureFilters.tsweb/components/pages/features/hooks/useToggleFeatureWithToast.tsweb/components/pages/features/hooks/useRemoveFeatureWithToast.tsweb/components/feature-summary/FeatureRowSkeleton.tsxweb/styles/components/_feature-row-skeleton.scssModified:
web/components/pages/features/FeaturesPage.tsx(major refactor)web/components/pages/features/components/FeaturesPageHeader.tsxweb/components/pages/features/components/FeaturesTableFilters.tsxRemoved:
web/components/pages/features/components/FeaturesList.tsxweb/components/pages/features/hooks/useFeaturePageDisplay.tsBenefits
Breaking Changes
None - all functionality preserved, this is a pure refactor.
How did you test this code?
Manual Testing:
Features Page Load
Filter Functionality
Feature Actions
Error Handling
Type Safety
Accessibility
Validation: