Skip to content
Snippets Groups Projects
Unverified Commit c00349dc authored by Nemanja Glumac's avatar Nemanja Glumac Committed by GitHub
Browse files

Fix `editable?` function so that it respects native write permissions (#37788)


* Fix `editable?` source logic

The logic now takes into account whether or not a database
has native write permissions.

* Fix related metadata test

* DRY test code

* Set the test database metadata native permissions to true

* Mock the database native permissions in test

* Add issue reference to the test

Co-authored-by: default avatarmetamben <103100869+metamben@users.noreply.github.com>
parent 80017100
Branches
Tags
No related merge requests found
......@@ -215,6 +215,7 @@
There's no editable flag! Instead, a query is **not** editable if:
- Database is missing from the metadata (no permissions at all);
- Database is present but it doesn't have native write permissions;
- Database is present but tables (at least the `:source-table`) are missing (missing table permissions); or
- Similarly, the card specified by `:source-card` is missing from the metadata.
If metadata for the `:source-table` or `:source-card` can be found, then the query is editable."
......@@ -224,4 +225,8 @@
(= (:database query) id))
(or (and source-table (table query source-table))
(and source-card (card query source-card))
(= (:lib/type stage0) :mbql.stage/native))))))
(and
(= (:lib/type stage0) :mbql.stage/native)
;; Couldn't import and use `lib.native/has-write-permissions` here due to a circular dependency
;; TODO Find a way to unify has-write-permissions and this function?
(= :write (:native-permissions (database query)))))))))
......@@ -5,6 +5,7 @@
[clojure.walk :as walk]
[metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.query :as lib.query]
[metabase.lib.test-metadata :as meta]
[metabase.lib.test-util :as lib.tu]
......@@ -122,12 +123,18 @@
; source-card not visible
(assoc :source-card 999999999)
(dissoc :source-table))))))
(testing "on native queries"
(let [editable (lib/native-query meta/metadata-provider "SELECT * FROM Venues;")
bad-db (assoc editable :database 999999999)]
(is (= {:is-native true
:is-editable true}
(lib/display-info editable -1 editable)))
(is (= {:is-native true
:is-editable false}
(lib/display-info bad-db -1 bad-db)))))))
(testing "on native queries (#37765)"
(let [editable (lib/native-query meta/metadata-provider "SELECT * FROM Venues;")
;; Logic for the native-query mock borrowed from metabase.lib.native/has-write-permission-test
mock-db-native-perms #(lib/native-query (lib.tu/mock-metadata-provider
meta/metadata-provider
{:database (merge (lib.metadata/database meta/metadata-provider) {:native-permissions %})})
"select * from x;")]
(are [editable? query] (= {:is-native true
:is-editable editable?}
(mu/disable-enforcement
(lib/display-info query -1 query)))
true editable
false (assoc editable :database 999999999) ; database unknown - no permissions
false (mock-db-native-perms :none) ; native-permissions explicitly set to :none
false (mock-db-native-perms nil)))))) ; native-permissions not found on the database
......@@ -2413,6 +2413,7 @@
:options nil
:engine :h2
:initial-sync-status "complete"
:native-permissions :write
:dbms-version {:flavor "H2", :version "2.1.212 (2022-04-09)", :semantic-version [2 1]}
:refingerprint nil
:points-of-interest nil
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment