diff --git a/ddpui/api/charts_api.py b/ddpui/api/charts_api.py index 5681f16fc..03202d9d4 100644 --- a/ddpui/api/charts_api.py +++ b/ddpui/api/charts_api.py @@ -303,8 +303,12 @@ def list_charts( total_pages = (total + page_size - 1) // page_size # Ceiling division + favorited_chart_ids = ChartService.get_favorited_chart_ids( + [chart.id for chart in charts], orguser + ) access_map = get_user_access_map(orguser, ResourceType.CHART, charts) + # Build response for each chart chart_responses = [ ChartResponse( id=chart.id, @@ -317,6 +321,7 @@ def list_charts( extra_config=chart.extra_config, created_at=chart.created_at, updated_at=chart.updated_at, + is_favorite=chart.id in favorited_chart_ids, access_level=access_map.get(chart.id), is_private=chart.is_private, ) @@ -982,6 +987,7 @@ def get_chart(request, chart_id: int): updated_at=chart.updated_at, access_level=request.access_level, is_private=chart.is_private, + is_favorite=ChartService.is_chart_favorited(chart.id, orguser), ) @@ -1193,6 +1199,7 @@ def update_chart(request, chart_id: int, payload: ChartUpdate): updated_at=chart.updated_at, access_level=request.access_level, is_private=chart.is_private, + is_favorite=ChartService.is_chart_favorited(chart.id, orguser), ) @@ -1282,3 +1289,37 @@ def get_chart_dashboards(request, chart_id: int): raise HttpError(404, "Chart not found") from None return dashboards + + +@charts_router.post("/{chart_id}/favorite/", response=dict) +@has_permission(["can_view_charts"]) +@has_access( + ResourceType.CHART, AccessLevel.VIEW, get_resource_id=lambda kwargs: kwargs.get("chart_id") +) +def favorite_chart(request, chart_id: int): + """Mark a chart as favorited by the current user""" + orguser: OrgUser = request.orguser + + try: + ChartService.favorite_chart(chart_id, orguser.org, orguser) + except ChartNotFoundError: + raise HttpError(404, "Chart not found") from None + + return {"is_favorite": True} + + +@charts_router.delete("/{chart_id}/favorite/", response=dict) +@has_permission(["can_view_charts"]) +@has_access( + ResourceType.CHART, AccessLevel.VIEW, get_resource_id=lambda kwargs: kwargs.get("chart_id") +) +def unfavorite_chart(request, chart_id: int): + """Remove the current user's favorite on a chart""" + orguser: OrgUser = request.orguser + + try: + ChartService.unfavorite_chart(chart_id, orguser.org, orguser) + except ChartNotFoundError: + raise HttpError(404, "Chart not found") from None + + return {"is_favorite": False} diff --git a/ddpui/api/dashboard_native_api.py b/ddpui/api/dashboard_native_api.py index f0f5116ee..11dbeaf33 100644 --- a/ddpui/api/dashboard_native_api.py +++ b/ddpui/api/dashboard_native_api.py @@ -75,9 +75,18 @@ def list_dashboards( orguser=orguser, ) + favorited_dashboard_ids = DashboardService.get_favorited_dashboard_ids( + [d.id for d in dashboards], orguser + ) levels = access_control.get_user_access_map(orguser, ResourceType.DASHBOARD, dashboards) + return [ - DashboardResponse(**DashboardService.get_dashboard_response(d), access_level=levels[d.pk]) + DashboardResponse( + **DashboardService.get_dashboard_response( + d, is_favorite=d.id in favorited_dashboard_ids + ), + access_level=levels[d.pk], + ) for d in dashboards ] @@ -99,7 +108,11 @@ def get_dashboard(request, dashboard_id: int): raise HttpError(404, "Dashboard not found") from err return DashboardResponse( - **DashboardService.get_dashboard_response(dashboard), access_level=request.access_level + **DashboardService.get_dashboard_response( + dashboard, + is_favorite=DashboardService.is_dashboard_favorited(dashboard.id, orguser), + ), + access_level=request.access_level, ) @@ -230,7 +243,11 @@ def _blank_normalized(value): ) return DashboardResponse( - **DashboardService.get_dashboard_response(dashboard), access_level=request.access_level + **DashboardService.get_dashboard_response( + dashboard, + is_favorite=DashboardService.is_dashboard_favorited(dashboard.id, orguser), + ), + access_level=request.access_level, ) @@ -721,3 +738,41 @@ def resolve_user_landing_page(request): # 3. No landing page set return {"dashboard_id": None, "dashboard_title": None, "dashboard_type": None, "source": "none"} + + +@dashboard_native_router.post("/{dashboard_id}/favorite/", response=dict) +@has_permission(["can_view_dashboards"]) +@has_access( + ResourceType.DASHBOARD, + AccessLevel.VIEW, + get_resource_id=lambda kwargs: kwargs.get("dashboard_id"), +) +def favorite_dashboard(request, dashboard_id: int): + """Mark a dashboard as favorited by the current user""" + orguser: OrgUser = request.orguser + + try: + DashboardService.favorite_dashboard(dashboard_id, orguser.org, orguser) + except DashboardNotFoundError as err: + raise HttpError(404, "Dashboard not found") from err + + return {"is_favorite": True} + + +@dashboard_native_router.delete("/{dashboard_id}/favorite/", response=dict) +@has_permission(["can_view_dashboards"]) +@has_access( + ResourceType.DASHBOARD, + AccessLevel.VIEW, + get_resource_id=lambda kwargs: kwargs.get("dashboard_id"), +) +def unfavorite_dashboard(request, dashboard_id: int): + """Remove the current user's favorite on a dashboard""" + orguser: OrgUser = request.orguser + + try: + DashboardService.unfavorite_dashboard(dashboard_id, orguser.org, orguser) + except DashboardNotFoundError as err: + raise HttpError(404, "Dashboard not found") from err + + return {"is_favorite": False} diff --git a/ddpui/migrations/0181_favorite.py b/ddpui/migrations/0181_favorite.py new file mode 100644 index 000000000..95aed59c9 --- /dev/null +++ b/ddpui/migrations/0181_favorite.py @@ -0,0 +1,61 @@ +# Generated by Django 4.2 on 2026-09-02 06:06 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("ddpui", "0180_remove_orguser_has_seen_rbac_notice_and_more"), + ] + + operations = [ + migrations.CreateModel( + name="Favorite", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "resource_type", + models.CharField( + choices=[ + ("dashboard", "Dashboard"), + ("chart", "Chart"), + ("report", "Report"), + ("kpi", "KPI"), + ], + max_length=20, + ), + ), + ("resource_id", models.BigIntegerField()), + ("created_at", models.DateTimeField(auto_now_add=True)), + ( + "org_user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="favorites", + to="ddpui.orguser", + ), + ), + ], + options={ + "db_table": "favorite", + }, + ), + migrations.AddIndex( + model_name="favorite", + index=models.Index( + fields=["resource_type", "resource_id"], name="favorite_resourc_ad1413_idx" + ), + ), + migrations.AddConstraint( + model_name="favorite", + constraint=models.UniqueConstraint( + fields=("org_user", "resource_type", "resource_id"), name="unique_favorite" + ), + ), + ] diff --git a/ddpui/models/favorite.py b/ddpui/models/favorite.py new file mode 100644 index 000000000..6cca3b141 --- /dev/null +++ b/ddpui/models/favorite.py @@ -0,0 +1,34 @@ +"""Shared favorite model for charts, dashboards, and other favoritable resources. + +Mirrors Apache Superset's own `favstar` table design (superset/models/core.py): +one shared table keyed by (user, resource_type, resource_id) instead of a +separate table per resource type. +""" + +from django.db import models +from ddpui.models.org_user import OrgUser +from ddpui.models.resource_share import ResourceType + + +class Favorite(models.Model): + """Tracks which org users have favorited which resources. Favoriting is + personal — one user's favorite has no effect on any other user.""" + + org_user = models.ForeignKey(OrgUser, on_delete=models.CASCADE, related_name="favorites") + resource_type = models.CharField(max_length=20, choices=ResourceType.choices) + resource_id = models.BigIntegerField() + created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + db_table = "favorite" + constraints = [ + models.UniqueConstraint( + fields=["org_user", "resource_type", "resource_id"], name="unique_favorite" + ) + ] + indexes = [ + models.Index(fields=["resource_type", "resource_id"]), + ] + + def __str__(self): + return f"{self.org_user.user.email} favorited {self.resource_type} {self.resource_id}" diff --git a/ddpui/schemas/chart_schemas/crud.py b/ddpui/schemas/chart_schemas/crud.py index 05f6f40e0..300b16552 100644 --- a/ddpui/schemas/chart_schemas/crud.py +++ b/ddpui/schemas/chart_schemas/crud.py @@ -71,6 +71,7 @@ class ChartResponse(Schema): extra_config: dict created_at: datetime updated_at: datetime + is_favorite: bool = False access_level: Optional[str] = None # "view" | "edit"; None for admins/owners (implicit edit) is_private: bool = False diff --git a/ddpui/schemas/dashboard_schema.py b/ddpui/schemas/dashboard_schema.py index 02cfac93a..77d0f07cd 100644 --- a/ddpui/schemas/dashboard_schema.py +++ b/ddpui/schemas/dashboard_schema.py @@ -78,6 +78,7 @@ class DashboardResponse(Schema): published_at: Optional[datetime] = None is_locked: bool = False locked_by: Optional[str] = None + is_favorite: bool = False created_by: Optional[str] = None # creator's email; None if the creator was deleted org_id: int last_modified_by: Optional[str] = None diff --git a/ddpui/services/chart_service.py b/ddpui/services/chart_service.py index 1d55eab3d..f89ad9a39 100644 --- a/ddpui/services/chart_service.py +++ b/ddpui/services/chart_service.py @@ -4,9 +4,10 @@ separating it from the API layer for better testability and maintainability. """ -from typing import Dict, List, Optional, Tuple, Any +from typing import Dict, List, Optional, Set, Tuple, Any from dataclasses import dataclass +from django.db import transaction from django.db.models import Q from ddpui.core.access.access_control import accessible_filter @@ -16,6 +17,7 @@ from ddpui.models.org import Org from ddpui.models.org_user import OrgUser from ddpui.models.dashboard import Dashboard, DashboardComponentType +from ddpui.services.favorite_service import FavoriteService from ddpui.utils.custom_logger import CustomLogger logger = CustomLogger("ddpui.chart_service") @@ -240,7 +242,9 @@ def delete_chart(chart_id: int, org: Org, orguser: OrgUser) -> str: raise ChartPermissionError("Only the owner or an admin can delete this chart.") chart_title = chart.title - chart.delete() + with transaction.atomic(): + chart.delete() + FavoriteService.remove_favorites_for_resource(ResourceType.CHART, chart_id) logger.info(f"Deleted chart '{chart_title}' (id={chart_id}) by {orguser.user.email}") return chart_title @@ -284,8 +288,11 @@ def bulk_delete_charts(chart_ids: List[int], org: Org, orguser: OrgUser) -> Dict f"Charts not deletable by {orguser.user.email} (not owner or admin): {forbidden_ids}" ) + deletable_ids = [chart.id for chart in deletable] deleted_titles = [chart.title for chart in deletable] - deleted_count = Chart.objects.filter(id__in=[chart.id for chart in deletable]).delete()[0] + with transaction.atomic(): + deleted_count = Chart.objects.filter(id__in=deletable_ids).delete()[0] + FavoriteService.remove_favorites_for_resources(ResourceType.CHART, deletable_ids) logger.info(f"Bulk deleted {deleted_count} charts by {orguser.user.email}") @@ -336,3 +343,21 @@ def get_chart_dashboards(chart_id: int, org: Org) -> List[Dict[str, Any]]: break return dashboards_with_chart + + @staticmethod + def favorite_chart(chart_id: int, org: Org, orguser: OrgUser) -> None: + ChartService.get_chart(chart_id, org) # raises ChartNotFoundError if not in org + FavoriteService.add_favorite(ResourceType.CHART, chart_id, orguser) + + @staticmethod + def unfavorite_chart(chart_id: int, org: Org, orguser: OrgUser) -> None: + ChartService.get_chart(chart_id, org) # raises ChartNotFoundError if not in org + FavoriteService.remove_favorite(ResourceType.CHART, chart_id, orguser) + + @staticmethod + def get_favorited_chart_ids(chart_ids: List[int], orguser: OrgUser) -> Set[int]: + return FavoriteService.get_favorited_ids(ResourceType.CHART, chart_ids, orguser) + + @staticmethod + def is_chart_favorited(chart_id: int, orguser: OrgUser) -> bool: + return FavoriteService.is_favorited(ResourceType.CHART, chart_id, orguser) diff --git a/ddpui/services/dashboard_service.py b/ddpui/services/dashboard_service.py index dd592e76d..f2b563a9a 100644 --- a/ddpui/services/dashboard_service.py +++ b/ddpui/services/dashboard_service.py @@ -4,7 +4,7 @@ separating it from the API layer for better testability and maintainability. """ -from typing import Dict, List, Optional, Any, Union, Tuple +from typing import Dict, List, Optional, Any, Set, Union, Tuple from datetime import datetime, timedelta from dataclasses import dataclass import json @@ -12,6 +12,7 @@ import uuid from django.core.cache import cache +from django.db import transaction from django.db.models import Q from django.utils import timezone from sqlalchemy import text, distinct, column @@ -30,6 +31,7 @@ from ddpui.models.org import Org, OrgWarehouse from ddpui.models.org_user import OrgUser from ddpui.models.visualization import Chart +from ddpui.services.favorite_service import FavoriteService from ddpui.utils.warehouse.client.warehouse_factory import WarehouseFactory from ddpui.utils.warehouse.client.warehouse_interface import Warehouse from ddpui.core.charts.charts_service import ( @@ -240,11 +242,12 @@ def get_dashboard(dashboard_id: int, org: Org) -> Dashboard: raise DashboardNotFoundError(dashboard_id) @staticmethod - def get_dashboard_response(dashboard: Dashboard) -> Dict[str, Any]: + def get_dashboard_response(dashboard: Dashboard, is_favorite: bool = False) -> Dict[str, Any]: """Convert dashboard model to response dict. Args: dashboard: The dashboard instance + is_favorite: Whether the requesting user has favorited this dashboard Returns: Dictionary containing dashboard response data @@ -256,6 +259,7 @@ def get_dashboard_response(dashboard: Dashboard) -> Dict[str, Any]: response_data["locked_by"] = ( lock.locked_by.user.email if lock and not lock.is_expired() else None ) + response_data["is_favorite"] = is_favorite # Add filters without position data in settings filters_data = [] @@ -309,6 +313,24 @@ def list_dashboards( return list(Dashboard.objects.filter(query).order_by("-updated_at")) + @staticmethod + def favorite_dashboard(dashboard_id: int, org: Org, orguser: OrgUser) -> None: + DashboardService.get_dashboard(dashboard_id, org) # raises if not in org + FavoriteService.add_favorite(ResourceType.DASHBOARD, dashboard_id, orguser) + + @staticmethod + def unfavorite_dashboard(dashboard_id: int, org: Org, orguser: OrgUser) -> None: + DashboardService.get_dashboard(dashboard_id, org) # raises if not in org + FavoriteService.remove_favorite(ResourceType.DASHBOARD, dashboard_id, orguser) + + @staticmethod + def get_favorited_dashboard_ids(dashboard_ids: List[int], orguser: OrgUser) -> Set[int]: + return FavoriteService.get_favorited_ids(ResourceType.DASHBOARD, dashboard_ids, orguser) + + @staticmethod + def is_dashboard_favorited(dashboard_id: int, orguser: OrgUser) -> bool: + return FavoriteService.is_favorited(ResourceType.DASHBOARD, dashboard_id, orguser) + @staticmethod def create_dashboard(data: DashboardData, orguser: OrgUser) -> Dashboard: """Create a new dashboard. @@ -1120,7 +1142,9 @@ def delete_dashboard_safely(dashboard_id: int, orguser: OrgUser) -> tuple[bool, # Delete the dashboard dashboard_title = dashboard.title - dashboard.delete() + with transaction.atomic(): + dashboard.delete() + FavoriteService.remove_favorites_for_resource(ResourceType.DASHBOARD, dashboard_id) logger.info(f"Dashboard '{dashboard_title}' deleted by {orguser.user.email}") return True, "" diff --git a/ddpui/services/favorite_service.py b/ddpui/services/favorite_service.py new file mode 100644 index 000000000..247786910 --- /dev/null +++ b/ddpui/services/favorite_service.py @@ -0,0 +1,90 @@ +"""Shared favorite service for charts, dashboards, and other favoritable resources. + +Mirrors Superset's own favorite DAO pattern (superset/daos/chart.py, +superset/daos/dashboard.py): favorite/unfavorite/favorited_ids, scoped to the +current user, against one shared table rather than one per resource type. +""" + +from typing import List, Set + +from ddpui.models.favorite import Favorite +from ddpui.models.org_user import OrgUser +from ddpui.models.resource_share import ResourceType + +# ResourceType also covers REPORT and KPI (for access control elsewhere), but +# favoriting itself is only wired up for these two — enforced here rather than +# left implicit, since FavoriteService has no other guard against a future +# caller favoriting a resource type nothing validates existence for. +SUPPORTED_RESOURCE_TYPES = {ResourceType.CHART, ResourceType.DASHBOARD} + + +def _assert_supported(resource_type: ResourceType) -> None: + assert ( + resource_type in SUPPORTED_RESOURCE_TYPES + ), f"Favoriting is not supported for resource type '{resource_type}'" + + +class FavoriteService: + """Service class for favorite operations, shared across resource types""" + + @staticmethod + def add_favorite(resource_type: ResourceType, resource_id: int, orguser: OrgUser) -> None: + """Mark a resource as favorited by this user""" + _assert_supported(resource_type) + Favorite.objects.get_or_create( + org_user=orguser, resource_type=resource_type.value, resource_id=resource_id + ) + + @staticmethod + def remove_favorite(resource_type: ResourceType, resource_id: int, orguser: OrgUser) -> None: + """Remove this user's favorite on a resource, if any""" + _assert_supported(resource_type) + Favorite.objects.filter( + org_user=orguser, resource_type=resource_type.value, resource_id=resource_id + ).delete() + + @staticmethod + def is_favorited(resource_type: ResourceType, resource_id: int, orguser: OrgUser) -> bool: + """Whether this user has favorited a single resource. Single-resource + counterpart to get_favorited_ids, for detail/update responses.""" + _assert_supported(resource_type) + return Favorite.objects.filter( + org_user=orguser, resource_type=resource_type.value, resource_id=resource_id + ).exists() + + @staticmethod + def get_favorited_ids( + resource_type: ResourceType, resource_ids: List[int], orguser: OrgUser + ) -> Set[int]: + """Return the subset of resource_ids this user has favorited""" + _assert_supported(resource_type) + if not resource_ids: + return set() + return set( + Favorite.objects.filter( + org_user=orguser, + resource_type=resource_type.value, + resource_id__in=resource_ids, + ).values_list("resource_id", flat=True) + ) + + @staticmethod + def remove_favorites_for_resource(resource_type: ResourceType, resource_id: int) -> None: + """Delete every user's favorite on a resource. resource_id isn't a real + ForeignKey (it's generic across resource types), so there's no DB-level + cascade when the chart/dashboard itself is deleted — callers must call + this explicitly at the point of deletion to avoid orphaned rows.""" + _assert_supported(resource_type) + Favorite.objects.filter(resource_type=resource_type.value, resource_id=resource_id).delete() + + @staticmethod + def remove_favorites_for_resources( + resource_type: ResourceType, resource_ids: List[int] + ) -> None: + """Bulk variant of remove_favorites_for_resource, for bulk-delete flows.""" + _assert_supported(resource_type) + if not resource_ids: + return + Favorite.objects.filter( + resource_type=resource_type.value, resource_id__in=resource_ids + ).delete() diff --git a/ddpui/tests/api_tests/test_charts_api.py b/ddpui/tests/api_tests/test_charts_api.py index 798896bf4..a02c5f9b5 100644 --- a/ddpui/tests/api_tests/test_charts_api.py +++ b/ddpui/tests/api_tests/test_charts_api.py @@ -27,6 +27,7 @@ from ddpui.models.role_based_access import Role from ddpui.models.visualization import Chart from ddpui.models.dashboard import Dashboard, DashboardComponentType +from ddpui.models.favorite import Favorite from ddpui.auth import ACCOUNT_MANAGER_ROLE from ddpui.api.charts_api import ( list_charts, @@ -37,6 +38,8 @@ bulk_delete_charts, get_chart_dashboards, get_chart_data, + favorite_chart, + unfavorite_chart, get_map_data_overlay, download_chart_data_csv, BulkDeleteRequest, @@ -643,6 +646,160 @@ def test_get_chart_dashboards_not_found(self, orguser, seed_db): assert excinfo.value.status_code == 404 +def _chart_is_favorite(request, chart_id): + """Read is_favorite off list_charts, the endpoint the star UI actually renders from.""" + response = list_charts(request) + return next(c for c in response.data if c.id == chart_id).is_favorite + + +class TestFavoriteChart: + """Tests for favorite_chart / unfavorite_chart endpoints""" + + def test_favorite_chart_success(self, orguser, sample_chart, seed_db): + """Favoriting a chart returns is_favorite=True and is reflected on list_charts""" + request = mock_request(orguser) + + response = favorite_chart(request, chart_id=sample_chart.id) + + assert response == {"is_favorite": True} + assert _chart_is_favorite(request, sample_chart.id) is True + + def test_favorite_chart_idempotent(self, orguser, sample_chart, seed_db): + """Favoriting an already-favorited chart doesn't error or duplicate""" + request = mock_request(orguser) + + favorite_chart(request, chart_id=sample_chart.id) + response = favorite_chart(request, chart_id=sample_chart.id) + + assert response == {"is_favorite": True} + + def test_unfavorite_chart_success(self, orguser, sample_chart, seed_db): + """Unfavoriting a chart returns is_favorite=False and is reflected on list_charts""" + request = mock_request(orguser) + favorite_chart(request, chart_id=sample_chart.id) + + response = unfavorite_chart(request, chart_id=sample_chart.id) + + assert response == {"is_favorite": False} + assert _chart_is_favorite(request, sample_chart.id) is False + + def test_unfavorite_chart_not_favorited(self, orguser, sample_chart, seed_db): + """Unfavoriting a chart that was never favorited is a no-op, not an error""" + request = mock_request(orguser) + + response = unfavorite_chart(request, chart_id=sample_chart.id) + + assert response == {"is_favorite": False} + + def test_favorite_chart_not_found(self, orguser, seed_db): + """Favoriting a non-existent chart returns 404""" + request = mock_request(orguser) + + with pytest.raises(HttpError) as excinfo: + favorite_chart(request, chart_id=99999) + + assert excinfo.value.status_code == 404 + + def test_favorite_is_per_user(self, orguser, sample_chart, org, seed_db): + """One user's favorite has no effect on another user's view of the same chart""" + other_user = User.objects.create(username="otherfavuser", email="otherfav@test.com") + other_orguser = OrgUser.objects.create( + user=other_user, + org=org, + new_role=Role.objects.filter(slug=ACCOUNT_MANAGER_ROLE).first(), + ) + + favorite_chart(mock_request(orguser), chart_id=sample_chart.id) + + assert _chart_is_favorite(mock_request(orguser), sample_chart.id) is True + assert _chart_is_favorite(mock_request(other_orguser), sample_chart.id) is False + + # Cleanup + other_orguser.delete() + other_user.delete() + + def test_list_charts_reflects_favorite(self, orguser, sample_chart, seed_db): + """list_charts marks only the favorited chart as is_favorite""" + request = mock_request(orguser) + favorite_chart(request, chart_id=sample_chart.id) + + response = list_charts(request) + + favorited = [c for c in response.data if c.id == sample_chart.id] + assert len(favorited) == 1 + assert favorited[0].is_favorite is True + + def test_get_chart_reflects_favorite(self, orguser, sample_chart, seed_db): + """get_chart reports the user's real favorite state, not the schema default""" + request = mock_request(orguser) + + assert get_chart(request, chart_id=sample_chart.id).is_favorite is False + + favorite_chart(request, chart_id=sample_chart.id) + + assert get_chart(request, chart_id=sample_chart.id).is_favorite is True + + def test_get_chart_favorite_is_per_user(self, orguser, sample_chart, org, seed_db): + """get_chart scopes is_favorite to the requesting user""" + other_user = User.objects.create(username="otherdetailuser", email="otherdetail@test.com") + other_orguser = OrgUser.objects.create( + user=other_user, + org=org, + new_role=Role.objects.filter(slug=ACCOUNT_MANAGER_ROLE).first(), + ) + + favorite_chart(mock_request(orguser), chart_id=sample_chart.id) + + assert get_chart(mock_request(orguser), chart_id=sample_chart.id).is_favorite is True + assert get_chart(mock_request(other_orguser), chart_id=sample_chart.id).is_favorite is False + + # Cleanup + other_orguser.delete() + other_user.delete() + + def test_delete_chart_removes_favorite_rows(self, orguser, sample_chart, seed_db): + """Deleting a chart cleans up its Favorite rows instead of orphaning them""" + chart_id = sample_chart.id + favorite_chart(mock_request(orguser), chart_id=chart_id) + assert Favorite.objects.filter(resource_type="chart", resource_id=chart_id).exists() + + delete_chart(mock_request(orguser), chart_id=chart_id) + + assert not Favorite.objects.filter(resource_type="chart", resource_id=chart_id).exists() + + def test_bulk_delete_charts_removes_favorite_rows(self, orguser, org, seed_db): + """Bulk-deleting charts cleans up their Favorite rows too""" + chart_a = Chart.objects.create( + title="Bulk Fav Chart A", + chart_type="bar", + schema_name="public", + table_name="test", + extra_config={}, + created_by=orguser, + last_modified_by=orguser, + org=org, + ) + chart_b = Chart.objects.create( + title="Bulk Fav Chart B", + chart_type="bar", + schema_name="public", + table_name="test", + extra_config={}, + created_by=orguser, + last_modified_by=orguser, + org=org, + ) + request = mock_request(orguser) + favorite_chart(request, chart_id=chart_a.id) + favorite_chart(request, chart_id=chart_b.id) + + bulk_delete_charts(request, BulkDeleteRequest(chart_ids=[chart_a.id, chart_b.id])) + + assert not Favorite.objects.filter( + resource_type="chart", resource_id__in=[chart_a.id, chart_b.id] + ).exists() + + # ================================================================================ # Test get_chart_data endpoint # ================================================================================ diff --git a/ddpui/tests/api_tests/test_dashboard_native_api.py b/ddpui/tests/api_tests/test_dashboard_native_api.py index 06da6592b..55243666c 100644 --- a/ddpui/tests/api_tests/test_dashboard_native_api.py +++ b/ddpui/tests/api_tests/test_dashboard_native_api.py @@ -27,6 +27,7 @@ from ddpui.models.role_based_access import Role from ddpui.models.dashboard import Dashboard, DashboardFilter from ddpui.models.visualization import Chart +from ddpui.models.favorite import Favorite from ddpui.auth import ACCOUNT_MANAGER_ROLE, ANALYST_ROLE from ddpui.api.dashboard_native_api import ( list_dashboards, @@ -38,6 +39,8 @@ create_filter, update_filter, delete_filter, + favorite_dashboard, + unfavorite_dashboard, set_personal_landing_dashboard, set_org_default_dashboard, ) @@ -271,6 +274,148 @@ def test_get_dashboard_wrong_org(self, orguser, seed_db): other_org.delete() +def _dashboard_is_favorite(request, dashboard_id): + """Read is_favorite off list_dashboards, the endpoint the star UI actually renders from.""" + response = list_dashboards(request) + return next(d for d in response if d.id == dashboard_id).is_favorite + + +class TestFavoriteDashboard: + """Tests for favorite_dashboard / unfavorite_dashboard endpoints""" + + def test_favorite_dashboard_success(self, orguser, sample_dashboard, seed_db): + """Favoriting a dashboard returns is_favorite=True and is reflected on list_dashboards""" + request = mock_request(orguser) + + response = favorite_dashboard(request, dashboard_id=sample_dashboard.id) + + assert response == {"is_favorite": True} + assert _dashboard_is_favorite(request, sample_dashboard.id) is True + + def test_favorite_dashboard_idempotent(self, orguser, sample_dashboard, seed_db): + """Favoriting an already-favorited dashboard doesn't error or duplicate""" + request = mock_request(orguser) + + favorite_dashboard(request, dashboard_id=sample_dashboard.id) + response = favorite_dashboard(request, dashboard_id=sample_dashboard.id) + + assert response == {"is_favorite": True} + + def test_unfavorite_dashboard_success(self, orguser, sample_dashboard, seed_db): + """Unfavoriting a dashboard returns is_favorite=False and is reflected on list_dashboards""" + request = mock_request(orguser) + favorite_dashboard(request, dashboard_id=sample_dashboard.id) + + response = unfavorite_dashboard(request, dashboard_id=sample_dashboard.id) + + assert response == {"is_favorite": False} + assert _dashboard_is_favorite(request, sample_dashboard.id) is False + + def test_unfavorite_dashboard_not_favorited(self, orguser, sample_dashboard, seed_db): + """Unfavoriting a dashboard that was never favorited is a no-op, not an error""" + request = mock_request(orguser) + + response = unfavorite_dashboard(request, dashboard_id=sample_dashboard.id) + + assert response == {"is_favorite": False} + + def test_favorite_dashboard_not_found(self, orguser, seed_db): + """Favoriting a non-existent dashboard returns 404""" + request = mock_request(orguser) + + with pytest.raises(HttpError) as excinfo: + favorite_dashboard(request, dashboard_id=99999) + + assert excinfo.value.status_code == 404 + + def test_favorite_is_per_user(self, orguser, sample_dashboard, org, seed_db): + """One user's favorite has no effect on another user's view of the same dashboard""" + other_user = User.objects.create(username="otherfavuser", email="otherfav@test.com") + other_orguser = OrgUser.objects.create( + user=other_user, + org=org, + new_role=Role.objects.filter(slug=ACCOUNT_MANAGER_ROLE).first(), + ) + + favorite_dashboard(mock_request(orguser), dashboard_id=sample_dashboard.id) + + assert _dashboard_is_favorite(mock_request(orguser), sample_dashboard.id) is True + assert _dashboard_is_favorite(mock_request(other_orguser), sample_dashboard.id) is False + + # Cleanup + other_orguser.delete() + other_user.delete() + + def test_list_dashboards_reflects_favorite(self, orguser, sample_dashboard, seed_db): + """list_dashboards marks only the favorited dashboard as is_favorite""" + request = mock_request(orguser) + favorite_dashboard(request, dashboard_id=sample_dashboard.id) + + response = list_dashboards(request) + + favorited = [d for d in response if d.id == sample_dashboard.id] + assert len(favorited) == 1 + assert favorited[0].is_favorite is True + + def test_get_dashboard_reflects_favorite(self, orguser, sample_dashboard, seed_db): + """get_dashboard reports the user's real favorite state, not the schema default""" + request = mock_request(orguser) + + assert get_dashboard(request, dashboard_id=sample_dashboard.id).is_favorite is False + + favorite_dashboard(request, dashboard_id=sample_dashboard.id) + + assert get_dashboard(request, dashboard_id=sample_dashboard.id).is_favorite is True + + def test_get_dashboard_favorite_is_per_user(self, orguser, sample_dashboard, org, seed_db): + """get_dashboard scopes is_favorite to the requesting user""" + other_user = User.objects.create( + username="otherdashdetailuser", email="otherdashdetail@test.com" + ) + other_orguser = OrgUser.objects.create( + user=other_user, + org=org, + new_role=Role.objects.filter(slug=ACCOUNT_MANAGER_ROLE).first(), + ) + + favorite_dashboard(mock_request(orguser), dashboard_id=sample_dashboard.id) + + assert ( + get_dashboard(mock_request(orguser), dashboard_id=sample_dashboard.id).is_favorite + is True + ) + assert ( + get_dashboard(mock_request(other_orguser), dashboard_id=sample_dashboard.id).is_favorite + is False + ) + + # Cleanup + other_orguser.delete() + other_user.delete() + + def test_delete_dashboard_removes_favorite_rows(self, orguser, sample_dashboard, seed_db): + """Deleting a dashboard cleans up its Favorite rows instead of orphaning them""" + # Create another dashboard so we're not deleting the last one in the org + Dashboard.objects.create( + title="Another Dashboard", + dashboard_type="native", + grid_columns=12, + created_by=orguser, + org=orguser.org, + ) + + dashboard_id = sample_dashboard.id + request = mock_request(orguser) + favorite_dashboard(request, dashboard_id=dashboard_id) + assert Favorite.objects.filter(resource_type="dashboard", resource_id=dashboard_id).exists() + + delete_dashboard(request, dashboard_id=dashboard_id) + + assert not Favorite.objects.filter( + resource_type="dashboard", resource_id=dashboard_id + ).exists() + + # ================================================================================ # Test create_dashboard endpoint # ================================================================================