From 3cb56f219638646472d214526c530da7b4c4dc6f Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Thu, 13 May 2021 00:00:25 +0300
Subject: [PATCH] Fix "Our analytics" is initially suggested only to admins
 when saving items (#15999)

* Fix 'Our collection' is suggested only to admins

* Fix permission test

* Fix test according to new collections behavior
---
 frontend/src/metabase/entities/collections.js | 58 +++++++++----------
 frontend/src/metabase/selectors/user.js       |  6 --
 .../collections/permissions.cy.spec.js        | 11 ++++
 .../scenarios/native/native.cy.spec.js        |  1 -
 4 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/frontend/src/metabase/entities/collections.js b/frontend/src/metabase/entities/collections.js
index 410628afa80..5f36d19aa28 100644
--- a/frontend/src/metabase/entities/collections.js
+++ b/frontend/src/metabase/entities/collections.js
@@ -8,17 +8,37 @@ import { createSelector } from "reselect";
 
 import { GET } from "metabase/lib/api";
 
-import {
-  getUser,
-  getUserDefaultCollectionId,
-  getUserPersonalCollectionId,
-} from "metabase/selectors/user";
+import { getUser, getUserPersonalCollectionId } from "metabase/selectors/user";
 
 import { t } from "ttag";
 
 const listCollectionsTree = GET("/api/collection/tree");
 const listCollections = GET("/api/collection");
 
+export const ROOT_COLLECTION = {
+  id: "root",
+  name: t`Our analytics`,
+  location: "",
+  path: [],
+};
+
+export const PERSONAL_COLLECTION = {
+  id: undefined, // to be filled in by getExpandedCollectionsById
+  name: t`My personal collection`,
+  location: "/",
+  path: [ROOT_COLLECTION.id],
+  can_write: true,
+};
+
+// fake collection for admins that contains all other user's collections
+export const PERSONAL_COLLECTIONS = {
+  id: "personal", // placeholder id
+  name: t`All personal collections`,
+  location: "/",
+  path: [ROOT_COLLECTION.id],
+  can_write: false,
+};
+
 const Collections = createEntity({
   name: "collections",
   path: "/api/collection",
@@ -86,7 +106,8 @@ const Collections = createEntity({
         (state, { params }) => (params ? params.collectionId : undefined),
         (state, { location }) =>
           location && location.query ? location.query.collectionId : undefined,
-        getUserDefaultCollectionId,
+        () => ROOT_COLLECTION.id,
+        getUserPersonalCollectionId,
       ],
       (collections, ...collectionIds) => {
         for (const collectionId of collectionIds) {
@@ -169,31 +190,6 @@ export const getCollectionType = (collectionId: string, state: {}) =>
     ? "other"
     : null;
 
-export const ROOT_COLLECTION = {
-  id: "root",
-  name: t`Our analytics`,
-  location: "",
-  path: [],
-};
-
-// the user's personal collection
-export const PERSONAL_COLLECTION = {
-  id: undefined, // to be filled in by getExpandedCollectionsById
-  name: t`My personal collection`,
-  location: "/",
-  path: ["root"],
-  can_write: true,
-};
-
-// fake collection for admins that contains all other user's collections
-export const PERSONAL_COLLECTIONS = {
-  id: "personal", // placeholder id
-  name: t`All personal collections`,
-  location: "/",
-  path: ["root"],
-  can_write: false,
-};
-
 type UserId = number;
 
 // a "real" collection
diff --git a/frontend/src/metabase/selectors/user.js b/frontend/src/metabase/selectors/user.js
index d9676fe8ecc..07f75ae45e3 100644
--- a/frontend/src/metabase/selectors/user.js
+++ b/frontend/src/metabase/selectors/user.js
@@ -16,9 +16,3 @@ export const getUserPersonalCollectionId = createSelector(
   [getUser],
   user => (user && user.personal_collection_id) || null,
 );
-
-export const getUserDefaultCollectionId = createSelector(
-  [getUser, getUserIsAdmin, getUserPersonalCollectionId],
-  (user, isAdmin, personalCollectionId) =>
-    isAdmin ? null : personalCollectionId,
-);
diff --git a/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js b/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js
index 314d52d7f7d..5a993ba61b0 100644
--- a/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js
+++ b/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js
@@ -654,6 +654,17 @@ describe("collection permissions", () => {
       });
     });
   });
+
+  it("should offer to save items to 'Our analytics' if user has a 'curate' access to it", () => {
+    cy.signIn("normal");
+
+    cy.visit("/question/new");
+    cy.findByText("Native query").click();
+    cy.get(".ace_content").type("select * from people");
+    cy.findByText("Save").click();
+
+    cy.get(".AdminSelect").findByText("Our analytics");
+  });
 });
 
 function clickRevert(event_name, index = 0) {
diff --git a/frontend/test/metabase/scenarios/native/native.cy.spec.js b/frontend/test/metabase/scenarios/native/native.cy.spec.js
index 46fa2ba2965..4d43eb4c4d0 100644
--- a/frontend/test/metabase/scenarios/native/native.cy.spec.js
+++ b/frontend/test/metabase/scenarios/native/native.cy.spec.js
@@ -229,7 +229,6 @@ describe("scenarios > question > native", () => {
     cy.findByText("Simple question").click();
     popover().within(() => {
       cy.findByText("Saved Questions").click();
-      cy.findByText("Robert Tableton's Personal Collection").click();
       cy.findByText(QUESTION).click();
     });
 
-- 
GitLab