From 2b11cfdfc7ad5f04808bdfe12eeec69f6fecf0ab Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Thu, 22 Sep 2022 09:16:24 -0400
Subject: [PATCH] Remove Question's `update` method & related code (#25531)
* Remove _update from Question
* Update TimeseriesGroupingWidget
* Update ExtendedOptions
* Update SummarizeSidebar
* Update AddAggregationButton and AggreationItem
* Remove update method from MBQLClause
* Update ChartTypeSidebar
* Update GuiQueryEditor
* Update NativeQueryEditor
* Update BulkFilterModal
* Update Notebook
* Update NotebookSteps
* Update NotebookStep
* Update AggregateStep
* Update BreakoutStep
* Update DataStep
* Update ExpressionStep
* Update FilterStep
* Update JoinStep
* Update LimitStep
* Update SortStep
* Update NativeQueryButton
* Update QuestionDataSelector
* Update QuestionFilters
* Update QuestionRowCount
* Update View
* Update random doc
* Ensure we have a 'datasetQuery' in the setDatasetQuery function
* Ensure we have a 'datasetQuery' in the setDatasetQuery action
---
frontend/src/metabase-lib/README.md | 3 +-
frontend/src/metabase-lib/lib/Question.ts | 37 +------
.../src/metabase-lib/lib/queries/Query.ts | 13 ---
.../lib/queries/structured/MBQLClause.ts | 14 ---
.../components/PartialQueryBuilder.jsx | 5 +
.../components/TimeseriesGroupingWidget.jsx | 23 +++--
.../modes/components/modes/TimeseriesMode.jsx | 11 ++-
.../query_builder/actions/core/core.js | 5 +
.../components/ExtendedOptions.jsx | 23 +++--
.../components/GuiQueryEditor.jsx | 20 ++--
.../components/NativeQueryEditor.jsx | 31 +++---
.../query_builder/components/QueryModals.jsx | 11 ++-
.../BulkFilterModal/BulkFilterModal.tsx | 6 +-
.../components/notebook/Notebook.jsx | 4 +-
.../components/notebook/NotebookStep.jsx | 2 +-
.../components/notebook/NotebookSteps.jsx | 8 +-
.../notebook/steps/AggregateStep.jsx | 6 +-
.../notebook/steps/BreakoutStep.jsx | 6 +-
.../components/notebook/steps/DataStep.jsx | 14 +--
.../notebook/steps/ExpressionStep.jsx | 10 +-
.../components/notebook/steps/FilterStep.jsx | 6 +-
.../components/notebook/steps/JoinStep.jsx | 97 ++++++++++---------
.../components/notebook/steps/LimitStep.jsx | 2 +-
.../components/notebook/steps/SortStep.jsx | 15 +--
.../components/view/NativeQueryButton.jsx | 18 ++--
.../components/view/QuestionDataSelector.jsx | 10 +-
.../components/view/QuestionFilters.jsx | 17 ++--
.../components/view/QuestionRowCount.jsx | 5 +-
.../query_builder/components/view/View.jsx | 29 ++++--
.../View/NewQuestionView/NewQuestionView.tsx | 5 +-
.../components/view/ViewFooter.jsx | 12 ++-
.../components/view/ViewHeader.jsx | 18 +++-
.../view/sidebars/ChartTypeSidebar.jsx | 14 +--
.../AddAggregationButton.jsx | 8 +-
.../AggregationItem/AggregationItem.jsx | 10 +-
.../SummarizeSidebar/SummarizeSidebar.jsx | 18 +++-
.../view/sidebars/SummarizeSidebar/utils.js | 4 -
.../query_builder/containers/QueryBuilder.jsx | 7 --
38 files changed, 295 insertions(+), 252 deletions(-)
delete mode 100644 frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/utils.js
diff --git a/frontend/src/metabase-lib/README.md b/frontend/src/metabase-lib/README.md
index 4979d46aa4f..1b789a31047 100644
--- a/frontend/src/metabase-lib/README.md
+++ b/frontend/src/metabase-lib/README.md
@@ -3,11 +3,10 @@
- `setFoo(bar)`: returns clone of the wrapper but with the "foo" attribute to set to `bar`
- `replace(object)`: returns clone of parent wrapper with this object replaced by `object`
- `remove()`: returns clone of the parent wrapper with this object removed
-- `update()`: propagates current wrapper update to parent wrapper, recursively
Examples:
-- `question().query().aggregation()[0].setDimension(dimension).update()`
+- `question().query().aggregation()[0].setDimension(dimension)`
Exceptions:
diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index 4f6c0c7b101..7fedba9b2fe 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -82,8 +82,6 @@ import {
ALERT_TYPE_TIMESERIES_GOAL,
} from "metabase-lib/lib/Alert";
-type QuestionUpdateFn = (q: Question) => Promise<void> | null | undefined;
-
export type QuestionCreatorOpts = {
databaseId?: DatabaseId;
tableId?: TableId;
@@ -120,11 +118,6 @@ class QuestionInner {
*/
_parameterValues: ParameterValues;
- /**
- * Bound update function, if any
- */
- _update: QuestionUpdateFn | null | undefined;
-
/**
* Question constructor
*/
@@ -132,7 +125,6 @@ class QuestionInner {
card: CardObject,
metadata?: Metadata,
parameterValues?: ParameterValues,
- update?: QuestionUpdateFn | null | undefined,
) {
this._card = card;
this._metadata =
@@ -146,16 +138,10 @@ class QuestionInner {
questions: {},
});
this._parameterValues = parameterValues || {};
- this._update = update;
}
clone() {
- return new Question(
- this._card,
- this._metadata,
- this._parameterValues,
- this._update,
- );
+ return new Question(this._card, this._metadata, this._parameterValues);
}
metadata(): Metadata {
@@ -172,27 +158,6 @@ class QuestionInner {
return q;
}
- /**
- * calls the passed in update function (useful for chaining) or bound update function with the question
- * NOTE: this passes Question instead of card, unlike how Query passes dataset_query
- */
- update(update?: QuestionUpdateFn, ...args: any[]) {
- // TODO: if update returns a new card, create a new Question based on that and return it
- if (update) {
- update(this, ...args);
- } else if (this._update) {
- this._update(this, ...args);
- } else {
- throw new Error("Question update function not provided or bound");
- }
- }
-
- bindUpdate(update: QuestionUpdateFn) {
- const q = this.clone();
- q._update = update;
- return q;
- }
-
withoutNameAndId() {
return this.setCard(
chain(this.card())
diff --git a/frontend/src/metabase-lib/lib/queries/Query.ts b/frontend/src/metabase-lib/lib/queries/Query.ts
index 43f6cdec7ec..0e9a68213c4 100644
--- a/frontend/src/metabase-lib/lib/queries/Query.ts
+++ b/frontend/src/metabase-lib/lib/queries/Query.ts
@@ -9,11 +9,9 @@ import Variable from "metabase-lib/lib/variables/Variable";
import { memoizeClass } from "metabase-lib/lib/utils";
import DimensionOptions from "metabase-lib/lib/DimensionOptions";
-type QueryUpdateFn = (datasetQuery: DatasetQuery) => void;
/**
* An abstract class for all query types (StructuredQuery & NativeQuery)
*/
-
class QueryInner {
_metadata: Metadata;
@@ -119,17 +117,6 @@ class QueryInner {
setDefaultQuery(): QueryInner {
return this;
}
-
- /**
- * Helper for updating with functions that expect a DatasetQuery object, or proxy to parent question
- */
- update(update?: QueryUpdateFn, ...args: any[]) {
- if (update) {
- return update(this.datasetQuery(), ...args);
- } else {
- return this.question().update(undefined, ...args);
- }
- }
}
export default class Query extends memoizeClass<QueryInner>("question")(
diff --git a/frontend/src/metabase-lib/lib/queries/structured/MBQLClause.ts b/frontend/src/metabase-lib/lib/queries/structured/MBQLClause.ts
index 18d1bcf6fe2..00da3b0e6e4 100644
--- a/frontend/src/metabase-lib/lib/queries/structured/MBQLClause.ts
+++ b/frontend/src/metabase-lib/lib/queries/structured/MBQLClause.ts
@@ -49,13 +49,6 @@ export default class MBQLArrayClause extends Array {
return this._index;
}
- /**
- * replaces the previous clause with this one and propagates an update, recursively
- */
- update(...args: any) {
- return this.replace(this).update(undefined, ...args);
- }
-
parent() {
return this.replace(this);
}
@@ -110,13 +103,6 @@ export class MBQLObjectClause {
return this._index;
}
- /**
- * replaces the previous clause with this one and propagates an update, recursively
- */
- update(...args: any) {
- return this.replace(this).update(undefined, ...args);
- }
-
parent() {
return this.replace(this);
}
diff --git a/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx b/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx
index cc1d716d531..943b6f3b012 100644
--- a/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx
+++ b/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx
@@ -11,6 +11,7 @@ import Tables from "metabase/entities/tables";
import GuiQueryEditor from "metabase/query_builder/components/GuiQueryEditor";
import * as Urls from "metabase/lib/urls";
import Question from "metabase-lib/lib/Question";
+import Query from "metabase-lib/lib/queries/Query";
import withTableMetadataLoaded from "../hoc/withTableMetadataLoaded";
@@ -75,6 +76,10 @@ class PartialQueryBuilder extends Component {
}
setDatasetQuery = datasetQuery => {
+ if (datasetQuery instanceof Query) {
+ datasetQuery = datasetQuery.datasetQuery();
+ }
+
this.props.onChange(datasetQuery.query);
this.props.updatePreviewSummary(datasetQuery);
};
diff --git a/frontend/src/metabase/modes/components/TimeseriesGroupingWidget.jsx b/frontend/src/metabase/modes/components/TimeseriesGroupingWidget.jsx
index 00aadd40b97..1caade1b9e5 100644
--- a/frontend/src/metabase/modes/components/TimeseriesGroupingWidget.jsx
+++ b/frontend/src/metabase/modes/components/TimeseriesGroupingWidget.jsx
@@ -9,20 +9,16 @@ import TimeGroupingPopover from "metabase/query_builder/components/TimeGroupingP
import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
import SelectButton from "metabase/core/components/SelectButton";
-// set the display automatically then run
-function updateAndRun(query) {
- query.question().setDefaultDisplay().update(null, { run: true });
-}
-
export default class TimeseriesGroupingWidget extends Component {
static propTypes = {
query: PropTypes.object.isRequired,
+ onChange: PropTypes.func.isRequired,
};
_popover;
render() {
- const { query } = this.props;
+ const { query, onChange } = this.props;
if (isStructured(query.datasetQuery())) {
const breakouts = query.breakouts();
@@ -49,9 +45,20 @@ export default class TimeseriesGroupingWidget extends Component {
d.isSameBaseDimension(dimension),
);
if (index >= 0) {
- updateAndRun(query.updateBreakout(index, dimension.mbql()));
+ const newQuestion = query
+ .updateBreakout(index, dimension.mbql())
+ .question()
+ .setDefaultDisplay();
+
+ onChange(newQuestion);
} else {
- updateAndRun(query.clearBreakouts().breakout(dimension.mbql()));
+ const newQuestion = query
+ .clearBreakouts()
+ .breakout(dimension.mbql())
+ .question()
+ .setDefaultDisplay();
+
+ onChange(newQuestion);
}
if (this._popover) {
this._popover.close();
diff --git a/frontend/src/metabase/modes/components/modes/TimeseriesMode.jsx b/frontend/src/metabase/modes/components/modes/TimeseriesMode.jsx
index e11f5ee9013..be4da8dff57 100644
--- a/frontend/src/metabase/modes/components/modes/TimeseriesMode.jsx
+++ b/frontend/src/metabase/modes/components/modes/TimeseriesMode.jsx
@@ -13,12 +13,21 @@ import PivotByCategoryDrill from "../drill/PivotByCategoryDrill";
import PivotByLocationDrill from "../drill/PivotByLocationDrill";
const TimeseriesModeFooter = props => {
+ const onChange = question => {
+ const { updateQuestion } = props;
+ updateQuestion(question, { run: true });
+ };
+
return (
<div className="flex layout-centered">
<span className="mr1">{t`View`}</span>
<TimeseriesFilterWidget {...props} card={props.lastRunCard} />
<span className="mx1">{t`by`}</span>
- <TimeseriesGroupingWidget {...props} card={props.lastRunCard} />
+ <TimeseriesGroupingWidget
+ {...props}
+ onChange={onChange}
+ card={props.lastRunCard}
+ />
</div>
);
};
diff --git a/frontend/src/metabase/query_builder/actions/core/core.js b/frontend/src/metabase/query_builder/actions/core/core.js
index fc53c482d40..224235b008b 100644
--- a/frontend/src/metabase/query_builder/actions/core/core.js
+++ b/frontend/src/metabase/query_builder/actions/core/core.js
@@ -18,6 +18,7 @@ import { openUrl } from "metabase/redux/app";
import Questions from "metabase/entities/questions";
import Databases from "metabase/entities/databases";
import { fetchAlertsForQuestion } from "metabase/alert/alert";
+import Query from "metabase-lib/lib/queries/Query";
import { trackNewQuestionSaved } from "../../analytics";
import {
@@ -160,6 +161,10 @@ export const navigateToNewCardInsideQB = createThunkAction(
// DEPRECATED, still used in a couple places
export const setDatasetQuery =
(datasetQuery, options) => (dispatch, getState) => {
+ if (datasetQuery instanceof Query) {
+ datasetQuery = datasetQuery.datasetQuery();
+ }
+
const question = getQuestion(getState());
dispatch(updateQuestion(question.setDatasetQuery(datasetQuery), options));
};
diff --git a/frontend/src/metabase/query_builder/components/ExtendedOptions.jsx b/frontend/src/metabase/query_builder/components/ExtendedOptions.jsx
index 221d228d383..1d957d20a4d 100644
--- a/frontend/src/metabase/query_builder/components/ExtendedOptions.jsx
+++ b/frontend/src/metabase/query_builder/components/ExtendedOptions.jsx
@@ -34,9 +34,10 @@ export class ExtendedOptionsPopover extends Component {
setExpression(name, expression, previousName) {
const { query, setDatasetQuery } = this.props;
- query
- .updateExpression(name, expression, previousName)
- .update(setDatasetQuery);
+
+ const newQuery = query.updateExpression(name, expression, previousName);
+ setDatasetQuery(newQuery);
+
this.setState({ editExpression: null });
MetabaseAnalytics.trackStructEvent(
"QueryBuilder",
@@ -47,7 +48,10 @@ export class ExtendedOptionsPopover extends Component {
removeExpression(name) {
const { query, setDatasetQuery } = this.props;
- query.removeExpression(name).update(setDatasetQuery);
+
+ const newQuery = query.removeExpression(name);
+ setDatasetQuery(newQuery);
+
this.setState({ editExpression: null });
MetabaseAnalytics.trackStructEvent("QueryBuilder", "Remove Expression");
@@ -55,7 +59,10 @@ export class ExtendedOptionsPopover extends Component {
setLimit = limit => {
const { query, setDatasetQuery } = this.props;
- query.updateLimit(limit).update(setDatasetQuery);
+
+ const newQuery = query.setLimit(limit);
+ setDatasetQuery(newQuery);
+
MetabaseAnalytics.trackStructEvent("QueryBuilder", "Set Limit", limit);
if (this.props.onClose) {
this.props.onClose();
@@ -78,9 +85,9 @@ export class ExtendedOptionsPopover extends Component {
tableMetadata={query.table()}
sort={sort}
fieldOptions={query.sortOptions(sort)}
- removeOrderBy={() => query.removeSort(index).update(setDatasetQuery)}
+ removeOrderBy={() => setDatasetQuery(query.removeSort(index))}
updateOrderBy={orderBy =>
- query.updateSort(index, orderBy).update(setDatasetQuery)
+ setDatasetQuery(query.updateSort(index, orderBy))
}
/>
));
@@ -90,7 +97,7 @@ export class ExtendedOptionsPopover extends Component {
<AddClauseButton
text={t`Pick a field to sort by`}
onClick={() => {
- query.sort(["asc", null]).update(setDatasetQuery);
+ setDatasetQuery(query.sort(["asc", null]));
}}
/>
);
diff --git a/frontend/src/metabase/query_builder/components/GuiQueryEditor.jsx b/frontend/src/metabase/query_builder/components/GuiQueryEditor.jsx
index 85de1b6f93d..beb7946429b 100644
--- a/frontend/src/metabase/query_builder/components/GuiQueryEditor.jsx
+++ b/frontend/src/metabase/query_builder/components/GuiQueryEditor.jsx
@@ -94,11 +94,9 @@ export default class GuiQueryEditor extends React.Component {
<FilterWidgetList
query={query}
filters={filters}
- removeFilter={index =>
- query.removeFilter(index).update(setDatasetQuery)
- }
+ removeFilter={index => setDatasetQuery(query.removeFilter(index))}
updateFilter={(index, filter) =>
- query.updateFilter(index, filter).update(setDatasetQuery)
+ setDatasetQuery(query.updateFilter(index, filter))
}
/>
);
@@ -135,9 +133,7 @@ export default class GuiQueryEditor extends React.Component {
<FilterPopover
isNew
query={query}
- onChangeFilter={filter =>
- query.filter(filter).update(setDatasetQuery)
- }
+ onChangeFilter={filter => setDatasetQuery(query.filter(filter))}
onClose={() => this.filterPopover.current.close()}
/>
</PopoverWithTrigger>
@@ -178,10 +174,8 @@ export default class GuiQueryEditor extends React.Component {
query={query}
onChangeAggregation={aggregation =>
aggregation
- ? query
- .updateAggregation(index, aggregation)
- .update(setDatasetQuery)
- : query.removeAggregation(index).update(setDatasetQuery)
+ ? setDatasetQuery(query.updateAggregation(index, aggregation))
+ : setDatasetQuery(query.removeAggregation(index))
}
showMetrics={false}
showRawData
@@ -241,8 +235,8 @@ export default class GuiQueryEditor extends React.Component {
breakoutOptions={query.breakoutOptions(breakout)}
onChangeBreakout={breakout =>
breakout
- ? query.updateBreakout(index, breakout).update(setDatasetQuery)
- : query.removeBreakout(index).update(setDatasetQuery)
+ ? setDatasetQuery(query.updateBreakout(index, breakout))
+ : setDatasetQuery(query.removeBreakout(index))
}
>
{this.renderAdd(index === 0 ? t`Add a grouping` : null)}
diff --git a/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx b/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx
index fed1b56b149..db4fabcd751 100644
--- a/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx
+++ b/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx
@@ -423,14 +423,15 @@ class NativeQueryEditor extends Component {
}, AUTOCOMPLETE_DEBOUNCE_DURATION);
onChange() {
- const { query } = this.props;
+ const { query, setDatasetQuery } = this.props;
if (this._editor && !this._localUpdate) {
this._updateSize();
if (query.queryText() !== this._editor.getValue()) {
- query
- .setQueryText(this._editor.getValue())
- .updateSnippetsWithIds(this.props.snippets)
- .update(this.props.setDatasetQuery);
+ setDatasetQuery(
+ query
+ .setQueryText(this._editor.getValue())
+ .updateSnippetsWithIds(this.props.snippets),
+ );
}
}
@@ -443,12 +444,9 @@ class NativeQueryEditor extends Component {
/// Change the Database we're currently editing a query for.
setDatabaseId = databaseId => {
- const { query } = this.props;
+ const { query, setDatasetQuery } = this.props;
if (query.databaseId() !== databaseId) {
- query
- .setDatabaseId(databaseId)
- .setDefaultCollection()
- .update(this.props.setDatasetQuery);
+ setDatasetQuery(query.setDatabaseId(databaseId).setDefaultCollection());
if (this._editor && !this.props.readOnly) {
// HACK: the cursor doesn't blink without this intended small delay
setTimeout(() => this._editor.focus(), 50);
@@ -458,18 +456,16 @@ class NativeQueryEditor extends Component {
setTableId = tableId => {
// TODO: push more of this into metabase-lib?
- const { query } = this.props;
+ const { query, setDatasetQuery } = this.props;
const table = query.metadata().table(tableId);
if (table?.name !== query.collection()) {
- query.setCollectionName(table.name).update(this.props.setDatasetQuery);
+ setDatasetQuery(query.setCollectionName(table.name));
}
};
setParameterIndex = (parameterId, parameterIndex) => {
const { query, setDatasetQuery } = this.props;
- query
- .setParameterIndex(parameterId, parameterIndex)
- .update(setDatasetQuery);
+ setDatasetQuery(query.setParameterIndex(parameterId, parameterIndex));
};
handleFilterButtonClick = () => {
@@ -492,6 +488,7 @@ class NativeQueryEditor extends Component {
snippetCollections = [],
resizable,
requireWriteback = false,
+ setDatasetQuery,
} = this.props;
const parameters = query.question().parameters();
@@ -568,9 +565,7 @@ class NativeQueryEditor extends Component {
<SnippetModal
onSnippetUpdate={(newSnippet, oldSnippet) => {
if (newSnippet.name !== oldSnippet.name) {
- query
- .updateSnippetNames([newSnippet])
- .update(this.props.setDatasetQuery);
+ setDatasetQuery(query.updateSnippetNames([newSnippet]));
}
}}
snippet={this.props.modalSnippet}
diff --git a/frontend/src/metabase/query_builder/components/QueryModals.jsx b/frontend/src/metabase/query_builder/components/QueryModals.jsx
index df6f6f91b60..4b7a5cf5c9d 100644
--- a/frontend/src/metabase/query_builder/components/QueryModals.jsx
+++ b/frontend/src/metabase/query_builder/components/QueryModals.jsx
@@ -54,6 +54,11 @@ class QueryModals extends React.Component {
}
};
+ onQueryChange = query => {
+ const question = query.question();
+ this.props.updateQuestion(question, { run: true });
+ };
+
render() {
const {
modal,
@@ -166,7 +171,11 @@ class QueryModals extends React.Component {
</Modal>
) : modal === MODAL_TYPES.FILTERS ? (
<Modal fit onClose={onCloseModal}>
- <BulkFilterModal question={question} onClose={onCloseModal} />
+ <BulkFilterModal
+ question={question}
+ onQueryChange={this.onQueryChange}
+ onClose={onCloseModal}
+ />
</Modal>
) : modal === MODAL_TYPES.HISTORY ? (
<Modal onClose={onCloseModal}>
diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx
index cf255729bd3..affd301f008 100644
--- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx
+++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterModal.tsx
@@ -36,12 +36,14 @@ import { fixBetweens, getSearchHits } from "./utils";
export interface BulkFilterModalProps {
question: Question;
+ onQueryChange: (query: StructuredQuery) => void;
onClose?: () => void;
}
const BulkFilterModal = ({
question,
onClose,
+ onQueryChange,
}: BulkFilterModalProps): JSX.Element | null => {
const [query, setQuery] = useState(getQuery(question));
const [isChanged, setIsChanged] = useState(false);
@@ -87,9 +89,9 @@ const BulkFilterModal = ({
const handleApplyQuery = useCallback(() => {
const preCleanedQuery = fixBetweens(query);
- preCleanedQuery.clean().update(undefined, { run: true });
+ onQueryChange(preCleanedQuery.clean());
onClose?.();
- }, [query, onClose]);
+ }, [query, onClose, onQueryChange]);
const clearFilters = () => {
setQuery(query.clearFilters());
diff --git a/frontend/src/metabase/query_builder/components/notebook/Notebook.jsx b/frontend/src/metabase/query_builder/components/notebook/Notebook.jsx
index d5756487ce7..8253263799c 100644
--- a/frontend/src/metabase/query_builder/components/notebook/Notebook.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/Notebook.jsx
@@ -16,6 +16,7 @@ export default function Notebook({ className, ...props }) {
isResultDirty,
runQuestionQuery,
setQueryBuilderMode,
+ updateQuestion,
hasVisualizeButton = true,
} = props;
@@ -26,7 +27,8 @@ export default function Notebook({ className, ...props }) {
if (cleanQuestion.display() === "table") {
cleanQuestion = cleanQuestion.setDefaultDisplay();
}
- await cleanQuestion.update();
+
+ await updateQuestion(cleanQuestion);
}
// vizualize switches the view to the question's visualization.
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookStep.jsx b/frontend/src/metabase/query_builder/components/notebook/NotebookStep.jsx
index 5a0438f500c..857e635919c 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookStep.jsx
@@ -166,7 +166,7 @@ export default class NotebookStep extends React.Component {
name="close"
className="ml-auto cursor-pointer text-light text-medium-hover hover-child"
tooltip={t`Remove`}
- onClick={() => step.revert(step.query).update(updateQuery)}
+ onClick={() => updateQuery(step.revert(step.query))}
data-testid="remove-step"
/>
</StepHeader>
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps.jsx b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps.jsx
index f51e92eca6f..e78904868ba 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps.jsx
@@ -38,7 +38,7 @@ export default class NotebookSteps extends React.Component {
};
render() {
- const { question, className } = this.props;
+ const { question, className, updateQuestion } = this.props;
const { openSteps, lastOpenedStep } = this.state;
if (!question) {
@@ -51,8 +51,10 @@ export default class NotebookSteps extends React.Component {
<div className={cx(className, "pt3")}>
{steps.map((step, index) => {
// pass a version of updateQuery that cleans subsequent steps etc
- const updateQuery = async datasetQuery => {
- await step.update(datasetQuery).update();
+ const updateQuery = async query => {
+ const datasetQuery = query.datasetQuery();
+ const updatedQuery = step.update(datasetQuery);
+ await updateQuestion(updatedQuery.question());
// mark the step as "closed" since we can assume it's been added or removed by the updateQuery
this.closeStep(step.id);
};
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep.jsx
index 065aa1b1ab2..35647cb3152 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep.jsx
@@ -36,13 +36,13 @@ export default function AggregateStep({
aggregation={aggregation}
onChangeAggregation={newAggregation =>
aggregation
- ? aggregation.replace(newAggregation).update(updateQuery)
- : query.aggregate(newAggregation).update(updateQuery)
+ ? updateQuery(aggregation.replace(newAggregation))
+ : updateQuery(query.aggregate(newAggregation))
}
/>
)}
isLastOpened={isLastOpened}
- onRemove={aggregation => aggregation.remove().update(updateQuery)}
+ onRemove={aggregation => updateQuery(aggregation.remove())}
canRemove={aggregation => aggregation.canRemove()}
/>
);
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep.jsx
index 8b8fa96e334..95a448262eb 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep.jsx
@@ -36,13 +36,13 @@ export default function BreakoutStep({
breakout={breakout}
onChangeBreakout={newBreakout =>
breakout
- ? breakout.replace(newBreakout).update(updateQuery)
- : query.breakout(newBreakout).update(updateQuery)
+ ? updateQuery(breakout.replace(newBreakout))
+ : updateQuery(query.breakout(newBreakout))
}
/>
)}
isLastOpened={isLastOpened}
- onRemove={breakout => breakout.remove().update(updateQuery)}
+ onRemove={breakout => updateQuery(breakout.remove())}
/>
);
}
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx
index 0761e7aa9f4..dc8e4784487 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx
@@ -54,7 +54,7 @@ function DataStep({ color, query, updateQuery }) {
selectedDatabaseId={query.databaseId()}
selectedTableId={query.tableId()}
setSourceTableFn={tableId =>
- query.setTableId(tableId).setDefaultQuery().update(updateQuery)
+ updateQuery(query.setTableId(tableId).setDefaultQuery())
}
isInitiallyOpen={!query.tableId()}
triggerElement={
@@ -80,12 +80,12 @@ const DataFieldsPicker = ({ query, updateQuery, ...props }) => {
const fields = query.fields();
const handleSelectNone = () => {
- query
- .setFields([
+ updateQuery(
+ query.setFields([
dimensions[0].mbql(),
...expressionDimensions.map(d => d.mbql()),
- ])
- .update(updateQuery);
+ ]),
+ );
};
const handleToggleDimension = dimension => {
@@ -99,7 +99,7 @@ const DataFieldsPicker = ({ query, updateQuery, ...props }) => {
})
.map(d => d.mbql());
- query.setFields(newFields).update(updateQuery);
+ updateQuery(query.setFields(newFields));
};
const hasOneColumnSelected = fields.filter(isLocalField).length === 1;
@@ -110,7 +110,7 @@ const DataFieldsPicker = ({ query, updateQuery, ...props }) => {
dimensions={dimensions}
selectedDimensions={selectedDimensions}
isAll={!fields || fields.length === 0}
- onSelectAll={() => query.clearFields().update(updateQuery)}
+ onSelectAll={() => updateQuery(query.clearFields())}
onSelectNone={handleSelectNone}
disableSelected={hasOneColumnSelected}
onToggleDimension={handleToggleDimension}
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.jsx
index 1ed4d2ae480..e1abb412ddd 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.jsx
@@ -23,16 +23,16 @@ export default function ExpressionStep({
expression={expression}
onChangeExpression={(newName, newExpression) =>
expression
- ? query
- .updateExpression(newName, newExpression, name)
- .update(updateQuery)
- : query.addExpression(newName, newExpression).update(updateQuery)
+ ? updateQuery(
+ query.updateExpression(newName, newExpression, name),
+ )
+ : updateQuery(query.addExpression(newName, newExpression))
}
/>
)}
isLastOpened={isLastOpened}
onRemove={([name, expression]) =>
- query.removeExpression(name).update(updateQuery)
+ updateQuery(query.removeExpression(name))
}
/>
);
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep.jsx
index 9987ee3e91c..73e0142e177 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep.jsx
@@ -23,13 +23,13 @@ export default function FilterStep({
filter={filter}
onChangeFilter={newFilter =>
filter
- ? filter.replace(newFilter).update(updateQuery)
- : query.filter(newFilter).update(updateQuery)
+ ? updateQuery(filter.replace(newFilter))
+ : updateQuery(query.filter(newFilter))
}
/>
)}
isLastOpened={isLastOpened}
- onRemove={filter => filter.remove().update(updateQuery)}
+ onRemove={filter => updateQuery(filter.remove())}
/>
);
}
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx
index e63aa29273f..34ab183c389 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx
@@ -95,7 +95,7 @@ export default function JoinStep({
const valid = _.all(joins, join => join.isValid());
function addNewJoinClause() {
- query.join(new Join({ fields: "all" })).update(updateQuery);
+ updateQuery(query.join(new Join({ fields: "all" })));
}
return (
@@ -176,33 +176,35 @@ function JoinClause({ color, join, updateQuery, showRemove }) {
}
function onParentDimensionChange(index, fieldRef, { overwrite } = {}) {
- join
- .setParentDimension({
- index,
- dimension: fieldRef,
- overwriteTemporalUnit: overwrite,
- })
- .setDefaultAlias()
- .parent()
- .update(updateQuery);
+ updateQuery(
+ join
+ .setParentDimension({
+ index,
+ dimension: fieldRef,
+ overwriteTemporalUnit: overwrite,
+ })
+ .setDefaultAlias()
+ .parent(),
+ );
if (!join.joinDimensions()[index]) {
joinDimensionPickersRef.current[index]?.open();
}
}
function onJoinDimensionChange(index, fieldRef, { overwrite } = {}) {
- join
- .setJoinDimension({
- index,
- dimension: fieldRef,
- overwriteTemporalUnit: overwrite,
- })
- .parent()
- .update(updateQuery);
+ updateQuery(
+ join
+ .setJoinDimension({
+ index,
+ dimension: fieldRef,
+ overwriteTemporalUnit: overwrite,
+ })
+ .parent(),
+ );
}
function addNewDimensionsPair(index) {
- join.addEmptyDimensionsPair().parent().update(updateQuery);
+ updateQuery(join.addEmptyDimensionsPair().parent());
// Need to wait, so a new dimensions pair renders
// and a corresponding ref is created, so we can reference it here
@@ -212,7 +214,7 @@ function JoinClause({ color, join, updateQuery, showRemove }) {
}
function removeJoin() {
- join.remove().update(updateQuery);
+ updateQuery(join.remove());
}
return (
@@ -252,25 +254,23 @@ function JoinClause({ color, join, updateQuery, showRemove }) {
)?.name;
function removeParentDimension() {
- join
- .setParentDimension({ index, dimension: null })
- .parent()
- .update(updateQuery);
+ updateQuery(
+ join.setParentDimension({ index, dimension: null }).parent(),
+ );
}
function removeJoinDimension() {
- join
- .setJoinDimension({ index, dimension: null })
- .parent()
- .update(updateQuery);
+ updateQuery(
+ join.setJoinDimension({ index, dimension: null }).parent(),
+ );
}
function removeDimensionPair() {
- join.removeCondition(index).parent().update(updateQuery);
+ updateQuery(join.removeCondition(index).parent());
}
function updateOperator({ target: { value } }) {
- join.setOperator(index, value).parent().update(updateQuery);
+ updateQuery(join.setOperator(index, value).parent());
}
return (
@@ -417,7 +417,7 @@ function JoinTablePicker({
.setJoinSourceTableId(tableId)
.setDefaultCondition()
.setDefaultAlias();
- newJoin.parent().update(updateQuery);
+ updateQuery(newJoin.parent());
onSourceTableSet(newJoin);
}
@@ -469,7 +469,7 @@ function JoinTypePicker({ join, color, updateQuery }) {
const strategyOption = join.strategyOption();
function onChange(strategy) {
- join.setStrategy(strategy).parent().update(updateQuery);
+ updateQuery(join.setStrategy(strategy).parent());
}
return (
@@ -669,28 +669,29 @@ const JoinFieldsPicker = ({ join, updateQuery, ...props }) => {
const selected = new Set(selectedDimensions.map(d => d.key()));
function onSelectAll() {
- join.setFields("all").parent().update(updateQuery);
+ updateQuery(join.setFields("all").parent());
}
function onSelectNone() {
- join.setFields("none").parent().update(updateQuery);
+ updateQuery(join.setFields("none").parent());
}
function onToggleDimension(dimension) {
- join
- .setFields(
- dimensions
- .filter(d => {
- if (d === dimension) {
- return !selected.has(d.key());
- } else {
- return selected.has(d.key());
- }
- })
- .map(d => d.mbql()),
- )
- .parent()
- .update(updateQuery);
+ updateQuery(
+ join
+ .setFields(
+ dimensions
+ .filter(d => {
+ if (d === dimension) {
+ return !selected.has(d.key());
+ } else {
+ return selected.has(d.key());
+ }
+ })
+ .map(d => d.mbql()),
+ )
+ .parent(),
+ );
}
return (
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/LimitStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/LimitStep.jsx
index 386e48520d1..4753a86a871 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/LimitStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/LimitStep.jsx
@@ -24,7 +24,7 @@ export default function LimitStep({
onChange={e => {
const limit = parseInt(e.target.value, 0);
if (limit >= 1) {
- query.updateLimit(limit).update(updateQuery);
+ updateQuery(query.updateLimit(limit));
}
}}
/>
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/SortStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/SortStep.jsx
index 4fc7115a659..87a55fa8d2c 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/SortStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/SortStep.jsx
@@ -20,9 +20,12 @@ export default function SortStep({
className="flex align-center"
onClick={e => {
e.stopPropagation();
- query
- .updateSort(index, [sort[0] === "asc" ? "desc" : "asc", sort[1]])
- .update(updateQuery);
+ updateQuery(
+ query.updateSort(index, [
+ sort[0] === "asc" ? "desc" : "asc",
+ sort[1],
+ ]),
+ );
}}
>
<Icon
@@ -38,13 +41,13 @@ export default function SortStep({
sort={sort}
onChangeSort={newSort =>
sort
- ? query.updateSort(index, newSort).update(updateQuery)
- : query.sort(newSort).update(updateQuery)
+ ? updateQuery(query.updateSort(index, newSort))
+ : updateQuery(query.sort(newSort))
}
/>
)}
isLastOpened={isLastOpened}
- onRemove={(sort, index) => query.removeSort(index).update(updateQuery)}
+ onRemove={(sort, index) => updateQuery(query.removeSort(index))}
/>
);
}
diff --git a/frontend/src/metabase/query_builder/components/view/NativeQueryButton.jsx b/frontend/src/metabase/query_builder/components/view/NativeQueryButton.jsx
index e14280f27d8..a6c867d898b 100644
--- a/frontend/src/metabase/query_builder/components/view/NativeQueryButton.jsx
+++ b/frontend/src/metabase/query_builder/components/view/NativeQueryButton.jsx
@@ -56,13 +56,17 @@ export default class NativeQueryButton extends React.Component {
this.setState({ open: false });
};
handleConvert = () => {
- this.props.question
- .setDatasetQuery({
- type: "native",
- native: { query: this.getFormattedQuery() },
- database: this.state.datasetQuery.database,
- })
- .update(null, { shouldUpdateUrl: true });
+ const { question, updateQuestion } = this.props;
+
+ const newQuestion = question.setDatasetQuery({
+ type: "native",
+ native: { query: this.getFormattedQuery() },
+ database: this.state.datasetQuery.database,
+ });
+
+ updateQuestion(newQuestion, {
+ shouldUpdateUrl: true,
+ });
};
getFormattedQuery() {
diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDataSelector.jsx b/frontend/src/metabase/query_builder/components/view/QuestionDataSelector.jsx
index 1157fe9832a..f4afc303040 100644
--- a/frontend/src/metabase/query_builder/components/view/QuestionDataSelector.jsx
+++ b/frontend/src/metabase/query_builder/components/view/QuestionDataSelector.jsx
@@ -3,7 +3,11 @@ import React from "react";
import { DataSourceSelector } from "metabase/query_builder/components/DataSelector";
-export default function QuestionDataSelector({ query, triggerElement }) {
+export default function QuestionDataSelector({
+ query,
+ updateQuestion,
+ triggerElement,
+}) {
return (
<DataSourceSelector
containerClassName="DataPopoverContainer"
@@ -12,7 +16,9 @@ export default function QuestionDataSelector({ query, triggerElement }) {
selectedDatabaseId={query.databaseId()}
selectedTableId={query.tableId()}
setSourceTableFn={tableId =>
- query.setTableId(tableId).setDefaultQuery().update(null, { run: true })
+ updateQuestion(query.setTableId(tableId).setDefaultQuery().question(), {
+ run: true,
+ })
}
triggerElement={triggerElement}
isOpen
diff --git a/frontend/src/metabase/query_builder/components/view/QuestionFilters.jsx b/frontend/src/metabase/query_builder/components/view/QuestionFilters.jsx
index bcf04213ff7..4dca876c189 100644
--- a/frontend/src/metabase/query_builder/components/view/QuestionFilters.jsx
+++ b/frontend/src/metabase/query_builder/components/view/QuestionFilters.jsx
@@ -25,12 +25,14 @@ export default function QuestionFilters({
expanded,
onExpand,
onCollapse,
+ onQueryChange,
}) {
const query = question.query();
const filters = query.topLevelFilters();
if (filters.length === 0) {
return null;
}
+
return (
<div className={className}>
<div className="flex flex-wrap align-center mbn1 mrn1">
@@ -56,9 +58,7 @@ export default function QuestionFilters({
key={index}
triggerElement={
<FilterPill
- onRemove={() =>
- filter.remove().rootQuery().update(null, { run: true })
- }
+ onRemove={() => onQueryChange(filter.remove().rootQuery())}
>
{filter.displayName()}
</FilterPill>
@@ -71,7 +71,7 @@ export default function QuestionFilters({
query={query}
filter={filter}
onChangeFilter={newFilter =>
- newFilter.replace().rootQuery().update(null, { run: true })
+ onQueryChange(newFilter.replace().rootQuery())
}
className="scroll-y"
/>
@@ -88,6 +88,7 @@ export function FilterHeaderToggle({
onExpand,
expanded,
onCollapse,
+ onQueryChange,
}) {
const query = question.query();
const filters = query.topLevelFilters();
@@ -112,7 +113,7 @@ export function FilterHeaderToggle({
);
}
-export function FilterHeader({ question, expanded }) {
+export function FilterHeader({ question, expanded, onQueryChange }) {
const query = question.query();
const filters = query.topLevelFilters();
if (filters.length === 0 || !expanded) {
@@ -126,9 +127,7 @@ export function FilterHeader({ question, expanded }) {
key={index}
triggerElement={
<FilterPill
- onRemove={() =>
- filter.remove().rootQuery().update(null, { run: true })
- }
+ onRemove={() => onQueryChange(filter.remove().rootQuery())}
>
{filter.displayName()}
</FilterPill>
@@ -141,7 +140,7 @@ export function FilterHeader({ question, expanded }) {
query={query}
filter={filter}
onChangeFilter={newFilter =>
- newFilter.replace().rootQuery().update(null, { run: true })
+ onQueryChange(newFilter.replace().rootQuery())
}
className="scroll-y"
/>
diff --git a/frontend/src/metabase/query_builder/components/view/QuestionRowCount.jsx b/frontend/src/metabase/query_builder/components/view/QuestionRowCount.jsx
index fca0bd339a2..50a16e98614 100644
--- a/frontend/src/metabase/query_builder/components/view/QuestionRowCount.jsx
+++ b/frontend/src/metabase/query_builder/components/view/QuestionRowCount.jsx
@@ -16,6 +16,7 @@ const QuestionRowCount = ({
result,
className,
isResultDirty,
+ onQueryChange,
...props
}) => {
const formatRowCount = count => {
@@ -71,9 +72,9 @@ const QuestionRowCount = ({
limit={limit}
onChangeLimit={limit => {
if (limit > 0) {
- query.updateLimit(limit).update();
+ onQueryChange(query.updateLimit(limit));
} else {
- query.clearLimit().update();
+ onQueryChange(query.clearLimit());
}
}}
onClose={onClose}
diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx
index d339f7ec596..49d8875e337 100644
--- a/frontend/src/metabase/query_builder/components/view/View.jsx
+++ b/frontend/src/metabase/query_builder/components/view/View.jsx
@@ -60,6 +60,10 @@ class View extends React.Component {
...DEFAULT_POPOVER_STATE,
};
+ onUpdateQuery = (query, options = { run: true }) => {
+ this.props.updateQuestion(query.question(), options);
+ };
+
handleAddSeries = e => {
this.setState({
...DEFAULT_POPOVER_STATE,
@@ -77,7 +81,7 @@ class View extends React.Component {
handleRemoveSeries = (e, index) => {
const { query } = this.props;
- query.removeAggregation(index).update(null, { run: true });
+ this.onUpdateQuery(query.removeAggregation(index));
};
handleEditBreakout = (e, index) => {
@@ -98,11 +102,11 @@ class View extends React.Component {
const { query } = this.props;
const { aggregationIndex } = this.state;
if (aggregationIndex != null) {
- query
- .updateAggregation(aggregationIndex, aggregation)
- .update(null, { run: true });
+ this.onUpdateQuery(
+ query.updateAggregation(aggregationIndex, aggregation),
+ );
} else {
- query.aggregate(aggregation).update(null, { run: true });
+ this.onUpdateQuery(query.aggregate(aggregation));
}
this.handleClosePopover();
};
@@ -111,9 +115,9 @@ class View extends React.Component {
const { query } = this.props;
const { breakoutIndex } = this.state;
if (breakoutIndex != null) {
- query.updateBreakout(breakoutIndex, breakout).update(null, { run: true });
+ this.onUpdateQuery(query.updateBreakout(breakoutIndex, breakout));
} else {
- query.breakout(breakout).update(null, { run: true });
+ this.onUpdateQuery(query.breakout(breakout));
}
this.handleClosePopover();
};
@@ -148,6 +152,7 @@ class View extends React.Component {
isShowingTimelineSidebar,
isShowingQuestionInfoSidebar,
runQuestionQuery,
+ updateQuestion,
visibleTimelineIds,
selectedTimelineEventIds,
xDomain,
@@ -170,6 +175,7 @@ class View extends React.Component {
onClose={onCloseSummary}
isResultDirty={isResultDirty}
runQuestionQuery={runQuestionQuery}
+ updateQuestion={updateQuestion}
/>
);
}
@@ -428,6 +434,7 @@ class View extends React.Component {
onConfirmToast,
isShowingToaster,
isHeaderVisible,
+ updateQuestion,
} = this.props;
// if we don't have a card at all or no databases then we are initializing, so keep it simple
@@ -441,7 +448,13 @@ class View extends React.Component {
isStructured && !query.sourceTableId() && !query.sourceQuery();
if (isNewQuestion && queryBuilderMode === "view") {
- return <NewQuestionView query={query} className="full-height" />;
+ return (
+ <NewQuestionView
+ query={query}
+ updateQuestion={updateQuestion}
+ className="full-height"
+ />
+ );
}
if (card.dataset && queryBuilderMode === "dataset") {
diff --git a/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx b/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx
index 14e56f986a6..f3badfa472f 100644
--- a/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx
+++ b/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx
@@ -2,20 +2,23 @@ import React from "react";
import { t } from "ttag";
import Subhead from "metabase/components/type/Subhead";
+import type { updateQuestion } from "metabase/query_builder/actions";
import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
import QuestionDataSelector from "../../QuestionDataSelector";
type Props = {
query: StructuredQuery;
+ updateQuestion: typeof updateQuestion;
};
-function NewQuestionView({ query }: Props) {
+function NewQuestionView({ query, updateQuestion }: Props) {
return (
<div className="full-height">
<div className="p4 mx2">
<QuestionDataSelector
query={query}
+ updateQuestion={updateQuestion}
triggerElement={
<Subhead className="mb2">{t`Pick your data`}</Subhead>
}
diff --git a/frontend/src/metabase/query_builder/components/view/ViewFooter.jsx b/frontend/src/metabase/query_builder/components/view/ViewFooter.jsx
index 4c0c276171b..8bb2455247d 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewFooter.jsx
+++ b/frontend/src/metabase/query_builder/components/view/ViewFooter.jsx
@@ -1,5 +1,5 @@
/* eslint-disable react/prop-types */
-import React from "react";
+import React, { useCallback } from "react";
import { t } from "ttag";
import cx from "classnames";
@@ -51,7 +51,16 @@ const ViewFooter = ({
isShowingTimelineSidebar,
onOpenTimelines,
onCloseTimelines,
+ updateQuestion,
}) => {
+ const onQueryChange = useCallback(
+ query => {
+ const newQuestion = query.question();
+ updateQuestion(newQuestion, { run: true });
+ },
+ [updateQuestion],
+ );
+
if (!result) {
return null;
}
@@ -117,6 +126,7 @@ const ViewFooter = ({
question={question}
isResultDirty={isResultDirty}
result={result}
+ onQueryChange={onQueryChange}
/>
),
QuestionLastUpdated.shouldRender({ result }) && (
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
index 2dbf8897689..84ae8f94b1a 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
@@ -69,6 +69,7 @@ const viewTitleHeaderPropTypes = {
runQuestionQuery: PropTypes.func,
cancelQuery: PropTypes.func,
+ updateQuestion: PropTypes.func,
onOpenModal: PropTypes.func,
onEditSummary: PropTypes.func,
@@ -80,7 +81,7 @@ const viewTitleHeaderPropTypes = {
};
export function ViewTitleHeader(props) {
- const { question, className, style, isNavBarOpen } = props;
+ const { question, className, style, isNavBarOpen, updateQuestion } = props;
const [
areFiltersExpanded,
@@ -110,6 +111,13 @@ export function ViewTitleHeader(props) {
const isSummarized =
isStructured && question.query().topLevelQuery().hasAggregations();
+ const onQueryChange = useCallback(
+ newQuery => {
+ updateQuestion(newQuery.question(), { run: true });
+ },
+ [updateQuestion],
+ );
+
return (
<>
<ViewHeaderContainer
@@ -136,6 +144,7 @@ export function ViewTitleHeader(props) {
areFiltersExpanded={areFiltersExpanded}
onExpandFilters={expandFilters}
onCollapseFilters={collapseFilters}
+ onQueryChange={onQueryChange}
/>
</ViewHeaderContainer>
{QuestionFilters.shouldRender(props) && (
@@ -143,6 +152,7 @@ export function ViewTitleHeader(props) {
{...props}
expanded={areFiltersExpanded}
question={question}
+ onQueryChange={onQueryChange}
/>
)}
</>
@@ -322,6 +332,7 @@ ViewTitleHeaderRightSide.propTypes = {
isResultDirty: PropTypes.bool,
isActionListVisible: PropTypes.bool,
runQuestionQuery: PropTypes.func,
+ updateQuestion: PropTypes.func.isRequired,
cancelQuery: PropTypes.func,
onOpenModal: PropTypes.func,
onEditSummary: PropTypes.func,
@@ -339,6 +350,7 @@ ViewTitleHeaderRightSide.propTypes = {
onCloseQuestionInfo: PropTypes.func,
isShowingQuestionInfoSidebar: PropTypes.bool,
onModelPersistenceChange: PropTypes.bool,
+ onQueryChange: PropTypes.func,
};
function ViewTitleHeaderRightSide(props) {
@@ -359,6 +371,7 @@ function ViewTitleHeaderRightSide(props) {
isResultDirty,
isActionListVisible,
runQuestionQuery,
+ updateQuestion,
cancelQuery,
onOpenModal,
onEditSummary,
@@ -374,6 +387,7 @@ function ViewTitleHeaderRightSide(props) {
onCloseQuestionInfo,
onOpenQuestionInfo,
onModelPersistenceChange,
+ onQueryChange,
} = props;
const isShowingNotebook = queryBuilderMode === "notebook";
const query = question.query();
@@ -416,6 +430,7 @@ function ViewTitleHeaderRightSide(props) {
expanded={areFiltersExpanded}
onExpand={onExpandFilters}
onCollapse={onCollapseFilters}
+ onQueryChange={onQueryChange}
/>
)}
{QuestionFilterWidget.shouldRender(props) && (
@@ -453,6 +468,7 @@ function ViewTitleHeaderRightSide(props) {
<NativeQueryButton
size={16}
question={question}
+ updateQuestion={updateQuestion}
data-metabase-event="Notebook Mode; Convert to SQL Click"
/>
</ViewHeaderIconButtonContainer>
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/ChartTypeSidebar.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/ChartTypeSidebar.jsx
index 9f36750575f..bdec6daee64 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/ChartTypeSidebar.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/ChartTypeSidebar.jsx
@@ -25,6 +25,7 @@ const ChartTypeSidebar = ({
result,
onOpenChartSettings,
onCloseChartType,
+ updateQuestion,
isShowingChartTypeSidebar,
setUIControls,
...props
@@ -64,13 +65,14 @@ const ChartTypeSidebar = ({
visualization.isSensible(result.data, props.query)
}
onClick={() => {
- question
+ const newQuestion = question
.setDisplay(type)
- .lockDisplay(true) // prevent viz auto-selection
- .update(null, {
- reload: false,
- shouldUpdateUrl: question.query().isEditable(),
- });
+ .lockDisplay(true); // prevent viz auto-selection
+
+ updateQuestion(newQuestion, {
+ reload: false,
+ shouldUpdateUrl: question.query().isEditable(),
+ });
onOpenChartSettings({ section: t`Data` });
setUIControls({ isShowingRawTable: false });
}}
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.jsx
index 8b10752865b..159eb3f10fb 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.jsx
@@ -7,17 +7,21 @@ import AggregationPopover from "metabase/query_builder/components/AggregationPop
import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
import Icon from "metabase/components/Icon";
-import { updateAndRunQuery } from "../utils";
import { AddAggregationButtonRoot } from "./AddAggregationButton.styled";
const propTypes = {
query: PropTypes.object,
shouldShowLabel: PropTypes.boolean,
+ updateAndRunQuery: PropTypes.func.isRequired,
};
const LABEL = t`Add a metric`;
-export const AddAggregationButton = ({ query, shouldShowLabel = false }) => {
+export const AddAggregationButton = ({
+ query,
+ shouldShowLabel = false,
+ updateAndRunQuery,
+}) => {
return (
<PopoverWithTrigger
triggerElement={
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.jsx
index 34442cef3a4..5930c004f06 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.jsx
@@ -6,7 +6,6 @@ import AggregationPopover from "metabase/query_builder/components/AggregationPop
import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
import Icon from "metabase/components/Icon";
-import { updateAndRunQuery } from "../utils";
import { AggregationItemRoot } from "./AggregationItem.styled";
const propTypes = {
@@ -15,9 +14,16 @@ const propTypes = {
index: PropTypes.number.isRequired,
query: PropTypes.object,
onRemove: PropTypes.func,
+ updateAndRunQuery: PropTypes.func.isRequired,
};
-export const AggregationItem = ({ aggregation, index, query, onRemove }) => {
+export const AggregationItem = ({
+ aggregation,
+ index,
+ query,
+ onRemove,
+ updateAndRunQuery,
+}) => {
return (
<PopoverWithTrigger
triggerClasses="flex-full"
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.jsx
index eafc98408e2..10a17ba27d8 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from "react";
+import React, { useState, useEffect, useCallback } from "react";
import PropTypes from "prop-types";
import { t } from "ttag";
import cx from "classnames";
@@ -6,7 +6,6 @@ import cx from "classnames";
import { color } from "metabase/lib/colors";
import SidebarContent from "metabase/query_builder/components/SidebarContent";
-import { updateAndRunQuery } from "./utils";
import { AddAggregationButton } from "./AddAggregationButton";
import { AggregationItem } from "./AggregationItem";
import { DimensionList } from "./DimensionList";
@@ -20,6 +19,7 @@ const propTypes = {
question: PropTypes.object,
isResultDirty: PropTypes.bool,
runQuestionQuery: PropTypes.func.isRequired,
+ updateQuestion: PropTypes.func.isRequired,
onClose: PropTypes.func.isRequired,
className: PropTypes.string,
};
@@ -35,6 +35,7 @@ const SummarizeSidebar = ({
question,
isResultDirty,
runQuestionQuery,
+ updateQuestion,
onClose,
className,
}) => {
@@ -45,6 +46,15 @@ const SummarizeSidebar = ({
getQuery(question, isDefaultAggregationRemoved),
);
+ const updateAndRunQuery = useCallback(
+ query => {
+ updateQuestion(query.question().setDefaultDisplay(), {
+ run: true,
+ });
+ },
+ [updateQuestion],
+ );
+
useEffect(() => {
const nextQuery = getQuery(question, isDefaultAggregationRemoved);
setQuery(nextQuery);
@@ -96,7 +106,7 @@ const SummarizeSidebar = ({
runQuestionQuery();
}
if (hasDefaultAggregation) {
- query.update(null, { run: true });
+ updateQuestion(query.question(), { run: true });
}
onClose();
}}
@@ -110,11 +120,13 @@ const SummarizeSidebar = ({
index={index}
query={query}
onRemove={handleAggregationRemove}
+ updateAndRunQuery={updateAndRunQuery}
/>
))}
<AddAggregationButton
query={query}
shouldShowLabel={!hasAggregations}
+ updateAndRunQuery={updateAndRunQuery}
/>
</AggregationsContainer>
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/utils.js b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/utils.js
deleted file mode 100644
index 39c56fdd3b0..00000000000
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/utils.js
+++ /dev/null
@@ -1,4 +0,0 @@
-// set the display automatically then run
-export function updateAndRunQuery(query) {
- query.question().setDefaultDisplay().update(null, { run: true });
-}
diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
index 1e8b10ea526..649cd3d6e2b 100644
--- a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
+++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
@@ -206,7 +206,6 @@ function QueryBuilder(props) {
initializeQB,
apiCreateQuestion,
apiUpdateQuestion,
- updateQuestion,
updateUrl,
locationChanged,
onChangeLocation,
@@ -357,12 +356,6 @@ function QueryBuilder(props) {
}
}, [location, params, previousLocation, locationChanged]);
- useEffect(() => {
- if (question) {
- question._update = updateQuestion;
- }
- });
-
const [isShowingToaster, setIsShowingToaster] = useState(false);
const { isRunning } = uiControls;
--
GitLab