aminghadersohi commented on PR #45864: URL: https://github.com/apache/superset/pull/45864#issuecomment-6515084180
End-to-end check at a248826c944f79f2a2abcaeb1a857949ce4f5755 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. Something I found that is not caused by this PR (same on the merge-base): after a database rename, the chart's `perm` column keeps the old database name when the dataset has a schema. The core UPDATE in `_update_vm_datasources_access` rewrites it, but `SyncPermissionsCommand` then sets `chart.schema_perm` through the ORM and `set_related_perm` copies `perm` back from a stale dataset object. A user whose access comes from the dataset grant then gets 404 on `GET /api/v1/chart/<id>/data/` until the chart is saved again. I'll handle that separately. A small test gap here: `database_rename_permissions_test.py` has no chart, so it does not assert chart `perm`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
