Permissions and related tables to toucan2 (#30726)

* Permissions to toucan2

* PermissionsGroup to toucan2

* PermissionsGroupMembership to toucan2

* PermissionsRevision to toucan2

* GroupTableAccessPolicy to toucan2

* ApplicationPermissionsRevision to toucan2

* CollectionPermissionGraphRevision to toucan2 too

enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj

@@ -19,17 +19,24 @@
    [metabase.util.i18n :refer [tru]]
    [metabase.util.log :as log]
    [metabase.util.schema :as su]
+   [methodical.core :as methodical]
    [schema.core :as s]
-   [toucan.models :as models]
    [toucan2.core :as t2]))
 
 (set! *warn-on-reflection* true)
 
-(models/defmodel GroupTableAccessPolicy :sandboxes)
+(def GroupTableAccessPolicy
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all the symbols in our codebase."
+  :model/GroupTableAccessPolicy)
 
-;;; only admins can work with GTAPs
-(derive GroupTableAccessPolicy ::mi/read-policy.superuser)
-(derive GroupTableAccessPolicy ::mi/write-policy.superuser)
+(methodical/defmethod t2/table-name :model/GroupTableAccessPolicy [_model] :sandboxes)
+
+(doto :model/GroupTableAccessPolicy
+  (derive :metabase/model)
+  ;;; only admins can work with GTAPs
+  (derive ::mi/read-policy.superuser)
+  (derive ::mi/write-policy.superuser))
 
 ;; This guard is to make sure this file doesn't get compiled twice when building the uberjar -- that will totally
 ;; screw things up because Toucan models use Potemkin `defrecord+` under the hood.
@@ -49,10 +56,10 @@
    mbql.normalize/normalize
    attribute-remappings))
 
-;; for GTAPs
-(models/add-type! ::attribute-remappings
-  :in  (comp mi/json-in normalize-attribute-remapping-targets)
-  :out (comp normalize-attribute-remapping-targets mi/json-out-without-keywordization))
+(t2/deftransforms :model/GroupTableAccessPolicy
+  {:attribute_remappings {:in  (comp mi/json-in normalize-attribute-remapping-targets)
+                          :out (comp normalize-attribute-remapping-targets mi/json-out-without-keywordization)}})
+
 
 (defn table-field-names->cols
   "Return a mapping of field names to corresponding cols for given table."
@@ -143,11 +150,13 @@
         (when-let [permission-path-id (t2/select-one-fn :id Permissions :object expected-permission-path)]
           (first (t2/insert-returning-instances! GroupTableAccessPolicy (assoc sandbox :permission_id permission-path-id))))))))
 
-(defn- pre-insert [gtap]
+(t2/define-before-insert :model/GroupTableAccessPolicy
+  [gtap]
   (u/prog1 gtap
     (check-columns-match-table gtap)))
 
-(defn- pre-update [{:keys [id], :as updates}]
+(t2/define-before-update :model/GroupTableAccessPolicy
+  [{:keys [id], :as updates}]
   (u/prog1 updates
     (let [original (t2/original updates)
           updated  (merge original updates)]
@@ -157,9 +166,3 @@
                          :status-code 400})))
       (when (:card_id updates)
         (check-columns-match-table updated)))))
-
-(mi/define-methods
- GroupTableAccessPolicy
- {:types      (constantly {:attribute_remappings ::attribute-remappings})
-  :pre-insert pre-insert
-  :pre-update pre-update})

enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj

@@ -38,13 +38,13 @@
     :model/ImplicitAction
     :model/QueryAction
     :model/Activity
-    :metabase.models.application-permissions-revision/ApplicationPermissionsRevision
+    :model/ApplicationPermissionsRevision
     :model/BookmarkOrdering
     :model/CardBookmark
     :model/CollectionBookmark
     :model/DashboardBookmark
     :metabase.models.collection.root/RootCollection
-    :metabase.models.collection-permission-graph-revision/CollectionPermissionGraphRevision
+    :model/CollectionPermissionGraphRevision
     :model/DashboardCardSeries
     :model/LoginHistory
     :model/FieldValues
@@ -53,10 +53,10 @@
     :model/ModelIndexValue
     :model/ModerationReview
     :model/ParameterCard
-    :metabase.models.permissions/Permissions
-    :metabase.models.permissions-group/PermissionsGroup
-    :metabase.models.permissions-group-membership/PermissionsGroupMembership
-    :metabase.models.permissions-revision/PermissionsRevision
+    :model/Permissions
+    :model/PermissionsGroup
+    :model/PermissionsGroupMembership
+    :model/PermissionsRevision
     :model/PersistedInfo
     :model/PulseCard
     :model/PulseChannel
