diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj index 2829d4e94abd85377c0e153e80b15ed9321a73c8..92df54817a0d763dcc6f8cb75920865703a01444 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj @@ -30,8 +30,7 @@ (is (= nil (entity-id))) (testing "Should return truthy on success" - (is (= true - (v2.entity-ids/seed-entity-ids!)))) + (is (= true (v2.entity-ids/seed-entity-ids!)))) (is (= "998b109c" (entity-id)))) (testing "Error: duplicate entity IDs" diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj index d3df49618b9621e4ae3869e3f20ffbc7cb201ef5..c5313e0279a271c8a8a8a5c9183193465db95766 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/models_test.clj @@ -33,5 +33,5 @@ (testing (str "Model shouldn't have entity_id defined: " (name model)) (is (not custom-entity-id?)) ;; TODO: strip serialization stuff off Pulse* - (when-not (#{"Pulse" "PulseChannel" "PulseCard"} (name model)) + (when-not (#{"Pulse" "PulseChannel" "PulseCard" "User" "PermissionsGroup"} (name model)) (is (not random-entity-id?)))))))))) diff --git a/resources/migrations/001_update_migrations.yaml b/resources/migrations/001_update_migrations.yaml index 74de023f826ba32b8866fcb146589b869db20913..6bfddb98b0342c1f08343b063820d2277ca3a249 100644 --- a/resources/migrations/001_update_migrations.yaml +++ b/resources/migrations/001_update_migrations.yaml @@ -6579,6 +6579,38 @@ databaseChangeLog: sql: > DELETE FROM data_permissions where perm_type = 'perms/data-access' OR perm_type = 'perms/native-query-editing'; + - changeSet: + id: v50.2024-04-25T03:15:01 + author: noahmoss + comment: Add entity_id to core_user + changes: + - addColumn: + columns: + - column: + remarks: NanoID tag for each user + name: entity_id + type: char(21) + constraints: + nullable: true + unique: true + tableName: core_user + + - changeSet: + id: v50.2024-04-25T03:15:02 + author: noahmoss + comment: Add entity_id to permissions_group + changes: + - addColumn: + columns: + - column: + remarks: NanoID tag for each user + name: entity_id + type: char(21) + constraints: + nullable: true + unique: true + tableName: permissions_group + - changeSet: id: v50.2024-04-25T16:29:31 author: calherries diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index 2ea69d3cadbc8a7bdaae3e4f7b40473681af7ff0..82da74f710d3eca60475c55e5c2d1d2c4815335f 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -466,6 +466,7 @@ add-entity-id)) (methodical/prefer-method! #'t2.before-insert/before-insert :hook/timestamped? :hook/entity-id) +(methodical/prefer-method! #'t2.before-insert/before-insert :hook/updated-at-timestamped? :hook/entity-id) ;; --- helper fns (defn changes-with-pk diff --git a/src/metabase/models/permissions_group.clj b/src/metabase/models/permissions_group.clj index 080d6493f2ec18579c53edd345dae17cd9fcc502..0fbee433b31664a61bbd9b9c17af2f737645cb0e 100644 --- a/src/metabase/models/permissions_group.clj +++ b/src/metabase/models/permissions_group.clj @@ -13,6 +13,7 @@ [metabase.db.query :as mdb.query] [metabase.models.data-permissions :as data-perms] [metabase.models.interface :as mi] + [metabase.models.serialization :as serdes] [metabase.models.setting :as setting] [metabase.plugins.classloader :as classloader] [metabase.public-settings.premium-features :as premium-features] @@ -28,14 +29,20 @@ (methodical/defmethod t2/table-name :model/PermissionsGroup [_model] :permissions_group) -(derive :model/PermissionsGroup :metabase/model) +(doto :model/PermissionsGroup + (derive :metabase/model) + (derive :hook/entity-id)) + +(defmethod serdes/hash-fields :model/PermissionsGroup + [_user] + [:name]) ;;; -------------------------------------------- Magic Groups Getter Fns --------------------------------------------- (defn- magic-group [group-name] (mdb/memoize-for-application-db (fn [] - (u/prog1 (t2/select-one PermissionsGroup :name group-name) + (u/prog1 (t2/select-one [PermissionsGroup :id :name] :name group-name) ;; normally it is impossible to delete the magic [[all-users]] or [[admin]] Groups -- see ;; [[check-not-magic-group]]. This assertion is here to catch us if we do something dumb when hacking on ;; the MB code -- to make tests fail fast. For that reason it's not i18n'ed. @@ -119,7 +126,9 @@ [group] (let [changes (t2/changes group)] (u/prog1 group - (check-not-magic-group group) + (when (contains? changes :name) + ;; Allow backfilling the entity ID for magic groups, but not changing anything else + (check-not-magic-group group)) (when-let [group-name (:name changes)] (check-name-not-already-taken group-name))))) diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index 25f76b51a93b99fa2c9ab9a2f9624667dfced93f..511722e69bec3416e64c9296f438bb612a3b707c 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -50,7 +50,8 @@ (doto :model/User (derive :metabase/model) - (derive :hook/updated-at-timestamped?)) + (derive :hook/updated-at-timestamped?) + (derive :hook/entity-id)) (t2/deftransforms :model/User {:login_attributes mi/transform-json-no-keywordization diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj index 41f9c157cfc69873c61242242ef8f5ec180261ee..b13ed77fb91a0fcfb4393e7e9671c8074e52500c 100644 --- a/test/metabase/db/custom_migrations_test.clj +++ b/test/metabase/db/custom_migrations_test.clj @@ -17,7 +17,6 @@ [metabase.db.custom-migrations :as custom-migrations] [metabase.db.schema-migrations-test.impl :as impl] [metabase.driver :as driver] - [metabase.models :refer [User]] [metabase.models.database :as database] [metabase.models.interface :as mi] [metabase.models.permissions-group :as perms-group] @@ -306,11 +305,11 @@ ;; SOMETIMES the rollback of custom migration doens't get triggered on mysql and this test got flaky. (impl/test-migrations "v47.00-030" [migrate!] (migrate!) - (let [user-id (first (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now})) + (let [user-id (first (t2/insert-returning-pks! (t2/table-name :model/User) {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now})) dashboard-id (first (t2/insert-returning-pks! :model/Dashboard {:name "A dashboard" :creator_id user-id})) tab1-id (first (t2/insert-returning-pks! :model/DashboardTab {:name "Tab 1" @@ -396,11 +395,12 @@ (deftest migrate-dashboard-revision-grid-from-18-to-24-test (impl/test-migrations ["v47.00-032" "v47.00-033"] [migrate!] - (let [user-id (first (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now})) + (let [user-id (first (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now})) cards [{:row 15 :col 0 :size_x 12 :size_y 8} {:row 7 :col 12 :size_x 6 :size_y 8} @@ -448,11 +448,12 @@ (deftest migrate-dashboard-revision-grid-from-18-to-24-handle-faliure-test (impl/test-migrations ["v47.00-032" "v47.00-033"] [migrate!] - (let [user-id (first (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now})) + (let [user-id (first (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now})) cards [{:id 1 :row 0 :col 0 :size_x 4 :size_y 4} ; correct case {:id 2 :row 0 :col 0 :sizeX 4 :sizeY 4} ; sizeX and sizeY are legacy names @@ -581,11 +582,12 @@ ["ref" ["field" 40 {"source-field" 39}]] {"column_title" "ID3"}, ["ref" ["field" 42 {"source-field" 41}]] {"column_title" "ID4"}} (update-keys json/generate-string))} - user-id (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + user-id (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) card {:visualization_settings visualization-settings} revision-id (t2/insert-returning-pks! (t2/table-name :model/Revision) {:model "Card" @@ -657,11 +659,12 @@ :source-table 5}] :source-table 2} :type :query}} - user-id (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + user-id (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) revision-id (t2/insert-returning-pks! (t2/table-name :model/Revision) {:model "Card" :model_id 1 ;; TODO: this could be a foreign key in the future @@ -707,11 +710,12 @@ ["ref" ["field" 40 {"source-field" 39}]] {"column_title" "ID3"}, ["ref" ["field" 42 {"source-field" 41}]] {"column_title" "ID4"}} (update-keys json/generate-string))} - user-id (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + user-id (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) database-id (t2/insert-returning-pks! (t2/table-name :model/Database) {:name "DB" :engine "h2" @@ -797,11 +801,12 @@ ["ref" ["field" "column_name" {"base-type" "type/Text"}]] {"column_title" "5"} ["name" "column_name"] {"column_title" "6"}} (update-keys json/generate-string))} - user-id (t2/insert-returning-pks! :model/User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + user-id (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) database-id (t2/insert-returning-pks! (t2/table-name :model/Database) {:name "DB" :engine "h2" @@ -865,11 +870,12 @@ ["ref" ["field" 40 {"source-field" 39}]] {"column_title" "ID3"}, ["ref" ["field" 42 {"source-field" 41}]] {"column_title" "ID4"}} (update-keys json/generate-string))} - user-id (t2/insert-returning-pks! :model/User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + user-id (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) dashboard {:cards [{:visualization_settings visualization-settings}]} revision-id (t2/insert-returning-pks! (t2/table-name :model/Revision) {:model "Dashboard" @@ -931,11 +937,12 @@ "join-alias" "Joined table"}]] {"column_title" "3"} ["name" "column_name"] {"column_title" "4"}} (update-keys json/generate-string))} - user-id (t2/insert-returning-pks! :model/User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + user-id (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) database-id (t2/insert-returning-pks! (t2/table-name :model/Database) {:name "DB" :engine "h2" @@ -1362,11 +1369,12 @@ "http://localhost:3001/?year={{CREATED_AT}}&cat={{CATEGORY}}&count={{count}}", "graph.dimensions" ["CREATED_AT" "CATEGORY"], "graph.metrics" ["count"]}) - [user-id] (t2/insert-returning-pks! User {:first_name "Howard" - :last_name "Hughes" - :email "howard@aircraft.com" - :password "superstrong" - :date_joined :%now}) + [user-id] (t2/insert-returning-pks! (t2/table-name :model/User) + {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) [database-id] (t2/insert-returning-pks! (t2/table-name :model/Database) {:name "DB" :engine "h2" diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj index 6354c41255034e9876ee50fd9dcf798b3eb1e506..1dd24277324c494c25ba41529c0761b0a451e130 100644 --- a/test/metabase/db/schema_migrations_test.clj +++ b/test/metabase/db/schema_migrations_test.clj @@ -29,8 +29,7 @@ Permissions PermissionsGroup Table - User - Dashboard]] + User]] [metabase.test :as mt] [metabase.test.fixtures :as fixtures] [toucan2.core :as t2])) @@ -542,7 +541,7 @@ (testing "Migration v48.00-049" (mt/test-drivers [:postgres :mysql] (impl/test-migrations "v48.00-049" [migrate!] - (create-raw-user! "noah@metabase.com") + (create-raw-user! (mt/random-email)) ;; Use raw :activity keyword as table name since the model has since been removed (let [_activity-1 (t2/insert-returning-pks! :activity {:topic "card-create" @@ -574,7 +573,7 @@ (mt/test-drivers [:h2] (impl/test-migrations "v48.00-049" [migrate!] - (create-raw-user! "noah@metabase.com") + (create-raw-user! (mt/random-email)) (let [_activity-1 (t2/insert-returning-pks! "activity" {:topic "card-create" :user_id 1 @@ -663,11 +662,12 @@ {:name "Normal Collection" :type nil :slug "normal_collection"})) - _internal-user-id (first (t2/insert-returning-pks! :model/User + _internal-user-id (first (t2/insert-returning-pks! (t2/table-name :model/User) {:id 13371338 :first_name "Metabase Internal User" :email "internal@metabase.com" - :password (str (random-uuid))})) + :password (str (random-uuid)) + :date_joined :%now})) original-db-names (t2/select-fn-set :name :metabase_database) original-collections (t2/select-fn-set :name :collection) check-before (fn [] @@ -1444,19 +1444,28 @@ (mdb.query/update-or-insert! :model/Setting {:key "query-caching-ttl-ratio"} (constantly {:value "100"})) (mdb.query/update-or-insert! :model/Setting {:key "query-caching-min-ttl"} (constantly {:value "123"})) - (let [db (t2/insert-returning-pk! :metabase_database (-> (mt/with-temp-defaults Database) + (let [user (create-raw-user! (mt/random-email)) + db (t2/insert-returning-pk! :metabase_database (-> (mt/with-temp-defaults Database) (update :details json/generate-string) (update :engine str) (assoc :cache_ttl 10))) - dash (t2/insert-returning-pk! :report_dashboard (-> (mt/with-temp-defaults Dashboard) - (assoc :cache_ttl 20 - :parameters ""))) - card (t2/insert-returning-pk! :report_card (-> (mt/with-temp-defaults Card) - (update :display str) - (update :visualization_settings json/generate-string) - (update :dataset_query json/generate-string) - (assoc :cache_ttl 30)))] - + dash (t2/insert-returning-pk! (t2/table-name :model/Dashboard) + {:name "A dashboard" + :creator_id (:id user) + :created_at :%now + :updated_at :%now + :cache_ttl 20 + :parameters ""}) + card (t2/insert-returning-pk! (t2/table-name Card) + {:name "Card" + :display "table" + :dataset_query "{}" + :visualization_settings "{}" + :cache_ttl 30 + :creator_id (:id user) + :database_id db + :created_at :%now + :updated_at :%now})] (migrate!) (is (=? [{:model "root"