-
Notifications
You must be signed in to change notification settings - Fork 461
refactor: extract empty state and metrics components from FeaturesPage #6314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
talissoncosta
wants to merge
43
commits into
refactor/migrate-organisationsettingspage
Choose a base branch
from
refactor/migrate-critical-components-featurespagejs-1
base: refactor/migrate-organisationsettingspage
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
refactor: extract empty state and metrics components from FeaturesPage #6314
talissoncosta
wants to merge
43
commits into
refactor/migrate-organisationsettingspage
from
refactor/migrate-critical-components-featurespagejs-1
+1,954
−1,259
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]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…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]>
- 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
- Add OrganisationInformation component for name updates - Add Force2FASetting toggle component - Add RestrictProjectCreationSetting toggle component - Add DeleteOrganisation danger zone component with useHistory hook - All components use RTK Query mutations via custom hooks Part of OrganisationSettingsPage TypeScript migration
- Compose all General tab sections - Include JSONReference for debugging - Organize with SettingTitle sections - Clean component structure with proper TypeScript types Part of OrganisationSettingsPage TypeScript migration
- Add BillingTab with RTK Query useGetSubscriptionMetadataQuery - Add LicensingTab, CustomFieldsTab, APIKeysTab wrappers - Add WebhooksTab and SAMLTab wrappers - All tabs accept organisationId and use existing components Part of OrganisationSettingsPage TypeScript migration
- Use useRouteContext for organisationId (no URL param parsing) - Use useGetOrganisationQuery for all organisation data - Check permissions from organisation.role (no AccountStore) - Implement tabs array pattern matching ProjectSettingsPage - Add proper loading and error states - No legacy HOCs (withRouter, ConfigProvider) - Export directly without wrappers BREAKING CHANGE: Completely removes OrganisationProvider and AccountStore dependencies
- Remove old 466-line JavaScript class component - Update routes to import from organisation-settings folder - Complete migration to modern React with hooks and RTK Query Closes #5779
Replace legacy AccountStore usage with RTK Query for determining next
organisation after deletion. The hook now fetches the organisations
list and calculates the next available organisation ID, passing it to
the onSuccess callback for explicit navigation handling.
Changes:
- useDeleteOrganisationWithToast: Add useGetOrganisationsQuery hook
- Calculate next organisation ID before passing to callback
- DeleteOrganisation: Remove AccountStore and Utils imports
- Use direct URL construction for navigation
- Navigate to /organisation/{id}/projects or /organisations
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Add AppActions.refreshOrganisation() call after successful organisation deletion to ensure legacy components (navbar, permissions UI) that still use OrganisationStore stay in sync. This follows the same pattern as useUpdateOrganisationWithToast and useUpdateProjectWithToast hooks. Without this, legacy store-based components would show stale data after an organisation is deleted, while RTK Query components would update correctly via cache invalidation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add isLoading state to Delete Organisation button to prevent double-clicks and provide visual feedback during deletion. Button is disabled and shows "Deleting..." text while the deletion is in progress. This matches the pattern already implemented in OrganisationInformation for the Update button. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove self-explanatory comments that don't add value: - useUpdateOrganisationWithToast: Remove "Don't re-throw" comment - OrganisationSettingsPage: Remove "Check permission" comment - DeleteOrganisation: Remove navigation redirect comment Keep important comments that explain backward compatibility decisions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
During the migration, the data-test attribute was changed from 'organisation-name' to 'org-name', breaking E2E tests that rely on this selector. Restored the original 'organisation-name' selector to maintain compatibility with existing E2E tests in invite-test.ts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
4fd15de to
c08d4de
Compare
- Create FeaturesEmptyState.tsx component (109 lines) - Create FeatureMetricsSection.tsx component (30 lines) - Reduce FeaturesPage.js by ~110 lines - Move feature flag check into component (better encapsulation) - Preserve all E2E test selectors - Zero visual changes Part of incremental FeaturesPage TypeScript migration (PR #1/6)
5dfaa4e to
798187f
Compare
f5b8b06 to
2d7b53f
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!Changes
Please describe.
How did you test this code?
Please describe.