From 26885d98cfa9f07381106c372f8d3ed83c68b954 Mon Sep 17 00:00:00 2001 From: Elijah Ahianyo Date: Thu, 28 Sep 2023 16:31:01 +0000 Subject: [PATCH] rx.table __bool__ regression fix (#1828) --- integration/test_table.py | 168 +++++++++++++++++++ reflex/components/datadisplay/table.py | 100 +++++++++++- reflex/utils/types.py | 15 ++ tests/components/datadisplay/test_table.py | 177 +++++++++++++++++++++ 4 files changed, 454 insertions(+), 6 deletions(-) create mode 100644 integration/test_table.py create mode 100644 tests/components/datadisplay/test_table.py diff --git a/integration/test_table.py b/integration/test_table.py new file mode 100644 index 000000000..cbdc37250 --- /dev/null +++ b/integration/test_table.py @@ -0,0 +1,168 @@ +"""Integration tests for table and related components.""" +from typing import Generator + +import pytest +from selenium.webdriver.common.by import By + +from reflex.testing import AppHarness + + +def Table(): + """App using table component.""" + from typing import List + + import reflex as rx + + class TableState(rx.State): + rows: List[List[str]] = [ + ["John", "30", "New York"], + ["Jane", "31", "San Fransisco"], + ["Joe", "32", "Los Angeles"], + ] + + headers: List[str] = ["Name", "Age", "Location"] + + footers: List[str] = ["footer1", "footer2", "footer3"] + + caption: str = "random caption" + + @rx.var + def token(self) -> str: + return self.get_token() + + app = rx.App(state=TableState) + + @app.add_page + def index(): + return rx.center( + rx.input(id="token", value=TableState.token, is_read_only=True), + rx.table_container( + rx.table( + headers=TableState.headers, + rows=TableState.rows, + footers=TableState.footers, + caption=TableState.caption, + variant="striped", + color_scheme="blue", + width="100%", + ), + ), + ) + + @app.add_page + def another(): + return rx.center( + rx.table_container( + rx.table( # type: ignore + rx.thead( # type: ignore + rx.tr( # type: ignore + rx.th("Name"), + rx.th("Age"), + rx.th("Location"), + ) + ), + rx.tbody( # type: ignore + rx.tr( # type: ignore + rx.td("John"), + rx.td(30), + rx.td("New York"), + ), + rx.tr( # type: ignore + rx.td("Jane"), + rx.td(31), + rx.td("San Francisco"), + ), + rx.tr( # type: ignore + rx.td("Joe"), + rx.td(32), + rx.td("Los Angeles"), + ), + ), + rx.tfoot( # type: ignore + rx.tr(rx.td("footer1"), rx.td("footer2"), rx.td("footer3")) # type: ignore + ), + rx.table_caption("random caption"), + variant="striped", + color_scheme="teal", + ) + ) + ) + + app.compile() + + +@pytest.fixture() +def table(tmp_path_factory) -> Generator[AppHarness, None, None]: + """Start Table app at tmp_path via AppHarness. + + Args: + tmp_path_factory: pytest tmp_path_factory fixture + + Yields: + running AppHarness instance + + """ + with AppHarness.create( + root=tmp_path_factory.mktemp("table"), + app_source=Table, # type: ignore + ) as harness: + assert harness.app_instance is not None, "app is not running" + yield harness + + +@pytest.fixture +def driver(table: AppHarness): + """GEt an instance of the browser open to the table app. + + Args: + table: harness for Table app + + Yields: + WebDriver instance. + """ + driver = table.frontend() + try: + token_input = driver.find_element(By.ID, "token") + assert token_input + # wait for the backend connection to send the token + token = table.poll_for_value(token_input) + assert token is not None + + yield driver + finally: + driver.quit() + + +@pytest.mark.parametrize("route", ["", "/another"]) +def test_table(driver, table: AppHarness, route): + """Test that a table component is rendered properly. + + Args: + driver: Selenium WebDriver open to the app + table: Harness for Table app + route: Page route or path. + """ + driver.get(f"{table.frontend_url}/{route}") + assert table.app_instance is not None, "app is not running" + + thead = driver.find_element(By.TAG_NAME, "thead") + # poll till page is fully loaded. + table.poll_for_content(element=thead) + # check headers + assert thead.find_element(By.TAG_NAME, "tr").text == "NAME AGE LOCATION" + # check first row value + assert ( + driver.find_element(By.TAG_NAME, "tbody") + .find_elements(By.TAG_NAME, "tr")[0] + .text + == "John 30 New York" + ) + # check footer + assert ( + driver.find_element(By.TAG_NAME, "tfoot") + .find_element(By.TAG_NAME, "tr") + .text.lower() + == "footer1 footer2 footer3" + ) + # check caption + assert driver.find_element(By.TAG_NAME, "caption").text == "random caption" diff --git a/reflex/components/datadisplay/table.py b/reflex/components/datadisplay/table.py index facc5e9f1..28b9a0b4a 100644 --- a/reflex/components/datadisplay/table.py +++ b/reflex/components/datadisplay/table.py @@ -1,9 +1,10 @@ """Table components.""" -from typing import List +from typing import List, Tuple from reflex.components.component import Component from reflex.components.layout.foreach import Foreach from reflex.components.libs.chakra import ChakraComponent +from reflex.utils import types from reflex.vars import Var @@ -44,16 +45,16 @@ class Table(ChakraComponent): if len(children) == 0: children = [] - if caption: + if caption is not None: children.append(TableCaption.create(caption)) - if headers: + if headers is not None: children.append(Thead.create(headers=headers)) - if rows: + if rows is not None: children.append(Tbody.create(rows=rows)) - if footers: + if footers is not None: children.append(Tfoot.create(footers=footers)) return super().create(*children, **props) @@ -77,11 +78,36 @@ class Thead(ChakraComponent): Returns: The table header component. + """ if len(children) == 0: + cls.validate_headers(headers) + children = [Tr.create(cell_type="header", cells=headers)] return super().create(*children, **props) + @staticmethod + def validate_headers(headers): + """Type checking for table headers. + + Args: + headers: The table headers. + + Raises: + TypeError: If headers are not of type list or type tuple. + + """ + allowed_types = (list, tuple) + if ( + ( + isinstance(headers, Var) + and not types.check_type_in_allowed_types(headers.type_, allowed_types) + ) + or not isinstance(headers, Var) + and not types.check_type_in_allowed_types(type(headers), allowed_types) + ): + raise TypeError("table headers should be a list or tuple") + class Tbody(ChakraComponent): """A table body component.""" @@ -101,9 +127,11 @@ class Tbody(ChakraComponent): **props: The properties of the component. Returns: - Component: _description_ + Component: The table body component """ if len(children) == 0: + cls.validate_rows(rows) if rows is not None else None + if isinstance(rows, Var): children = [ Foreach.create( @@ -116,6 +144,44 @@ class Tbody(ChakraComponent): ] return super().create(*children, **props) + @staticmethod + def validate_rows(rows): + """Type checking for table rows. + + Args: + rows: Table rows. + + Raises: + TypeError: If rows are not lists or tuples containing inner lists or tuples. + """ + allowed_subclasses = (List, Tuple) + if isinstance(rows, Var): + outer_type = rows.type_ + inner_type = ( + outer_type.__args__[0] if hasattr(outer_type, "__args__") else None + ) + + # check that the outer container and inner container types are lists or tuples. + if not ( + types._issubclass(types.get_base_class(outer_type), allowed_subclasses) + and ( + inner_type is None + or types._issubclass( + types.get_base_class(inner_type), allowed_subclasses + ) + ) + ): + raise TypeError( + f"table rows should be a list or tuple containing inner lists or tuples. Got {outer_type} instead" + ) + elif not ( + types._issubclass(type(rows), allowed_subclasses) + and (not rows or types._issubclass(type(rows[0]), allowed_subclasses)) + ): + raise TypeError( + "table rows should be a list or tuple containing inner lists or tuples." + ) + class Tfoot(ChakraComponent): """A table footer component.""" @@ -138,9 +204,31 @@ class Tfoot(ChakraComponent): The table footer component. """ if len(children) == 0: + cls.validate_footers(footers) children = [Tr.create(cell_type="header", cells=footers)] return super().create(*children, **props) + @staticmethod + def validate_footers(footers): + """Type checking for table footers. + + Args: + footers: Table rows. + + Raises: + TypeError: If footers are not of type list. + """ + allowed_types = (list, tuple) + if ( + ( + isinstance(footers, Var) + and not types.check_type_in_allowed_types(footers.type_, allowed_types) + ) + or not isinstance(footers, Var) + and not types.check_type_in_allowed_types(type(footers), allowed_types) + ): + raise TypeError("table headers should be a list or tuple") + class Tr(ChakraComponent): """A table row component.""" diff --git a/reflex/utils/types.py b/reflex/utils/types.py index 8a3db553e..169d0de8c 100644 --- a/reflex/utils/types.py +++ b/reflex/utils/types.py @@ -170,6 +170,21 @@ def is_backend_variable(name: str) -> bool: return name.startswith("_") and not name.startswith("__") +def check_type_in_allowed_types( + value_type: Type, allowed_types: typing.Iterable +) -> bool: + """Check that a value type is found in a list of allowed types. + + Args: + value_type: Type of value. + allowed_types: Iterable of allowed types. + + Returns: + If the type is found in the allowed types. + """ + return get_base_class(value_type) in allowed_types + + # Store this here for performance. StateBases = get_base_class(StateVar) StateIterBases = get_base_class(StateIterVar) diff --git a/tests/components/datadisplay/test_table.py b/tests/components/datadisplay/test_table.py new file mode 100644 index 000000000..94e39f6e5 --- /dev/null +++ b/tests/components/datadisplay/test_table.py @@ -0,0 +1,177 @@ +import sys +from typing import List, Tuple + +import pytest + +from reflex.components.datadisplay.table import Tbody, Tfoot, Thead +from reflex.state import State + +PYTHON_GT_V38 = sys.version_info.major >= 3 and sys.version_info.minor > 8 + + +class TableState(State): + """Test State class.""" + + rows_List_List_str: List[List[str]] = [["random", "row"]] + rows_List_List: List[List] = [["random", "row"]] + rows_List_str: List[str] = ["random", "row"] + rows_Tuple_List_str: Tuple[List[str]] = (["random", "row"],) + rows_Tuple_List: Tuple[List] = ["random", "row"] # type: ignore + rows_Tuple_str_str: Tuple[str, str] = ( + "random", + "row", + ) + rows_Tuple_Tuple_str_str: Tuple[Tuple[str, str]] = ( + ( + "random", + "row", + ), + ) + rows_Tuple_Tuple: Tuple[Tuple] = ( + ( + "random", + "row", + ), + ) + rows_str: str = "random, row" + headers_List_str: List[str] = ["header1", "header2"] + headers_Tuple_str_str: Tuple[str, str] = ( + "header1", + "header2", + ) + headers_str: str = "headers1, headers2" + footers_List_str: List[str] = ["footer1", "footer2"] + footers_Tuple_str_str: Tuple[str, str] = ( + "footer1", + "footer2", + ) + footers_str: str = "footer1, footer2" + + if sys.version_info.major >= 3 and sys.version_info.minor > 8: + rows_list_list_str: list[list[str]] = [["random", "row"]] + rows_list_list: list[list] = [["random", "row"]] + rows_list_str: list[str] = ["random", "row"] + rows_tuple_list_str: tuple[list[str]] = (["random", "row"],) + rows_tuple_list: tuple[list] = ["random", "row"] # type: ignore + rows_tuple_str_str: tuple[str, str] = ( + "random", + "row", + ) + rows_tuple_tuple_str_str: tuple[tuple[str, str]] = ( + ( + "random", + "row", + ), + ) + rows_tuple_tuple: tuple[tuple] = ( + ( + "random", + "row", + ), + ) + + +valid_extras = ( + [ + TableState.rows_list_list_str, + TableState.rows_list_list, + TableState.rows_tuple_list_str, + TableState.rows_tuple_list, + TableState.rows_tuple_tuple_str_str, + TableState.rows_tuple_tuple, + ] + if PYTHON_GT_V38 + else [] +) +invalid_extras = ( + [TableState.rows_list_str, TableState.rows_tuple_str_str] if PYTHON_GT_V38 else [] +) + + +@pytest.mark.parametrize( + "rows", + [ + [["random", "row"]], + TableState.rows_List_List_str, + TableState.rows_List_List, + TableState.rows_Tuple_List_str, + TableState.rows_Tuple_List, + TableState.rows_Tuple_Tuple_str_str, + TableState.rows_Tuple_Tuple, + *valid_extras, + ], +) +def test_create_table_body_with_valid_rows_prop(rows): + render_dict = Tbody.create(rows=rows).render() + assert render_dict["name"] == "Tbody" + assert len(render_dict["children"]) == 1 + + +@pytest.mark.parametrize( + "rows", + [ + ["random", "row"], + "random, rows", + TableState.rows_List_str, + TableState.rows_Tuple_str_str, + TableState.rows_str, + *invalid_extras, + ], +) +def test_create_table_body_with_invalid_rows_prop(rows): + with pytest.raises(TypeError): + Tbody.create(rows=rows) + + +@pytest.mark.parametrize( + "headers", + [ + ["random", "header"], + TableState.headers_List_str, + TableState.headers_Tuple_str_str, + ], +) +def test_create_table_head_with_valid_headers_prop(headers): + render_dict = Thead.create(headers=headers).render() + assert render_dict["name"] == "Thead" + assert len(render_dict["children"]) == 1 + assert render_dict["children"][0]["name"] == "Tr" + + +@pytest.mark.parametrize( + "headers", + [ + "random, header", + TableState.headers_str, + ], +) +def test_create_table_head_with_invalid_headers_prop(headers): + with pytest.raises(TypeError): + Thead.create(headers=headers) + + +@pytest.mark.parametrize( + "footers", + [ + ["random", "footers"], + TableState.footers_List_str, + TableState.footers_Tuple_str_str, + ], +) +def test_create_table_footer_with_valid_footers_prop(footers): + render_dict = Tfoot.create(footers=footers).render() + assert render_dict["name"] == "Tfoot" + assert len(render_dict["children"]) == 1 + assert render_dict["children"][0]["name"] == "Tr" + + +@pytest.mark.parametrize( + "footers", + [ + "random, footers", + TableState.footers_str, + ], +) +def test_create_table_footer_with_invalid_footers_prop(footers): + with pytest.raises(TypeError): + Tfoot.create(footers=footers)