codeant-ai-for-open-source[bot] commented on code in PR #37932:
URL: https://github.com/apache/superset/pull/37932#discussion_r3544341247
##########
tests/unit_tests/commands/dataset/test_duplicate.py:
##########
@@ -165,3 +170,360 @@ def
test_duplicate_dataset_blocked_by_soft_deleted_twin(session: Session) -> Non
)
with pytest.raises(DatasetInvalidError):
command.validate()
+
+
+def test_duplicate_dataset_success():
Review Comment:
**Suggestion:** Add an explicit return type annotation to this test function
signature (for example, `-> None`). [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The function signature in the final file has no return type annotation. The
custom rule requires new or modified Python functions to include type hints, so
this is a real violation.
</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=197e45d7df7c4ddb9ba15f7f4f0cf0eb&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=197e45d7df7c4ddb9ba15f7f4f0cf0eb&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/dataset/test_duplicate.py
**Line:** 175:175
**Comment:**
*Custom Rule: Add an explicit return type annotation to this test
function signature (for example, `-> None`).
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%2F37932&comment_hash=e3e43473f32cb0c2a47a6da2c886d501afe19e7c3e933a3ee11b83a518bc619a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37932&comment_hash=e3e43473f32cb0c2a47a6da2c886d501afe19e7c3e933a3ee11b83a518bc619a&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dataset/test_duplicate.py:
##########
@@ -165,3 +170,360 @@ def
test_duplicate_dataset_blocked_by_soft_deleted_twin(session: Session) -> Non
)
with pytest.raises(DatasetInvalidError):
command.validate()
+
+
+def test_duplicate_dataset_success():
+ """Test successful duplication of a virtual dataset."""
+ # Create a mock base model (virtual dataset)
+ mock_base_model = Mock(spec=SqlaTable)
+ mock_base_model.id = 1
+ mock_base_model.database_id = 1
+ mock_base_model.table_name = "original_dataset"
+ mock_base_model.schema = "public"
+ mock_base_model.catalog = None
+ mock_base_model.kind = "virtual"
+ mock_base_model.sql = "SELECT 1 as c"
+ mock_base_model.template_params = None
+ mock_base_model.normalize_columns = False
+ mock_base_model.always_filter_main_dttm = False
+ mock_base_model.columns = []
+ mock_base_model.metrics = []
+
+ # Create a mock database without spec to allow SQLAlchemy relationship
assignment
+ mock_database = MagicMock()
+ mock_database.id = 1
+ mock_database.get_default_catalog.return_value = None
+ # Add _sa_instance_state to allow SQLAlchemy relationship assignment
+ mock_database._sa_instance_state = MagicMock()
+
+ with (
+ patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=mock_base_model,
+ ),
+
patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"),
+ ):
+ with patch(
+ "superset.commands.dataset.duplicate.db.session.query"
+ ) as mock_query:
+ mock_query.return_value.get.return_value = mock_database
+ with patch(
+
"superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness",
+ return_value=True,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ with
patch("superset.commands.dataset.duplicate.db.session.add"):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 1,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ # Should not raise any errors
+ command.validate()
+ # Test the actual duplication
+ result = command.run()
+ assert result.table_name == "duplicated_dataset"
+ assert result.database.id == 1
+ assert result.schema == "public"
+ assert result.is_sqllab_view is True
+ assert result.sql == "SELECT 1 as c"
+
+
+def test_duplicate_dataset_not_found():
+ """Test duplication when base dataset doesn't exist."""
+ with patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=None,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 999,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ with pytest.raises(DatasetInvalidError) as exc_info:
+ command.validate()
+
+ # Verify the exception contains DatasetNotFoundError
+ validation_errors = exc_info.value._exceptions
+ assert any(
+ isinstance(error, DatasetNotFoundError) for error in
validation_errors
+ )
+
+
+def test_duplicate_dataset_not_virtual():
+ """Test that duplication fails for physical (non-virtual) datasets."""
+ mock_base_model = Mock(spec=SqlaTable)
+ mock_base_model.id = 1
+ mock_base_model.database_id = 1
+ mock_base_model.kind = "physical" # Not virtual
+
+ with (
+ patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=mock_base_model,
+ ),
+
patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"),
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness",
+ return_value=True,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 1,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ with pytest.raises(DatasetInvalidError) as exc_info:
+ command.validate()
+
+ # Verify the exception contains DatasourceTypeInvalidError
+ validation_errors = exc_info.value._exceptions
+ assert len(validation_errors) == 1
+ assert isinstance(validation_errors[0],
DatasourceTypeInvalidError)
+
+
+def test_duplicate_dataset_name_exists_same_database_schema():
Review Comment:
**Suggestion:** Add an explicit return type annotation to this test function
signature (for example, `-> None`). [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added test function lacks an explicit return type annotation in
the final file, which matches the custom typing rule violation.
</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=bf4f8ee8d0fc4863a17fefa9ae08596c&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=bf4f8ee8d0fc4863a17fefa9ae08596c&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/dataset/test_duplicate.py
**Line:** 305:305
**Comment:**
*Custom Rule: Add an explicit return type annotation to this test
function signature (for example, `-> None`).
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%2F37932&comment_hash=72dbb29542640733d53648c68a06bf261e909a382c657a8f55c16c551e4279b6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37932&comment_hash=72dbb29542640733d53648c68a06bf261e909a382c657a8f55c16c551e4279b6&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dataset/test_duplicate.py:
##########
@@ -165,3 +170,360 @@ def
test_duplicate_dataset_blocked_by_soft_deleted_twin(session: Session) -> Non
)
with pytest.raises(DatasetInvalidError):
command.validate()
+
+
+def test_duplicate_dataset_success():
+ """Test successful duplication of a virtual dataset."""
+ # Create a mock base model (virtual dataset)
+ mock_base_model = Mock(spec=SqlaTable)
+ mock_base_model.id = 1
+ mock_base_model.database_id = 1
+ mock_base_model.table_name = "original_dataset"
+ mock_base_model.schema = "public"
+ mock_base_model.catalog = None
+ mock_base_model.kind = "virtual"
+ mock_base_model.sql = "SELECT 1 as c"
+ mock_base_model.template_params = None
+ mock_base_model.normalize_columns = False
+ mock_base_model.always_filter_main_dttm = False
+ mock_base_model.columns = []
+ mock_base_model.metrics = []
+
+ # Create a mock database without spec to allow SQLAlchemy relationship
assignment
+ mock_database = MagicMock()
+ mock_database.id = 1
+ mock_database.get_default_catalog.return_value = None
+ # Add _sa_instance_state to allow SQLAlchemy relationship assignment
+ mock_database._sa_instance_state = MagicMock()
+
+ with (
+ patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=mock_base_model,
+ ),
+
patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"),
+ ):
+ with patch(
+ "superset.commands.dataset.duplicate.db.session.query"
+ ) as mock_query:
+ mock_query.return_value.get.return_value = mock_database
+ with patch(
+
"superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness",
+ return_value=True,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ with
patch("superset.commands.dataset.duplicate.db.session.add"):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 1,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ # Should not raise any errors
+ command.validate()
+ # Test the actual duplication
+ result = command.run()
+ assert result.table_name == "duplicated_dataset"
+ assert result.database.id == 1
+ assert result.schema == "public"
+ assert result.is_sqllab_view is True
+ assert result.sql == "SELECT 1 as c"
+
+
+def test_duplicate_dataset_not_found():
+ """Test duplication when base dataset doesn't exist."""
+ with patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=None,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 999,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ with pytest.raises(DatasetInvalidError) as exc_info:
+ command.validate()
+
+ # Verify the exception contains DatasetNotFoundError
+ validation_errors = exc_info.value._exceptions
+ assert any(
+ isinstance(error, DatasetNotFoundError) for error in
validation_errors
+ )
+
+
+def test_duplicate_dataset_not_virtual():
Review Comment:
**Suggestion:** Add an explicit return type annotation to this test function
signature (for example, `-> None`). [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The final code shows this test function without a return type hint. That
violates the Python type-hint requirement for modified 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=1c24770e9cc54d85a40ff94cc56e8ac1&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=1c24770e9cc54d85a40ff94cc56e8ac1&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/dataset/test_duplicate.py
**Line:** 266:266
**Comment:**
*Custom Rule: Add an explicit return type annotation to this test
function signature (for example, `-> None`).
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%2F37932&comment_hash=66eb17a8bea4ccd35edae9f72589fdc850a1c85fb758904f19a76cb866e0633f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37932&comment_hash=66eb17a8bea4ccd35edae9f72589fdc850a1c85fb758904f19a76cb866e0633f&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dataset/test_duplicate.py:
##########
@@ -165,3 +170,360 @@ def
test_duplicate_dataset_blocked_by_soft_deleted_twin(session: Session) -> Non
)
with pytest.raises(DatasetInvalidError):
command.validate()
+
+
+def test_duplicate_dataset_success():
+ """Test successful duplication of a virtual dataset."""
+ # Create a mock base model (virtual dataset)
+ mock_base_model = Mock(spec=SqlaTable)
+ mock_base_model.id = 1
+ mock_base_model.database_id = 1
+ mock_base_model.table_name = "original_dataset"
+ mock_base_model.schema = "public"
+ mock_base_model.catalog = None
+ mock_base_model.kind = "virtual"
+ mock_base_model.sql = "SELECT 1 as c"
+ mock_base_model.template_params = None
+ mock_base_model.normalize_columns = False
+ mock_base_model.always_filter_main_dttm = False
+ mock_base_model.columns = []
+ mock_base_model.metrics = []
+
+ # Create a mock database without spec to allow SQLAlchemy relationship
assignment
+ mock_database = MagicMock()
+ mock_database.id = 1
+ mock_database.get_default_catalog.return_value = None
+ # Add _sa_instance_state to allow SQLAlchemy relationship assignment
+ mock_database._sa_instance_state = MagicMock()
+
+ with (
+ patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=mock_base_model,
+ ),
+
patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"),
+ ):
+ with patch(
+ "superset.commands.dataset.duplicate.db.session.query"
+ ) as mock_query:
+ mock_query.return_value.get.return_value = mock_database
+ with patch(
+
"superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness",
+ return_value=True,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ with
patch("superset.commands.dataset.duplicate.db.session.add"):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 1,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ # Should not raise any errors
+ command.validate()
+ # Test the actual duplication
+ result = command.run()
+ assert result.table_name == "duplicated_dataset"
+ assert result.database.id == 1
+ assert result.schema == "public"
+ assert result.is_sqllab_view is True
+ assert result.sql == "SELECT 1 as c"
+
+
+def test_duplicate_dataset_not_found():
+ """Test duplication when base dataset doesn't exist."""
+ with patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=None,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 999,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ with pytest.raises(DatasetInvalidError) as exc_info:
+ command.validate()
+
+ # Verify the exception contains DatasetNotFoundError
+ validation_errors = exc_info.value._exceptions
+ assert any(
+ isinstance(error, DatasetNotFoundError) for error in
validation_errors
+ )
+
+
+def test_duplicate_dataset_not_virtual():
+ """Test that duplication fails for physical (non-virtual) datasets."""
+ mock_base_model = Mock(spec=SqlaTable)
+ mock_base_model.id = 1
+ mock_base_model.database_id = 1
+ mock_base_model.kind = "physical" # Not virtual
+
+ with (
+ patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=mock_base_model,
+ ),
+
patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"),
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness",
+ return_value=True,
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 1,
+ "table_name": "duplicated_dataset",
+ "owners": [],
+ }
+ )
+
+ with pytest.raises(DatasetInvalidError) as exc_info:
+ command.validate()
+
+ # Verify the exception contains DatasourceTypeInvalidError
+ validation_errors = exc_info.value._exceptions
+ assert len(validation_errors) == 1
+ assert isinstance(validation_errors[0],
DatasourceTypeInvalidError)
+
+
+def test_duplicate_dataset_name_exists_same_database_schema():
+ """Test that duplication fails when name exists in same database/schema."""
+ mock_base_model = Mock(spec=SqlaTable)
+ mock_base_model.id = 1
+ mock_base_model.database_id = 1
+ mock_base_model.table_name = "original_dataset"
+ mock_base_model.schema = "public"
+ mock_base_model.catalog = None
+ mock_base_model.kind = "virtual"
+
+ with (
+ patch(
+ "superset.commands.dataset.duplicate.DatasetDAO.find_by_id",
+ return_value=mock_base_model,
+ ),
+
patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"),
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness",
+ return_value=False, # Name already exists
+ ):
+ with patch(
+
"superset.commands.dataset.duplicate.DuplicateDatasetCommand.populate_owners",
+ return_value=[],
+ ):
+ command = DuplicateDatasetCommand(
+ {
+ "base_model_id": 1,
+ "table_name": "existing_dataset",
+ "owners": [],
+ }
+ )
+
+ with pytest.raises(DatasetInvalidError) as exc_info:
+ command.validate()
+
+ # Verify the exception contains DatasetExistsValidationError
+ validation_errors = exc_info.value._exceptions
+ assert len(validation_errors) == 1
+ assert isinstance(validation_errors[0],
DatasetExistsValidationError)
+
+
+def test_duplicate_dataset_catalog_preserved():
Review Comment:
**Suggestion:** Add an explicit return type annotation to this test function
signature (for example, `-> None`). [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The test function is missing a return type annotation in the final state of
the file, so the suggestion correctly identifies a type-hint violation.
</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=49c04788558448219566de6b7138aa1d&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=49c04788558448219566de6b7138aa1d&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/dataset/test_duplicate.py
**Line:** 347:347
**Comment:**
*Custom Rule: Add an explicit return type annotation to this test
function signature (for example, `-> None`).
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%2F37932&comment_hash=d6e9ce27c5f4976fd14252bcb868e129ba7bdff435a2230efd1b4dd5a7df0c0f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37932&comment_hash=d6e9ce27c5f4976fd14252bcb868e129ba7bdff435a2230efd1b4dd5a7df0c0f&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]
