diff --git a/superset/commands/report/create.py b/superset/commands/report/create.py index e238102f90a..b3cd0dd16f0 100644 --- a/superset/commands/report/create.py +++ b/superset/commands/report/create.py @@ -18,6 +18,7 @@ import logging from functools import partial from typing import Any, Optional +from flask import g from flask_babel import gettext as _ from marshmallow import ValidationError @@ -30,11 +31,13 @@ from superset.commands.report.exceptions import ( ReportScheduleCreationMethodUniquenessValidationError, ReportScheduleInvalidError, ReportScheduleNameUniquenessValidationError, + ReportScheduleUserEmailNotFoundError, ) from superset.daos.database import DatabaseDAO from superset.daos.report import ReportScheduleDAO from superset.reports.models import ( ReportCreationMethod, + ReportRecipientType, ReportSchedule, ReportScheduleType, ) @@ -54,6 +57,35 @@ class CreateReportScheduleCommand(CreateMixin, BaseReportScheduleCommand): self.validate() return ReportScheduleDAO.create(attributes=self._properties) + def _populate_recipients(self, exceptions: list[ValidationError]) -> None: + """ + Populate recipients based on creation method and current user. + + For reports initiated from charts or dashboards, always use + the current user's email as the recipient, ignoring any + client-provided recipient values. Raises validation error if + user has no email address. + """ + creation_method = self._properties.get("creation_method") + + # For reports from charts/dashboards, always use current user + if creation_method in ( + ReportCreationMethod.CHARTS, + ReportCreationMethod.DASHBOARDS, + ): + if hasattr(g, "user") and g.user and g.user.email: + # Override any provided recipients with current user's email + self._properties["recipients"] = [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": g.user.email}, + } + ] + else: + # User doesn't have an email address - can't create report + exceptions.append(ReportScheduleUserEmailNotFoundError()) + # For creation from alerts_reports view, keep the recipients as provided + def validate(self) -> None: """ Validates the properties of a report schedule configuration, including uniqueness @@ -75,6 +107,9 @@ class CreateReportScheduleCommand(CreateMixin, BaseReportScheduleCommand): exceptions: list[ValidationError] = [] + # Populate recipients if needed (may add validation errors) + self._populate_recipients(exceptions) + # Validate name type uniqueness if not ReportScheduleDAO.validate_update_uniqueness(name, report_type): exceptions.append( diff --git a/superset/commands/report/exceptions.py b/superset/commands/report/exceptions.py index 8688627810b..27966e6f09e 100644 --- a/superset/commands/report/exceptions.py +++ b/superset/commands/report/exceptions.py @@ -309,3 +309,18 @@ class ReportScheduleForbiddenError(ForbiddenError): class ReportSchedulePruneLogError(CommandException): message = _("An error occurred while pruning logs ") + + +class ReportScheduleUserEmailNotFoundError(ValidationError): + """ + Validation error when user email is required but not found + """ + + def __init__(self) -> None: + super().__init__( + _( + "Unable to create report: User email address is required but not " + "found. Please ensure your user profile has a valid email address." + ), + field_name="recipients", + ) diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index cfccc579bc0..76108f976ff 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -224,7 +224,7 @@ class ReportSchedulePostSchema(Schema): validate=[Range(min=1, error=_("Value must be greater than 0"))], ) - recipients = fields.List(fields.Nested(ReportRecipientSchema)) + recipients = fields.List(fields.Nested(ReportRecipientSchema), required=False) report_format = fields.String( dump_default=ReportDataFormat.PNG, validate=validate.OneOf(choices=tuple(key.value for key in ReportDataFormat)), diff --git a/tests/unit_tests/commands/report/test_create_recipients.py b/tests/unit_tests/commands/report/test_create_recipients.py new file mode 100644 index 00000000000..3956302e045 --- /dev/null +++ b/tests/unit_tests/commands/report/test_create_recipients.py @@ -0,0 +1,179 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from unittest.mock import MagicMock, patch + +from superset.commands.report.create import CreateReportScheduleCommand +from superset.commands.report.exceptions import ReportScheduleUserEmailNotFoundError +from superset.reports.models import ( + ReportCreationMethod, + ReportRecipientType, +) + + +def test_populate_recipients_chart_creation_with_user_email() -> None: + """Test that chart/dashboard creation methods use current user's email.""" + with patch("superset.commands.report.create.g") as mock_g: + # Setup user with email + mock_user = MagicMock() + mock_user.email = "user@example.com" + mock_g.user = mock_user + + # Create command instance + command = CreateReportScheduleCommand({}) + command._properties = { + "creation_method": ReportCreationMethod.CHARTS, + "recipients": [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "ignored@example.com"}, + } + ], + } + + exceptions: list[Exception] = [] + command._populate_recipients(exceptions) + + # Check that recipients were overridden + assert len(command._properties["recipients"]) == 1 + assert command._properties["recipients"][0]["type"] == ReportRecipientType.EMAIL + assert ( + command._properties["recipients"][0]["recipient_config_json"]["target"] + == "user@example.com" + ) + assert len(exceptions) == 0 + + +def test_populate_recipients_dashboard_creation_with_user_email() -> None: + """Test that dashboard creation uses current user's email.""" + with patch("superset.commands.report.create.g") as mock_g: + # Setup user with email + mock_user = MagicMock() + mock_user.email = "dashboard_user@example.com" + mock_g.user = mock_user + + # Create command instance + command = CreateReportScheduleCommand({}) + command._properties = { + "creation_method": ReportCreationMethod.DASHBOARDS, + # No recipients provided initially + } + + exceptions: list[Exception] = [] + command._populate_recipients(exceptions) + + # Check that recipients were set + assert len(command._properties["recipients"]) == 1 + assert command._properties["recipients"][0]["type"] == ReportRecipientType.EMAIL + assert ( + command._properties["recipients"][0]["recipient_config_json"]["target"] + == "dashboard_user@example.com" + ) + assert len(exceptions) == 0 + + +def test_populate_recipients_alerts_reports_keeps_original() -> None: + """Test that alerts_reports creation method preserves provided recipients.""" + command = CreateReportScheduleCommand({}) + original_recipients = [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "admin@example.com"}, + }, + { + "type": ReportRecipientType.SLACK, + "recipient_config_json": {"target": "#alerts"}, + }, + ] + command._properties = { + "creation_method": ReportCreationMethod.ALERTS_REPORTS, + "recipients": original_recipients, + } + + exceptions: list[Exception] = [] + command._populate_recipients(exceptions) + + # Check that recipients were NOT changed + assert command._properties["recipients"] == original_recipients + assert len(exceptions) == 0 + + +def test_populate_recipients_chart_creation_no_user_email() -> None: + """Test that chart creation fails when user has no email.""" + with patch("superset.commands.report.create.g") as mock_g: + # Setup user without email + mock_user = MagicMock() + mock_user.email = None + mock_g.user = mock_user + + command = CreateReportScheduleCommand({}) + command._properties = { + "creation_method": ReportCreationMethod.CHARTS, + } + + exceptions: list[Exception] = [] + command._populate_recipients(exceptions) + + # Check that validation error was added + assert len(exceptions) == 1 + assert isinstance(exceptions[0], ReportScheduleUserEmailNotFoundError) + # Recipients should not be set + assert ( + "recipients" not in command._properties + or command._properties["recipients"] == [] + ) + + +def test_populate_recipients_dashboard_creation_no_user() -> None: + """Test that dashboard creation fails when there's no user.""" + with patch("superset.commands.report.create.g") as mock_g: + # No user in context + mock_g.user = None + + command = CreateReportScheduleCommand({}) + command._properties = { + "creation_method": ReportCreationMethod.DASHBOARDS, + } + + exceptions: list[Exception] = [] + command._populate_recipients(exceptions) + + # Check that validation error was added + assert len(exceptions) == 1 + assert isinstance(exceptions[0], ReportScheduleUserEmailNotFoundError) + + +def test_populate_recipients_no_creation_method() -> None: + """Test that recipients are unchanged when no creation_method is specified.""" + command = CreateReportScheduleCommand({}) + original_recipients = [ + { + "type": ReportRecipientType.EMAIL, + "recipient_config_json": {"target": "user@example.com"}, + } + ] + command._properties = { + # No creation_method specified + "recipients": original_recipients, + } + + exceptions: list[Exception] = [] + command._populate_recipients(exceptions) + + # Check that recipients were NOT changed + assert command._properties["recipients"] == original_recipients + assert len(exceptions) == 0