fix + api test (#29194)

src/metabase/api/permission_graph.clj

@@ -41,6 +41,7 @@
   [:int {:decode/perm-graph kw-int->int-decoder}])
 
 (def ^:private Id DecodableKwInt)
+(def ^:private GroupId DecodableKwInt)
 
 ;; ids come in as keywordized numbers
 (s/def ::id (s/with-gen (s/or :kw->int (s/and keyword? #(re-find #"^\d+$" (name %))))
@@ -119,12 +120,12 @@
 
 (def DataPermissionsGraph
   "Used to transform, and verify data permissions graph"
-  [:map [:groups [:map-of Id DbGraph]]])
+  [:map [:groups [:map-of GroupId [:maybe DbGraph]]]])
 
 (def StrictData
   "Top level strict data graph schema"
   [:map
-   [:groups [:map-of Id StrictDbGraph]]
+   [:groups [:map-of GroupId [:maybe StrictDbGraph]]]
    [:revision int?]])
 
 ;;; --------------------------------------------- Collection Permissions ---------------------------------------------

src/metabase/api/permissions.clj

@@ -54,8 +54,7 @@
   (throw (ex-info (tru "Sandboxes are an Enterprise feature. Please upgrade to a paid plan to use this feature.")
                   {:status-code 402})))
 
-#_{:clj-kondo/ignore [:deprecated-var]}
-(api/defendpoint-schema PUT "/graph"
+(api/defendpoint PUT "/graph"
   "Do a batch update of Permissions by passing in a modified graph. This should return the same graph, in the same
   format, that you got from `GET /api/permissions/graph`, with any changes made in the wherever necessary. This
   modified graph must correspond to the `PermissionsGraph` schema. If successful, this endpoint returns the updated
@@ -70,7 +69,7 @@
   response will be returned if this key is present and the server is not running the Enterprise Edition, and/or the
   `:sandboxes` feature flag is not present."
   [:as {body :body}]
-  {body su/Map}
+  {body :map}
   (api/check-superuser)
   (let [graph (mc/decode api.permission-graph/DataPermissionsGraph
                          body
@@ -82,8 +81,9 @@
         (throw (ex-info (tru "Cannot parse permissions graph because it is invalid: {0}"
                              (pr-str explained))
                         {:status-code 400
-                         :error explained
-                         :humanized (me/humanize explained)}))))
+                         :error (str (me/humanize explained)
+                                     "\n"
+                                     (pr-str explained))}))))
     (db/transaction
      (perms/update-data-perms-graph! (dissoc graph :sandboxes))
      (if-let [sandboxes (:sandboxes body)]

src/metabase/models/permissions.clj

@@ -1322,7 +1322,7 @@
     (throw (ee-permissions-exception perm-type))))
 
 (mu/defn ^:private update-group-permissions!
-  [group-id :- pos-int? new-group-perms :- api.permission-graph/StrictDbGraph]
+  [group-id :- pos-int? new-group-perms :- [:maybe api.permission-graph/StrictDbGraph]]
   (doseq [[db-id new-db-perms] new-group-perms
           [perm-type new-perms] new-db-perms]
     (case perm-type

test/metabase/api/permissions_test.clj

@@ -213,7 +213,14 @@
         (is (= {:data {:schemas :all}}
                (get-in (perms/data-perms-graph) [:groups (u/the-id group) db-id])))
         (is (= {:query {:schemas :all}, :data {:native :write}}
-               (get-in (perms/data-perms-graph-v2) [:groups (u/the-id group) db-id])))))))
+               (get-in (perms/data-perms-graph-v2) [:groups (u/the-id group) db-id])))))
+
+    (testing "permissions when group has no permissions"
+      (mt/with-temp* [PermissionsGroup [group]]
+        (mt/user-http-request :crowberto :put 200 "permissions/graph"
+         (assoc-in (perms/data-perms-graph) [:groups (u/the-id group)] nil))
+        (is (= nil (get-in (perms/data-perms-graph) [:groups (u/the-id group)])))
+        (is (= nil (get-in (perms/data-perms-graph-v2) [:groups (u/the-id group)])))))))
 
 (deftest update-perms-graph-error-test
   (testing "PUT /api/permissions/graph"