feat(sqllab): SQLEditor Extension

[Antonio-RiveroMartnez]

SUMMARY

In order to extend capabilities in our SQLEditor we need to provide an extension. This PR is adding such extension and adding tests for it.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

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]

Codecov Report

Merging #24205 (72b14fe) into master (8375baa) will decrease coverage by 1.54%.
The diff coverage is 40.90%.

❗ Current head 72b14fe differs from pull request most recent head b51fe1f. Consider uploading reports for the commit b51fe1f to get more accurate results

@@            Coverage Diff             @@
##           master   #24205      +/-   ##
==========================================
- Coverage   68.22%   66.69%   -1.54%     
==========================================
  Files        1957     1957              
  Lines       75594    75600       +6     
  Branches     8224     8223       -1     
==========================================
- Hits        51572    50418    -1154     
- Misses      21914    23074    +1160     
  Partials     2108     2108              

Flag	Coverage Δ
javascript	54.73% <38.09%> (+<0.01%)	⬆️
unit	?

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

Impacted Files	Coverage Δ
...nd/src/components/ErrorMessage/BasicErrorAlert.tsx	93.75% <ø> (ø)
superset-frontend/src/components/Table/index.tsx	75.29% <ø> (ø)
...veFilters/FilterBar/FilterControls/FilterValue.tsx	5.66% <0.00%> (-0.06%)	⬇️
superset-frontend/src/dashboard/types.ts	100.00% <ø> (ø)
...perset-frontend/src/hooks/apiResources/queryApi.ts	100.00% <ø> (ø)
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx	55.36% <28.57%> (+0.19%)	⬆️
...frontend/src/SqlLab/components/SqlEditor/index.jsx	59.25% <100.00%> (+0.65%)	⬆️
...onents/ErrorMessage/ErrorMessageWithStackTrace.tsx	83.33% <100.00%> (+3.33%)	⬆️
...set-frontend/src/dashboard/util/permissionUtils.ts	86.66% <100.00%> (ø)
superset/config.py	92.02% <100.00%> (+0.02%)	⬆️

... and 93 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[justinpark]

@Antonio-RiveroMartnez
Not all custom extension requires all these properties such like tables and database
Therefore, I think it's not a good idea passing these specified properties because you can access these values from redux. (you can also access the queryEditor object by useQueryEditor hook)
Only thing you need to pass is queryEditorId and then each custom extension component should use a hook to retrieve the properties that needed.

Suggested change

	tables: TTables[];
	queryEditor: TQueryEditor;
	database: TDatabase;
	queryEditorId: string;

[justinpark]

cc: @michael-s-molina

[Antonio-RiveroMartnez]

Hey @justinpark ! Good call, I will change this. I am working a bit more on this extension feature so I'll move this PR to be a DRAFT until I get everything in place and final props etc are figured out. ILYK. Thanks for the input as always 👏

[michael-s-molina]

@Antonio-RiveroMartnez could you help me review #24214 and rebase your PR on top of that one?

[Antonio-RiveroMartnez]

@Antonio-RiveroMartnez could you help me review #24214 and rebase your PR on top of that one?

Oh nice! Yes I will do so.

[geido]

@Antonio-RiveroMartnez I am thinking if we need to pass setQueryEditorAndSaveSqlWithDebounce and startQuery or if those can be implemented by hooking into the state?

[Antonio-RiveroMartnez]

Hey @geido good call! I understand we want to keep the extension with as little props/dependencies as possible, but for example in the case of startQuery, it needs to dispatch two actions in the main component, so, right now I don't see a clearer way than passing the whole callback, like we do on other extensions like SSHTunneling, etc. But If anyone has an idea on how this might work, I'm more than happy to discuss. Thanks

[justinpark]

@geido @Antonio-RiveroMartnez similar to the previous comment, this action props doesn't make sense since your custom component only needs these actions. You can hook up these actions in your custom component by combining of existing action helpers

For example you can build startQuery in your component by combining of runQueryFromSqlEditor and setActiveSouthPaneTab

      dispatch(
        runQueryFromSqlEditor(
          database,
          queryEditor,
          defaultQueryLimit,
          ctasArg ? ctas : '',
          ctasArg,
          ctas_method,
        ),
      );
      dispatch(setActiveSouthPaneTab('Results'));

[Antonio-RiveroMartnez]

Hey @justinpark sure, I can recreate all those with the helpers, but since we are not changing how they work, just invoking them, I treated them like other props our existing extensions are using. IMHO I see no major benefit by forcing a duplication of code in the extension. However, if you truly see this like an scenario we should force such duplication, I can work on it in a follow up PR. Thanks as always for the input.

[betodealmeida]

Nice!

[eschutho]

LGTM!