From 4fd373b376c2ad61b203a35d729a5e37b1822ec1 Mon Sep 17 00:00:00 2001 From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Date: Thu, 3 Mar 2022 18:24:08 +0100 Subject: [PATCH] Fix and reproduce #20499: The term "SQL query" used in DBs that don't support SQL (#20506) * Use the term "Native query" * Fix/update e2e tests * Add repro for #20499 * make text conditional on engine and write permissions * Revert "Fix/update e2e tests" This reverts commit 3462cd113ec641f29cd3ff128fb7a32281adc315. Co-authored-by: Dalton Johnson <daltojohnso@users.noreply.github.com> --- .../src/metabase-lib/lib/metadata/Database.ts | 4 ++++ .../lib/metadata/Database.unit.spec.ts | 20 +++++++++++++++++++ .../src/metabase/nav/containers/Navbar.jsx | 8 ++++++-- frontend/src/metabase/new_query/selectors.js | 9 +++++++++ .../scenarios/native/native-mongo.cy.spec.js | 6 ++++-- 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/frontend/src/metabase-lib/lib/metadata/Database.ts b/frontend/src/metabase-lib/lib/metadata/Database.ts index 68633e3e81d..90248dc2bbe 100644 --- a/frontend/src/metabase-lib/lib/metadata/Database.ts +++ b/frontend/src/metabase-lib/lib/metadata/Database.ts @@ -96,6 +96,10 @@ export default class Database extends Base { return this.hasFeature("expressions") && this.hasFeature("left-join"); } + canWrite() { + return this.native_permissions === "write"; + } + // QUESTIONS newQuestion() { return this.question() diff --git a/frontend/src/metabase-lib/lib/metadata/Database.unit.spec.ts b/frontend/src/metabase-lib/lib/metadata/Database.unit.spec.ts index fa3b6b55ebc..9c1b257cdb4 100644 --- a/frontend/src/metabase-lib/lib/metadata/Database.unit.spec.ts +++ b/frontend/src/metabase-lib/lib/metadata/Database.unit.spec.ts @@ -260,4 +260,24 @@ describe("Database", () => { expect(database1.savedQuestionsDatabase()).toBe(database2); }); }); + + describe("canWrite", () => { + it("should be true for a db with write permissions", () => { + const database = new Database({ + id: 1, + native_permissions: "write", + }); + + expect(database.canWrite()).toBe(true); + }); + + it("should be false for a db without write permissions", () => { + const database = new Database({ + id: 1, + native_permissions: "none", + }); + + expect(database.canWrite()).toBe(false); + }); + }); }); diff --git a/frontend/src/metabase/nav/containers/Navbar.jsx b/frontend/src/metabase/nav/containers/Navbar.jsx index 6c810fb5137..f7f5f654599 100644 --- a/frontend/src/metabase/nav/containers/Navbar.jsx +++ b/frontend/src/metabase/nav/containers/Navbar.jsx @@ -28,6 +28,7 @@ import { getHasDataAccess, getHasNativeWrite, getPlainNativeQuery, + getHasDbWithJsonEngine, } from "metabase/new_query/selectors"; import Database from "metabase/entities/databases"; @@ -38,6 +39,7 @@ const mapStateToProps = (state, props) => ({ plainNativeQuery: getPlainNativeQuery(state), hasDataAccess: getHasDataAccess(state), hasNativeWrite: getHasNativeWrite(state), + hasDbWithJsonEngine: getHasDbWithJsonEngine(state, props), }); import { getDefaultSearchColor } from "metabase/nav/constants"; @@ -114,7 +116,7 @@ export default class Navbar extends Component { } renderMainNav() { - const { hasDataAccess, hasNativeWrite } = this.props; + const { hasDataAccess, hasNativeWrite, hasDbWithJsonEngine } = this.props; return ( <NavRoot @@ -178,7 +180,9 @@ export default class Navbar extends Component { ...(hasNativeWrite ? [ { - title: t`SQL query`, + title: hasDbWithJsonEngine + ? t`Native query` + : t`SQL query`, icon: `sql`, link: Urls.newQuestion({ type: "native", diff --git a/frontend/src/metabase/new_query/selectors.js b/frontend/src/metabase/new_query/selectors.js index d0590a53268..8f2f19d3c2a 100644 --- a/frontend/src/metabase/new_query/selectors.js +++ b/frontend/src/metabase/new_query/selectors.js @@ -4,9 +4,11 @@ */ import { createSelector } from "reselect"; + import { getMetadata, getDatabases } from "metabase/selectors/metadata"; import NativeQuery from "metabase-lib/lib/queries/NativeQuery"; import Question from "metabase-lib/lib/Question"; +import { getEngineNativeType } from "metabase/lib/engine"; export const getPlainNativeQuery = state => { const metadata = getMetadata(state); @@ -37,3 +39,10 @@ export const getHasNativeWrite = createSelector( (databaseMap = {}) => Object.values(databaseMap).some(d => d.native_permissions === "write"), ); + +export const getHasDbWithJsonEngine = (state, props) => { + return (props.databases || []).some(database => { + const isJsonEngine = getEngineNativeType(database.engine) === "json"; + return database.canWrite() && isJsonEngine; + }); +}; diff --git a/frontend/test/metabase/scenarios/native/native-mongo.cy.spec.js b/frontend/test/metabase/scenarios/native/native-mongo.cy.spec.js index 07b0c81a787..2783c2be0bb 100644 --- a/frontend/test/metabase/scenarios/native/native-mongo.cy.spec.js +++ b/frontend/test/metabase/scenarios/native/native-mongo.cy.spec.js @@ -2,14 +2,16 @@ import { restore, modal } from "__support__/e2e/cypress"; const MONGO_DB_NAME = "QA Mongo4"; -describe("scenatios > question > native > mongo", () => { +describe("scenarios > question > native > mongo", () => { before(() => { cy.intercept("POST", "/api/card").as("createQuestion"); restore("mongo-4"); cy.signInAsNormalUser(); - cy.visit("/question/new"); + cy.visit("/"); + cy.findByText("New").click(); + // Reproduces metabase#20499 issue cy.findByText("Native query").click(); cy.findByText(MONGO_DB_NAME).click(); -- GitLab