Hide permission controls for personal collection's sub-collections (#16668)

* Unskip the repro

* Add `isPersonalCollectionChild` utility

* Hide permission control for personal collections

* Hide permissions modal for personal collections

* Handle missing `collection.effective_ancestors`

* Use `sidebar` cypress selector

frontend/src/metabase/admin/permissions/containers/CollectionPermissionsModal.jsx

@@ -4,16 +4,20 @@ import { connect } from "react-redux";
 import { t } from "ttag";
 import _ from "underscore";
 
-import ModalContent from "metabase/components/ModalContent";
-import Button from "metabase/components/Button";
-import Link from "metabase/components/Link";
-import PermissionsGrid from "../components/PermissionsGrid";
-
 import * as Urls from "metabase/lib/urls";
 import { CollectionsApi } from "metabase/services";
+
 import Collections from "metabase/entities/collections";
 import SnippetCollections from "metabase/entities/snippet-collections";
 
+import { isPersonalCollectionChild } from "metabase/collections/utils";
+
+import ModalContent from "metabase/components/ModalContent";
+import Button from "metabase/components/Button";
+import Link from "metabase/components/Link";
+
+import PermissionsGrid from "../components/PermissionsGrid";
+
 import {
   getCollectionsPermissionsGrid,
   getIsDirty,
@@ -37,6 +41,7 @@ const mapStateToProps = (state, props) => {
     collection: getCollectionEntity(props).selectors.getObject(state, {
       entityId: collectionId,
     }),
+    collectionsList: getCollectionEntity(props).selectors.getList(state, props),
   };
 };
 
@@ -64,6 +69,21 @@ export default class CollectionPermissionsModal extends Component {
     );
     loadCollections();
   }
