Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset/models/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def get_query_context_factory(self) -> QueryContextFactory:

@classmethod
def get(cls, id_or_uuid: str) -> Slice:
qry = db.session.query(Slice).filter_by(id_or_uuid_filter(id_or_uuid))
qry = db.session.query(Slice).filter(id_or_uuid_filter(id_or_uuid))
return qry.one_or_none()


Expand Down
38 changes: 38 additions & 0 deletions tests/integration_tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.

import uuid
from io import BytesIO
from unittest import mock
from unittest.mock import patch
Expand Down Expand Up @@ -1076,6 +1077,43 @@ def test_get_chart_not_found(self):
rv = self.get_assert_metric(uri, "get")
assert rv.status_code == 404

@parameterized.expand(
[
("by_id", lambda chart: str(chart.id), "id"),
(
"by_uuid",
lambda chart: str(chart.uuid) if chart.uuid else pytest.skip("No UUID"),
"uuid",
),
]
)
def test_slice_get_existing(self, test_name, get_identifier, field_type):
"""Test Slice.get() successfully retrieves existing charts."""
admin = self.get_user("admin")
chart = self.insert_chart(f"test_slice_get_{field_type}", [admin.id], 1)

identifier = get_identifier(chart)
result = Slice.get(identifier)

assert result is not None
assert result.id == chart.id
if field_type == "uuid" and chart.uuid:
assert result.uuid == chart.uuid

db.session.delete(chart)
db.session.commit()

@parameterized.expand(
[
("nonexistent_id", "999999"),
("nonexistent_uuid", str(uuid.uuid4())),
]
)
def test_slice_get_not_found(self, test_name, identifier):
"""Test Slice.get() returns None for non-existent identifiers."""
result = Slice.get(identifier)
assert result is None

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_get_chart_no_data_access(self):
"""
Expand Down
86 changes: 86 additions & 0 deletions tests/unit_tests/models/slice_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# 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.

import uuid
from unittest.mock import MagicMock, patch

import pytest
from parameterized import parameterized

from superset.models.slice import id_or_uuid_filter, Slice


class TestSlice:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment from above

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Antonio-RiveroMartnez good call. Parameterized it so I'd still know if one of the use cases failed but no duplicated code.

"""Test cases for Slice model functionality."""

@parameterized.expand(
[
("numeric_id", "123"),
("uuid_string", "550e8400-e29b-41d4-a716-446655440000"),
]
)
def test_slice_get_calls_filter_correctly(self, test_name, id_or_uuid):
"""Test Slice.get() calls filter() correctly for ID and UUID."""
with patch("superset.models.slice.db") as mock_db:
# Setup mock chain
mock_query = MagicMock()
mock_filtered_query = MagicMock()
mock_db.session.query.return_value = mock_query
mock_query.filter.return_value = mock_filtered_query
mock_filtered_query.one_or_none.return_value = None

# Call the method
result = Slice.get(id_or_uuid)

# Verify correct methods called
mock_db.session.query.assert_called_once_with(Slice)
mock_query.filter.assert_called_once() # Not filter_by!
mock_filtered_query.one_or_none.assert_called_once()
assert result is None

@parameterized.expand(
[
("numeric_id", "123"),
("large_id", "999999"),
("uuid_string", str(uuid.uuid4())),
]
)
def test_slice_get_no_type_error(self, test_name, input_value):
"""Verify Slice.get() doesn't raise TypeError for various inputs."""
try:
result = Slice.get(input_value)
# Success - no TypeError, result can be None or a Slice
assert result is None or hasattr(result, "id")
except TypeError as e:
if "filter_by() takes 1 positional argument" in str(e):
pytest.fail(
f"filter_by() bug exists: Slice.get('{input_value}') failed with {e}" # noqa: E501
)
else:
raise

@parameterized.expand(
[
("numeric_id", "123"),
("uuid_format", "550e8400-e29b-41d4-a716-446655440000"),
("invalid_string", "not-a-number"),
]
)
def test_id_or_uuid_filter(self, test_name, input_value):
"""Test id_or_uuid_filter returns correct BinaryExpression."""
result = id_or_uuid_filter(input_value)
assert result is not None
Loading