[aminghadersohi]

SUMMARY

Renaming a database (PUT /api/v1/database/<id>) can fail with 'NoneType' object has no attribute 'id' when one of its datasets has no datasource_access view menu under the old database name.

Root cause: on rename, database_after_update calls _update_vm_datasources_access, which renames each dataset view menu with UPDATE ab_view_menu ... WHERE name = <old dataset perm> and then reads the row back by the new name with _find_view_menu_on_sqla_event. If the old dataset view menu is missing, the UPDATE matches no row, the read returns None, and view_menu_.id raises. When the old database permission is missing too, the logs show "Could not find previous database permission ..." first; that warning comes from the database path, which already recreates the permission and does not crash. The crash comes from the dataset path.

Changes in superset/security/manager.py:

• _update_vm_datasources_access: if the renamed dataset view menu cannot be read back, log a warning and create the datasource_access permission under the new name (this matches what _update_dataset_perm already does when the old dataset permission is missing). The dataset and chart perm columns are still rewritten.
• _find_permission_on_sqla_event / _find_view_menu_on_sqla_event return None when no row matches instead of raising AttributeError.
• The other callers handle None: _update_vm_database_access skips the update hook, and _insert_pvm_on_sqla_event logs and returns if it cannot read back a row it just inserted (permission, view menu or permission view).

Other code that renames a view menu and then reads it back, and why it is safe:

• _update_vm_database_access: renames by the id of a permission view it has already found, and skips the read-back when there is none.
• _update_dataset_perm: checks that the old dataset view menu exists before renaming, and inserts a new permission otherwise.
• semantic_layer_before_update and its view cascade, semantic_view_before_update: check that the old view menu exists before renaming (insert, or skip, otherwise).
• _insert_pvm_on_sqla_event: re-reads rows it has just inserted on the same connection. Commit a248826c944f79f2a2abcaeb1a857949ce4f5755 adds explicit missing-row guards for those reads.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

pytest tests/unit_tests/security/database_rename_permissions_test.py

test_rename_database_with_missing_permission creates a database with one schema-less dataset and a chart, deletes the old database permission, the dataset permission or both, then renames the database. Without the fix, the dataset and database-and-dataset cases fail with AttributeError: 'NoneType' object has no attribute 'id'. With the fix all three pass, the database and dataset permissions exist under the new name, and the persisted chart perm references the renamed database. The database-only case, including the chart assertion, also passes without the source fix.

test_database_permission_update_skips_hook_when_view_menu_missing injects a missing database view-menu readback to pin the defensive guard: the update hook must not receive None. A successful update is visible on the same connection, so this is a fault-injection test rather than a natural missing-row scenario.

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

❌ Patch coverage is 42.10526% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.70%. Comparing base (492494b) to head (a39853b).
⚠️ Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/security/manager.py	42.10%	7 Missing and 4 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #45864   +/-   ##
=======================================
  Coverage   84.70%   84.70%           
=======================================
  Files        3069     3069           
  Lines      205610   205668   +58     
  Branches    48330    48342   +12     
=======================================
+ Hits       174157   174208   +51     
- Misses      28160    28164    +4     
- Partials     3293     3296    +3     

Flag	Coverage Δ
hive	34.50% <0.00%> (-0.01%)	⬇️
mysql	52.57% <0.00%> (-0.02%)	⬇️
postgres	52.58% <0.00%> (-0.02%)	⬇️
presto	36.23% <0.00%> (-0.01%)	⬇️
python	87.75% <42.10%> (+<0.01%)	⬆️
sqlite	52.34% <0.00%> (-0.02%)	⬇️
unit	82.48% <42.10%> (+<0.01%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aminghadersohi]

End-to-end check at a248826 against the merge-base, in-process with the real app (sqlite metadata DB, real PUT /api/v1/database/<id> through the test client, a schema'd dataset plus a chart, a Gamma role granted the dataset's datasource_access and a Gamma user without it):

• Old dataset permission missing, or both missing: merge-base returns 500 ('NoneType' object has no attribute 'id'); this branch returns 200 and recreates database_access and datasource_access under the new name. Old database permission missing: 200 on both.
• Healthy rename: identical result on both (same permission names, one row each, no warnings, no duplicate view menus).
• The granted role keeps dataset access (can_access_datasource, raise_for_access); the ungranted user stays denied.
• Dataset rename (PUT /api/v1/dataset/<id>) is unchanged.

Correction to what I first wrote here about the chart perm: it only goes wrong when the dataset is already loaded in the session before the database rename (my harness kept a single app context across the PUT). A rename from a fresh request session is not affected on master. The same stale-instance problem is reachable on master through asset import (a dataset renamed or moved to another schema leaves its chart on the old perm); it is fixed separately in #45874, which does not touch the lines this PR edits.

[codeant-ai-for-open-source]

  CodeAnt PR Risk

🟢  Low Risk

• The PR appears safe to merge; no actionable issues were identified in the review.

Assessed commit: a39853b3fc2e