-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
fix(sqllab): Sort db selector options by the API order #28749
fix(sqllab): Sort db selector options by the API order #28749
Conversation
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.
@justinpark if you look at the default DEFAULT_SORT_COMPARATOR implementation, you can see that it will sort items alphabetically if label
is of type string
. In this case, it's not sorting alphabetically because label
is being set to a React component here instead of using customLabel
for that purpose. If you set label
to database_name
and customLabel
to the React component, you'll get the desired behavior.
You are right, but I think that if the API that includes the actual sorting items is implemented, we would want the results to be sorted according to the API. Also, relying on client-side sorting can lead to inconsistencies, especially when handling paginated results, so I believe it is better to follow the API's results. Additionally, I have optimized the code to use the suggested order field. |
@justinpark Could you also update the PR description to say that the objective is to sort by the API order? |
SUMMARY
Currently, the order of the db selector dropdown in SQL Lab is always sorted by record ID, regardless of the intended sort order.
This issue arises due to two reasons:
This commit fixes these issues, ensuring that the db selector is sorted by the API order as intended.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Added unit tests.
ADDITIONAL INFORMATION