codeant-ai-for-open-source[bot] commented on code in PR #40885:
URL: https://github.com/apache/superset/pull/40885#discussion_r3601290705
##########
superset/utils/core.py:
##########
@@ -120,6 +120,27 @@
logging.getLogger("MARKDOWN").setLevel(logging.INFO)
logger = logging.getLogger(__name__)
+EMAIL_ATTACHMENT_SUBTYPES: dict[str, str] = {
+ ".pdf": "pdf",
+ ".zip": "zip",
+ ".xlsx": "vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+}
+
+
+def build_email_attachment(name: str, body: bytes | str) -> MIMEApplication:
+ """
+ Create an email attachment part with stable filename metadata.
+ """
+ subtype = EMAIL_ATTACHMENT_SUBTYPES.get(os.path.splitext(name)[1].lower())
+ attachment = MIMEApplication(
+ body,
+ _subtype=subtype or "octet-stream",
+ Name=name,
+ )
Review Comment:
**Suggestion:** `MIMEApplication` expects binary payloads, but
`build_email_attachment` now accepts `str` and forwards it directly as `body`.
For non-ASCII text attachments this can produce incorrect encoding (or encoding
errors) and corrupt delivered files. Normalize `str` bodies to UTF-8 bytes (or
route textual payloads through `MIMEText`) before creating the attachment so
attachment bytes are stable across character sets. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Email attachments with non-ASCII text may be corrupted.
- ⚠️ Text-attachment reports via send_email_smtp become unreliable.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start a Python shell inside the Superset environment so you can call the
existing mail
helper `send_email_smtp` defined in `superset/utils/core.py:9-23` (see lines
848-868 in
the file for its signature and documentation).
2. Import the helper and config: `from superset.utils.core import
send_email_smtp` and
obtain a running Flask app context exposing `current_app.config` (the same
config used in
`superset/reports/notifications/email.py:117-121` when sending report
emails).
3. Invoke `send_email_smtp` with a textual attachment value instead of
bytes, for example:
`send_email_smtp("[email protected]", "subject", "content",
current_app.config,
data={"report.txt": "café"})`, which flows into the `for name, body in (data
or
{}).items()` loop at `superset/utils/core.py:65-66`.
4. Observe that `build_email_attachment` at `superset/utils/core.py:130-139`
receives
`body="café"` (a `str`), passes it directly to `MIMEApplication` as the
payload, and the
stdlib’s `MIMEApplication` base64 encoder treats this as binary data; for
non-ASCII text
this either raises a `UnicodeEncodeError` during ASCII encoding or produces
incorrectly
encoded bytes, resulting in a failed send or a corrupted `report.txt`
attachment when the
email is delivered.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=311ac4ae032d4acb84200dd7af5bcfd3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=311ac4ae032d4acb84200dd7af5bcfd3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/core.py
**Line:** 130:139
**Comment:**
*Api Mismatch: `MIMEApplication` expects binary payloads, but
`build_email_attachment` now accepts `str` and forwards it directly as `body`.
For non-ASCII text attachments this can produce incorrect encoding (or encoding
errors) and corrupt delivered files. Normalize `str` bodies to UTF-8 bytes (or
route textual payloads through `MIMEText`) before creating the attachment so
attachment bytes are stable across character sets.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=0b225068a73642f0fe39c6a389d3c09b219398abe37529ca4d4617201fde1923&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=0b225068a73642f0fe39c6a389d3c09b219398abe37529ca4d4617201fde1923&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def
test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) ->
None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
Review Comment:
**Suggestion:** This test calls a method that does not exist on
`BaseReportState`, so it will fail with `AttributeError` before the intended
assertion is evaluated. Update the test to call the current API
(`_get_data(...)`) or reintroduce the removed method if that was intended. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ JSON-format rejection test crashes with AttributeError.
- ⚠️ Unsupported chart-data format handling remains untested.
- ⚠️ Failing unit test can block CI runs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/commands/report/execute_test.py and locate function
test_get_chart_data_rejects_non_table_format where line 1368 calls
report_state._get_chart_data(ChartDataResultFormat.JSON) inside a
pytest.raises(ReportScheduleExecuteUnexpectedError, ...) context.
2. Open superset/commands/report/execute.py and inspect BaseReportState
(lines 126-792),
confirming it defines _get_data and _get_chart_data_request_payload but no
_get_chart_data
method (verified by grep for "def _get_chart_data").
3. From the repository root, run pytest
tests/unit_tests/commands/report/execute_test.py::test_get_chart_data_rejects_non_table_format
to execute this specific test.
4. During test execution, when Python evaluates
report_state._get_chart_data(ChartDataResultFormat.JSON) at
tests/unit_tests/commands/report/execute_test.py:1368 on a BaseReportState
instance, it
raises AttributeError: 'BaseReportState' object has no attribute
'_get_chart_data',
causing the test to fail before any ReportScheduleExecuteUnexpectedError can
be raised for
the unsupported JSON format.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b3152c66d8294e63a7936217b5e60249&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b3152c66d8294e63a7936217b5e60249&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/commands/report/execute_test.py
**Line:** 1368:1368
**Comment:**
*Api Mismatch: This test calls a method that does not exist on
`BaseReportState`, so it will fail with `AttributeError` before the intended
assertion is evaluated. Update the test to call the current API
(`_get_data(...)`) or reintroduce the removed method if that was intended.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=008156b28aeab4e1c21851e448ebb5f401c62cb8881dcef0d52ef8cd57ba1f72&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=008156b28aeab4e1c21851e448ebb5f401c62cb8881dcef0d52ef8cd57ba1f72&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def
test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) ->
None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
+
+ get_url.assert_not_called()
+
+
+def _mock_xlsx_chart_data_dependencies(
+ mocker: MockerFixture,
+ report_state: BaseReportState,
+) -> tuple[MagicMock, dict[str, str]]:
+ """Mock external services used by the chart data download path."""
+ report_state._report_schedule.chart.query_context = None
+ mocker.patch.object(report_state, "_update_query_context")
+ mocker.patch("superset.commands.report.execute.db.session.refresh")
+ get_url = mocker.patch.object(
+ report_state,
+ "_get_url",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_executor",
+ return_value=(None, "report_executor"),
+ )
+ user = mocker.MagicMock(username="report_executor")
+ mocker.patch(
+ "superset.commands.report.execute.security_manager.find_user",
+ return_value=user,
+ )
+ auth_cookies = {"session": "cookie"}
+ auth_provider = mocker.patch(
+ "superset.commands.report.execute.machine_auth_provider_factory"
+ )
+ auth_provider.instance.get_auth_cookies.return_value = auth_cookies
+ return get_url, auth_cookies
+
+
+def test_get_xlsx_data_fetches_chart_data(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """XLSX report data should be fetched through the chart data endpoint."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url, auth_cookies = _mock_xlsx_chart_data_dependencies(mocker,
report_state)
+ get_chart_csv_data = mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=b"xlsx-data",
+ )
+
+ assert report_state._get_xlsx_data() == b"xlsx-data"
+ get_url.assert_called_once_with(result_format=ChartDataResultFormat.XLSX)
+ get_chart_csv_data.assert_called_once_with(
+ chart_url="/api/v1/chart/1/data/xlsx",
+ auth_cookies=auth_cookies,
+ timeout=app.config["ALERT_REPORTS_CSV_REQUEST_TIMEOUT"],
+ )
+
+
[email protected](
+ ("side_effect", "expected_exception", "expected_message"),
+ [
+ (
+ SoftTimeLimitExceeded(),
+ ReportScheduleXlsxTimeout,
+ "timeout occurred while generating an xlsx",
+ ),
+ (
+ RuntimeError("export failed"),
+ ReportScheduleXlsxFailedError,
+ "Failed generating xlsx export failed",
+ ),
+ ],
+)
+def test_get_xlsx_data_maps_errors(
+ app: SupersetApp,
+ mocker: MockerFixture,
+ side_effect: Exception,
+ expected_exception: type[Exception],
+ expected_message: str,
+) -> None:
+ """XLSX generation errors should use XLSX-specific report exceptions."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ side_effect=side_effect,
+ )
+
+ with pytest.raises(expected_exception, match=expected_message) as exc_info:
+ report_state._get_xlsx_data()
+
+ assert exc_info.value.__cause__ is side_effect
+
+
+def test_get_xlsx_data_rejects_empty_result(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """An empty XLSX response should fail report generation."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ _mock_xlsx_chart_data_dependencies(mocker, report_state)
+ mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=None,
+ )
+
+ with pytest.raises(
+ ReportScheduleXlsxFailedError,
+ match="Report Schedule execution failed when generating an xlsx",
+ ) as exc_info:
+ report_state._get_xlsx_data()
+
+ assert exc_info.value.__cause__ is None
+
+
+def test_notification_content_contains_xlsx(mocker: MockerFixture) -> None:
+ """XLSX chart reports should populate the XLSX notification field."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.report_format = ReportDataFormat.XLSX
+ report_schedule.force_screenshot = False
+ report_schedule.email_subject = None
+ report_schedule.owners = []
+ report_schedule.recipients = []
+ report_state = BaseReportState(
+ report_schedule,
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ mocker.patch.object(report_state, "_get_url", return_value="/chart/1")
+ mocker.patch.object(report_state, "_get_log_data", return_value={})
+ get_xlsx_data = mocker.patch.object(
+ report_state,
+ "_get_xlsx_data",
+ return_value=b"xlsx-data",
+ )
Review Comment:
**Suggestion:** This patch target is invalid because `_get_xlsx_data` is not
defined on `BaseReportState`, so `patch.object` will raise immediately. Patch
`_get_data` with the XLSX result format path instead, and assert against the
actual method used by `_get_notification_content`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ XLSX notification-content test fails during setup.
- ⚠️ NotificationContent.xlsx population remains unvalidated.
- ⚠️ Test suite stability degraded by setup error.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/commands/report/execute_test.py and locate
test_notification_content_contains_xlsx around lines 214-239; within this
test, lines
1508-1512 define get_xlsx_data = mocker.patch.object(report_state,
"_get_xlsx_data",
return_value=b"xlsx-data").
2. Open superset/commands/report/execute.py and inspect BaseReportState
(lines 126-792),
confirming that it implements _get_data for CSV/XLSX but does not define a
_get_xlsx_data
method (grep for "_get_xlsx_data" shows only references in this test file).
3. Recall that unittest.mock.patch.object by default uses create=False,
meaning it first
checks that the target object has the named attribute and raises
AttributeError if it does
not.
4. From the repository root, run pytest
tests/unit_tests/commands/report/execute_test.py::test_notification_content_contains_xlsx;
when pytest executes the test, patch.object(report_state, "_get_xlsx_data",
...) at line
1508 raises AttributeError because BaseReportState lacks _get_xlsx_data, so
the test fails
during setup and never reaches state._get_notification_content() or asserts
that
NotificationContent.xlsx is populated.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c6b54c91251c4ddb8cf3807273c0755b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c6b54c91251c4ddb8cf3807273c0755b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/commands/report/execute_test.py
**Line:** 1508:1512
**Comment:**
*Api Mismatch: This patch target is invalid because `_get_xlsx_data` is
not defined on `BaseReportState`, so `patch.object` will raise immediately.
Patch `_get_data` with the XLSX result format path instead, and assert against
the actual method used by `_get_notification_content`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=f63aebb231bbf4832f940fb7ce11766773f37d72ac365c085dc9002bbf1f9479&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=f63aebb231bbf4832f940fb7ce11766773f37d72ac365c085dc9002bbf1f9479&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1326,6 +1327,197 @@ def
test_get_csv_data_keeps_screenshot_fallback_without_query_context(
post_chart_data.assert_not_called()
+def test_get_url_for_xlsx_report(mocker: MockerFixture) -> None:
+ """XLSX reports should request post-processed chart data."""
+ report_schedule = create_report_schedule(mocker)
+ report_schedule.chart_id = 1
+ report_schedule.force_screenshot = False
+ report_state = BaseReportState(
+ report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ get_url_path = mocker.patch(
+ "superset.commands.report.execute.get_url_path",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+
+ url = report_state._get_url(result_format=ChartDataResultFormat.XLSX)
+
+ assert url == "/api/v1/chart/1/data/xlsx"
+ get_url_path.assert_called_once_with(
+ "ChartDataRestApi.get_data",
+ pk=1,
+ format=ChartDataResultFormat.XLSX.value,
+ type=ChartDataResultType.POST_PROCESSED.value,
+ force="false",
+ )
+
+
+def test_get_chart_data_rejects_non_table_format(mocker: MockerFixture) ->
None:
+ """Chart data retrieval should reject formats it cannot download."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url = mocker.patch.object(report_state, "_get_url")
+
+ with pytest.raises(
+ ReportScheduleExecuteUnexpectedError,
+ match="Unsupported chart data result format: json",
+ ):
+ report_state._get_chart_data(ChartDataResultFormat.JSON)
+
+ get_url.assert_not_called()
+
+
+def _mock_xlsx_chart_data_dependencies(
+ mocker: MockerFixture,
+ report_state: BaseReportState,
+) -> tuple[MagicMock, dict[str, str]]:
+ """Mock external services used by the chart data download path."""
+ report_state._report_schedule.chart.query_context = None
+ mocker.patch.object(report_state, "_update_query_context")
+ mocker.patch("superset.commands.report.execute.db.session.refresh")
+ get_url = mocker.patch.object(
+ report_state,
+ "_get_url",
+ return_value="/api/v1/chart/1/data/xlsx",
+ )
+ mocker.patch(
+ "superset.commands.report.execute.get_executor",
+ return_value=(None, "report_executor"),
+ )
+ user = mocker.MagicMock(username="report_executor")
+ mocker.patch(
+ "superset.commands.report.execute.security_manager.find_user",
+ return_value=user,
+ )
+ auth_cookies = {"session": "cookie"}
+ auth_provider = mocker.patch(
+ "superset.commands.report.execute.machine_auth_provider_factory"
+ )
+ auth_provider.instance.get_auth_cookies.return_value = auth_cookies
+ return get_url, auth_cookies
+
+
+def test_get_xlsx_data_fetches_chart_data(
+ app: SupersetApp,
+ mocker: MockerFixture,
+) -> None:
+ """XLSX report data should be fetched through the chart data endpoint."""
+ report_state = BaseReportState(
+ create_report_schedule(mocker),
+ "January 1, 2021",
+ "execution_id_example",
+ )
+ get_url, auth_cookies = _mock_xlsx_chart_data_dependencies(mocker,
report_state)
+ get_chart_csv_data = mocker.patch(
+ "superset.commands.report.execute.get_chart_csv_data",
+ return_value=b"xlsx-data",
+ )
+
+ assert report_state._get_xlsx_data() == b"xlsx-data"
Review Comment:
**Suggestion:** These new XLSX tests invoke `_get_xlsx_data()`, but the
implementation now routes XLSX through `_get_data(ChartDataResultFormat.XLSX)`.
As written, the tests will fail with `AttributeError` instead of validating
XLSX behavior; switch these calls to the current method contract. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ XLSX fetch tests crash before exercising export path.
- ⚠️ XLSX timeout and error mapping unverified.
- ⚠️ CI runs fail when XLSX tests are executed.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/commands/report/execute_test.py and locate
test_get_xlsx_data_fetches_chart_data at lines 124-147, where line 1419
asserts
report_state._get_xlsx_data() == b"xlsx-data"; similar calls appear in
test_get_xlsx_data_maps_errors (line 1463) and
test_get_xlsx_data_rejects_empty_result
(line 1488).
2. Open superset/commands/report/execute.py and confirm that BaseReportState
implements
_get_data(result_format: ChartDataResultFormat) at lines 713-792, which
handles both CSV
and XLSX, but there is no _get_xlsx_data method anywhere in the module
(verified by grep
for "_get_xlsx_data").
3. From the repository root, run pytest
tests/unit_tests/commands/report/execute_test.py::test_get_xlsx_data_fetches_chart_data
to
execute the first XLSX test.
4. When the test reaches line 1419 and evaluates
report_state._get_xlsx_data() on the
BaseReportState instance, Python raises AttributeError: 'BaseReportState'
object has no
attribute '_get_xlsx_data', so the test never exercises the
XLSX-through-_get_data path
and fails before validating that XLSX bytes are fetched via the chart data
endpoint.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5347de9eca6a408290e6b6a572ee5484&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5347de9eca6a408290e6b6a572ee5484&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/commands/report/execute_test.py
**Line:** 1419:1419
**Comment:**
*Api Mismatch: These new XLSX tests invoke `_get_xlsx_data()`, but the
implementation now routes XLSX through `_get_data(ChartDataResultFormat.XLSX)`.
As written, the tests will fail with `AttributeError` instead of validating
XLSX behavior; switch these calls to the current method contract.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=019391d04815982664352c24df06ed35600b60d5519eeb2516f1bf0f1be852e5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40885&comment_hash=019391d04815982664352c24df06ed35600b60d5519eeb2516f1bf0f1be852e5&reaction=dislike'>👎</a>
--
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]
