aminghadersohi commented on code in PR #45036:
URL: https://github.com/apache/superset/pull/45036#discussion_r4244585916
##########
tests/unit_tests/models/dashboard_test.py:
##########
@@ -737,3 +740,191 @@ def
test_datasets_trimmed_for_slices_keeps_colliding_ids_separate() -> None:
# with the exact slice list (no semantic-view slice leaking into it).
assert result == [(table_datasource, {"cols": ["column"]})]
table_datasource.data_for_slices.assert_called_once_with([sesh_table_slice])
+
+
+def test_custom_user_model_dashboard_copy_listener(app_context: None) -> None:
+ """Ensure dashboard copy events can be registered dynamically
+ for custom user models.
+ """
+ from superset.models.dashboard import register_dashboard_copy_events
+
+ assert callable(register_dashboard_copy_events)
Review Comment:
NIT: `assert callable(...)` can't fail once the import succeeds, so this
test pins nothing; `test_register_dashboard_copy_events_dynamic_and_idempotent`
below already covers registration. Safe to drop.
##########
superset/models/dashboard.py:
##########
@@ -70,26 +70,41 @@
logger = logging.getLogger(__name__)
-def copy_dashboard(_mapper: Mapper, _connection: Connection, target:
Dashboard) -> None:
- dashboard_id = app.config["DASHBOARD_TEMPLATE_ID"]
- if dashboard_id is None:
- return
+def _copy_dashboard_for_user(
+ session: Session,
+ target: Any,
+ dashboard_id: int | str,
+) -> None:
+ """Copy the configured template dashboard to a newly created user.
+ Clones the template dashboard metadata and slices, associates the new user
+ as an editor and their groups as viewers, and configures the cloned
dashboard
+ as the user's welcome dashboard.
+
+ :param session: The active SQLAlchemy session to use for queries and
persistence.
+ :param target: The newly created user instance.
+ :param dashboard_id: The ID of the template dashboard to copy.
+ """
+ from superset.subjects.models import Subject
from superset.subjects.utils import (
get_default_viewers_for_groups,
get_user_subject,
)
- session = sqla.inspect(target).session # pylint: disable=disallowed-name
- new_user = session.query(User).filter_by(id=target.id).first()
+ def _rebind(subjects: Sequence[Subject | None]) -> list[Subject]:
+ """The helpers resolve subjects on ``db.session``; a persistent object
+ cannot be cascaded into a second session."""
+ ids = [s.id for s in subjects if s is not None]
+ return session.query(Subject).filter(Subject.id.in_(ids)).all() if ids
else []
- # copy template dashboard to user
template = session.query(Dashboard).filter_by(id=int(dashboard_id)).first()
- editors = []
- if new_user:
- subj = get_user_subject(new_user.id)
- if subj:
- editors.append(subj)
+ if not template:
Review Comment:
NIT: if `DASHBOARD_TEMPLATE_ID` points at a missing dashboard, new users
silently get no welcome dashboard. A `logger.warning` here would make that
misconfiguration visible.
--
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]
