fix: sorting on "Modified By" in chart table #16208

pkdotson · 2021-08-11T20:48:13Z

SUMMARY

This pr fixes sorting problem for last_saved_by in the chartlist view. The accessor was set to the wrong key.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

before
https://user-images.githubusercontent.com/17326228/129097514-0cdb53d5-27ca-4a86-914e-45cd147d19d3.mov

after

Screen.Recording.2021-08-11.at.1.47.25.PM.mov

TESTING INSTRUCTIONS

Go to the chart listview page and clicked the modified by sorting button to test out proper sorting.

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

nytai · 2021-08-11T21:11:29Z

superset-frontend/src/views/CRUD/chart/ChartList.tsx

@@ -285,7 +285,7 @@ function ChartList(props: ChartListProps) {
          </a>
        ),
        Header: t('Modified by'),
-        accessor: 'last_saved_by',
+        accessor: 'last_saved_by.id',


does this result in an alphabetical sort?

hmm should probably change this to .lastname then? I only tested with superset admin user

i'll do first name since the original logic was to use first name.

codecov · 2021-08-11T21:12:58Z

Codecov Report

Merging #16208 (9b788dd) into master (ccfc95f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #16208   +/-   ##
=======================================
  Coverage   76.73%   76.73%           
=======================================
  Files         996      996           
  Lines       52999    52999           
  Branches     6738     6738           
=======================================
  Hits        40668    40668           
  Misses      12102    12102           
  Partials      229      229

Flag	Coverage Δ
javascript	`70.97% <ø> (ø)`

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

Impacted Files	Coverage Δ
...perset-frontend/src/views/CRUD/chart/ChartList.tsx	`72.61% <ø> (ø)`

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 ccfc95f...9b788dd. Read the comment docs.

change sort to first name

betodealmeida

rusackas · 2021-08-11T21:48:30Z

/testenv up

github-actions · 2021-08-11T21:50:31Z

@rusackas Ephemeral environment spinning up at http://34.220.60.74:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

github-actions · 2021-08-11T22:31:30Z

Ephemeral environment shutdown and build artifacts deleted.

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

rosemarie-chiu · 2021-08-13T15:41:47Z

🏷 2021.31

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

* initial fix * Update ChartList.tsx change sort to first name

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

* initial fix * Update ChartList.tsx change sort to first name

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit 9ad119b)

initial fix

1e45c9a

superset-github-bot bot added the preset-io label Aug 11, 2021

pull-request-size bot added the size/XS label Aug 11, 2021

pkdotson requested review from junlincc, rusackas and michael-s-molina August 11, 2021 20:48

nytai reviewed Aug 11, 2021

View reviewed changes

Update ChartList.tsx

9b788dd

change sort to first name

rusackas changed the title ~~fix: sorting modified by in charttable~~ fix: sorting on "Modified By" in chart table Aug 11, 2021

betodealmeida approved these changes Aug 11, 2021

View reviewed changes

rusackas approved these changes Aug 11, 2021

View reviewed changes

pkdotson merged commit b4555df into apache:master Aug 11, 2021

rusackas deleted the fix-accessor-chartlist branch August 12, 2021 16:17

stevenuray pushed a commit to preset-io/superset that referenced this pull request Aug 12, 2021

fix: sorting on "Modified By" in chart table (apache#16208)

7e0dee6

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

superset-github-bot bot added the preset:2021.31 label Aug 13, 2021

villebro added the v1.3 label Aug 16, 2021

villebro pushed a commit that referenced this pull request Aug 16, 2021

fix: sorting on "Modified By" in chart table (#16208)

31c57db

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021

fix: sorting on "Modified By" in chart table (apache#16208)

21c9e33

* initial fix * Update ChartList.tsx change sort to first name

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

fix: sorting on "Modified By" in chart table (apache#16208)

e3063af

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

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

fix: sorting on "Modified By" in chart table (apache#16208)

88b7b5b

* initial fix * Update ChartList.tsx change sort to first name

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

fix: sorting on "Modified By" in chart table (apache#16208)

37be21b

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit b4555df)

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

fix: sorting on "Modified By" in chart table (apache#16208)

97cf58d

* initial fix * Update ChartList.tsx change sort to first name (cherry picked from commit 9ad119b)

mistercrunch added 🍒 1.3.0 🍒 1.3.1 🍒 1.3.2 labels Mar 13, 2024

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: sorting on "Modified By" in chart table #16208

fix: sorting on "Modified By" in chart table #16208

pkdotson commented Aug 11, 2021

nytai Aug 11, 2021

pkdotson Aug 11, 2021 •

edited

Loading

pkdotson Aug 11, 2021

codecov bot commented Aug 11, 2021 •

edited

Loading

betodealmeida left a comment

rusackas commented Aug 11, 2021

github-actions bot commented Aug 11, 2021

github-actions bot commented Aug 11, 2021

rosemarie-chiu commented Aug 13, 2021

fix: sorting on "Modified By" in chart table #16208

fix: sorting on "Modified By" in chart table #16208

Conversation

pkdotson commented Aug 11, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

nytai Aug 11, 2021

Choose a reason for hiding this comment

pkdotson Aug 11, 2021 • edited Loading

Choose a reason for hiding this comment

pkdotson Aug 11, 2021

Choose a reason for hiding this comment

codecov bot commented Aug 11, 2021 • edited Loading

Codecov Report

betodealmeida left a comment

Choose a reason for hiding this comment

rusackas commented Aug 11, 2021

github-actions bot commented Aug 11, 2021

github-actions bot commented Aug 11, 2021

rosemarie-chiu commented Aug 13, 2021

pkdotson Aug 11, 2021 •

edited

Loading

codecov bot commented Aug 11, 2021 •

edited

Loading