From 28bf49e299b2be5ff2200ccd0972803da65ad89f Mon Sep 17 00:00:00 2001
From: Beto Dealmeida <roberto@dealmeida.net>
Date: Thu, 6 Jul 2023 19:48:33 -0700
Subject: [PATCH] fix(report): edit without custom width (#24612)

(cherry picked from commit 93ba4ade099263e926ffdb3d1a2b5ccea186b168)
---
 superset/reports/schemas.py              | 18 +++++-
 tests/unit_tests/reports/__init__.py     | 16 +++++
 tests/unit_tests/reports/schemas_test.py | 75 ++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 tests/unit_tests/reports/__init__.py
 create mode 100644 tests/unit_tests/reports/schemas_test.py

diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
index 7bdbf34f12730..fe7ae7d9a5d60 100644
--- a/superset/reports/schemas.py
+++ b/superset/reports/schemas.py
@@ -14,7 +14,7 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from typing import Any, Union
+from typing import Any, Optional, Union
 
 from croniter import croniter
 from flask import current_app
@@ -220,7 +220,13 @@ class ReportSchedulePostSchema(Schema):
     )
 
     @validates("custom_width")
-    def validate_custom_width(self, value: int) -> None:  # pylint: disable=no-self-use
+    def validate_custom_width(  # pylint: disable=no-self-use
+        self,
+        value: Optional[int],
+    ) -> None:
+        if value is None:
+            return
+
         min_width = current_app.config["ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH"]
         max_width = current_app.config["ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH"]
         if not min_width <= value <= max_width:
@@ -344,7 +350,13 @@ class ReportSchedulePutSchema(Schema):
     )
 
     @validates("custom_width")
-    def validate_custom_width(self, value: int) -> None:  # pylint: disable=no-self-use
+    def validate_custom_width(  # pylint: disable=no-self-use
+        self,
+        value: Optional[int],
+    ) -> None:
+        if value is None:
+            return
+
         min_width = current_app.config["ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH"]
         max_width = current_app.config["ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH"]
         if not min_width <= value <= max_width:
diff --git a/tests/unit_tests/reports/__init__.py b/tests/unit_tests/reports/__init__.py
new file mode 100644
index 0000000000000..13a83393a9124
--- /dev/null
+++ b/tests/unit_tests/reports/__init__.py
@@ -0,0 +1,16 @@
+# 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.
diff --git a/tests/unit_tests/reports/schemas_test.py b/tests/unit_tests/reports/schemas_test.py
new file mode 100644
index 0000000000000..0fab6d11b9a70
--- /dev/null
+++ b/tests/unit_tests/reports/schemas_test.py
@@ -0,0 +1,75 @@
+# 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.
+
+import pytest
+from marshmallow import ValidationError
+from pytest_mock import MockFixture
+
+from superset.reports.schemas import ReportSchedulePostSchema, ReportSchedulePutSchema
+
+
+def test_report_post_schema_custom_width_validation(mocker: MockFixture) -> None:
+    """
+    Test the custom width validation.
+    """
+    current_app = mocker.patch("superset.reports.schemas.current_app")
+    current_app.config = {
+        "ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH": 100,
+        "ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH": 200,
+    }
+
+    schema = ReportSchedulePostSchema()
+
+    schema.load(
+        {
+            "type": "Report",
+            "name": "A report",
+            "description": "My report",
+            "active": True,
+            "crontab": "* * * * *",
+            "timezone": "America/Los_Angeles",
+            "custom_width": 100,
+        }
+    )
+
+    # not required
+    schema.load(
+        {
+            "type": "Report",
+            "name": "A report",
+            "description": "My report",
+            "active": True,
+            "crontab": "* * * * *",
+            "timezone": "America/Los_Angeles",
+        }
+    )
+
+    with pytest.raises(ValidationError) as excinfo:
+        schema.load(
+            {
+                "type": "Report",
+                "name": "A report",
+                "description": "My report",
+                "active": True,
+                "crontab": "* * * * *",
+                "timezone": "America/Los_Angeles",
+                "custom_width": 1000,
+            }
+        )
+    assert excinfo.value.messages == {
+        "custom_width": ["Screenshot width must be between 100px and 200px"]
+    }