+
+  componentDidUpdate() {
+    const { collection, collectionsList, onClose } = this.props;
+
+    const loadedPersonalCollection =
+      collection &&
+      Array.isArray(collectionsList) &&
+      (collection.personal_owner_id ||
+        isPersonalCollectionChild(collection, collectionsList));
+
+    if (loadedPersonalCollection) {
+      onClose();
+    }
+  }
+
   render() {
     const {
       grid,

frontend/src/metabase/collections/components/Header.jsx

@@ -11,7 +11,13 @@ import PageHeading from "metabase/components/type/PageHeading";
 import Tooltip from "metabase/components/Tooltip";
 import CollectionEditMenu from "metabase/collections/components/CollectionEditMenu";
 
-export default function Header({ collection, isAdmin, isRoot, collectionId }) {
+export default function Header({
+  collection,
+  isAdmin,
+  isRoot,
+  isPersonalCollectionChild,
+  collectionId,
+}) {
   return (
     <Flex align="center" py={3}>
       <Flex align="center">
@@ -30,15 +36,17 @@ export default function Header({ collection, isAdmin, isRoot, collectionId }) {
       </Flex>
 
       <Flex ml="auto">
-        {isAdmin && !collection.personal_owner_id && (
-          <Tooltip tooltip={t`Edit the permissions for this collection`}>
-            <Link to={`${Urls.collection(collection)}/permissions`}>
-              <IconWrapper>
-                <Icon name="lock" />
-              </IconWrapper>
-            </Link>
-          </Tooltip>
-        )}
+        {isAdmin &&
+          !collection.personal_owner_id &&
+          !isPersonalCollectionChild && (
+            <Tooltip tooltip={t`Edit the permissions for this collection`}>
+              <Link to={`${Urls.collection(collection)}/permissions`}>
+                <IconWrapper>
+                  <Icon name="lock" />
+                </IconWrapper>
+              </Link>
+            </Tooltip>
+          )}
         {collection &&
           collection.can_write &&
           !collection.personal_owner_id && (

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

@@ -14,6 +14,7 @@ import CollectionEmptyState from "metabase/components/CollectionEmptyState";
 import Header from "metabase/collections/components/Header";
 import ItemsTable from "metabase/collections/components/ItemsTable";
 import PinnedItemsTable from "metabase/collections/components/PinnedItemsTable";
+import { isPersonalCollectionChild } from "metabase/collections/utils";
 
 import ItemsDragLayer from "metabase/containers/dnd/ItemsDragLayer";
 import PaginationControls from "metabase/components/PaginationControls";
@@ -33,7 +34,13 @@ function mapStateToProps(state) {
   };
 }
 
-function CollectionContent({ collection, collectionId, isAdmin, isRoot }) {
+function CollectionContent({
+  collection,
+  collections: collectionList = [],
+  collectionId,
+  isAdmin,
+  isRoot,
+}) {
   const [selectedItems, setSelectedItems] = useState(null);
   const [selectedAction, setSelectedAction] = useState(null);
   const [unpinnedItemsSorting, setUnpinnedItemsSorting] = useState({
@@ -140,6 +147,10 @@ function CollectionContent({ collection, collectionId, isAdmin, isRoot }) {
                 isAdmin={isAdmin}
                 collectionId={collectionId}
                 collection={collection}
+                isPersonalCollectionChild={isPersonalCollectionChild(
+                  collection,
+                  collectionList,
+                )}
               />
 
               <PinnedItemsTable
@@ -248,6 +259,10 @@ function CollectionContent({ collection, collectionId, isAdmin, isRoot }) {
 }
 
 export default _.compose(
+  Collection.loadList({
+    query: () => ({ tree: true }),
+    loadingAndErrorWrapper: false,
+  }),
   Collection.load({
     id: (_, props) => props.collectionId,
     reload: true,

frontend/src/metabase/collections/utils.js

 import { t } from "ttag";
+import { canonicalCollectionId } from "metabase/entities/collections";
 
 export function nonPersonalCollection(collection) {
   // @TODO - should this be an API thing?
@@ -41,3 +42,23 @@ export function getParentPath(collections, targetId) {
   }
   return null; // didn't find it under any collection
 }
+
+function getNonRootParentId(collection) {
+  if (Array.isArray(collection.effective_ancestors)) {
+    // eslint-disable-next-line no-unused-vars
+    const [root, nonRootParent] = collection.effective_ancestors;
+    return nonRootParent ? nonRootParent.id : undefined;
+  }
+  // location is a string like "/1/4" where numbers are parent collection IDs
+  const [nonRootParentId] = collection.location.split("/");
+  return canonicalCollectionId(nonRootParentId);
+}
+
+export function isPersonalCollectionChild(collection, collectionList) {
+  const nonRootParentId = getNonRootParentId(collection);
+  if (!nonRootParentId) {
+    return false;
+  }
+  const parentCollection = collectionList.find(c => c.id === nonRootParentId);
+  return parentCollection && !!parentCollection.personal_owner_id;
+}

frontend/test/metabase/scenarios/collections/personal-collections.cy.spec.js

-import { restore, popover, modal } from "__support__/e2e/cypress";
+import { restore, popover, modal, sidebar } from "__support__/e2e/cypress";
 import { USERS } from "__support__/e2e/cypress_data";
 
 describe("personal collections", () => {
@@ -41,17 +41,35 @@ describe("personal collections", () => {
       cy.icon("pencil").should("not.exist");
     });
 
-    it.skip("shouldn't be able to change permission levels for sub-collections in personal collections (metabase#8406)", () => {
+    it("shouldn't be able to change permission levels for sub-collections in personal collections (metabase#8406)", () => {
       cy.visit("/collection/root");
       cy.findByText("Your personal collection").click();
       // Create new collection inside admin's personal collection and navigate to it
       addNewCollection("Foo");
-      cy.get("[class*=CollectionSidebar]")
+      sidebar()
         .findByText("Foo")
         .click();
       cy.icon("new_folder");
       cy.icon("pencil");
       cy.icon("lock").should("not.exist");
+
+      // Check can't open permissions modal via URL for personal collection
+      cy.findByText("Your personal collection").click();
+      cy.location().then(location => {
+        cy.visit(`${location}/permissions`);
+        cy.get(".Modal").should("not.exist");
+        cy.url().should("eq", String(location));
+
+        // Check can't open permissions modal via URL for personal collection child
+        sidebar()
+          .findByText("Foo")
+          .click();
+        cy.location().then(location => {
+          cy.visit(`${location}/permissions`);
+          cy.get(".Modal").should("not.exist");
+          cy.url().should("eq", String(location));
+        });
+      });
     });
 
     it.skip("should be able view other users' personal sub-collections (metabase#15339)", () => {
@@ -60,7 +78,7 @@ describe("personal collections", () => {
       cy.findByLabelText("Name").type("Foo");
       cy.findByText("Create").click();
       // This repro could possibly change depending on the design decision for this feature implementation
-      cy.get("[class*=CollectionSidebar]").findByText("Foo");
+      sidebar().findByText("Foo");
     });
   });
 
@@ -73,7 +91,7 @@ describe("personal collections", () => {
           cy.findByText("Your personal collection").click();
           // Create initial collection inside the personal collection and navigate inside it
           addNewCollection("Foo");
-          cy.get("[class*=CollectionSidebar]")
+          sidebar()
             .as("sidebar")
             .findByText("Foo")
             .click();