Merge pull request #6877 from metabase/update-dataselector-properly

Update DataSelector properly when props change

frontend/src/metabase/parameters/components/widgets/TextWidget.jsx

@@ -75,7 +75,7 @@ export default class TextWidget extends Component {
                     focusChanged(false);
                     this.setState({ value: this.props.value });
                 }}
-                placeholder={isEditing ? t`"Enter a default value...` : defaultPlaceholder}
+                placeholder={isEditing ? t`Enter a default value...` : defaultPlaceholder}
             />
         );
     }

frontend/src/metabase/query_builder/components/DataSelector.jsx

@@ -37,7 +37,7 @@ export const SchemaTableAndSegmentDataSelector = (props) =>
         getTriggerElementContent={SchemaAndSegmentTriggerContent}
         {...props}
     />
-const SchemaAndSegmentTriggerContent = ({ selectedTable, selectedSegment }) => {
+export const SchemaAndSegmentTriggerContent = ({ selectedTable, selectedSegment }) => {
     if (selectedTable) {
         return  <span className="text-grey no-decoration">{selectedTable.display_name || selectedTable.name}</span>;
     } else if (selectedSegment) {
@@ -53,7 +53,7 @@ export const DatabaseDataSelector = (props) =>
         getTriggerElementContent={DatabaseTriggerContent}
         {...props}
     />
-const DatabaseTriggerContent = ({ selectedDatabase }) =>
+export const DatabaseTriggerContent = ({ selectedDatabase }) =>
     selectedDatabase
         ? <span className="text-grey no-decoration">{selectedDatabase.name}</span>
         : <span className="text-grey-4 no-decoration">{t`Select a database`}</span>
@@ -66,7 +66,7 @@ export const SchemaTableAndFieldDataSelector = (props) =>
         renderAsSelect={true}
         {...props}
     />
-const FieldTriggerContent = ({ selectedDatabase, selectedField }) => {
+export const FieldTriggerContent = ({ selectedDatabase, selectedField }) => {
     if (!selectedField || !selectedField.table) {
         return <span className="flex-full text-grey-4 no-decoration">{t`Select...`}</span>
     } else {
@@ -94,7 +94,7 @@ export const SchemaAndTableDataSelector = (props) =>
         getTriggerElementContent={TableTriggerContent}
         {...props}
     />
-const TableTriggerContent = ({ selectedTable }) =>
+export const TableTriggerContent = ({ selectedTable }) =>
     selectedTable
         ? <span className="text-grey no-decoration">{selectedTable.display_name || selectedTable.name}</span>
         : <span className="text-grey-4 no-decoration">{t`Select a table`}</span>
@@ -102,8 +102,16 @@ const TableTriggerContent = ({ selectedTable }) =>
 @connect(state => ({metadata: getMetadata(state)}), { fetchTableMetadata })
 export default class DataSelector extends Component {
     constructor(props) {
-        super();
+        super()
 
+        this.state = {
+            ...this.getStepsAndSelectedEntities(props),
+            activeStep: null,
+            isLoading: false
+        }
+    }
+
+    getStepsAndSelectedEntities = (props) => {
         let selectedSchema, selectedTable;
         let selectedDatabaseId = props.selectedDatabaseId;
         // augment databases with schemas
@@ -151,22 +159,19 @@ export default class DataSelector extends Component {
         const selectedSegment = selectedSegmentId ? props.segments.find(segment => segment.id === selectedSegmentId) : null;
         const selectedField = props.selectedFieldId ? props.metadata.fields[props.selectedFieldId] : null
 
-        this.state = {
+        return {
             databases,
             selectedDatabase,
             selectedSchema,
             selectedTable,
             selectedSegment,
             selectedField,
-            activeStep: null,
-            steps,
-            isLoading: false,
-        };
+            steps
+        }
     }
 
     static propTypes = {
         selectedDatabaseId: PropTypes.number,
-        selectedSchemaId: PropTypes.number,
         selectedTableId: PropTypes.number,
         selectedFieldId: PropTypes.number,
         selectedSegmentId: PropTypes.number,
@@ -197,6 +202,13 @@ export default class DataSelector extends Component {
         this.hydrateActiveStep();
     }
 
+    componentWillReceiveProps(nextProps) {
+        const newStateProps = this.getStepsAndSelectedEntities(nextProps)
+
+        // only update non-empty properties
+        this.setState(_.pick(newStateProps, (propValue) => !!propValue))
+    }
+
     hydrateActiveStep() {
         if (this.props.selectedFieldId) {
             this.switchToStep(FIELD_STEP);
@@ -306,7 +318,7 @@ export default class DataSelector extends Component {
     onChangeSegment = (item) => {
         if (item.segment != null) {
             this.props.setSourceSegmentFn && this.props.setSourceSegmentFn(item.segment.id);
-            this.nextStep({ selectedSegment: item.segment })
+            this.nextStep({ selectedTable: null, selectedSegment: item.segment })
         }
     }
 
@@ -322,7 +334,7 @@ export default class DataSelector extends Component {
 
         return (
             <span className={className || "px2 py2 text-bold cursor-pointer text-default"} style={style}>
-                { getTriggerElementContent({ selectedDatabase, selectedSegment, selectedTable, selectedField }) }
+                { React.createElement(getTriggerElementContent, { selectedDatabase, selectedSegment, selectedTable, selectedField }) }
                 <Icon className="ml1" name="chevrondown" size={triggerIconSize || 8}/>
             </span>
         );
@@ -560,6 +572,12 @@ export const DatabaseSchemaPicker = ({ skipDatabaseSelection, databases, selecte
     }
 
 export const TablePicker = ({ selectedDatabase, selectedSchema, selectedTable, disabledTableIds, onChangeTable, hasAdjacentStep, onBack }) => {
+    // In case DataSelector props get reseted
+    if (!selectedDatabase) {
+        if (onBack) onBack()
+        return null
+    }
+
     const isSavedQuestionList = selectedDatabase.is_saved_questions;
     let header = (
         <div className="flex flex-wrap align-center">
@@ -623,6 +641,11 @@ export const TablePicker = ({ selectedDatabase, selectedSchema, selectedTable, d
 export class FieldPicker extends Component {
     render() {
         const { isLoading, selectedTable, selectedField, onChangeField, metadata, onBack } = this.props
+        // In case DataSelector props get reseted
+        if (!selectedTable) {
+            if (onBack) onBack()
+            return null
+        }
 
         const header = (
             <span className="flex align-center">

frontend/src/metabase/query_builder/components/GuiQueryEditor.jsx

@@ -277,6 +277,7 @@ export default class GuiQueryEditor extends Component {
         const { databases, query, isShowingTutorial } = this.props;
         const tableMetadata = query.tableMetadata();
         const datasetQuery = query.datasetQuery();
+        const databaseId = datasetQuery && datasetQuery.database
         const sourceTableId = datasetQuery && datasetQuery.query && datasetQuery.query.source_table;
         const isInitiallyOpen = (!datasetQuery.database || !sourceTableId) && !isShowingTutorial;
 
@@ -285,8 +286,9 @@ export default class GuiQueryEditor extends Component {
                 <span className="GuiBuilder-section-label Query-label">{t`Data`}</span>
                 { this.props.features.data ?
                     <DatabaseSchemaAndTableDataSelector
-                        ref="dataSection"
                         databases={databases}
+                        selected={sourceTableId}
+                        selectedDatabaseId={databaseId}
                         selectedTableId={sourceTableId}
                         setDatabaseFn={this.props.setDatabaseFn}
                         setSourceTableFn={this.props.setSourceTableFn}

frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx

@@ -292,7 +292,6 @@ export default class NativeQueryEditor extends Component {
                     <div key="table_selector" className="GuiBuilder-section GuiBuilder-data flex align-center">
                         <span className="GuiBuilder-section-label Query-label">{t`Table`}</span>
                         <SchemaAndTableDataSelector
-                            ref="dataSection"
                             selectedTableId={selectedTable ? selectedTable.id : null}
                             selectedDatabaseId={database && database.id}
                             databases={[database]}

frontend/src/metabase/visualizations/components/ChartClickActions.jsx

@@ -142,13 +142,12 @@ export default class ChartClickActions extends Component {
                             <div key={key} className="border-row-divider p2 flex align-center text-default-hover">
                                 <Icon name={SECTIONS[key] && SECTIONS[key].icon || "unknown"} className="mr3" size={16} />
                                 { actions.map((action, index) =>
-                                    <div
-                                        key={index}
-                                        className={cx("text-brand-hover cursor-pointer", { "pr2": index === actions.length - 1, "pr4": index != actions.length - 1})}
-                                        onClick={() => this.handleClickAction(action)}
-                                    >
-                                        {action.title}
-                                    </div>
+                                    <ChartClickAction
+                                        index={index}
+                                        action={action}
+                                        isLastItem={index === actions.length - 1}
+                                        handleClickAction={this.handleClickAction}
+                                    />
                                 )}
                             </div>
                         )}
@@ -158,3 +157,11 @@ export default class ChartClickActions extends Component {
         );
     }
 }
+
+export const ChartClickAction = ({ action, isLastItem, handleClickAction }: { action: any, isLastItem: any, handleClickAction: any }) =>
+    <div
+        className={cx("text-brand-hover cursor-pointer", { "pr2": isLastItem, "pr4": !isLastItem})}
+        onClick={() => handleClickAction(action)}
+    >
+        { action.title }
+    </div>

frontend/test/query_builder/qb_drillthrough.integ.spec.js

@@ -29,17 +29,19 @@ import RunButton from "metabase/query_builder/components/RunButton";
 import BreakoutWidget from "metabase/query_builder/components/BreakoutWidget";
 import { getCard } from "metabase/query_builder/selectors";
 import { TestTable } from "metabase/visualizations/visualizations/Table";
-import ChartClickActions from "metabase/visualizations/components/ChartClickActions";
+import ChartClickActions, { ChartClickAction } from "metabase/visualizations/components/ChartClickActions";
 
 import { delay } from "metabase/lib/promise";
 import * as Urls from "metabase/lib/urls";
+import DataSelector, { TableTriggerContent } from "metabase/query_builder/components/DataSelector";
+import ObjectDetail from "metabase/visualizations/visualizations/ObjectDetail";
 
 const initQbWithDbAndTable = (dbId, tableId) => {
     return async () => {
         const store = await createTestStore()
         store.pushPath(Urls.plainQuestion());
         const qb = mount(store.connectContainer(<QueryBuilder />));
-        await store.waitForActions([INITIALIZE_QB]);
+        await store.waitForActions([INITIALIZE_QB])
 
         // Use Products table
         store.dispatch(setQueryDatabase(dbId));
@@ -58,6 +60,37 @@ describe("QueryBuilder", () => {
     })
 
     describe("drill-through", () => {
+        describe("View details action", () => {
+            it("works for foreign keys", async () => {
+                const {store, qb} = await initQbWithOrdersTable();
+                click(qb.find(RunButton));
+                await store.waitForActions([QUERY_COMPLETED]);
+                const table = qb.find(TestTable);
+
+                expect(qb.find(DataSelector).find(TableTriggerContent).text()).toBe("Orders")
+                const headerCells = table.find("thead th").map((cell) => cell.text())
+                const productIdIndex = headerCells.indexOf("Product ID")
+
+                const firstRowCells = table.find("tbody tr").first().find("td");
+                const productIdCell = firstRowCells.at(productIdIndex)
+                click(productIdCell.children().first());
+
+                // Drill-through is delayed in handleVisualizationClick of Visualization.jsx by 100ms
+                await delay(150);
+
+                const viewDetailsButton = qb.find(ChartClickActions)
+                    .find(ChartClickAction)
+                    .filterWhere(action => /View details/.test(action.text()))
+                    .first()
+
+                click(viewDetailsButton);
+
+                await store.waitForActions([NAVIGATE_TO_NEW_CARD, UPDATE_URL, QUERY_COMPLETED]);
+
+                expect(qb.find(ObjectDetail).length).toBe(1)
+                expect(qb.find(DataSelector).find(TableTriggerContent).text()).toBe("Products")
+            })
+        })
         describe("Zoom In action for broken out fields", () => {
             it("works for Count of rows aggregation and Subtotal 50 Bins breakout", async () => {
                 const {store, qb} = await initQbWithOrdersTable();