codeant-ai-for-open-source[bot] commented on code in PR #41133:
URL: https://github.com/apache/superset/pull/41133#discussion_r3593582214
##########
superset/dashboards/api.py:
##########
@@ -1574,6 +1594,132 @@ def export_as_example(self, pk: int) -> Response:
response.set_cookie(token, "done", max_age=600)
return response
+ @expose("/<pk>/export_xlsx/", methods=("POST",))
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.export_xlsx",
+ log_to_statsd=False,
+ )
+ def export_xlsx(self, pk: int) -> WerkzeugResponse:
+ """Export all of a dashboard's chart data to an Excel workbook (async).
+ ---
+ post:
+ summary: Export dashboard chart data to Excel
+ description: >-
+ Enqueues an async task that writes each chart's data to its own
+ worksheet, uploads the .xlsx to S3, and emails the requesting user
a
+ pre-signed download link. Returns immediately with a job id.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The dashboard id
+ requestBody:
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/DashboardExportXlsxPostSchema'
+ responses:
+ 202:
+ description: Export task accepted
+ content:
+ application/json:
+ schema:
+ $ref:
'#/components/schemas/DashboardExportXlsxResponseSchema'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 403:
+ $ref: '#/components/responses/403'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ 501:
+ description: Excel export is not configured on this server
+ """
+ if not current_app.config["EXCEL_EXPORT_S3_BUCKET"]:
+ return self.response(
+ 501, message="Excel export is not configured on this server."
+ )
+ try:
+ # Tolerate an empty/non-JSON body (e.g. a POST with no
Content-Type);
+ # request.json would otherwise raise 415.
+ payload = DashboardExportXlsxPostSchema().load(
+ request.get_json(silent=True) or {}
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for the schema-loaded
request payload variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The new local variable `payload` is assigned without any type annotation,
even though it is a clearly annotatable Python variable loaded from a schema.
This matches the rule requiring type hints on relevant variables in modified
Python code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=49c0d2db5f9745a48484c6cfc9701128&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=49c0d2db5f9745a48484c6cfc9701128&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/dashboards/api.py
**Line:** 1652:1654
**Comment:**
*Custom Rule: Add an explicit type annotation for the schema-loaded
request payload variable.
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%2F41133&comment_hash=c85669cdcc8ab1f4a5a9b9e26e92e765e2e5af95e17232dc90f6f6c215e79272&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=c85669cdcc8ab1f4a5a9b9e26e92e765e2e5af95e17232dc90f6f6c215e79272&reaction=dislike'>๐</a>
##########
superset/dashboards/api.py:
##########
@@ -1574,6 +1594,132 @@ def export_as_example(self, pk: int) -> Response:
response.set_cookie(token, "done", max_age=600)
return response
+ @expose("/<pk>/export_xlsx/", methods=("POST",))
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.export_xlsx",
+ log_to_statsd=False,
+ )
+ def export_xlsx(self, pk: int) -> WerkzeugResponse:
+ """Export all of a dashboard's chart data to an Excel workbook (async).
+ ---
+ post:
+ summary: Export dashboard chart data to Excel
+ description: >-
+ Enqueues an async task that writes each chart's data to its own
+ worksheet, uploads the .xlsx to S3, and emails the requesting user
a
+ pre-signed download link. Returns immediately with a job id.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The dashboard id
+ requestBody:
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/DashboardExportXlsxPostSchema'
+ responses:
+ 202:
+ description: Export task accepted
+ content:
+ application/json:
+ schema:
+ $ref:
'#/components/schemas/DashboardExportXlsxResponseSchema'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 403:
+ $ref: '#/components/responses/403'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ 501:
+ description: Excel export is not configured on this server
+ """
+ if not current_app.config["EXCEL_EXPORT_S3_BUCKET"]:
+ return self.response(
+ 501, message="Excel export is not configured on this server."
+ )
+ try:
+ # Tolerate an empty/non-JSON body (e.g. a POST with no
Content-Type);
+ # request.json would otherwise raise 415.
+ payload = DashboardExportXlsxPostSchema().load(
+ request.get_json(silent=True) or {}
+ )
+ except ValidationError as error:
+ return self.response_400(message=error.messages)
+
+ # Image export drives the headless webdriver, so it is only available
+ # when the same screenshot flags the UI checks are enabled. The
decorator
+ # form (``@validate_feature_flags``) can't be used here because it
would
+ # also block ``mode="data"``; mirror its 404 behavior inline instead.
+ if payload.get("mode") == "images" and not (
+ is_feature_enabled("ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS")
+ and
is_feature_enabled("ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT")
+ ):
+ return self.response_404()
+
+ dashboard = cast(Dashboard, self.datamodel.get(pk, self._base_filters))
+ if not dashboard:
+ return self.response_404()
+ try:
+ security_manager.raise_for_access(dashboard=dashboard)
+ except SupersetSecurityException:
+ return self.response_403()
+
+ # Email delivery is the only result channel, so an account with an
email
+ # address is required; embedded guest users are excluded in this
version.
+ if isinstance(g.user, GuestUser) or not getattr(g.user, "email", None):
+ return self.response_400(
+ message="Excel export requires an account with an email
address."
+ )
+ if not dashboard.slices:
+ return self.response_400(message="Dashboard has no charts to
export.")
+
+ # Throttle: one concurrent export per user+dashboard. Acquire a shared,
+ # atomic distributed lock (Redis when configured, the metadata DB
+ # otherwise) so the guard works across the web server and workers and
is
+ # not a no-op under the default cache. The task releases it when it
+ # settles; the TTL is the backstop if that release is ever lost.
+ lock_params = export_lock_params(g.user.id, dashboard.id)
Review Comment:
**Suggestion:** Add a concrete type annotation to the distributed lock
parameter variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The local variable `lock_params` is introduced without a type annotation,
despite being a new annotatable Python variable in modified code. This is a
real match for the type-hint requirement.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1f50fde3b9a24eb3aff566cb413ccc2f&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=1f50fde3b9a24eb3aff566cb413ccc2f&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/dashboards/api.py
**Line:** 1690:1690
**Comment:**
*Custom Rule: Add a concrete type annotation to the distributed lock
parameter variable.
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%2F41133&comment_hash=1b0c7b551484d676fdf3fa8725e821efc34864aa59a80b5e1aabe2837fabb079&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=1b0c7b551484d676fdf3fa8725e821efc34864aa59a80b5e1aabe2837fabb079&reaction=dislike'>๐</a>
##########
superset/dashboards/api.py:
##########
@@ -1574,6 +1594,132 @@ def export_as_example(self, pk: int) -> Response:
response.set_cookie(token, "done", max_age=600)
return response
+ @expose("/<pk>/export_xlsx/", methods=("POST",))
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.export_xlsx",
+ log_to_statsd=False,
+ )
+ def export_xlsx(self, pk: int) -> WerkzeugResponse:
+ """Export all of a dashboard's chart data to an Excel workbook (async).
+ ---
+ post:
+ summary: Export dashboard chart data to Excel
+ description: >-
+ Enqueues an async task that writes each chart's data to its own
+ worksheet, uploads the .xlsx to S3, and emails the requesting user
a
+ pre-signed download link. Returns immediately with a job id.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The dashboard id
+ requestBody:
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/DashboardExportXlsxPostSchema'
+ responses:
+ 202:
+ description: Export task accepted
+ content:
+ application/json:
+ schema:
+ $ref:
'#/components/schemas/DashboardExportXlsxResponseSchema'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 403:
+ $ref: '#/components/responses/403'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ 501:
+ description: Excel export is not configured on this server
+ """
+ if not current_app.config["EXCEL_EXPORT_S3_BUCKET"]:
+ return self.response(
+ 501, message="Excel export is not configured on this server."
+ )
+ try:
+ # Tolerate an empty/non-JSON body (e.g. a POST with no
Content-Type);
+ # request.json would otherwise raise 415.
+ payload = DashboardExportXlsxPostSchema().load(
+ request.get_json(silent=True) or {}
+ )
+ except ValidationError as error:
+ return self.response_400(message=error.messages)
+
+ # Image export drives the headless webdriver, so it is only available
+ # when the same screenshot flags the UI checks are enabled. The
decorator
+ # form (``@validate_feature_flags``) can't be used here because it
would
+ # also block ``mode="data"``; mirror its 404 behavior inline instead.
+ if payload.get("mode") == "images" and not (
+ is_feature_enabled("ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS")
+ and
is_feature_enabled("ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT")
+ ):
+ return self.response_404()
+
+ dashboard = cast(Dashboard, self.datamodel.get(pk, self._base_filters))
+ if not dashboard:
+ return self.response_404()
+ try:
+ security_manager.raise_for_access(dashboard=dashboard)
+ except SupersetSecurityException:
+ return self.response_403()
+
+ # Email delivery is the only result channel, so an account with an
email
+ # address is required; embedded guest users are excluded in this
version.
+ if isinstance(g.user, GuestUser) or not getattr(g.user, "email", None):
+ return self.response_400(
+ message="Excel export requires an account with an email
address."
+ )
+ if not dashboard.slices:
+ return self.response_400(message="Dashboard has no charts to
export.")
+
+ # Throttle: one concurrent export per user+dashboard. Acquire a shared,
+ # atomic distributed lock (Redis when configured, the metadata DB
+ # otherwise) so the guard works across the web server and workers and
is
+ # not a no-op under the default cache. The task releases it when it
+ # settles; the TTL is the backstop if that release is ever lost.
+ lock_params = export_lock_params(g.user.id, dashboard.id)
+ try:
+ AcquireDistributedLock(
+ EXPORT_LOCK_NAMESPACE,
+ lock_params,
+ ttl_seconds=EXPORT_LOCK_TTL_SECONDS,
+ ).run()
+ except LockAlreadyHeldException:
+ return self.response(
+ 202,
+ message="An Excel export for this dashboard is already in
progress.",
+ )
+
+ job_id = str(uuid.uuid4())
Review Comment:
**Suggestion:** Add an explicit type annotation to the generated export job
identifier variable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The variable `job_id` is newly introduced and unannotated, even though it is
a straightforward string-valued local that can be typed. This is a valid
omission under the type-hint rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cf294ce9f7cc4e398b85983789bee930&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=cf294ce9f7cc4e398b85983789bee930&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/dashboards/api.py
**Line:** 1703:1703
**Comment:**
*Custom Rule: Add an explicit type annotation to the generated export
job identifier variable.
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%2F41133&comment_hash=739e5a41e2eb87035a720c4e9044cb4ac27fb3473ac83c5b81655c05fe9170bc&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=739e5a41e2eb87035a720c4e9044cb4ac27fb3473ac83c5b81655c05fe9170bc&reaction=dislike'>๐</a>
##########
superset/utils/excel.py:
##########
@@ -41,18 +41,21 @@
"created": NEUTRAL_TIMESTAMP,
}
+# Leading characters that turn a cell into a formula in spreadsheet apps.
Shared
+# with the streaming writer (superset.utils.excel_streaming) so both export
paths
+# guard against the same formula-injection vectors.
+FORMULA_PREFIXES = {"=", "+", "-", "@"}
Review Comment:
**Suggestion:** Add an explicit type annotation to this new constant so it
complies with the required type-hinting rule for annotatable variables.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The new module-level constant is introduced without a type annotation even
though it can be typed as a set of strings. This violates the Python type-hint
requirement for annotatable variables.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4b994601bfac45e1bcd0de5ed349b87c&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=4b994601bfac45e1bcd0de5ed349b87c&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/excel.py
**Line:** 44:47
**Comment:**
*Custom Rule: Add an explicit type annotation to this new constant so
it complies with the required type-hinting rule for annotatable variables.
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%2F41133&comment_hash=5d6d52c6ccc5785bde39124ed88ef94b7e8d215d7065d35afe1fd428fa79b990&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=5d6d52c6ccc5785bde39124ed88ef94b7e8d215d7065d35afe1fd428fa79b990&reaction=dislike'>๐</a>
##########
tests/unit_tests/dashboards/test_excel_export_screenshot.py:
##########
@@ -0,0 +1,164 @@
+# 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 __future__ import annotations
+
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+import pytest
+from celery.exceptions import SoftTimeLimitExceeded
+
+from superset.charts.data.dashboard_filter_context import
DashboardFilterContext
+from superset.dashboards.excel_export import screenshot as screenshot_module
+from superset.dashboards.excel_export.screenshot import render_chart_image
+from superset.utils import json
+
+MODULE = "superset.dashboards.excel_export.screenshot"
+
+
+def _chart(
+ chart_id: int = 42,
+ params: str = '{"viz_type": "pie"}',
+ digest: str = "abc123",
+) -> MagicMock:
+ chart = MagicMock()
+ chart.id = chart_id
+ chart.params = params
+ chart.digest = digest
+ return chart
+
+
+def _patched_app() -> Any:
+ app = MagicMock()
+ app.config = {"WEBDRIVER_WINDOW": {"slice": (3000, 1200)}}
+ return app
+
+
+def test_render_chart_image_builds_url_with_slice_id_and_filters() -> None:
+ chart = _chart()
+ active_mask = {"NATIVE_FILTER-1": {"extraFormData": {"filters": [{"col":
"a"}]}}}
Review Comment:
**Suggestion:** Add a concrete type annotation for this structured test
variable so new code consistently provides hints for relevant variables.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This local variable is a newly added complex Python value that can be
annotated, but it is not typed. That is a real violation of the requirement to
add type hints to relevant variables in new or modified Python code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6f79e2837bf1479881b6eee9cdff58c4&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=6f79e2837bf1479881b6eee9cdff58c4&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/dashboards/test_excel_export_screenshot.py
**Line:** 53:53
**Comment:**
*Custom Rule: Add a concrete type annotation for this structured test
variable so new code consistently provides hints for relevant variables.
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%2F41133&comment_hash=3865e5bd7c6e4800439e4adcd2ac929a48f4db2e34e07d6867a090b66aef6940&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=3865e5bd7c6e4800439e4adcd2ac929a48f4db2e34e07d6867a090b66aef6940&reaction=dislike'>๐</a>
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -3262,6 +3264,173 @@ def
test_get_all_related_viewers_with_extra_filters(self):
response = json.loads(rv.data.decode("utf-8"))
assert response["count"] > 0
+ def test_export_xlsx_501_when_bucket_unset(self):
+ """Dashboard API: export_xlsx returns 501 when the S3 bucket is
unset."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-501", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 501
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_404_for_missing_dashboard(self, mock_task):
+ """Dashboard API: export_xlsx returns 404 for an unknown dashboard."""
+ self.login(ADMIN_USERNAME)
+ rv = self.client.post("api/v1/dashboard/99999999/export_xlsx/")
+ assert rv.status_code == 404
+ mock_task.apply_async.assert_not_called()
+
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_400_for_empty_dashboard(self, mock_task):
+ """Dashboard API: export_xlsx returns 400 for a dashboard with no
charts."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard("xlsx-empty", None, [admin.id])
+ self.login(ADMIN_USERNAME)
+ try:
+ rv =
self.client.post(f"api/v1/dashboard/{dashboard.id}/export_xlsx/")
+ assert rv.status_code == 400
+ mock_task.apply_async.assert_not_called()
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.AcquireDistributedLock")
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_202_enqueues_task(self, mock_task, mock_acquire):
+ """Dashboard API: export_xlsx enqueues the task and returns 202 +
job_id."""
+ self.login(ADMIN_USERNAME)
+ dashboard =
db.session.query(Dashboard).filter_by(slug="world_health").first()
+ rv = self.client.post(
+ f"api/v1/dashboard/{dashboard.id}/export_xlsx/",
+ json={"active_data_mask": {}},
+ )
+ assert rv.status_code == 202
+ body = json.loads(rv.data.decode("utf-8"))
+ job_id = body["job_id"]
+ assert job_id
+ # The in-flight lock is acquired before the task is enqueued.
+ mock_acquire.return_value.run.assert_called_once()
+ mock_task.apply_async.assert_called_once()
+ _, kwargs = mock_task.apply_async.call_args
+ assert kwargs["task_id"] == job_id
+ assert kwargs["kwargs"]["dashboard_id"] == dashboard.id
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ @with_config({"EXCEL_EXPORT_S3_BUCKET": "exports"})
+ @patch("superset.dashboards.api.AcquireDistributedLock")
+ @patch("superset.dashboards.api.export_dashboard_excel")
+ def test_export_xlsx_202_when_export_already_in_progress(
+ self, mock_task, mock_acquire
+ ):
+ """Dashboard API: export_xlsx does not enqueue a second concurrent
export."""
+ # An in-flight lock is already held for this user+dashboard.
+ mock_acquire.return_value.run.side_effect =
LockAlreadyHeldException("held")
+ self.login(ADMIN_USERNAME)
+ dashboard =
db.session.query(Dashboard).filter_by(slug="world_health").first()
+ rv = self.client.post(
+ f"api/v1/dashboard/{dashboard.id}/export_xlsx/",
+ json={"active_data_mask": {}},
+ )
+ assert rv.status_code == 202
+ assert "already in progress" in rv.data.decode("utf-8")
+ mock_task.apply_async.assert_not_called()
Review Comment:
**Suggestion:** Annotate each parameter and the return type in this
multiline method definition to satisfy typing requirements. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This multiline newly added function signature also omits type hints for its
parameters and return type, so the custom typing rule applies.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6706e046b6a34d5a8763e48e4f18963f&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=6706e046b6a34d5a8763e48e4f18963f&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/integration_tests/dashboards/api_tests.py
**Line:** 3330:3344
**Comment:**
*Custom Rule: Annotate each parameter and the return type in this
multiline method definition to satisfy typing requirements.
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%2F41133&comment_hash=5ac08b1a0a73808e2d9c153a53ed5c37a690c5804e23474e0d00ce9ab8691b5e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=5ac08b1a0a73808e2d9c153a53ed5c37a690c5804e23474e0d00ce9ab8691b5e&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]
