diff --git a/frontend/src/metabase/App.jsx b/frontend/src/metabase/App.jsx
index 89922c39f14769a232e61c36a5a64bdbe913df61..389ac21c4902d205209073709fe9d4ed561e6a13 100644
--- a/frontend/src/metabase/App.jsx
+++ b/frontend/src/metabase/App.jsx
@@ -1,7 +1,7 @@
/* @flow weak */
-import React, { Component } from "react";
-import { connect } from "react-redux";
+import React, {Component} from "react";
+import {connect} from "react-redux";
import Navbar from "metabase/nav/containers/Navbar.jsx";
@@ -9,26 +9,33 @@ import UndoListing from "metabase/containers/UndoListing";
import NotFound from "metabase/components/NotFound.jsx";
import Unauthorized from "metabase/components/Unauthorized.jsx";
+import Archived from "metabase/components/Archived.jsx";
const mapStateToProps = (state, props) => ({
errorPage: state.app.errorPage
-})
+});
+
+const getErrorComponent = ({status, data, context}) => {
+ if (status === 403) {
+ return <Unauthorized />
+ } else if (data && data.error_code === "archived" && context === "dashboard") {
+ return <Archived entityName="dashboard" linkTo="/dashboards/archive" />
+ } else if (data && data.error_code === "archived" && context === "query-builder") {
+ return <Archived entityName="question" linkTo="/questions/archive" />
+ } else {
+ return <NotFound />
+ }
+}
@connect(mapStateToProps)
export default class App extends Component {
render() {
const { children, location, errorPage } = this.props;
+
return (
<div className="spread flex flex-column">
- <Navbar location={location} className="flex-no-shrink" />
- { errorPage && errorPage.status === 403 ?
- <Unauthorized />
- : errorPage ?
- // TODO: different error page for non-404 errors
- <NotFound />
- :
- children
- }
+ <Navbar location={location} className="flex-no-shrink"/>
+ { errorPage ? getErrorComponent(errorPage) : children }
<UndoListing />
</div>
)
diff --git a/frontend/src/metabase/admin/permissions/selectors.js b/frontend/src/metabase/admin/permissions/selectors.js
index 2b212a7b67ade21268d5c74219d1d0d4ccc7528c..7ec22a97101b9cc2fd5aad360b0ddd9c9baa1b25 100644
--- a/frontend/src/metabase/admin/permissions/selectors.js
+++ b/frontend/src/metabase/admin/permissions/selectors.js
@@ -146,7 +146,7 @@ function getRevokingAccessToAllTablesWarningModal(database, permissions, groupId
// allTableEntityIds contains tables from all schemas
const allTableEntityIds = database.tables().map((table) => ({
databaseId: table.db_id,
- schemaName: table.schema,
+ schemaName: table.schema || "",
tableId: table.id
}));
@@ -456,7 +456,7 @@ export const getDatabasesPermissionsGrid = createSelector(
const getCollections = (state) => state.admin.permissions.collections;
const getCollectionPermission = (permissions, groupId, { collectionId }) =>
- getIn(permissions, [groupId, collectionId])
+ getIn(permissions, [groupId, collectionId]);
export const getCollectionsPermissionsGrid = createSelector(
getCollections, getGroups, getPermissions,
@@ -504,7 +504,6 @@ export const getCollectionsPermissionsGrid = createSelector(
}
);
-
export const getDiff = createSelector(
getMeta, getGroups, getPermissions, getOriginalPermissions,
(metadata: Metadata, groups: Array<Group>, permissions: GroupsPermissions, originalPermissions: GroupsPermissions) =>
diff --git a/frontend/src/metabase/admin/permissions/selectors.spec.fixtures.js b/frontend/src/metabase/admin/permissions/selectors.spec.fixtures.js
new file mode 100644
index 0000000000000000000000000000000000000000..e6e4109acd2a636a20a1ebb4a95a82d3544c0b2e
--- /dev/null
+++ b/frontend/src/metabase/admin/permissions/selectors.spec.fixtures.js
@@ -0,0 +1,93 @@
+// Database 2 contains an imaginary multi-schema database (like Redshift for instance)
+// Database 3 contains an imaginary database which doesn't have any schemas (like MySQL)
+// (A single-schema database was originally Database 1 but it got removed as testing against it felt redundant)
+
+export const normalizedMetadata = {
+ "metrics": {},
+ "segments": {},
+ "databases": {
+ "2": {
+ "name": "Imaginary Multi-Schema Dataset",
+ "tables": [
+ // In schema_1
+ 5,
+ 6,
+ // In schema_2
+ 7,
+ 8,
+ 9
+ ],
+ "id": 2
+ },
+ "3": {
+ "name": "Imaginary Schemaless Dataset",
+ "tables": [
+ 10,
+ 11,
+ 12,
+ 13
+ ],
+ "id": 3
+ }
+ },
+ "tables": {
+ "5": {
+ "schema": "schema_1",
+ "name": "Avian Singles Messages",
+ "id": 5,
+ "db_id": 2
+ },
+ "6": {
+ "schema": "schema_1",
+ "name": "Avian Singles Users",
+ "id": 6,
+ "db_id": 2
+ },
+ "7": {
+ "schema": "schema_2",
+ "name": "Tupac Sightings Sightings",
+ "id": 7,
+ "db_id": 2
+ },
+ "8": {
+ "schema": "schema_2",
+ "name": "Tupac Sightings Categories",
+ "id": 8,
+ "db_id": 2
+ },
+ "9": {
+ "schema": "schema_2",
+ "name": "Tupac Sightings Cities",
+ "id": 9,
+ "db_id": 2
+ },
+ "10": {
+ "schema": null,
+ "name": "Badminton Men's Double Results",
+ "id": 10,
+ "db_id": 3
+ },
+ "11": {
+ "schema": null,
+ "name": "Badminton Mixed Double Results",
+ "id": 11,
+ "db_id": 3
+ },
+ "12": {
+ "schema": null,
+ "name": "Badminton Women's Singles Results",
+ "id": 12,
+ "db_id": 3
+ },
+ "13": {
+ "schema": null,
+ "name": "Badminton Mixed Singles Results",
+ "id": 13,
+ "db_id": 3
+ },
+ },
+ "fields": {/* stripped out */},
+ "revisions": {},
+ "databasesList": [2, 3]
+};
+
diff --git a/frontend/src/metabase/admin/permissions/selectors.spec.js b/frontend/src/metabase/admin/permissions/selectors.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..1b2b32e50005d88263c1c06b23630417013ef887
--- /dev/null
+++ b/frontend/src/metabase/admin/permissions/selectors.spec.js
@@ -0,0 +1,468 @@
+/**
+ * Tests granting and revoking permissions against three kinds of datasets:
+ * - dataset with tables in a single PUBLIC schema (for instance H2 or PostgreSQL if no custom schemas created)
+ * - dataset with no schemas (for instance MySQL)
+ * - dataset with multiple schemas (for instance Redshift)
+ */
+
+import { setIn } from "icepick";
+
+jest.mock('metabase/lib/analytics');
+
+import {GroupsPermissions} from "metabase/meta/types/Permissions";
+import { normalizedMetadata } from "./selectors.spec.fixtures";
+import { getTablesPermissionsGrid, getSchemasPermissionsGrid, getDatabasesPermissionsGrid } from "./selectors";
+
+/******** INITIAL TEST STATE ********/
+
+const groups = [{
+ id: 1,
+ name: "Group starting with full access",
+}, {
+ id: 2,
+ name: "Group starting with no access at all",
+}];
+
+const initialPermissions: GroupsPermissions = {
+ 1: {
+ // Sample dataset
+ 1: {
+ "native": "write",
+ "schemas": "all"
+ },
+ // Imaginary multi-schema
+ 2: {
+ "native": "write",
+ "schemas": "all"
+ },
+ // Imaginary schemaless
+ 3: {
+ "native": "write",
+ "schemas": "all"
+ }
+ },
+ 2: {
+ // Sample dataset
+ 1: {
+ "native": "none",
+ "schemas": "none"
+ },
+ // Imaginary multi-schema
+ 2: {
+ "native": "none",
+ "schemas": "none"
+ },
+ // Imaginary schemaless
+ 3: {
+ "native": "none",
+ "schemas": "none"
+ }
+ }
+};
+
+
+/******** MANAGING THE CURRENT (SIMULATED) STATE TREE ********/
+
+const initialState = {
+ admin: {
+ permissions: {
+ permissions: initialPermissions,
+ originalPermissions: initialPermissions,
+ groups
+ }
+ },
+ metadata: normalizedMetadata
+};
+
+var state = initialState;
+const resetState = () => { state = initialState };
+const getPermissionsTree = () => state.admin.permissions.permissions;
+const getPermissionsForDb = ({ entityId, groupId }) => getPermissionsTree()[groupId][entityId.databaseId];
+
+const updatePermissionsInState = (permissions) => {
+ state = setIn(state, ["admin", "permissions", "permissions"], permissions);
+};
+
+const getProps = ({ databaseId, schemaName }) => ({
+ params: {
+ databaseId,
+ schemaName
+ }
+});
+
+/******** HIGH-LEVEL METHODS FOR UPDATING PERMISSIONS ********/
+
+const changePermissionsForEntityInGrid = ({ grid, category, entityId, groupId, permission }) => {
+ const newPermissions = grid.permissions[category].updater(groupId, entityId, permission);
+ updatePermissionsInState(newPermissions);
+ return newPermissions;
+};
+
+const changeDbNativePermissionsForEntity = ({ entityId, groupId, permission }) => {
+ const grid = getDatabasesPermissionsGrid(state, getProps(entityId));
+ return changePermissionsForEntityInGrid({ grid, category: "native", entityId, groupId, permission });
+};
+
+const changeDbDataPermissionsForEntity = ({ entityId, groupId, permission }) => {
+ const grid = getDatabasesPermissionsGrid(state, getProps(entityId));
+ return changePermissionsForEntityInGrid({ grid, category: "schemas", entityId, groupId, permission });
+};
+
+const changeSchemaPermissionsForEntity = ({ entityId, groupId, permission }) => {
+ const grid = getSchemasPermissionsGrid(state, getProps(entityId));
+ return changePermissionsForEntityInGrid({ grid, category: "tables", entityId, groupId, permission });
+};
+
+const changeTablePermissionsForEntity = ({ entityId, groupId, permission }) => {
+ const grid = getTablesPermissionsGrid(state, getProps(entityId));
+ return changePermissionsForEntityInGrid({ grid, category: "fields", entityId, groupId, permission });
+};
+
+const getMethodsForDbAndSchema = (entityId) => ({
+ changeDbNativePermissions: ({ groupId, permission }) =>
+ changeDbNativePermissionsForEntity({ entityId, groupId, permission }),
+ changeDbDataPermissions: ({ groupId, permission }) =>
+ changeDbDataPermissionsForEntity({ entityId, groupId, permission }),
+ changeSchemaPermissions: ({ groupId, permission }) =>
+ changeSchemaPermissionsForEntity({ entityId, groupId, permission }),
+ changeTablePermissions: ({ tableId, groupId, permission }) =>
+ changeTablePermissionsForEntity({ entityId: {...entityId, tableId}, groupId, permission }),
+ getPermissions: ({ groupId }) =>
+ getPermissionsForDb({ entityId, groupId })
+});
+
+/******** ACTUAL TESTS ********/
+
+describe("permissions selectors", () => {
+ beforeEach(resetState);
+
+ describe("for a schemaless dataset", () => {
+ // Schema "name" (better description would be a "permission path identifier") is simply an empty string
+ // for databases where the metadata value for table schema is `null`
+ const schemalessDataset = getMethodsForDbAndSchema({ databaseId: 3, schemaName: "" });
+
+ it("should restrict access correctly on table level", () => {
+ // Revoking access to one table should downgrade the native permissions to "read"
+ schemalessDataset.changeTablePermissions({ tableId: 10, groupId: 1, permission: "none" });
+ expect(schemalessDataset.getPermissions({ groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": {
+ "": {
+ "10": "none",
+ "11": "all",
+ "12": "all",
+ "13": "all"
+ }
+ }
+ });
+
+ // Revoking access to the rest of tables one-by-one...
+ schemalessDataset.changeTablePermissions({ tableId: 11, groupId: 1, permission: "none" });
+ schemalessDataset.changeTablePermissions({ tableId: 12, groupId: 1, permission: "none" });
+ schemalessDataset.changeTablePermissions({ tableId: 13, groupId: 1, permission: "none" });
+
+ expect(schemalessDataset.getPermissions({groupId: 1})).toMatchObject({
+ // ...should revoke all permissions for that database
+ "native": "none",
+ "schemas": "none"
+ });
+
+ });
+
+ it("should restrict access correctly on db level", () => {
+ // Should let change the native permission to "read"
+ schemalessDataset.changeDbNativePermissions({ groupId: 1, permission: "read" });
+ expect(schemalessDataset.getPermissions({groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": "all"
+ });
+
+ // Should not let change the native permission to none
+ schemalessDataset.changeDbNativePermissions({ groupId: 1, permission: "none" });
+ expect(schemalessDataset.getPermissions({groupId: 1})).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+
+ resetState(); // ad-hoc state reset for the next test
+ // Revoking the data access to the database at once should revoke all permissions for that database
+ schemalessDataset.changeDbDataPermissions({ groupId: 1, permission: "none" });
+ expect(schemalessDataset.getPermissions({groupId: 1})).toMatchObject({
+ "native": "none",
+ "schemas": "none"
+ });
+ });
+
+ it("should grant more access correctly on table level", () => {
+ // Simply grant an access to a single table
+ schemalessDataset.changeTablePermissions({ tableId: 12, groupId: 2, permission: "all" });
+ expect(schemalessDataset.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "": {
+ "10": "none",
+ "11": "none",
+ "12": "all",
+ "13": "none"
+ }
+ }
+ });
+
+ // Grant the access to rest of tables
+ schemalessDataset.changeTablePermissions({ tableId: 10, groupId: 2, permission: "all" });
+ schemalessDataset.changeTablePermissions({ tableId: 11, groupId: 2, permission: "all" });
+ schemalessDataset.changeTablePermissions({ tableId: 13, groupId: 2, permission: "all" });
+ expect(schemalessDataset.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+
+
+ // Should pass changes to native permissions through
+ schemalessDataset.changeDbNativePermissions({ groupId: 2, permission: "read" });
+ expect(schemalessDataset.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "read",
+ "schemas": "all"
+ });
+ });
+
+ it("should grant more access correctly on db level", () => {
+ // Setting limited access should produce a permission tree where each schema has "none" access
+ // (this is a strange, rather no-op edge case but the UI currently enables this)
+ schemalessDataset.changeDbDataPermissions({ groupId: 2, permission: "controlled" });
+ expect(schemalessDataset.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "": "none"
+ }
+ });
+
+ // Granting native access should also grant a full write access
+ schemalessDataset.changeDbNativePermissions({ groupId: 2, permission: "write" });
+ expect(schemalessDataset.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "write",
+ "schemas": "all"
+ });
+
+ resetState(); // ad-hoc reset (normally run before tests)
+ // test that setting full access works too
+ schemalessDataset.changeDbDataPermissions({ groupId: 2, permission: "all" });
+ expect(schemalessDataset.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+ })
+ });
+
+ describe("for a dataset with multiple schemas", () => {
+ const schema1 = getMethodsForDbAndSchema({ databaseId: 2, schemaName: "schema_1" });
+ const schema2 = getMethodsForDbAndSchema({ databaseId: 2, schemaName: "schema_2" });
+
+ it("should restrict access correctly on table level", () => {
+ // Revoking access to one table should downgrade the native permissions to "read"
+ schema1.changeTablePermissions({ tableId: 5, groupId: 1, permission: "none" });
+ expect(schema1.getPermissions({ groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": {
+ "schema_1": {
+ "5": "none",
+ "6": "all"
+ },
+ "schema_2": "all"
+ }
+ });
+
+ // State where both schemas have mixed permissions
+ schema2.changeTablePermissions({ tableId: 8, groupId: 1, permission: "none" });
+ schema2.changeTablePermissions({ tableId: 9, groupId: 1, permission: "none" });
+ expect(schema2.getPermissions({groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": {
+ "schema_1": {
+ "5": "none",
+ "6": "all"
+ },
+ "schema_2": {
+ "7": "all",
+ "8": "none",
+ "9": "none"
+ }
+ }
+ });
+
+ // Completely revoke access to the first schema with table-level changes
+ schema1.changeTablePermissions({ tableId: 6, groupId: 1, permission: "none" });
+
+ expect(schema1.getPermissions({groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": {
+ "schema_1": "none",
+ "schema_2": {
+ "7": "all",
+ "8": "none",
+ "9": "none"
+ }
+ }
+ });
+
+ // Revoking all permissions of the other schema should revoke all db permissions too
+ schema2.changeTablePermissions({ tableId: 7, groupId: 1, permission: "none" });
+ expect(schema2.getPermissions({groupId: 1})).toMatchObject({
+ "native": "none",
+ "schemas": "none"
+ });
+ });
+
+ it("should restrict access correctly on schema level", () => {
+ // Revoking access to one schema
+ schema2.changeSchemaPermissions({ groupId: 1, permission: "none" });
+ expect(schema2.getPermissions({groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": {
+ "schema_1": "all",
+ "schema_2": "none"
+ }
+ });
+
+ // Revoking access to other too
+ schema1.changeSchemaPermissions({ groupId: 1, permission: "none" });
+ expect(schema1.getPermissions({groupId: 1})).toMatchObject({
+ "native": "none",
+ "schemas": "none"
+ });
+ });
+
+ it("should restrict access correctly on db level", () => {
+ // Should let change the native permission to "read"
+ schema1.changeDbNativePermissions({ groupId: 1, permission: "read" });
+ expect(schema1.getPermissions({groupId: 1})).toMatchObject({
+ "native": "read",
+ "schemas": "all"
+ });
+
+ // Should let change the native permission to none
+ schema1.changeDbNativePermissions({ groupId: 1, permission: "none" });
+ expect(schema1.getPermissions({groupId: 1})).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+
+ resetState(); // ad-hoc state reset for the next test
+ // Revoking the data access to the database at once should revoke all permissions for that database
+ schema1.changeDbDataPermissions({ groupId: 1, permission: "none" });
+ expect(schema1.getPermissions({groupId: 1})).toMatchObject({
+ "native": "none",
+ "schemas": "none"
+ });
+ });
+
+ it("should grant more access correctly on table level", () => {
+ // Simply grant an access to a single table
+ schema2.changeTablePermissions({ tableId: 7, groupId: 2, permission: "all" });
+ expect(schema2.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "schema_1": "none",
+ "schema_2": {
+ "7": "all",
+ "8": "none",
+ "9": "none"
+ }
+ }
+ });
+
+ // State where both schemas have mixed permissions
+ schema1.changeTablePermissions({ tableId: 5, groupId: 2, permission: "all" });
+ expect(schema1.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "schema_1": {
+ "5": "all",
+ "6": "none"
+ },
+ "schema_2": {
+ "7": "all",
+ "8": "none",
+ "9": "none"
+ }
+ }
+ });
+
+ // Grant full access to the second schema
+ schema2.changeTablePermissions({ tableId: 8, groupId: 2, permission: "all" });
+ schema2.changeTablePermissions({ tableId: 9, groupId: 2, permission: "all" });
+ expect(schema2.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "schema_1": {
+ "5": "all",
+ "6": "none"
+ },
+ "schema_2": "all"
+ }
+ });
+
+ // Grant the access to whole db (no native yet)
+ schema1.changeTablePermissions({ tableId: 5, groupId: 2, permission: "all" });
+ schema1.changeTablePermissions({ tableId: 6, groupId: 2, permission: "all" });
+ expect(schema1.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+
+ // Should pass changes to native permissions through
+ schema1.changeDbNativePermissions({ groupId: 2, permission: "read" });
+ expect(schema1.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "read",
+ "schemas": "all"
+ });
+ });
+
+ it("should grant more access correctly on schema level", () => {
+ // Granting full access to one schema
+ schema1.changeSchemaPermissions({ groupId: 2, permission: "all" });
+ expect(schema1.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "schema_1": "all",
+ "schema_2": "none"
+ }
+ });
+
+ // Granting access to the other as well
+ schema2.changeSchemaPermissions({ groupId: 2, permission: "all" });
+ expect(schema2.getPermissions({groupId: 2})).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+ });
+
+ it("should grant more access correctly on db level", () => {
+ // Setting limited access should produce a permission tree where each schema has "none" access
+ // (this is a strange, rather no-op edge case but the UI currently enables this)
+ schema1.changeDbDataPermissions({ groupId: 2, permission: "controlled" });
+ expect(schema1.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "none",
+ "schemas": {
+ "schema_1": "none",
+ "schema_2": "none"
+ }
+ });
+
+ // Granting native access should also grant a full write access
+ schema1.changeDbNativePermissions({ groupId: 2, permission: "write" });
+ expect(schema1.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "write",
+ "schemas": "all"
+ });
+
+ resetState(); // ad-hoc reset (normally run before tests)
+ // test that setting full access works too
+ schema1.changeDbDataPermissions({ groupId: 2, permission: "all" });
+ expect(schema1.getPermissions({ groupId: 2 })).toMatchObject({
+ "native": "none",
+ "schemas": "all"
+ });
+ })
+ });
+});
diff --git a/frontend/src/metabase/components/Archived.jsx b/frontend/src/metabase/components/Archived.jsx
new file mode 100644
index 0000000000000000000000000000000000000000..64d514cb43c5d2784174a61fb6d342799b38c17a
--- /dev/null
+++ b/frontend/src/metabase/components/Archived.jsx
@@ -0,0 +1,16 @@
+import React from 'react';
+import EmptyState from "metabase/components/EmptyState";
+import Link from "metabase/components/Link";
+
+const Archived = ({ entityName, linkTo }) =>
+ <div className="full-height flex justify-center align-center">
+ <EmptyState
+ message={<div>
+ <div>This {entityName} has been archived</div>
+ <Link to={linkTo} className="my2 link" style={{fontSize: "14px"}}>View the archive</Link>
+ </div>}
+ icon="viewArchive"
+ />
+ </div>;
+
+export default Archived;
\ No newline at end of file
diff --git a/frontend/src/metabase/components/LoadingAndErrorWrapper.jsx b/frontend/src/metabase/components/LoadingAndErrorWrapper.jsx
index c5fd86d101bbcf8736044228f4230f1de6ce0212..f37e11c77ab98338e86add30b197ca05e06c6617 100644
--- a/frontend/src/metabase/components/LoadingAndErrorWrapper.jsx
+++ b/frontend/src/metabase/components/LoadingAndErrorWrapper.jsx
@@ -28,7 +28,8 @@ export default class LoadingAndErrorWrapper extends Component {
getErrorMessage() {
const { error } = this.props;
return (
- error.data ||
+ // NOTE Atte Keinänen 5/10/17 Dashboard API endpoint returns the error as JSON with `message` field
+ error.data && (error.data.message ? error.data.message : error.data) ||
error.statusText ||
error.message ||
"An error occured"
diff --git a/frontend/src/metabase/dashboard/components/DashCard.jsx b/frontend/src/metabase/dashboard/components/DashCard.jsx
index b6b0fc202a3908d23b6c564f91e9eb3883a1fc1c..6474eca68af3b13631a7b8811e5b11bedddd284c 100644
--- a/frontend/src/metabase/dashboard/components/DashCard.jsx
+++ b/frontend/src/metabase/dashboard/components/DashCard.jsx
@@ -34,6 +34,7 @@ export default class DashCard extends Component {
parameterValues: PropTypes.object.isRequired,
markNewCardSeen: PropTypes.func.isRequired,
fetchCardData: PropTypes.func.isRequired,
+ navigateToNewCardFromDashboard: PropTypes.func.isRequired
};
async componentDidMount() {
@@ -60,7 +61,7 @@ export default class DashCard extends Component {
isEditingParameter,
onAddSeries,
onRemove,
- navigateToNewCard,
+ navigateToNewCardFromDashboard,
metadata
} = this.props;
@@ -134,8 +135,9 @@ export default class DashCard extends Component {
onUpdateVisualizationSettings={this.props.onUpdateVisualizationSettings}
replacementContent={isEditingParameter && <DashCardParameterMapper dashcard={dashcard} />}
metadata={metadata}
- onChangeCardAndRun={ navigateToNewCard ? (card: UnsavedCard) => {
- navigateToNewCard(card, dashcard)
+ onChangeCardAndRun={ navigateToNewCardFromDashboard ? ({ nextCard, previousCard }) => {
+ // navigateToNewCardFromDashboard needs `dashcard` for applying active filters to the query
+ navigateToNewCardFromDashboard({ nextCard, previousCard, dashcard })
} : null}
/>
</div>
diff --git a/frontend/src/metabase/dashboard/components/Dashboard.jsx b/frontend/src/metabase/dashboard/components/Dashboard.jsx
index e9c8458e90b33a59bc1f01f56b1090cd843db6f6..ef76f09aad0989851e9965cacb57614e5a982ff7 100644
--- a/frontend/src/metabase/dashboard/components/Dashboard.jsx
+++ b/frontend/src/metabase/dashboard/components/Dashboard.jsx
@@ -147,7 +147,7 @@ export default class Dashboard extends Component {
}
} catch (error) {
if (error.status === 404) {
- setErrorPage(error);
+ setErrorPage({ ...error, context: "dashboard" });
} else {
console.error(error);
this.setState({ error });
@@ -198,40 +198,40 @@ export default class Dashboard extends Component {
return (
<LoadingAndErrorWrapper className={cx("Dashboard flex-full pb4", { "Dashboard--fullscreen": isFullscreen, "Dashboard--night": isNightMode})} loading={!dashboard} error={error}>
- {() =>
- <div className="full" style={{ overflowX: "hidden" }}>
- <header className="DashboardHeader relative z2">
- <DashboardHeader
- {...this.props}
- onEditingChange={this.setEditing}
- setDashboardAttribute={this.setDashboardAttribute}
- addParameter={this.props.addParameter}
- parametersWidget={parametersWidget}
- />
- </header>
- {!isFullscreen && parametersWidget &&
- <div className="wrapper flex flex-column align-start mt2 relative z2">
- {parametersWidget}
- </div>
- }
- <div className="wrapper">
-
- { dashboard.ordered_cards.length === 0 ?
- <div className="absolute z1 top bottom left right flex flex-column layout-centered">
- <span className="QuestionCircle">?</span>
- <div className="text-normal mt3 mb1">This dashboard is looking empty.</div>
- <div className="text-normal text-grey-2">Add a question to start making it useful!</div>
- </div>
- :
- <DashboardGrid
+ {() =>
+ <div className="full" style={{ overflowX: "hidden" }}>
+ <header className="DashboardHeader relative z2">
+ <DashboardHeader
{...this.props}
onEditingChange={this.setEditing}
+ setDashboardAttribute={this.setDashboardAttribute}
+ addParameter={this.props.addParameter}
+ parametersWidget={parametersWidget}
/>
+ </header>
+ {!isFullscreen && parametersWidget &&
+ <div className="wrapper flex flex-column align-start mt2 relative z2">
+ {parametersWidget}
+ </div>
}
+ <div className="wrapper">
+
+ { dashboard.ordered_cards.length === 0 ?
+ <div className="absolute z1 top bottom left right flex flex-column layout-centered">
+ <span className="QuestionCircle">?</span>
+ <div className="text-normal mt3 mb1">This dashboard is looking empty.</div>
+ <div className="text-normal text-grey-2">Add a question to start making it useful!</div>
+ </div>
+ :
+ <DashboardGrid
+ {...this.props}
+ onEditingChange={this.setEditing}
+ />
+ }
+ </div>
</div>
- </div>
- }
+ }
</LoadingAndErrorWrapper>
);
}
-}
+}
\ No newline at end of file
diff --git a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx
index b1ab2839a3bd0217bec0f776911e6a6e99de07be..ce36abfd94c59470b0421a9600e69f82b3313f37 100644
--- a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx
+++ b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx
@@ -193,7 +193,7 @@ export default class DashboardGrid extends Component {
onAddSeries={this.onDashCardAddSeries.bind(this, dc)}
onUpdateVisualizationSettings={this.props.onUpdateDashCardVisualizationSettings.bind(this, dc.id)}
onReplaceAllVisualizationSettings={this.props.onReplaceAllDashCardVisualizationSettings.bind(this, dc.id)}
- navigateToNewCard={this.props.navigateToNewCard}
+ navigateToNewCardFromDashboard={this.props.navigateToNewCardFromDashboard}
metadata={this.props.metadata}
/>
)
diff --git a/frontend/src/metabase/dashboard/dashboard.js b/frontend/src/metabase/dashboard/dashboard.js
index 2bff4047b2ccefd3e7a7b1d656efa228f65bacc1..24aebceb82f09b61cc71f0925de46df7259400ad 100644
--- a/frontend/src/metabase/dashboard/dashboard.js
+++ b/frontend/src/metabase/dashboard/dashboard.js
@@ -14,7 +14,7 @@ import { applyParameters, questionUrlWithParameters } from "metabase/meta/Card";
import { getParametersBySlug } from "metabase/meta/Parameter";
import type { DashboardWithCards, DashCard, DashCardId } from "metabase/meta/types/Dashboard";
-import type { UnsavedCard, Card, CardId } from "metabase/meta/types/Card";
+import type { Card, CardId } from "metabase/meta/types/Card";
import Utils from "metabase/lib/utils";
import { getPositionForNewDashCard } from "metabase/lib/dashboard_grid";
@@ -25,6 +25,7 @@ import { push } from "react-router-redux";
import { DashboardApi, CardApi, RevisionApi, PublicApi, EmbedApi } from "metabase/services";
import { getDashboard, getDashboardComplete } from "./selectors";
+import {getCardAfterVisualizationClick} from "metabase/visualizations/lib/utils";
const DATASET_SLOW_TIMEOUT = 15 * 1000;
@@ -491,24 +492,43 @@ export const deletePublicLink = createAction(DELETE_PUBLIC_LINK, async ({ id })
return { id };
});
-/** All navigation actions from dashboards to cards (e.x. clicking a title, drill through)
- * should go through this action, which merges any currently applied dashboard filters
- * into the new card / URL parameters.
+/**
+ * All navigation actions from dashboards to cards (e.x. clicking a title, drill through)
+ * should go through this action, which merges any currently applied dashboard filters
+ * into the new card / URL parameters.
+ *
+ * User-triggered events that are handled here:
+ * - clicking a dashcard legend:
+ * * question title legend (only for single-question cards)
+ * * series legend (multi-aggregation, multi-breakout, multiple questions)
+ * - clicking the visualization inside dashcard
+ * * drill-through (single series, multi-aggregation, multi-breakout, multiple questions)
+ * * (not in 0.24.2 yet: drag on line/area/bar visualization)
+ * - those all can be applied without or with a dashboard filter
*/
-// TODO Atte Keinänen 5/2/17: This could be combined with `setCardAndRun` of query_builder/actions.js
-// Having two separate actions for very similar behavior was a source of initial confusion for me
const NAVIGATE_TO_NEW_CARD = "metabase/dashboard/NAVIGATE_TO_NEW_CARD";
-export const navigateToNewCard = createThunkAction(NAVIGATE_TO_NEW_CARD, (card: UnsavedCard, dashcard: DashCard) =>
- (dispatch, getState) => {
- const { metadata } = getState();
- const { dashboardId, dashboards, parameterValues } = getState().dashboard;
- const dashboard = dashboards[dashboardId];
+export const navigateToNewCardFromDashboard = createThunkAction(
+ NAVIGATE_TO_NEW_CARD,
+ ({ nextCard, previousCard, dashcard }) =>
+ (dispatch, getState) => {
+ const {metadata} = getState();
+ const {dashboardId, dashboards, parameterValues} = getState().dashboard;
+ const dashboard = dashboards[dashboardId];
+ const cardIsDirty = !_.isEqual(previousCard.dataset_query, nextCard.dataset_query);
+
+ const url = questionUrlWithParameters(
+ getCardAfterVisualizationClick(nextCard, previousCard),
+ metadata,
+ dashboard.parameters,
+ parameterValues,
+ dashcard && dashcard.parameter_mappings,
+ cardIsDirty
+ );
- // $FlowFixMe
- const url = questionUrlWithParameters(card, metadata, dashboard.parameters, parameterValues, dashcard && dashcard.parameter_mappings);
- dispatch(push(url));
- });
+ dispatch(push(url));
+ }
+);
// reducers
diff --git a/frontend/src/metabase/dashboards/containers/Dashboards.jsx b/frontend/src/metabase/dashboards/containers/Dashboards.jsx
index b989c2685fdbda5712c697c2cc73c34119ee119c..3b6d558a99c13c1d2aaf253de8afbf982733941f 100644
--- a/frontend/src/metabase/dashboards/containers/Dashboards.jsx
+++ b/frontend/src/metabase/dashboards/containers/Dashboards.jsx
@@ -144,9 +144,8 @@ export class Dashboards extends Component {
return (
<LoadingAndErrorWrapper
- style={{ backgroundColor: "#f9fbfc" }}
loading={isLoading}
- className={cx("relative px4 full-height", {"flex flex-full flex-column": noDashboardsCreated})}
+ className={cx("relative px4 full-height bg-slate-extra-light", {"flex flex-full flex-column": noDashboardsCreated})}
noBackground
>
{ modalOpen ? this.renderCreateDashboardModal() : null }
diff --git a/frontend/src/metabase/lib/dom.js b/frontend/src/metabase/lib/dom.js
index eb402e5403b9d17d28572e893cc2b57457b959ed..af5a76cbd5017d7a543328d49755f8bbe1e490a4 100644
--- a/frontend/src/metabase/lib/dom.js
+++ b/frontend/src/metabase/lib/dom.js
@@ -209,6 +209,14 @@ export function constrainToScreen(element, direction, padding) {
return false;
}
+// Used for tackling Safari rendering issues
+// http://stackoverflow.com/a/3485654
+export function forceRedraw(domNode) {
+ domNode.style.display='none';
+ domNode.offsetHeight;
+ domNode.style.display='';
+}
+
export function moveToBack(element) {
if (element && element.parentNode) {
element.parentNode.insertBefore(
diff --git a/frontend/src/metabase/lib/permissions.js b/frontend/src/metabase/lib/permissions.js
index cf2e925ee2debcebad380dc7173cca56ced9d61e..afc79b85473a3bee618190ca8d6cb6100a02180d 100644
--- a/frontend/src/metabase/lib/permissions.js
+++ b/frontend/src/metabase/lib/permissions.js
@@ -109,13 +109,15 @@ export function downgradeNativePermissionsIfNeeded(permissions: GroupsPermission
}
}
-// $FlowFixMe
-const metadataTableToTableEntityId = (table: Table): TableEntityId => ({ databaseId: table.db_id, schemaName: table.schema, tableId: table.id });
+const metadataTableToTableEntityId = (table: Table): TableEntityId => ({ databaseId: table.db_id, schemaName: table.schema || "", tableId: table.id });
+
+// TODO Atte Keinänen 6/24/17 See if this method could be simplified
const entityIdToMetadataTableFields = (entityId: EntityId) => ({
...(entityId.databaseId ? {db_id: entityId.databaseId} : {}),
- ...(entityId.schemaName ? {schema: entityId.schemaName} : {}),
- ...(entityId.tableId ? {tableId: entityId.tableId} : {})
-})
+ // $FlowFixMe Because schema name can be an empty string, which means an empty schema, this check becomes a little nasty
+ ...(entityId.schemaName !== undefined ? {schema: entityId.schemaName !== "" ? entityId.schemaName : null} : {}),
+ ...(entityId.tableId ? {id: entityId.tableId} : {})
+});
function inferEntityPermissionValueFromChildTables(permissions: GroupsPermissions, groupId: GroupId, entityId: DatabaseEntityId|SchemaEntityId, metadata: Metadata) {
const { databaseId } = entityId;
@@ -177,7 +179,7 @@ export function updateFieldsPermission(permissions: GroupsPermissions, groupId:
export function updateTablesPermission(permissions: GroupsPermissions, groupId: GroupId, { databaseId, schemaName }: SchemaEntityId, value: string, metadata: Metadata): GroupsPermissions {
const database = metadata && metadata.database(databaseId);
- const tableIds: ?number[] = database && database.tables().filter(t => t.schema === schemaName).map(t => t.id);
+ const tableIds: ?number[] = database && database.tables().filter(t => (t.schema || "") === schemaName).map(t => t.id);
permissions = updateSchemasPermission(permissions, groupId, { databaseId }, "controlled", metadata);
permissions = updatePermission(permissions, groupId, [databaseId, "schemas", schemaName], value, tableIds);
@@ -186,11 +188,12 @@ export function updateTablesPermission(permissions: GroupsPermissions, groupId:
}
export function updateSchemasPermission(permissions: GroupsPermissions, groupId: GroupId, { databaseId }: DatabaseEntityId, value: string, metadata: Metadata): GroupsPermissions {
- let database = metadata.database(databaseId);
- let schemaNames = database && database.schemaNames();
+ const database = metadata.database(databaseId);
+ const schemaNames = database && database.schemaNames();
+ const schemaNamesOrNoSchema = (schemaNames && schemaNames.length > 0) ? schemaNames : [""];
permissions = downgradeNativePermissionsIfNeeded(permissions, groupId, { databaseId }, value, metadata);
- return updatePermission(permissions, groupId, [databaseId, "schemas"], value, schemaNames);
+ return updatePermission(permissions, groupId, [databaseId, "schemas"], value, schemaNamesOrNoSchema);
}
export function updateNativePermission(permissions: GroupsPermissions, groupId: GroupId, { databaseId }: DatabaseEntityId, value: string, metadata: Metadata): GroupsPermissions {
diff --git a/frontend/src/metabase/lib/redux.js b/frontend/src/metabase/lib/redux.js
index c1259080e302c6e871fc9b1eedef5f3d89e4b62f..21d8e93d7e385fe9645d9d381fd616937b2959b0 100644
--- a/frontend/src/metabase/lib/redux.js
+++ b/frontend/src/metabase/lib/redux.js
@@ -110,6 +110,36 @@ export const updateData = async ({
}
}
+// helper for working with normalizr
+// merge each entity from newEntities with existing entity, if any
+// this ensures partial entities don't overwrite existing entities with more properties
+export function mergeEntities(entities, newEntities) {
+ entities = { ...entities };
+ for (const id in newEntities) {
+ if (id in entities) {
+ entities[id] = { ...entities[id], ...newEntities[id] };
+ } else {
+ entities[id] = newEntities[id];
+ }
+ }
+ return entities;
+}
+
+// helper for working with normalizr
+// reducer that merges payload.entities
+export function handleEntities(actionPattern, entityType, reducer) {
+ return (state, action) => {
+ if (state === undefined) {
+ state = {};
+ }
+ let entities = getIn(action, ["payload", "entities", entityType]);
+ if (actionPattern.test(action.type) && entities) {
+ state = mergeEntities(state, entities);
+ }
+ return reducer(state, action);
+ }
+}
+
// for filtering non-DOM props from redux-form field objects
// https://github.com/erikras/redux-form/issues/1441
export const formDomOnlyProps = ({
diff --git a/frontend/src/metabase/meta/Card.js b/frontend/src/metabase/meta/Card.js
index ca2bc5d82cb5a536716ab632b543452689bbdb03..6db71543c4a7ca406d39036225f722aa2c660dde 100644
--- a/frontend/src/metabase/meta/Card.js
+++ b/frontend/src/metabase/meta/Card.js
@@ -131,7 +131,11 @@ export function applyParameters(
continue;
}
- const mapping = _.findWhere(parameterMappings, { card_id: card.id, parameter_id: parameter.id });
+ const mapping = _.findWhere(parameterMappings, {
+ // $FlowFixMe original_card_id not included in the flow type of card
+ card_id: card.id || card.original_card_id,
+ parameter_id: parameter.id
+ });
if (mapping) {
// mapped target, e.x. on a dashboard
datasetQuery.parameters.push({
@@ -158,7 +162,8 @@ export function questionUrlWithParameters(
metadata: Metadata,
parameters: Parameter[],
parameterValues: ParameterValues = {},
- parameterMappings: ParameterMapping[] = []
+ parameterMappings: ParameterMapping[] = [],
+ cardIsDirty: boolean = true
): DatasetQuery {
if (!card.dataset_query) {
return Urls.question(card.id);
@@ -174,6 +179,11 @@ export function questionUrlWithParameters(
parameterMappings
);
+ // If we have a clean question without parameters applied, don't add the dataset query hash
+ if (!cardIsDirty && datasetQuery.parameters && datasetQuery.parameters.length === 0) {
+ return Urls.question(card.id);
+ }
+
const query = {};
for (const datasetParameter of datasetQuery.parameters || []) {
const cardParameter = _.find(cardParameters, p =>
diff --git a/frontend/src/metabase/meta/Card.spec.js b/frontend/src/metabase/meta/Card.spec.js
index 8830d733ab259bb4bdc9441ec8ee4124670040ed..f94eef948e3822f26ec664669b05071d275f8e4f 100644
--- a/frontend/src/metabase/meta/Card.spec.js
+++ b/frontend/src/metabase/meta/Card.spec.js
@@ -135,6 +135,26 @@ describe("metabase/meta/Card", () => {
)
});
});
+ it("should return question URL even if only original_card_id is present", () => {
+ const cardWithOnlyOriginalCardId = { ...card, id: undefined, original_card_id: card.id };
+
+ const url = Card.questionUrlWithParameters(
+ cardWithOnlyOriginalCardId,
+ metadata,
+ parameters,
+ { "1": "bar" },
+ parameterMappings
+ );
+ expect(parseUrl(url)).toEqual({
+ pathname: "/question",
+ query: {},
+ card: assocIn(
+ cardWithOnlyOriginalCardId,
+ ["dataset_query", "query", "filter"],
+ ["AND", ["=", ["field-id", 1], "bar"]]
+ )
+ });
+ });
it("should return question URL with number MBQL filter added", () => {
const url = Card.questionUrlWithParameters(
card,
diff --git a/frontend/src/metabase/meta/types/Visualization.js b/frontend/src/metabase/meta/types/Visualization.js
index 75b6f11e09ca8b7fd40c614047f02da0108b8585..2486a723541fd9063b3446fee0a099bcb7b8c70e 100644
--- a/frontend/src/metabase/meta/types/Visualization.js
+++ b/frontend/src/metabase/meta/types/Visualization.js
@@ -53,7 +53,7 @@ export type ClickActionProps = {
}
export type ClickActionPopoverProps = {
- onChangeCardAndRun: (card: ?Card) => void,
+ onChangeCardAndRun: (Object) => void,
onClose: () => void,
}
@@ -81,7 +81,7 @@ export type VisualizationProps = {
onHoverChange: (?HoverObject) => void,
onVisualizationClick: (?ClickObject) => void,
visualizationIsClickable: (?ClickObject) => boolean,
- onChangeCardAndRun: (card: Card) => void,
+ onChangeCardAndRun: (Object) => void,
onUpdateVisualizationSettings: ({ [key: string]: any }) => void
}
diff --git a/frontend/src/metabase/qb/components/actions/PivotByAction.jsx b/frontend/src/metabase/qb/components/actions/PivotByAction.jsx
index a26f75a88c74bcb73115d693338025dbe63f935f..c52cddaddc5e101b9b02e9c1859d496d4812c5e7 100644
--- a/frontend/src/metabase/qb/components/actions/PivotByAction.jsx
+++ b/frontend/src/metabase/qb/components/actions/PivotByAction.jsx
@@ -88,9 +88,14 @@ export default (name: string, icon: string, fieldFilter: FieldFilter) =>
fieldOptions={fieldOptions}
customFieldOptions={customFieldOptions}
onCommitBreakout={breakout => {
- onChangeCardAndRun(
- pivot(card, breakout, tableMetadata, dimensions)
- );
+ onChangeCardAndRun({
+ nextCard: pivot(
+ card,
+ breakout,
+ tableMetadata,
+ dimensions
+ )
+ });
}}
onClose={onClose}
/>
diff --git a/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx b/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx
index 851400885e1aa61b750b3a204911d6c1c8afa231..94242389f2afeee09121e3b649f4823841bf8c33 100644
--- a/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx
+++ b/frontend/src/metabase/qb/components/actions/SummarizeBySegmentMetricAction.jsx
@@ -34,9 +34,13 @@ export default ({ card, tableMetadata }: ClickActionProps): ClickAction[] => {
customFields={Query.getExpressions(query)}
availableAggregations={tableMetadata.aggregation_options}
onCommitAggregation={aggregation => {
- onChangeCardAndRun(
- summarize(card, aggregation, tableMetadata)
- );
+ onChangeCardAndRun({
+ nextCard: summarize(
+ card,
+ aggregation,
+ tableMetadata
+ )
+ });
onClose && onClose();
}}
/>
diff --git a/frontend/src/metabase/qb/lib/actions.js b/frontend/src/metabase/qb/lib/actions.js
index 7ea9a5ca859ca05dc568eac3bf34cd1d01ae5c85..2eea6601f716b8dba58a420fd84e9e0efc0d7323 100644
--- a/frontend/src/metabase/qb/lib/actions.js
+++ b/frontend/src/metabase/qb/lib/actions.js
@@ -22,6 +22,7 @@ export const toUnderlyingData = (card: CardObject): ?CardObject => {
const newCard = startNewCard("query");
newCard.dataset_query = card.dataset_query;
newCard.display = "table";
+ newCard.original_card_id = card.id;
return newCard;
};
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index b131d3cee9958201dab34ca3abb3ea5b262fd7c1..b02e64cbb780a4dca19a4294a40c84ebe63f35d2 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -29,6 +29,7 @@ import { MetabaseApi, CardApi, UserApi } from "metabase/services";
import { parse as urlParse } from "url";
import querystring from "querystring";
+import {getCardAfterVisualizationClick} from "metabase/visualizations/lib/utils";
export const SET_CURRENT_STATE = "metabase/qb/SET_CURRENT_STATE"; const setCurrentState = createAction(SET_CURRENT_STATE);
@@ -183,7 +184,7 @@ export const initializeQB = createThunkAction(INITIALIZE_QB, (location, params)
MetabaseAnalytics.trackEvent("QueryBuilder", "Query Loaded", card.dataset_query.type);
// if we have deserialized card from the url AND loaded a card by id then the user should be dropped into edit mode
- uiControls.isEditing = !!options.edit
+ uiControls.isEditing = !!options.edit;
// if this is the users first time loading a saved card on the QB then show them the newb modal
if (params.cardId && currentUser.is_qbnewb) {
@@ -191,9 +192,20 @@ export const initializeQB = createThunkAction(INITIALIZE_QB, (location, params)
MetabaseAnalytics.trackEvent("QueryBuilder", "Show Newb Modal");
}
+ if (card.archived) {
+ // use the error handler in App.jsx for showing "This question has been archived" message
+ dispatch(setErrorPage({
+ data: {
+ error_code: "archived"
+ },
+ context: "query-builder"
+ }));
+ card = null;
+ }
+
preserveParameters = true;
} catch(error) {
- console.warn(error)
+ console.warn(error);
card = null;
dispatch(setErrorPage(error));
}
@@ -480,9 +492,12 @@ export const reloadCard = createThunkAction(RELOAD_CARD, () => {
};
});
-// setCardAndRun
-// Used when navigating browser history, when drilling through in visualizations / action widget,
-// and when having the entity details view open and clicking its cells
+/**
+ * `setCardAndRun` is used when:
+ * - navigating browser history
+ * - clicking in the entity details view
+ * - `navigateToNewCardInsideQB` is being called (see below)
+ */
export const SET_CARD_AND_RUN = "metabase/qb/SET_CARD_AND_RUN";
export const setCardAndRun = createThunkAction(SET_CARD_AND_RUN, (nextCard, shouldUpdateUrl = true) => {
return async (dispatch, getState) => {
@@ -507,8 +522,31 @@ export const setCardAndRun = createThunkAction(SET_CARD_AND_RUN, (nextCard, shou
};
});
+/**
+ * User-triggered events that are handled with this action:
+ * - clicking a legend:
+ * * series legend (multi-aggregation, multi-breakout, multiple questions)
+ * - clicking the visualization itself
+ * * drill-through (single series, multi-aggregation, multi-breakout, multiple questions)
+ * * (not in 0.24.2 yet: drag on line/area/bar visualization)
+ * - clicking an action widget action
+ *
+ * All these events can be applied either for an unsaved question or a saved question.
+ */
+export const NAVIGATE_TO_NEW_CARD = "metabase/qb/NAVIGATE_TO_NEW_CARD";
+export const navigateToNewCardInsideQB = createThunkAction(NAVIGATE_TO_NEW_CARD, ({ nextCard, previousCard }) => {
+ return async (dispatch, getState) => {
+ const nextCardIsClean = _.isEqual(previousCard.dataset_query, nextCard.dataset_query) && previousCard.display === nextCard.display;
+
+ if (nextCardIsClean) {
+ // This is mainly a fallback for scenarios where a visualization legend is clicked inside QB
+ dispatch(setCardAndRun(await loadCard(nextCard.id)));
+ } else {
+ dispatch(setCardAndRun(getCardAfterVisualizationClick(nextCard, previousCard)));
+ }
+ }
+});
-// setDatasetQuery
export const SET_DATASET_QUERY = "metabase/qb/SET_DATASET_QUERY";
export const setDatasetQuery = createThunkAction(SET_DATASET_QUERY, (dataset_query, run = false) => {
return (dispatch, getState) => {
diff --git a/frontend/src/metabase/query_builder/components/ActionsWidget.jsx b/frontend/src/metabase/query_builder/components/ActionsWidget.jsx
index fba9d3ffc8d7440a54c32047303bcacab1fbeaa0..6e57f4eda6a21852562218ec6616b79ceb6f6a0b 100644
--- a/frontend/src/metabase/query_builder/components/ActionsWidget.jsx
+++ b/frontend/src/metabase/query_builder/components/ActionsWidget.jsx
@@ -12,7 +12,7 @@ import MetabaseAnalytics from "metabase/lib/analytics";
import cx from "classnames";
import _ from "underscore";
-import type { Card, UnsavedCard } from "metabase/meta/types/Card";
+import type { Card, UnsavedCard} from "metabase/meta/types/Card";
import type { QueryMode, ClickAction } from "metabase/meta/types/Visualization";
import type { TableMetadata } from "metabase/meta/types/Metadata";
@@ -21,7 +21,7 @@ type Props = {
mode: QueryMode,
card: Card,
tableMetadata: TableMetadata,
- setCardAndRun: (card: Card) => void
+ navigateToNewCardInsideQB: any => void
};
type State = {
@@ -78,18 +78,9 @@ export default class ActionsWidget extends Component {
});
};
- handleOnChangeCardAndRun(nextCard: UnsavedCard|Card) {
- const { card } = this.props;
-
- // Include the original card id if present for showing the lineage next to title
- const nextCardWithOriginalId = {
- ...nextCard,
- // $FlowFixMe
- original_card_id: card.id || card.original_card_id
- };
- if (nextCardWithOriginalId) {
- this.props.setCardAndRun(nextCardWithOriginalId);
- }
+ handleOnChangeCardAndRun = ({ nextCard }: { nextCard: Card|UnsavedCard}) => {
+ const { card: previousCard } = this.props;
+ this.props.navigateToNewCardInsideQB({ nextCard, previousCard });
}
handleActionClick = (index: number) => {
@@ -101,7 +92,7 @@ export default class ActionsWidget extends Component {
const nextCard = action.card();
if (nextCard) {
MetabaseAnalytics.trackEvent("Actions", "Executed Action", `${action.section||""}:${action.name||""}`);
- this.handleOnChangeCardAndRun(nextCard);
+ this.handleOnChangeCardAndRun({ nextCard });
}
this.close();
}
@@ -177,12 +168,12 @@ export default class ActionsWidget extends Component {
</div>
</div>
<PopoverComponent
- onChangeCardAndRun={(card) => {
- if (card) {
+ onChangeCardAndRun={({ nextCard }) => {
+ if (nextCard) {
if (selectedAction) {
MetabaseAnalytics.trackEvent("Actions", "Executed Action", `${selectedAction.section||""}:${selectedAction.name||""}`);
}
- this.handleOnChangeCardAndRun(card)
+ this.handleOnChangeCardAndRun({ nextCard })
}
}}
onClose={this.close}
diff --git a/frontend/src/metabase/query_builder/components/VisualizationResult.jsx b/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
index d959a936de3aa3bd747248fb0e3213abc5c18bb3..def5c8c63df9aadc09776c1812e99a8390de311c 100644
--- a/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
+++ b/frontend/src/metabase/query_builder/components/VisualizationResult.jsx
@@ -7,7 +7,7 @@ import VisualizationErrorMessage from './VisualizationErrorMessage';
import Visualization from "metabase/visualizations/components/Visualization.jsx";
import { datasetContainsNoResults } from "metabase/lib/dataset";
-const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, result, ...props}) => {
+const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, navigateToNewCardInsideQB, result, ...props}) => {
const noResults = datasetContainsNoResults(result.data);
if (isObjectDetail) {
@@ -34,7 +34,7 @@ const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, result,
};
return <Visualization
series={[{ card: vizCard, data: result.data }]}
- onChangeCardAndRun={props.setCardAndRun}
+ onChangeCardAndRun={navigateToNewCardInsideQB}
isEditing={true}
// Table:
{...props}
@@ -43,11 +43,11 @@ const VisualizationResult = ({card, isObjectDetail, lastRunDatasetQuery, result,
}
VisualizationResult.propTypes = {
- card: PropTypes.object.isRequired,
- isObjectDetail: PropTypes.bool.isRequired,
- lastRunDatasetQuery: PropTypes.object.isRequired,
- result: PropTypes.object.isRequired,
- setCardAndRun: PropTypes.func,
+ card: PropTypes.object.isRequired,
+ isObjectDetail: PropTypes.bool.isRequired,
+ lastRunDatasetQuery: PropTypes.object.isRequired,
+ result: PropTypes.object.isRequired,
+ navigateToNewCardInsideQB: PropTypes.func,
}
export default VisualizationResult;
diff --git a/frontend/src/metabase/query_builder/components/filters/OperatorSelector.jsx b/frontend/src/metabase/query_builder/components/filters/OperatorSelector.jsx
index 8c08585fde9c5a40c56a6ed37c8ae8dac1b1c942..de0242e100d42785acc3bcdd6190ef520769556c 100644
--- a/frontend/src/metabase/query_builder/components/filters/OperatorSelector.jsx
+++ b/frontend/src/metabase/query_builder/components/filters/OperatorSelector.jsx
@@ -1,13 +1,15 @@
/* @flow */
import React, { Component } from "react";
+import ReactDOM from "react-dom";
import PropTypes from "prop-types";
-
-import Icon from "metabase/components/Icon.jsx";
-
import cx from "classnames";
import _ from "underscore";
+import {forceRedraw} from "metabase/lib/dom";
+
+import Icon from "metabase/components/Icon.jsx";
+
import type { Operator, OperatorName } from "metabase/meta/types/Metadata"
type Props = {
@@ -39,6 +41,13 @@ export default class OperatorSelector extends Component {
onOperatorChange: PropTypes.func.isRequired
};
+ expandOperators = () => {
+ this.setState({ expanded: true }, () => {
+ // HACK: Address Safari rendering bug which causes https://github.com/metabase/metabase/issues/5075
+ forceRedraw(ReactDOM.findDOMNode(this));
+ });
+ };
+
render() {
let { operator, operators } = this.props;
let { expanded } = this.state;
@@ -65,7 +74,7 @@ export default class OperatorSelector extends Component {
</button>
)}
{ !expanded && expandedOperators.length > 0 ?
- <div className="text-grey-3 cursor-pointer" onClick={() => this.setState({ expanded: true })}>
+ <div className="text-grey-3 cursor-pointer" onClick={this.expandOperators}>
<Icon className="px1" name="chevrondown" size={14} />
More Options
</div>
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index fe202d7e3485d164e40b944d5904cca9355004c0..af49d2d8b0db302be3df69799a3018aaef8d2fb2 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -156,13 +156,15 @@ export const getIsRunnable = createSelector(
const getLastRunDatasetQuery = createSelector([getLastRunCard], (card) => card && card.dataset_query);
const getNextRunDatasetQuery = createSelector([getCard], (card) => card && card.dataset_query);
-const getLastRunParameters = createSelector([getQueryResult], (queryResult) => queryResult && queryResult.json_query.parameters || [])
-const getLastRunParameterValues = createSelector([getLastRunParameters], (parameters) => parameters.map(parameter => parameter.value))
-const getNextRunParameterValues = createSelector([getParameters], (parameters) => parameters.map(parameter => parameter.value))
+const getLastRunParameters = createSelector([getQueryResult], (queryResult) => queryResult && queryResult.json_query && queryResult.json_query.parameters || []);
+const getLastRunParameterValues = createSelector([getLastRunParameters], (parameters) => parameters.map(parameter => parameter.value));
+const getNextRunParameterValues = createSelector([getParameters], (parameters) =>
+ parameters.map(parameter => parameter.value).filter(p => p !== undefined)
+);
export const getIsResultDirty = createSelector(
[getLastRunDatasetQuery, getNextRunDatasetQuery, getLastRunParameterValues, getNextRunParameterValues],
(lastDatasetQuery, nextDatasetQuery, lastParameters, nextParameters) => {
return !Utils.equals(lastDatasetQuery, nextDatasetQuery) || !Utils.equals(lastParameters, nextParameters);
}
-)
+);
diff --git a/frontend/src/metabase/questions/components/CollectionActions.jsx b/frontend/src/metabase/questions/components/CollectionActions.jsx
index bbecfd2290d86eaa8822f00606d50474f39649b5..27ff146af85c162cd3507931510ff938b7872f27 100644
--- a/frontend/src/metabase/questions/components/CollectionActions.jsx
+++ b/frontend/src/metabase/questions/components/CollectionActions.jsx
@@ -1,11 +1,11 @@
import React from "react";
const CollectionActions = ({ children }) =>
- <div onClick={(e) => { e.stopPropagation(); e.preventDefault() }}>
+ <div className="flex align-center" onClick={(e) => { e.stopPropagation(); e.preventDefault() }}>
{React.Children.map(children, (child, index) =>
- <span key={index} className="cursor-pointer text-brand-hover mx1">
+ <div key={index} className="cursor-pointer text-brand-hover mx1">
{child}
- </span>
+ </div>
)}
</div>
diff --git a/frontend/src/metabase/questions/containers/ArchiveCollectionWidget.jsx b/frontend/src/metabase/questions/containers/ArchiveCollectionWidget.jsx
index 3fc8b913ec6a795af113601f8e55110d7fb76786..c988729ce759c81ccecbdc23e97fc7ee32af44ca 100644
--- a/frontend/src/metabase/questions/containers/ArchiveCollectionWidget.jsx
+++ b/frontend/src/metabase/questions/containers/ArchiveCollectionWidget.jsx
@@ -43,7 +43,7 @@ export default class ArchiveCollectionWidget extends Component {
ref="modal"
triggerElement={
<Tooltip tooltip="Archive collection">
- <Icon name="archive" />
+ <Icon size={18} name="archive" />
</Tooltip>
}
title="Archive this collection?"
diff --git a/frontend/src/metabase/questions/containers/CollectionPage.jsx b/frontend/src/metabase/questions/containers/CollectionPage.jsx
index 49822508801a6e045b1436d90cb7b8b7aa3623fd..a90a2e56b37da7546510f01931d46f172c5e020d 100644
--- a/frontend/src/metabase/questions/containers/CollectionPage.jsx
+++ b/frontend/src/metabase/questions/containers/CollectionPage.jsx
@@ -50,8 +50,8 @@ export default class CollectionPage extends Component {
<div className="ml-auto">
<CollectionActions>
{ canEdit && <ArchiveCollectionWidget collectionId={this.props.collection.id} onArchived={this.props.goToQuestions}/> }
- { canEdit && <Icon name="pencil" tooltip="Edit collection" onClick={() => this.props.editCollection(this.props.collection.id)} /> }
- { canEdit && <Icon name="lock" tooltip="Set permissions" onClick={() => this.props.editPermissions(this.props.collection.id)} /> }
+ { canEdit && <Icon size={18} name="pencil" tooltip="Edit collection" onClick={() => this.props.editCollection(this.props.collection.id)} /> }
+ { canEdit && <Icon size={18} name="lock" tooltip="Set permissions" onClick={() => this.props.editPermissions(this.props.collection.id)} /> }
</CollectionActions>
</div>
</div>
diff --git a/frontend/src/metabase/questions/containers/EntityList.jsx b/frontend/src/metabase/questions/containers/EntityList.jsx
index 3e42b9c9db4ae2d586fbbdab95df3461f41b7717..3ee2b5a9bfc341f48f341ce0943285d530894b83 100644
--- a/frontend/src/metabase/questions/containers/EntityList.jsx
+++ b/frontend/src/metabase/questions/containers/EntityList.jsx
@@ -175,11 +175,13 @@ export default class EntityList extends Component {
const section = this.getSection();
+
const hasEntitiesInPlainState = entityIds.length > 0 || section.section !== "all";
const showActionHeader = (editable && selectedCount > 0);
const showSearchHeader = (hasEntitiesInPlainState && showSearchWidget);
const showEntityFilterWidget = onChangeSection;
+
return (
<div style={style}>
{ (showActionHeader || showSearchHeader || showEntityFilterWidget) &&
diff --git a/frontend/src/metabase/questions/containers/QuestionIndex.jsx b/frontend/src/metabase/questions/containers/QuestionIndex.jsx
index e0b95acbc50cd9ce174d5086c5c9b789af5a5d2f..4c07ec137db1a3d36187cbfe96177704b923fd8f 100644
--- a/frontend/src/metabase/questions/containers/QuestionIndex.jsx
+++ b/frontend/src/metabase/questions/containers/QuestionIndex.jsx
@@ -1,12 +1,12 @@
import React, { Component } from "react";
import { connect } from "react-redux";
import { Link } from "react-router";
-import Collapse from "react-collapse";
+import cx from "classnames";
import Icon from "metabase/components/Icon";
import Button from "metabase/components/Button";
-import DisclosureTriangle from "metabase/components/DisclosureTriangle";
import TitleAndDescription from "metabase/components/TitleAndDescription";
+
import ExpandingSearchField from "../components/ExpandingSearchField";
import CollectionActions from "../components/CollectionActions";
@@ -16,114 +16,140 @@ import EntityList from "./EntityList";
import { search } from "../questions";
import { loadCollections } from "../collections";
-import { getAllCollections, getAllEntities } from "../selectors";
+import { getLoadingInitialEntities, getAllCollections, getAllEntities } from "../selectors";
import { getUserIsAdmin } from "metabase/selectors/user";
import { replace, push } from "react-router-redux";
+import EmptyState from "metabase/components/EmptyState";
+import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
+
+export const CollectionEmptyState = () =>
+ <div className="flex align-center p2 mt4 bordered border-med border-brand rounded bg-grey-0 text-brand">
+ <Icon name="collection" size={32} className="mr2"/>
+ <div className="flex-full">
+ <h3>Create collections for your saved questions</h3>
+ <div className="mt1">
+ Collections help you organize your questions and allow you to decide who gets to see what.
+ {" "}
+ <a href="http://www.metabase.com/docs/latest/administration-guide/06-collections.html" target="_blank">
+ Learn more
+ </a>
+ </div>
+ </div>
+ <Link to="/collections/create">
+ <Button primary>Create a collection</Button>
+ </Link>
+ </div>;
+
+export const NoSavedQuestionsState = () =>
+ <div className="flex-full flex align-center justify-center mb4">
+ <EmptyState
+ message={<span>Explore your data, create charts or maps, and save what you find.</span>}
+ image="/app/img/questions_illustration"
+ action="Ask a question"
+ link="/question"
+ />
+ </div>;
+
+export const QuestionIndexHeader = ({questions, collections, isAdmin, onSearch}) => {
+ // Some replication of logic for making writing tests easier
+ const hasCollections = collections && collections.length > 0;
+ const hasQuestionsWithoutCollection = questions && questions.length > 0;
+
+ const showSearch = hasCollections || hasQuestionsWithoutCollection;
+ const showSetPermissionsLink = isAdmin && hasCollections;
+
+ return (<div className="flex align-center pt4 pb2">
+ <TitleAndDescription title={ hasCollections ? "Collections of Questions" : "Saved Questions" }/>
+
+ <div className="flex align-center ml-auto">
+ { showSearch &&
+ <ExpandingSearchField className="mr2" onSearch={onSearch}/>
+ }
+
+ <CollectionActions>
+ { showSetPermissionsLink &&
+ <Link to="/collections/permissions">
+ <Icon size={18} name="lock" tooltip="Set permissions for collections"/>
+ </Link>
+ }
+ <Link to="/questions/archive">
+ <Icon size={20} name="viewArchive" tooltip="View the archive"/>
+ </Link>
+ </CollectionActions>
+ </div>
+ </div>);
+};
const mapStateToProps = (state, props) => ({
+ loading: getLoadingInitialEntities(state, props),
questions: getAllEntities(state, props),
collections: getAllCollections(state, props),
- isAdmin: getUserIsAdmin(state, props),
-})
+ isAdmin: getUserIsAdmin(state, props)
+});
const mapDispatchToProps = ({
search,
loadCollections,
replace,
push,
-})
-
-@connect(mapStateToProps, mapDispatchToProps)
-export default class QuestionIndex extends Component {
- constructor (props) {
- super(props);
- this.state = {
- questionsExpanded: true
- }
- }
- componentWillMount () {
+});
+
+/* connect() is in the end of this file because of the plain QuestionIndex component is used in Jest tests */
+export class QuestionIndex extends Component {
+ componentWillMount() {
this.props.loadCollections();
}
render () {
- const { questions, collections, replace, push, location, isAdmin } = this.props;
- const { questionsExpanded } = this.state;
+ const { loading, questions, collections, replace, push, location, isAdmin } = this.props;
+
const hasCollections = collections && collections.length > 0;
- const hasQuestions = questions && questions.length > 0;
- const showCollections = isAdmin || hasCollections;
- const showQuestions = hasQuestions || !showCollections || location.query.f != null;
+ const hasQuestionsWithoutCollection = questions && questions.length > 0;
+
+ const showNoCollectionsState = !loading && isAdmin && !hasCollections;
+ const showNoSavedQuestionsState = !loading && !hasCollections && !hasQuestionsWithoutCollection;
+
+ const hasEntityListSectionQuery = !!(location.query && location.query.f);
+ const showEntityList = hasQuestionsWithoutCollection || hasEntityListSectionQuery;
+ const showEverythingElseTitle = showEntityList && hasCollections;
+
return (
- <div className="relative mx4">
- <div className="flex align-center pt4 pb2">
- <TitleAndDescription title={ showCollections ? "Collections of Questions" : "Saved Questions" } />
- <div className="flex align-center ml-auto">
- <ExpandingSearchField className="mr2" onSearch={this.props.search} />
-
- <CollectionActions>
- { isAdmin && hasCollections &&
- <Link to="/collections/permissions">
- <Icon name="lock" tooltip="Set permissions for collections" />
- </Link>
- }
- <Link to="/questions/archive">
- <Icon name="viewArchive" tooltip="View the archive" />
- </Link>
- </CollectionActions>
- </div>
- </div>
- { showCollections &&
- <div>
- { collections.length > 0 ?
- <CollectionButtons collections={collections} isAdmin={isAdmin} push={push} />
- :
- <CollectionEmptyState />
- }
- </div>
- }
- {/* only show title if we're showing the questions AND collections, otherwise title goes in the main header */}
- { showQuestions && showCollections &&
- <div
- className="inline-block mt2 mb2 cursor-pointer text-brand-hover"
- onClick={() => this.setState({ questionsExpanded: !questionsExpanded })}
- >
- <div className="flex align-center">
- <DisclosureTriangle open={questionsExpanded} />
- <h2>Everything Else</h2>
- </div>
- </div>
- }
- <Collapse isOpened={showQuestions && (questionsExpanded || !showCollections)} keepCollapsedContent={true}>
+ <div className={cx("relative px4", {"full-height flex flex-column bg-slate-extra-light": showNoSavedQuestionsState})}>
+ {/* Use loading wrapper only for displaying the loading indicator as EntityList component should always be in DOM */}
+ { loading && <LoadingAndErrorWrapper loading={true} noBackground /> }
+
+ { showNoCollectionsState && <CollectionEmptyState /> }
+
+ { !loading && <QuestionIndexHeader
+ questions={questions}
+ collections={collections}
+ isAdmin={isAdmin}
+ onSearch={this.props.search}
+ /> }
+
+ { hasCollections && <CollectionButtons collections={collections} isAdmin={isAdmin} push={push} /> }
+
+ { showNoSavedQuestionsState && <NoSavedQuestionsState /> }
+
+ { showEverythingElseTitle && <h2 className="mt2 mb2">Everything Else</h2> }
+
+ <div className={cx({ "hide": !showEntityList })}>
+ {/* EntityList loads `questions` according to the query specified in the url query string */}
<EntityList
entityType="cards"
- entityQuery={{ f: "all", collection: "", ...location.query }}
+ entityQuery={{f: "all", collection: "", ...location.query}}
// use replace when changing sections so back button still takes you back to collections page
onChangeSection={(section) => replace({
...location,
- query: { ...location.query, f: section }
+ query: {...location.query, f: section}
})}
- defaultEmptyState="Questions that aren’t in a collection will be shown here"
/>
- </Collapse>
+ </div>
</div>
)
}
}
-const CollectionEmptyState = () =>
- <div className="flex align-center p2 bordered border-med border-brand rounded bg-grey-0 text-brand">
- <Icon name="collection" size={32} className="mr2"/>
- <div className="flex-full">
- <h3>Create collections for your saved questions</h3>
- <div className="mt1">
- Collections help you organize your questions and allow you to decide who gets to see what.
- {" "}
- <a href="http://www.metabase.com/docs/latest/administration-guide/06-collections.html" target="_blank">
- Learn more
- </a>
- </div>
- </div>
- <Link to="/collections/create">
- <Button primary>Create a collection</Button>
- </Link>
- </div>
+export default connect(mapStateToProps, mapDispatchToProps)(QuestionIndex);
+
diff --git a/frontend/src/metabase/questions/containers/QuestionIndex.spec.js b/frontend/src/metabase/questions/containers/QuestionIndex.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..179d516e11d9cd9b813dc9ca5000998c056c1ffb
--- /dev/null
+++ b/frontend/src/metabase/questions/containers/QuestionIndex.spec.js
@@ -0,0 +1,140 @@
+import React from 'react'
+import {shallow, mount} from 'enzyme'
+
+import {
+ QuestionIndex,
+ CollectionEmptyState,
+ NoSavedQuestionsState,
+ QuestionIndexHeader
+} from './QuestionIndex';
+
+const someQuestions = [{}, {}, {}];
+const someCollections = [{}, {}];
+
+const dummyFunction = () => {};
+
+const getQuestionIndex = ({ questions, collections, isAdmin }) =>
+ <QuestionIndex
+ questions={questions}
+ collections={collections}
+ isAdmin={isAdmin}
+ replace={dummyFunction}
+ push={dummyFunction}
+ location={dummyFunction}
+ search={dummyFunction}
+ loadCollections={dummyFunction}
+ />;
+
+describe('QuestionIndex', () => {
+ describe('info box about collections', () => {
+ it("should be shown to admins if no collections", () => {
+ const component = shallow(getQuestionIndex({
+ questions: null,
+ collections: null,
+ isAdmin: true
+ }));
+
+ expect(component.find(CollectionEmptyState).length).toEqual(1);
+
+ const component2 = shallow(getQuestionIndex({
+ questions: someQuestions,
+ collections: null,
+ isAdmin: true
+ }));
+
+ expect(component2.find(CollectionEmptyState).length).toEqual(1);
+ });
+
+ it("should not be shown to admins if there are collections", () => {
+ const component = shallow(getQuestionIndex({
+ questions: null,
+ collections: someCollections,
+ isAdmin: false
+ }));
+
+ expect(component.find(CollectionEmptyState).length).toEqual(0);
+ });
+
+ it("should not be shown to non-admins", () => {
+ const component = shallow(getQuestionIndex({
+ questions: null,
+ collections: null,
+ isAdmin: false
+ }));
+
+ expect(component.find(CollectionEmptyState).length).toEqual(0);
+ })
+ });
+
+ describe('no saved questions state', () => {
+ const eitherAdminOrNot = [true, false];
+
+ it("should be shown if no collections or questions", () => {
+ eitherAdminOrNot.forEach(isAdmin => {
+ const component = shallow(getQuestionIndex({
+ questions: null,
+ collections: null,
+ isAdmin
+ }));
+
+ expect(component.find(NoSavedQuestionsState).length).toEqual(1);
+ })
+ });
+
+ it("should not be shown otherwise", () => {
+ eitherAdminOrNot.forEach(isAdmin => {
+ const component = shallow(getQuestionIndex({
+ questions: someQuestions,
+ collections: null,
+ isAdmin
+ }));
+
+ expect(component.find(NoSavedQuestionsState).length).toEqual(0);
+
+ const component2 = shallow(getQuestionIndex({
+ questions: null,
+ collections: someCollections,
+ isAdmin
+ }));
+
+ expect(component2.find(NoSavedQuestionsState).length).toEqual(0);
+ });
+ })
+ })
+
+ describe('collection actions', () => {
+ it("should let admins change permissions if collections exist", () => {
+ const component = mount(
+ <QuestionIndexHeader
+ collections={someCollections}
+ isAdmin={true}
+ />
+ );
+
+ // Why `find` does not work for matching React props: https://github.com/airbnb/enzyme/issues/582
+ expect(component.findWhere((node) => node.prop('to') === "/collections/permissions" ).length).toEqual(1);
+ });
+
+ it("should not let admins change permissions if no collections", () => {
+ const component = mount(
+ <QuestionIndexHeader
+ collections={null}
+ isAdmin={true}
+ />
+ );
+
+ expect(component.findWhere((node) => node.prop('to') === "/collections/permissions" ).length).toEqual(0);
+ });
+
+ it("should not let non-admins change permissions", () => {
+ const component = mount(
+ <QuestionIndexHeader
+ collections={someCollections}
+ isAdmin={false}
+ />
+ );
+
+ expect(component.findWhere((node) => node.prop('to') === "/collections/permissions" ).length).toEqual(0);
+ });
+ })
+});
\ No newline at end of file
diff --git a/frontend/src/metabase/questions/labels.js b/frontend/src/metabase/questions/labels.js
index 36a1aaa0f50a9cffde368d4768fe5239445707fb..776451a0018c00b39f6ac7b1ee3f3d0272692ed1 100644
--- a/frontend/src/metabase/questions/labels.js
+++ b/frontend/src/metabase/questions/labels.js
@@ -1,5 +1,5 @@
-import { createAction, createThunkAction } from "metabase/lib/redux";
+import {createAction, createThunkAction, mergeEntities} from "metabase/lib/redux";
import { reset } from 'redux-form';
import { normalize, schema } from "normalizr";
@@ -8,7 +8,6 @@ import MetabaseAnalytics from "metabase/lib/analytics";
const label = new schema.Entity('labels');
import { LabelApi } from "metabase/services";
-import { assoc, merge } from "icepick";
import _ from "underscore";
const LOAD_LABELS = 'metabase/labels/LOAD_LABELS';
@@ -76,19 +75,17 @@ const initialState = {
};
export default function(state = initialState, { type, payload, error }) {
- if (payload && payload.entities) {
- state = assoc(state, "entities", merge(state.entities, payload.entities));
- }
- if (payload && payload.message) {
- state = assoc(state, "message", payload.message);
- }
-
switch (type) {
case LOAD_LABELS:
if (error) {
return { ...state, error: payload };
} else {
- return { ...state, labelIds: payload.result, error: null };
+ return {
+ ...state,
+ entities: mergeEntities(state.entities, payload.entities),
+ labelIds: payload.result,
+ error: null
+ };
}
case SAVE_LABEL:
if (error || payload == null) {
diff --git a/frontend/src/metabase/questions/questions.js b/frontend/src/metabase/questions/questions.js
index 661fa2b8196d1f6a8df41814c78bc2cac2b0dd14..02d4924030df1b86d66269c3a158cad335a3a3b5 100644
--- a/frontend/src/metabase/questions/questions.js
+++ b/frontend/src/metabase/questions/questions.js
@@ -1,8 +1,8 @@
-import { createAction, createThunkAction, momentifyArraysTimestamps } from "metabase/lib/redux";
+import {createAction, createThunkAction, mergeEntities, momentifyArraysTimestamps} from "metabase/lib/redux";
import { normalize, schema } from "normalizr";
-import { getIn, assoc, assocIn, updateIn, merge, chain } from "icepick";
+import { getIn, assocIn, updateIn, chain } from "icepick";
import _ from "underscore";
import { inflect } from "metabase/lib/formatting";
@@ -217,6 +217,7 @@ const initialState = {
lastEntityType: null,
lastEntityQuery: null,
entities: {},
+ loadingInitialEntities: true,
itemsBySection: {},
searchText: "",
selectedIds: {},
@@ -224,10 +225,6 @@ const initialState = {
};
export default function(state = initialState, { type, payload, error }) {
- if (payload && payload.entities) {
- state = assoc(state, "entities", merge(state.entities, payload.entities));
- }
-
switch (type) {
case SET_SEARCH_TEXT:
return { ...state, searchText: payload };
@@ -240,6 +237,8 @@ export default function(state = initialState, { type, payload, error }) {
return assocIn(state, ["itemsBySection", payload.entityType, payload.entityQuery, "error"], payload.error);
} else {
return (chain(state)
+ .assoc("loadingInitialEntities", false)
+ .assoc("entities", mergeEntities(state.entities, payload.entities))
.assoc("lastEntityType", payload.entityType)
.assoc("lastEntityQuery", payload.entityQuery)
.assoc("selectedIds", {})
diff --git a/frontend/src/metabase/questions/selectors.js b/frontend/src/metabase/questions/selectors.js
index ced278541ee04724e49ee195254a10379d068584..3c9aae81994841fce3c999aac3cfd150c6f4c23a 100644
--- a/frontend/src/metabase/questions/selectors.js
+++ b/frontend/src/metabase/questions/selectors.js
@@ -7,17 +7,18 @@ import _ from "underscore";
import visualizations from "metabase/visualizations";
import {caseInsensitiveSearch} from "metabase/lib/string"
-export const getEntityType = (state, props) => props && props.entityType ? props.entityType : state.questions.lastEntityType;
-export const getEntityQuery = (state, props) => props && props.entityQuery ? JSON.stringify(props.entityQuery) : state.questions.lastEntityQuery;
+export const getEntityType = (state, props) => props && props.entityType ? props.entityType : state.questions.lastEntityType;
+export const getEntityQuery = (state, props) => props && props.entityQuery ? JSON.stringify(props.entityQuery) : state.questions.lastEntityQuery;
-export const getSection = (state, props) => props.entityQuery && JSON.stringify(props.entityQuery);
-export const getEntities = (state, props) => state.questions.entities
-export const getItemsBySection = (state, props) => state.questions.itemsBySection
+export const getSection = (state, props) => props.entityQuery && JSON.stringify(props.entityQuery);
+export const getLoadingInitialEntities = (state, props) => state.questions.loadingInitialEntities
+export const getEntities = (state, props) => state.questions.entities
+export const getItemsBySection = (state, props) => state.questions.itemsBySection
-export const getSearchText = (state, props) => state.questions.searchText;
-export const getSelectedIds = (state, props) => state.questions.selectedIds;
+export const getSearchText = (state, props) => state.questions.searchText;
+export const getSelectedIds = (state, props) => state.questions.selectedIds;
-export const getAllCollections = (state, props) => state.collections.collections;
+export const getAllCollections = (state, props) => state.collections.collections;
export const getWritableCollections = createSelector(
[getAllCollections],
diff --git a/frontend/src/metabase/redux/metadata.js b/frontend/src/metabase/redux/metadata.js
index 6530f3d7e43c379d0545461b1e631363fc52d42d..0e7de82de78ac3e077062285fda5af476d02e08a 100644
--- a/frontend/src/metabase/redux/metadata.js
+++ b/frontend/src/metabase/redux/metadata.js
@@ -6,6 +6,7 @@ import {
resourceListToMap,
fetchData,
updateData,
+ handleEntities
} from "metabase/lib/redux";
import { normalize } from "normalizr";
@@ -427,34 +428,6 @@ const revisions = handleActions({
[FETCH_REVISIONS]: { next: (state, { payload }) => payload }
}, {});
-// merge each entity from newEntities with existing entity, if any
-// this ensures partial entities don't overwrite existing entities with more properties
-function mergeEntities(entities, newEntities) {
- entities = { ...entities };
- for (const id in newEntities) {
- if (id in entities) {
- entities[id] = { ...entities[id], ...newEntities[id] };
- } else {
- entities[id] = newEntities[id];
- }
- }
- return entities;
-}
-
-// reducer that merges payload.entities
-function handleEntities(actionPattern, entityType, reducer) {
- return (state, action) => {
- if (state === undefined) {
- state = {};
- }
- let entities = getIn(action, ["payload", "entities", entityType])
- if (actionPattern.test(action.type) && entities) {
- state = mergeEntities(state, entities);
- }
- return reducer(state, action);
- }
-}
-
export default combineReducers({
metrics: handleEntities(/^metabase\/metadata\//, "metrics", metrics),
segments: handleEntities(/^metabase\/metadata\//, "segments", segments),
diff --git a/frontend/src/metabase/visualizations/components/ChartClickActions.jsx b/frontend/src/metabase/visualizations/components/ChartClickActions.jsx
index 73b71b471e66cbe37ea7b2319249b5c2835ea75d..d5d5ed9808703cf1ebead874abb0c88fb129466b 100644
--- a/frontend/src/metabase/visualizations/components/ChartClickActions.jsx
+++ b/frontend/src/metabase/visualizations/components/ChartClickActions.jsx
@@ -9,7 +9,6 @@ import Popover from "metabase/components/Popover";
import MetabaseAnalytics from "metabase/lib/analytics";
import type { ClickObject, ClickAction } from "metabase/meta/types/Visualization";
-import type { Card } from "metabase/meta/types/Card";
import _ from "underscore";
@@ -54,7 +53,7 @@ Object.values(SECTIONS).map((section, index) => {
type Props = {
clicked: ?ClickObject,
clickActions: ?ClickAction[],
- onChangeCardAndRun: (card: ?Card) => void,
+ onChangeCardAndRun: (Object) => void,
onClose: () => void
};
@@ -80,12 +79,12 @@ export default class ChartClickActions extends Component {
if (action.popover) {
this.setState({ popoverAction: action });
} else if (action.card) {
- const card = action.card();
+ const nextCard = action.card();
MetabaseAnalytics.trackEvent("Actions", "Executed Click Action", `${action.section||""}:${action.name||""}`);
- onChangeCardAndRun(card);
+ onChangeCardAndRun({ nextCard });
this.close();
}
- }
+ };
render() {
const { clicked, clickActions, onChangeCardAndRun } = this.props;
@@ -100,11 +99,11 @@ export default class ChartClickActions extends Component {
const PopoverContent = popoverAction.popover;
popover = (
<PopoverContent
- onChangeCardAndRun={(card) => {
+ onChangeCardAndRun={({ nextCard }) => {
if (popoverAction) {
MetabaseAnalytics.trackEvent("Action", "Executed Click Action", `${popoverAction.section||""}:${popoverAction.name||""}`);
}
- onChangeCardAndRun(card);
+ onChangeCardAndRun({ nextCard });
}}
onClose={() => {
MetabaseAnalytics.trackEvent("Action", "Dismissed Click Action Menu");
diff --git a/frontend/src/metabase/visualizations/components/LegendHeader.jsx b/frontend/src/metabase/visualizations/components/LegendHeader.jsx
index bec9352dd3b06ea6fec5dbb6a499bbfbc9e63cc2..bd7aa4221770d9a310cb5375dfb44daec8fbdaff 100644
--- a/frontend/src/metabase/visualizations/components/LegendHeader.jsx
+++ b/frontend/src/metabase/visualizations/components/LegendHeader.jsx
@@ -71,7 +71,7 @@ export default class LegendHeader extends Component {
onClick={s.clicked && visualizationIsClickable(s.clicked) ?
((e) => onVisualizationClick({ ...s.clicked, element: e.currentTarget }))
: onChangeCardAndRun ?
- ((e) => onChangeCardAndRun(s.card))
+ (() => onChangeCardAndRun({ nextCard: s.card, seriesIndex: index }))
: null }
/>,
onRemoveSeries && index > 0 &&
diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
index 759c375e64b3773fc659569ce4bbd3bba45646ea..a669b82448b11c1890a1961734a54d2f52dedf8d 100644
--- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
+++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx
@@ -191,11 +191,15 @@ export default class LineAreaBarChart extends Component {
// $FlowFixMe
let originalSeries = series._raw || series;
let cardIds = _.uniq(originalSeries.map(s => s.card.id))
+ const isComposedOfMultipleQuestions = cardIds.length > 1;
if (showTitle && settings["card.title"]) {
titleHeaderSeries = [{ card: {
name: settings["card.title"],
- id: cardIds.length === 1 ? cardIds[0] : null
+ ...(isComposedOfMultipleQuestions ? {} : {
+ id: cardIds[0],
+ dataset_query: originalSeries[0].card.dataset_query
+ }),
}}];
}
@@ -211,7 +215,9 @@ export default class LineAreaBarChart extends Component {
series={titleHeaderSeries}
description={settings["card.description"]}
actionButtons={actionButtons}
- onChangeCardAndRun={onChangeCardAndRun}
+ // If a dashboard card is composed of multiple questions, its custom card title
+ // shouldn't act as a link as it's ambiguous that which question it should open
+ onChangeCardAndRun={ isComposedOfMultipleQuestions ? null : onChangeCardAndRun }
/>
: null }
{ multiseriesHeaderSeries || (!titleHeaderSeries && actionButtons) ? // always show action buttons if we have them
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index d476580e73e54b212b40dffc89878b989dc7f095..141f8c8a2480954829a6dedb9252be1a450f301f 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -31,7 +31,7 @@ import cx from "classnames";
export const ERROR_MESSAGE_GENERIC = "There was a problem displaying this chart.";
export const ERROR_MESSAGE_PERMISSION = "Sorry, you don't have permission to see this card."
-import type { UnsavedCard, VisualizationSettings} from "metabase/meta/types/Card";
+import type { VisualizationSettings} from "metabase/meta/types/Card";
import type { HoverObject, ClickObject, Series } from "metabase/meta/types/Visualization";
import type { Metadata } from "metabase/meta/types/Metadata";
@@ -63,7 +63,7 @@ type Props = {
// for click actions
metadata: Metadata,
- onChangeCardAndRun: (card: UnsavedCard) => void,
+ onChangeCardAndRun: any => void,
// used for showing content in place of visualization, e.x. dashcard filter mapping
replacementContent: Element<any>,
@@ -221,27 +221,16 @@ export default class Visualization extends Component {
setTimeout(() => {
this.setState({ clicked });
}, 100);
- }
+ };
- handleOnChangeCardAndRun = (card: UnsavedCard) => {
+ // Add the underlying card of current series to onChangeCardAndRun if available
+ handleOnChangeCardAndRun = ({ nextCard, seriesIndex }) => {
const { series, clicked } = this.state;
- const index = (clicked && clicked.seriesIndex) || 0;
- const originalCard = series && series[index] && series[index].card;
+ const index = seriesIndex || (clicked && clicked.seriesIndex) || 0;
+ const previousCard = series && series[index] && series[index].card;
- let cardId = card.id || card.original_card_id;
- // if the supplied card doesn't have an id, get it from the original card
- if (cardId == null && originalCard) {
- // $FlowFixMe
- cardId = originalCard.id || originalCard.original_card_id;
- }
-
- this.props.onChangeCardAndRun({
- ...card,
- id: cardId,
- // $FlowFixMe
- original_card_id: cardId
- });
+ this.props.onChangeCardAndRun({ nextCard, previousCard });
}
onRender = ({ yAxisSplit, warnings = [] } = {}) => {
diff --git a/frontend/src/metabase/visualizations/lib/utils.js b/frontend/src/metabase/visualizations/lib/utils.js
index c776525135fcb9e7ac46f0d0841c7020bb82785c..5aa2aa826e3e930abb3b51c26fa759567b2889f3 100644
--- a/frontend/src/metabase/visualizations/lib/utils.js
+++ b/frontend/src/metabase/visualizations/lib/utils.js
@@ -249,3 +249,32 @@ function wrapMethod(object, name, method) {
}
}
}
+// TODO Atte Keinänen 5/30/17 Extract to metabase-lib card/question logic
+export const cardHasBecomeDirty = (nextCard, previousCard) =>
+ !_.isEqual(previousCard.dataset_query, nextCard.dataset_query) || previousCard.display !== nextCard.display;
+
+export function getCardAfterVisualizationClick(nextCard, previousCard) {
+ if (cardHasBecomeDirty(nextCard, previousCard)) {
+ const isMultiseriesQuestion = !nextCard.id;
+ const alreadyHadLineage = !!previousCard.original_card_id;
+
+ return {
+ ...nextCard,
+ // Original card id is needed for showing the "started from" lineage in dirty cards.
+ original_card_id: alreadyHadLineage
+ // Just recycle the original card id of previous card if there was one
+ ? previousCard.original_card_id
+ // A multi-aggregation or multi-breakout series legend / drill-through action
+ // should always use the id of underlying/previous card
+ : (isMultiseriesQuestion ? previousCard.id : nextCard.id)
+ }
+ } else {
+ // Even though the card is currently clean, we might still apply dashboard parameters to it,
+ // so add the original_card_id to ensure a correct behavior in that context
+ return {
+ ...nextCard,
+ original_card_id: nextCard.id
+ };
+ }
+}
+
diff --git a/frontend/src/metabase/visualizations/lib/utils.spec.js b/frontend/src/metabase/visualizations/lib/utils.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..4123abdd97ab53dc8c74e3356cea72ec1c1d4959
--- /dev/null
+++ b/frontend/src/metabase/visualizations/lib/utils.spec.js
@@ -0,0 +1,171 @@
+import {cardHasBecomeDirty, getCardAfterVisualizationClick} from "./utils";
+import _ from "underscore";
+
+// TODO Atte Keinänen 5/31/17 Rewrite tests using metabase-lib methods instead of a raw format
+
+const baseQuery = {
+ "database": 1,
+ "type": "query",
+ "query": {
+ "source_table": 2,
+ "aggregation": [
+ [
+ "count"
+ ]
+ ],
+ "breakout": [
+ [
+ "field-id",
+ 2
+ ]
+ ]
+ }
+};
+const derivedQuery = {
+ ...baseQuery,
+ "query": {
+ ...baseQuery.query,
+ "filter": [
+ "time-interval",
+ [
+ "field-id",
+ 1
+ ],
+ -7,
+ "day"
+ ]
+ }
+};
+
+const breakoutMultiseriesQuery = {
+ ...baseQuery,
+ "query": {
+ ...baseQuery.query,
+ "breakout": [
+ ...baseQuery.query.breakout,
+ [
+ "fk->",
+ 1,
+ 10
+ ]
+ ]
+ }
+};
+const derivedBreakoutMultiseriesQuery = {
+ ...breakoutMultiseriesQuery,
+ "query": {
+ ...breakoutMultiseriesQuery.query,
+ "filter": [
+ "time-interval",
+ [
+ "field-id",
+ 1
+ ],
+ -7,
+ "day"
+ ]
+ }
+};
+
+const savedCard = {
+ id: 3,
+ dataset_query: baseQuery,
+ display: "line"
+};
+const clonedSavedCard = {
+ id: 3,
+ dataset_query: _.clone(baseQuery),
+ display: "line"
+};
+const dirtyCardOnlyOriginalId = {
+ original_card_id: 7,
+ dataset_query: baseQuery,
+ display: "line"
+};
+
+const derivedCard = {
+ ...dirtyCardOnlyOriginalId,
+ dataset_query: derivedQuery
+};
+const derivedCardModifiedId = {
+ ...savedCard,
+ dataset_query: derivedQuery
+};
+const derivedDirtyCard = {
+ ...dirtyCardOnlyOriginalId,
+ dataset_query: derivedQuery
+};
+
+const derivedCardWithDifferentDisplay = {
+ ...savedCard,
+ display: "table"
+};
+
+const savedMultiseriesCard = {
+ ...savedCard,
+ dataset_query: breakoutMultiseriesQuery
+};
+const derivedMultiseriesCard = {
+ // id is not present when drilling through series / multiseries
+ dataset_query: derivedBreakoutMultiseriesQuery,
+ display: savedCard.display
+};
+const newCard = {
+ dataset_query: baseQuery,
+ display: "line"
+};
+const modifiedNewCard = {
+ dataset_query: derivedQuery,
+ display: "line"
+};
+
+describe("metabase/visualization/lib/utils", () => {
+ describe("cardHasBecomeDirty", () => {
+ it("should consider cards with different display types dirty", () => {
+ // mostly for action widget actions that only change the display type
+ expect(cardHasBecomeDirty(derivedCardWithDifferentDisplay, savedCard)).toEqual(true);
+ });
+
+ it("should consider cards with different data data dirty", () => {
+ expect(cardHasBecomeDirty(derivedCard, savedCard)).toEqual(true);
+ });
+
+ it("should consider cards with same display type and data clean", () => {
+ // i.e. the card is practically the same as original card
+ expect(cardHasBecomeDirty(clonedSavedCard, savedCard)).toEqual(false);
+ });
+ });
+
+ describe("getCardAfterVisualizationClick", () => {
+ it("should use the id of a previous card in case of a multi-breakout visualization", () => {
+ expect(getCardAfterVisualizationClick(derivedMultiseriesCard, savedMultiseriesCard))
+ .toMatchObject({original_card_id: savedMultiseriesCard.id})
+ });
+
+ // TODO: Atte Keinänen 5/31/17 This scenario is a little fuzzy at the moment as there have been
+ // some specific corner cases where the id in previousCard is wrong/missing
+ // We should validate that previousCard always has an id as it should
+ it("if the new card contains the id it's more reliable to use it for initializing lineage", () => {
+ expect(getCardAfterVisualizationClick(derivedCardModifiedId, savedCard))
+ .toMatchObject({original_card_id: derivedCardModifiedId.id})
+ });
+
+ it("should be able to continue the lineage even if the previous question was dirty already", () => {
+ expect(getCardAfterVisualizationClick(derivedDirtyCard, dirtyCardOnlyOriginalId))
+ .toMatchObject({original_card_id: dirtyCardOnlyOriginalId.original_card_id})
+ });
+
+ it("should just pass the new question if the previous question was new", () => {
+ expect(getCardAfterVisualizationClick(modifiedNewCard, newCard))
+ .toMatchObject(modifiedNewCard)
+ });
+
+ it("should populate original_card_id even if the question isn't modified", () => {
+ // This is the hack to interoperate with questionUrlWithParameters when
+ // dashboard parameters are applied to a dashcards
+ expect(getCardAfterVisualizationClick(clonedSavedCard, savedCard))
+ .toMatchObject({original_card_id: savedCard.id})
+ });
+ })
+});
+
diff --git a/frontend/src/metabase/visualizations/visualizations/Scalar.jsx b/frontend/src/metabase/visualizations/visualizations/Scalar.jsx
index bc0a4c79f2208ffc94905e6e44477cc3688c93e4..9608011fb23640ceefe4dbb0494bb6eaeaab9448 100644
--- a/frontend/src/metabase/visualizations/visualizations/Scalar.jsx
+++ b/frontend/src/metabase/visualizations/visualizations/Scalar.jsx
@@ -196,7 +196,7 @@ export default class Scalar extends Component {
<div className={styles.Title + " flex align-center relative"}>
<Ellipsified tooltip={card.name}>
<span
- onClick={onChangeCardAndRun && (() => onChangeCardAndRun(card))}
+ onClick={onChangeCardAndRun && (() => onChangeCardAndRun({ nextCard: card }))}
className={cx("fullscreen-normal-text fullscreen-night-text", {
"cursor-pointer": !!onChangeCardAndRun
})}
diff --git a/resources/frontend_client/app/img/questions_illustration.png b/resources/frontend_client/app/img/questions_illustration.png
new file mode 100644
index 0000000000000000000000000000000000000000..22f75051960c64b575aa00cd794e199988914740
Binary files /dev/null and b/resources/frontend_client/app/img/questions_illustration.png differ
diff --git a/resources/frontend_client/app/img/questions_illustration@2x.png b/resources/frontend_client/app/img/questions_illustration@2x.png
new file mode 100644
index 0000000000000000000000000000000000000000..22f75051960c64b575aa00cd794e199988914740
Binary files /dev/null and b/resources/frontend_client/app/img/questions_illustration@2x.png differ
diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj
index 4b8514c881880868c17a05d7c98940a6083f1f18..098ba60567ddd645eebc063b5c9f4d3c97695e04 100644
--- a/src/metabase/api/common.clj
+++ b/src/metabase/api/common.clj
@@ -40,6 +40,8 @@
"Assertion mechanism for use inside API functions.
Checks that TEST is true, or throws an `ExceptionInfo` with STATUS-CODE and MESSAGE.
+ MESSAGE can be either a plain string error message, or a map including the key `:message` and any additional details, such as an `:error_code`.
+
This exception is automatically caught in the body of `defendpoint` functions, and the appropriate HTTP response is generated.
`check` can be called with the form
@@ -59,7 +61,9 @@
[code-or-code-message-pair rest-args]
[[code-or-code-message-pair (first rest-args)] (rest rest-args)])]
(when-not tst
- (throw (ex-info message {:status-code code})))
+ (throw (if (map? message)
+ (ex-info (:message message) (assoc message :status-code code))
+ (ex-info message {:status-code code}))))
(if (empty? rest-args) tst
(recur (first rest-args) (second rest-args) (drop 2 rest-args))))))
@@ -313,8 +317,9 @@
[400 "Embedding is not enabled."]))
(defn check-not-archived
- "Check that the OBJECT is not `:archived`, or throw a `404`. Returns OBJECT as-is if check passes."
+ "Check that the OBJECT exists and is not `:archived`, or throw a `404`. Returns OBJECT as-is if check passes."
[object]
(u/prog1 object
+ (check-404 object)
(check (not (:archived object))
- [404 "The object has been archived."])))
+ [404 {:message "The object has been archived.", :error_code "archived"}])))
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index ce3aea62759dca69bcff184f877ce5c2723470ac..a902540a5dac7515be02d5ea236d7503cab49dd8 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -680,7 +680,7 @@
;; Test that we *cannot* share a Card if the Card has been archived
(expect
- "The object has been archived."
+ {:message "The object has been archived.", :error_code "archived"}
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card {:archived true}]
((user->client :crowberto) :post 404 (format "card/%d/public_link" (u/get-id card))))))