fix(profile-filter): support owner object payloads and normalize owners response
This commit is contained in:
@@ -12,6 +12,7 @@ from src.app import app
|
|||||||
from src.api.routes.dashboards import DashboardsResponse
|
from src.api.routes.dashboards import DashboardsResponse
|
||||||
from src.dependencies import get_current_user, has_permission, get_config_manager, get_task_manager, get_resource_service, get_mapping_service
|
from src.dependencies import get_current_user, has_permission, get_config_manager, get_task_manager, get_resource_service, get_mapping_service
|
||||||
from src.core.database import get_db
|
from src.core.database import get_db
|
||||||
|
from src.services.profile_service import ProfileService as DomainProfileService
|
||||||
|
|
||||||
# Global mock user for get_current_user dependency overrides
|
# Global mock user for get_current_user dependency overrides
|
||||||
mock_user = MagicMock()
|
mock_user = MagicMock()
|
||||||
@@ -806,4 +807,71 @@ def test_get_dashboards_profile_filter_matches_display_alias_without_detail_fano
|
|||||||
# [/DEF:test_get_dashboards_profile_filter_matches_display_alias_without_detail_fanout:Function]
|
# [/DEF:test_get_dashboards_profile_filter_matches_display_alias_without_detail_fanout:Function]
|
||||||
|
|
||||||
|
|
||||||
|
# [DEF:test_get_dashboards_profile_filter_matches_owner_object_payload_contract:Function]
|
||||||
|
# @TEST: GET /api/dashboards profile-default filter matches Superset owner object payloads.
|
||||||
|
# @PRE: Profile-default preference is enabled and owners list contains dict payloads.
|
||||||
|
# @POST: Response keeps dashboards where owner object resolves to bound username alias.
|
||||||
|
def test_get_dashboards_profile_filter_matches_owner_object_payload_contract(mock_deps):
|
||||||
|
mock_env = MagicMock()
|
||||||
|
mock_env.id = "prod"
|
||||||
|
mock_deps["config"].get_environments.return_value = [mock_env]
|
||||||
|
mock_deps["task"].get_all_tasks.return_value = []
|
||||||
|
mock_deps["resource"].get_dashboards_with_status = AsyncMock(return_value=[
|
||||||
|
{
|
||||||
|
"id": 701,
|
||||||
|
"title": "Featured Charts",
|
||||||
|
"slug": "featured-charts",
|
||||||
|
"owners": [
|
||||||
|
{
|
||||||
|
"id": 11,
|
||||||
|
"first_name": "user",
|
||||||
|
"last_name": "1",
|
||||||
|
"username": None,
|
||||||
|
"email": "user_1@example.local",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"modified_by": "another_user",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": 702,
|
||||||
|
"title": "Other Dashboard",
|
||||||
|
"slug": "other-dashboard",
|
||||||
|
"owners": [
|
||||||
|
{
|
||||||
|
"id": 12,
|
||||||
|
"first_name": "other",
|
||||||
|
"last_name": "user",
|
||||||
|
"username": None,
|
||||||
|
"email": "other@example.local",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"modified_by": "other_user",
|
||||||
|
},
|
||||||
|
])
|
||||||
|
|
||||||
|
with patch("src.api.routes.dashboards.ProfileService") as profile_service_cls, patch(
|
||||||
|
"src.api.routes.dashboards._resolve_profile_actor_aliases",
|
||||||
|
return_value=["user_1"],
|
||||||
|
):
|
||||||
|
profile_service = DomainProfileService(db=MagicMock(), config_manager=MagicMock())
|
||||||
|
profile_service.get_my_preference = MagicMock(
|
||||||
|
return_value=_build_profile_preference_stub(
|
||||||
|
username="user_1",
|
||||||
|
enabled=True,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
profile_service_cls.return_value = profile_service
|
||||||
|
|
||||||
|
response = client.get(
|
||||||
|
"/api/dashboards?env_id=prod&page_context=dashboards_main&apply_profile_default=true"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
payload = response.json()
|
||||||
|
assert payload["total"] == 1
|
||||||
|
assert {item["id"] for item in payload["dashboards"]} == {701}
|
||||||
|
assert payload["dashboards"][0]["title"] == "Featured Charts"
|
||||||
|
# [/DEF:test_get_dashboards_profile_filter_matches_owner_object_payload_contract:Function]
|
||||||
|
|
||||||
|
|
||||||
# [/DEF:backend.src.api.routes.__tests__.test_dashboards:Module]
|
# [/DEF:backend.src.api.routes.__tests__.test_dashboards:Module]
|
||||||
|
|||||||
@@ -274,6 +274,72 @@ def _normalize_actor_alias_token(value: Any) -> Optional[str]:
|
|||||||
# [/DEF:_normalize_actor_alias_token:Function]
|
# [/DEF:_normalize_actor_alias_token:Function]
|
||||||
|
|
||||||
|
|
||||||
|
# [DEF:_normalize_owner_display_token:Function]
|
||||||
|
# @PURPOSE: Project owner payload value into stable display string for API response contracts.
|
||||||
|
# @PRE: owner can be scalar, dict or None.
|
||||||
|
# @POST: Returns trimmed non-empty owner display token or None.
|
||||||
|
def _normalize_owner_display_token(owner: Any) -> Optional[str]:
|
||||||
|
if owner is None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
if isinstance(owner, dict):
|
||||||
|
username = str(owner.get("username") or owner.get("user_name") or owner.get("name") or "").strip()
|
||||||
|
full_name = str(owner.get("full_name") or "").strip()
|
||||||
|
first_name = str(owner.get("first_name") or "").strip()
|
||||||
|
last_name = str(owner.get("last_name") or "").strip()
|
||||||
|
combined = " ".join(part for part in [first_name, last_name] if part).strip()
|
||||||
|
email = str(owner.get("email") or "").strip()
|
||||||
|
|
||||||
|
for candidate in [username, full_name, combined, email]:
|
||||||
|
if candidate:
|
||||||
|
return candidate
|
||||||
|
return None
|
||||||
|
|
||||||
|
normalized = str(owner).strip()
|
||||||
|
return normalized or None
|
||||||
|
# [/DEF:_normalize_owner_display_token:Function]
|
||||||
|
|
||||||
|
|
||||||
|
# [DEF:_normalize_dashboard_owner_values:Function]
|
||||||
|
# @PURPOSE: Normalize dashboard owners payload to optional list of display strings.
|
||||||
|
# @PRE: owners payload can be None, scalar, or list with mixed values.
|
||||||
|
# @POST: Returns deduplicated owner labels preserving order, or None when absent.
|
||||||
|
def _normalize_dashboard_owner_values(owners: Any) -> Optional[List[str]]:
|
||||||
|
if owners is None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
raw_items: List[Any]
|
||||||
|
if isinstance(owners, list):
|
||||||
|
raw_items = owners
|
||||||
|
else:
|
||||||
|
raw_items = [owners]
|
||||||
|
|
||||||
|
normalized: List[str] = []
|
||||||
|
for owner in raw_items:
|
||||||
|
token = _normalize_owner_display_token(owner)
|
||||||
|
if token and token not in normalized:
|
||||||
|
normalized.append(token)
|
||||||
|
|
||||||
|
return normalized
|
||||||
|
# [/DEF:_normalize_dashboard_owner_values:Function]
|
||||||
|
|
||||||
|
|
||||||
|
# [DEF:_project_dashboard_response_items:Function]
|
||||||
|
# @PURPOSE: Project dashboard payloads to response-contract-safe shape.
|
||||||
|
# @PRE: dashboards is a list of dict-like dashboard payloads.
|
||||||
|
# @POST: Returned items satisfy DashboardItem owners=list[str]|None contract.
|
||||||
|
def _project_dashboard_response_items(dashboards: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
|
||||||
|
projected: List[Dict[str, Any]] = []
|
||||||
|
for dashboard in dashboards:
|
||||||
|
projected_dashboard = dict(dashboard)
|
||||||
|
projected_dashboard["owners"] = _normalize_dashboard_owner_values(
|
||||||
|
projected_dashboard.get("owners")
|
||||||
|
)
|
||||||
|
projected.append(projected_dashboard)
|
||||||
|
return projected
|
||||||
|
# [/DEF:_project_dashboard_response_items:Function]
|
||||||
|
|
||||||
|
|
||||||
# [DEF:_resolve_profile_actor_aliases:Function]
|
# [DEF:_resolve_profile_actor_aliases:Function]
|
||||||
# @PURPOSE: Resolve stable actor aliases for profile filtering without per-dashboard detail fan-out.
|
# @PURPOSE: Resolve stable actor aliases for profile filtering without per-dashboard detail fan-out.
|
||||||
# @PRE: bound username is available and env is valid.
|
# @PRE: bound username is available and env is valid.
|
||||||
@@ -620,8 +686,10 @@ async def get_dashboards(
|
|||||||
f"(page {page}/{total_pages}, total: {total}, profile_filter_applied={effective_profile_filter.applied})"
|
f"(page {page}/{total_pages}, total: {total}, profile_filter_applied={effective_profile_filter.applied})"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
response_dashboards = _project_dashboard_response_items(paginated_dashboards)
|
||||||
|
|
||||||
return DashboardsResponse(
|
return DashboardsResponse(
|
||||||
dashboards=paginated_dashboards,
|
dashboards=response_dashboards,
|
||||||
total=total,
|
total=total,
|
||||||
page=page,
|
page=page,
|
||||||
page_size=page_size,
|
page_size=page_size,
|
||||||
|
|||||||
@@ -343,9 +343,30 @@ class ProfileService:
|
|||||||
return []
|
return []
|
||||||
normalized: List[str] = []
|
normalized: List[str] = []
|
||||||
for owner in owners:
|
for owner in owners:
|
||||||
token = self._normalize_username(str(owner or ""))
|
owner_candidates: List[Any]
|
||||||
if token and token not in normalized:
|
if isinstance(owner, dict):
|
||||||
normalized.append(token)
|
first_name = self._sanitize_username(str(owner.get("first_name") or ""))
|
||||||
|
last_name = self._sanitize_username(str(owner.get("last_name") or ""))
|
||||||
|
full_name = " ".join(part for part in [first_name, last_name] if part).strip()
|
||||||
|
snake_name = "_".join(part for part in [first_name, last_name] if part).strip("_")
|
||||||
|
owner_candidates = [
|
||||||
|
owner.get("username"),
|
||||||
|
owner.get("user_name"),
|
||||||
|
owner.get("name"),
|
||||||
|
owner.get("full_name"),
|
||||||
|
first_name,
|
||||||
|
last_name,
|
||||||
|
full_name or None,
|
||||||
|
snake_name or None,
|
||||||
|
owner.get("email"),
|
||||||
|
]
|
||||||
|
else:
|
||||||
|
owner_candidates = [owner]
|
||||||
|
|
||||||
|
for candidate in owner_candidates:
|
||||||
|
token = self._normalize_username(str(candidate or ""))
|
||||||
|
if token and token not in normalized:
|
||||||
|
normalized.append(token)
|
||||||
return normalized
|
return normalized
|
||||||
# [/DEF:_normalize_owner_tokens:Function]
|
# [/DEF:_normalize_owner_tokens:Function]
|
||||||
# [/DEF:ProfileService:Class]
|
# [/DEF:ProfileService:Class]
|
||||||
|
|||||||
@@ -219,3 +219,8 @@ Mandatory UX verification tasks are included at the end of each user story phase
|
|||||||
- [x] D007 Investigate regression where `/api/dashboards` triggers per-dashboard `SupersetClient.get_dashboard` fan-out
|
- [x] D007 Investigate regression where `/api/dashboards` triggers per-dashboard `SupersetClient.get_dashboard` fan-out
|
||||||
- [x] D008 Replace O(N) hydration with O(1) profile actor alias lookup (username + display name) in dashboards filter path
|
- [x] D008 Replace O(N) hydration with O(1) profile actor alias lookup (username + display name) in dashboards filter path
|
||||||
- [x] D009 Verify updated dashboards filter behavior with targeted backend tests including no-detail-fanout assertion
|
- [x] D009 Verify updated dashboards filter behavior with targeted backend tests including no-detail-fanout assertion
|
||||||
|
- [x] D010 Investigate profile filter mismatch for Superset `owners` object payloads (`first_name`/`last_name`) that excluded `user_1` dashboards
|
||||||
|
- [x] D011 Extend actor matching normalization in `backend/src/services/profile_service.py` to include owner dict aliases (`full_name`, `first_name last_name`, `first_name_last_name`, username/email)
|
||||||
|
- [x] D012 Normalize dashboards API response owners to `List[str]` via route projection in `backend/src/api/routes/dashboards.py` to preserve `DashboardsResponse` contract when upstream owners include dicts
|
||||||
|
- [x] D013 Add regression test for owners-object profile filtering in `backend/src/api/routes/__tests__/test_dashboards.py`
|
||||||
|
- [x] D014 Verify fix with targeted backend test run: `cd backend && .venv/bin/python3 -m pytest src/api/routes/__tests__/test_dashboards.py` (23 passed)
|
||||||
Reference in New Issue
Block a user