Skip to content

fix: dashboard conversion crashes and needless element cache fetch - #20

Merged
xvalovic merged 1 commit into
gooddata:masterfrom
xvalovic:fix/dashboard-conversion-and-element-cache
Oct 7, 2026
Merged

xvalovic merged 1 commit into
gooddata:masterfrom
xvalovic:fix/dashboard-conversion-and-element-cache

Conversation

@xvalovic

@xvalovic xvalovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Dashboard conversion crashes

Two Legacy dashboard shapes made CloudDashboard raise, so the dashboard was not migrated at all (24 dashboards across 18 workspaces):

  • Empty dashboards (21×): a dashboard with no widgets has no layout key in Legacy. The [] fallback was then indexed with "fluidLayout" → TypeError: list indices must be integers or slices, not str. A missing layout now produces an empty Cloud layout (sections: []).
  • Attribute filters without localIdentifier (3×, KPI dashboards from 2020): older Legacy filter contexts don't store it → KeyError: 'localIdentifier'. A stable {idx}_attributeFilter id is generated, mirroring the existing {n}_dateFilter handling; it is used both in the filter and in attributeFilterConfigs.

Skip element cache when nothing is migrated

migrate_metrics/insights/dashboards called legacy_client.initialize_attribute_elements_cache() even when the filtered object list was empty. With --client-prefix (default --without-mapped-objects default_only) most client workspaces have nothing custom to migrate, so this slow fetch was pure overhead. It now runs only when there are objects to process.

Tests

  • tests/test_dashboards.py: two new cases, empty_dashboard and dashboard_with_attribute_filter_without_local_identifier, derived from existing sanitized fixtures.
  • tests/test_migrate_metrics_element_cache.py: the cache is not loaded for an empty metric list and is loaded otherwise.

All new tests fail on master with the original errors and pass with the fix.

Validation

  • make check on a clean checkout: format, lint, 111 tests, type-check pass.
  • Real data: both fixes ran in the RingCentral migration (1405 workspaces). The 24 failing dashboards were re-migrated with --overwrite-existing --only-object-ids and verified in Cloud (original title, filter context, widgets).

risk: low

Two Legacy dashboard shapes crashed the conversion:
- dashboards with no widgets have no layout key; the fallback []
  was then indexed with "fluidLayout" (TypeError). Missing layout
  now yields an empty Cloud layout.
- older KPI dashboards store attribute filters without
  localIdentifier (KeyError). A stable "{idx}_attributeFilter" id
  is generated, mirroring the existing date filter handling.

Metrics, insights and dashboards migrations also loaded the Legacy
attribute elements cache when the filtered object list was empty
(the common case for client workspaces with --client-prefix); the
slow fetch now runs only when there are objects to process.

risk: low

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@xvalovic
xvalovic requested a review from janmatzek October 7, 2026 09:19
@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3ac24a10-3239-4cfc-bab2-c18807214682
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@marmil-cz marmil-cz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xvalovic
xvalovic merged commit fdba101 into gooddata:master Oct 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants