From ef597af3e14f92371acf8e31b5f16c3a6775466e Mon Sep 17 00:00:00 2001
From: Alexander Lesnenko <alxnddr@users.noreply.github.com>
Date: Tue, 11 Jan 2022 22:55:35 +0000
Subject: [PATCH] Fix creating a new collection from navbar (#19626)
* Fix creating a new collection from navbar
* add specs
---
.../containers/CollectionCreate.jsx | 17 +++++--
frontend/src/metabase/collections/utils.js | 2 +-
.../src/metabase/nav/containers/Navbar.jsx | 38 +++++++++++++---
.../onboarding/navbar/new-menu.cy.spec.js | 45 +++++++++++++++++++
4 files changed, 91 insertions(+), 11 deletions(-)
create mode 100644 frontend/test/metabase/scenarios/onboarding/navbar/new-menu.cy.spec.js
diff --git a/frontend/src/metabase/collections/containers/CollectionCreate.jsx b/frontend/src/metabase/collections/containers/CollectionCreate.jsx
index 9337332ec17..7622ea23a9e 100644
--- a/frontend/src/metabase/collections/containers/CollectionCreate.jsx
+++ b/frontend/src/metabase/collections/containers/CollectionCreate.jsx
@@ -28,18 +28,29 @@ const mapDispatchToProps = {
@connect(mapStateToProps, mapDispatchToProps)
export default class CollectionCreate extends Component {
+ handleClose = () => {
+ const { goBack, onClose } = this.props;
+ return onClose ? onClose() : goBack();
+ };
+
+ handleSaved = collection => {
+ const { goBack, onSaved } = this.props;
+ return onSaved ? onSaved(collection) : goBack();
+ };
+
render() {
- const { form, initialCollectionId, goBack } = this.props;
+ const { form, initialCollectionId } = this.props;
return (
<Collection.ModalForm
+ overwriteOnInitialValuesChange
formName={FORM_NAME}
form={form}
collection={{
parent_id: initialCollectionId,
authority_level: REGULAR_COLLECTION.type,
}}
- onSaved={goBack}
- onClose={goBack}
+ onSaved={this.handleSaved}
+ onClose={this.handleClose}
/>
);
}
diff --git a/frontend/src/metabase/collections/utils.js b/frontend/src/metabase/collections/utils.js
index f0199fc50f2..1064da6cffb 100644
--- a/frontend/src/metabase/collections/utils.js
+++ b/frontend/src/metabase/collections/utils.js
@@ -54,7 +54,7 @@ function getNonRootParentId(collection) {
return nonRootParent ? nonRootParent.id : undefined;
}
// location is a string like "/1/4" where numbers are parent collection IDs
- const [nonRootParentId] = collection.location.split("/");
+ const nonRootParentId = collection.location?.split("/")?.[0];
return canonicalCollectionId(nonRootParentId);
}
diff --git a/frontend/src/metabase/nav/containers/Navbar.jsx b/frontend/src/metabase/nav/containers/Navbar.jsx
index b7f89564c05..a7360df8ad8 100644
--- a/frontend/src/metabase/nav/containers/Navbar.jsx
+++ b/frontend/src/metabase/nav/containers/Navbar.jsx
@@ -20,6 +20,7 @@ import Modal from "metabase/components/Modal";
import ProfileLink from "metabase/nav/components/ProfileLink";
import SearchBar from "metabase/nav/components/SearchBar";
+import CollectionCreate from "metabase/collections/containers/CollectionCreate";
import CreateDashboardModal from "metabase/components/CreateDashboardModal";
import { AdminNavbar } from "../components/AdminNavbar";
@@ -47,6 +48,7 @@ const mapDispatchToProps = {
};
const MODAL_NEW_DASHBOARD = "MODAL_NEW_DASHBOARD";
+const MODAL_NEW_COLLECTION = "MODAL_NEW_COLLECTION";
@Database.loadList({
// set this to false to prevent a potential spinner on the main nav
@@ -192,7 +194,7 @@ export default class Navbar extends Component {
{
title: t`Collection`,
icon: `all`,
- link: Urls.newCollection("root"),
+ action: () => this.setModal(MODAL_NEW_COLLECTION),
event: `NavBar;New Collection Click;`,
},
]}
@@ -220,17 +222,39 @@ export default class Navbar extends Component {
);
}
+ renderModalContent() {
+ const { modal } = this.state;
+ const { onChangeLocation } = this.props;
+
+ switch (modal) {
+ case MODAL_NEW_COLLECTION:
+ return (
+ <CollectionCreate
+ onClose={() => this.setState({ modal: null })}
+ onSaved={collection => {
+ this.setState({ modal: null });
+ onChangeLocation(Urls.collection(collection));
+ }}
+ />
+ );
+ case MODAL_NEW_DASHBOARD:
+ return (
+ <CreateDashboardModal
+ onClose={() => this.setState({ modal: null })}
+ />
+ );
+ default:
+ return null;
+ }
+ }
+
renderModal() {
const { modal } = this.state;
+
if (modal) {
return (
<Modal onClose={() => this.setState({ modal: null })}>
- {modal === MODAL_NEW_DASHBOARD ? (
- <CreateDashboardModal
- createDashboard={this.props.createDashboard}
- onClose={() => this.setState({ modal: null })}
- />
- ) : null}
+ {this.renderModalContent()}
</Modal>
);
} else {
diff --git a/frontend/test/metabase/scenarios/onboarding/navbar/new-menu.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/navbar/new-menu.cy.spec.js
new file mode 100644
index 00000000000..98a44b9b615
--- /dev/null
+++ b/frontend/test/metabase/scenarios/onboarding/navbar/new-menu.cy.spec.js
@@ -0,0 +1,45 @@
+import { restore, popover, modal } from "__support__/e2e/cypress";
+
+describe("metabase > scenarios > navbar > new menu", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsAdmin();
+
+ cy.visit("/");
+ cy.findByText("New").click();
+ });
+
+ it("question item opens question notebook editor", () => {
+ popover().within(() => {
+ cy.findByText("Question").click();
+ });
+
+ cy.url("should.contain", "/question/notebook#");
+ });
+
+ it("question item opens SQL query editor", () => {
+ popover().within(() => {
+ cy.findByText("SQL query").click();
+ });
+
+ cy.url("should.contain", "/question#");
+ cy.get(".ace_content");
+ });
+
+ it("collection opens modal and redirects to a created collection after saving", () => {
+ popover().within(() => {
+ cy.findByText("Collection").click();
+ });
+
+ modal().within(() => {
+ cy.findByText("Our analytics");
+
+ cy.findByLabelText("Name").type("Test collection");
+ cy.findByLabelText("Description").type("Test collection description");
+
+ cy.findByText("Create").click();
+ });
+
+ cy.get("h1").should("have.text", "Test collection");
+ });
+});
--
GitLab