permissions bugs (#10918)

fix bugs around setting data permissions for new database

dev/src/dev.clj

@@ -9,7 +9,9 @@
              [util :as u]]
             [metabase.api.common :as api-common]
             [metabase.models.interface :as mi]
-            [toucan.db :as tdb]))
+            [metabase.test.util] ;; extensions
+            [toucan.db :as tdb]
+            [toucan.util.test :as tt]))
 
 (defn init!
   []
@@ -31,12 +33,16 @@
   (stop!)
   (start!))
 
-(defn run-tests
+(defn reload-run-tests
   [& ns-names]
   (doseq [ns-name ns-names]
     (require ns-name :reload))
   (expectations/run-tests ns-names))
 
+(defn run-tests
+  [& ns-names]
+  (expectations/run-tests ns-names))
+
 (defmacro require-model
   "Rather than requiring all models inn the ns declaration, make it easy to require the ones you need for your current session"
   [model-sym]

src/metabase/api/permissions.clj

@@ -42,8 +42,9 @@
 
 (defn- dejsonify-dbs [dbs]
   (into {} (for [[db-id {:keys [native schemas]}] dbs]
-             {(->int db-id) {:native  (keyword native)
-                             :schemas (dejsonify-schemas schemas)}})))
+             {(->int db-id) (cond-> {}
+                              native (assoc :native (keyword native))
+                              schemas (assoc :schemas (dejsonify-schemas schemas)))})))
 
 (defn- dejsonify-groups [groups]
   (into {} (for [[group-id dbs] groups]

src/metabase/models/permissions.clj

@@ -697,7 +697,8 @@
    returns the newly created `PermissionsRevision` entry."
   ([new-graph :- StrictPermissionsGraph]
    (let [old-graph (graph)
-         [old new] (data/diff (:groups old-graph) (:groups new-graph))]
+         [old new] (data/diff (:groups old-graph) (:groups new-graph))
+         old       (or old {})]
      (when (or (seq old) (seq new))
        (log-permissions-changes old new)
        (check-revision-numbers old-graph new-graph)

test/metabase/api/permissions_test.clj

@@ -2,8 +2,10 @@
   "Tests for `/api/permissions` endpoints."
   (:require [expectations :refer :all]
             [metabase.models
+             [database :refer [Database]]
              [permissions :as perms]
-             [permissions-group :as group :refer [PermissionsGroup]]]
+             [permissions-group :as group :refer [PermissionsGroup]]
+             [table :refer [Table]]]
             [metabase.test.data :as data]
             [metabase.test.data.users :as test-users]
             [metabase.util :as u]
@@ -70,3 +72,19 @@
               [:groups (u/get-id group) (data/id) :schemas]
               {"PUBLIC" {(data/id :venues) {:read :all, :query :segmented}}}))
    (get-in (perms/graph) [:groups (u/get-id group) (data/id) :schemas "PUBLIC"])))
+
+;; permissions for new dba
+(expect
+  :all
+  (let [new-id (inc (data/id))]
+    (tt/with-temp* [PermissionsGroup [group]
+                    Database         [{db-id :id}]
+                    Table            [_ {:db_id db-id}]]
+      (test-users/create-users-if-needed!)
+      ((test-users/user->client :crowberto) :put 200 "permissions/graph"
+       (assoc-in (perms/graph)
+                 [:groups (u/get-id group) db-id :schemas]
+                 :all))
+      (get-in (perms/graph) [:groups (u/get-id group) db-id :schemas]))))
+
+;; figure out failing case for when old doesn't exist