@@ -72,7 +72,7 @@
     :model/TimelineEvent
     :model/User
     :model/ViewLog
-    :metabase-enterprise.sandbox.models.group-table-access-policy/GroupTableAccessPolicy})
+    :model/GroupTableAccessPolicy})
 
 (deftest ^:parallel comprehensive-entity-id-test
   (doseq [model (->> (v2.seed-entity-ids/toucan-models)

src/metabase/db/custom_migrations.clj

@@ -89,7 +89,7 @@
 (define-reversible-migration SplitDataPermissions
   (let [current-perms-set (t2/select-fn-set
                            (juxt :object :group_id)
-                           :metabase.models.permissions/Permissions
+                           :model/Permissions
                            {:where [:or
                                     [:like :object (h2x/literal "/db/%")]
                                     [:like :object (h2x/literal "/data/db/%")]

src/metabase/models/application_permissions_revision.clj

@@ -2,17 +2,27 @@
   (:require
    [metabase.models.interface :as mi]
    [metabase.util.i18n :refer [tru]]
-   [toucan.models :as models]
+   [methodical.core :as methodical]
    [toucan2.core :as t2]))
 
-(models/defmodel ApplicationPermissionsRevision :application_permissions_revision)
+(def ApplicationPermissionsRevision
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all the symbols in our codebase."
+  :model/ApplicationPermissionsRevision)
 
-(mi/define-methods
- ApplicationPermissionsRevision
- {:types      (constantly {:before :json
-                           :after  :json})
-  :properties (constantly {::mi/created-at-timestamped? true})
-  :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a ApplicationPermissionsRevision!"))))})
+(methodical/defmethod t2/table-name :model/ApplicationPermissionsRevision [_model] :application_permissions_revision)
+
+(doto :model/ApplicationPermissionsRevision
+  (derive :metabase/model)
+  (derive :hook/created-at-timestamped?))
+
+(t2/deftransforms :model/ApplicationPermissionsRevision
+  {:before mi/transform-json
+   :after  mi/transform-json})
+
+(t2/define-before-update :model/ApplicationPermissionsRevision
+  [_]
+  (throw (Exception. (tru "You cannot update a PermissionsRevision!"))))
 
 (defn latest-id
   "Return the ID of the newest `ApplicationPermissionsRevision`, or zero if none have been made yet.

src/metabase/models/collection_permission_graph_revision.clj

@@ -2,17 +2,27 @@
   (:require
    [metabase.models.interface :as mi]
    [metabase.util.i18n :refer [tru]]
-   [toucan.models :as models]
+   [methodical.core :as methodical]
    [toucan2.core :as t2]))
 
-(models/defmodel CollectionPermissionGraphRevision :collection_permission_graph_revision)
+(def CollectionPermissionGraphRevision
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all these symbols in our codebase."
+  :model/CollectionPermissionGraphRevision)
 
-(mi/define-methods
- CollectionPermissionGraphRevision
- {:types      (constantly {:before :json
-                           :after  :json})
-  :properties (constantly {::mi/created-at-timestamped? true})
-  :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a CollectionPermissionGraphRevision!"))))})
+(methodical/defmethod t2/table-name :model/CollectionPermissionGraphRevision [_model] :collection_permission_graph_revision)
+
+(doto :model/CollectionPermissionGraphRevision
+  (derive :metabase/model)
+  (derive :hook/created-at-timestamped?))
+
+(t2/deftransforms :model/CollectionPermissionGraphRevision
+  {:before mi/transform-json
+   :after  mi/transform-json})
+
+(t2/define-before-update :model/CollectionPermissionGraphRevision
+  [_]
+  (throw (Exception. (tru "You cannot update a CollectionPermissionGraphRevision!"))))
 
 (defn latest-id
   "Return the ID of the newest `CollectionPermissionGraphRevision`, or zero if none have been made yet.

src/metabase/models/permissions.clj

@@ -193,8 +193,8 @@
    [metabase.util.malli :as mu]
    [metabase.util.regex :as u.regex]
    [metabase.util.schema :as su]
+   [methodical.core :as methodical]
    [schema.core :as s]
-   [toucan.models :as models]
    [toucan2.core :as t2]))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
@@ -688,31 +688,34 @@
 ;;; |                                               ENTITY + LIFECYCLE                                               |
 ;;; +----------------------------------------------------------------------------------------------------------------+
 
-(models/defmodel Permissions :permissions)
+(def Permissions
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all the symbols in our codebase."
+  :model/Permissions)
 
-(defn- pre-insert [permissions]
+(methodical/defmethod t2/table-name :model/Permissions [_model] :permissions)
+
+(derive :model/Permissions :metabase/model)
+
+(t2/define-before-insert :model/Permissions
+  [permissions]
   (u/prog1 permissions
     (assert-valid permissions)
     (log/debug (u/colorize 'green (trs "Granting permissions for group {0}: {1}"
                                        (:group_id permissions)
                                        (:object permissions))))))
 
-(defn- pre-update [_]
+(t2/define-before-update :model/Permissions
+  [_]
   (throw (Exception. (tru "You cannot update a permissions entry! Delete it and create a new one."))))
 
-(defn- pre-delete [permissions]
+(t2/define-before-delete :model/Permissions
+  [permissions]
   (log/debug (u/colorize 'red (trs "Revoking permissions for group {0}: {1}"
                                    (:group_id permissions)
                                    (:object permissions))))
   (assert-not-admin-group permissions))
 
-(mi/define-methods
- Permissions
- {:pre-insert pre-insert
-  :pre-update pre-update
-  :pre-delete pre-delete})
-
-
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                                  GRAPH SCHEMA                                                  |
 ;;; +----------------------------------------------------------------------------------------------------------------+

src/metabase/models/permissions_group.clj

@@ -17,11 +17,17 @@
    [metabase.public-settings.premium-features :as premium-features]
    [metabase.util :as u]
    [metabase.util.i18n :refer [tru]]
-   [toucan.models :as models]
+   [methodical.core :as methodical]
    [toucan2.core :as t2]))
 
-(models/defmodel PermissionsGroup :permissions_group)
+(def PermissionsGroup
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all the symbols in our codebase."
+  :model/PermissionsGroup)
 
+(methodical/defmethod t2/table-name :model/PermissionsGroup [_model] :permissions_group)
+
+(derive :model/PermissionsGroup :metabase/model)
 
 ;;; -------------------------------------------- Magic Groups Getter Fns ---------------------------------------------
 
@@ -80,32 +86,30 @@
 
 ;;; --------------------------------------------------- Lifecycle ----------------------------------------------------
 
-(defn- pre-insert [{group-name :name, :as group}]
-  (u/prog1 group
-    (check-name-not-already-taken group-name)))
+(t2/define-before-insert :model/PermissionsGroup
+ [{group-name :name, :as group}]
+ (u/prog1 group
+   (check-name-not-already-taken group-name)))
 
-(defn- pre-delete [{id :id, :as group}]
+(t2/define-before-delete :model/PermissionsGroup
+  [{id :id, :as group}]
   (check-not-magic-group group)
   ;; Remove from LDAP mappings
   (classloader/require 'metabase.integrations.ldap)
   (setting/set-value-of-type!
-   :json :ldap-group-mappings
-   (when-let [mappings (setting/get-value-of-type :json :ldap-group-mappings)]
-     (zipmap (keys mappings)
-             (for [val (vals mappings)]
-               (remove (partial = id) val))))))
-
-(defn- pre-update [{group-name :name, :as group}]
-  (u/prog1 group
-    (check-not-magic-group group)
-    (when group-name
-      (check-name-not-already-taken group-name))))
-
-(mi/define-methods
- PermissionsGroup
- {:pre-delete  pre-delete
-  :pre-insert  pre-insert
-  :pre-update  pre-update})
+    :json :ldap-group-mappings
+    (when-let [mappings (setting/get-value-of-type :json :ldap-group-mappings)]
+      (zipmap (keys mappings)
+              (for [val (vals mappings)]
+                (remove (partial = id) val))))))
+
+(t2/define-before-update :model/PermissionsGroup
+  [group]
+  (let [changes (t2/changes group)]
+   (u/prog1 group
+     (check-not-magic-group group)
+     (when-let [group-name (:name changes)]
+       (check-name-not-already-taken group-name)))))
 
 ;;; ---------------------------------------------------- Util Fns ----------------------------------------------------
 

src/metabase/models/permissions_group_membership.clj

 (ns metabase.models.permissions-group-membership
   (:require
    [metabase.db.query :as mdb.query]
-   [metabase.models.interface :as mi]
    [metabase.models.permissions-group :as perms-group]
    [metabase.util :as u]
    [metabase.util.i18n :refer [deferred-tru tru]]
-   [toucan.models :as models]
+   [methodical.core :as methodical]
    [toucan2.core :as t2]))
 
-(models/defmodel PermissionsGroupMembership :permissions_group_membership)
+(def PermissionsGroupMembership
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all the symbols in our codebase."
+  :model/PermissionsGroupMembership)
+
+(methodical/defmethod t2/table-name :model/PermissionsGroupMembership [_model] :permissions_group_membership)
+
+(derive :model/PermissionsGroupMembership :metabase/model)
 
 (def fail-to-remove-last-admin-msg
   "Exception message when try to remove the last admin."
@@ -47,7 +53,8 @@
     (throw (ex-info (str fail-to-remove-last-admin-msg)
                     {:status-code 400}))))
 
-(defn- pre-delete [{:keys [group_id user_id]}]
+(t2/define-before-delete :model/PermissionsGroupMembership
+  [{:keys [group_id user_id]}]
   (check-not-all-users-group group_id)
   ;; Otherwise if this is the Admin group...
   (when (= group_id (:id (perms-group/admin)))
@@ -56,19 +63,15 @@
     ;; ...otherwise we're ok. Unset the `:is_superuser` flag for the user whose membership was revoked
     (t2/update! 'User user_id {:is_superuser false})))
 
-(defn- pre-insert [{:keys [group_id], :as membership}]
+(t2/define-before-insert :model/PermissionsGroupMembership
+  [{:keys [group_id], :as membership}]
   (u/prog1 membership
     (check-not-all-users-group group_id)))
 
-(defn- post-insert [{:keys [group_id user_id], :as membership}]
+(t2/define-after-insert :model/PermissionsGroupMembership
+  [{:keys [group_id user_id], :as membership}]
   (u/prog1 membership
     ;; If we're adding a user to the admin group, set the `:is_superuser` flag for the user to whom membership was
     ;; granted
     (when (= group_id (:id (perms-group/admin)))
       (t2/update! 'User user_id {:is_superuser true}))))
-
-(mi/define-methods
- PermissionsGroupMembership
- {:pre-delete  pre-delete
-  :pre-insert  pre-insert
-  :post-insert post-insert})

src/metabase/models/permissions_revision.clj

@@ -2,17 +2,27 @@
   (:require
    [metabase.models.interface :as mi]
    [metabase.util.i18n :refer [tru]]
-   [toucan.models :as models]
+   [methodical.core :as methodical]
    [toucan2.core :as t2]))
 
-(models/defmodel PermissionsRevision :permissions_revision)
+(def PermissionsRevision
+  "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], now it's a reference to the toucan2 model name.
+  We'll keep this till we replace all these symbols in our codebase."
+  :model/PermissionsRevision)
 
-(mi/define-methods
- PermissionsRevision
- {:types      (constantly {:before :json
-                           :after  :json})
-  :properties (constantly {::mi/created-at-timestamped? true})
-  :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a PermissionsRevision!"))))})
+(methodical/defmethod t2/table-name :model/PermissionsRevision [_model] :permissions_revision)
+
+(doto :model/PermissionsRevision
+  (derive :metabase/model)
+  (derive :hook/created-at-timestamped?))
+
+(t2/deftransforms :model/PermissionsRevision
+  {:before mi/transform-json
+   :after  mi/transform-json})
+
+(t2/define-before-update :model/PermissionsRevision
+  [_]
+  (throw (Exception. (tru "You cannot update a PermissionsRevision!"))))
 
 (defn latest-id
   "Return the ID of the newest `PermissionsRevision`, or zero if none have been made yet.

test/metabase/api/dataset_test.clj

@@ -319,7 +319,7 @@
              (mt/user-http-request :rasta :post 200 "dataset/native"
                                    (assoc
                                     (mt/mbql-query venues {:fields [$id $name]})
-                                     :pretty true)))))
+                                    :pretty true)))))
     (testing "The default behavior is to format the SQL"
       (is (= {:query  (str "SELECT\n"
                            "  \"PUBLIC\".\"VENUES\".\"ID\" AS \"ID\",\n"

test/metabase/test/util.clj

@@ -178,7 +178,7 @@
             :created_at    (t/zoned-date-time)
             :creator_id    (rasta-id)})
 
-   PermissionsGroup
+   :model/PermissionsGroup
    (fn [_] {:name (tu.random/random-name)})
 
    :model/Pulse