Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate databaseId, database to use databaseID from MLv2 #37252

Merged
merged 40 commits into from
Jan 9, 2024
Merged

Migrate databaseId, database to use databaseID from MLv2 #37252

merged 40 commits into from
Jan 9, 2024

Conversation

kamilmielnik
Copy link
Contributor

@kamilmielnik kamilmielnik commented Jan 4, 2024 •
edited

Closes #37169

Description

  • In 2 places metadata and query: Lib.Query were not available, so I used legacyQuery.question()'s methods which use MLv2 under the hood
    • databaseSupportsPivotTables in PivotTable/utils.ts (visualization.checkRenderable() signature would need to change)
    • deprecated FilterPopover
  • StructuredQuery & NativeQuery still use databaseId and database but the methods have been marked as deprecated and were prefixed with _

@kamilmielnik kamilmielnik self-assigned this Jan 4, 2024
@kamilmielnik kamilmielnik added the no-backport Do not backport this PR to any branch label Jan 4, 2024
@kamilmielnik kamilmielnik changed the title Migrate databaseId, database to use databaseID from MLv2 [WIP] Migrate databaseId, database to use databaseID from MLv2 Jan 4, 2024
@kamilmielnik kamilmielnik marked this pull request as ready for review January 4, 2024 12:08
@metabase-bot metabase-bot bot added the visual Run Percy visual testing label Jan 4, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 4, 2024 •
edited

Codenotify: Notifying subscribers in CODENOTIFY files for diff d27cf2a...5133606.

Notify File(s)
@alxnddr frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx
frontend/src/metabase/visualizations/visualizations/PivotTable/utils.ts

@kamilmielnik kamilmielnik marked this pull request as draft January 4, 2024 12:34
@kamilmielnik kamilmielnik marked this pull request as ready for review January 4, 2024 12:41
@kamilmielnik kamilmielnik changed the title [WIP] Migrate databaseId, database to use databaseID from MLv2 Migrate databaseId, database to use databaseID from MLv2 Jan 9, 2024
@kamilmielnik kamilmielnik requested a review from a team January 9, 2024 09:01
@kamilmielnik kamilmielnik mentioned this pull request Jan 9, 2024
Merged
ranquild
ranquild approved these changes Jan 9, 2024
View reviewed changes
frontend/src/metabase-lib/Question.ts Outdated Show resolved Hide resolved
frontend/src/metabase/query_builder/components/view/View.jsx Outdated Show resolved Hide resolved
@kamilmielnik kamilmielnik enabled auto-merge (squash) January 9, 2024 12:35
This was referenced Jan 9, 2024
Closed
Closed
@kamilmielnik kamilmielnik merged commit 9fd149d into master Jan 9, 2024
106 checks passed
@kamilmielnik kamilmielnik deleted the 37169-query-database branch January 9, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ranquild ranquild ranquild approved these changes

Assignees

@kamilmielnik kamilmielnik

Labels
no-backport Do not backport this PR to any branch .Team/QueryingComponents visual Run Percy visual testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MLv2] [FE] Migrate databaseId, database to use databaseID from MLv2
2 participants
@kamilmielnik @ranquild