Remove duplicate users in api/user/recipients request (#32340)

src/metabase/api/user.clj

@@ -214,12 +214,13 @@
     (let [user_group_ids (map :id (:user_group_memberships
                                    (-> (fetch-user :id api/*current-user-id*)
                                        (t2/hydrate :user_group_memberships))))
-          data           (t2/select
-                          (vec (cons User (user-visible-columns)))
-                          (cond-> (user-clauses nil nil (remove #{1} user_group_ids) nil)
-                            true (sql.helpers/order-by [:%lower.last_name :asc] [:%lower.first_name :asc])
-                            (some? mw.offset-paging/*limit*)  (sql.helpers/limit mw.offset-paging/*limit*)
-                            (some? mw.offset-paging/*offset*) (sql.helpers/offset mw.offset-paging/*offset*)))]
+          data           (into #{}
+                          (t2/select
+                           (vec (cons User (user-visible-columns)))
+                           (cond-> (user-clauses nil nil (remove #{1} user_group_ids) nil)
+                             true (sql.helpers/order-by [:%lower.last_name :asc] [:%lower.first_name :asc])
+                             (some? mw.offset-paging/*limit*)  (sql.helpers/limit mw.offset-paging/*limit*)
+                             (some? mw.offset-paging/*offset*) (sql.helpers/offset mw.offset-paging/*offset*))))]
       {:data   data
        :total  (count data)
        :limit  mw.offset-paging/*limit*

test/metabase/api/user_test.clj

@@ -106,13 +106,13 @@
             (is (= [crowberto lucky rasta]
                    (->> ((mt/user-http-request :rasta :get 200 "user/recipients") :data)
                         (filter mt/test-user?)
-                        (map :email)))))
-         (testing "Returns all users when admin"
-           (mt/with-temporary-setting-values [user-visibility "none"]
-             (is (= [crowberto lucky rasta]
-                    (->> ((mt/user-http-request :crowberto :get 200 "user/recipients") :data)
-                         (filter mt/test-user?)
-                         (map :email)))))))
+                        (map :email))))))
+        (testing "Returns all users when admin"
+          (mt/with-temporary-setting-values [user-visibility "none"]
+            (is (= [crowberto lucky rasta]
+                   (->> ((mt/user-http-request :crowberto :get 200 "user/recipients") :data)
+                        (filter mt/test-user?)
+                        (map :email))))))
         (testing "Returns users in the group when user-visibility is same group"
           (mt/with-temporary-setting-values [user-visibility :group]
             (mt/with-temp* [PermissionsGroup           [{group-id :id} {:name "Test delete group"}]
@@ -120,13 +120,24 @@
                             PermissionsGroupMembership [_ {:user_id (mt/user->id :crowberto) :group_id group-id}]]
               (is (= [crowberto rasta]
                      (->> ((mt/user-http-request :rasta :get 200 "user/recipients") :data)
-                          (map :email))))))
-         (testing "Returns only self when user-visibility is none"
-           (mt/with-temporary-setting-values [user-visibility :none]
-             (is (= [rasta]
-                    (->> ((mt/user-http-request :rasta :get 200 "user/recipients") :data)
-                         (filter mt/test-user?)
-                         (map :email)))))))))))
+                          (map :email)))))))
+        (testing "Doesn't return multiple of the same user when they share the same group"
+          (mt/with-temporary-setting-values [user-visibility :group]
+            (mt/with-temp* [PermissionsGroup           [{group-id1 :id} {:name "Test delete group1"}]
+                            PermissionsGroup           [{group-id2 :id} {:name "Test delete group2"}]
+                            PermissionsGroupMembership [_ {:user_id (mt/user->id :rasta) :group_id group-id1}]
+                            PermissionsGroupMembership [_ {:user_id (mt/user->id :crowberto) :group_id group-id1}]
+                            PermissionsGroupMembership [_ {:user_id (mt/user->id :rasta) :group_id group-id2}]
+                            PermissionsGroupMembership [_ {:user_id (mt/user->id :crowberto) :group_id group-id2}]]
+              (is (= [crowberto rasta]
+                     (->> ((mt/user-http-request :rasta :get 200 "user/recipients") :data)
+                          (map :email)))))))
+        (testing "Returns only self when user-visibility is none"
+          (mt/with-temporary-setting-values [user-visibility :none]
+            (is (= [rasta]
+                   (->> ((mt/user-http-request :rasta :get 200 "user/recipients") :data)
+                        (filter mt/test-user?)
+                        (map :email))))))))))
 
 (deftest admin-user-list-test
   (testing "GET /api/user"