diff --git a/frontend/src/metabase-lib/lib/Mode.js b/frontend/src/metabase-lib/lib/Mode.js index 3560bc40b899c095684ec3679b0edde0a97c036d..385a5ffba4befdf3fb0a280b6fef48c0b13680db 100644 --- a/frontend/src/metabase-lib/lib/Mode.js +++ b/frontend/src/metabase-lib/lib/Mode.js @@ -39,17 +39,17 @@ export default class Mode { return this._queryMode.name; } - actions(): ClickAction[] { + actions(settings): ClickAction[] { return _.flatten( this._queryMode.actions.map(actionCreator => - actionCreator({ question: this._question })) + actionCreator({ question: this._question, settings })) ); } - actionsForClick(clicked: ?ClickObject): ClickAction[] { + actionsForClick(clicked: ?ClickObject, settings): ClickAction[] { return _.flatten( this._queryMode.drills.map(actionCreator => - actionCreator({ question: this._question, clicked })) + actionCreator({ question: this._question, settings, clicked })) ); } } diff --git a/frontend/src/metabase-lib/lib/Question.js b/frontend/src/metabase-lib/lib/Question.js index 7c1bfb638f9b804f2ae5a2e475a006a447cd394a..e9169b0e3e4335c3ed9362de1ac9037188f5f0ae 100644 --- a/frontend/src/metabase-lib/lib/Question.js +++ b/frontend/src/metabase-lib/lib/Question.js @@ -290,15 +290,6 @@ export default class Question { return Mode.forQuestion(this); } - // see if the card can be xrayed - canXray() { - return this._card.canXray; - } - - xrayCost() { - return this._card.xrayCost; - } - /** * A user-defined name for the question */ diff --git a/frontend/src/metabase/meta/types/Card.js b/frontend/src/metabase/meta/types/Card.js index 9621602715c65d2c5181d7df7349ed4e2aef346f..97fef51e191b924f23575f58e5a1a8d002e770f5 100644 --- a/frontend/src/metabase/meta/types/Card.js +++ b/frontend/src/metabase/meta/types/Card.js @@ -32,8 +32,6 @@ export type Card = { // Not part of the card API contract, a field used by query builder for showing lineage original_card_id?: CardId, - // pulled in from settings to let the QB know whether to expose xray features - canXray: ?boolean }; export type StructuredDatasetQuery = { diff --git a/frontend/src/metabase/meta/types/Visualization.js b/frontend/src/metabase/meta/types/Visualization.js index 8effdb1ec7266ff58b5f2a844680ba73ebedb175..8b2268d2b26f859b8c2d52b91ce51f2b21cd5a6d 100644 --- a/frontend/src/metabase/meta/types/Visualization.js +++ b/frontend/src/metabase/meta/types/Visualization.js @@ -50,7 +50,8 @@ export type ClickAction = { export type ClickActionProps = { question: Question, - clicked?: ClickObject + clicked?: ClickObject, + settings: {} } export type OnChangeCardAndRun = ({ nextCard: Card, previousCard?: ?Card }) => void diff --git a/frontend/src/metabase/qb/components/actions/XRayCard.jsx b/frontend/src/metabase/qb/components/actions/XRayCard.jsx index 496004fa8cc4089d0ca805b78600b92904f716f7..70dc74c80683325b6ca91d53be8311fca32bb181 100644 --- a/frontend/src/metabase/qb/components/actions/XRayCard.jsx +++ b/frontend/src/metabase/qb/components/actions/XRayCard.jsx @@ -5,12 +5,12 @@ import type { ClickActionProps } from "metabase/meta/types/Visualization"; -export default ({ question }: ClickActionProps): ClickAction[] => { +export default ({ question, settings }: ClickActionProps): ClickAction[] => { // currently time series xrays require the maximum fidelity if ( question.card().id && - question.canXray() && - question.xrayCost() === "extended" + settings["enable_xrays"] && + settings["xray_max_cost"] === "extended" ) { return [ { diff --git a/frontend/src/metabase/qb/components/actions/XRaySegment.jsx b/frontend/src/metabase/qb/components/actions/XRaySegment.jsx index f3afc99bf982f0c4ef4f1ca728ff5fcf53a8f01b..8c57f6a827656f0124e17c4ce0281965addcba0c 100644 --- a/frontend/src/metabase/qb/components/actions/XRaySegment.jsx +++ b/frontend/src/metabase/qb/components/actions/XRaySegment.jsx @@ -7,8 +7,8 @@ import type { import { isSegmentFilter } from "metabase/lib/query/filter"; -export default ({ question }: ClickActionProps): ClickAction[] => { - if (question.card().id) { +export default ({ question, settings }: ClickActionProps): ClickAction[] => { + if (question.card().id && settings["enable_xrays"]) { return question .query() .filters() diff --git a/frontend/src/metabase/qb/components/modes/SegmentMode.jsx b/frontend/src/metabase/qb/components/modes/SegmentMode.jsx index db0ada763a45a36d4b3cf566f6f1e2b30df460e6..35cac1ba624cb2871839bd47e85d1e1c41e2d850 100644 --- a/frontend/src/metabase/qb/components/modes/SegmentMode.jsx +++ b/frontend/src/metabase/qb/components/modes/SegmentMode.jsx @@ -19,9 +19,9 @@ const SegmentMode: QueryMode = { name: "segment", actions: [ ...DEFAULT_ACTIONS, - XRaySegment, CommonMetricsAction, CountByTimeAction, + XRaySegment, SummarizeBySegmentMetricAction // commenting this out until we sort out viz settings in QB2 // PlotSegmentField diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js index 830fd50a30d53b370495086e7f898dda03e66a83..44c694797d1e281cefadba4afa10282df3f0f99a 100644 --- a/frontend/src/metabase/query_builder/actions.js +++ b/frontend/src/metabase/query_builder/actions.js @@ -22,8 +22,6 @@ import { addUndo } from "metabase/redux/undo"; import Question from "metabase-lib/lib/Question"; import { cardIsEquivalent } from "metabase/meta/Card"; -import { getXrayEnabled, getMaxCost } from "metabase/xray/selectors"; - import { getTableMetadata, getNativeDatabases, @@ -294,12 +292,6 @@ export const initializeQB = (location, params) => { // Fetch the question metadata card && dispatch(loadMetadataForCard(card)); - // see if xrays are enabled - if(card) { - card.canXray = getXrayEnabled(getState()) - card.xrayCost = getMaxCost(getState()) - } - const question = card && new Question(getMetadata(getState()), card); // if we have loaded up a card that we can run then lets kick that off as well diff --git a/frontend/src/metabase/query_builder/components/ActionsWidget.jsx b/frontend/src/metabase/query_builder/components/ActionsWidget.jsx index a5534492c742a348873b1e523dbd402025be2c11..a8d76411c612e88d2201a92ece3c2a4eee530c4d 100644 --- a/frontend/src/metabase/query_builder/components/ActionsWidget.jsx +++ b/frontend/src/metabase/query_builder/components/ActionsWidget.jsx @@ -22,7 +22,8 @@ type Props = { navigateToNewCardInsideQB: (any) => void, router: { push: (string) => void - } + }, + instanceSettings: {} }; type State = { @@ -98,10 +99,10 @@ export default class ActionsWidget extends Component { } handleActionClick = (index: number) => { - const { question, router } = this.props; + const { question, router, instanceSettings } = this.props; const mode = question.mode() if (mode) { - const action = mode.actions()[index]; + const action = mode.actions(instanceSettings)[index]; if (action && action.popover) { this.setState({ selectedActionIndex: index }); } else if (action && action.question) { @@ -119,15 +120,16 @@ export default class ActionsWidget extends Component { } }; render() { - const { className, question } = this.props; + const { className, question, instanceSettings } = this.props; const { popoverIsOpen, iconIsVisible, selectedActionIndex } = this.state; const mode = question.mode(); - const actions = mode ? mode.actions() : []; + const actions = mode ? mode.actions(instanceSettings) : []; if (actions.length === 0) { return null; } + const selectedAction: ?ClickAction = selectedActionIndex == null ? null : actions[selectedActionIndex]; let PopoverComponent = selectedAction && selectedAction.popover; diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx index 1dd5ec9d967b320ef901df06b3792c15d381204e..23a04ece6dbc4179302ae4322fb864bc7007d2e7 100644 --- a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx +++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx @@ -46,7 +46,8 @@ import { getMode, getQuery, getQuestion, - getOriginalQuestion + getOriginalQuestion, + getSettings } from "../selectors"; import { getMetadata, getDatabasesList } from "metabase/selectors/metadata"; @@ -116,6 +117,7 @@ const mapStateToProps = (state, props) => { loadTableAndForeignKeysFn: loadTableAndForeignKeys, autocompleteResultsFn: (prefix) => autocompleteResults(state.qb.card, prefix), + instanceSettings: getSettings(state) } } @@ -202,7 +204,7 @@ export default class QueryBuilder extends Component { class LegacyQueryBuilder extends Component { render() { - const { query, card, isDirty, databases, uiControls, mode } = this.props; + const { query, card, isDirty, databases, uiControls, mode, } = this.props; // if we don't have a card at all or no databases then we are initializing, so keep it simple if (!card || !databases) { diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js index be79650aa595ee7c52ec3899572e23a2e431935e..6dffe0319d318cc42c64259115363f4ea9c79f12 100644 --- a/frontend/src/metabase/query_builder/selectors.js +++ b/frontend/src/metabase/query_builder/selectors.js @@ -29,6 +29,10 @@ export const getParameterValues = state => state.qb.parameterValues; export const getQueryResult = state => state.qb.queryResult; export const getQueryResults = state => state.qb.queryResults; +// get instance settings, used for determining whether to display certain actions, +// currently used only for xrays +export const getSettings = state => state.settings.values + export const getIsDirty = createSelector( [getCard, getOriginalCard], (card, originalCard) => { diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx index 97750266e04ac634a4d953d86714c6df98759abc..2f7040e5670fb2ab050b7e9a60615c5cd6ade4a2 100644 --- a/frontend/src/metabase/visualizations/components/Visualization.jsx +++ b/frontend/src/metabase/visualizations/components/Visualization.jsx @@ -204,7 +204,7 @@ export default class Visualization extends Component { const card = series[seriesIndex].card; const question = new Question(metadata, card); const mode = question.mode(); - return mode ? mode.actionsForClick(clicked) : []; + return mode ? mode.actionsForClick({}, clicked) : []; } visualizationIsClickable = (clicked: ClickObject) => { @@ -213,7 +213,7 @@ export default class Visualization extends Component { return false; } try { - return this.getClickActions(clicked).length > 0; + return this.getClickActions({}, clicked).length > 0; } catch (e) { console.warn(e); return false; diff --git a/frontend/test/query_builder/actions.integ.spec.js b/frontend/test/query_builder/actions.integ.spec.js index a16a748ef088cc9c89733efc9f27f8af3924f665..2b30afb5d9c313eb42bbd75c8d81c72df3189b28 100644 --- a/frontend/test/query_builder/actions.integ.spec.js +++ b/frontend/test/query_builder/actions.integ.spec.js @@ -69,7 +69,7 @@ describe("QueryBuilder", () => { }); it("results in the correct card object in redux state", async () => { - expect(getCard(store.getState())).toMatchObject(_.omit(savedCleanQuestion.card(), "original_card_id", "canXray", "xrayCost")) + expect(getCard(store.getState())).toMatchObject(_.omit(savedCleanQuestion.card(), "original_card_id")) }) it("results in the correct original_card object in redux state", async () => { @@ -86,7 +86,7 @@ describe("QueryBuilder", () => { }); it("results in the correct card object in redux state", async () => { - expect(dirtyQuestion.card()).toMatchObject(_.omit(getCard(store.getState()), "canXray", "xrayCost")) + expect(dirtyQuestion.card()).toMatchObject(getCard(store.getState())) }) it("results in the correct original_card object in redux state", async () => { diff --git a/frontend/test/xray/xray.integ.spec.js b/frontend/test/xray/xray.integ.spec.js index 9d00f448a9249875141107fc712489d86d468e32..fb2a3d01c6199c157cf7899ddffcc797058a59ac 100644 --- a/frontend/test/xray/xray.integ.spec.js +++ b/frontend/test/xray/xray.integ.spec.js @@ -119,6 +119,7 @@ describe("xray integration tests", () => { }) it("let you see segment xray for a question containing a segment", async () => { + await SettingsApi.put({ key: 'enable-xrays', value: true }) const store = await createTestStore() store.pushPath(Urls.question(segmentQuestion.id())) const app = mount(store.getAppContainer()); @@ -163,14 +164,12 @@ describe("xray integration tests", () => { // the toggle should be on by default expect(xrayToggle.props().value).toEqual(true) - // toggle the... toggle click(xrayToggle) await store.waitForActions([UPDATE_SETTING]) expect(getXrayEnabled(store.getState())).toEqual(false) - // navigate to a previosuly x-ray-able entity store.pushPath(Urls.question(timeBreakoutQuestion.id())) await store.waitForActions(INITIALIZE_QB, QUERY_COMPLETED) @@ -186,7 +185,25 @@ describe("xray integration tests", () => { expect(xrayOptionIcon.length).toEqual(0) }) - it("should properly reflect the an admin set the max cost of xrays", async () => { + it("should not show xray options for segments when xrays are disabled", async () => { + // turn off xrays + await SettingsApi.put({ key: 'enable-xrays', value: false }) + + const store = await createTestStore() + + store.pushPath(Urls.question(segmentQuestion.id())) + const app = mount(store.getAppContainer()) + + await store.waitForActions(INITIALIZE_QB, QUERY_COMPLETED) + await delay(500); + + const actionsWidget = app.find(ActionsWidget) + click(actionsWidget.childAt(0)) + const xrayOptionIcon = actionsWidget.find('.Icon.Icon-beaker') + expect(xrayOptionIcon.length).toEqual(0) + }) + + it("should properly reflect the an admin set the max cost of xrays", async () => { await SettingsApi.put({ key: 'enable-xrays', value: true }) const store = await createTestStore()