diff --git a/e2e/support/cypress_sample_instance_data.js b/e2e/support/cypress_sample_instance_data.js index 36be9adad511d5c0a34bd88d69c12dc6d8ce3b9d..0accec58fde06c08e4e5b7d9eaffa3825839d89b 100644 --- a/e2e/support/cypress_sample_instance_data.js +++ b/e2e/support/cypress_sample_instance_data.js @@ -29,3 +29,8 @@ export const ADMIN_PERSONAL_COLLECTION_ID = _.findWhere( SAMPLE_INSTANCE_DATA.collections, { name: "Bobby Tables's Personal Collection" }, ).id; + +export const NORMAL_PERSONAL_COLLECTION_ID = _.findWhere( + SAMPLE_INSTANCE_DATA.collections, + { name: "Robert Tableton's Personal Collection" }, +).id; diff --git a/e2e/test/scenarios/onboarding/urls.cy.spec.js b/e2e/test/scenarios/onboarding/urls.cy.spec.js index b34d9a7cf3d2120454159b39d06aa4b26c837408..0a17224464d00bf96b685a0c2165fd328faa6483 100644 --- a/e2e/test/scenarios/onboarding/urls.cy.spec.js +++ b/e2e/test/scenarios/onboarding/urls.cy.spec.js @@ -8,6 +8,7 @@ import { USERS, SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { ORDERS_QUESTION_ID, ADMIN_PERSONAL_COLLECTION_ID, + NORMAL_PERSONAL_COLLECTION_ID, } from "e2e/support/cypress_sample_instance_data"; import { SAVED_QUESTIONS_VIRTUAL_DB_ID } from "metabase-lib/metadata/utils/saved-questions"; @@ -106,7 +107,9 @@ describe("URLs", () => { cy.findByText(getFullName(normal)).click(); cy.location("pathname").should( "eq", - `/collection/9-${getUsersPersonalCollectionSlug(normal)}`, + `/collection/${NORMAL_PERSONAL_COLLECTION_ID}-${getUsersPersonalCollectionSlug( + normal, + )}`, ); }); @@ -127,7 +130,11 @@ describe("URLs", () => { `${getFullName(admin)}'s Personal Collection`, ); - cy.visit(`/collection/9-${getUsersPersonalCollectionSlug(normal)}`); + cy.visit( + `/collection/${NORMAL_PERSONAL_COLLECTION_ID}-${getUsersPersonalCollectionSlug( + normal, + )}`, + ); cy.findByTestId("collection-name-heading").should( "have.text", `${getFullName(normal)}'s Personal Collection`, diff --git a/e2e/test/scenarios/permissions/sandboxes.cy.spec.js b/e2e/test/scenarios/permissions/sandboxes.cy.spec.js index a964e46e016f7dad1f843548dbe21297dd3605a8..d5823e90c3238e16fa502951442db9e69c50e928 100644 --- a/e2e/test/scenarios/permissions/sandboxes.cy.spec.js +++ b/e2e/test/scenarios/permissions/sandboxes.cy.spec.js @@ -45,7 +45,7 @@ describeEE("formatting > sandboxes", () => { }); it("should add key attributes to an existing user", () => { - cy.icon("ellipsis").last().click(); + cy.icon("ellipsis").first().click(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Edit user").click(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj index 15dd2b30e36cba6ff5faadc347f7824c255ff433..e741d82f5296babafcb8f997c3318cec7fa6fa7e 100644 --- a/src/metabase/api/user.clj +++ b/src/metabase/api/user.clj @@ -177,7 +177,7 @@ (vec (cons User (user-visible-columns))) (cond-> (user-clauses status query (if (some? group_id) [group_id] nil) include_deactivated) (some? group_id) (sql.helpers/order-by [:core_user.is_superuser :desc] [:is_group_manager :desc]) - true (sql.helpers/order-by [:%lower.last_name :asc] [:%lower.first_name :asc]) + true (sql.helpers/order-by [:%lower.first_name :asc] [:%lower.last_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*))) ;; For admins also include the IDs of Users' Personal Collections @@ -203,7 +203,7 @@ {:data (t2/select (vec (cons User (user-visible-columns))) (cond-> (user-clauses nil nil nil nil) - true (sql.helpers/order-by [:%lower.last_name :asc] [:%lower.first_name :asc]) + true (sql.helpers/order-by [:%lower.first_name :asc] [:%lower.last_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*))) :total (t2/count User (user-clauses nil nil nil nil)) @@ -213,11 +213,11 @@ (let [user_group_ids (map :id (:user_group_memberships (-> (fetch-user :id api/*current-user-id*) (t2/hydrate :user_group_memberships)))) - data (into #{} + data (distinct (t2/select (vec (cons User (user-visible-columns))) (cond-> (user-clauses nil nil (remove #{(u/the-id (perms-group/all-users))} user_group_ids) nil) - true (sql.helpers/order-by [:%lower.last_name :asc] [:%lower.first_name :asc]) + true (sql.helpers/order-by [:%lower.first_name :asc] [:%lower.last_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 diff --git a/test/metabase/api/user_test.clj b/test/metabase/api/user_test.clj index e4727620e6f37f54453542835f425425192bf097..1b115c0e9ac814b8540938b946ae50b9c2840200 100644 --- a/test/metabase/api/user_test.clj +++ b/test/metabase/api/user_test.clj @@ -198,14 +198,7 @@ (is (= (t2/count User) ((mt/user-http-request :crowberto :get 200 "user" :status "all") :total)))) (testing "for admins, it should include those inactive users as we'd expect" - (is (= (->> [{:email "trashbird@metabase.com" - :first_name "Trash" - :last_name "Bird" - :is_active false - :group_ids #{(u/the-id (perms-group/all-users))} - :personal_collection_id true - :common_name "Trash Bird"} - {:email "crowberto@metabase.com" + (is (= (->> [{:email "crowberto@metabase.com" :first_name "Crowberto" :last_name "Corv" :is_superuser true @@ -224,7 +217,14 @@ :last_name "Toucan" :group_ids #{(u/the-id (perms-group/all-users))} :personal_collection_id true - :common_name "Rasta Toucan"}] + :common_name "Rasta Toucan"} + {:email "trashbird@metabase.com" + :first_name "Trash" + :last_name "Bird" + :is_active false + :group_ids #{(u/the-id (perms-group/all-users))} + :personal_collection_id true + :common_name "Trash Bird"}] (map (partial merge @user-defaults)) (map #(dissoc % :is_qbnewb :last_login))) (->> ((mt/user-http-request :crowberto :get 200 "user", :include_deactivated true) :data) @@ -232,14 +232,7 @@ group-ids->sets mt/boolean-ids-and-timestamps (map #(dissoc % :is_qbnewb :last_login))))) - (is (= (->> [{:email "trashbird@metabase.com" - :first_name "Trash" - :last_name "Bird" - :is_active false - :group_ids #{(u/the-id (perms-group/all-users))} - :personal_collection_id true - :common_name "Trash Bird"} - {:email "crowberto@metabase.com" + (is (= (->> [{:email "crowberto@metabase.com" :first_name "Crowberto" :last_name "Corv" :is_superuser true @@ -258,7 +251,14 @@ :last_name "Toucan" :group_ids #{(u/the-id (perms-group/all-users))} :personal_collection_id true - :common_name "Rasta Toucan"}] + :common_name "Rasta Toucan"} + {:email "trashbird@metabase.com" + :first_name "Trash" + :last_name "Bird" + :is_active false + :group_ids #{(u/the-id (perms-group/all-users))} + :personal_collection_id true + :common_name "Trash Bird"}] (map (partial merge @user-defaults)) (map #(dissoc % :is_qbnewb :last_login))) (->> ((mt/user-http-request :crowberto :get 200 "user", :status "all") :data)