diff --git a/ddpui/api/dashboard_native_api.py b/ddpui/api/dashboard_native_api.py index d2cfa0370..c501b94db 100644 --- a/ddpui/api/dashboard_native_api.py +++ b/ddpui/api/dashboard_native_api.py @@ -1,6 +1,5 @@ """Native Dashboard API endpoints""" -import copy from typing import Optional, List from datetime import timedelta @@ -174,15 +173,12 @@ def duplicate_dashboard(request, dashboard_id: int): # Create a copy of the dashboard with transaction.atomic(): - # First create new dashboard WITHOUT layout_config and components (we'll update them later) new_dashboard = Dashboard.objects.create( title=f"Copy of {original_dashboard.title}", description=original_dashboard.description, dashboard_type=original_dashboard.dashboard_type, grid_columns=original_dashboard.grid_columns, target_screen_size=original_dashboard.target_screen_size, - layout_config=[], # Will be updated after filter duplication - components={}, # Will be updated after filter duplication created_by=orguser, org=orguser.org, last_modified_by=orguser, @@ -204,47 +200,6 @@ def duplicate_dashboard(request, dashboard_id: int): ) filter_id_mapping[str(original_filter.id)] = str(new_filter.id) - # Now update layout_config and components with new filter IDs - - # Deep copy the original data to avoid modifying it - new_layout_config = copy.deepcopy(original_dashboard.layout_config or []) - new_components = copy.deepcopy(original_dashboard.components or {}) - - # Update layout_config: change component IDs from "filter-{old_id}" to "filter-{new_id}" - for layout_item in new_layout_config: - item_id = layout_item.get("i", "") - if item_id.startswith("filter-"): - # Extract old filter ID and replace with new one - old_filter_id = item_id.replace("filter-", "") - if old_filter_id in filter_id_mapping: - new_filter_id = filter_id_mapping[old_filter_id] - layout_item["i"] = f"filter-{new_filter_id}" - - # Update components: change component keys and filterId references - updated_components = {} - for component_id, component_data in new_components.items(): - new_component_id = component_id - new_component_data = copy.deepcopy(component_data) - - # If this is a filter component - if component_id.startswith("filter-"): - old_filter_id = component_id.replace("filter-", "") - if old_filter_id in filter_id_mapping: - new_filter_id = filter_id_mapping[old_filter_id] - new_component_id = f"filter-{new_filter_id}" - - # Update the filterId reference in the component config - if ( - "config" in new_component_data - and "filterId" in new_component_data["config"] - ): - new_component_data["config"]["filterId"] = int(new_filter_id) - - updated_components[new_component_id] = new_component_data - - # Update the dashboard with the corrected layout_config, components, and tabs - new_dashboard.layout_config = new_layout_config - new_dashboard.components = updated_components new_dashboard.tabs = DashboardService.copy_tabs_with_filter_remapping( original_dashboard.tabs or [], filter_id_mapping ) @@ -422,8 +377,6 @@ def get_filter_options( raise HttpError(400, "No warehouse configured for organization") # Get filter options from service - from ddpui.services.dashboard_service import DashboardService - options = DashboardService.generate_filter_options( schema=schema_name, table=table_name, diff --git a/ddpui/core/reports/report_service.py b/ddpui/core/reports/report_service.py index 2be650692..8ea40c627 100644 --- a/ddpui/core/reports/report_service.py +++ b/ddpui/core/reports/report_service.py @@ -62,8 +62,6 @@ def _freeze_dashboard(dashboard: Dashboard) -> Dict[str, Any]: "description": dashboard.description, "grid_columns": dashboard.grid_columns, "target_screen_size": dashboard.target_screen_size, - "layout_config": dashboard.layout_config, - "components": dashboard.components, "tabs": dashboard.tabs, "filter_layout": dashboard.filter_layout, "filters": [f.to_json() for f in filters], @@ -71,7 +69,7 @@ def _freeze_dashboard(dashboard: Dashboard) -> Dict[str, Any]: @staticmethod def _extract_chart_ids(dashboard: Dashboard) -> List[int]: - """Extract chart IDs from tabs (new structure) and root components (backward compat).""" + """Extract chart IDs from tabs.""" chart_ids = [] for tab in dashboard.tabs or []: @@ -81,12 +79,6 @@ def _extract_chart_ids(dashboard: Dashboard) -> List[int]: if chart_id: chart_ids.append(chart_id) - for component in (dashboard.components or {}).values(): - if component.get("type") == "chart": - chart_id = component.get("config", {}).get("chartId") - if chart_id: - chart_ids.append(chart_id) - return list(set(chart_ids)) @staticmethod diff --git a/ddpui/management/commands/cleanup_frozen_dashboard_root_fields.py b/ddpui/management/commands/cleanup_frozen_dashboard_root_fields.py new file mode 100644 index 000000000..2aa680926 --- /dev/null +++ b/ddpui/management/commands/cleanup_frozen_dashboard_root_fields.py @@ -0,0 +1,86 @@ +from django.core.management.base import BaseCommand +from ddpui.models.report import ReportSnapshot + + +class Command(BaseCommand): + """ + Remove stale root-level layout_config and components keys from + ReportSnapshot.frozen_dashboard JSON blobs. + + These keys were left in place after the initial tabs migration for rollback + safety. All data now lives inside frozen_dashboard.tabs — the root-level + keys are no longer read and can be removed. + """ + + help = "Remove root-level layout_config and components from frozen_dashboard JSON" + + def add_arguments(self, parser): + """Add --dry-run argument to preview changes without applying them.""" + parser.add_argument( + "--dry-run", + action="store_true", + help="Preview changes without applying them", + ) + + def handle(self, *args, **options): + """Execute the cleanup, iterating all ReportSnapshot records in batches.""" + dry_run = options["dry_run"] + + if dry_run: + self.stdout.write("\n=== DRY RUN MODE: No changes will be made ===\n") + + cleaned_count = 0 + skipped_count = 0 + + snapshots_to_update = [] + + for snapshot in ReportSnapshot.objects.only("id", "frozen_dashboard").iterator( + chunk_size=1000 + ): + frozen = snapshot.frozen_dashboard + if not isinstance(frozen, dict): + skipped_count += 1 + continue + + has_layout = "layout_config" in frozen + has_components = "components" in frozen + + if not has_layout and not has_components: + skipped_count += 1 + continue + + cleaned_count += 1 + + if dry_run: + self.stdout.write( + f"[DRY RUN] Would clean - Snapshot ID: {snapshot.id}, " + f"keys to remove: {[k for k in ('layout_config', 'components') if k in frozen]}" + ) + else: + frozen.pop("layout_config", None) + frozen.pop("components", None) + snapshot.frozen_dashboard = frozen + snapshots_to_update.append(snapshot) + + if not dry_run and snapshots_to_update: + ReportSnapshot.objects.bulk_update( + snapshots_to_update, ["frozen_dashboard"], batch_size=500 + ) + for snapshot in snapshots_to_update: + self.stdout.write(f"Cleaned - Snapshot ID: {snapshot.id}") + + self.stdout.write("") + if dry_run: + self.stdout.write( + self.style.WARNING( + f"=== DRY RUN COMPLETE: {cleaned_count} snapshots would be cleaned, " + f"{skipped_count} skipped ===" + ) + ) + else: + self.stdout.write( + self.style.SUCCESS( + f"=== CLEANUP COMPLETE: {cleaned_count} snapshots cleaned, " + f"{skipped_count} skipped ===" + ) + ) diff --git a/ddpui/management/commands/migrate_dashboards_to_tabs.py b/ddpui/management/commands/migrate_dashboards_to_tabs.py deleted file mode 100644 index e1aa43428..000000000 --- a/ddpui/management/commands/migrate_dashboards_to_tabs.py +++ /dev/null @@ -1,95 +0,0 @@ -import time -from django.core.management.base import BaseCommand -from ddpui.models.dashboard import Dashboard - - -class Command(BaseCommand): - """ - Migrate existing dashboards to use tabs structure. - Moves layout_config and components from root level into a default tab. - """ - - help = "Migrate existing dashboards to tabs structure for all orgs" - - def add_arguments(self, parser): - parser.add_argument( - "--dry-run", - action="store_true", - help="Preview changes without applying them", - ) - - def handle(self, *args, **options): - dry_run = options["dry_run"] - - if dry_run: - self.stdout.write("\n=== DRY RUN MODE: No changes will be made ===\n") - - migrated_count = 0 - skipped_count = 0 - - for dashboard in Dashboard.objects.only( - "id", "title", "tabs", "layout_config", "components", "org" - ).iterator(chunk_size=1000): - # Skip if already has tabs - if dashboard.tabs and len(dashboard.tabs) > 0: - skipped_count += 1 - continue - - # Skip if no data to migrate - has_layout = dashboard.layout_config and len(dashboard.layout_config) > 0 - has_components = dashboard.components and len(dashboard.components) > 0 - - if not has_layout and not has_components: - skipped_count += 1 - continue - - # This dashboard needs migration - migrated_count += 1 - org_id = getattr(dashboard, "org_id", "N/A") - - if dry_run: - self.stdout.write( - f"[DRY RUN] Would migrate - " - f"Dashboard ID: {dashboard.id}, " - f"Title: {dashboard.title}, " - f"Org ID: {org_id}, " - f"Charts: {len(dashboard.layout_config or [])}" - ) - else: - # Create default tab with existing data - default_tab = { - "id": f"tab-{int(time.time() * 1000)}", - "title": "Untitled Tab 1", - "layout_config": dashboard.layout_config or [], - "components": dashboard.components or {}, - } - - # Update dashboard structure — only set tabs, leave layout_config - # and components intact for rollback safety. They will be removed - # from the DB in a follow-up cleanup PR after the release is stable. - dashboard.tabs = [default_tab] - dashboard.save(update_fields=["tabs"]) - - self.stdout.write( - f"Migrated - " - f"Dashboard ID: {dashboard.id}, " - f"Title: {dashboard.title}, " - f"Org ID: {org_id}, " - f"Charts: {len(default_tab['layout_config'])}" - ) - - self.stdout.write("") - if dry_run: - self.stdout.write( - self.style.WARNING( - f"=== DRY RUN COMPLETE: {migrated_count} dashboards would be migrated, " - f"{skipped_count} skipped ===" - ) - ) - else: - self.stdout.write( - self.style.SUCCESS( - f"=== MIGRATION COMPLETE: {migrated_count} dashboards migrated, " - f"{skipped_count} skipped ===" - ) - ) diff --git a/ddpui/management/commands/migrate_report_snapshots_to_tabs.py b/ddpui/management/commands/migrate_report_snapshots_to_tabs.py deleted file mode 100644 index b3fcf0ba7..000000000 --- a/ddpui/management/commands/migrate_report_snapshots_to_tabs.py +++ /dev/null @@ -1,107 +0,0 @@ -import time -from django.core.management.base import BaseCommand -from ddpui.models.report import ReportSnapshot - - -class Command(BaseCommand): - """ - Migrate existing report snapshots to use tabs structure. - Moves layout_config and components from root level of frozen_dashboard - into a default tab, matching the new dashboard tabs format. - """ - - help = "Migrate existing report snapshots to tabs structure for all orgs" - - def add_arguments(self, parser): - parser.add_argument( - "--dry-run", - action="store_true", - help="Preview changes without applying them", - ) - - def handle(self, *args, **options): - dry_run = options["dry_run"] - - if dry_run: - self.stdout.write("\n=== DRY RUN MODE: No changes will be made ===\n") - - migrated_count = 0 - skipped_count = 0 - - for snapshot in ReportSnapshot.objects.only( - "id", "title", "frozen_dashboard", "org" - ).iterator(chunk_size=1000): - frozen = snapshot.frozen_dashboard - if not isinstance(frozen, dict): - skipped_count += 1 - continue - - existing_tabs = frozen.get("tabs") or [] - - # Skip if already has tabs - if existing_tabs: - skipped_count += 1 - continue - - layout_config = frozen.get("layout_config") or [] - components = frozen.get("components") or {} - - # Skip if no root-level data to migrate - has_layout = bool(layout_config) - has_components = bool(components) - - if not has_layout and not has_components: - skipped_count += 1 - continue - - # This snapshot needs migration - migrated_count += 1 - org_id = getattr(snapshot, "org_id", "N/A") - - if dry_run: - self.stdout.write( - f"[DRY RUN] Would migrate - " - f"Snapshot ID: {snapshot.id}, " - f"Title: {snapshot.title}, " - f"Org ID: {org_id}, " - f"Layout items: {len(layout_config)}" - ) - else: - default_tab = { - "id": f"tab-{int(time.time() * 1000)}", - "title": "Untitled Tab 1", - "layout_config": layout_config, - "components": components, - } - - # Only add tabs — leave root-level layout_config and components intact - # for rollback safety. They will be removed in a follow-up cleanup PR - # after the release is confirmed stable. - frozen["tabs"] = [default_tab] - - snapshot.frozen_dashboard = frozen - snapshot.save(update_fields=["frozen_dashboard"]) - - self.stdout.write( - f"Migrated - " - f"Snapshot ID: {snapshot.id}, " - f"Title: {snapshot.title}, " - f"Org ID: {org_id}, " - f"Layout items: {len(default_tab['layout_config'])}" - ) - - self.stdout.write("") - if dry_run: - self.stdout.write( - self.style.WARNING( - f"=== DRY RUN COMPLETE: {migrated_count} snapshots would be migrated, " - f"{skipped_count} skipped ===" - ) - ) - else: - self.stdout.write( - self.style.SUCCESS( - f"=== MIGRATION COMPLETE: {migrated_count} snapshots migrated, " - f"{skipped_count} skipped ===" - ) - ) diff --git a/ddpui/migrations/0163_remove_dashboard_root_layout_components.py b/ddpui/migrations/0163_remove_dashboard_root_layout_components.py new file mode 100644 index 000000000..a4cf08c78 --- /dev/null +++ b/ddpui/migrations/0163_remove_dashboard_root_layout_components.py @@ -0,0 +1,27 @@ +""" +Schema migration: drop layout_config and components columns from the dashboard table. + +All dashboard data now lives inside the tabs JSON array. The root-level columns +were kept for rollback safety after the initial tabs migration; this PR removes them. +""" + +from django.db import migrations + + +class Migration(migrations.Migration): + """Drop deprecated root-level layout_config and components columns from dashboard table.""" + + dependencies = [ + ("ddpui", "0162_kpi_last_modified_by_metric_last_modified_by"), + ] + + operations = [ + migrations.RemoveField( + model_name="dashboard", + name="layout_config", + ), + migrations.RemoveField( + model_name="dashboard", + name="components", + ), + ] diff --git a/ddpui/models/dashboard.py b/ddpui/models/dashboard.py index 95bc2a424..b08717e75 100644 --- a/ddpui/models/dashboard.py +++ b/ddpui/models/dashboard.py @@ -65,12 +65,6 @@ class Dashboard(models.Model): help_text="Target screen size for dashboard design", ) - # Layout configuration stored as JSON - layout_config = models.JSONField(default=list, help_text="Grid layout positions and sizes") - - # Components configuration - components = models.JSONField(default=dict, help_text="Dashboard components configuration") - # Tabs configuration - each tab contains its own layout_config and components tabs = models.JSONField( default=list, help_text="Array of tab objects: [{id, title, layout_config, components}]" @@ -138,8 +132,6 @@ def to_json(self): "dashboard_type": self.dashboard_type, "grid_columns": self.grid_columns, "target_screen_size": self.target_screen_size, - "layout_config": self.layout_config, - "components": self.components, "tabs": self.tabs or [], "filter_layout": self.filter_layout, "is_published": self.is_published, diff --git a/ddpui/models/report.py b/ddpui/models/report.py index c0a03615b..18c4289bc 100644 --- a/ddpui/models/report.py +++ b/ddpui/models/report.py @@ -35,25 +35,16 @@ class ReportSnapshot(models.Model): help_text="End of reporting period (inclusive). NULL for snapshots without date filtering.", ) - # --- FROZEN DATA (2 layers) --- - - # Layer 1: Dashboard layout, structure & filters - # Contains: title, description, grid_columns, target_screen_size, - # layout_config, components, filter_layout, filters frozen_dashboard = models.JSONField( default=dict, help_text="Frozen dashboard config + filters at snapshot time", ) - # Layer 2: Full chart configs keyed by chart_id - # {str(chart_id): {id, title, description, chart_type, schema_name, table_name, extra_config}} - # CRITICAL: ensures snapshots survive chart deletion or editing frozen_chart_configs = models.JSONField( default=dict, help_text="Frozen chart configs keyed by chart_id", ) - # User-editable summary (the ONLY mutable field) summary = models.TextField( blank=True, null=True, diff --git a/ddpui/schemas/dashboard_schema.py b/ddpui/schemas/dashboard_schema.py index 7adb916a5..56a352704 100644 --- a/ddpui/schemas/dashboard_schema.py +++ b/ddpui/schemas/dashboard_schema.py @@ -73,8 +73,6 @@ class DashboardResponse(Schema): grid_columns: int target_screen_size: str filter_layout: str - layout_config: list[dict] - components: dict tabs: List[DashboardTabSchema] = [] # Array of tabs is_published: bool published_at: Optional[datetime] = None diff --git a/ddpui/schemas/report_schema.py b/ddpui/schemas/report_schema.py index dd12c455d..0d9e43316 100644 --- a/ddpui/schemas/report_schema.py +++ b/ddpui/schemas/report_schema.py @@ -27,8 +27,6 @@ class FrozenDashboardConfig(Schema): description: Optional[str] = None grid_columns: Optional[int] = None target_screen_size: Optional[str] = None - layout_config: Optional[Any] = None - components: Optional[Dict[str, Any]] = None tabs: Optional[List[Dict[str, Any]]] = None filter_layout: Optional[str] = None filters: List[Dict[str, Any]] = [] diff --git a/ddpui/services/chart_service.py b/ddpui/services/chart_service.py index 179e06701..b7d570aff 100644 --- a/ddpui/services/chart_service.py +++ b/ddpui/services/chart_service.py @@ -323,8 +323,9 @@ def get_chart_dashboards(chart_id: int, org: Org) -> List[Dict[str, Any]]: dashboards = Dashboard.objects.filter(org=org) for dashboard in dashboards: - if dashboard.components: - for component_id, component in dashboard.components.items(): + found = False + for tab in dashboard.tabs or []: + for component in (tab.get("components") or {}).values(): if ( component.get("type") == DashboardComponentType.CHART.value and component.get("config", {}).get("chartId") == chart_id @@ -336,6 +337,9 @@ def get_chart_dashboards(chart_id: int, org: Org) -> List[Dict[str, Any]]: "dashboard_type": dashboard.dashboard_type, } ) - break # Found chart in this dashboard + found = True + break + if found: + break return dashboards_with_chart diff --git a/ddpui/services/dashboard_service.py b/ddpui/services/dashboard_service.py index c1c00b380..c9f959c9b 100644 --- a/ddpui/services/dashboard_service.py +++ b/ddpui/services/dashboard_service.py @@ -238,12 +238,7 @@ def get_dashboard(dashboard_id: int, org: Org) -> Dashboard: @staticmethod def get_dashboard_response(dashboard: Dashboard) -> Dict[str, Any]: - """Convert dashboard model to response dict with migration support. - - This method handles: - - Migration of old filter format to new format - - Adding lock information - - Formatting filters data + """Convert dashboard model to response dict. Args: dashboard: The dashboard instance @@ -253,53 +248,6 @@ def get_dashboard_response(dashboard: Dashboard) -> Dict[str, Any]: """ lock = getattr(dashboard, "lock", None) - # Check if components need migration from old filter format to new format - components = dashboard.components or {} - needs_update = False - - for comp_id, component in components.items(): - if component.get("type") == "filter": - # Check if this is old format (full config) vs new format (just filterId reference) - config = component.get("config", {}) - if "filterId" not in config and "column_name" in config: - # This is old format, needs migration - # Find or create matching filter in DashboardFilter table - matching_filter = dashboard.filters.filter( - column_name=config.get("column_name"), - table_name=config.get("table_name", ""), - schema_name=config.get("schema_name", ""), - ).first() - - if not matching_filter: - # Create filter if it doesn't exist - matching_filter = DashboardFilter.objects.create( - dashboard=dashboard, - name=config.get("name", config.get("column_name", "")), - filter_type=config.get("filter_type", "value"), - schema_name=config.get("schema_name", ""), - table_name=config.get("table_name", ""), - column_name=config.get("column_name", ""), - settings=config.get("settings", {}), - order=0, - ) - - # Update component to new format with just filterId reference - components[comp_id] = { - "id": comp_id, - "type": "filter", - "config": { - "filterId": matching_filter.id, - "name": config.get("name", matching_filter.column_name), - }, - } - needs_update = True - - if needs_update: - # Save migrated components back to database - dashboard.components = components - dashboard.save(update_fields=["components"]) - logger.info(f"Migrated filter components for dashboard {dashboard.id}") - response_data = dashboard.to_json() response_data["is_locked"] = bool(lock and not lock.is_expired()) response_data["locked_by"] = ( @@ -779,27 +727,27 @@ def apply_filters(dashboard_id: int, filters: Dict[str, Any], orguser) -> Dict[s if not org_warehouse: raise ValueError("No warehouse configured for organization") - # Get all chart components from dashboard - components = dashboard.components + # Get all chart components from tabs chart_results = {} - for component_id, component_config in components.items(): - if component_config.get("type") == DashboardComponentType.CHART.value: - chart_id = component_config.get("config", {}).get("chartId") - if chart_id: - try: - # Get chart configuration - chart = Chart.objects.get(id=chart_id, org=orguser.org) - - # Apply filters to chart query - filtered_data = DashboardService._apply_filters_to_chart( - chart, dashboard.filters.all(), filters, org_warehouse - ) + for tab in dashboard.tabs or []: + for component_id, component_config in (tab.get("components") or {}).items(): + if component_config.get("type") == DashboardComponentType.CHART.value: + chart_id = component_config.get("config", {}).get("chartId") + if chart_id: + try: + # Get chart configuration + chart = Chart.objects.get(id=chart_id, org=orguser.org) + + # Apply filters to chart query + filtered_data = DashboardService._apply_filters_to_chart( + chart, dashboard.filters.all(), filters, org_warehouse + ) - chart_results[component_id] = {"success": True, "data": filtered_data} - except Exception as e: - logger.error(f"Error applying filters to chart {chart_id}: {str(e)}") - chart_results[component_id] = {"success": False, "error": str(e)} + chart_results[component_id] = {"success": True, "data": filtered_data} + except Exception as e: + logger.error(f"Error applying filters to chart {chart_id}: {str(e)}") + chart_results[component_id] = {"success": False, "error": str(e)} return chart_results @@ -1001,15 +949,15 @@ def get_dashboard_charts(dashboard_id: int, orguser) -> List[Dict[str, Any]]: except Dashboard.DoesNotExist: raise ValueError("Dashboard not found") - components = dashboard.components chart_ids = [] - # Extract chart IDs from components - for component_id, component_config in components.items(): - if component_config.get("type") == DashboardComponentType.CHART.value: - chart_id = component_config.get("config", {}).get("chartId") - if chart_id: - chart_ids.append(chart_id) + # Extract chart IDs from tabs + for tab in dashboard.tabs or []: + for _, component_config in (tab.get("components") or {}).items(): + if component_config.get("type") == DashboardComponentType.CHART.value: + chart_id = component_config.get("config", {}).get("chartId") + if chart_id: + chart_ids.append(chart_id) # Fetch chart details charts = Chart.objects.filter(id__in=chart_ids, org=orguser.org) @@ -1088,36 +1036,37 @@ def validate_dashboard_config(dashboard: Dashboard) -> Dict[str, Any]: errors = [] warnings = [] - # Validate layout config - layout_config = dashboard.layout_config or {} - components = dashboard.components or {} + # Validate layout config per tab + for tab in dashboard.tabs or []: + layout_config = {item["i"]: item for item in (tab.get("layout_config") or [])} + components = tab.get("components") or {} - # Check all components have layout entries - for component_id in components: - if component_id not in layout_config: - warnings.append(f"Component {component_id} has no layout configuration") + for component_id in components: + if component_id not in layout_config: + warnings.append(f"Component {component_id} has no layout configuration") - # Check all layout entries have components - for layout_id in layout_config: - if layout_id not in components: - errors.append(f"Layout entry {layout_id} has no corresponding component") + for layout_id in layout_config: + if layout_id not in components: + errors.append(f"Layout entry {layout_id} has no corresponding component") - # Validate component configurations - for component_id, component_config in components.items(): - component_type = component_config.get("type") + # Validate component configurations across all tabs + for tab in dashboard.tabs or []: + components = tab.get("components") or {} + for component_id, component_config in components.items(): + component_type = component_config.get("type") - if not component_type: - errors.append(f"Component {component_id} has no type specified") - continue + if not component_type: + errors.append(f"Component {component_id} has no type specified") + continue - if component_type not in [ct.value for ct in DashboardComponentType]: - errors.append(f"Component {component_id} has invalid type: {component_type}") + if component_type not in [ct.value for ct in DashboardComponentType]: + errors.append(f"Component {component_id} has invalid type: {component_type}") - # Validate chart components - if component_type == DashboardComponentType.CHART.value: - config = component_config.get("config", {}) - if not config.get("chartId"): - errors.append(f"Chart component {component_id} has no chartId") + # Validate chart components + if component_type == DashboardComponentType.CHART.value: + config = component_config.get("config", {}) + if not config.get("chartId"): + errors.append(f"Chart component {component_id} has no chartId") # Validate filters for filter in dashboard.filters.all(): diff --git a/ddpui/tests/api_tests/test_charts_api.py b/ddpui/tests/api_tests/test_charts_api.py index 13ce3fa9e..91de0e6ab 100644 --- a/ddpui/tests/api_tests/test_charts_api.py +++ b/ddpui/tests/api_tests/test_charts_api.py @@ -542,13 +542,19 @@ def test_get_chart_dashboards_with_dashboard(self, orguser, sample_chart, org, s title="Dashboard with Chart", dashboard_type="native", grid_columns=12, - layout_config=[], - components={ - "1": { - "type": DashboardComponentType.CHART.value, - "config": {"chartId": sample_chart.id}, + tabs=[ + { + "id": "tab-1", + "title": "Tab 1", + "layout_config": [], + "components": { + "1": { + "type": DashboardComponentType.CHART.value, + "config": {"chartId": sample_chart.id}, + } + }, } - }, + ], created_by=orguser, org=org, ) diff --git a/ddpui/tests/api_tests/test_dashboard_native_api.py b/ddpui/tests/api_tests/test_dashboard_native_api.py index 7d7396212..c13beb4ca 100644 --- a/ddpui/tests/api_tests/test_dashboard_native_api.py +++ b/ddpui/tests/api_tests/test_dashboard_native_api.py @@ -97,8 +97,6 @@ def sample_dashboard(orguser, org): description="Test Description", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -419,8 +417,6 @@ def test_delete_dashboard_success(self, orguser, sample_dashboard, seed_db): title="Another Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=orguser.org, ) diff --git a/ddpui/tests/api_tests/test_public_report_api.py b/ddpui/tests/api_tests/test_public_report_api.py index b28379a5d..b808983f8 100644 --- a/ddpui/tests/api_tests/test_public_report_api.py +++ b/ddpui/tests/api_tests/test_public_report_api.py @@ -106,8 +106,9 @@ def sample_dashboard(orguser, org): grid_columns=12, tabs=[ { - "name": "Tab 1", - "layout": [{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], + "id": "tab-1", + "title": "Tab 1", + "layout_config": [{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], "components": { "chart-1": { "id": "chart-1", @@ -481,14 +482,20 @@ def public_dashboard(orguser, org): description="Test", dashboard_type="native", grid_columns=12, - layout_config=[{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], - components={ - "chart-1": { - "id": "chart-1", - "type": "chart", - "config": {"chartId": 1, "chartType": "bar", "title": "Bar"}, + tabs=[ + { + "id": "tab-1", + "title": "Tab 1", + "layout_config": [{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], + "components": { + "chart-1": { + "id": "chart-1", + "type": "chart", + "config": {"chartId": 1, "chartType": "bar", "title": "Bar"}, + } + }, } - }, + ], created_by=orguser, org=org, is_public=True, diff --git a/ddpui/tests/api_tests/test_report_api.py b/ddpui/tests/api_tests/test_report_api.py index 03bef1064..c8cf25438 100644 --- a/ddpui/tests/api_tests/test_report_api.py +++ b/ddpui/tests/api_tests/test_report_api.py @@ -119,8 +119,9 @@ def sample_dashboard(orguser, org): grid_columns=12, tabs=[ { - "name": "Tab 1", - "layout": [{"i": "chart-123", "x": 0, "y": 0, "w": 6, "h": 4}], + "id": "tab-1", + "title": "Tab 1", + "layout_config": [{"i": "chart-123", "x": 0, "y": 0, "w": 6, "h": 4}], "components": { "chart-123": { "id": "chart-123", @@ -220,8 +221,6 @@ def empty_dashboard(orguser, org): description="No charts", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) diff --git a/ddpui/tests/core/reports/test_report_service.py b/ddpui/tests/core/reports/test_report_service.py index 4af0a00ea..f250f233a 100644 --- a/ddpui/tests/core/reports/test_report_service.py +++ b/ddpui/tests/core/reports/test_report_service.py @@ -104,12 +104,10 @@ def sample_dashboard(orguser, org): dashboard_type="native", grid_columns=12, target_screen_size="desktop", - layout_config=[], - components={}, tabs=[ { "id": "tab-1", - "title": "Overview", + "title": "Tab 1", "layout_config": [{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], "components": { "chart-1": { @@ -419,7 +417,6 @@ def test_captures_all_expected_keys(self, sample_dashboard, sample_filter): assert frozen["description"] == "Test Description" assert frozen["grid_columns"] == 12 assert frozen["target_screen_size"] == "desktop" - assert frozen["layout_config"] is not None assert len(frozen["tabs"]) == 1 assert "chart-1" in frozen["tabs"][0]["components"] assert len(frozen["filters"]) == 1 @@ -437,8 +434,6 @@ def test_empty_dashboard_no_filters(self, orguser, org): title="Empty", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -480,8 +475,6 @@ def test_empty_components(self, orguser, org): title="Empty", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -495,14 +488,14 @@ def test_component_with_missing_chart_id(self, orguser, org): title="Missing ID", dashboard_type="native", grid_columns=12, - layout_config=[], - components={ - "chart-1": { - "id": "chart-1", - "type": "chart", - "config": {}, # No chartId + tabs=[ + { + "id": "tab-1", + "title": "Tab 1", + "layout_config": [], + "components": {"chart-1": {"id": "chart-1", "type": "chart", "config": {}}}, } - }, + ], created_by=orguser, org=org, ) @@ -668,7 +661,7 @@ def test_view_data_structure( assert dd["title"] == "Test Dashboard" assert dd["dashboard_type"] == "native" assert dd["is_published"] is True - assert "components" in dd + assert "tabs" in dd assert "filters" in dd # report_metadata keys @@ -931,7 +924,7 @@ def test_deleting_dashboard_does_not_affect_snapshot( dd = view_data["dashboard_data"] assert dd["title"] == original_title assert dd["dashboard_type"] == "native" - assert "components" in dd + assert "tabs" in dd assert "filters" in dd rm = view_data["report_metadata"] @@ -1082,8 +1075,6 @@ def tab_dashboard(orguser, org, tab_chart): title="Tab Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", @@ -1132,24 +1123,25 @@ def test_extracts_ids_from_tabs_sample(self, sample_dashboard, sample_chart): chart_ids = ReportService._extract_chart_ids(sample_dashboard) assert sample_chart.id in chart_ids - def test_deduplicates_when_chart_in_both(self, orguser, org): - """Same chartId in tabs and root components appears only once""" + def test_deduplicates_when_chart_in_multiple_tabs(self, orguser, org): + """Same chartId in multiple tabs appears only once""" dashboard = Dashboard.objects.create( title="Overlap", dashboard_type="native", grid_columns=12, - layout_config=[], - components={ - "chart-1": {"type": "chart", "config": {"chartId": 99}}, - }, tabs=[ { "id": "tab-1", - "title": "T", - "components": { - "chart-1": {"type": "chart", "config": {"chartId": 99}}, - }, - } + "title": "T1", + "layout_config": [], + "components": {"chart-1": {"type": "chart", "config": {"chartId": 99}}}, + }, + { + "id": "tab-2", + "title": "T2", + "layout_config": [], + "components": {"chart-1": {"type": "chart", "config": {"chartId": 99}}}, + }, ], created_by=orguser, org=org, @@ -1176,13 +1168,6 @@ def test_tabs_captured_in_frozen_output(self, tab_dashboard): assert frozen["tabs"][0]["id"] == "tab-1" assert frozen["tabs"][0]["title"] == "Overview" - def test_root_fields_empty_for_tab_dashboard(self, tab_dashboard): - """Root layout_config and components are empty for tab-based dashboards""" - frozen = ReportService._freeze_dashboard(tab_dashboard) - - assert frozen["layout_config"] == [] - assert frozen["components"] == {} - # ================================================================================ # Test _freeze_chart_configs with tabs @@ -1267,8 +1252,6 @@ def kpi_dashboard(orguser, org, sample_chart, sample_kpi): title="KPI Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", @@ -1434,8 +1417,6 @@ def test_kpi_only_dashboard(self, mock_kpi_service, mock_org_wh, orguser, org, s title="KPI Only", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", @@ -1463,8 +1444,6 @@ def test_kpi_component_without_kpi_id(self, mock_kpi_service, mock_org_wh, orgus title="Missing KPI ID", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", @@ -1509,8 +1488,6 @@ def test_kpi_expression_metric_frozen(self, mock_kpi_service, mock_org_wh, orgus title="Expr Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", @@ -1567,8 +1544,6 @@ def test_returns_kpi_data_from_frozen_config( title="Report Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", @@ -1645,8 +1620,6 @@ def test_kpi_survives_deletion(self, mock_compute, orguser, org, sample_chart, s title="Deletion Test", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, tabs=[ { "id": "tab-1", diff --git a/ddpui/tests/models/test_dashboard_models.py b/ddpui/tests/models/test_dashboard_models.py index 39ad2dc14..7fdb95a2d 100644 --- a/ddpui/tests/models/test_dashboard_models.py +++ b/ddpui/tests/models/test_dashboard_models.py @@ -76,8 +76,6 @@ def sample_dashboard(orguser, org): description="Test Description", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -139,8 +137,14 @@ def test_dashboard_create_success(orguser, org, seed_db): description="Test Description", dashboard_type="native", grid_columns=12, - layout_config=[{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], - components={"chart-1": {"type": "chart", "chart_id": 1}}, + tabs=[ + { + "id": "tab-1", + "title": "Tab 1", + "layout_config": [{"i": "chart-1", "x": 0, "y": 0, "w": 6, "h": 4}], + "components": {"chart-1": {"type": "chart", "config": {"chartId": 1}}}, + } + ], created_by=orguser, org=org, ) @@ -171,8 +175,7 @@ def test_dashboard_create_with_defaults(orguser, org, seed_db): assert dashboard.is_published is False assert dashboard.is_public is False assert dashboard.public_access_count == 0 - assert dashboard.layout_config == [] - assert dashboard.components == {} + assert dashboard.tabs == [] dashboard.delete() @@ -253,8 +256,14 @@ def test_dashboard_to_json(orguser, org, seed_db): description="JSON Description", dashboard_type="native", grid_columns=12, - layout_config=[{"i": "1", "x": 0, "y": 0}], - components={"1": {"type": "chart"}}, + tabs=[ + { + "id": "tab-1", + "title": "Tab 1", + "layout_config": [{"i": "1", "x": 0, "y": 0}], + "components": {"1": {"type": "chart"}}, + } + ], created_by=orguser, org=org, ) @@ -266,8 +275,9 @@ def test_dashboard_to_json(orguser, org, seed_db): assert json_data["description"] == "JSON Description" assert json_data["dashboard_type"] == "native" assert json_data["grid_columns"] == 12 - assert json_data["layout_config"] == [{"i": "1", "x": 0, "y": 0}] - assert json_data["components"] == {"1": {"type": "chart"}} + assert len(json_data["tabs"]) == 1 + assert json_data["tabs"][0]["layout_config"] == [{"i": "1", "x": 0, "y": 0}] + assert json_data["tabs"][0]["components"] == {"1": {"type": "chart"}} assert json_data["is_published"] is False assert json_data["created_by"] == "dashmodeluser@test.com" assert json_data["org_id"] == org.id diff --git a/ddpui/tests/schema_tests/test_dashboard_schema.py b/ddpui/tests/schema_tests/test_dashboard_schema.py index 48d73cfc7..f1957991e 100644 --- a/ddpui/tests/schema_tests/test_dashboard_schema.py +++ b/ddpui/tests/schema_tests/test_dashboard_schema.py @@ -411,8 +411,6 @@ def test_dashboard_response_with_filters(self): grid_columns=12, target_screen_size="desktop", filter_layout="horizontal", - layout_config=[], - components={}, is_published=False, published_at=None, is_locked=False, diff --git a/ddpui/tests/services/test_chart_service.py b/ddpui/tests/services/test_chart_service.py index 03c0cca6b..ab59fd760 100644 --- a/ddpui/tests/services/test_chart_service.py +++ b/ddpui/tests/services/test_chart_service.py @@ -251,13 +251,19 @@ def test_get_chart_dashboards_multiple(self, orguser, sample_chart, org, seed_db title=f"Dashboard {i}", dashboard_type="native", grid_columns=12, - layout_config=[], - components={ - "1": { - "type": DashboardComponentType.CHART.value, - "config": {"chartId": sample_chart.id}, + tabs=[ + { + "id": "tab-1", + "title": "Tab 1", + "layout_config": [], + "components": { + "1": { + "type": DashboardComponentType.CHART.value, + "config": {"chartId": sample_chart.id}, + } + }, } - }, + ], created_by=orguser, org=org, ) diff --git a/ddpui/tests/services/test_dashboard_service.py b/ddpui/tests/services/test_dashboard_service.py index a7e0bfd76..45fde4d48 100644 --- a/ddpui/tests/services/test_dashboard_service.py +++ b/ddpui/tests/services/test_dashboard_service.py @@ -111,8 +111,6 @@ def sample_dashboard(orguser, org): description="Test Description", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -218,8 +216,6 @@ def test_delete_dashboard_permission_denied_not_creator(self, orguser, orguser2, title="Protected Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -238,8 +234,6 @@ def test_delete_dashboard_org_default_fails(self, orguser, org, seed_db): title="Org Default Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, is_org_default=True, created_by=orguser, org=org, @@ -259,8 +253,6 @@ def test_delete_dashboard_with_landing_page_fails(self, orguser, org, seed_db): title="Landing Page Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) @@ -283,8 +275,6 @@ def test_delete_dashboard_locked_fails(self, orguser, orguser2, org, seed_db): title="Locked Dashboard", dashboard_type="native", grid_columns=12, - layout_config=[], - components={}, created_by=orguser, org=org, ) diff --git a/ddpui/tests/services/test_kpi_service.py b/ddpui/tests/services/test_kpi_service.py index b5cebced9..7437e2f51 100644 --- a/ddpui/tests/services/test_kpi_service.py +++ b/ddpui/tests/services/test_kpi_service.py @@ -272,8 +272,6 @@ def test_get_kpi_dashboards(self, orguser, org, sample_kpi, seed_db): title="Test Dashboard", org=org, created_by=orguser, - components={}, - layout_config=[], tabs=[ { "id": "tab-1",