fix(superset): Fixed API for bulk delete of embedded dashboards

[sinhashubham95]

SUMMARY

This change fixes bulk delete of dashboards which have embedding enabled.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

NA

TESTING INSTRUCTIONS

curl $'http://localhost:8088/api/v1/dashboard/?q=\u0021(id1,id2)' \ -X 'DELETE' \ -H 'Accept: application/json' \ -H 'Cookie: session=some-session' \ -H 'Origin: http://localhost:8088' \ -H 'X-CSRFToken: some-token' \ --compressed

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 #21911 (88172e2) into master (04b017e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #21911   +/-   ##
=======================================
  Coverage   66.91%   66.91%           
=======================================
  Files        1807     1807           
  Lines       69139    69140    +1     
  Branches     7392     7392           
=======================================
+ Hits        46266    46267    +1     
  Misses      20963    20963           
  Partials     1910     1910           

Flag	Coverage Δ
hive	52.92% <0.00%> (-0.01%)	⬇️
mysql	78.35% <100.00%> (+<0.01%)	⬆️
postgres	78.41% <100.00%> (+<0.01%)	⬆️
presto	52.82% <0.00%> (-0.01%)	⬇️
python	81.47% <100.00%> (+<0.01%)	⬆️
sqlite	76.90% <100.00%> (+<0.01%)	⬆️
unit	51.07% <0.00%> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
superset/dashboards/dao.py	96.37% <100.00%> (+0.02%)	⬆️

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

[sinhashubham95]

@villebro can you please check this?

[sinhashubham95]

@lilykuang @michael-s-molina can you please check this?

[sinhashubham95]

@villebro @lilykuang @michael-s-molina can you please check this?

[villebro]

FYI @sinhashubham95 I restarted the cypress test on CI, will review now

[sinhashubham95]

Thanks @villebro

[villebro]

The functional change LGTM with a minor nit on the test. However, I wasn't able to make the test fail by removing the functional change. Can you double check that the test is accurately testing the fix?

[sinhashubham95]

@villebro I have refactored the test case as per your suggestion. Also, I verified the test case failing for me without the change when I was running from my local.

[villebro]

@villebro I have refactored the test case as per your suggestion. Also, I verified the test case failing for me without the change when I was running from my local.

@sinhashubham95 can you post the failing test output without the functional changes? I'm still not able to reproduce the test failure without the functional change. What I did to run the tests locally:

tox -e sqlite tests/integration_tests/dashboards/api_tests.py

Btw, I'm getting an unrelated test failure when running the tests locally and I don't fully understand how that test (test_gets_not_certified_dashboards_filter) is passing on CI (the test seems wrong), but I can look into that, as it's unrelated.

[sinhashubham95]

@villebro just to specify this part of the test case failing without this change, I removed the change keeping the test case. Check these job run failures - MYSQL and Postgres.

[sinhashubham95]

@villebro please check now.

[villebro]

@villebro just to specify this part of the test case failing without this change, I removed the change keeping the test case. Check these job run failures - MYSQL and Postgres.

@sinhashubham95 oh, now I get it, the test didn't fail on sqlite because it doesn't enforce foreign keys 🤦 thanks for helping me get to the bottom of this!

[villebro]

LGTM