chore(command): Condense delete/bulk-delete operations

[john-bodley]

SUMMARY

This a a fast follow to #24466 which condensed the delete/bulk-delete DAO operations. Specifically it condenses the delete/bulk-delete commands into a single operation which helps to ensure we adhere to the DRY principle (and removes ~ 500 lines of code).

Though the change maybe somewhat difficult to grok from a GitHub diff perspective the logic is as follows. If there exists commands/bulk_delete.py and optionally commands/delete.py file, then:

1. Delete the commands/delete.py file.
2. Rename commands/bulk_delete.py to commands/delete.py.
3. Rename BulkDelete... to Delete... for commands, exceptions, etc.

Note that delete DAOs currently support either one or a list of models (a feature added in #24466) whereas the commands currently only support a list. Ideally this should be more consistent, but I didn't want to go down the yak shaving route as there likely is more cleanup needed when the commands are refactored as the current validation logic (which mutates state) likely needs to be addressed.

Finally I didn't update the unit tests. The single and bulk deletion tests are still separate.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

• Has associated issue: [SIP-92] Proposal for restructuring the Python code base #20630
• 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 #24607 (a0669d4) into master (767afef) will decrease coverage by 0.07%.
The diff coverage is 91.22%.

❗ Current head a0669d4 differs from pull request most recent head 8d64ce9. Consider uploading reports for the commit 8d64ce9 to get more accurate results

@@            Coverage Diff             @@
##           master   #24607      +/-   ##
==========================================
- Coverage   69.03%   68.97%   -0.07%     
==========================================
  Files        1908     1902       -6     
  Lines       74210    74007     -203     
  Branches     8186     8186              
==========================================
- Hits        51232    51047     -185     
+ Misses      20857    20839      -18     
  Partials     2121     2121              

Flag	Coverage Δ
hive	54.12% <21.92%> (+<0.01%)	⬆️
mysql	79.35% <91.22%> (-0.07%)	⬇️
postgres	79.44% <91.22%> (-0.07%)	⬇️
presto	54.02% <21.92%> (+<0.01%)	⬆️
python	83.45% <91.22%> (-0.05%)	⬇️
sqlite	78.03% <78.94%> (-0.05%)	⬇️
unit	54.84% <21.92%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
superset/reports/commands/exceptions.py	98.33% <ø> (-0.03%)	⬇️
superset/annotation_layers/api.py	86.20% <50.00%> (-0.12%)	⬇️
superset/css_templates/commands/delete.py	87.50% <50.00%> (ø)
superset/queries/saved_queries/commands/delete.py	87.50% <50.00%> (ø)
superset/row_level_security/commands/delete.py	86.36% <50.00%> (ø)
superset/annotation_layers/annotations/api.py	89.23% <66.66%> (-0.09%)	⬇️
superset/css_templates/api.py	95.74% <66.66%> (ø)
superset/datasets/api.py	88.62% <66.66%> (-0.04%)	⬇️
superset/queries/saved_queries/api.py	92.37% <66.66%> (ø)
superset/reports/api.py	90.22% <66.66%> (-0.08%)	⬇️
... and 17 more

... and 2 files with indirect coverage changes

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

[john-bodley]

This illustrates a great example of the benefits of condensing logic. The bulk exceptions:

1. Is subclassed from DeleteFailedError
2. Uses language of the form Annotations could not be deleted.

whereas the non-bulk exceptions:

1. Is subclassed from CommandException.
2. Uses language of the form Annotation delete failed.

Here the legacy bulk logic seems more correct in terms of consistency with how create, update, etc. exceptions are defined.

[john-bodley]

Rather than chart(s), query(ies), or query/queries I opted for—per a Google Search suggestion—always using the plural form for consistency.

[john-bodley]

This was another positive byproduct of the change. Here we're catching a DeleteFailedError whereas in the non-bulk we were catching a DAODeleteFailedError error. The later is more accurate as it's a DAO rather than command failure.

[michael-s-molina]

LGTM. Thank you for the improvements @john-bodley. I left some non-blocking comments.