Skip to content
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(sql lab): select edit on query from history doesn't upload editor properly #19290

Conversation

diegomedina248
Copy link
Contributor

@diegomedina248 diegomedina248 commented Mar 21, 2022

SUMMARY

In SQL Lab, after executing a query, any query selected from the history tab (by pressing the edit button) does not fire unless the user modify something in it.

This PR does two things:

  • Updates the SQL state property when it's changed on the reducer
  • Modifies the SQL Editor text selection handling to make sure it properly captures the text selected

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

Screen.Recording.2022-03-21.at.16.02.50.mov

After:

Screen.Recording.2022-03-21.at.15.59.51.mov

TESTING INSTRUCTIONS

  1. Run a query in SQL Lab (Query A)
  2. In the same tab, edit your query and run a completely different query in SQL Lab (Query B)
  3. In the Query History for that tab, select the "edit" option next to Query A
  4. Do NOT edit your query and try to run it

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • 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

@codecov
Copy link

codecov bot commented Mar 21, 2022

Codecov Report

Merging #19290 (4583cd3) into master (c07a707) will increase coverage by 0.00%.
The diff coverage is 20.00%.

@@           Coverage Diff           @@
##           master   #19290   +/-   ##
=======================================
  Coverage   66.51%   66.52%           
=======================================
  Files        1667     1667           
  Lines       64415    64427   +12     
  Branches     6503     6505    +2     
=======================================
+ Hits        42846    42857   +11     
- Misses      19884    19885    +1     
  Partials     1685     1685           
Flag Coverage Δ
javascript 51.36% <20.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...frontend/src/SqlLab/components/SqlEditor/index.jsx 50.27% <0.00%> (-0.57%) ⬇️
...d/src/SqlLab/components/AceEditorWrapper/index.tsx 45.76% <33.33%> (+0.93%) ⬆️
...nd/src/components/MessageToasts/ToastPresenter.tsx 93.33% <0.00%> (-6.67%) ⬇️
superset-frontend/src/embedded/index.tsx 0.00% <0.00%> (ø)
...ontend/src/components/MessageToasts/withToasts.tsx 100.00% <0.00%> (ø)
...rset-ui-core/src/connection/SupersetClientClass.ts 100.00% <0.00%> (ø)
...nd/src/components/MessageToasts/ToastContainer.jsx
...nd/src/components/MessageToasts/ToastContainer.tsx 100.00% <0.00%> (ø)
superset-frontend/src/views/components/Menu.tsx 60.00% <0.00%> (+1.02%) ⬆️
...eFilters/FiltersConfigModal/FiltersConfigModal.tsx 64.67% <0.00%> (+1.63%) ⬆️

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 c07a707...4583cd3. Read the comment docs.

@rusackas rusackas requested a review from kgabryje March 21, 2022 22:03
@rusackas rusackas requested a review from eschutho April 6, 2022 05:03
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's add a task to convert AceEditorWrapper to be a functional component and clean up the lifecycle quirks.

@rusackas rusackas merged commit bbe0af3 into apache:master Apr 15, 2022
philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels Preset-Patch size/S 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants