fix: database permissions on update and delete (avoid orphaned perms)

[dpgaspar]

SUMMARY

Currently updating a database_name or deleting a database does not update or delete any associated permissions.

Examples:
Creating a database named db1 with 2 datasources named: (ds1, ds2), creates the following permissions:
database_access -> [db1](id:1)
datasource_access -> [db1].[ds1](id:1)
datasource_access -> [db1].[ds2](id:2)

Then updating the database name to db1_updated, leaves the permissions at the following state:
database_access -> [db1](id:1)
datasource_access -> [db1].[ds1](id:1)
datasource_access -> [db1].[ds2](id:2)
database_access -> [db2](id:1)
datasource_access -> [db2].[ds1](id:1)
datasource_access -> [db2].[ds2](id:2)

This means that changing a database_name will remove access to all non admin users to that database on SQLLab. Besides leaving orphaned permissions on the DB.

This PR updates the permissions has expected, final result:
database_access -> [db2](id:1)
datasource_access -> [db2].[ds1](id:1)
datasource_access -> [db2].[ds2](id:2)

cc: @zephyring

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 #20081 (62a6ab0) into master (77db065) will decrease coverage by 11.45%.
The diff coverage is 51.06%.

❗ Current head 62a6ab0 differs from pull request most recent head 002ead3. Consider uploading reports for the commit 002ead3 to get more accurate results

@@             Coverage Diff             @@
##           master   #20081       +/-   ##
===========================================
- Coverage   66.25%   54.80%   -11.46%     
===========================================
  Files        1758     1758               
  Lines       67048    67058       +10     
  Branches     7118     7092       -26     
===========================================
- Hits        44423    36748     -7675     
- Misses      20808    28497     +7689     
+ Partials     1817     1813        -4     

Flag	Coverage Δ
hive	53.20% <41.28%> (-0.05%)	⬇️
mysql	?
postgres	?
presto	53.10% <41.28%> (-0.05%)	⬇️
python	57.78% <43.11%> (-23.78%)	⬇️
sqlite	?
unit	50.17% <27.52%> (-0.09%)	⬇️

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

Impacted Files	Coverage Δ
...frontend/src/components/DatabaseSelector/index.tsx	88.88% <ø> (ø)
...et-frontend/src/components/TableSelector/index.tsx	76.08% <ø> (ø)
...t-frontend/src/filters/components/GroupBy/types.ts	100.00% <ø> (ø)
...rontend/src/filters/components/TimeColumn/types.ts	100.00% <ø> (ø)
...frontend/src/filters/components/TimeGrain/types.ts	100.00% <ø> (ø)
superset-frontend/src/reports/actions/reports.js	39.39% <0.00%> (ø)
...rontend/src/visualizations/TimeTable/TimeTable.jsx	0.00% <ø> (ø)
superset/dashboards/api.py	51.15% <0.00%> (-41.33%)	⬇️
superset/dashboards/filters.py	48.75% <ø> (-45.00%)	⬇️
superset/databases/commands/create.py	70.83% <ø> (-21.01%)	⬇️
... and 303 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

[zephyring]

why can't these logic sit inside database_after_update

[dpgaspar]

Very valid point, I would prefer to place it on the event listener also, but my decision is that database.get_all_schema_names() will query the analytics DB for all available schemas, so it needs the database to be online, it seems unexpected to place this operation on a "background" task

[dpgaspar]

also if we move this to after update SQLAlchemy event we loose the capability of correctly identifying any possible failure has a db connection failure and reply with a proper response back to the requester

[zephyring]

nit: I think this line is unnecessary as you already filter by id