Vitor-Avila commented on code in PR #44992:
URL: https://github.com/apache/superset/pull/44992#discussion_r4202645786
##########
superset/commands/report/update.py:
##########
@@ -42,17 +47,61 @@
from superset.exceptions import SupersetSecurityException
from superset.reports.models import (
ReportCreationMethod,
+ ReportDataFormat,
ReportRecipientType,
ReportSchedule,
ReportScheduleType,
ReportState,
)
+from superset.tasks.exceptions import ExecutorNotFoundError
+from superset.tasks.types import ExecutorType
+from superset.tasks.utils import get_executor
from superset.utils import json
-from superset.utils.core import get_user_email
+from superset.utils.core import get_user, get_user_email, get_user_id
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
+# Payload fields that change the delivered asset, its rendering, or its
recipients.
+# When a schedule executes as another user, only admins may change these.
+CONTENT_FIELDS: frozenset[str] = frozenset(
Review Comment:
Updated logic to cover this.
##########
superset/commands/report/update.py:
##########
@@ -42,17 +47,61 @@
from superset.exceptions import SupersetSecurityException
from superset.reports.models import (
ReportCreationMethod,
+ ReportDataFormat,
ReportRecipientType,
ReportSchedule,
ReportScheduleType,
ReportState,
)
+from superset.tasks.exceptions import ExecutorNotFoundError
+from superset.tasks.types import ExecutorType
+from superset.tasks.utils import get_executor
from superset.utils import json
-from superset.utils.core import get_user_email
+from superset.utils.core import get_user, get_user_email, get_user_id
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
+# Payload fields that change the delivered asset, its rendering, or its
recipients.
+# When a schedule executes as another user, only admins may change these.
+CONTENT_FIELDS: frozenset[str] = frozenset(
+ {
+ "chart",
+ "dashboard",
+ "extra",
+ "recipients",
+ "report_format",
+ }
+)
+
+
+def _normalize_json(value: Any) -> str:
+ """Canonical JSON string for a dict/JSON-string value (for comparisons)."""
+ if isinstance(value, str):
+ try:
+ value = json.loads(value)
+ except json.JSONDecodeError:
+ return value
+ if value is None:
+ value = {}
+ if not isinstance(value, (dict, list)):
+ return str(value)
+ return json.dumps(value, sort_keys=True)
+
+
+def _normalize_recipients(recipients: Any) -> list[tuple[str, str]]:
+ """Canonical, order-independent representation of a recipients list."""
+ normalized: list[tuple[str, str]] = []
+ for recipient in recipients or []:
+ if isinstance(recipient, dict):
+ recipient_type = recipient.get("type")
+ config = recipient.get("recipient_config_json")
+ else:
+ recipient_type = recipient.type
+ config = recipient.recipient_config_json
+ normalized.append((str(recipient_type), _normalize_json(config)))
Review Comment:
Great catch as well. Improved this flow.
--
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]
