EnxDev commented on code in PR #42472:
URL: https://github.com/apache/superset/pull/42472#discussion_r3656970997
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -449,10 +449,16 @@ def import_dashboard( # noqa: C901
db.session.flush()
if not existing and user:
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
subj = get_user_subject(user.id)
if subj and subj not in dashboard.editors:
dashboard.editors.append(subj)
+ for viewer in get_default_viewers_for_new_asset(user.id):
+ if viewer not in dashboard.viewers:
+ dashboard.viewers.append(viewer)
Review Comment:
This one doesn't apply here, so no change. viewers isn't in
Dashboard.export_fields/extra_import_fields, so import_from_dict never reads
viewers from a v1 payload — a fresh dashboard's viewers is always empty at this
point, so appending the creator's defaults is correct rather than "expanding an
explicit list." The if not existing guard already blocks the re-import case.
(This is the pattern the chart importer was corrected to match in the thread
above.)
##########
superset/models/helpers.py:
##########
@@ -664,6 +667,10 @@ def reset_ownership(self) -> None:
user_subject = get_user_subject(g.user.id)
if user_subject:
self.editors = [user_subject]
+ # Only dashboards and charts have ``viewers``; this mixin also
+ # serves datasets, which have editors only.
+ if hasattr(self, "viewers"):
+ self.viewers = get_default_viewers_for_new_asset(g.user.id)
Review Comment:
Guarded. reset_ownership now only assigns defaults when none are set: if
hasattr(self, "viewers") and not self.viewers:, so an explicitly-set viewer
list is preserved. Note the create paths listed in the suggestion don't
actually call reset_ownership (only the v0 importers do; the create commands
set viewers via populate_subjects, which already honours explicit viewers) —
but the unconditional assignment was a real sharp edge, now closed, with a
regression test (test_reset_ownership_preserves_explicitly_set_viewers).
--
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]
