diff --git a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py index 6fe2a304c..47bfc2dff 100644 --- a/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py +++ b/packages/gooddata-eval/src/gooddata_eval/core/agentic/dashboard_skill.py @@ -186,16 +186,41 @@ def _sections_of(dashboard: dict) -> list[dict]: def _widgets_of(dashboard: dict) -> list[dict]: - """Every widget of the dashboard, flattened across sections. + """Every leaf widget of the dashboard, flattened across sections and the containers in them. Charts are matched dashboard-wide on purpose: the expectation names charts, not a layout, - so which section or tab one lands on is the agent's call. The date filter is deliberately - the opposite -- ``_check_date_range`` requires *every* tab to match. That one is defensive - rather than observed: ``draft_dashboard`` copies a single filter onto every tab + so which section, tab or container one lands on is the agent's call. The date filter is + deliberately the opposite -- ``_check_date_range`` requires *every* tab to match. That one is + defensive rather than observed: ``draft_dashboard`` copies a single filter onto every tab unconditionally, so a draft cannot currently disagree with itself across tabs, and the per-tab check is there to notice if that stops being true. """ - return [widget for section in _sections_of(dashboard) for widget in section.get("widgets") or []] + return _leaf_widgets(_sections_of(dashboard)) + + +def _is_container(widget: dict) -> bool: + """Whether a widget is a container, named by either of its two keys. + + The AAC converter writes ``container`` on every container but ``sections`` only when it has + some, so a saved empty container arrives with ``container`` alone. + """ + return "container" in widget or "sections" in widget + + +def _leaf_widgets(sections: list[dict]) -> list[dict]: + """The widgets of ``sections`` in document order, each container replaced by the widgets inside it. + + A switcher stays one widget, as it does at the top level, so the chart checks reading this + list do not see its member charts. + """ + leaves: list[dict] = [] + for section in sections: + for widget in section.get("widgets") or []: + if _is_container(widget): + leaves.extend(_leaf_widgets(widget.get("sections") or [])) + else: + leaves.append(widget) + return leaves def _apply_patch(base: dict, operations: list[dict]) -> tuple[dict | None, str | None]: @@ -747,7 +772,7 @@ def _check_tabs(dashboard: dict, new_ids: set[str], expected: list[dict]) -> lis for index, (tab, exp) in enumerate(zip(tabs, expected)): if "title" in exp and _norm(str(tab.get("title") or "")) != _norm(str(exp["title"] or "")): failures.append(f"tab {index} is titled {tab.get('title')!r}, expected {exp['title']!r}") - tab_widgets = [w for section in tab.get("sections") or [] for w in section.get("widgets") or []] + tab_widgets = _leaf_widgets(tab.get("sections") or []) failures.extend( f"tab {index} ({tab.get('title')!r}) does not hold chart {chart!r}" for chart in exp.get("visualizations") or [] @@ -902,15 +927,20 @@ def _named_tab(dashboard: dict, tab_id: str | None) -> tuple[dict | None, str]: def _check_layout(dashboard: dict, new_ids: set[str], expected: list[dict]) -> list[str]: - """Each named tab must hold exactly the expected sections, in order, each with exactly its charts in order. + """Each named layout must hold exactly the expected sections, in order, each with exactly its widgets in order. Exact on purpose. A move that leaves the widget where it was, places a second copy, or puts it in the wrong section keeps every chart "on the dashboard", and so does a removal that - never happened; only the whole arrangement tells them apart. A section's ``title`` is - compared when the entry states it, ``null`` meaning untitled. Its ``visualizations`` are - always stated, ``[]`` for a section without charts. A chart is an id, or - ``{"title": ...}`` for one the agent authored. Widgets that show no visualization are not - charts and are left out on both sides. + never happened; only the whole arrangement tells them apart. + + The layout is a tab's own, or with ``container_path`` the container that path reaches from + it, compared on ``layout_direction`` too when the entry states it. A section's ``title`` is + compared when the entry states it, ``null`` meaning untitled. Its ``visualizations`` list the + charts, ``[]`` for a section without any, and the widgets that show no visualization are + left out on both sides. Its ``widgets`` instead list every child: a chart is an id, or + ``{"title": ...}`` for one the agent authored, a note ``{"text": ...}`` and a switcher + ``{"switcher": [charts]}``, which is how a swap of a note and a chart, or a switcher taken + apart, is told from the arrangement asked for. """ failures: list[str] = [] for entry in expected: @@ -919,6 +949,16 @@ def _check_layout(dashboard: dict, new_ids: set[str], expected: list[dict]) -> l failures.append(where) continue sections = tab.get("sections") or [] + if "container_path" in entry: + container, where = _container_at(sections, entry["container_path"], where) + if container is None: + failures.append(where) + continue + sections = container.get("sections") or [] + # The converter writes no direction for a row, and the dashboard lays a container out as one. + direction = container.get("layout_direction") or "row" + if "layout_direction" in entry and direction != entry["layout_direction"]: + failures.append(f"{where} is laid out as a {direction}, expected a {entry['layout_direction']}") wanted = entry["sections"] if len(sections) != len(wanted): titles = ", ".join(repr(s.get("title")) for s in sections) @@ -929,6 +969,14 @@ def _check_layout(dashboard: dict, new_ids: set[str], expected: list[dict]) -> l failures.append( f"{where} section {index} is titled {section.get('title')!r}, expected {want['title']!r}" ) + if "widgets" in want: + children = section.get("widgets") or [] + if len(children) != len(want["widgets"]) or not all( + _holds(w, new_ids, c) for w, c in zip(children, want["widgets"], strict=True) + ): + shown = [_widget_entry(w) for w in children] + failures.append(f"{where} section {index} holds {shown}, expected {want['widgets']}") + continue charts = [w for w in section.get("widgets") or [] if w.get("visualization")] charts_wanted = want["visualizations"] if len(charts) != len(charts_wanted) or not all( @@ -939,6 +987,64 @@ def _check_layout(dashboard: dict, new_ids: set[str], expected: list[dict]) -> l return failures +def _container_at(sections: list[dict], path: list[dict], where: str) -> tuple[dict | None, str]: + """The container ``path`` reaches from ``sections``, and how to name it in a failure. + + Each hop is a section and a widget index inside the layout the previous hop reached. A hop + that lands on nothing, or on a widget that is not a container, gives ``None`` and the reason: + the result left no container where the case expects one, which fails the case. + """ + container: dict | None = None + for depth, hop in enumerate(path): + at = f"section {hop['section']}, widget_index {hop['widget_index']}" + widgets = (sections[hop["section"]].get("widgets") or []) if hop["section"] < len(sections) else [] + if hop["widget_index"] >= len(widgets): + return None, f"{where} has no widget at {at}" + container = widgets[hop["widget_index"]] + if not _is_container(container): + return None, f"{where} holds {_widget_entry(container)!r} at {at}, not a container" + where = f"{where} {'container at' if depth == 0 else '>'} {at}" + sections = container.get("sections") or [] + return container, where + + +def _holds(widget: dict, new_ids: set[str], expected: str | dict) -> bool: + """Whether ``widget`` is the child a layout section's ``widgets`` entry names. + + The widget is read as the kind ``_widget_entry`` names it, so a widget carrying the keys of + two kinds matches only the one it renders as. + """ + shown = _widget_entry(widget) + if isinstance(expected, dict) and "text" in expected: + return shown == expected + if isinstance(expected, dict) and "switcher" in expected: + members = widget.get("visualizations") or [] + return ( + isinstance(shown, dict) + and "switcher" in shown + and len(members) == len(expected["switcher"]) + and all( + _matching_widgets([m], new_ids, _chart_entry(c)) + for m, c in zip(members, expected["switcher"], strict=True) + ) + ) + return not isinstance(shown, dict) and bool(_matching_widgets([widget], new_ids, _chart_entry(expected))) + + +def _widget_entry(widget: dict) -> object: + """A child widget written the way a layout section's ``widgets`` entry names it, for a failure message. + + Kinds are read in the converter's order, container first, so a widget is named as it renders. + """ + if _is_container(widget): + return {"container": widget.get("container")} + if "visualizations" in widget: + return {"switcher": [m.get("visualization") for m in widget["visualizations"] or []]} + if "content" in widget: + return {"text": widget["content"]} + return widget.get("visualization") + + def _check_filter_bar(dashboard: dict, expected: list[dict]) -> list[str]: """Each named tab must carry exactly the expected filters: the kept ones by key, the new ones by content. @@ -1416,24 +1522,34 @@ def _validate_layout_expectations(expected_output: dict) -> None: raise ValueError(f"a preserved widget needs its visualization and a key to guard, got {widget!r}") if "layout" in expected_output: - for entry in _tab_entries(expected_output, "layout", {"tab", "sections"}): + for entry in _layout_entries(expected_output): sections = entry.get("sections") if not isinstance(sections, list): raise ValueError(f"a layout entry needs its sections as a list, got {entry!r}") for section in sections: - if not isinstance(section, dict) or section.keys() - {"title", "visualizations"}: - raise ValueError(f"a layout section takes only title and visualizations, got {section!r}") + if not isinstance(section, dict) or section.keys() - {"title", "visualizations", "widgets"}: + raise ValueError( + f"a layout section takes only title and visualizations or widgets, got {section!r}" + ) if section.get("title") is not None and not isinstance(section["title"], str): raise ValueError(f"a layout section's title must be a string or null, got {section!r}") - if "visualizations" not in section: + if ("visualizations" in section) == ("widgets" in section): # Missing would read as "no charts" here but as "any charts" in `tabs`. - raise ValueError(f"a layout section must list its visualizations, [] for none, got {section!r}") + raise ValueError( + f"a layout section must list either its visualizations or its widgets, [] for none, " + f"got {section!r}" + ) + if "widgets" in section: + if not isinstance(section["widgets"], list) or not all( + _is_widget_entry(w) for w in section["widgets"] + ): + raise ValueError( + "a layout section's widgets must be chart ids, {'title': ...}, {'text': ...} or " + f"{{'switcher': [charts]}}, got {section!r}" + ) + continue charts = section["visualizations"] - if not isinstance(charts, list) or not all( - (isinstance(c, str) and c) - or (isinstance(c, dict) and c.keys() == {"title"} and isinstance(c["title"], str) and c["title"]) - for c in charts - ): + if not isinstance(charts, list) or not all(_is_chart_entry(c) for c in charts): raise ValueError( f"a layout section's visualizations must be chart ids or {{'title': ...}}, got {section!r}" ) @@ -1470,8 +1586,94 @@ def _validate_bar_filter(label: str, body: object) -> None: _validate_tab_attribute_filter(label, body) +def _layout_entries(expected_output: dict) -> list[dict]: + """The entries of ``layout``, each naming one layout: a tab's own, or a container on it by path. + + Raises: + ValueError: not a non-empty list of objects, an unknown key, a malformed tab, path or + direction, or one layout named twice. A layout is its tab and its path, the tab's own + layout having none, so one tab can hold several entries. An entry without ``tab`` is + for a dashboard with one tab, so it cannot sit beside an entry that names a tab: both + could name that one tab. + """ + entries = expected_output["layout"] + if not isinstance(entries, list) or not entries or not all(isinstance(e, dict) for e in entries): + raise ValueError(f"layout must be a non-empty list of layout objects, got {entries!r}") + seen: set[tuple[str | None, tuple[tuple[int, int], ...]]] = set() + for entry in entries: + unknown = sorted(entry.keys() - {"tab", "container_path", "layout_direction", "sections"}) + if unknown: + raise ValueError(f"a layout entry cannot state {unknown}, got {entry!r}") + tab = entry.get("tab") + if tab is not None and not (isinstance(tab, str) and tab): + raise ValueError(f"a layout entry's tab must be a tab id, got {entry!r}") + path = _container_path(entry) + if "layout_direction" in entry and (not path or entry["layout_direction"] not in ("row", "column")): + raise ValueError(f"a layout entry's layout_direction is 'row' or 'column' on a container, got {entry!r}") + if (tab, path) in seen: + target = f"the container at {entry['container_path']}" if path else "the layout" + owner = "the dashboard" if tab is None else f"tab {tab!r}" + raise ValueError(f"layout names {target} of {owner} more than once") + seen.add((tab, path)) + tabs = {tab for tab, _ in seen} + if None in tabs and len(tabs) > 1: + raise ValueError(f"a layout entry without tab cannot sit beside one that names a tab, got {entries!r}") + return entries + + +def _container_path(entry: dict) -> tuple[tuple[int, int], ...]: + """A layout entry's ``container_path`` as ``(section, widget_index)`` hops, ``()`` for the tab's own layout. + + Raises: + ValueError: present, but not a non-empty list of hops that each carry exactly a + non-negative ``section`` and ``widget_index``. ``null`` and ``[]`` are refused rather + than read as the tab's own layout: leaving the key out says that. + """ + if "container_path" not in entry: + return () + path = entry["container_path"] + # bool is an int, so `True` would otherwise read as index 1. + if ( + not isinstance(path, list) + or not path + or not all( + isinstance(hop, dict) + and hop.keys() == {"section", "widget_index"} + and all(isinstance(i, int) and not isinstance(i, bool) and i >= 0 for i in hop.values()) + for hop in path + ) + ): + raise ValueError( + f"a layout entry's container_path must be a non-empty list of {{'section', 'widget_index'}} hops, " + f"got {entry!r}" + ) + return tuple((hop["section"], hop["widget_index"]) for hop in path) + + +def _is_chart_entry(entry: object) -> bool: + """A chart as a layout section names it: an id, or ``{"title": ...}`` for one the agent authored.""" + return (isinstance(entry, str) and bool(entry)) or ( + isinstance(entry, dict) + and entry.keys() == {"title"} + and isinstance(entry["title"], str) + and bool(entry["title"]) + ) + + +def _is_widget_entry(entry: object) -> bool: + """A child as a layout section's ``widgets`` names it: a chart, ``{"text": ...}`` or ``{"switcher": [charts]}``.""" + if _is_chart_entry(entry): + return True + if not isinstance(entry, dict) or len(entry) != 1: + return False + if "text" in entry: + return isinstance(entry["text"], str) + charts = entry.get("switcher") + return isinstance(charts, list) and bool(charts) and all(_is_chart_entry(c) for c in charts) + + def _tab_entries(expected_output: dict, key: str, allowed: set[str]) -> list[dict]: - """The entries of a per-tab expectation, checked for the shape ``layout`` and ``filter_bar`` share. + """The entries of a per-tab expectation such as ``filter_bar``, checked for the shape they share. Raises: ValueError: not a non-empty list of objects, a key outside ``allowed``, or a tab named diff --git a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py index e7d577df2..9b6075b9f 100644 --- a/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py +++ b/packages/gooddata-eval/tests/test_agentic_dashboard_skill.py @@ -10,6 +10,8 @@ DashboardSkillAssertionError, _date_range_text, _extract_tool_result, + _holds, + _is_container, _skill_activated, _validate_expectation, _widgets_of, @@ -1089,6 +1091,51 @@ def test_widgets_are_read_from_tabs_when_present(self): drafted = _dashboard_part([_widget("A", "id-a")], date_filter=_ALL_TIME_FILTER)["dashboard"] assert [w["visualization"] for w in _widgets_of(drafted)] == ["id-a"] + def test_widgets_inside_containers_are_read_and_a_switcher_stays_whole(self): + switcher = {"visualizations": [{"visualization": "id-c"}, {"visualization": "id-d"}]} + inner = {"container": "inner", "sections": [{"widgets": [_widget("B", "id-b"), switcher]}]} + outer = {"container": "outer", "sections": [{"widgets": [_widget("A", "id-a"), inner]}]} + base = _edit_base_part([outer, _widget("E", "id-e")], visualizations=[])["dashboard"] + assert _widgets_of(base) == [_widget("A", "id-a"), _widget("B", "id-b"), switcher, _widget("E", "id-e")] + + def test_an_empty_container_or_section_holds_no_widgets(self): + """The AAC converter leaves out `sections` of an empty container and `widgets` of an empty section.""" + group = {"container": "g", "sections": [{"title": "Empty"}, {"widgets": [_widget("A", "id-a")]}]} + emptied = {"container": "emptied", "columns": 12, "rows": 10} + base = _edit_base_part([emptied, group], visualizations=[])["dashboard"] + assert _widgets_of(base) == [_widget("A", "id-a")] + + @pytest.mark.parametrize( + ("widget", "container"), + [ + ({"container": "g", "sections": []}, True), + ({"sections": []}, True), + # An empty container: the AAC converter writes `sections` only when there are some. + ({"container": "g", "columns": 12, "rows": 10}, True), + (_widget("A", "id-a"), False), + ({"content": "Europe only"}, False), + ({"id": "s", "visualizations": [{"visualization": "id-a"}]}, False), + ], + ) + def test_a_widget_is_a_container_by_its_container_or_sections_key(self, widget, container): + assert _is_container(widget) is container + + # The converter reads a widget's kind as container, then switcher, then note, then chart. + @pytest.mark.parametrize( + ("widget", "expected", "holds"), + [ + ({"content": "Europe only"}, {"text": "Europe only"}, True), + ({"visualizations": [{"visualization": "id-a"}], "content": "Europe only"}, {"text": "Europe only"}, False), + ({"container": "g", "content": "Europe only"}, {"text": "Europe only"}, False), + ({"visualizations": [{"visualization": "id-a"}]}, {"switcher": ["id-a"]}, True), + ({"container": "g", "visualizations": [{"visualization": "id-a"}]}, {"switcher": ["id-a"]}, False), + (_widget("A", "id-a"), "id-a", True), + ({**_widget("A", "id-a"), "content": "Europe only"}, "id-a", False), + ], + ) + def test_a_child_matches_only_as_the_kind_it_renders_as(self, widget, expected, holds): + assert _holds(widget, set(), expected) is holds + class TestSimulatedReply: def test_existing_charts_are_listed_verbatim(self): @@ -1883,6 +1930,104 @@ def _score_root_edit(expected: dict, operations: list[dict], new_visualizations: _ROOT_EDIT = {**_DE01_EXPECTED, "visualizations": [{"id": _NET_SALES, "title": "Net Sales"}]} +# A container on the overview tab: a note, a drilled chart and a switcher side by side, and beside +# it a second container that nests a third one. Unit-fixture ids, not the live workspace's. +_PRODUCT_BREAKDOWN = "product_breakdown" +_ORDER_VOLUME = "order_volume_by_category" +_NET_SALES_BY_CATEGORY = "net_sales_by_product_category" +_EUROPE_NOTE = "All figures cover Europe only." +_GROUP_PATH = [{"section": 0, "widget_index": 0}] +_INNER_GROUP_PATH = [{"section": 0, "widget_index": 1}, {"section": 0, "widget_index": 1}] + + +def _group(group_id: str, *widgets: dict, direction: str | None = None) -> dict: + group = {"container": group_id, "columns": 12, "rows": 22, "sections": [{"widgets": list(widgets)}]} + return group if direction is None else {**group, "layout_direction": direction} + + +def _switcher(*charts: str) -> dict: + return {"visualizations": [{"visualization": c, "title": c} for c in charts], "columns": 4, "rows": 22} + + +def _grouped_base_part(*, tabbed: bool) -> dict: + product_group = _group( + "product_group", + {"content": _EUROPE_NOTE, "columns": 4, "rows": 22}, + {**_widget("Product breakdown", _PRODUCT_BREAKDOWN, columns=4), "interactions": _DRILL}, + _switcher(_ORDER_VOLUME, _NET_SALES_BY_CATEGORY), + direction="row", + ) + inner_group = _group( + "inner_group", _widget("Active Customers", _ACTIVE_CUSTOMERS), _widget("Return Customers", _RETURN_CUSTOMERS) + ) + kpi_group = _group("kpi_group", _widget("Total Customers", _TOTAL_CUSTOMERS), inner_group) + sections = [{"widgets": [product_group, kpi_group]}] + charts = [_PRODUCT_BREAKDOWN, _ORDER_VOLUME, _NET_SALES_BY_CATEGORY, _TOTAL_CUSTOMERS, _ACTIVE_CUSTOMERS] + part = _edit_base_part(visualizations=[*charts, _RETURN_CUSTOMERS], filters={}) + if tabbed: + part["dashboard"] = { + **part["dashboard"], + "version": "3", + "tabs": [{"id": "overview", "title": "Overview", "filters": {}, "sections": sections}], + } + del part["dashboard"]["sections"], part["dashboard"]["filters"] + else: + part["dashboard"]["sections"] = sections + return part + + +def _group_ops(tabbed: bool, *, swap: bool = True, column: bool = True) -> list[dict]: + """Stack the product group: the chart moves ahead of the note, and the row becomes a column.""" + group = f"{'/tabs/0' if tabbed else ''}/sections/0/widgets/0" + chart = f"{group}/sections/0/widgets/1" + operations = [{"op": "test", "path": f"{chart}/visualization", "value": _PRODUCT_BREAKDOWN}] + if swap: + operations.append({"op": "move", "from": chart, "path": f"{group}/sections/0/widgets/0"}) + if column: + operations.append({"op": "replace", "path": f"{group}/layout_direction", "value": "column"}) + return operations + + +_STACKED = { + "container_path": _GROUP_PATH, + "layout_direction": "column", + "sections": [ + { + "widgets": [ + _PRODUCT_BREAKDOWN, + {"text": _EUROPE_NOTE}, + {"switcher": [_ORDER_VOLUME, _NET_SALES_BY_CATEGORY]}, + ] + } + ], +} +# The inner container carries no layout_direction, which the dashboard lays out as a row. +_INNER_ACTIVE_FIRST = { + "container_path": _INNER_GROUP_PATH, + "layout_direction": "row", + "sections": [{"visualizations": [_ACTIVE_CUSTOMERS, _RETURN_CUSTOMERS]}], +} +_GROUP_EDIT = { + **_DE01_EXPECTED, + "visualizations": [{"id": _PRODUCT_BREAKDOWN, "title": "Product breakdown"}], + "preserved_widgets": [{"visualization": _PRODUCT_BREAKDOWN, "interactions": _DRILL}], +} + + +def _score_group_edit( + layout: list[dict], operations: list[dict], *, tabbed: bool = True, new_visualizations: list[str] | None = None +): + if tabbed: + layout = [{"tab": "overview", **entry} for entry in layout] + patch_part = _patch_part(operations, new_visualizations=new_visualizations) + return evaluate_dashboard_response( + _patch_result(len(new_visualizations or [])), + _grouped_base_part(tabbed=tabbed), + {**_GROUP_EDIT, "layout": layout}, + True, + patch_part=patch_part, + ) + class TestLayout: """A move or a removal is only visible in the whole arrangement: every chart staying "on the @@ -1982,6 +2127,43 @@ def test_an_entry_without_a_tab_on_a_tabbed_dashboard_fails(self): # On a one-tab dashboard both entries could name that tab. [{"sections": []}, {"tab": "tab_customers", "sections": []}], [{"tabs": "tab_customers", "sections": []}], + # A container path is a non-empty list of two non-negative indices per hop. + [{"container_path": None, "sections": []}], + [{"container_path": [], "sections": []}], + [{"container_path": "0/0", "sections": []}], + [{"container_path": [{"section": 0}], "sections": []}], + [{"container_path": [{"section": 0, "widget_index": True}], "sections": []}], + [{"container_path": [{"section": -1, "widget_index": 0}], "sections": []}], + [{"container_path": [{"section": "0", "widget_index": 0}], "sections": []}], + [{"container_path": [{"section": 0, "widget_index": 0, "tab": "x"}], "sections": []}], + # A direction belongs to a container, and is row or column. + [{"layout_direction": "column", "sections": []}], + [{"container_path": _GROUP_PATH, "layout_direction": "diagonal", "sections": []}], + [{"container_path": _GROUP_PATH, "layout_direction": None, "sections": []}], + # A section lists its charts or its widgets, never both. + [{"sections": [{"visualizations": [], "widgets": []}]}], + [{"sections": [{"widgets": "chart"}]}], + *( + [{"sections": [{"widgets": [entry]}]}] + for entry in ( + "", + 3, + {"text": 3}, + {"switcher": []}, + {"switcher": "a"}, + {"switcher": [{"id": "a"}]}, + {"switcher": [{"switcher": ["a"]}]}, + {"chart": "a"}, + {"title": ""}, + {"title": "A", "text": "B"}, + ) + ), + # One target per (tab, path); an unnamed tab still cannot sit beside a named one. + [ + {"tab": "tab_customers", "container_path": _GROUP_PATH, "sections": []}, + {"tab": "tab_customers", "container_path": _GROUP_PATH, "sections": []}, + ], + [{"container_path": _GROUP_PATH, "sections": []}, {"tab": "tab_customers", "sections": []}], ], ) def test_a_malformed_expectation_is_rejected_up_front(self, layout): @@ -1990,6 +2172,221 @@ def test_a_malformed_expectation_is_rejected_up_front(self, layout): _run_with(client, {**_TABBED_EDIT, "layout": layout}) client.send_message.assert_not_called() + @pytest.mark.parametrize( + "layout", + [ + # Distinct containers on one tab, beside the tab's own layout. + [ + {"tab": "tab_customers", "sections": []}, + {"tab": "tab_customers", "container_path": _GROUP_PATH, "layout_direction": "row", "sections": []}, + {"tab": "tab_customers", "container_path": [*_GROUP_PATH, _GROUP_PATH[0]], "sections": []}, + ], + # Several unnamed entries all name the one implicit tab. + [{"sections": []}, {"container_path": _GROUP_PATH, "sections": []}], + [ + { + "container_path": _GROUP_PATH, + "sections": [ + {"title": None, "widgets": ["a", {"title": "B"}, {"text": "C"}, {"switcher": ["d"]}]}, + {"widgets": []}, + ], + } + ], + ], + ) + def test_a_container_expectation_is_accepted(self, layout): + _validate_expectation({**_TABBED_EDIT, "layout": layout}) + + def test_widgets_also_describe_a_tab_layout(self): + layout = [{"tab": "tab_customers", "sections": [{"widgets": [_TOTAL_CUSTOMERS, _NET_SALES]}]}] + swap = [{"op": "move", "from": "/tabs/0/sections/0/widgets/1", "path": "/tabs/0/sections/0/widgets/0"}] + assert _score_tabbed_edit({**_TABBED_EDIT, "layout": layout}, swap).layout_correct + assert not _score_tabbed_edit({**_TABBED_EDIT, "layout": layout}, _RESIZE_AMOUNT).layout_correct + + +class TestContainerLayout: + """A container is checked by its path: its direction and its exact children, charts, notes and + switchers alike. Charts inside it count as on the dashboard for every chart check.""" + + @pytest.mark.parametrize("tabbed", [True, False]) + def test_the_stacked_container_matches(self, tabbed): + ev = _score_group_edit([_STACKED, _INNER_ACTIVE_FIRST], _group_ops(tabbed), tabbed=tabbed) + assert ev.strict_checks["dashboard_layout_correct"] is True + assert ev.strict_checks["dashboard_widgets_preserved"] is True + assert ev.strict_pass, ev.failures + + @pytest.mark.parametrize( + ("operations", "failure"), + [ + pytest.param( + _group_ops(True, swap=False), + "tab 'overview' container at section 0, widget_index 0 section 0 holds " + "[{'text': 'All figures cover Europe only.'}, 'product_breakdown', ", + id="note-and-chart-not-swapped", + ), + pytest.param( + _group_ops(True, column=False), + "tab 'overview' container at section 0, widget_index 0 is laid out as a row, expected a column", + id="left-as-a-row", + ), + pytest.param( + [ + *_group_ops(True), + { + "op": "move", + "from": "/tabs/0/sections/0/widgets/0/sections/0/widgets/2/visualizations/1", + "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/2/visualizations/0", + }, + ], + "{'switcher': ['net_sales_by_product_category', 'order_volume_by_category']}]", + id="switcher-members-reversed", + ), + pytest.param( + [ + *_group_ops(True), + { + "op": "add", + "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/-", + "value": _widget("Product breakdown", _PRODUCT_BREAKDOWN), + }, + ], + "'order_volume_by_category', 'net_sales_by_product_category']}, 'product_breakdown']", + id="chart-placed-twice", + ), + pytest.param( + [*_group_ops(True), {"op": "remove", "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/1"}], + "holds ['product_breakdown', {'switcher':", + id="note-removed", + ), + pytest.param( + [ + *_group_ops(True), + { + "op": "replace", + "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/2", + "value": _widget("Order volume", _ORDER_VOLUME), + }, + ], + "{'text': 'All figures cover Europe only.'}, 'order_volume_by_category']", + id="switcher-replaced-by-a-chart", + ), + pytest.param( + [ + *_group_ops(True), + { + "op": "replace", + "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/1/content", + "value": "All figures cover Europe.", + }, + ], + "holds ['product_breakdown', {'text': 'All figures cover Europe.'}, ", + id="note-text-changed", + ), + pytest.param( + [ + *_group_ops(True), + { + "op": "replace", + "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/0/visualization", + "value": _TOTAL_CUSTOMERS, + }, + ], + f"holds [{_TOTAL_CUSTOMERS!r}, {{'text': ", + id="another-chart-in-place", + ), + pytest.param([], "the patch carries no operations", id="empty-patch"), + ], + ) + def test_a_wrong_arrangement_fails(self, operations, failure): + ev = _score_group_edit([_STACKED], operations) + assert not ev.layout_correct + assert any(failure in f for f in ev.failures), ev.failures + + def test_another_container_on_the_same_tab_is_checked_too(self): + reversed_inner = { + **_INNER_ACTIVE_FIRST, + "sections": [{"visualizations": [_RETURN_CUSTOMERS, _ACTIVE_CUSTOMERS]}], + } + ev = _score_group_edit([_STACKED, reversed_inner], _group_ops(True)) + assert not ev.layout_correct + assert ev.failures == [ + "tab 'overview' container at section 0, widget_index 1 > section 0, widget_index 1 section 0 holds " + f"[{_ACTIVE_CUSTOMERS!r}, {_RETURN_CUSTOMERS!r}], expected [{_RETURN_CUSTOMERS!r}, {_ACTIVE_CUSTOMERS!r}]" + ] + + @pytest.mark.parametrize( + ("path", "failure"), + [ + ([{"section": 0, "widget_index": 5}], "tab 'overview' has no widget at section 0, widget_index 5"), + ([{"section": 1, "widget_index": 0}], "tab 'overview' has no widget at section 1, widget_index 0"), + ( + [*_GROUP_PATH, {"section": 0, "widget_index": 0}], + "tab 'overview' container at section 0, widget_index 0 holds 'product_breakdown' at " + "section 0, widget_index 0, not a container", + ), + ( + [*_GROUP_PATH, {"section": 0, "widget_index": 2}], + "holds {'switcher': ['order_volume_by_category', 'net_sales_by_product_category']} at " + "section 0, widget_index 2, not a container", + ), + ], + ) + def test_a_path_that_reaches_no_container_fails_the_case(self, path, failure): + ev = _score_group_edit([{**_STACKED, "container_path": path}], _group_ops(True)) + assert not ev.layout_correct + assert any(failure in f for f in ev.failures), ev.failures + + def test_an_authored_chart_in_a_container_is_matched_by_title(self): + authored = "b5c2f8d4-0000-4000-8000-000000000001" + operations = [ + *_group_ops(True), + { + "op": "add", + "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/-", + "value": _widget("Returns by Category", authored), + }, + ] + layout = [ + { + **_STACKED, + "sections": [{"widgets": [*_STACKED["sections"][0]["widgets"], {"title": "returns by category"}]}], + } + ] + assert _score_group_edit(layout, operations, new_visualizations=[authored]).layout_correct + assert not _score_group_edit(layout, operations).layout_correct + + @pytest.mark.parametrize( + ("last", "passes"), + [(_switcher(_ORDER_VOLUME), True), (_widget("Order volume", _ORDER_VOLUME), False)], + ids=["one-chart-switcher", "plain-chart"], + ) + def test_a_one_chart_switcher_is_not_a_chart(self, last, passes): + replace = {"op": "replace", "path": "/tabs/0/sections/0/widgets/0/sections/0/widgets/2", "value": last} + layout = [ + { + **_STACKED, + "sections": [{"widgets": [_PRODUCT_BREAKDOWN, {"text": _EUROPE_NOTE}, {"switcher": [_ORDER_VOLUME]}]}], + } + ] + assert _score_group_edit(layout, [*_group_ops(True), replace]).layout_correct is passes + + def test_tabs_and_chart_checks_see_charts_inside_containers(self): + expected = { + **_GROUP_EDIT, + "visualizations": [ + {"id": _PRODUCT_BREAKDOWN, "title": "Product breakdown"}, + {"id": _ACTIVE_CUSTOMERS, "title": "Active Customers"}, + ], + "tabs": [{"title": "Overview", "visualizations": [_PRODUCT_BREAKDOWN, _ACTIVE_CUSTOMERS]}], + } + patch_part = _patch_part(_group_ops(True)) + ev = evaluate_dashboard_response( + _patch_result(), _grouped_base_part(tabbed=True), expected, True, patch_part=patch_part + ) + assert ev.strict_checks["charts_matched"] is True + assert ev.strict_checks["dashboard_tabs_correct"] is True + assert ev.strict_pass, ev.failures + _CUSTOMER_CONTAINS = {"type": "text_filter", "using": "label/customer_name", "condition": "contains", "value": "Aaron"} # A list filter turned into a text filter as the agent emits it: removed, then added under a minted key.