codeant-ai-for-open-source[bot] commented on code in PR #37523:
URL: https://github.com/apache/superset/pull/37523#discussion_r3510843120
##########
tests/unit_tests/views/test_base_theme_helpers.py:
##########
@@ -892,6 +893,137 @@ def test_brandappname_handles_missing_common_in_payload(
assert result["default_title"] == "Superset"
+class TestBrandSpinnerUrlPrefix:
+ """Test brandSpinnerUrl static asset prefix handling."""
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_adds_static_assets_prefix(self, mock_app,
mock_payload):
Review Comment:
**Suggestion:** Add type annotations to this new test method signature,
including parameter types and an explicit <code>-> None</code> return type.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is newly added Python code and the method signature omits parameter
type annotations and an explicit return type, which matches the type-hint
requirement rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=68025ee358e54660a0c2c0e7ea92f5d2&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=68025ee358e54660a0c2c0e7ea92f5d2&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/views/test_base_theme_helpers.py
**Line:** 901:901
**Comment:**
*Custom Rule: Add type annotations to this new test method signature,
including parameter types and an explicit <code>-> None</code> return type.
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%2F37523&comment_hash=68dd96c8c802d0c540149013e839a19be6f3eae1d7365ac1f0a965f36639fb31&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37523&comment_hash=68dd96c8c802d0c540149013e839a19be6f3eae1d7365ac1f0a965f36639fb31&reaction=dislike'>👎</a>
##########
tests/unit_tests/views/test_base_theme_helpers.py:
##########
@@ -892,6 +893,137 @@ def test_brandappname_handles_missing_common_in_payload(
assert result["default_title"] == "Superset"
+class TestBrandSpinnerUrlPrefix:
+ """Test brandSpinnerUrl static asset prefix handling."""
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_adds_static_assets_prefix(self, mock_app,
mock_payload):
+ """Test that root-relative spinner URLs include the static assets
prefix."""
+ from superset.views.base import get_spa_template_context
+
+ mock_app.config = {
+ "STATIC_ASSETS_PREFIX": "/analytics",
+ }
+ mock_payload.return_value = {
+ "common": {
+ "theme": {
+ "default": {
+ "token": {
+ "brandSpinnerUrl": "/static/assets/spinner.gif",
+ }
+ },
+ "dark": {
+ "token": {
+ "brandSpinnerUrl":
"/static/assets/dark-spinner.gif",
+ }
+ },
+ }
+ }
+ }
+
+ result = get_spa_template_context("app")
+
+ assert (
+ result["theme_tokens"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/spinner.gif"
+ )
+ bootstrap_data = json.loads(result["bootstrap_data"])
+ assert (
+
bootstrap_data["common"]["theme"]["default"]["token"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/spinner.gif"
+ )
+ assert (
+
bootstrap_data["common"]["theme"]["dark"]["token"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/dark-spinner.gif"
+ )
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_keeps_absolute_url(self, mock_app, mock_payload):
Review Comment:
**Suggestion:** Add type hints for all parameters and the return type on
this newly added method. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added test method has no type annotations on its parameters or
return type, so it violates the Python type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d17bf87db9f94ef39d3095a86d766f32&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=d17bf87db9f94ef39d3095a86d766f32&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/views/test_base_theme_helpers.py
**Line:** 943:943
**Comment:**
*Custom Rule: Add type hints for all parameters and the return type on
this newly added method.
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%2F37523&comment_hash=d7b8dda200e6f4dcaa5460d5807f7aec8ed0ad201e1f8b1ff4433ddc7c316c5c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37523&comment_hash=d7b8dda200e6f4dcaa5460d5807f7aec8ed0ad201e1f8b1ff4433ddc7c316c5c&reaction=dislike'>👎</a>
##########
tests/unit_tests/views/test_base_theme_helpers.py:
##########
@@ -892,6 +893,137 @@ def test_brandappname_handles_missing_common_in_payload(
assert result["default_title"] == "Superset"
+class TestBrandSpinnerUrlPrefix:
+ """Test brandSpinnerUrl static asset prefix handling."""
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_adds_static_assets_prefix(self, mock_app,
mock_payload):
+ """Test that root-relative spinner URLs include the static assets
prefix."""
+ from superset.views.base import get_spa_template_context
+
+ mock_app.config = {
+ "STATIC_ASSETS_PREFIX": "/analytics",
+ }
+ mock_payload.return_value = {
+ "common": {
+ "theme": {
+ "default": {
+ "token": {
+ "brandSpinnerUrl": "/static/assets/spinner.gif",
+ }
+ },
+ "dark": {
+ "token": {
+ "brandSpinnerUrl":
"/static/assets/dark-spinner.gif",
+ }
+ },
+ }
+ }
+ }
+
+ result = get_spa_template_context("app")
+
+ assert (
+ result["theme_tokens"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/spinner.gif"
+ )
+ bootstrap_data = json.loads(result["bootstrap_data"])
+ assert (
+
bootstrap_data["common"]["theme"]["default"]["token"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/spinner.gif"
+ )
+ assert (
+
bootstrap_data["common"]["theme"]["dark"]["token"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/dark-spinner.gif"
+ )
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_keeps_absolute_url(self, mock_app, mock_payload):
+ """Test that absolute spinner URLs are not prefixed."""
+ from superset.views.base import get_spa_template_context
+
+ mock_app.config = {
+ "STATIC_ASSETS_PREFIX": "/analytics",
+ }
+ mock_payload.return_value = {
+ "common": {
+ "theme": {
+ "default": {
+ "token": {
+ "brandSpinnerUrl":
"https://cdn.example.com/spinner.gif";,
+ }
+ }
+ }
+ }
+ }
+
+ result = get_spa_template_context("app")
+
+ assert (
+ result["theme_tokens"]["brandSpinnerUrl"]
+ == "https://cdn.example.com/spinner.gif";
+ )
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_keeps_protocol_relative_url(self, mock_app,
mock_payload):
Review Comment:
**Suggestion:** Annotate this method with appropriate parameter types and an
explicit <code>-> None</code> return annotation. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is new Python code and the method signature is untyped, so it falls
under the rule requiring type hints for functions and methods.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1289e9d53b0a495d8bd9f0c3f0887ebe&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=1289e9d53b0a495d8bd9f0c3f0887ebe&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/views/test_base_theme_helpers.py
**Line:** 971:971
**Comment:**
*Custom Rule: Annotate this method with appropriate parameter types and
an explicit <code>-> None</code> return annotation.
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%2F37523&comment_hash=b7017314ccf6ce3496b6e11b80862649d4decfe3f6de65f2f2f4a43b34002ecb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37523&comment_hash=b7017314ccf6ce3496b6e11b80862649d4decfe3f6de65f2f2f4a43b34002ecb&reaction=dislike'>👎</a>
##########
tests/unit_tests/views/test_base_theme_helpers.py:
##########
@@ -892,6 +893,137 @@ def test_brandappname_handles_missing_common_in_payload(
assert result["default_title"] == "Superset"
+class TestBrandSpinnerUrlPrefix:
+ """Test brandSpinnerUrl static asset prefix handling."""
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_adds_static_assets_prefix(self, mock_app,
mock_payload):
+ """Test that root-relative spinner URLs include the static assets
prefix."""
+ from superset.views.base import get_spa_template_context
+
+ mock_app.config = {
+ "STATIC_ASSETS_PREFIX": "/analytics",
+ }
+ mock_payload.return_value = {
+ "common": {
+ "theme": {
+ "default": {
+ "token": {
+ "brandSpinnerUrl": "/static/assets/spinner.gif",
+ }
+ },
+ "dark": {
+ "token": {
+ "brandSpinnerUrl":
"/static/assets/dark-spinner.gif",
+ }
+ },
+ }
+ }
+ }
+
+ result = get_spa_template_context("app")
+
+ assert (
+ result["theme_tokens"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/spinner.gif"
+ )
+ bootstrap_data = json.loads(result["bootstrap_data"])
+ assert (
+
bootstrap_data["common"]["theme"]["default"]["token"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/spinner.gif"
+ )
+ assert (
+
bootstrap_data["common"]["theme"]["dark"]["token"]["brandSpinnerUrl"]
+ == "/analytics/static/assets/dark-spinner.gif"
+ )
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_keeps_absolute_url(self, mock_app, mock_payload):
+ """Test that absolute spinner URLs are not prefixed."""
+ from superset.views.base import get_spa_template_context
+
+ mock_app.config = {
+ "STATIC_ASSETS_PREFIX": "/analytics",
+ }
+ mock_payload.return_value = {
+ "common": {
+ "theme": {
+ "default": {
+ "token": {
+ "brandSpinnerUrl":
"https://cdn.example.com/spinner.gif";,
+ }
+ }
+ }
+ }
+ }
+
+ result = get_spa_template_context("app")
+
+ assert (
+ result["theme_tokens"]["brandSpinnerUrl"]
+ == "https://cdn.example.com/spinner.gif";
+ )
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_keeps_protocol_relative_url(self, mock_app,
mock_payload):
+ """Test that protocol-relative spinner URLs are not prefixed."""
+ from superset.views.base import get_spa_template_context
+
+ mock_app.config = {
+ "STATIC_ASSETS_PREFIX": "/analytics",
+ }
+ mock_payload.return_value = {
+ "common": {
+ "theme": {
+ "default": {
+ "token": {
+ "brandSpinnerUrl": "//cdn.example.com/spinner.gif",
+ }
+ }
+ }
+ }
+ }
+
+ result = get_spa_template_context("app")
+
+ assert (
+ result["theme_tokens"]["brandSpinnerUrl"] ==
"//cdn.example.com/spinner.gif"
+ )
+
+ @patch("superset.views.base.get_spa_payload")
+ @patch("superset.views.base.app")
+ def test_brandspinnerurl_does_not_duplicate_static_assets_prefix(
+ self, mock_app, mock_payload
+ ):
Review Comment:
**Suggestion:** Add type annotations to this multi-line method definition
for each argument and the return type. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added multi-line method definition still omits type annotations for its
parameters and return type, so the violation is real.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=936e7ad625c648629e7fcdff92cbe514&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=936e7ad625c648629e7fcdff92cbe514&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/views/test_base_theme_helpers.py
**Line:** 998:1000
**Comment:**
*Custom Rule: Add type annotations to this multi-line method definition
for each argument and the return type.
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%2F37523&comment_hash=97124ee009b02f0f8ded5fe9e8d50e413e0298bca31fde60b434abd47afb918e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37523&comment_hash=97124ee009b02f0f8ded5fe9e8d50e413e0298bca31fde60b434abd47afb918e&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]
![https://cdn.example.com/spinner.gif"](https://cdn.example.com/spinner.gif"){kind=link}