codeant-ai-for-open-source[bot] commented on code in PR #41424:
URL: https://github.com/apache/superset/pull/41424#discussion_r3475046184
##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -246,6 +247,19 @@ def create_report_email_chart_with_csv():
cleanup_report_schedule(report_schedule)
[email protected]
+def create_report_email_chart_with_xlsx():
Review Comment:
**Suggestion:** Add an explicit return type annotation to this new fixture
function so newly added code remains fully typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is newly added Python code and the function has no return type
annotation. The custom rule requires new Python functions to be fully typed, so
this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=29c5c0a2b67449afa403bc9703e9cefc&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=29c5c0a2b67449afa403bc9703e9cefc&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/reports/commands_tests.py
**Line:** 251:251
**Comment:**
*Custom Rule: Add an explicit return type annotation to this new
fixture function so newly added code remains fully typed.
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%2F41424&comment_hash=6adb216285f06cee8db43fe55346a8fac65c3b9465700c8b8bb0a6fbef90827e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41424&comment_hash=6adb216285f06cee8db43fe55346a8fac65c3b9465700c8b8bb0a6fbef90827e&reaction=dislike'>👎</a>
##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -978,6 +1006,50 @@ def test_email_chart_report_schedule_with_csv(
assert_log(ReportState.SUCCESS)
[email protected](
+ "load_birth_names_dashboard_with_slices",
+ "create_report_email_chart_with_xlsx",
+)
+@patch("superset.utils.csv.urllib.request.urlopen")
+@patch("superset.utils.csv.urllib.request.OpenerDirector.open")
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch("superset.utils.csv.get_chart_csv_data")
+def test_email_chart_report_schedule_with_xlsx(
+ xlsx_mock,
+ email_mock,
+ mock_open,
+ mock_urlopen,
+ create_report_email_chart_with_xlsx,
+):
Review Comment:
**Suggestion:** Add type hints for all parameters and the return value in
this new test function to align with the fully typed new-code rule.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The new test function has no parameter annotations or return type
annotation. Under the custom rule, newly added Python functions in changed code
should be fully typed, so this is a verified violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8a3a7129151a44b8baf25931ad42c364&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=8a3a7129151a44b8baf25931ad42c364&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/reports/commands_tests.py
**Line:** 1017:1023
**Comment:**
*Custom Rule: Add type hints for all parameters and the return value in
this new test function to align with the fully typed new-code rule.
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%2F41424&comment_hash=b17ab872ad54b17d6af578154308ffb7c250ba8c130e22484ae90c87b62103c5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41424&comment_hash=b17ab872ad54b17d6af578154308ffb7c250ba8c130e22484ae90c87b62103c5&reaction=dislike'>👎</a>
##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -334,6 +348,20 @@ def create_report_slack_chart_with_csv():
cleanup_report_schedule(report_schedule)
[email protected]
+def create_report_slack_chart_with_xlsx():
+ chart = db.session.query(Slice).first()
+ chart.query_context = '{"mock": "query_context"}'
+ report_schedule = create_report_notification(
+ slack_channel="slack_channel",
+ chart=chart,
+ report_format=ReportDataFormat.XLSX,
+ )
+ yield report_schedule
+
+ cleanup_report_schedule(report_schedule)
Review Comment:
**Suggestion:** Add a concise docstring to this new fixture function so its
setup intent is documented inline. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced fixture function and it does not have a
docstring. That matches the custom rule requiring docstrings for newly added
Python functions and classes.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=92f7c3cbd53642caa8630cc6d6eb9efd&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=92f7c3cbd53642caa8630cc6d6eb9efd&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/reports/commands_tests.py
**Line:** 352:362
**Comment:**
*Custom Rule: Add a concise docstring to this new fixture function so
its setup intent is documented inline.
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%2F41424&comment_hash=613f5e7fe4480353ddd7584a82a7ee291ffa6e5c9ff7792c7b9910f1f805d49c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41424&comment_hash=613f5e7fe4480353ddd7584a82a7ee291ffa6e5c9ff7792c7b9910f1f805d49c&reaction=dislike'>👎</a>
##########
tests/unit_tests/reports/notifications/email_tests.py:
##########
@@ -144,3 +144,34 @@ def test_email_subject_with_datetime() -> None:
subject = notification._get_subject()
assert datetime_pattern not in subject
assert frozen_now.strftime(datetime_pattern) in subject
+
+
+def test_email_content_with_xlsx_attachment() -> None:
+ # `superset.models.helpers`, a dependency of following imports,
+ # requires app context
Review Comment:
**Suggestion:** Add a function docstring immediately under the test
definition to document the behavior being validated before the inline comments
and setup logic. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added test function and it does not include a docstring
immediately under the function definition. The custom rule requires newly added
Python functions and classes to be documented inline, so this is a real
violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bdc803ee11ff463e856cabfb9036bdcd&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=bdc803ee11ff463e856cabfb9036bdcd&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/reports/notifications/email_tests.py
**Line:** 149:151
**Comment:**
*Custom Rule: Add a function docstring immediately under the test
definition to document the behavior being validated before the inline comments
and setup logic.
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%2F41424&comment_hash=646160368fdc32e418b1798a9f700ee87e2e0b56686e7de55d176890a7c5fbf1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41424&comment_hash=646160368fdc32e418b1798a9f700ee87e2e0b56686e7de55d176890a7c5fbf1&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1208,10 +1208,24 @@ def test_get_notification_content_csv_format(mock_ff,
mocker: MockerFixture) ->
state = _make_notification_state(
mocker, report_format=ReportDataFormat.CSV, has_chart=True
)
- mocker.patch.object(state, "_get_csv_data", return_value=b"col1,col2\n1,2")
+ mocker.patch.object(state, "_get_data", return_value=b"col1,col2\n1,2")
content = state._get_notification_content()
assert content.csv == b"col1,col2\n1,2"
+ assert content.xlsx is None
+
+
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_xlsx_format(mock_ff, mocker: MockerFixture)
-> None:
Review Comment:
**Suggestion:** Add a type annotation for the untyped `mock_ff` parameter so
this newly added test function has complete parameter and return typing.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python test function, and the custom rule requires new
or modified Python functions to be fully typed. The `mock_ff` parameter has no
type annotation, so the suggestion correctly identifies a real rule violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=25d498cd5e6c42f58a1f4145a53eb3f7&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=25d498cd5e6c42f58a1f4145a53eb3f7&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:** 1219:1219
**Comment:**
*Custom Rule: Add a type annotation for the untyped `mock_ff` parameter
so this newly added test function has complete parameter and return typing.
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%2F41424&comment_hash=c4041717d0b375f4dcf0b18906a3cad10b6e97e18174a1d0c174350bc77e7e9a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41424&comment_hash=c4041717d0b375f4dcf0b18906a3cad10b6e97e18174a1d0c174350bc77e7e9a&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]
