fix: address review issues -- validate color, guard KeyError, surface visibility, detect no-op, fix -g shorthand

- Guard missing 'visual' key in visual_set_container with PbiCliError
- Detect no-op in visual_set_container when no args are provided
- Remove -g shorthand from set-container --page option
- Add is_hidden field to page_list and page_get return dicts
- Validate hex color format in page_set_background before writing
- Add tests for all new validation and behaviour
This commit is contained in:
MinaSaad1 2026-04-01 19:25:33 +02:00
parent c39c049d6a
commit a8272926f3
5 changed files with 83 additions and 2 deletions

View file

@ -577,7 +577,7 @@ def calc_list(
@visual.command(name="set-container")
@click.argument("name")
@click.option("--page", "-g", required=True, help="Page name/ID.")
@click.option("--page", required=True, help="Page name/ID.")
@click.option(
"--border-show",
type=bool,

View file

@ -8,6 +8,7 @@ Python dict suitable for ``format_result()``.
from __future__ import annotations
import json
import re
import secrets
from pathlib import Path
from typing import Any
@ -292,6 +293,7 @@ def page_list(definition_path: Path) -> list[dict[str, Any]]:
"height": data.get("height", 720),
"display_option": data.get("displayOption", "FitToPage"),
"visual_count": visual_count,
"is_hidden": data.get("visibility") == "HiddenInViewMode",
})
# Sort by page order if available, then by ordinal
@ -385,6 +387,7 @@ def page_get(definition_path: Path, page_name: str) -> dict[str, Any]:
"height": data.get("height", 720),
"display_option": data.get("displayOption", "FitToPage"),
"visual_count": visual_count,
"is_hidden": data.get("visibility") == "HiddenInViewMode",
}
@ -398,6 +401,11 @@ def page_set_background(
Updates the ``objects.background`` property in ``page.json``.
The color must be a hex string, e.g. ``'#F8F9FA'``.
"""
if not re.fullmatch(r"#[0-9A-Fa-f]{3,8}", color):
raise PbiCliError(
f"Invalid color '{color}' -- expected hex format like '#F8F9FA'."
)
page_dir = get_page_dir(definition_path, page_name)
page_json_path = page_dir / "page.json"
if not page_json_path.exists():

View file

@ -404,7 +404,22 @@ def visual_set_container(
)
data = _read_json(visual_json_path)
visual = data["visual"]
visual = data.get("visual")
if visual is None:
raise PbiCliError(
f"Visual '{visual_name}' has invalid JSON -- missing 'visual' key."
)
if border_show is None and background_show is None and title is None:
return {
"status": "no-op",
"visual": visual_name,
"page": page_name,
"border_show": None,
"background_show": None,
"title": None,
}
vco: dict[str, Any] = dict(visual.get("visualContainerObjects", {}))
def _bool_entry(value: bool) -> list[dict[str, Any]]:

View file

@ -980,3 +980,53 @@ def test_page_set_visibility_idempotent_visible(sample_report: Path) -> None:
def test_page_set_visibility_raises_for_missing_page(sample_report: Path) -> None:
with pytest.raises(PbiCliError, match="not found"):
page_set_visibility(sample_report, "ghost_page", hidden=True)
# ---------------------------------------------------------------------------
# Fix 4: hex color validation in page_set_background
# ---------------------------------------------------------------------------
def test_page_set_background_rejects_invalid_color(sample_report: Path) -> None:
with pytest.raises(PbiCliError, match="Invalid color"):
page_set_background(sample_report, "page1", "F8F9FA") # missing #
def test_page_set_background_rejects_invalid_color_wrong_chars(sample_report: Path) -> None:
with pytest.raises(PbiCliError, match="Invalid color"):
page_set_background(sample_report, "page1", "#GGHHII") # non-hex chars
def test_page_set_background_accepts_valid_color(sample_report: Path) -> None:
result = page_set_background(sample_report, "page1", "#F8F9FA")
assert result["status"] == "updated"
assert result["background_color"] == "#F8F9FA"
# ---------------------------------------------------------------------------
# Fix 3: is_hidden surfaced in page_list and page_get
# ---------------------------------------------------------------------------
def test_page_list_shows_hidden_status(sample_report: Path) -> None:
pages = page_list(sample_report)
assert all("is_hidden" in p for p in pages)
# Initially the page is visible
assert pages[0]["is_hidden"] is False
# Hide the first page and verify is_hidden flips
first_page = pages[0]["name"]
page_set_visibility(sample_report, first_page, hidden=True)
updated = page_list(sample_report)
hidden_page = next(p for p in updated if p["name"] == first_page)
assert hidden_page["is_hidden"] is True
def test_page_get_shows_hidden_status(sample_report: Path) -> None:
result = page_get(sample_report, "page1")
assert "is_hidden" in result
assert result["is_hidden"] is False
page_set_visibility(sample_report, "page1", hidden=True)
result = page_get(sample_report, "page1")
assert result["is_hidden"] is True

View file

@ -780,3 +780,11 @@ def test_visual_set_container_raises_for_missing_visual(
visual_set_container(
report_with_page, "test_page", "nonexistent_visual", border_show=False
)
def test_visual_set_container_no_op_returns_no_op_status(
page_with_bar_visual: tuple[Path, str],
) -> None:
defn, vname = page_with_bar_visual
result = visual_set_container(defn, "test_page", vname)
assert result["status"] == "no-op"