rusackas commented on code in PR #41294:
URL: https://github.com/apache/superset/pull/41294#discussion_r3456953681
##########
superset/commands/security/update.py:
##########
@@ -50,14 +50,19 @@ def validate(self) -> None:
self._model = RLSDAO.find_by_id(int(self._model_id))
if not self._model:
raise RLSRuleNotFoundError()
- roles = populate_roles(self._roles)
- tables = (
- db.session.query(SqlaTable)
- .filter(SqlaTable.id.in_(self._tables)) # type:
ignore[attr-defined]
- .all()
- )
- if len(tables) != len(self._tables):
- raise DatasourceNotFoundValidationError()
- raise_for_datasource_access(tables)
- self._properties["roles"] = roles
- self._properties["tables"] = tables
+ # Only resolve and overwrite the relationships that are actually
present
+ # in the request body. A partial update (e.g. changing only the name)
+ # must leave the rule's existing tables/roles bindings untouched rather
+ # than replacing them with empty lists.
+ if "roles" in self._properties:
+ self._properties["roles"] = populate_roles(self._roles)
+ if "tables" in self._properties:
+ tables = (
+ db.session.query(SqlaTable)
+ .filter(SqlaTable.id.in_(self._tables)) # type:
ignore[attr-defined]
+ .all()
+ )
+ if len(tables) != len(self._tables):
+ raise DatasourceNotFoundValidationError()
+ raise_for_datasource_access(tables)
+ self._properties["tables"] = tables
Review Comment:
Good catch. Fixed: when `tables` is omitted I now run
`raise_for_datasource_access` against `self._model.tables`, matching what
delete already does. Added a test.
--
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]
