Skip to content

fix: respect --keep-original-ids for dashboard with drills#14

Merged
janmatzek merged 1 commit into
gooddata:masterfrom
janmatzek:jmat/SVS-1334-migration-of-kpi-dashboard-with-drill-down-to-another-dashboard-with-keep-original-ids-fails
Jul 3, 2026
Merged

fix: respect --keep-original-ids for dashboard with drills#14
janmatzek merged 1 commit into
gooddata:masterfrom
janmatzek:jmat/SVS-1334-migration-of-kpi-dashboard-with-drill-down-to-another-dashboard-with-keep-original-ids-fails

Conversation

@janmatzek

@janmatzek janmatzek commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

JIRA: SVS-1334

Summary by CodeRabbit

  • New Features

    • Added support for preserving original dashboard IDs during dashboard conversion when drill actions target another dashboard.
    • Included new dashboard fixtures and examples covering this behavior.
  • Bug Fixes

    • Ensured drill-to-dashboard target identifiers are kept as expected when ID preservation is enabled.
  • Tests

    • Added a new pytest to validate drill conversion ID preservation end-to-end using updated legacy and cloud fixtures.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f621bf07-a7e1-4852-b335-d2326a640d21

📥 Commits

Reviewing files that changed from the base of the PR and between bd304f8 and ab12caa.

📒 Files selected for processing (5)
  • src/gooddata_legacy2cloud/dashboards/cloud_dashboards_builder.py
  • tests/data/dashboards/legacy_objects/objects_by_uri.json
  • tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_cloud.json
  • tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_legacy.json
  • tests/test_dashboards.py
✅ Files skipped from review due to trivial changes (2)
  • tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_legacy.json
  • tests/data/dashboards/legacy_objects/objects_by_uri.json
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/gooddata_legacy2cloud/dashboards/cloud_dashboards_builder.py
  • tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_cloud.json
  • tests/test_dashboards.py

📝 Walkthrough

Walkthrough

This PR changes drill-to-dashboard ID mapping so legacy dashboard IDs are preserved when keep_original_ids is enabled. It also adds drill-specific legacy/cloud fixtures and a test that verifies the generated dashboards match the expected output.

Changes

Keep Original IDs for Drill-to-Dashboard Mapping

Layer / File(s) Summary
Conditional Cloud ID derivation
src/gooddata_legacy2cloud/dashboards/cloud_dashboards_builder.py
Legacy-to-Cloud dashboard ID mapping for drill conversion now uses the legacy ID directly when keep_original_ids is set, otherwise falls back to get_cloud_id.
New drill test fixtures
tests/data/dashboards/legacy_objects/objects_by_uri.json, tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_legacy.json, tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_cloud.json
Adds a legacy object with a drillToDashboard config and paired legacy/cloud dashboard fixtures for source and target dashboards with a drill widget.
Drill ID preservation test
tests/test_dashboards.py
Adds test_dashboard_drill_to_dashboard_keep_original_ids, which processes legacy fixtures with keep_original_ids and asserts the output matches expected cloud dashboard data.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test as test_dashboard_drill_to_dashboard_keep_original_ids
  participant Builder as CloudDashboardsBuilder
  participant Ctx as dashboards_context_keep_ids

  Test->>Builder: process_legacy_dashboards(legacy_json, skip_deploy=True)
  Builder->>Ctx: check keep_original_ids
  alt keep_original_ids true
    Ctx-->>Builder: preserve legacy_id
  else keep_original_ids false
    Ctx-->>Builder: derive cloud id
  end
  Builder-->>Test: cloud dashboards with resolved drill ids
  Test->>Test: assert dicts_are_equal(actual, expected)
Loading

Suggested reviewers: xvalovic

Poem

A rabbit hops through drill and dash,
Keeping old IDs safe from crash. 🐇
Source to target, the trail stays true,
New fixtures test it through and through,
Hop hop hooray, the drills all pass!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: preserving original dashboard IDs for drill conversions when --keep-original-ids is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_dashboards.py`:
- Line 133: The comparison loop in the dashboard test uses zip(actual_sorted,
expected_sorted) without strict checking, so mismatched lengths could be
silently ignored. Update the zip call in the test that iterates over
actual_sorted and expected_sorted to use strict=True so the assertion fails when
the lists differ in size.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c2fdef25-802c-46a4-81a0-d241f07005c5

📥 Commits

Reviewing files that changed from the base of the PR and between 3ca99d5 and bd304f8.

📒 Files selected for processing (5)
  • src/gooddata_legacy2cloud/dashboards/cloud_dashboards_builder.py
  • tests/data/dashboards/legacy_objects/objects_by_uri.json
  • tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_cloud.json
  • tests/data/dashboards/test_cases/dashboard_with_drills_keep_original_ids_legacy.json
  • tests/test_dashboards.py

Comment thread tests/test_dashboards.py Outdated
@janmatzek janmatzek force-pushed the jmat/SVS-1334-migration-of-kpi-dashboard-with-drill-down-to-another-dashboard-with-keep-original-ids-fails branch from bd304f8 to ab12caa Compare July 3, 2026 07:54
@janmatzek janmatzek enabled auto-merge (rebase) July 3, 2026 07:55
@janmatzek janmatzek merged commit f2a299f into gooddata:master Jul 3, 2026
2 checks passed
@janmatzek janmatzek deleted the jmat/SVS-1334-migration-of-kpi-dashboard-with-drill-down-to-another-dashboard-with-keep-original-ids-fails branch July 3, 2026 08:18
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