Group personal collections in the saved question picker (#16875)

* hide other personal collections folder when no other users

* expand collections on click but disallow to select all personal collections, show all empty collections

frontend/src/metabase/collections/containers/CollectionSidebar.jsx

@@ -20,7 +20,7 @@ import CollectionLink from "metabase/collections/components/CollectionLink";
 
 import { SIDEBAR_SPACER } from "metabase/collections/constants";
 import {
-  nonPersonalCollection,
+  nonPersonalOrArchivedCollection,
   currentUserPersonalCollections,
   getParentPath,
 } from "metabase/collections/utils";
@@ -109,7 +109,7 @@ class CollectionSidebar extends React.Component {
             onClose={this.onClose}
             onOpen={this.onOpen}
             collections={list}
-            filter={nonPersonalCollection}
+            filter={nonPersonalOrArchivedCollection}
             currentCollection={collectionId}
           />
 

frontend/src/metabase/collections/utils.js

 import { t } from "ttag";
 import { canonicalCollectionId } from "metabase/entities/collections";
 
-export function nonPersonalCollection(collection) {
+export function nonPersonalOrArchivedCollection(collection) {
   // @TODO - should this be an API thing?
-  return !collection.personal_owner_id && !collection.archived;
+  return !isPersonalCollection(collection) && !collection.archived;
+}
+
+export function isPersonalCollection(collection) {
+  return typeof collection.personal_owner_id === "number";
 }
 
 // Replace the name for the current user's collection

frontend/src/metabase/components/tree/TreeNode.jsx

@@ -39,13 +39,11 @@ export const TreeNode = React.memo(function TreeNode({
 }) {
   const { name, icon, hasRightArrow, id } = item;
 
-  const handleExpand = e => {
-    e.stopPropagation();
+  const handleSelect = () => {
+    onSelect(item);
     onToggleExpand(id);
   };
 
-  const handleSelect = () => onSelect(item);
-
   const handleKeyDown = ({ key }) => {
     switch (key) {
       case "Enter":
@@ -70,7 +68,7 @@ export const TreeNode = React.memo(function TreeNode({
       isSelected={isSelected}
       onKeyDown={handleKeyDown}
     >
-      <ExpandToggleButton onClick={handleExpand} hidden={!hasChildren}>
+      <ExpandToggleButton hidden={!hasChildren}>
         <ExpandToggleIcon isExpanded={isExpanded} />
       </ExpandToggleButton>
 

frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionList.jsx

@@ -10,54 +10,71 @@ import EmptyState from "metabase/components/EmptyState";
 import { generateSchemaId } from "metabase/schema";
 
 import { SavedQuestionListRoot } from "./SavedQuestionList.styled";
+import { PERSONAL_COLLECTIONS } from "metabase/entities/collections";
 
 const propTypes = {
   databaseId: PropTypes.string,
   schema: PropTypes.object.isRequired,
   onSelect: PropTypes.func.isRequired,
   selectedId: PropTypes.string,
-  schemaName: PropTypes.string,
+  collection: PropTypes.shape({
+    id: PropTypes.oneOf([
+      PropTypes.string.isRequired,
+      PropTypes.number.isRequired,
+    ]),
+    schemaName: PropTypes.string.isRequired,
+  }).isRequired,
 };
 
 export default function SavedQuestionList({
   onSelect,
   databaseId,
   selectedId,
-  schemaName,
+  collection,
 }) {
+  const emptyState = (
+    <Box my="120px">
+      <EmptyState message={t`Nothing here`} icon="all" />
+    </Box>
+  );
+
+  const isVirtualCollection = collection.id === PERSONAL_COLLECTIONS.id;
+
   return (
     <SavedQuestionListRoot>
-      <Schemas.Loader
-        id={generateSchemaId(SAVED_QUESTIONS_VIRTUAL_DB_ID, schemaName)}
-      >
-        {({ schema }) => {
-          const tables =
-            databaseId != null
-              ? schema.tables.filter(table => table.db_id === databaseId)
-              : schema.tables;
-          return (
-            <React.Fragment>
-              {tables.map(t => (
-                <SelectList.Item
-                  id={t.id}
-                  isSelected={selectedId === t.id}
-                  key={t.id}
-                  size="small"
-                  name={t.display_name}
-                  icon="table2"
-                  onSelect={() => onSelect(t)}
-                />
-              ))}
+      {!isVirtualCollection && (
+        <Schemas.Loader
+          id={generateSchemaId(
+            SAVED_QUESTIONS_VIRTUAL_DB_ID,
+            collection.schemaName,
+          )}
+        >
+          {({ schema }) => {
+            const tables =
+              databaseId != null
+                ? schema.tables.filter(table => table.db_id === databaseId)
+                : schema.tables;
+            return (
+              <React.Fragment>
+                {tables.map(t => (
+                  <SelectList.Item
+                    id={t.id}
+                    isSelected={selectedId === t.id}
+                    key={t.id}
+                    size="small"
+                    name={t.display_name}
+                    icon="table2"
+                    onSelect={() => onSelect(t)}
+                  />
+                ))}
 
-              {tables.length === 0 ? (
-                <Box my="120px">
-                  <EmptyState message={t`Nothing here`} icon="all" />
-                </Box>
-              ) : null}
-            </React.Fragment>
-          );
-        }}
-      </Schemas.Loader>
+                {tables.length === 0 ? emptyState : null}
+              </React.Fragment>
+            );
+          }}
+        </Schemas.Loader>
+      )}
+      {isVirtualCollection && emptyState}
     </SavedQuestionListRoot>
   );
 }

frontend/src/metabase/query_builder/components/saved-question-picker/SavedQuestionPicker.jsx

@@ -5,12 +5,17 @@ import PropTypes from "prop-types";
 import { t } from "ttag";
 import { connect } from "react-redux";
 
-import Collection, { ROOT_COLLECTION } from "metabase/entities/collections";
 import Icon from "metabase/components/Icon";
 import { Tree } from "metabase/components/tree";
-
-import { SAVED_QUESTIONS_VIRTUAL_DB_ID } from "metabase/lib/constants";
-import Schemas from "metabase/entities/schemas";
+import Collection, {
+  ROOT_COLLECTION,
+  PERSONAL_COLLECTIONS,
+} from "metabase/entities/collections";
+import {
+  isPersonalCollection,
+  nonPersonalOrArchivedCollection,
+  currentUserPersonalCollections,
+} from "metabase/collections/utils";
 
 import SavedQuestionList from "./SavedQuestionList";
 import {
@@ -24,22 +29,26 @@ const propTypes = {
   onSelect: PropTypes.func.isRequired,
   onBack: PropTypes.func.isRequired,
   collections: PropTypes.array.isRequired,
-  schemas: PropTypes.array.isRequired,
+  currentUser: PropTypes.object.isRequired,
   databaseId: PropTypes.string,
   tableId: PropTypes.string,
 };
 
 const OUR_ANALYTICS_COLLECTION = {
+  ...ROOT_COLLECTION,
   schemaName: "Everything else",
   icon: "folder",
-  ...ROOT_COLLECTION,
+};
+
+const ALL_PERSONAL_COLLECTIONS_ROOT = {
+  ...PERSONAL_COLLECTIONS,
 };
 
 function SavedQuestionPicker({
   onBack,
   onSelect,
   collections,
-  schemas,
+  currentUser,
   databaseId,
   tableId,
 }) {
@@ -47,21 +56,46 @@ function SavedQuestionPicker({
     OUR_ANALYTICS_COLLECTION,
   );
 
-  const handleSelect = useCallback(id => {
-    setSelectedCollection(id);
+  const handleSelect = useCallback(collection => {
+    if (collection.id === PERSONAL_COLLECTIONS.id) {
+      return;
+    }
+    setSelectedCollection(collection);
   }, []);
 
   const collectionTree = useMemo(() => {
-    return schemas.length > 0
-      ? [
-          OUR_ANALYTICS_COLLECTION,
-          ...buildCollectionTree(
-            collections,
-            new Set(schemas.map(schema => schema.name)),
-          ),
-        ]
-      : [OUR_ANALYTICS_COLLECTION];
-  }, [collections, schemas]);
+    const preparedCollections = [];
+    const userPersonalCollections = currentUserPersonalCollections(
+      collections,
+      currentUser.id,
+    );
+    const nonPersonalOrArchivedCollections = collections.filter(
+      nonPersonalOrArchivedCollection,
+    );
+
+    preparedCollections.push(...nonPersonalOrArchivedCollections);
+    preparedCollections.push(...userPersonalCollections);
+
+    if (currentUser.is_superuser) {
+      const otherPersonalCollections = collections.filter(
+        collection =>
+          isPersonalCollection(collection) &&
+          collection.personal_owner_id !== currentUser.id,
+      );
+
+      if (otherPersonalCollections.length > 0) {
+        preparedCollections.push({
+          ...ALL_PERSONAL_COLLECTIONS_ROOT,
+          children: otherPersonalCollections,
+        });
+      }
+    }
+
+    return [
+      OUR_ANALYTICS_COLLECTION,
+      ...buildCollectionTree(preparedCollections),
+    ];
+  }, [collections, currentUser]);
 
   return (
     <SavedQuestionPickerRoot>
@@ -79,9 +113,9 @@ function SavedQuestionPicker({
         </Box>
       </CollectionsContainer>
       <SavedQuestionList
+        collection={selectedCollection}
         selectedId={tableId}
         databaseId={databaseId}
-        schemaName={selectedCollection.schemaName}
         onSelect={onSelect}
       />
     </SavedQuestionPickerRoot>
@@ -93,9 +127,6 @@ SavedQuestionPicker.propTypes = propTypes;
 const mapStateToProps = ({ currentUser }) => ({ currentUser });
 
 export default _.compose(
-  Schemas.loadList({
-    query: { dbId: SAVED_QUESTIONS_VIRTUAL_DB_ID },
-  }),
   Collection.loadList({
     query: () => ({ tree: true }),
   }),

frontend/src/metabase/query_builder/components/saved-question-picker/utils.js

-// FIXME: Collections must be filtered on the back end
-export function buildCollectionTree(collections, allowedSchemas) {
+import { isPersonalCollection } from "metabase/collections/utils";
+import { PERSONAL_COLLECTIONS } from "metabase/entities/collections";
+
+const getCollectionIcon = collection => {
+  if (collection.id === PERSONAL_COLLECTIONS.id) {
+    return "group";
+  }
+
+  return isPersonalCollection(collection) ? "person" : "folder";
+};
+
+export function buildCollectionTree(collections) {
   if (collections == null) {
     return [];
   }
 
-  return collections
-    .map(collection => {
-      const children = buildCollectionTree(collection.children, allowedSchemas);
-      const isPersonal = !!collection.personal_owner_id;
-      const shouldInclude =
-        allowedSchemas.has(collection.name) || children.length > 0;
-
-      return shouldInclude
-        ? {
-            id: collection.id,
-            name: collection.name,
-            schemaName: collection.name,
-            icon: isPersonal ? "person" : "folder",
-            children,
-          }
-        : null;
-    })
-    .filter(Boolean);
+  return collections.map(collection => ({
+    id: collection.id,
+    name: collection.name,
+    schemaName: collection.originalName || collection.name,
+    icon: getCollectionIcon(collection),
+    children: buildCollectionTree(collection.children),
+  }));
 }