diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj
index d9816989b98fcdad74583f324e0e6728c300d4ee..a6036adea084421c23417467f805a20b8cb0a9f8 100644
--- a/src/metabase/api/user.clj
+++ b/src/metabase/api/user.clj
@@ -9,6 +9,7 @@
[metabase.email.messages :as email]
[metabase.integrations.ldap :as ldap]
[metabase.models
+ [collection :as collection :refer [Collection]]
[permissions-group :as group]
[user :as user :refer [User]]]
[metabase.util :as u]
@@ -45,6 +46,22 @@
(api/check-superuser)
(user/set-permissions-groups! user-or-id new-groups-or-ids))))
+(defn- updated-user-name [user-before-update first_name last_name]
+ (let [prev_first_name (:first_name user-before-update)
+ prev_last_name (:last_name user-before-update)
+ first_name (or first_name prev_first_name)
+ last_name (or last_name prev_last_name)]
+ (when (or (not= first_name prev_first_name)
+ (not= last_name prev_last_name))
+ [first_name last_name])))
+
+(defn- maybe-update-user-personal-collection-name! [user-before-update first_name last_name]
+ ;; If the user name is updated, we shall also update the personal collection name (if such collection exists).
+ (when-some [[first_name last_name] (updated-user-name user-before-update first_name last_name)]
+ (when-some [collection (collection/user->existing-personal-collection (u/get-id user-before-update))]
+ (let [new-collection-name (collection/format-personal-collection-name first_name last_name)]
+ (when-not (= new-collection-name (:name collection))
+ (db/update! Collection (:id collection) :name new-collection-name))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Fetching Users -- GET /api/user, GET /api/user/current, GET /api/user/:id |
@@ -140,17 +157,18 @@
(api/check-403 (valid-email-update? user-before-update email))
;; can't change email if it's already taken BY ANOTHER ACCOUNT
(api/checkp (not (db/exists? User, :email email, :id [:not= id]))
- "email" (tru "Email address already associated to another user.")))
- (db/transaction
- (api/check-500
- (db/update! User id
- (u/select-keys-when body
- :present (when api/*is-superuser?*
- #{:login_attributes})
- :non-nil (set (concat [:first_name :last_name :email]
- (when api/*is-superuser?*
- [:is_superuser]))))))
- (maybe-set-user-permissions-groups! id group_ids is_superuser))
+ "email" (tru "Email address already associated to another user."))
+ (db/transaction
+ (api/check-500
+ (db/update! User id
+ (u/select-keys-when body
+ :present (when api/*is-superuser?*
+ #{:login_attributes})
+ :non-nil (set (concat [:first_name :last_name :email]
+ (when api/*is-superuser?*
+ [:is_superuser]))))))
+ (maybe-set-user-permissions-groups! id group_ids is_superuser)
+ (maybe-update-user-personal-collection-name! user-before-update first_name last_name)))
(-> (fetch-user :id id)
(hydrate :group_ids)))
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index 699d99bfc67cc5759af7016773be98bae1abf670..d16d8c2590ec72326d77b2b29d5b87bf57c22f7d 100644
--- a/src/metabase/models/collection.clj
+++ b/src/metabase/models/collection.clj
@@ -1012,6 +1012,11 @@
;;; | Personal Collections |
;;; +----------------------------------------------------------------------------------------------------------------+
+(defn format-personal-collection-name
+ "Constructs the personal collection name from user name."
+ [first-name last-name]
+ (tru "{0} {1}''s Personal Collection" first-name last-name))
+
(s/defn ^:private user->personal-collection-name :- su/NonBlankString
"Come up with a nice name for the Personal Collection for `user-or-id`."
[user-or-id]
@@ -1019,12 +1024,16 @@
;; the same first & last name! This will *ruin* their lives :(
(let [{first-name :first_name, last-name :last_name} (db/select-one ['User :first_name :last_name]
:id (u/get-id user-or-id))]
- (tru "{0} {1}''s Personal Collection" first-name last-name)))
+ (format-personal-collection-name first-name last-name)))
+
+(s/defn user->existing-personal-collection :- (s/maybe CollectionInstance)
+ [user-or-id]
+ (db/select-one Collection :personal_owner_id (u/get-id user-or-id)))
(s/defn user->personal-collection :- CollectionInstance
"Return the Personal Collection for `user-or-id`, if it already exists; if not, create it and return it."
[user-or-id]
- (or (db/select-one Collection :personal_owner_id (u/get-id user-or-id))
+ (or (user->existing-personal-collection user-or-id)
(try
(db/insert! Collection
:name (user->personal-collection-name user-or-id)
diff --git a/test/metabase/api/user_test.clj b/test/metabase/api/user_test.clj
index aec7998e4bdef19aef922e26160c9ad257a9671b..cad110b4e1bfe226db3d0de3a7f91aa70ed0b62c 100644
--- a/test/metabase/api/user_test.clj
+++ b/test/metabase/api/user_test.clj
@@ -8,6 +8,7 @@
[util :as u]]
[metabase.middleware.util :as middleware.u]
[metabase.models
+ [collection :as collection :refer [Collection]]
[permissions-group :as group :refer [PermissionsGroup]]
[permissions-group-membership :refer [PermissionsGroupMembership]]
[user :refer [User]]
@@ -17,7 +18,9 @@
[fixtures :as fixtures]
[util :as tu :refer [random-name]]]
[metabase.test.data.users :as test-users]
- [toucan.db :as db]
+ [toucan
+ [db :as db]
+ [hydrate :refer [hydrate]]]
[toucan.util.test :as tt]))
(use-fixtures :once (fixtures/initialize :test-users-personal-collections))
@@ -326,9 +329,18 @@
;;; | Updating a User -- PUT /api/user/:id |
;;; +----------------------------------------------------------------------------------------------------------------+
+(defn include-personal-collection-name
+ {:hydrate :personal_collection_name}
+ [user]
+ (db/select-one-field :name Collection :id (:personal_collection_id user)))
+
;; Test that admins can edit other Users
(expect
- {:before {:first_name "Cam", :last_name "Era", :is_superuser true, :email "cam.era@metabase.com"}
+ {:before {:first_name "Cam",
+ :last_name "Era",
+ :is_superuser true,
+ :email "cam.era@metabase.com"
+ :personal_collection_name "Cam Era's Personal Collection"}
:response (merge
user-defaults
{:common_name "Cam Eron"
@@ -338,13 +350,19 @@
:is_superuser true
:group_ids #{(u/get-id (group/all-users))
(u/get-id (group/admin))}})
- :after {:first_name "Cam", :last_name "Eron", :is_superuser true, :email "cam.eron@metabase.com"}}
- (tt/with-temp User [{user-id :id} {:first_name "Cam"
- :last_name "Era"
- :email "cam.era@metabase.com"
- :is_superuser true}]
- (let [user (fn [] (into {} (-> (db/select-one [User :first_name :last_name :is_superuser :email], :id user-id)
- (dissoc :common_name))))]
+ :after {:first_name "Cam"
+ :last_name "Eron"
+ :is_superuser true
+ :email "cam.eron@metabase.com"
+ :personal_collection_name "Cam Eron's Personal Collection"}}
+ (tt/with-temp* [User [{user-id :id} {:first_name "Cam"
+ :last_name "Era"
+ :email "cam.era@metabase.com"
+ :is_superuser true}]
+ Collection [coll]]
+ (let [user (fn [] (into {} (-> (db/select-one [User :id :first_name :last_name :is_superuser :email], :id user-id)
+ (hydrate :personal_collection_id :personal_collection_name)
+ (dissoc :id :personal_collection_id :common_name))))]
(array-map
:before (user)
:response (-> ((test-users/user->client :crowberto) :put 200 (str "user/" user-id)