fix(filters): preserve backend metric-based sorting#35152
Conversation
There was a problem hiding this comment.
I've completed my review and didn't find any issues.
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
|
🎪 Showtime deployed environment on GHA for 5fe1955 • Environment: http://35.92.5.54:8080 (admin/admin) |
|
🎪 Showtime deployed environment on GHA for 5fe1955 • Environment: http://34.217.58.160:8080 (admin/admin) |
Fixes broken metric-based sorting (e.g., sort by MAU) in select filter components by preventing frontend alphabetical sorting from overriding backend sort order. Problem: - When sortMetric was specified in filter configuration, the backend would correctly sort data by the specified metric (e.g., MAU, revenue, etc.) - However, the frontend sortComparator was then applying alphabetical sorting on the labels, completely overriding the intended metric-based order - This resulted in filters showing values sorted alphabetically instead of by their actual metric values Solution: - Check if formData.sortMetric is specified before applying sort logic - When sortMetric exists, return 0 to preserve the original backend order - Only apply alphabetical sorting when no sortMetric is configured - Add sortMetric to useCallback dependency array for proper re-rendering This ensures that metric-based sorting (like "sort by MAU") works correctly while maintaining alphabetical sorting as a fallback for other cases. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds test coverage for the sortMetric sorting behavior implemented in the SelectFilterPlugin component: - Tests that backend order is preserved when sortMetric is specified - Tests that alphabetical sorting is applied when no sortMetric is specified - Tests ascending and descending alphabetical sorting behavior - Tests that sortMetric takes precedence over sortAscending setting These tests ensure the fix for metric-based sorting works correctly across all scenarios while maintaining backwards compatibility with existing alphabetical sorting behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
5fe1955 to
fa88e17
Compare
|
🎪 Showtime deployed environment on GHA for fa88e17 • Environment: http://35.90.252.245:8080 (admin/admin) |
|
🎪 Showtime deployed environment on GHA for fa88e17 • Environment: http://35.85.221.158:8080 (admin/admin) |
SUMMARY
Fixes broken metric-based sorting (e.g., sort by MAU) in select filter components by preventing frontend alphabetical sorting from overriding backend sort order. Also adds comprehensive test coverage for this functionality.
Problem:
sortMetricwas specified in filter configuration, the backend would correctly sort data by the specified metric (e.g., MAU, revenue, etc.)sortComparatorwas then applying alphabetical sorting on the labels, completely overriding the intended metric-based orderSolution:
formData.sortMetricis specified before applying sort logicsortMetricexists, return 0 to preserve the original backend ordersortMetricis configuredsortMetricto useCallback dependency array for proper re-renderingThis ensures that metric-based sorting (like "sort by MAU") works correctly while maintaining alphabetical sorting as a fallback for other cases.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Select filter options sorted alphabetically regardless of sortMetric setting
After: Select filter options preserve backend metric-based order when sortMetric is specified
TESTING INSTRUCTIONS
You configure a Select filter to sort by a metric like Monthly Active Users (MAU).
Superset’s backend correctly sorts the options: ['Google', 'Microsoft', 'Oracle'] based on MAU.
But the frontend says: “Oh, a list of strings! I’ll sort them alphabetically!” 😬
So you’d see: ['Google', 'Microsoft', 'Oracle'] — even if MAU was 1000, 900, 800.
After:
If sortMetric is set, the frontend does not override the order.
If sortMetric is not set, it still sorts alphabetically (fallback behavior).
🧪 How to Manually Test This in Superset (Step-by-Step)
🔧 1. Create a Filter Box or Filter Select on a Dashboard
Open a dashboard.
Click Edit Dashboard → Add Filter.
Choose the Select Filter plugin.
In the filter config:
Set Column = dimension (e.g., company_name)
Set Sort Metric = metric (e.g., MAU)
Save and view the dashboard.
👀 2. Verify: Options Appear in Metric Order
Example:
ADDITIONAL INFORMATION
Test Coverage Added:
sortMetricis specifiedsortMetricis specifiedsortMetricprecedence oversortAscendingsettingCommits:
fix(filters): preserve backend metric-based sorting in select filters- The core fixtest(filters): add comprehensive tests for sortMetric functionality- Test coverage🤖 Generated with Claude Code