-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(dashboards): Allow custom sorting of releases dropdown #106978
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
Open
gggritso
wants to merge
47
commits into
master
Choose a base branch
from
georgegritsouk/dain-1111-allow-custom-sorting-of-releases-dropdown
base: master
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.
Open
feat(dashboards): Allow custom sorting of releases dropdown #106978
gggritso
wants to merge
47
commits into
master
from
georgegritsouk/dain-1111-allow-custom-sorting-of-releases-dropdown
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
…ompactSelect API This commit performs cleanup tasks following the sortable releases implementation: 1. Extract reusable hook (useSyncedQueryParamState): - Created new hook combining Nuqs URL state with localStorage fallback - URL takes precedence for sharing, localStorage provides persistence - Includes comprehensive unit tests (7 test cases) 2. Update to new CompactSelect API: - Changed from triggerProps object to trigger render function - Updated ReleasesSelectControl to use SelectTrigger.Button - Follows recent CompactSelect API migration 3. Revert shared Insights code: - Restored ReleasesSort.tsx to master version - Removes unwanted modifications to shared components - Insights component already uses correct API 4. Simplify FiltersBar: - Replace manual Nuqs+localStorage pattern with new hook - Reduces 20 lines to 4 lines - Cleaner and more maintainable All tests passing (including new hook tests and dashboard tests).
- Rename urlParamName → key for brevity - Change localStorageKey → namespace with automatic key construction (namespace:key) - Remove self-explanatory comments - Use nullish coalescing (??) instead of OR (||) for better semantics - Update filtersBar to use cleaner namespace: 'dashboards' - Update all tests to match new API
More descriptive variable name that clearly indicates this is for the release filter's sort option.
Move the sortBy validation logic from SortableReleasesFilter up to its parent (filtersBar.tsx). This way: - filtersBar validates the sortBy before passing it down - SortableReleasesFilter becomes a simpler presentational component - Clearer separation of concerns (parent owns validation, child just renders) - Removed unused ReleasesSortOption import from sortableReleasesFilter
…d update tests - Remove .withDefault() from parseAsString to properly handle null URL values - Use nullish coalescing chain (urlValue ?? localStorage ?? default) - Update tests to use official withNuqsTestingAdapter from nuqs/adapters/testing - Fix test assertions to check onUrlUpdate's object.queryString property - All 7 tests now pass
- Create new useReleases hook in dashboards/hooks/ using useApiQuery
- Fetches releases and enriches with event counts (similar to Mobile Vitals pattern)
- Remove ReleasesProvider context wrapper from sortableReleasesFilter
- Update releasesSelectControl to use new hook with local search state
- Remove duplicate event count fetching code from releasesSelectControl
- Update test mocks: events API returns {data: []} format
- Update test matcher to check options.query instead of options.data
Benefits:
- Cleaner architecture without unnecessary context provider
- Direct query hook pattern matches Mobile Vitals
- Event count fetching centralized in one place
- All dashboard tests passing
- Remove unnecessary ReleasesProvider wrapper in widget builder filtersBar - ReleasesSelectControl now manages its own data fetching via useReleases hook - No functional change since the control is disabled in widget builder
…sual style - Create new ReleasesSortSelect component using CompactSelect instead of CompositeSelect - Uses SelectTrigger.Button to match the full button style of the release selector - Update all imports to use the new Dashboards-specific component - This fixes the visual mismatch where the sort button appeared unpressed Before: Sort button used CompositeSelect with icon-only trigger (size=xs) After: Sort button uses CompactSelect with full button trigger matching release selector
Add hideChevron prop to SelectTrigger.Button to remove the dropdown chevron, making it clearer that this is a sort button rather than a filter selector.
Change from hideChevron (invalid) to showChevron={false} (correct prop name).
The DropdownButton component uses showChevron prop, not hideChevron.
- Change SortableReleasesFilter to named export - Change ReleasesSelectControl to named export - Change WidgetBuilderFilterBar to named export - Update all imports to use named imports This improves code clarity and makes imports more explicit.
…e components - Remove environments prop from ReleasesSortSelect, fetch from usePageFilters internally - This makes it consistent with ReleasesSelectControl which also doesn't accept environments - Move LabelDetails component below ReleasesSelectControl since it's a helper component - Remove unused usePageFilters import from sortableReleasesFilter
Add styled wrapper to remove left/right padding from the sort icon button, making it more compact and visually balanced with just the icon.
…or button" This reverts commit cb2cde7.
Apply 'muted' variant to IconSort to match the color of chevrons in CompactSelect, making the icon more subtle and visually consistent with other dropdown affordances.
…ing-of-releases-dropdown Resolved conflicts: - filtersBar.tsx: Merged retention/maxPickableDays functionality with release sorting - releasesSelectControl.tsx: Updated SelectTrigger to OverlayTrigger (renamed in master) - releasesSortSelect.tsx: Updated SelectTrigger to OverlayTrigger Both functionalities now work together: - Release sorting with URL/localStorage sync (my feature) - Date range limits based on widget types (retention feature from master)
Destructure and discard the children prop from triggerProps to prevent the selected sort option label from appearing in the button. Now the sort selector displays only the sort icon, making it more compact.
…ing-of-releases-dropdown
…ing-of-releases-dropdown
…hook - Replace ReleasesContext mocking with MockApiClient for API responses - Initialize PageFiltersStore in all tests - Fix import to use named export instead of default export - Mock both /releases/ and /events/ endpoints - Update search and filter tests to work with new hook pattern
Replaces legacy ReleasesProvider context pattern with modern React Query hook. Changes: - Created new useReleases hook using useApiQuery for dashboards - Removed ReleasesProvider wrapper from dashboard filters - Updated ReleasesSelectControl to use hook directly instead of context - Simplified architecture by removing context provider boilerplate - Maintained all existing functionality and API request behavior The new hook is similar to Insights version but without mobile-specific metrics queries, making it suitable for general dashboard use. Tests: - Updated component tests to mock the hook - 5/5 tests passing with comprehensive coverage - Verified API requests are made with correct parameters
…ol tests Replace jest.mock of useReleases hook with MockApiClient network mocking to provide better integration testing. Changes: - Remove jest.mock for useReleases hook - Add MockApiClient.addMockResponse for each test - Initialize PageFiltersStore in beforeEach to enable the hook - Wait for API requests to complete using waitFor - Tests now verify actual hook behavior with network mocking All 5 tests passing with full integration testing of the hook.
- Use getApiUrl for type-safe URL construction in useReleases hook - Remove unused ReleasesProvider and its test file
The test was failing because the mock only matched {query: 's'} but the
new useReleases hook sends additional query parameters (project,
per_page, environment, sort). Updated both mocks to match all params.
…ze-release-fetching' into georgegritsouk/dain-1111-allow-custom-sorting-of-releases-dropdown ; Conflicts: ; static/app/utils/releases/releasesProvider.tsx ; static/app/views/dashboards/detail.spec.tsx ; static/app/views/dashboards/filtersBar.tsx ; static/app/views/dashboards/hooks/useReleases.tsx ; static/app/views/dashboards/releasesSelectControl.spec.tsx ; static/app/views/dashboards/releasesSelectControl.tsx ; static/app/views/dashboards/widgetBuilder/components/filtersBar.tsx
… errors - Delete generic useSyncedQueryParamState hook (made non-reusable as planned) - Inline URL+localStorage sync logic directly in filtersBar.tsx - Fix TypeScript errors in releasesSortSelect.tsx and releasesSelectControl.tsx - Change Flex justify prop from "space-between" to "between" - Add empty string children to OverlayTrigger.Button - Implementation now uses modernized release fetching with sortBy and stats
Keep the PR tidy by using default export for WidgetBuilderFilterBar.
…-1111-allow-custom-sorting-of-releases-dropdown ; Conflicts: ; static/app/views/dashboards/filtersBar.tsx ; static/app/views/dashboards/hooks/useReleases.tsx ; static/app/views/dashboards/releasesSelectControl.spec.tsx ; static/app/views/dashboards/releasesSelectControl.tsx ; static/app/views/dashboards/widgetBuilder/components/filtersBar.tsx
Remove localStorage persistence for release sort selection. Just use URL state via Nuqs for now - will add back localStorage later when Nuqs has better primitives for it.
…ing-of-releases-dropdown
Remove transaction.op:[ui.load,navigation] filter from release event counts query. This was cruft from the Insights version and shouldn't be in the general dashboards code. Now just queries for event counts by release without operation filtering.
Move RELEASES_SORT_OPTIONS constant and ReleasesSortByOption type to constants/releases.tsx to eliminate duplication across components. Replace parseAsString with custom Nuqs parser for validated sort state. Update useReleases to use fetchDataQuery instead of api.requestPromise. Co-Authored-By: Claude <[email protected]>
- Type onChange callback to return ReleasesSortByOption instead of string - Rename Props to SortableReleasesFilterProps for consistency - Remove redundant spread in handleChangeFilter callback - Simplify onSortChange prop assignment in filtersBar Co-Authored-By: Claude <[email protected]>
- Remove redundant ReleasesSortOption type alias in favor of enum - Rename SortableReleasesFilter to SortableReleasesSelect - Simplify filtersBar by removing validatedReleaseFilterSortBy - Restore docstring in useReleases hook - Replace EventView.fromNewQueryWithPageFilters with plain query params - Conditionally show event counts only when defined - Fix duplicate type imports across components Co-Authored-By: Claude <[email protected]>
- Remove unused onSearch callback from useReleases hook - Add explanatory comments for flatten parameter and eslint-disable - Replace inline style with styled component in LabelDetails Co-Authored-By: Claude <[email protected]>
- Lazy-load event counts only when dropdown is open to reduce API calls - Improve type safety by casting at Object.keys() source instead of callback - Use OverlayTrigger.IconButton for icon-only sort button - Add comment explaining excluded CRASH_FREE_* sort options - Restore original export style in widgetBuilder filtersBar
…ing-of-releases-dropdown
Add allReleases to the metricsStats useMemo dependency array to ensure metrics data is invalidated when the release list changes. Previously, when the sort changed, stale metrics from the previous release list could be displayed because metricsFetched remained true while the metrics queries were disabled during the releases refetch. Co-Authored-By: Claude <[email protected]>
Replace the metricsStats useMemo with TanStack Query's combine option, which provides structurally shared results that maintain referential stability. This eliminates the need for the eslint-disable comment and follows React Query best practices for handling multiple queries. Co-Authored-By: Claude <[email protected]>
- Fix escapeFilterValue to escape individual version strings, not the entire query string, preventing malformed queries with special chars - Add type guard for release version instead of non-null assertion - Extract chunk size to named constant RELEASES_PER_CHUNK - Improve JSDoc with accurate description and @param tags - Add explicit comma separator to join() for clarity Co-Authored-By: Claude <[email protected]>
Keep grid containers rendered even when event count or date is missing to maintain consistent two-column layout alignment. Co-Authored-By: Claude <[email protected]>
Member
Author
|
@sentry review |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
… met ADOPTION sort requires exactly one environment. Previously, if a user had ADOPTION selected and then changed environments, the API request would still use an invalid sort configuration. Now the sort is automatically reset to DATE when environments.length !== 1.
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.
Adds a sort selector to the dashboard releases filter dropdown, allowing users to sort releases by date, sessions, users, or adoption rate. This is a copy (mostly) of the behaviour from Mobile Vitals Insights, and paves the way for transitioning Mobile Vitals to a Dashboards. Plus, it's just useful for everyone!
e.g.,
The main changes are:
useReleasesis now clever, and will fetch transaction counts for the releases that are visible. This is how it works in Insights today