EnxDev commented on code in PR #42472:
URL: https://github.com/apache/superset/pull/42472#discussion_r3673682845
##########
superset/subjects/utils.py:
##########
@@ -79,6 +80,117 @@ def get_user_subject_ids_subquery(user_id: int) ->
CompoundSelect:
return union_all(user_subj, role_subj, group_subj, group_role_subj)
+def get_user_group_subject_ids_subquery(user_id: int) -> Select:
+ """Return a Select of GROUP-type Subject IDs for a user's groups.
+
+ Unlike :func:`get_user_subject_ids_subquery` this covers group membership
+ only — it excludes the user's own subject and any roles.
+ """
+ from flask_appbuilder.security.sqla.models import assoc_user_group
+
+ return (
+ select(Subject.id)
+ .join(assoc_user_group, Subject.group_id ==
assoc_user_group.c.group_id)
+ .where(
+ assoc_user_group.c.user_id == user_id,
+ Subject.type == SubjectType.GROUP,
+ )
+ )
+
+
+def get_user_group_subjects(user_id: int) -> list[Subject]:
+ """Return the GROUP-type Subjects for every group a user belongs to.
+
+ Any group whose Subject row is missing is backfilled on demand (see
+ :func:`get_or_create_group_subject`) rather than skipped: a single viewer
+ disables an asset's datasource-access fallback, so a partial viewer set
+ would silently lock out the members of an unsynced group.
+ """
+ from flask_appbuilder.security.sqla.models import assoc_user_group
+
+ subjects = (
+ db.session.query(Subject)
+ .filter(Subject.id.in_(get_user_group_subject_ids_subquery(user_id)))
+ .all()
+ )
+ member_group_ids = {
+ row[0]
+ for row in db.session.execute(
+ select(assoc_user_group.c.group_id).where(
+ assoc_user_group.c.user_id == user_id
+ )
+ )
+ }
+ for group_id in member_group_ids - {subject.group_id for subject in
subjects}:
+ if subject := get_or_create_group_subject(group_id):
+ subjects.append(subject)
Review Comment:
Good catch — addressed together with the except scope below. The real risk
was that an unrelated failure surfaced by the savepoint's flush would be
swallowed; get_or_create_group_subject now re-raises any IntegrityError that
did not leave a matching row, so an unrelated failure propagates instead of
being masked into a missing group. (Keeping the SAVEPOINT is still necessary to
isolate the expected concurrent unique-key insert without poisoning the outer
transaction; a fully separate session would leave the new Subject invisible to
the caller.)
--
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]
