From 80a2e45a1bfd33d6c1cf67e2e2ee0a1555d48a0c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Atte=20Kein=C3=A4nen?= <atte.keinanen@gmail.com>
Date: Tue, 13 Jun 2017 11:33:26 -0700
Subject: [PATCH] Fixing flow errors
---
frontend/src/metabase-lib/lib/Question.js | 24 ++++++++++++-------
.../src/metabase-lib/lib/metadata/Metric.js | 2 +-
.../src/metabase-lib/lib/metadata/Table.js | 1 +
.../metabase-lib/lib/queries/MultiQuery.js | 6 +++--
.../lib/queries/StructuredQuery.js | 4 ++--
.../DashCardCardParameterMapper.jsx | 4 +++-
frontend/src/metabase/meta/types/Card.js | 1 -
.../new_query/containers/NewQueryOptions.jsx | 1 +
.../src/metabase/query_builder/actions.js | 10 ++------
9 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/frontend/src/metabase-lib/lib/Question.js b/frontend/src/metabase-lib/lib/Question.js
index df65715dd34..6940c470dad 100644
--- a/frontend/src/metabase-lib/lib/Question.js
+++ b/frontend/src/metabase-lib/lib/Question.js
@@ -48,10 +48,12 @@ import type {
} from "metabase/meta/types/Visualization";
import { MetabaseApi, CardApi } from "metabase/services";
import { DatetimeFieldDimension } from "metabase-lib/lib/Dimension";
-import { TableId } from "metabase/meta/types/Table";
-import { DatabaseId } from "metabase/meta/types/Database";
import AtomicQuery from "metabase-lib/lib/queries/AtomicQuery";
-import { Dataset } from "metabase/meta/types/Dataset";
+
+import type { Dataset } from "metabase/meta/types/Dataset";
+import type { TableId } from "metabase/meta/types/Table";
+import type { DatabaseId } from "metabase/meta/types/Database";
+import { STRUCTURED_QUERY_TEMPLATE } from "metabase-lib/lib/queries/StructuredQuery";
// TODO: move these
type DownloadFormat = "csv" | "json" | "xlsx";
@@ -76,7 +78,7 @@ export default class Question {
/**
* Parameter values mean either the current values of dashboard filters or SQL editor template parameters.
- * TODO Atte Keinänen 6/6/17: Why are parameter values considered a part of a Question?
+ * They are in the grey area between UI state and question state, but having them in Question wrapper is convenient.
*/
_parameterValues: ParameterValues;
@@ -94,19 +96,25 @@ export default class Question {
}
/**
- * TODO: Write a docstring, rename the latter newQuestion instance method
+ * TODO Atte Keinänen 6/13/17: Discussed with Tom that we could use the default Question constructor instead,
+ * but it would require changing the constructor signature so that `card` is an optional parameter and has a default value
*/
- static newQuestion({
+ static create({
databaseId, tableId, metadata, parameterValues, ...cardProps
}: { databaseId?: DatabaseId, tableId?: TableId, metadata: Metadata, parameterValues?: ParameterValues }) {
+
const card = {
name: cardProps.name || null,
display: cardProps.display || "table",
visualization_settings: cardProps.visualization_settings || {},
- dataset_query: cardProps.dataset_query || StructuredQuery.newStucturedQuery({ question: this, databaseId, tableId })
+ dataset_query: STRUCTURED_QUERY_TEMPLATE // temporary placeholder
};
- return new Question(metadata, card, parameterValues);
+ // $FlowFixMe Passing an incomplete card object
+ const initialQuestion = new Question(metadata, card, parameterValues);
+ const query = StructuredQuery.newStucturedQuery({ question: initialQuestion, databaseId, tableId })
+
+ return initialQuestion.setQuery(query);
}
/**
diff --git a/frontend/src/metabase-lib/lib/metadata/Metric.js b/frontend/src/metabase-lib/lib/metadata/Metric.js
index ede67af4ab7..5320a57ec01 100644
--- a/frontend/src/metabase-lib/lib/metadata/Metric.js
+++ b/frontend/src/metabase-lib/lib/metadata/Metric.js
@@ -5,7 +5,7 @@ import Question from "../Question";
import Database from "./Database";
import Table from "./Table";
import Dimension, { DatetimeFieldDimension } from "metabase-lib/lib/Dimension";
-import { Aggregation } from "metabase/meta/types/Query";
+import type { Aggregation } from "metabase/meta/types/Query";
/**
* Wrapper class for a metric. Belongs to a {@link Database} and possibly a {@link Table}
diff --git a/frontend/src/metabase-lib/lib/metadata/Table.js b/frontend/src/metabase-lib/lib/metadata/Table.js
index a80fe9cc6e7..ee9728e9afd 100644
--- a/frontend/src/metabase-lib/lib/metadata/Table.js
+++ b/frontend/src/metabase-lib/lib/metadata/Table.js
@@ -22,6 +22,7 @@ export default class Table extends Base {
fields: Field[];
+ // $FlowFixMe Could be replaced with hydrated database property in selectors/metadata.js (instead / in addition to `db`)
get database() {
return this.db;
}
diff --git a/frontend/src/metabase-lib/lib/queries/MultiQuery.js b/frontend/src/metabase-lib/lib/queries/MultiQuery.js
index 4d4ee92842c..3993e0df9c0 100644
--- a/frontend/src/metabase-lib/lib/queries/MultiQuery.js
+++ b/frontend/src/metabase-lib/lib/queries/MultiQuery.js
@@ -16,6 +16,8 @@ import type {
DatasetQuery,
MultiDatasetQuery
} from "metabase/meta/types/Card";
+import AtomicQuery from "metabase-lib/lib/queries/AtomicQuery";
+import Metric from "metabase-lib/lib/metadata/Metric";
export const MULTI_QUERY_TEMPLATE: MultiDatasetQuery = {
type: "multi",
@@ -206,7 +208,7 @@ export default class MultiQuery extends Query {
const breakoutDimension = new DatetimeFieldDimension(compatibleFields[0].dimension(), [sharedDimension.bucketing()])
const metricQuery = StructuredQuery
- .newStucturedQuery({ question: this, databaseId: metric.table.db.id, tableId: metric.table.id })
+ .newStucturedQuery({ question: this._originalQuestion, databaseId: metric.table.db.id, tableId: metric.table.id })
.addAggregation(metric.aggregationClause())
.addBreakout(breakoutDimension.mbql());
@@ -245,7 +247,7 @@ export default class MultiQuery extends Query {
/* Internal methods */
_updateQueries(queries: AtomicQuery[]) {
- const datasetQuery: MultiDatasetQuery = this.datasetQuery();
+ const datasetQuery: DatasetQuery = this.datasetQuery();
return new MultiQuery(this._originalQuestion, {
...datasetQuery,
queries: queries.map((query) => query.datasetQuery())
diff --git a/frontend/src/metabase-lib/lib/queries/StructuredQuery.js b/frontend/src/metabase-lib/lib/queries/StructuredQuery.js
index df20f9093b1..0808a110b12 100644
--- a/frontend/src/metabase-lib/lib/queries/StructuredQuery.js
+++ b/frontend/src/metabase-lib/lib/queries/StructuredQuery.js
@@ -46,11 +46,11 @@ import type Table from "../metadata/Table";
import type { DatabaseEngine, DatabaseId } from "metabase/meta/types/Database";
import type Database from "../metadata/Database";
import type Question from "../Question";
-import { TableId } from "metabase/meta/types/Table";
+import type { TableId } from "metabase/meta/types/Table";
import AtomicQuery from "./AtomicQuery";
import AggregationWrapper from './Aggregation';
-const STRUCTURED_QUERY_TEMPLATE = {
+export const STRUCTURED_QUERY_TEMPLATE = {
database: null,
type: "query",
query: {
diff --git a/frontend/src/metabase/dashboard/containers/DashCardCardParameterMapper.jsx b/frontend/src/metabase/dashboard/containers/DashCardCardParameterMapper.jsx
index c8b56fa32f7..b6d7354131b 100644
--- a/frontend/src/metabase/dashboard/containers/DashCardCardParameterMapper.jsx
+++ b/frontend/src/metabase/dashboard/containers/DashCardCardParameterMapper.jsx
@@ -26,6 +26,7 @@ import type { Parameter, ParameterId, ParameterMappingUIOption, ParameterTarget
import type { DatabaseId } from "metabase/meta/types/Database";
import type { MappingsByParameter } from "../selectors";
+import AtomicQuery from "metabase-lib/lib/queries/AtomicQuery";
const makeMapStateToProps = () => {
const getParameterMappingOptions = makeGetParameterMappingOptions()
@@ -67,7 +68,8 @@ export default class DashCardCardParameterMapper extends Component {
componentDidMount() {
const { card } = this.props;
- this.props.fetchDatabaseMetadata(card.dataset_query.database);
+ // Type check for Flow
+ card.dataset_query instanceof AtomicQuery && this.props.fetchDatabaseMetadata(card.dataset_query.database);
}
onChange = (option: ?ParameterMappingUIOption) => {
diff --git a/frontend/src/metabase/meta/types/Card.js b/frontend/src/metabase/meta/types/Card.js
index 110940ac787..b2d027da898 100644
--- a/frontend/src/metabase/meta/types/Card.js
+++ b/frontend/src/metabase/meta/types/Card.js
@@ -3,7 +3,6 @@
import type { DatabaseId } from "./Database";
import type { StructuredQuery, NativeQuery } from "./Query";
import type { Parameter, ParameterInstance } from "./Parameter";
-import { BreakoutClause, FilterClause } from "metabase/meta/types/Query";
export type CardId = number;
diff --git a/frontend/src/metabase/new_query/containers/NewQueryOptions.jsx b/frontend/src/metabase/new_query/containers/NewQueryOptions.jsx
index b48b64614c2..118c1d9669a 100644
--- a/frontend/src/metabase/new_query/containers/NewQueryOptions.jsx
+++ b/frontend/src/metabase/new_query/containers/NewQueryOptions.jsx
@@ -21,6 +21,7 @@ class OptionListItem extends Component {
props: {
item: any,
action: (any) => void,
+ children?: React$Element<any>
}
onClick = () => { this.props.action(this.props.item); }
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index d6b3f987574..548c38c224a 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -291,24 +291,18 @@ export const initializeQB = (location, params) => {
// NOTE: timeout to allow Parameters widget to set parameterValues
setTimeout(() =>
// TODO Atte Keinänen 5/31/17: Check if it is dangerous to create a question object without metadata
- dispatch(runQuestionQuery({ overrideWithCard: card, shouldUpdateUrl: false }))
+ dispatch(runQuestionQuery({ shouldUpdateUrl: false }))
, 0);
}
- const originalQuestion = originalCard && new Question(getMetadata(getState()), originalCard);
// clean up the url and make sure it reflects our card state
+ const originalQuestion = originalCard && new Question(getMetadata(getState()), originalCard);
dispatch(updateUrl(card, {
dirty: originalQuestion && question.isDirtyComparedTo(originalQuestion),
replaceState: true,
preserveParameters
}));
}
-
- // if we have loaded up a card that we can run then lets kick that off as well
- if (question.canRun()) {
- // NOTE: timeout to allow Parameters widget to set parameterValues
- setTimeout(() => dispatch(runQuestionQuery({ shouldUpdateUrl: false })), 0);
- }
};
};
--
GitLab