fix: Database List Sorted #15619

AAfghahi · 2021-07-09T17:58:27Z

SUMMARY

Screen.Recording.2021-07-09.at.1.55.10.PM.mov

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

eschutho · 2021-07-09T18:09:53Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -559,11 +560,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
        onChange={setDatabaseModel}
        placeholder="Choose a database..."
      >
-        {availableDbs?.databases
-          ?.sort((a: DatabaseForm, b: DatabaseForm) =>
-            a.name.localeCompare(b.name),


you're losing your sorting here by name.

eschutho · 2021-07-09T18:10:14Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -342,6 +342,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
  const dbImages = getDatabaseImages();
  const connectionAlert = getConnectionAlert();
  const isEditMode = !!databaseId;
+  const sortedAvailableDBs = availableDbs?.databases.sort();


this won't actually sort because the databases are objects... see note below.

eschutho · 2021-07-09T18:10:43Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -613,7 +611,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({

  const renderPreferredSelector = () => (
    <div className="preferred">
-      {availableDbs?.databases
+      {sortedAvailableDBs


This should keep the original sort order from the api response.

codecov · 2021-07-09T18:26:27Z

Codecov Report

Merging #15619 (6f2bf3f) into master (f67e402) will increase coverage by 0.07%.
The diff coverage is 81.81%.

❗ Current head 6f2bf3f differs from pull request most recent head 46eecbd. Consider uploading reports for the commit 46eecbd to get more accurate results

@@            Coverage Diff             @@
##           master   #15619      +/-   ##
==========================================
+ Coverage   76.88%   76.95%   +0.07%     
==========================================
  Files         976      976              
  Lines       51325    51325              
  Branches     6910     6912       +2     
==========================================
+ Hits        39459    39496      +37     
+ Misses      11647    11608      -39     
- Partials      219      221       +2

Flag	Coverage Δ
javascript	`71.41% <81.81%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/views/CRUD/hooks.ts	`48.23% <ø> (ø)`
...s/CRUD/data/database/DatabaseModal/ModalHeader.tsx	`89.18% <66.66%> (-2.48%)`	⬇️
...et-frontend/src/messageToasts/components/Toast.tsx	`92.50% <85.71%> (-0.19%)`	⬇️
...c/views/CRUD/data/database/DatabaseModal/index.tsx	`47.56% <100.00%> (-0.31%)`	⬇️
superset/models/core.py	`90.02% <0.00%> (+0.26%)`	⬆️
superset/connectors/sqla/models.py	`89.90% <0.00%> (+1.40%)`	⬆️
superset/db_engine_specs/presto.py	`89.89% <0.00%> (+5.47%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f67e402...46eecbd. Read the comment docs.

eschutho · 2021-07-09T18:50:14Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

+  const renderAvailableSelector = () => {
+    let sortedAvailableDBs;
+    if (availableDbs) {
+      sortedAvailableDBs = availableDbs?.databases.slice();


you can also make a simple copy with the spread operator.

eschutho · 2021-07-09T19:11:53Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

+  const renderAvailableSelector = () => {
+    let sortedAvailableDBs;
+    if (availableDbs) {
+      sortedAvailableDBs = [...(availableDbs?.databases || [])];


one last thing.. can you move this down to line 568 and remove a few lines of code that way?

eschutho · 2021-07-09T19:20:01Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

-              {database.name}
-            </Select.Option>
-          ))}
+        {availableDbs &&


since you have the conditional operator below, you don't need to check for availableDbs again here.

eschutho

Looks great!

rosemarie-chiu · 2021-07-09T21:15:01Z

🏷 2021.27

* sorted the database list * revisions * cloned the array * one more time with feeling * added documentation link as well (cherry picked from commit faf6fcd)

* sorted the database list * revisions * cloned the array * one more time with feeling * added documentation link as well

pull-request-size bot added the size/S label Jul 9, 2021

sorted the database list

5cb434c

AAfghahi force-pushed the ch19555_shuffleFiles branch from d790d5f to 5cb434c Compare July 9, 2021 18:03

eschutho reviewed Jul 9, 2021

View reviewed changes

revisions

3f89a8d

pull-request-size bot added size/L and removed size/S labels Jul 9, 2021

eschutho reviewed Jul 9, 2021

View reviewed changes

cloned the array

7569da5

AAfghahi force-pushed the ch19555_shuffleFiles branch from 1ffc3a3 to 7569da5 Compare July 9, 2021 19:08

eschutho reviewed Jul 9, 2021

View reviewed changes

one more time with feeling

a24b2e2

pull-request-size bot added size/S and removed size/L labels Jul 9, 2021

eschutho reviewed Jul 9, 2021

View reviewed changes

added documentation link as well

46eecbd

eschutho approved these changes Jul 9, 2021

View reviewed changes

eschutho merged commit faf6fcd into apache:master Jul 9, 2021

eschutho deleted the ch19555_shuffleFiles branch July 9, 2021 20:06

superset-github-bot bot added the preset:2021.27 label Jul 9, 2021

cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021

fix: Database List Sorted (apache#15619)

3e2d56c

* sorted the database list * revisions * cloned the array * one more time with feeling * added documentation link as well

QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021

fix: Database List Sorted (apache#15619)

396b3e5

* sorted the database list * revisions * cloned the array * one more time with feeling * added documentation link as well

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

fix: Database List Sorted (apache#15619)

16c29d3

* sorted the database list * revisions * cloned the array * one more time with feeling * added documentation link as well

mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label Mar 12, 2024

mistercrunch added the 🚢 1.3.0 label Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Database List Sorted #15619

fix: Database List Sorted #15619

AAfghahi commented Jul 9, 2021

eschutho Jul 9, 2021

eschutho Jul 9, 2021

eschutho Jul 9, 2021

codecov bot commented Jul 9, 2021 •

edited

Loading

eschutho Jul 9, 2021

eschutho Jul 9, 2021

eschutho Jul 9, 2021

eschutho left a comment

rosemarie-chiu commented Jul 9, 2021

fix: Database List Sorted #15619

fix: Database List Sorted #15619

Conversation

AAfghahi commented Jul 9, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

eschutho Jul 9, 2021

Choose a reason for hiding this comment

eschutho Jul 9, 2021

Choose a reason for hiding this comment

eschutho Jul 9, 2021

Choose a reason for hiding this comment

codecov bot commented Jul 9, 2021 • edited Loading

Codecov Report

eschutho Jul 9, 2021

Choose a reason for hiding this comment

eschutho Jul 9, 2021

Choose a reason for hiding this comment

eschutho Jul 9, 2021

Choose a reason for hiding this comment

eschutho left a comment

Choose a reason for hiding this comment

rosemarie-chiu commented Jul 9, 2021

codecov bot commented Jul 9, 2021 •

edited

Loading