sadpandajoe commented on code in PR #45090:
URL: https://github.com/apache/superset/pull/45090#discussion_r4295133846


##########
tests/unit_tests/canvas/render_test.py:
##########
@@ -59,3 +62,73 @@ def 
test_placements_include_auto_placed_nodes_and_grid_containers() -> None:
         "c": "chart",
     }
     assert context["gridColumns"] == {"g": 12}
+
+
+def test_layout_constraints_carry_declared_span_limits() -> None:
+    """Only the limits a widget actually declares reach the client."""
+    rules, resolver = canvas_widget_types(), FakeResolver()
+    rules["sized"] = type(
+        "SizedWidget",
+        (Widget,),
+        {
+            "widget_type": "sized",
+            "name": "Sized",
+            "controls_class": Props,
+            "behavior": WidgetBehavior(),
+            "ui": WidgetUi(default_size=(6, 4), min_col_span=4, 
max_row_span=8),
+        },
+    )
+    definition = normalize_definition(
+        canvas(
+            {
+                "tile": node("sized-1", layout={"colSpan": 6}),
+                # The same widget, placed inside a grid container.
+                "g": node("group", layout={"colSpan": 24}, children=["inner"]),
+                "inner": node("sized-2", layout={"colSpan": 6}),
+                # A widget that declares nothing is left out entirely.
+                "plain": node("chart-1", layout={"colSpan": 4}),
+            },
+            children=["tile", "g", "plain"],
+        ),
+        rules,
+        resolver,
+    )
+
+    context = render_context(definition, rules, resolver)
+
+    assert context["layoutConstraints"] == {
+        "tile": {"minColSpan": 4, "maxRowSpan": 8},

Review Comment:
   `test_layout_constraints_carry_declared_span_limits` only declares 
`min_col_span` and `max_row_span`, so `maxColSpan` and `minRowSpan` in 
`_span_constraints` are never asserted. Swapping or dropping either mapping in 
`render.py` leaves this test and every other canvas unit test green, while the 
client then gets no `maxColSpan`/`minRowSpan` clamp and a resize past the 
widget's declared limit is sent to the server and rejected. Could the fixture 
also set `max_col_span` and `min_row_span` to distinct values and assert all 
four keys here?



-- 
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]

Reply via email to