From c5a3001e32881599d5578f1c738479e3228d2787 Mon Sep 17 00:00:00 2001 From: busya Date: Fri, 6 Mar 2026 15:02:03 +0300 Subject: [PATCH] fix(profile-filter): support owner object payloads and normalize owners response --- .../api/routes/__tests__/test_dashboards.py | 68 ++++++++++++++++++ backend/src/api/routes/dashboards.py | 70 ++++++++++++++++++- backend/src/services/profile_service.py | 27 ++++++- specs/024-user-dashboard-filter/tasks.md | 7 +- 4 files changed, 167 insertions(+), 5 deletions(-) diff --git a/backend/src/api/routes/__tests__/test_dashboards.py b/backend/src/api/routes/__tests__/test_dashboards.py index 45f8f22..0558655 100644 --- a/backend/src/api/routes/__tests__/test_dashboards.py +++ b/backend/src/api/routes/__tests__/test_dashboards.py @@ -12,6 +12,7 @@ from src.app import app 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.core.database import get_db +from src.services.profile_service import ProfileService as DomainProfileService # Global mock user for get_current_user dependency overrides 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_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] diff --git a/backend/src/api/routes/dashboards.py b/backend/src/api/routes/dashboards.py index 77de72e..ee6373f 100644 --- a/backend/src/api/routes/dashboards.py +++ b/backend/src/api/routes/dashboards.py @@ -274,6 +274,72 @@ def _normalize_actor_alias_token(value: Any) -> Optional[str]: # [/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] # @PURPOSE: Resolve stable actor aliases for profile filtering without per-dashboard detail fan-out. # @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})" ) + response_dashboards = _project_dashboard_response_items(paginated_dashboards) + return DashboardsResponse( - dashboards=paginated_dashboards, + dashboards=response_dashboards, total=total, page=page, page_size=page_size, diff --git a/backend/src/services/profile_service.py b/backend/src/services/profile_service.py index 46aba55..a2d7445 100644 --- a/backend/src/services/profile_service.py +++ b/backend/src/services/profile_service.py @@ -343,9 +343,30 @@ class ProfileService: return [] normalized: List[str] = [] for owner in owners: - token = self._normalize_username(str(owner or "")) - if token and token not in normalized: - normalized.append(token) + owner_candidates: List[Any] + if isinstance(owner, dict): + 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 # [/DEF:_normalize_owner_tokens:Function] # [/DEF:ProfileService:Class] diff --git a/specs/024-user-dashboard-filter/tasks.md b/specs/024-user-dashboard-filter/tasks.md index aaa483d..03859a8 100644 --- a/specs/024-user-dashboard-filter/tasks.md +++ b/specs/024-user-dashboard-filter/tasks.md @@ -218,4 +218,9 @@ Mandatory UX verification tasks are included at the end of each user story phase - [x] D009 Verify updated dashboards filter behavior with targeted backend tests including no-detail-fanout assertion - [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] D009 Verify updated dashboards filter behavior with targeted backend tests including no-detail-fanout assertion \ No newline at end of file +- [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) \ No newline at end of file