From 422556ce62781cff75e47611e39e450977af1009 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Mon, 27 Jun 2022 15:40:42 +0300
Subject: [PATCH] Remove collection edit form (#23537)
---
.../FormCollectionAuthorityLevel.jsx | 41 +-----
.../metabase-enterprise/collections/index.js | 4 -
.../CollectionHeader/CollectionMenu.tsx | 6 -
.../CollectionEdit/CollectionEdit.tsx | 92 --------------
.../CollectionEdit/CollectionEditForm.tsx | 54 --------
.../containers/CollectionEdit/index.ts | 1 -
frontend/src/metabase/routes.jsx | 2 -
.../collections/collections.cy.spec.js | 14 +--
.../personal-collections.cy.spec.js | 34 ++---
.../moderation-collection.cy.spec.js | 117 +++---------------
.../timelines-collection.cy.spec.js | 9 +-
11 files changed, 37 insertions(+), 337 deletions(-)
delete mode 100644 frontend/src/metabase/collections/containers/CollectionEdit/CollectionEdit.tsx
delete mode 100644 frontend/src/metabase/collections/containers/CollectionEdit/CollectionEditForm.tsx
delete mode 100644 frontend/src/metabase/collections/containers/CollectionEdit/index.ts
diff --git a/enterprise/frontend/src/metabase-enterprise/collections/components/FormCollectionAuthorityLevel.jsx b/enterprise/frontend/src/metabase-enterprise/collections/components/FormCollectionAuthorityLevel.jsx
index e0692de7e7b..e5dcdb20446 100644
--- a/enterprise/frontend/src/metabase-enterprise/collections/components/FormCollectionAuthorityLevel.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/collections/components/FormCollectionAuthorityLevel.jsx
@@ -1,15 +1,10 @@
import React from "react";
import PropTypes from "prop-types";
-import { t } from "ttag";
-
-import CheckBox from "metabase/core/components/CheckBox";
import {
- SegmentedControl,
optionShape,
+ SegmentedControl,
} from "metabase/components/SegmentedControl";
-
-import { AUTHORITY_LEVELS } from "../constants";
-import { FormFieldRoot, Label } from "./FormCollectionAuthorityLevel.styled";
+import { FormFieldRoot } from "./FormCollectionAuthorityLevel.styled";
const propTypes = {
field: PropTypes.shape({
@@ -18,25 +13,9 @@ const propTypes = {
onChange: PropTypes.func.isRequired,
}).isRequired,
options: PropTypes.arrayOf(optionShape).isRequired,
- values: PropTypes.shape({
- id: PropTypes.number,
- authority_level: PropTypes.oneOf(["official"]),
- update_collection_tree_authority_level: PropTypes.bool,
- }),
- onChangeField: PropTypes.func.isRequired,
};
-export function FormCollectionAuthorityLevel({
- field,
- options,
- values,
- onChangeField,
-}) {
- const isNewCollection = !values.id;
- const selectedAuthorityLevel =
- AUTHORITY_LEVELS[field.value] || AUTHORITY_LEVELS.regular;
- const shouldSuggestToUpdateChildren =
- !isNewCollection && field.initialValue !== field.value;
+export function FormCollectionAuthorityLevel({ field, options }) {
return (
<FormFieldRoot>
<SegmentedControl
@@ -46,20 +25,6 @@ export function FormCollectionAuthorityLevel({
variant="fill-background"
inactiveColor="text-dark"
/>
- {shouldSuggestToUpdateChildren && (
- <CheckBox
- label={
- <Label>{t`Make all sub-collections ${selectedAuthorityLevel.name}, too.`}</Label>
- }
- checked={values.update_collection_tree_authority_level}
- onChange={e =>
- onChangeField(
- "update_collection_tree_authority_level",
- e.target.checked,
- )
- }
- />
- )}
</FormFieldRoot>
);
}
diff --git a/enterprise/frontend/src/metabase-enterprise/collections/index.js b/enterprise/frontend/src/metabase-enterprise/collections/index.js
index cac6de3f015..d22ddbcdb88 100644
--- a/enterprise/frontend/src/metabase-enterprise/collections/index.js
+++ b/enterprise/frontend/src/metabase-enterprise/collections/index.js
@@ -65,10 +65,6 @@ PLUGIN_COLLECTIONS.getAuthorityLevelFormFields = () => [
},
],
},
- {
- name: "update_collection_tree_authority_level",
- type: "hidden",
- },
];
PLUGIN_FORM_WIDGETS.collectionAuthorityLevel = FormCollectionAuthorityLevel;
diff --git a/frontend/src/metabase/collections/components/CollectionHeader/CollectionMenu.tsx b/frontend/src/metabase/collections/components/CollectionHeader/CollectionMenu.tsx
index 950f83dc198..7d49096d0c1 100644
--- a/frontend/src/metabase/collections/components/CollectionHeader/CollectionMenu.tsx
+++ b/frontend/src/metabase/collections/components/CollectionHeader/CollectionMenu.tsx
@@ -49,12 +49,6 @@ const CollectionMenu = ({
if (!isRoot && !isPersonal && canWrite) {
items.push(
- {
- title: t`Edit this collection`,
- icon: "edit_document",
- link: `${url}/edit`,
- event: `${ANALYTICS_CONTEXT};Edit Menu;Edit Collection Click`,
- },
{
title: t`Move`,
icon: "move",
diff --git a/frontend/src/metabase/collections/containers/CollectionEdit/CollectionEdit.tsx b/frontend/src/metabase/collections/containers/CollectionEdit/CollectionEdit.tsx
deleted file mode 100644
index f2cf96dc4ee..00000000000
--- a/frontend/src/metabase/collections/containers/CollectionEdit/CollectionEdit.tsx
+++ /dev/null
@@ -1,92 +0,0 @@
-import React, { useCallback, useEffect, useState } from "react";
-import { connect } from "react-redux";
-import { goBack, push, LocationAction } from "react-router-redux";
-import _ from "underscore";
-
-import * as Urls from "metabase/lib/urls";
-import Collections from "metabase/entities/collections";
-
-import { Collection as BaseCollection, CollectionId } from "metabase-types/api";
-import { State } from "metabase-types/store";
-
-import CollectionEditForm from "./CollectionEditForm";
-
-type Collection = BaseCollection & {
- parent_id: CollectionId;
-};
-
-interface CollectionEditOwnProps {
- params: {
- slug: string;
- };
-}
-
-interface CollectionEditLoaderProps {
- collection: Collection;
-}
-
-interface CollectionEditDispatchProps {
- push: LocationAction;
- goBack: () => void;
-}
-
-interface CollectionEditProps
- extends CollectionEditOwnProps,
- CollectionEditLoaderProps,
- CollectionEditDispatchProps {}
-
-const mapDispatchToProps = {
- push,
- goBack,
-};
-
-function CollectionEdit({ collection, push, goBack }: CollectionEditProps) {
- const [parentCollectionId, setParentCollectionId] = useState<CollectionId>(
- collection?.parent_id,
- );
- const [hasSetParentCollection, setHasSetParentCollection] = useState(false);
-
- useEffect(() => {
- if (collection && !hasSetParentCollection) {
- setParentCollectionId(collection.parent_id);
- }
- }, [collection, hasSetParentCollection]);
-
- const onChangeValues = useCallback(
- (collection: Collection) => {
- if (collection.parent_id !== parentCollectionId) {
- setParentCollectionId(collection.parent_id);
- setHasSetParentCollection(true);
- }
- },
- [parentCollectionId],
- );
-
- const onSave = useCallback(
- collection => {
- push(Urls.collection(collection));
- },
- [push],
- );
-
- return (
- <CollectionEditForm
- collection={collection}
- parentCollectionId={parentCollectionId}
- onChange={onChangeValues}
- onSave={onSave}
- onClose={goBack}
- />
- );
-}
-
-export default _.compose(
- connect<unknown, CollectionEditDispatchProps, CollectionEditOwnProps, State>(
- null,
- mapDispatchToProps,
- ),
- Collections.load({
- id: (state: State, props: CollectionEditOwnProps) =>
- Urls.extractCollectionId(props.params.slug),
- }),
-)(CollectionEdit);
diff --git a/frontend/src/metabase/collections/containers/CollectionEdit/CollectionEditForm.tsx b/frontend/src/metabase/collections/containers/CollectionEdit/CollectionEditForm.tsx
deleted file mode 100644
index 21a60833c70..00000000000
--- a/frontend/src/metabase/collections/containers/CollectionEdit/CollectionEditForm.tsx
+++ /dev/null
@@ -1,54 +0,0 @@
-import React from "react";
-import { connect } from "react-redux";
-
-import Collections from "metabase/entities/collections";
-
-import { Collection as BaseCollection, CollectionId } from "metabase-types/api";
-import { State } from "metabase-types/store";
-
-type Collection = BaseCollection & {
- parent_id: CollectionId;
-};
-
-interface CollectionEditFormOwnProps {
- collection: Collection;
- parentCollectionId: CollectionId;
- onChange: (collection: Collection) => void;
- onSave: (collection: Collection) => void;
- onClose: () => void;
-}
-
-interface CollectionEditFormStateProps {
- form: unknown;
-}
-
-interface CollectionEditProps
- extends CollectionEditFormOwnProps,
- CollectionEditFormStateProps {}
-
-function mapStateToProps(state: State, props: CollectionEditFormOwnProps) {
- return {
- form: Collections.selectors.getForm(state, props),
- };
-}
-
-function CollectionEditForm({
- form,
- collection,
- onChange,
- onSave,
- onClose,
-}: CollectionEditProps) {
- return (
- <Collections.ModalForm
- form={form}
- collection={collection}
- overwriteOnInitialValuesChange
- onChange={onChange}
- onSaved={onSave}
- onClose={onClose}
- />
- );
-}
-
-export default connect(mapStateToProps)(CollectionEditForm);
diff --git a/frontend/src/metabase/collections/containers/CollectionEdit/index.ts b/frontend/src/metabase/collections/containers/CollectionEdit/index.ts
deleted file mode 100644
index b7bebb0cfa4..00000000000
--- a/frontend/src/metabase/collections/containers/CollectionEdit/index.ts
+++ /dev/null
@@ -1 +0,0 @@
-export { default } from "./CollectionEdit";
diff --git a/frontend/src/metabase/routes.jsx b/frontend/src/metabase/routes.jsx
index a269409668d..598f28a73f8 100644
--- a/frontend/src/metabase/routes.jsx
+++ b/frontend/src/metabase/routes.jsx
@@ -33,7 +33,6 @@ import TableBrowser from "metabase/browse/containers/TableBrowser";
import QueryBuilder from "metabase/query_builder/containers/QueryBuilder";
-import CollectionEdit from "metabase/collections/containers/CollectionEdit";
import CollectionCreate from "metabase/collections/containers/CollectionCreate";
import MoveCollectionModal from "metabase/collections/containers/MoveCollectionModal";
import ArchiveCollectionModal from "metabase/components/ArchiveCollectionModal";
@@ -218,7 +217,6 @@ export const getRoutes = store => (
</Route>
<Route path="collection/:slug" component={CollectionLanding}>
- <ModalRoute path="edit" modal={CollectionEdit} />
<ModalRoute path="move" modal={MoveCollectionModal} />
<ModalRoute path="archive" modal={ArchiveCollectionModal} />
<ModalRoute path="new_collection" modal={CollectionCreate} />
diff --git a/frontend/test/metabase/scenarios/collections/collections.cy.spec.js b/frontend/test/metabase/scenarios/collections/collections.cy.spec.js
index 520735f48a0..a52b591aa0b 100644
--- a/frontend/test/metabase/scenarios/collections/collections.cy.spec.js
+++ b/frontend/test/metabase/scenarios/collections/collections.cy.spec.js
@@ -473,23 +473,15 @@ function ensureCollectionIsExpanded(collection, { children = [] } = {}) {
function moveOpenedCollectionTo(newParent) {
openCollectionMenu();
- cy.findByTextEnsureVisible("Edit this collection").click();
-
- // Open the select dropdown menu
- modal()
- .findByTestId("select-button")
- .click();
+ popover().within(() => cy.findByText("Move").click());
cy.findAllByTestId("item-picker-item")
.contains(newParent)
.click();
- // Make sure the correct value is selected
- cy.findAllByTestId("select-button-content").contains(newParent);
-
- cy.button("Update").click();
+ cy.button("Move").click();
// Make sure modal closed
- cy.button("Update").should("not.exist");
+ cy.button("Move").should("not.exist");
}
function dragAndDrop(subjectAlias, targetAlias) {
diff --git a/frontend/test/metabase/scenarios/collections/personal-collections.cy.spec.js b/frontend/test/metabase/scenarios/collections/personal-collections.cy.spec.js
index 0b2e02866ed..d6ed2768d9c 100644
--- a/frontend/test/metabase/scenarios/collections/personal-collections.cy.spec.js
+++ b/frontend/test/metabase/scenarios/collections/personal-collections.cy.spec.js
@@ -76,9 +76,9 @@ describe("personal collections", () => {
.click();
// It should be possible to edit sub-collections' details, but not its permissions
+ cy.findByDisplayValue("Foo").should("be.enabled");
openCollectionMenu();
popover().within(() => {
- cy.findByText("Edit this collection").should("be.visible");
cy.findByText("Edit permissions").should("not.exist");
});
@@ -130,31 +130,13 @@ describe("personal collections", () => {
cy.get("@sidebar")
.findByText("Bar")
.click();
- openCollectionMenu();
- /**
- * We're testing a few things here:
- * 1. editing collection's title
- * 2. editing collection's description and
- * 3. moving that collection within personal collection
- * 4. archiving the collection within personal collection (metabase#15343)
- */
- popover().within(() => cy.findByText("Edit this collection").click());
- modal().within(() => {
- cy.findByLabelText("Name") /* [1] */
- .click()
- .type("1");
-
- cy.findByLabelText("Description") /* [2] */
- .click()
- .type("ex-bar", { delay: 0 });
- cy.findByTestId("select-button").click();
- });
- popover()
- .findByText("My personal collection") /* [3] */
- .click();
- cy.button("Update").click();
- // Clicking on "Foo" would've closed it and would hide its sub-collections (if there were any).
- // By doing this, we're making sure "Bar" lives at the same level as "Foo"
+ cy.findByPlaceholderText("Add title")
+ .type("1")
+ .blur();
+ cy.findByPlaceholderText("Add description")
+ .type("ex-bar")
+ .blur();
+
cy.get("@sidebar")
.findByText("Foo")
.click();
diff --git a/frontend/test/metabase/scenarios/organization/moderation-collection.cy.spec.js b/frontend/test/metabase/scenarios/organization/moderation-collection.cy.spec.js
index bf09ea43f3e..de787509fd2 100644
--- a/frontend/test/metabase/scenarios/organization/moderation-collection.cy.spec.js
+++ b/frontend/test/metabase/scenarios/organization/moderation-collection.cy.spec.js
@@ -8,6 +8,8 @@ import {
navigationSidebar,
closeNavigationSidebar,
getCollectionActions,
+ popover,
+ openCollectionMenu,
} from "__support__/e2e/helpers";
import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database";
@@ -27,10 +29,6 @@ describeEE("collections types", () => {
restore();
});
- const TREE_UPDATE_REGULAR_MESSAGE = "Make all sub-collections Regular, too.";
- const TREE_UPDATE_OFFICIAL_MESSAGE =
- "Make all sub-collections Official, too.";
-
it("should be able to manage collection authority level", () => {
cy.signInAsAdmin();
cy.visit("/collection/root");
@@ -58,56 +56,6 @@ describeEE("collections types", () => {
testOfficialQuestionBadgeInRegularDashboard();
});
- it("should be able to update authority level for collection children", () => {
- cy.signInAsAdmin();
- cy.visit("/collection/root");
- cy.findByText("First collection").click();
-
- // Test not visible when creating a new collection
- openNewCollectionItemFlowFor("collection");
- modal().within(() => {
- cy.findByText(TREE_UPDATE_REGULAR_MESSAGE).should("not.exist");
- cy.findByText(TREE_UPDATE_OFFICIAL_MESSAGE).should("not.exist");
- setOfficial();
- cy.findByText(TREE_UPDATE_REGULAR_MESSAGE).should("not.exist");
- cy.findByText(TREE_UPDATE_OFFICIAL_MESSAGE).should("not.exist");
- cy.icon("close").click();
- });
-
- // Test can make all children official
- editCollection();
- modal().within(() => {
- cy.findByText(TREE_UPDATE_REGULAR_MESSAGE).should("not.exist");
- cy.findByText(TREE_UPDATE_OFFICIAL_MESSAGE).should("not.exist");
- setOfficial();
- cy.findByText(TREE_UPDATE_REGULAR_MESSAGE).should("not.exist");
- cy.findByText(TREE_UPDATE_OFFICIAL_MESSAGE).click();
- cy.button("Update").click();
- });
-
- getSidebarCollectionChildrenFor("First collection").within(() => {
- expandCollectionChildren("Second collection");
- cy.icon("badge").should("have.length", 2);
- cy.icon("folder").should("not.exist");
- });
-
- // Test can make all children regular
- editCollection();
- modal().within(() => {
- cy.findByText(TREE_UPDATE_REGULAR_MESSAGE).should("not.exist");
- cy.findByText(TREE_UPDATE_OFFICIAL_MESSAGE).should("not.exist");
- setOfficial(false);
- cy.findByText(TREE_UPDATE_REGULAR_MESSAGE).click();
- cy.findByText(TREE_UPDATE_OFFICIAL_MESSAGE).should("not.exist");
- cy.button("Update").click();
- });
-
- getSidebarCollectionChildrenFor("First collection").within(() => {
- cy.icon("folder").should("have.length", 2);
- cy.icon("badge").should("not.exist");
- });
- });
-
it("should not see collection type field if not admin", () => {
cy.signIn("normal");
cy.visit("/collection/root");
@@ -120,9 +68,9 @@ describeEE("collections types", () => {
cy.icon("close").click();
});
- editCollection();
- modal().within(() => {
- assertNoCollectionTypeInput();
+ openCollectionMenu();
+ popover().within(() => {
+ assertNoCollectionTypeOption();
});
});
@@ -170,10 +118,8 @@ describe("collection types", { tags: "@OSS" }, () => {
});
openCollection("First collection");
- editCollection();
- modal().within(() => {
- assertNoCollectionTypeInput();
- });
+ openCollectionMenu();
+ assertNoCollectionTypeOption();
});
it("should not display official collection icon", () => {
@@ -279,42 +225,11 @@ function openCollection(collectionName) {
.click();
}
-function editCollection() {
- cy.findByTestId("collection-menu").within(() => cy.icon("ellipsis").click());
- cy.findByText("Edit this collection").click();
-}
-
-function expandCollectionChildren(collectionName) {
- cy.findByText(collectionName)
- .parentsUntil("[data-testid=sidebar-collection-link-root]")
- .find(".Icon-chevronright")
- .click();
-}
-
-function getSidebarCollectionChildrenFor(item) {
- return navigationSidebar()
- .findByText(item)
- .parentsUntil("[data-testid=sidebar-collection-link-root]")
- .parent()
- .next("ul");
-}
-
-function setOfficial(official = true) {
- const isOfficialNow = !official;
- cy.findByLabelText("Regular").should(
- isOfficialNow ? "not.be.checked" : "be.checked",
- );
- cy.findByLabelText("Official").should(
- isOfficialNow ? "be.checked" : "not.be.checked",
- );
- cy.findByText(official ? "Official" : "Regular").click();
-}
-
function createAndOpenOfficialCollection({ name }) {
openNewCollectionItemFlowFor("collection");
modal().within(() => {
cy.findByLabelText("Name").type(name);
- setOfficial();
+ cy.findByText("Official").click();
cy.button("Create").click();
});
navigationSidebar().within(() => {
@@ -323,10 +238,13 @@ function createAndOpenOfficialCollection({ name }) {
}
function changeCollectionTypeTo(type) {
- editCollection();
- modal().within(() => {
- setOfficial(type === "official");
- cy.button("Update").click();
+ openCollectionMenu();
+ popover().within(() => {
+ if (type === "official") {
+ cy.findByText("Make collection official").click();
+ } else {
+ cy.findByText("Make collection unofficial").click();
+ }
});
}
@@ -336,6 +254,11 @@ function assertNoCollectionTypeInput() {
cy.findByText("Official").should("not.exist");
}
+function assertNoCollectionTypeOption() {
+ cy.findByText("Make collection official").should("not.exist");
+ cy.findByText("Make collection unofficial").should("not.exist");
+}
+
function assertSidebarIcon(collectionName, expectedIcon) {
navigationSidebar()
.findByText(collectionName)
diff --git a/frontend/test/metabase/scenarios/organization/timelines-collection.cy.spec.js b/frontend/test/metabase/scenarios/organization/timelines-collection.cy.spec.js
index 89dbda156dd..99345279692 100644
--- a/frontend/test/metabase/scenarios/organization/timelines-collection.cy.spec.js
+++ b/frontend/test/metabase/scenarios/organization/timelines-collection.cy.spec.js
@@ -3,7 +3,6 @@ import {
enableTracking,
expectGoodSnowplowEvents,
expectNoBadSnowplowEvents,
- openCollectionMenu,
resetSnowplow,
restore,
} from "__support__/e2e/helpers";
@@ -493,12 +492,10 @@ describe("scenarios > organization > timelines > collection", () => {
cy.wait("@createTimeline");
cy.icon("close").click();
- openCollectionMenu();
- cy.findByText("Edit this collection").click();
- cy.findByLabelText("Name")
+ cy.findByDisplayValue("First collection")
.clear()
- .type("1st collection");
- cy.button("Update").click();
+ .type("1st collection")
+ .blur();
cy.wait("@updateCollection");
cy.icon("calendar").click();
--
GitLab