EnxDev commented on code in PR #42472:
URL: https://github.com/apache/superset/pull/42472#discussion_r3656986102
##########
superset/subjects/utils.py:
##########
@@ -176,6 +241,26 @@ def get_or_create_role_subject(role_id: int) -> Subject |
None:
return get_role_subject(role_id)
+def subjects_from_groups(groups: list[Group | int]) -> list[Subject]:
+ """Convert a list of Group objects or group IDs to GROUP-type Subjects.
+
+ Mirrors :func:`subjects_from_roles`, but resolves the whole list in one
+ query rather than one per group. Silently skips groups without a matching
+ Subject row.
Review Comment:
Good catch; fixed. Added get_or_create_group_subject (mirroring
get_or_create_role_subject) and used it in get_user_group_subjects to backfill
any group whose Subject row is missing, so a partial viewer set can't silently
lock out an unsynced group. I left subjects_from_groups skip-only on purpose:
its only caller (get_default_viewers_for_groups) runs inside the user's
after_insert flush event, where the on-demand sync's db.session.flush() is
unsafe, and it's handed pre-existing groups whose Subjects already exist I
documented that in its docstring. Tests added for the group get-or-create paths
and the backfill.
--
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]
