diff --git a/frontend/src/metabase-types/api/data-app.ts b/frontend/src/metabase-types/api/data-app.ts
index f0428feb2650c2beb0b4bd15777c24d0779ff040..24e816ff1aadf35fd547a4c7a388af773f3d8e01 100644
--- a/frontend/src/metabase-types/api/data-app.ts
+++ b/frontend/src/metabase-types/api/data-app.ts
@@ -47,6 +47,7 @@ export interface ActionDashboardCard
extends Omit<BaseDashboardOrderedCard, "parameter_mappings"> {
action_id: number | null;
action?: WritebackAction;
+ card_id?: number; // model card id for the associated action
parameter_mappings?: ActionParametersMapping[] | null;
visualization_settings: {
diff --git a/frontend/src/metabase-types/api/parameters.ts b/frontend/src/metabase-types/api/parameters.ts
index 03fbfb97d921e44a75e2d9f72360fbb725555f7b..78e5eda47abc366ce911ebcfd8413e88ffe66c45 100644
--- a/frontend/src/metabase-types/api/parameters.ts
+++ b/frontend/src/metabase-types/api/parameters.ts
@@ -27,3 +27,5 @@ export type ParameterType =
| DateParameterType;
export type ParameterId = string;
+
+export type ActionParameterValue = string | number;
diff --git a/frontend/src/metabase-types/api/writeback.ts b/frontend/src/metabase-types/api/writeback.ts
index f5d82977cb6e28de8f0f18373fb8167414659e14..cda2f510edaf8150db42d1266970fad8f92b1d98 100644
--- a/frontend/src/metabase-types/api/writeback.ts
+++ b/frontend/src/metabase-types/api/writeback.ts
@@ -5,10 +5,12 @@ export interface WritebackParameter extends Parameter {
target: ParameterTarget;
}
-export type WritebackActionType = "http" | "query";
+export type WritebackActionType = "http" | "query" | "implicit";
export interface WritebackActionBase {
id: number;
+ model_id?: number;
+ slug?: string;
name: string;
description: string | null;
parameters: WritebackParameter[];
@@ -23,7 +25,7 @@ export type QueryActionCard = Card & {
};
export interface QueryAction {
- type: "query";
+ type: "query" | "implicit";
card: QueryActionCard;
card_id: number;
}
@@ -53,22 +55,10 @@ export type WritebackAction = WritebackActionBase & (QueryAction | HttpAction);
export type ParameterMappings = Record<ParameterId, ParameterTarget>;
-type ParameterForActionExecutionBase = {
- type: string;
- value: string | number;
+export type ParametersForActionExecution = {
+ [id: ParameterId]: string | number;
};
-export type ParameterMappedForActionExecution =
- ParameterForActionExecutionBase & {
- id: ParameterId;
- target: ParameterTarget;
- };
-
-export type ArbitraryParameterForActionExecution =
- ParameterForActionExecutionBase & {
- target: ParameterTarget;
- };
-
export interface ActionFormSubmitResult {
success: boolean;
message?: string;
@@ -76,5 +66,17 @@ export interface ActionFormSubmitResult {
}
export type OnSubmitActionForm = (
- parameters: ArbitraryParameterForActionExecution[],
+ parameters: ParametersForActionExecution,
) => Promise<ActionFormSubmitResult>;
+
+export interface ModelAction {
+ id: number;
+ action_id?: number; // empty for implicit actions
+ name?: string; // empty for implicit actions
+ card_id: number; // the card id of the model
+ entity_id: string;
+ requires_pk: boolean;
+ slug: string;
+ parameter_mappings?: ParameterMappings;
+ visualization_settings?: ActionFormSettings;
+}
diff --git a/frontend/src/metabase/components/form/FormWidget.jsx b/frontend/src/metabase/components/form/FormWidget.jsx
index 033c2490e484b152ddbfb380202ba9fe4db2cafa..17da1cc9f5dcc1b701ba3c72ab404e3ea69f8807 100644
--- a/frontend/src/metabase/components/form/FormWidget.jsx
+++ b/frontend/src/metabase/components/form/FormWidget.jsx
@@ -20,6 +20,7 @@ import FormCollectionWidget from "./widgets/FormCollectionWidget";
import FormSnippetCollectionWidget from "./widgets/FormSnippetCollectionWidget";
import FormHiddenWidget from "./widgets/FormHiddenWidget";
import FormTextFileWidget from "./widgets/FormTextFileWidget";
+import FormModelWidget from "./widgets/FormModelWidget";
const WIDGETS = {
info: FormInfoWidget,
@@ -39,6 +40,7 @@ const WIDGETS = {
snippetCollection: FormSnippetCollectionWidget,
hidden: FormHiddenWidget,
textFile: FormTextFileWidget,
+ model: FormModelWidget,
};
export function getWidgetComponent(formField) {
diff --git a/frontend/src/metabase/components/form/widgets/FormModelWidget.tsx b/frontend/src/metabase/components/form/widgets/FormModelWidget.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..624387454a9d7868c3bf880d675adb056c3221b3
--- /dev/null
+++ b/frontend/src/metabase/components/form/widgets/FormModelWidget.tsx
@@ -0,0 +1,21 @@
+import React from "react";
+
+import ModelPicker from "metabase/containers/ModelPicker";
+import ItemSelect from "metabase/containers/ItemSelect";
+
+import Question from "metabase/entities/questions";
+
+import type { Card } from "metabase-types/api";
+import type { FormField } from "metabase-types/forms";
+
+const ModelSelect = ItemSelect(
+ ModelPicker,
+ ({ id }: { id: Card["id"] }) => <Question.Name id={id} />,
+ "dataset",
+);
+
+function FormModelWidget({ field }: { field: FormField<string, Card["id"]> }) {
+ return <ModelSelect {...field} />;
+}
+
+export default FormModelWidget;
diff --git a/frontend/src/metabase/containers/ActionPicker/ActionOptionItem.styled.tsx b/frontend/src/metabase/containers/ActionPicker/ActionOptionItem.styled.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..3cd5fcebf450a5ab8605621c6eb0fafa3a5c1b73
--- /dev/null
+++ b/frontend/src/metabase/containers/ActionPicker/ActionOptionItem.styled.tsx
@@ -0,0 +1,44 @@
+import _ from "underscore";
+import styled from "@emotion/styled";
+
+import { color, lighten } from "metabase/lib/colors";
+import { space } from "metabase/styled-components/theme";
+import Icon from "metabase/components/Icon";
+
+interface ActionOptionProps {
+ isSelected?: boolean;
+ hasDescription?: boolean;
+}
+
+export const ActionOptionListItem = styled.div<ActionOptionProps>`
+ color: ${props =>
+ props.isSelected ? color("text-white") : color("text-normal")};
+ background-color: ${props =>
+ props.isSelected ? color("brand") : color("white")};
+ cursor: pointer;
+
+ display: flex;
+ align-items: ${props => (props.hasDescription ? "flex-start" : "center")};
+ gap: ${space(1)};
+
+ border: 1px solid ${color("border")};
+ border-radius: ${space(1)};
+
+ padding: ${space(2)};
+ margin: ${space(1)} ${space(0)};
+
+ &:hover {
+ background-color: ${lighten("brand", 0.1)};
+ color: ${color("text-white")};
+ }
+`;
+
+export const ActionOptionTitle = styled.div`
+ font-size: 0.875rem;
+ font-weight: bold;
+`;
+
+export const ActionOptionDescription = styled.div`
+ font-size: 0.875rem;
+ margin-top: ${space(1)};
+`;
diff --git a/frontend/src/metabase/containers/ActionPicker/ActionOptionItem.tsx b/frontend/src/metabase/containers/ActionPicker/ActionOptionItem.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..2d42a2b0718831379f09a2a9f3fe12444b4cb6d4
--- /dev/null
+++ b/frontend/src/metabase/containers/ActionPicker/ActionOptionItem.tsx
@@ -0,0 +1,38 @@
+import React from "react";
+
+import Icon from "metabase/components/Icon";
+import {
+ ActionOptionListItem,
+ ActionOptionTitle,
+ ActionOptionDescription,
+} from "./ActionOptionItem.styled";
+
+interface ActionOptionProps {
+ name: string;
+ description?: string | null;
+ isSelected: boolean;
+ onClick: () => void;
+}
+
+export default function ActionOptionItem({
+ name,
+ description,
+ isSelected,
+ onClick,
+}: ActionOptionProps) {
+ return (
+ <ActionOptionListItem
+ onClick={onClick}
+ isSelected={isSelected}
+ hasDescription={!!description}
+ >
+ <Icon name="insight" size={22} />
+ <div>
+ <ActionOptionTitle>{name}</ActionOptionTitle>
+ {!!description && (
+ <ActionOptionDescription>{description}</ActionOptionDescription>
+ )}
+ </div>
+ </ActionOptionListItem>
+ );
+}
diff --git a/frontend/src/metabase/containers/ActionPicker/ActionPicker.styled.tsx b/frontend/src/metabase/containers/ActionPicker/ActionPicker.styled.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..5cf4ba6d3e4e031223f7aaab65bd5b6778fc57c5
--- /dev/null
+++ b/frontend/src/metabase/containers/ActionPicker/ActionPicker.styled.tsx
@@ -0,0 +1,34 @@
+import _ from "underscore";
+import styled from "@emotion/styled";
+
+import { color } from "metabase/lib/colors";
+
+import { SidebarItem } from "metabase/dashboard/components/ClickBehaviorSidebar/SidebarItem";
+
+export const ActionSidebarItem = styled(SidebarItem.Selectable)<{
+ hasDescription?: boolean;
+}>`
+ align-items: ${props => (props.hasDescription ? "flex-start" : "center")};
+ margin-top: 2px;
+`;
+
+export const ActionSidebarItemIcon = styled(SidebarItem.Icon)<{
+ isSelected?: boolean;
+}>`
+ .Icon {
+ color: ${props =>
+ props.isSelected ? color("text-white") : color("brand")};
+ }
+`;
+
+export const ActionDescription = styled.span<{ isSelected?: boolean }>`
+ width: 95%;
+ margin-top: 2px;
+
+ color: ${props =>
+ props.isSelected ? color("text-white") : color("text-medium")};
+`;
+
+export const ClickMappingsContainer = styled.div`
+ margin-top: 1rem;
+`;
diff --git a/frontend/src/metabase/containers/ActionPicker/ActionPicker.tsx b/frontend/src/metabase/containers/ActionPicker/ActionPicker.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..a7c28796f4c277b60806d3acd929ac0eff787880
--- /dev/null
+++ b/frontend/src/metabase/containers/ActionPicker/ActionPicker.tsx
@@ -0,0 +1,86 @@
+import React, { useState } from "react";
+import { t } from "ttag";
+
+import Button from "metabase/core/components/Button";
+import EmptyState from "metabase/components/EmptyState";
+
+import Actions from "metabase/entities/actions";
+import type { WritebackAction } from "metabase-types/api";
+import type { State } from "metabase-types/store";
+
+import ModelPicker from "../ModelPicker";
+import ActionOptionItem from "./ActionOptionItem";
+
+export default function ActionPicker({
+ value,
+ onChange,
+}: {
+ value: WritebackAction | undefined;
+ onChange: (value: WritebackAction) => void;
+}) {
+ const [modelId, setModelId] = useState<number | undefined>(value?.model_id);
+
+ return (
+ <div className="scroll-y">
+ {!modelId ? (
+ <ModelPicker value={modelId} onChange={setModelId} />
+ ) : (
+ <>
+ <Button
+ icon="arrow_left"
+ borderless
+ onClick={() => setModelId(undefined)}
+ >
+ {t`Select Model`}
+ </Button>
+
+ <ConnectedModelActionPicker
+ modelId={modelId}
+ value={value}
+ onChange={onChange}
+ />
+ </>
+ )}
+ </div>
+ );
+}
+
+function ModelActionPicker({
+ value,
+ onChange,
+ actions,
+}: {
+ value: WritebackAction | undefined;
+ onChange: (newValue: WritebackAction) => void;
+ actions: WritebackAction[];
+}) {
+ if (!actions?.length) {
+ return (
+ <EmptyState
+ message={t`There are no actions for this model`}
+ action={t`Create new action`}
+ link={"/action/create"}
+ />
+ );
+ }
+
+ return (
+ <ul>
+ {actions?.map(action => (
+ <ActionOptionItem
+ name={action.name}
+ description={action.description}
+ isSelected={action.id === value?.id}
+ key={action.slug}
+ onClick={() => onChange(action)}
+ />
+ ))}
+ </ul>
+ );
+}
+
+const ConnectedModelActionPicker = Actions.loadList({
+ query: (state: State, props: { modelId?: number | null }) => ({
+ "model-id": props?.modelId,
+ }),
+})(ModelActionPicker);
diff --git a/frontend/src/metabase/containers/ActionPicker/index.ts b/frontend/src/metabase/containers/ActionPicker/index.ts
new file mode 100644
index 0000000000000000000000000000000000000000..36962a14f13c669494001e5065d385cec07a81e9
--- /dev/null
+++ b/frontend/src/metabase/containers/ActionPicker/index.ts
@@ -0,0 +1 @@
+export { default } from "./ActionPicker";
diff --git a/frontend/src/metabase/containers/ItemSelect.jsx b/frontend/src/metabase/containers/ItemSelect.jsx
index 3e26e2be2d4a7c3ef1eeac5e6e64d2597163ef72..5dc62573f1fc874484147dd1c5cfa96f6ee71945 100644
--- a/frontend/src/metabase/containers/ItemSelect.jsx
+++ b/frontend/src/metabase/containers/ItemSelect.jsx
@@ -10,6 +10,14 @@ import SelectButton from "metabase/core/components/SelectButton";
const MIN_POPOVER_WIDTH = 300;
+const typeNameMap = {
+ card: () => t`question`,
+ dataset: () => t`model`,
+ table: () => t`table`,
+ dashboard: () => t`dashboard`,
+ page: () => t`page`,
+};
+
export default (PickerComponent, NameComponent, type) =>
class ItemSelect extends React.Component {
state = {
@@ -28,7 +36,7 @@ export default (PickerComponent, NameComponent, type) =>
};
static defaultProps = {
- placeholder: t`Select a ${type}`,
+ placeholder: t`Select a ${typeNameMap[type]?.() ?? type}`,
inheritWidth: true,
};
diff --git a/frontend/src/metabase/containers/ModelPicker.jsx b/frontend/src/metabase/containers/ModelPicker.jsx
new file mode 100644
index 0000000000000000000000000000000000000000..322a4aae2bddabfaf89a0f8547b3f7c93fc179e4
--- /dev/null
+++ b/frontend/src/metabase/containers/ModelPicker.jsx
@@ -0,0 +1,20 @@
+import React from "react";
+import PropTypes from "prop-types";
+
+import ItemPicker from "./ItemPicker";
+
+const ModelPicker = ({ value, onChange, ...props }) => (
+ <ItemPicker
+ {...props}
+ value={value === undefined ? undefined : { model: "page", id: value }}
+ onChange={page => onChange(page ? page.id : undefined)}
+ models={["dataset"]}
+ />
+);
+
+ModelPicker.propTypes = {
+ value: PropTypes.number,
+ onChange: PropTypes.func.isRequired,
+};
+
+export default ModelPicker;
diff --git a/frontend/src/metabase/dashboard/actions/save.js b/frontend/src/metabase/dashboard/actions/save.js
index 7253cb395a75e464545eda9b88137e7206c77e5e..f176e62fa547519409a6e907df6eb87e2412924a 100644
--- a/frontend/src/metabase/dashboard/actions/save.js
+++ b/frontend/src/metabase/dashboard/actions/save.js
@@ -113,8 +113,8 @@ export const saveDashboardAndCards = createThunkAction(
const cards = updatedDashcards.map(
({
id,
- action_id,
card_id,
+ action,
row,
col,
size_x,
@@ -124,8 +124,8 @@ export const saveDashboardAndCards = createThunkAction(
visualization_settings,
}) => ({
id,
- action_id,
card_id,
+ action,
row,
col,
size_x,
@@ -142,6 +142,7 @@ export const saveDashboardAndCards = createThunkAction(
}) &&
// filter out mappings for deleted series
(!card_id ||
+ action ||
card_id === mapping.card_id ||
_.findWhere(series, { id: mapping.card_id })),
),
diff --git a/frontend/src/metabase/dashboard/actions/writeback.ts b/frontend/src/metabase/dashboard/actions/writeback.ts
index cd2382b0ce507ede01869be33543a5acceee9211..f58ec5e5fe30d231268aa17c51dae993e438730a 100644
--- a/frontend/src/metabase/dashboard/actions/writeback.ts
+++ b/frontend/src/metabase/dashboard/actions/writeback.ts
@@ -22,9 +22,10 @@ import type {
Dashboard,
DashboardOrderedCard,
ActionDashboardCard,
- ParameterMappedForActionExecution,
- ArbitraryParameterForActionExecution,
+ ParametersForActionExecution,
ActionFormSubmitResult,
+ WritebackAction,
+ ActionParametersMapping,
} from "metabase-types/api";
import type { Dispatch } from "metabase-types/store";
@@ -44,9 +45,16 @@ export const closeActionParametersModal = createAction(
CLOSE_ACTION_PARAMETERS_MODAL,
);
+interface DashboardAttributes {
+ card_id?: number | null;
+ action?: WritebackAction | null;
+ parameter_mappings?: ActionParametersMapping[] | null;
+ visualization_settings?: ActionDashboardCard["visualization_settings"];
+}
+
export function updateButtonActionMapping(
dashCardId: number,
- attributes: { action_id?: number | null; parameter_mappings?: any },
+ attributes: DashboardAttributes,
) {
return (dispatch: Dispatch) => {
dispatch(
@@ -234,8 +242,7 @@ export const deleteManyRowsFromDataApp = (
export type ExecuteRowActionPayload = {
dashboard: Dashboard;
dashcard: ActionDashboardCard;
- parameters: ParameterMappedForActionExecution[];
- extra_parameters: ArbitraryParameterForActionExecution[];
+ parameters: ParametersForActionExecution;
dispatch: Dispatch;
shouldToast?: boolean;
};
@@ -244,7 +251,6 @@ export const executeRowAction = async ({
dashboard,
dashcard,
parameters,
- extra_parameters,
dispatch,
shouldToast = true,
}: ExecuteRowActionPayload): Promise<ActionFormSubmitResult> => {
@@ -253,16 +259,21 @@ export const executeRowAction = async ({
const result = await ActionsApi.execute({
dashboardId: dashboard.id,
dashcardId: dashcard.id,
+ modelId: dashcard.card_id,
+ slug: dashcard.action?.slug,
parameters,
- extra_parameters,
});
- if (result["rows-affected"] > 0) {
- dispatch(reloadDashboardCards());
+ if (result["rows-affected"] > 0 || result["rows-updated"]?.[0] > 0) {
message = t`Successfully executed the action`;
+ } else if (result["created-row"]) {
+ message = t`Successfully saved`;
+ } else if (result["rows-deleted"]?.[0] > 0) {
+ message = t`Successfully deleted`;
} else {
message = t`Success! The action returned: ${JSON.stringify(result)}`;
}
+ dispatch(reloadDashboardCards());
if (shouldToast) {
dispatch(
addUndo({
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptionItem.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptionItem.tsx
deleted file mode 100644
index 8330c569857bd9af91c2ea8a2d9b723479ade5c0..0000000000000000000000000000000000000000
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptionItem.tsx
+++ /dev/null
@@ -1,38 +0,0 @@
-import React from "react";
-
-import { SidebarItem } from "../SidebarItem";
-import {
- ActionSidebarItem,
- ActionSidebarItemIcon,
- ActionDescription,
-} from "./ActionOptions.styled";
-
-interface ActionOptionProps {
- name: string;
- description?: string | null;
- isSelected: boolean;
- onClick: () => void;
-}
-
-function ActionOptionItem({
- name,
- description,
- isSelected,
- onClick,
-}: ActionOptionProps) {
- return (
- <ActionSidebarItem
- onClick={onClick}
- isSelected={isSelected}
- hasDescription={!!description}
- >
- <ActionSidebarItemIcon name="insight" isSelected={isSelected} />
- <div>
- <SidebarItem.Name>{name}</SidebarItem.Name>
- {description && <ActionDescription>{description}</ActionDescription>}
- </div>
- </ActionSidebarItem>
- );
-}
-
-export default ActionOptionItem;
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.styled.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.styled.tsx
index 98a1e87300b5ebe6e2cf9a6a23b284038aae464b..d8b033260e31c68e1c1a4f7a564d928ecb97d454 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.styled.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.styled.tsx
@@ -32,3 +32,10 @@ export const ActionDescription = styled.span<{ isSelected?: boolean }>`
export const ClickMappingsContainer = styled.div`
margin-top: 1rem;
`;
+
+export const ActionPickerWrapper = styled.div`
+ display: flex;
+ flex-direction: column;
+ max-height: calc(100vh - 30rem);
+ overflow-y: auto;
+`;
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.tsx
index d6bf5523dca1ab463430c961057e41e19979b8e7..1b28a5f9b4f7b9c356a40ecd04747fd3f5ff2333 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.tsx
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/ActionOptions.tsx
@@ -2,11 +2,11 @@ import React, { useCallback } from "react";
import { t } from "ttag";
import { connect } from "react-redux";
-import Actions from "metabase/entities/actions";
-
import { updateButtonActionMapping } from "metabase/dashboard/actions";
import { updateSettings } from "metabase/visualizations/lib/settings";
+import ActionPicker from "metabase/containers/ActionPicker";
+
import type {
ActionDashboardCard,
ActionParametersMapping,
@@ -18,8 +18,12 @@ import type { UiParameter } from "metabase/parameters/types";
import { Heading, SidebarContent } from "../ClickBehaviorSidebar.styled";
import ActionClickMappings from "./ActionClickMappings";
-import ActionOptionItem from "./ActionOptionItem";
-import { ClickMappingsContainer } from "./ActionOptions.styled";
+import {
+ ClickMappingsContainer,
+ ActionPickerWrapper,
+} from "./ActionOptions.styled";
+
+import { ensureParamsHaveNames } from "./utils";
interface ActionOptionsOwnProps {
dashcard: ActionDashboardCard;
@@ -30,10 +34,10 @@ interface ActionOptionsDispatchProps {
onUpdateButtonActionMapping: (
dashCardId: number,
settings: {
- action_id?: number | null;
- action?: WritebackAction;
- visualization_settings?: ActionDashboardCard["visualization_settings"];
+ card_id?: number | null;
+ action?: WritebackAction | null;
parameter_mappings?: ActionParametersMapping[] | null;
+ visualization_settings?: ActionDashboardCard["visualization_settings"];
},
) => void;
}
@@ -45,24 +49,22 @@ const mapDispatchToProps = {
};
function ActionOptions({
- actions,
dashcard,
parameters,
onUpdateButtonActionMapping,
-}: ActionOptionsProps & { actions: WritebackAction[] }) {
- const connectedActionId = dashcard.action_id;
-
- const selectedAction = actions.find(
- action => action.id === connectedActionId,
- );
+}: ActionOptionsProps) {
+ const selectedAction = dashcard.action;
const handleActionSelected = useCallback(
(action: WritebackAction) => {
onUpdateButtonActionMapping(dashcard.id, {
- action_id: action.id,
+ card_id: action.model_id,
action,
visualization_settings: updateSettings(
- { "button.label": action.name },
+ {
+ "button.label": action.name,
+ action_slug: action.slug, // :-( so hacky
+ },
dashcard.visualization_settings,
),
// Clean mappings from previous action
@@ -83,27 +85,23 @@ function ActionOptions({
);
return (
- <>
- {actions.map(action => (
- <ActionOptionItem
- key={action.id}
- name={action.name}
- description={action.description}
- isSelected={action.id === connectedActionId}
- onClick={() => handleActionSelected(action)}
- />
- ))}
- {selectedAction && (
+ <ActionPickerWrapper>
+ <ActionPicker value={selectedAction} onChange={handleActionSelected} />
+
+ {!!selectedAction && (
<ClickMappingsContainer>
<ActionClickMappings
- action={selectedAction}
+ action={{
+ ...selectedAction,
+ parameters: ensureParamsHaveNames(selectedAction.parameters),
+ }}
dashcard={dashcard}
parameters={parameters}
onChange={handleParameterMappingChange}
/>
</ClickMappingsContainer>
)}
- </>
+ </ActionPickerWrapper>
);
}
@@ -111,11 +109,11 @@ function ActionOptionsContainer(props: ActionOptionsProps) {
return (
<SidebarContent>
<Heading className="text-medium">{t`Pick an action`}</Heading>
- <Actions.ListLoader loadingAndErrorWrapper={false}>
- {({ actions = [] }: { actions: WritebackAction[] }) => (
- <ActionOptions {...props} actions={actions} />
- )}
- </Actions.ListLoader>
+ <ActionOptions
+ dashcard={props.dashcard}
+ parameters={props.parameters}
+ onUpdateButtonActionMapping={props.onUpdateButtonActionMapping}
+ />
</SidebarContent>
);
}
diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/utils.ts b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/utils.ts
index d37b03b5f9519e6fcd797602e3381d03f6dffae9..1f0660e19bf8f8180966f0759b39382e6f2a592e 100644
--- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/utils.ts
+++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ActionOptions/utils.ts
@@ -5,6 +5,7 @@ import type {
ActionParametersMapping,
ClickBehaviorParameterMapping,
WritebackAction,
+ WritebackParameter,
} from "metabase-types/api";
import type { UiParameter } from "metabase/parameters/types";
@@ -71,3 +72,12 @@ export function turnClickBehaviorParameterMappingsIntoDashCardMappings(
return parameter_mappings;
}
+
+export function ensureParamsHaveNames(
+ parameters: WritebackParameter[],
+): WritebackParameter[] {
+ return parameters.map(parameter => ({
+ ...parameter,
+ name: parameter.name ?? parameter.id,
+ }));
+}
diff --git a/frontend/src/metabase/entities/actions/actions.ts b/frontend/src/metabase/entities/actions/actions.ts
index 790610456ec99ae31755c50b6e0f8a4bd2952d50..584f31136be987ca5cd411cff59f1abb41a538eb 100644
--- a/frontend/src/metabase/entities/actions/actions.ts
+++ b/frontend/src/metabase/entities/actions/actions.ts
@@ -1,8 +1,13 @@
import { createEntity } from "metabase/lib/entities";
-import type { ActionFormSettings } from "metabase-types/api";
+import type {
+ ActionFormSettings,
+ ModelAction,
+ WritebackAction,
+} from "metabase-types/api";
+import type { Dispatch } from "metabase-types/store";
-import { CardApi } from "metabase/services";
+import { ActionsApi, CardApi, ModelActionsApi } from "metabase/services";
import {
removeOrphanSettings,
@@ -11,11 +16,12 @@ import {
setTemplateTagTypesFromFieldSettings,
} from "metabase/entities/actions/utils";
import type Question from "metabase-lib/lib/Question";
-import { saveForm } from "./forms";
+import { saveForm, updateForm } from "./forms";
type ActionParams = {
name: string;
description?: string;
+ model_id?: number;
collection_id?: number;
question: Question;
formSettings: ActionFormSettings;
@@ -56,16 +62,79 @@ const getAPIFn =
const createAction = getAPIFn(CardApi.create);
const updateAction = getAPIFn(CardApi.update);
+const associateAction = ({
+ model_id,
+ action_id,
+}: {
+ model_id: number;
+ action_id: number;
+}) =>
+ ModelActionsApi.connectActionToModel({
+ card_id: model_id,
+ action_id: action_id,
+ slug: `action_${action_id}`,
+ requires_pk: false,
+ });
+
+const defaultImplicitActionCreateOptions = {
+ insert: true,
+ update: true,
+ delete: true,
+};
+
+const enableImplicitActionsForModel =
+ async (modelId: number, options = defaultImplicitActionCreateOptions) =>
+ async (dispatch: Dispatch) => {
+ const methodsToCreate = Object.entries(options)
+ .filter(([, shouldCreate]) => !!shouldCreate)
+ .map(([method]) => method);
+
+ const apiCalls = methodsToCreate.map(method =>
+ ModelActionsApi.createImplicitAction({
+ card_id: modelId,
+ slug: method,
+ requires_pk: method !== "insert",
+ }),
+ );
+
+ await Promise.all(apiCalls);
+
+ dispatch({ type: Actions.actionTypes.INVALIDATE_LISTS_ACTION });
+ };
+
const Actions = createEntity({
name: "actions",
nameOne: "action",
path: "/api/action",
api: {
- create: createAction,
+ create: async ({ model_id, ...params }: ActionParams) => {
+ const card = await createAction(params);
+ if (card?.action_id && model_id) {
+ const association = await associateAction({
+ model_id,
+ action_id: card.action_id,
+ });
+ return { ...card, association };
+ }
+ return card;
+ },
update: updateAction,
+ list: async (params: any) => {
+ const actions = await ActionsApi.list(params);
+
+ return actions.map((action: ModelAction | WritebackAction) => ({
+ ...action,
+ id: action.id ?? `implicit-${action.slug}`,
+ name: action.name ?? action.slug,
+ }));
+ },
+ },
+ actions: {
+ enableImplicitActionsForModel,
},
forms: {
saveForm,
+ updateForm,
},
});
diff --git a/frontend/src/metabase/entities/actions/forms.ts b/frontend/src/metabase/entities/actions/forms.ts
index 358ba9b082c9aae2ba32d897092c843f376ee660..bab7fceb86db9deb7a73544c1695b13f624e6885 100644
--- a/frontend/src/metabase/entities/actions/forms.ts
+++ b/frontend/src/metabase/entities/actions/forms.ts
@@ -1,35 +1,41 @@
import { t } from "ttag";
+const getFormFields = (formAction: "create" | "update") => [
+ {
+ name: "name",
+ title: t`Name`,
+ placeholder: t`My new fantastic action`,
+ autoFocus: true,
+ validate: (name: string) =>
+ (!name && t`Name is required`) ||
+ (name && name.length > 100 && t`Name must be 100 characters or less`),
+ },
+ {
+ name: "description",
+ title: t`Description`,
+ type: "text",
+ placeholder: t`It's optional but oh, so helpful`,
+ normalize: (description: string) => description || null, // expected to be nil or non-empty string
+ },
+ {
+ name: "model_id",
+ title: t`Model it's saved in`,
+ type: formAction === "create" ? "model" : "hidden",
+ },
+ {
+ name: "question",
+ type: "hidden",
+ },
+ {
+ name: "formSettings",
+ type: "hidden",
+ },
+];
+
export const saveForm = {
- fields: [
- {
- name: "name",
- title: t`Name`,
- placeholder: t`My new fantastic action`,
- autoFocus: true,
- validate: (name: string) =>
- (!name && t`Name is required`) ||
- (name && name.length > 100 && t`Name must be 100 characters or less`),
- },
- {
- name: "description",
- title: t`Description`,
- type: "text",
- placeholder: t`It's optional but oh, so helpful`,
- normalize: (description: string) => description || null, // expected to be nil or non-empty string
- },
- {
- name: "collection_id",
- title: t`Collection it's saved in`,
- type: "collection",
- },
- {
- name: "question",
- type: "hidden",
- },
- {
- name: "formSettings",
- type: "hidden",
- },
- ],
+ fields: getFormFields("create"),
+};
+
+export const updateForm = {
+ fields: getFormFields("update"),
};
diff --git a/frontend/src/metabase/entities/actions/utils.ts b/frontend/src/metabase/entities/actions/utils.ts
index 3430744eef35ea630a7be92d5ed59b0277f4f00a..856ef75026647ce8fc8a49b09acb19dff7d23dca 100644
--- a/frontend/src/metabase/entities/actions/utils.ts
+++ b/frontend/src/metabase/entities/actions/utils.ts
@@ -4,6 +4,7 @@ import type {
FieldType,
InputType,
ParameterType,
+ ModelAction,
} from "metabase-types/api";
import { getDefaultFieldSettings } from "metabase/writeback/components/ActionCreator/FormCreator";
diff --git a/frontend/src/metabase/entities/actions/utils.unit.spec.ts b/frontend/src/metabase/entities/actions/utils.unit.spec.ts
index b1bfef64cac82ee8b5056d11b2fa56175f88e951..d734e60f438ec39b5fc3907fcb93d6a45799e078 100644
--- a/frontend/src/metabase/entities/actions/utils.unit.spec.ts
+++ b/frontend/src/metabase/entities/actions/utils.unit.spec.ts
@@ -6,6 +6,7 @@ import {
import { metadata } from "__support__/sample_database_fixture";
import type { Parameter as ParameterObject } from "metabase-types/types/Parameter";
import { NativeDatasetQuery } from "metabase-types/types/Card";
+import { ModelAction } from "metabase-types/api";
import Question from "metabase-lib/lib/Question";
import {
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.styled.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.styled.tsx
index 31e408302e31535594150984d43156775df8a424..16b7780700acd752434ffa4ec2e0d7c928bf9656 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.styled.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.styled.tsx
@@ -15,7 +15,7 @@ export const ActionListItem = styled(Link)`
width: 100%;
border-radius: 8px;
- padding: 1rem 0.5rem;
+ padding: 1rem;
${ActionTitle} {
margin-left: 1rem;
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx
index f07856d488fa72163f8ebde53d5f114f0e60fd91..3a2b6661441074ef37cb22098b75f6aba2bf19bb 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx
@@ -1,32 +1,93 @@
import React from "react";
+import { t } from "ttag";
+import { connect } from "react-redux";
+import _ from "underscore";
+import Button from "metabase/core/components/Button";
+import Link from "metabase/core/components/Link";
import Icon from "metabase/components/Icon";
import Actions from "metabase/entities/actions";
+import { humanize } from "metabase/lib/formatting";
+import { hasImplicitActions, isImplicitAction } from "metabase/writeback/utils";
-import type { WritebackQueryAction } from "metabase-types/api";
-import type Question from "metabase-lib/lib/Question";
+import type { WritebackAction } from "metabase-types/api";
+import type { State } from "metabase-types/store";
+import {
+ EmptyStateContainer,
+ EmptyStateTitle,
+} from "../ModelDetailPage.styled";
import { ActionListItem, ActionTitle } from "./ModelActionDetails.styled";
+const mapDispatchToProps = {
+ enableImplicitActionsForModel: Actions.actions.enableImplicitActionsForModel,
+};
+
interface Props {
- model: Question;
- actions: WritebackQueryAction[];
+ modelId: number;
+ actions: WritebackAction[];
+ enableImplicitActionsForModel: (modelId: number) => void;
}
-function ModelActionDetails({ actions }: Props) {
+function ModelActionDetails({
+ actions,
+ modelId,
+ enableImplicitActionsForModel,
+}: Props) {
+ const handleCreateImplicitActions = async () => {
+ await enableImplicitActionsForModel(modelId);
+ };
+
+ if (!actions?.length) {
+ return (
+ <EmptyStateContainer>
+ <EmptyStateTitle>{t`This model does not have any actions yet.`}</EmptyStateTitle>
+ <Button onClick={handleCreateImplicitActions} icon="add">
+ {t`Enable implicit actions`}
+ </Button>
+ <Button
+ as={Link}
+ to="/action/create"
+ icon="add"
+ >{t`Create a new action`}</Button>
+ </EmptyStateContainer>
+ );
+ }
+
+ const modelHasImplicitActions = hasImplicitActions(actions);
+
return (
- <ul>
- {actions.map(action => (
- <li key={action.id}>
- <ActionListItem to={`/action/${action.id}`}>
- <Icon name="insight" />
- <ActionTitle>{action.name}</ActionTitle>
- </ActionListItem>
- </li>
- ))}
- </ul>
+ <>
+ {!modelHasImplicitActions && modelId && (
+ <Button onClick={handleCreateImplicitActions} icon="add">
+ {t`Enable implicit actions`}
+ </Button>
+ )}
+ <ul>
+ {actions.map(action => (
+ <li key={action.id}>
+ <ActionListItem
+ to={`/action/${action.id}`}
+ disabled={isImplicitAction(action)}
+ >
+ <Icon name="insight" />
+ <ActionTitle>
+ {action.name ?? humanize(action.slug ?? "")}
+ </ActionTitle>
+ </ActionListItem>
+ </li>
+ ))}
+ </ul>
+ </>
);
}
-export default Actions.loadList()(ModelActionDetails);
+export default Actions.loadList(
+ {
+ query: (state: State, props: { modelId?: number | null }) => ({
+ "model-id": props?.modelId,
+ }),
+ },
+ connect(null, mapDispatchToProps),
+)(ModelActionDetails);
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.styled.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.styled.tsx
index 715381973bdda94beb17ff9bced58ea489867d05..6cea9850586be0025f37a5da99703f2bc6159512 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.styled.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.styled.tsx
@@ -52,3 +52,22 @@ export const TabPanel = styled(BaseTabPanel)`
display: flex;
flex-direction: column;
`;
+
+export const EmptyStateContainer = styled.div`
+ display: flex;
+ flex-direction: column;
+ justify-content: center;
+ align-items: center;
+ gap: 1rem;
+
+ margin-top: 3rem;
+`;
+
+export const EmptyStateTitle = styled.span`
+ display: block;
+ color: ${color("text-medium")};
+ font-size: 1rem;
+ line-height: 1.5rem;
+ margin-bottom: 1rem;
+ text-align: center;
+`;
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.tsx
index f990cb51dd0f3170778e0b13745e57456df9a170..f8e43de0ef873c791f75b2a3a0aec4fbae4495f7 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelDetailPage.tsx
@@ -78,7 +78,7 @@ function ModelDetailPage({ model, mainTable, onChangeModel }: Props) {
options={[
{ value: "usage", name: t`Used by` },
{ value: "schema", name: t`Schema` },
- // { value: "actions", name: t`Actions` },
+ { value: "actions", name: t`Actions` },
]}
onChange={tab => setTab(tab as ModelTab)}
/>
@@ -89,7 +89,7 @@ function ModelDetailPage({ model, mainTable, onChangeModel }: Props) {
<ModelSchemaDetails model={model} />
</TabPanel>
<TabPanel value="actions">
- <ModelActionDetails model={model} />
+ <ModelActionDetails modelId={model.id()} />
</TabPanel>
</TabContent>
</ModelMain>
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.styled.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.styled.tsx
index 5d42399abe454fe60c4dafc4576aeba8b2606947..9770d1d747714bd511939aa943f81510bb3899fb 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.styled.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.styled.tsx
@@ -25,21 +25,3 @@ export const CardListItem = styled(Link)`
background-color: ${color("brand-light")};
}
`;
-
-export const EmptyStateContainer = styled.div`
- display: flex;
- flex-direction: column;
- justify-content: center;
- align-items: center;
-
- margin-top: 3rem;
-`;
-
-export const EmptyStateTitle = styled.span`
- display: block;
- color: ${color("text-medium")};
- font-size: 1rem;
- line-height: 1.5rem;
- margin-bottom: 1rem;
- text-align: center;
-`;
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.tsx
index b60d2b13a6b92d339111ee85e83409751cc22b07..ff93ad32afe2ec15e1f76762d20f5305315ab0e0 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelUsageDetails/ModelUsageDetails.tsx
@@ -17,13 +17,13 @@ import type { State } from "metabase-types/store";
import type { Card as LegacyCardType } from "metabase-types/types/Card";
import type Question from "metabase-lib/lib/Question";
-import { isQuestionUsingModel } from "./utils";
import {
- CardListItem,
- CardTitle,
EmptyStateContainer,
EmptyStateTitle,
-} from "./ModelUsageDetails.styled";
+} from "../ModelDetailPage.styled";
+
+import { isQuestionUsingModel } from "./utils";
+import { CardListItem, CardTitle } from "./ModelUsageDetails.styled";
interface OwnProps {
model: Question;
diff --git a/frontend/src/metabase/modes/components/drill/ActionClickDrill/ActionClickDrill.unit.spec.ts b/frontend/src/metabase/modes/components/drill/ActionClickDrill/ActionClickDrill.unit.spec.ts
index d6c5fb486d5e7e2d5b9d4850d5d67650dbabebf9..ff3d5d346d4334e449b153914e801eea2435fb14 100644
--- a/frontend/src/metabase/modes/components/drill/ActionClickDrill/ActionClickDrill.unit.spec.ts
+++ b/frontend/src/metabase/modes/components/drill/ActionClickDrill/ActionClickDrill.unit.spec.ts
@@ -3,7 +3,7 @@ import * as dashboardActions from "metabase/dashboard/actions/writeback";
import type {
ActionParametersMapping,
DashboardOrderedCard,
- ParameterMappedForActionExecution,
+ ParametersForActionExecution,
WritebackParameter,
} from "metabase-types/api";
import type { ParameterValueOrArray } from "metabase-types/types/Parameter";
@@ -104,12 +104,10 @@ describe("prepareParameter", () => {
dashboardParamterValues,
);
- expect(parameter).toEqual({
- id: DASHBOARD_FILTER_PARAMETER.id,
- type: WRITEBACK_PARAMETER.type,
- value: DASHBOARD_FILTER_PARAMETER.value,
- target: WRITEBACK_PARAMETER.target,
- });
+ expect(parameter).toEqual([
+ WRITEBACK_PARAMETER.id,
+ DASHBOARD_FILTER_PARAMETER.value,
+ ]);
});
it("handles array-like parameter value", () => {
@@ -123,12 +121,10 @@ describe("prepareParameter", () => {
dashboardParamterValues,
);
- expect(parameter).toEqual({
- id: DASHBOARD_FILTER_PARAMETER.id,
- type: WRITEBACK_PARAMETER.type,
- value: DASHBOARD_FILTER_PARAMETER.value,
- target: WRITEBACK_PARAMETER.target,
- });
+ expect(parameter).toEqual([
+ WRITEBACK_PARAMETER.id,
+ DASHBOARD_FILTER_PARAMETER.value,
+ ]);
});
});
@@ -136,21 +132,16 @@ describe("getNotProvidedActionParameters", () => {
it("returns empty list if no parameters passed", () => {
const action = createMockQueryAction();
- const result = getNotProvidedActionParameters(action, []);
+ const result = getNotProvidedActionParameters(action, {});
expect(result).toHaveLength(0);
});
it("returns empty list if all parameters have values", () => {
const action = createMockQueryAction({ parameters: [WRITEBACK_PARAMETER] });
- const result = getNotProvidedActionParameters(action, [
- {
- id: DASHBOARD_FILTER_PARAMETER.id,
- value: 5,
- type: "number",
- target: WRITEBACK_PARAMETER.target,
- },
- ]);
+ const result = getNotProvidedActionParameters(action, {
+ [WRITEBACK_PARAMETER.id]: 5,
+ });
expect(result).toHaveLength(0);
});
@@ -160,14 +151,9 @@ describe("getNotProvidedActionParameters", () => {
parameters: [WRITEBACK_PARAMETER, WRITEBACK_ARBITRARY_PARAMETER],
});
- const result = getNotProvidedActionParameters(action, [
- {
- id: DASHBOARD_FILTER_PARAMETER.id,
- value: 5,
- type: "number",
- target: WRITEBACK_ARBITRARY_PARAMETER.target,
- },
- ]);
+ const result = getNotProvidedActionParameters(action, {
+ [WRITEBACK_ARBITRARY_PARAMETER.id]: 5,
+ });
expect(result).toEqual([WRITEBACK_PARAMETER]);
});
@@ -177,7 +163,7 @@ describe("getNotProvidedActionParameters", () => {
parameters: [{ ...WRITEBACK_PARAMETER, default: 10 }],
});
- const result = getNotProvidedActionParameters(action, []);
+ const result = getNotProvidedActionParameters(action, {});
expect(result).toHaveLength(0);
});
diff --git a/frontend/src/metabase/modes/components/drill/ActionClickDrill/types.ts b/frontend/src/metabase/modes/components/drill/ActionClickDrill/types.ts
index 6be6b8a91a022c3a45393cd312a490e0bf393e01..0c78b38aadf166ebf5446fb8e2cd47417cb998e3 100644
--- a/frontend/src/metabase/modes/components/drill/ActionClickDrill/types.ts
+++ b/frontend/src/metabase/modes/components/drill/ActionClickDrill/types.ts
@@ -1,7 +1,7 @@
import type {
ActionDashboardCard,
Dashboard,
- ParameterMappedForActionExecution,
+ ParametersForActionExecution,
WritebackParameter,
} from "metabase-types/api";
import type { Column } from "metabase-types/types/Dataset";
@@ -22,7 +22,7 @@ export type ActionClickObject = Omit<ClickObject, "extraData"> & {
data: any;
extraData: ActionClickExtraData;
onSubmit: () => (
- parameters: ParameterMappedForActionExecution[],
+ parameters: ParametersForActionExecution,
) => Promise<boolean>;
missingParameters: WritebackParameter[];
};
diff --git a/frontend/src/metabase/modes/components/drill/ActionClickDrill/utils.ts b/frontend/src/metabase/modes/components/drill/ActionClickDrill/utils.ts
index d8db1047c1ad9e2ea3d430d23427ec9e16c73282..545dc91d373a477a909f0e7e02973c19a51ee6a4 100644
--- a/frontend/src/metabase/modes/components/drill/ActionClickDrill/utils.ts
+++ b/frontend/src/metabase/modes/components/drill/ActionClickDrill/utils.ts
@@ -4,9 +4,11 @@ import { isEmpty } from "metabase/lib/validate";
import type {
ActionDashboardCard,
ActionParametersMapping,
- ParameterMappedForActionExecution,
+ ParametersForActionExecution,
WritebackAction,
WritebackParameter,
+ ParameterId,
+ ActionParameterValue,
} from "metabase-types/api";
import type { ParameterValueOrArray } from "metabase-types/types/Parameter";
@@ -14,25 +16,29 @@ function formatParameterValue(value: ParameterValueOrArray) {
return Array.isArray(value) ? value[0] : value;
}
+type ActionParameterTuple = [ParameterId, ActionParameterValue];
+
export function getDashcardParamValues(
dashcard: ActionDashboardCard,
parameterValues: { [id: string]: ParameterValueOrArray },
-) {
+): ParametersForActionExecution {
if (!dashcard.action || !dashcard?.parameter_mappings?.length) {
- return [];
+ return {};
}
const { action, parameter_mappings } = dashcard;
- return parameter_mappings
- .map(mapping => prepareParameter(mapping, action, parameterValues))
- .filter(Boolean) as ParameterMappedForActionExecution[];
+ return Object.fromEntries(
+ parameter_mappings
+ ?.map(mapping => prepareParameter(mapping, action, parameterValues))
+ ?.filter(Boolean) as ActionParameterTuple[],
+ );
}
export function prepareParameter(
mapping: ActionParametersMapping,
action: WritebackAction,
parameterValues: { [id: string]: ParameterValueOrArray },
-) {
+): ActionParameterTuple | undefined {
const { parameter_id: sourceParameterId, target: actionParameterTarget } =
mapping;
@@ -46,29 +52,22 @@ export function prepareParameter(
return;
}
- return {
- id: sourceParameterId,
- type: actionParameter.type,
- value: formatParameterValue(parameterValue),
- target: actionParameterTarget,
- };
+ return [actionParameter.id, formatParameterValue(parameterValue)];
}
function isMappedParameter(
parameter: WritebackParameter,
- parameterMappings: ParameterMappedForActionExecution[],
+ dashboardParamValues: ParametersForActionExecution,
) {
- return parameterMappings.some(mapping =>
- _.isEqual(mapping.target, parameter.target),
- );
+ return parameter.id in dashboardParamValues;
}
export function getNotProvidedActionParameters(
action: WritebackAction,
- dashboardParamValues: ParameterMappedForActionExecution[],
+ dashboardParamValues: ParametersForActionExecution,
) {
// return any action parameters that don't have mapped values
- return action.parameters.filter(parameter => {
+ return (action.parameters ?? []).filter(parameter => {
if ("default" in parameter) {
return false;
}
diff --git a/frontend/src/metabase/routes.jsx b/frontend/src/metabase/routes.jsx
index 85aaaf7011dde080bc15b7ff484d74c1b7459863..348a0e64ff246582be349be912d94728592bce74 100644
--- a/frontend/src/metabase/routes.jsx
+++ b/frontend/src/metabase/routes.jsx
@@ -92,7 +92,6 @@ import { trackPageView } from "metabase/lib/analytics";
import { getAdminPaths } from "metabase/admin/app/selectors";
import ActionPage from "metabase/writeback/containers/ActionCreatorPage";
-import ActionsListPage from "metabase/writeback/containers/ActionsListPage";
import ModelDetailPage from "metabase/models/containers/ModelDetailPage";
@@ -401,8 +400,6 @@ export const getRoutes = store => (
<Route path="create" component={ActionPage} />
<Route path=":actionId" component={ActionPage} />
</Route>
- {/* DEV PAGE: REMOVE BEFORE SHIPPING */}
- <Route path="/actions" component={ActionsListPage} />
</Route>
</Route>
diff --git a/frontend/src/metabase/services.js b/frontend/src/metabase/services.js
index 3a2890f3d6f4aa53eb3bd71fc4b3775048c4e68c..294a911256622a404dd56e333044f812880977fc 100644
--- a/frontend/src/metabase/services.js
+++ b/frontend/src/metabase/services.js
@@ -476,12 +476,20 @@ function setParamsEndpoints(prefix) {
}
export const ActionsApi = {
+ list: GET("/api/action"),
create: POST("/api/action/row/create"),
update: POST("/api/action/row/update"),
delete: POST("/api/action/row/delete"),
bulkUpdate: POST("/api/action/bulk/update/:tableId"),
bulkDelete: POST("/api/action/bulk/delete/:tableId"),
execute: POST(
- "/api/dashboard/:dashboardId/dashcard/:dashcardId/action/execute",
+ "/api/dashboard/:dashboardId/dashcard/:dashcardId/execute/:slug",
),
};
+
+export const ModelActionsApi = {
+ connectActionToModel: POST("/api/model-action"),
+ createImplicitAction: POST("/api/model-action"),
+ updateConnection: PUT("/api/model-action/:id"),
+ disconnectActionFromModel: POST("/api/model-action"),
+};
diff --git a/frontend/src/metabase/visualizations/components/List/List.tsx b/frontend/src/metabase/visualizations/components/List/List.tsx
index cba58861b4131a025bd08a5a6c6883e040021a71..fb567481163b91c5407f68d7ba01319e4bbe515f 100644
--- a/frontend/src/metabase/visualizations/components/List/List.tsx
+++ b/frontend/src/metabase/visualizations/components/List/List.tsx
@@ -253,8 +253,7 @@ function List({
);
}, [connectedDashCard, settings, bulkActions]);
- const hasInlineActions =
- !isSelectingItems && (hasEditButton || hasDeleteButton);
+ const hasInlineActions = false; // TODO remove completely
const renderBulkSelectionControl = useCallback(
(rowIndex: number) => {
diff --git a/frontend/src/metabase/writeback/components/ActionCreator/ActionCreator.tsx b/frontend/src/metabase/writeback/components/ActionCreator/ActionCreator.tsx
index 5e63132a1be90ec8ed07919edbf547eba491e2b6..b2763d9aca8aa0c1560427d1cafe7485ee9432a5 100644
--- a/frontend/src/metabase/writeback/components/ActionCreator/ActionCreator.tsx
+++ b/frontend/src/metabase/writeback/components/ActionCreator/ActionCreator.tsx
@@ -110,7 +110,7 @@ function ActionCreatorComponent({
<Modal onClose={handleClose}>
<Actions.ModalForm
title={isNew ? t`New action` : t`Save action`}
- form={Actions.forms.saveForm}
+ form={isNew ? Actions.forms.saveForm : Actions.forms.updateForm}
action={{
id: (question.card() as SavedCard).id,
name: question.displayName(),
diff --git a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts
index af5f16f0c991e10420d7fe3540d3a877ceae9b14..4a7b5ef31ed38c4ff67da170368c297c8928cbb8 100644
--- a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts
+++ b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts
@@ -4,6 +4,7 @@ import type {
ActionFormSettings,
WritebackAction,
FieldSettings,
+ ParameterId,
} from "metabase-types/api";
import validate from "metabase/lib/validate";
@@ -102,7 +103,7 @@ export const getFormFieldForParameter = (
fieldSettings: FieldSettings,
) => ({
name: parameter.id,
- title: parameter.name,
+ title: parameter.name ?? parameter.id,
...getParameterFieldProps(fieldSettings),
});
@@ -111,3 +112,16 @@ export const getFormTitle = (action: WritebackAction): string =>
export const getSubmitButtonLabel = (action: WritebackAction): string =>
action.visualization_settings?.submitButtonLabel || t`Save`;
+
+export const generateFieldSettingsFromParameters = (params: Parameter[]) => {
+ const fieldSettings: Record<ParameterId, FieldSettings> = {};
+
+ params.forEach(param => {
+ fieldSettings[param.id] = getDefaultFieldSettings({
+ name: param.name ?? param.id,
+ fieldType: param.type.includes("Integer") ? "number" : "string",
+ inputType: param.type.includes("Integer") ? "number" : "string",
+ });
+ });
+ return fieldSettings;
+};
diff --git a/frontend/src/metabase/writeback/components/ActionViz/Action.tsx b/frontend/src/metabase/writeback/components/ActionViz/Action.tsx
index 4e55b8f09ed0bb2d922f3f71d65458f1f63b62f5..fd6c39c6f28e4d54f8b297915443237d71782eb9 100644
--- a/frontend/src/metabase/writeback/components/ActionViz/Action.tsx
+++ b/frontend/src/metabase/writeback/components/ActionViz/Action.tsx
@@ -6,10 +6,8 @@ import { isImplicitActionButton } from "metabase/writeback/utils";
import type {
ActionDashboardCard,
- ArbitraryParameterForActionExecution,
- ActionParametersMapping,
Dashboard,
- ParameterMappedForActionExecution,
+ ParametersForActionExecution,
WritebackQueryAction,
} from "metabase-types/api";
import type { VisualizationProps } from "metabase-types/types/Visualization";
@@ -19,7 +17,6 @@ import type { ParameterValueOrArray } from "metabase-types/types/Parameter";
import {
getDashcardParamValues,
getNotProvidedActionParameters,
- prepareParameter,
} from "metabase/modes/components/drill/ActionClickDrill/utils";
import { executeRowAction } from "metabase/dashboard/actions";
import { StyledButton } from "./ActionButton.styled";
@@ -69,12 +66,14 @@ function ActionComponent({
actionDisplayType !== "inline" || !missingParameters.length;
const onSubmit = useCallback(
- (extra_parameters: ArbitraryParameterForActionExecution[]) =>
+ (parameterMap: ParametersForActionExecution) =>
executeRowAction({
dashboard,
dashcard,
- parameters: dashcardParamValues,
- extra_parameters,
+ parameters: {
+ ...dashcardParamValues,
+ ...parameterMap,
+ },
dispatch,
shouldToast: shouldDisplayButton,
}),
diff --git a/frontend/src/metabase/writeback/components/ActionViz/ActionForm.tsx b/frontend/src/metabase/writeback/components/ActionViz/ActionForm.tsx
index 7fa68765389cede68ad7b74ffbf1235cfa6e0199..ecd3901e2817e073d034bb37356c6fc39c055b82 100644
--- a/frontend/src/metabase/writeback/components/ActionViz/ActionForm.tsx
+++ b/frontend/src/metabase/writeback/components/ActionViz/ActionForm.tsx
@@ -1,7 +1,6 @@
import React from "react";
import type {
- ArbitraryParameterForActionExecution,
WritebackQueryAction,
WritebackParameter,
OnSubmitActionForm,
diff --git a/frontend/src/metabase/writeback/containers/ActionParametersInputForm/ActionParametersInputForm.tsx b/frontend/src/metabase/writeback/containers/ActionParametersInputForm/ActionParametersInputForm.tsx
index 1b90a2bd4dc91ecc6456f695bb189aeb667d199b..3bc2786440852dc0bb595f4e153c61f410e67969 100644
--- a/frontend/src/metabase/writeback/containers/ActionParametersInputForm/ActionParametersInputForm.tsx
+++ b/frontend/src/metabase/writeback/containers/ActionParametersInputForm/ActionParametersInputForm.tsx
@@ -4,6 +4,7 @@ import Form from "metabase/containers/FormikForm";
import {
getFormFieldForParameter,
getSubmitButtonLabel,
+ generateFieldSettingsFromParameters,
} from "metabase/writeback/components/ActionCreator/FormCreator";
import type {
@@ -11,9 +12,8 @@ import type {
WritebackQueryAction,
OnSubmitActionForm,
} from "metabase-types/api";
-import type { FormFieldDefinition } from "metabase-types/forms";
-import { formatParametersBeforeSubmit, setDefaultValues } from "./utils";
+import { setDefaultValues, setNumericValues } from "./utils";
interface Props {
missingParameters: WritebackParameter[];
@@ -29,10 +29,11 @@ function ActionParametersInputForm({
onSubmitSuccess,
}: Props) {
const fieldSettings = useMemo(
- () => action.visualization_settings?.fields ?? {},
- [action],
+ () =>
+ action.visualization_settings?.fields ??
+ generateFieldSettingsFromParameters(missingParameters),
+ [action, missingParameters],
);
-
const formParams = useMemo(
() => missingParameters ?? Object.values(action.parameters) ?? [],
[missingParameters, action],
@@ -41,7 +42,7 @@ function ActionParametersInputForm({
const form = useMemo(() => {
return {
fields: formParams?.map(param =>
- getFormFieldForParameter(param, fieldSettings[param.id]),
+ getFormFieldForParameter(param, fieldSettings[param.id] ?? {}),
),
};
}, [formParams, fieldSettings]);
@@ -50,12 +51,13 @@ function ActionParametersInputForm({
async (params, actions) => {
actions.setSubmitting(true);
const paramsWithDefaultValues = setDefaultValues(params, fieldSettings);
-
- const formattedParams = formatParametersBeforeSubmit(
+ const paramsWithNumericValues = setNumericValues(
paramsWithDefaultValues,
- formParams,
+ fieldSettings,
);
- const { success, error } = await onSubmit(formattedParams);
+
+ const { success, error } = await onSubmit(paramsWithNumericValues);
+
if (success) {
actions.setErrors({});
onSubmitSuccess?.();
@@ -64,7 +66,7 @@ function ActionParametersInputForm({
throw new Error(error);
}
},
- [onSubmit, onSubmitSuccess, fieldSettings, formParams],
+ [onSubmit, onSubmitSuccess, fieldSettings],
);
const initialValues = useMemo(
diff --git a/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.ts b/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.ts
index d9669959f8ae495177b43ee9dcb1c51ea6283dd5..64262d9e51d53ca2204d72921c6b2af4dc6777fb 100644
--- a/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.ts
+++ b/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.ts
@@ -1,53 +1,31 @@
import { isEmpty } from "metabase/lib/validate";
import type {
- ArbitraryParameterForActionExecution,
- WritebackParameter,
FieldSettings,
+ ParametersForActionExecution,
} from "metabase-types/api";
-import type { Parameter, ParameterId } from "metabase-types/types/Parameter";
-
-type ParameterMap = Record<ParameterId, string | number>;
-
-function getActionParameterType(parameter: Parameter) {
- const { type } = parameter;
- if (type === "category") {
- return "string/=";
- }
- return type;
-}
-
-export function formatParametersBeforeSubmit(
- values: ParameterMap,
- missingParameters: WritebackParameter[],
+// set user-defined default values for any non-required empty parameters
+export function setDefaultValues(
+ params: ParametersForActionExecution,
+ fieldSettings: { [tagId: string]: FieldSettings },
) {
- const formattedParams: ArbitraryParameterForActionExecution[] = [];
-
- Object.keys(values).forEach(parameterId => {
- const parameter = missingParameters.find(
- parameter => parameter.id === parameterId,
- );
- if (parameter) {
- formattedParams.push({
- value: values[parameterId],
- type: getActionParameterType(parameter),
- target: parameter.target,
- });
+ Object.entries(params).forEach(([key, value]) => {
+ if (isEmpty(value) && fieldSettings[key] && !fieldSettings[key].required) {
+ params[key] = fieldSettings[key].defaultValue ?? "";
}
});
- return formattedParams;
+ return params;
}
-// set user-defined default values for any non-required empty parameters
-export function setDefaultValues(
- params: ParameterMap,
+export function setNumericValues(
+ params: ParametersForActionExecution,
fieldSettings: { [tagId: string]: FieldSettings },
) {
Object.entries(params).forEach(([key, value]) => {
- if (isEmpty(value) && fieldSettings[key] && !fieldSettings[key].required) {
- params[key] = fieldSettings[key].defaultValue ?? "";
+ if (fieldSettings[key]?.fieldType === "number") {
+ params[key] = Number(value) ?? 0;
}
});
diff --git a/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.unit.spec.ts b/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.unit.spec.ts
index c70f251c0ffcb6e1edf63130c386535f9e6b7612..5b9a7eebe791f12238923a214cbd857e7d02171b 100644
--- a/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.unit.spec.ts
+++ b/frontend/src/metabase/writeback/containers/ActionParametersInputForm/utils.unit.spec.ts
@@ -1,41 +1,7 @@
import type { FieldSettingsMap } from "metabase-types/api";
-import { formatParametersBeforeSubmit, setDefaultValues } from "./utils";
+import { setDefaultValues } from "./utils";
describe("writeback > containers > ActionParametersInputForm > utils", () => {
- describe("formatParametersBeforeSubmit", () => {
- it("should format with their ids for the API", () => {
- const values = {
- "abc-def": "1",
- "ghi-jkl": 2,
- };
- const missingParameters: any = [
- {
- id: "abc-def",
- type: "string/=",
- target: "",
- },
- {
- id: "ghi-jkl",
- type: "number/=",
- target: "",
- },
- ];
- const result = formatParametersBeforeSubmit(values, missingParameters);
- expect(result).toEqual([
- {
- value: "1",
- type: "string/=",
- target: "",
- },
- {
- value: 2,
- type: "number/=",
- target: "",
- },
- ]);
- });
- });
-
describe("setDefaultValues", () => {
it("should set a default value for a non-required empty form field", () => {
const params = {
diff --git a/frontend/src/metabase/writeback/containers/ActionsListPage.tsx b/frontend/src/metabase/writeback/containers/ActionsListPage.tsx
deleted file mode 100644
index 03832cb621012d683de52723bb9b8a0a017d36dc..0000000000000000000000000000000000000000
--- a/frontend/src/metabase/writeback/containers/ActionsListPage.tsx
+++ /dev/null
@@ -1,50 +0,0 @@
-// dev page, should NOT be shipped
-
-import React from "react";
-import { Link } from "react-router";
-import _ from "underscore";
-import { connect } from "react-redux";
-
-import Actions from "metabase/entities/actions";
-import type { WritebackQueryAction } from "metabase-types/api";
-
-function ActionsListPage({ actions }: { actions: WritebackQueryAction[] }) {
- return (
- <div className="p4">
- <div className="flex justify-between">
- <h1>Actions</h1>
- <Link to="/action/create" className="Button Button--primary">
- Create Action
- </Link>
- </div>
- <div className="bordered rounded mt3">
- {actions
- ?.sort((a, b) => b.id - a.id)
- .map(action => (
- <ActionListItem action={action} key={action.id} />
- ))}
- </div>
- </div>
- );
-}
-
-const ActionListItem = ({ action }: { action: WritebackQueryAction }) => {
- return (
- <Link to={`/action/${action.id}`}>
- <div className="border-bottom p1" style={{}}>
- <strong>
- <span className="text-primary">{action.name}</span>
- <span className="text-light ml1">{action.id}</span>
- </strong>
- {!!action.description && (
- <div className="mt1">{action.description}</div>
- )}
- </div>
- </Link>
- );
-};
-
-export default _.compose(
- Actions.loadList(),
- connect(ActionsListPage),
-)(ActionsListPage);
diff --git a/frontend/src/metabase/writeback/utils.ts b/frontend/src/metabase/writeback/utils.ts
index 063fb2a669b9e6c7badf0ba9d78c142fd840e4e2..41acafd2886c7094c517443a57c9d93348fd434c 100644
--- a/frontend/src/metabase/writeback/utils.ts
+++ b/frontend/src/metabase/writeback/utils.ts
@@ -3,6 +3,7 @@ import type {
BaseDashboardOrderedCard,
ClickBehavior,
Database as IDatabase,
+ WritebackAction,
} from "metabase-types/api";
import type { SavedCard } from "metabase-types/types/Card";
import { TYPE } from "metabase-lib/lib/types/constants";
@@ -81,7 +82,9 @@ export function isMappedExplicitActionButton(
dashCard: BaseDashboardOrderedCard,
): dashCard is ActionDashboardCard {
const isAction = isActionDashCard(dashCard);
- return isAction && typeof dashCard.action_id === "number";
+ return (
+ isAction && typeof dashCard.visualization_settings.action_slug === "string"
+ );
}
export function isValidImplicitActionClickBehavior(
@@ -123,3 +126,9 @@ export function getActionButtonLabel(dashCard: ActionDashboardCard) {
const label = dashCard.visualization_settings?.["button.label"];
return label || "";
}
+
+export const hasImplicitActions = (actions: WritebackAction[]): boolean =>
+ actions.some(isImplicitAction);
+
+export const isImplicitAction = (action: WritebackAction): boolean =>
+ action.type === "implicit";
diff --git a/frontend/src/metabase/writeback/utils.unit.spec.ts b/frontend/src/metabase/writeback/utils.unit.spec.ts
index 3c46d3d2b51d585436fd9d5ff2198b0b65e20b40..a839db3031f829fa32c9d94b9268411c71fe228d 100644
--- a/frontend/src/metabase/writeback/utils.unit.spec.ts
+++ b/frontend/src/metabase/writeback/utils.unit.spec.ts
@@ -19,7 +19,10 @@ const QUERY_ACTION = createMockQueryAction();
const EXPLICIT_ACTION = createMockDashboardActionButton({
action_id: QUERY_ACTION.id,
action: QUERY_ACTION,
- visualization_settings: { click_behavior: undefined },
+ visualization_settings: {
+ click_behavior: undefined,
+ action_slug: "action_1337",
+ },
});
const PARAMETER_MAPPINGS: ActionParametersMapping[] = [
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 6dbab243a1dabf875e463c89617d1be287e4b262..d230622e796f932c740762188373679314b4ec12 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -12825,6 +12825,80 @@ databaseChangeLog:
CHANGE updated_at
updated_at timestamp(6) NOT NULL DEFAULT current_timestamp(6);
+ - changeSet:
+ id: v45.00-036
+ author: snoe
+ comment: Added 0.45.0 - add model action table
+ changes:
+ - createTable:
+ tableName: model_action
+ remarks: Ties actions to models
+ columns:
+ - column:
+ name: id
+ type: int
+ autoIncrement: true
+ constraints:
+ primaryKey: true
+ nullable: false
+ - column:
+ name: entity_id
+ type: char(21)
+ remarks: Random NanoID tag for unique identity.
+ constraints:
+ nullable: false
+ unique: true
+ - column:
+ name: slug
+ type: varchar(254)
+ remarks: The referenceable name for this action
+ constraints:
+ nullable: false
+ - column:
+ name: card_id
+ type: int
+ remarks: The associated model
+ constraints:
+ nullable: false
+ referencedTableName: report_card
+ referencedColumnNames: id
+ foreignKeyName: fk_model_action_ref_card_id
+ deleteCascade: true
+ - column:
+ name: action_id
+ type: int
+ remarks: The associated action
+ constraints:
+ nullable: true
+ referencedTableName: action
+ referencedColumnNames: id
+ foreignKeyName: fk_model_action_ref_action_id
+ deleteCascade: true
+ - column:
+ name: requires_pk
+ remarks: Indicates that the primary key(s) need to be passed in as parameters
+ type: boolean
+ defaultValueBoolean: false
+ constraints:
+ nullable: false
+ - column:
+ name: parameter_mappings
+ type: ${text.type}
+ remarks: Mappings for primary keys to action parameters
+ - column:
+ name: visualization_settings
+ type: ${text.type}
+ remarks: Settings for rendering the action
+ - changeSet:
+ id: v45.00-037
+ author: snoe
+ comment: Added 0.45.0 - model action
+ changes:
+ - addUniqueConstraint:
+ tableName: model_action
+ columnNames: card_id, slug
+ constraintName: unique_model_action_card_id_slug
+
# The next 4 values add default values for Database `created_at` and `updated_at`; previously this was handled by
# Toucan but it's more convenient to do this at the application database level instead -- it facilitates stuff like
# schema migration tests that don't use Toucan, or other manual scripting
@@ -12919,7 +12993,6 @@ databaseChangeLog:
tableName: metabase_database
columnName: details
-
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
diff --git a/src/metabase/actions/execution.clj b/src/metabase/actions/execution.clj
index 58da59969822a6d66a333119f309841fb48f6c2f..12e147a6ba945ef6b0d1dc788bf9c26281c7b62b 100644
--- a/src/metabase/actions/execution.clj
+++ b/src/metabase/actions/execution.clj
@@ -1,63 +1,28 @@
(ns metabase.actions.execution
(:require
+ [clojure.set :as set]
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase.actions :as actions]
[metabase.actions.http-action :as http-action]
[metabase.api.common :as api]
- [metabase.mbql.normalize :as mbql.normalize]
- [metabase.models :refer [Dashboard DashboardCard]]
+ [metabase.models :refer [Card Dashboard DashboardCard ModelAction Table]]
[metabase.models.action :as action]
+ [metabase.models.query :as query]
[metabase.query-processor.error-type :as qp.error-type]
[metabase.query-processor.middleware.permissions :as qp.perms]
[metabase.query-processor.writeback :as qp.writeback]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
- [toucan.db :as db]))
-
-(defn- map-parameters
- "Take the `parameters` map passed in to an endpoint and map it to the parameters
- in the underlying `Action` so they can be attached to the query that gets passed to
- [[qp/execute-write-query!]] or [[execute-http-action!]].
-
- Incoming API request `:parameters` should look like
-
- [{:id \"my_id\",
- :type \"id\",
- :value \"12\"}]
-
- And `parameter_mappings` should look like
-
- [{:parameter_id \"my_id\", :target [:variable [:template-tag \"id\"]]}]
-
- We need to convert these to a list like
-
- [{:id \"my_id\"
- :type \"id\"
- :target [:variable [:template-tag \"id\"]]
- :value \"12\"}]
-
- before passing to the QP code."
- [parameters parameter-mappings]
- (let [mappings-by-id (m/index-by :parameter_id parameter-mappings)]
- (mapv (fn [{param-id :id :as parameter}]
- (let [target (or (get-in mappings-by-id [param-id :target])
- (throw (ex-info (tru "No parameter mapping found for parameter {0}. Found: {1}"
- (pr-str param-id)
- (pr-str (set (map :parameter_id parameter-mappings))))
- {:status-code 400
- :type qp.error-type/invalid-parameter
- :parameters parameters
- :mappings parameter-mappings})))]
- (assoc parameter :target target)))
- parameters)))
+ [toucan.db :as db]
+ [toucan.hydrate :refer [hydrate]]))
(defn- execute-query-action!
"Execute a `QueryAction` with parameters as passed in from an
- endpoint (see [[map-parameters]] for a description of their shape).
+ endpoint of shape `{<parameter-id> <value>}`.
`action` should already be hydrated with its `:card`."
- [{:keys [card] action-id :id :as action} parameters]
+ [{:keys [card] action-id :id :as action} request-parameters]
(when-not card
(throw (ex-info (tru "No Card found for Action {0}." action-id)
{:status-code 400, :action action})))
@@ -68,7 +33,9 @@
{:status-code 400, :action action})))
(log/tracef "Executing action\n\n%s" (u/pprint-to-str action))
(try
- (let [query (assoc (:dataset_query card) :parameters parameters)]
+ (let [parameters (for [parameter (:parameters action)]
+ (assoc parameter :value (get request-parameters (:id parameter))))
+ query (assoc (:dataset_query card) :parameters parameters)]
(log/debugf "Query (before preprocessing):\n\n%s" (u/pprint-to-str query))
(binding [qp.perms/*card-id* (:id card)]
(qp.writeback/execute-write-query! query)))
@@ -77,50 +44,105 @@
(api/throw-403 e)
(throw (ex-info (tru "Error executing Action: {0}" (ex-message e))
{:action action
- :parameters parameters}
+ :parameters request-parameters}
e))))))
-(defn- execute-http-action!
- [action parameters]
- (let [params->value (->> parameters
- (map (juxt (comp second :target) :value))
- (into {}))]
- (http-action/execute-http-action! action params->value)))
-
-(defn execute-dashcard!
- "Execute the given action in the dashboard/dashcard context with the given unmapped-parameters and extra-parameters.
- See [[map-parameters]] for a description of their expected shapes."
- [dashboard-id dashcard-id unmapped-parameters extra-parameters]
- (actions/check-actions-enabled)
- (api/read-check Dashboard dashboard-id)
- (let [dashcard (api/check-404 (db/select-one DashboardCard
- :id dashcard-id
- :dashboard_id dashboard-id))
- action (api/check-404 (first (action/select-actions :id (:action_id dashcard))))
+(defn- execute-custom-action [action-id request-parameters]
+ (let [action (api/check-404 (first (action/select-actions :id action-id)))
action-type (:type action)
- _ (log/tracef "Mapping parameters\n\n%s\nwith mappings\n\n%s"
- (u/pprint-to-str unmapped-parameters)
- (u/pprint-to-str (:parameter_mappings dashcard)))
- mapped-parameters (map-parameters
- (mbql.normalize/normalize-fragment [:parameters] unmapped-parameters)
- (:parameter_mappings dashcard))
- parameters (into (mbql.normalize/normalize-fragment [:parameters] extra-parameters)
- mapped-parameters)
- destination-parameters-by-target (m/index-by :target (:parameters action))]
- (doseq [{:keys [target]} parameters]
- (when-not (contains? destination-parameters-by-target target)
- (throw (ex-info (tru "No destination parameter found for target {0}. Found: {1}"
- (pr-str target)
- (pr-str (set (keys destination-parameters-by-target))))
+ destination-parameters-by-id (m/index-by :id (:parameters action))]
+ (doseq [[parameter-id _value] request-parameters]
+ (when-not (contains? destination-parameters-by-id parameter-id)
+ (throw (ex-info (tru "No destination parameter found for id {0}. Found: {1}"
+ (pr-str parameter-id)
+ (pr-str (set (keys destination-parameters-by-id))))
{:status-code 400
:type qp.error-type/invalid-parameter
- :parameters parameters
+ :parameters request-parameters
:destination-parameters (:parameters action)}))))
(case action-type
:query
- (execute-query-action! action parameters)
+ (execute-query-action! action request-parameters)
:http
- (execute-http-action! action parameters)
+ (http-action/execute-http-action! action request-parameters)
(throw (ex-info (tru "Unknown action type {0}." (name action-type)) action)))))
+
+(defn- implicit-action-table
+ [card_id]
+ (let [card (db/select-one Card :id card_id)
+ {:keys [table-id]} (query/query->database-and-table-ids (:dataset_query card))]
+ (hydrate (db/select-one Table :id table-id) :fields)))
+
+(defn- execute-implicit-action
+ [{:keys [card_id slug requires_pk] :as _model-action} request-parameters]
+ (let [{database-id :db_id table-id :id :as table} (implicit-action-table card_id)
+ pk-fields (filterv #(isa? (:semantic_type %) :type/PK) (:fields table))
+ slug->field-name (into {} (map (juxt (comp u/slugify :name) :name)) (:fields table))
+ _ (api/check (action/unique-field-slugs? (:fields table))
+ 400
+ (tru "Cannot execute implicit action on a table with ambiguous column names."))
+ _ (api/check (= (count pk-fields) 1)
+ 400
+ (tru "Must execute implicit action on a table with a single primary key."))
+ extra-parameters (set/difference (set (keys request-parameters))
+ (set (keys slug->field-name)))
+
+ pk-field (first pk-fields)
+ simple-parameters (update-keys request-parameters (comp keyword slug->field-name))
+ pk-field-name (keyword (:name pk-field))
+ _ (api/check (empty? extra-parameters)
+ 400
+ {:message (tru "No destination parameter found for {0}. Found: {1}"
+ (pr-str extra-parameters)
+ (pr-str (set (keys slug->field-name))))
+ :parameters request-parameters
+ :destination-parameters (keys slug->field-name)})
+
+ _ (api/check (or (not requires_pk)
+ (some? (get simple-parameters pk-field-name)))
+ 400
+ (tru "Missing primary key parameter: {0}"
+ (pr-str (u/slugify (:name pk-field)))))
+ query (cond-> {:database database-id,
+ :type :query,
+ :query {:source-table table-id}}
+ requires_pk
+ (assoc-in [:query :filter]
+ [:= [:field (:id pk-field) nil] (get simple-parameters pk-field-name)]))
+ implicit-action (cond
+ (= slug "delete")
+ :row/delete
+
+ requires_pk
+ :row/update
+
+ :else
+ :row/create)
+ row-parameters (cond-> simple-parameters
+ (not= implicit-action :row/create) (dissoc pk-field-name))
+ _ (api/check (or (= implicit-action :row/delete) (seq row-parameters))
+ 400
+ (tru "Implicit parameters must be provided."))
+ arg-map (cond-> query
+ (= implicit-action :row/create)
+ (assoc :create-row row-parameters)
+
+ (= implicit-action :row/update)
+ (assoc :update-row row-parameters))]
+ (actions/perform-action! implicit-action arg-map)))
+
+(defn execute-dashcard!
+ "Execute the given action in the dashboard/dashcard context with the given parameters
+ of shape `{<parameter-id> <value>}."
+ [dashboard-id dashcard-id slug request-parameters]
+ (actions/check-actions-enabled)
+ (api/read-check Dashboard dashboard-id)
+ (let [dashcard (api/check-404 (db/select-one DashboardCard
+ :id dashcard-id
+ :dashboard_id dashboard-id))
+ model-action (api/check-404 (db/select-one ModelAction :card_id (:card_id dashcard) :slug slug))]
+ (if-let [action-id (:action_id model-action)]
+ (execute-custom-action action-id request-parameters)
+ (execute-implicit-action model-action request-parameters))))
diff --git a/src/metabase/api/action.clj b/src/metabase/api/action.clj
index b25dd82483250a49684eacfcc1bcef77c2f6c372..1125c52e09dcb5e502b11776fe13a1e91d040693 100644
--- a/src/metabase/api/action.clj
+++ b/src/metabase/api/action.clj
@@ -71,8 +71,9 @@
(api/defendpoint GET "/"
"Returns cards that can be used for QueryActions"
- []
- (hydrate (action/select-actions) :action/emitter-usages))
+ [model-id]
+ {model-id su/IntGreaterThanZero}
+ (action/merged-model-action nil :card_id model-id))
(api/defendpoint GET "/:action-id"
[action-id]
diff --git a/src/metabase/api/app.clj b/src/metabase/api/app.clj
index 2a8557235fe28e483fc5af39570e187c632367b0..2a1c09ca840a1fb44d4aacf46cd6fa2031f3a8da 100644
--- a/src/metabase/api/app.clj
+++ b/src/metabase/api/app.clj
@@ -7,6 +7,7 @@
[metabase.api.card :as api.card]
[metabase.api.collection :as api.collection]
[metabase.api.common :as api]
+ [metabase.mbql.schema :as mbql.s]
[metabase.models :refer [App Collection Dashboard Table]]
[metabase.models.app.graph :as app.graph]
[metabase.models.collection :as collection]
@@ -101,8 +102,12 @@
(throw (ex-info (i18n/tru "A scaffold-target was not provided for Card: {0}" (:name card))
{:status-code 400})))
(let [card (api.card/create-card! (-> card
+ (replace-scaffold-targets accum)
(assoc :collection_id collection-id)
- (dissoc :scaffold-target)))]
+ (dissoc :scaffold-target)
+ (cond-> ;; card
+ (not (:dataset card))
+ (update-in [:dataset_query :query :source_table] #(str "card__" %)))))]
(assoc accum (into ["scaffold-target-id"] scaffold-target) (:id card))))
{}
cards)]
@@ -146,7 +151,9 @@
(pr-str (map :id tables)))
{:status-code 400})))
table-id->table (m/index-by :id tables)
- page-type-display {"list" {:name (i18n/tru "List")
+ page-type-display {"model" {:name (i18n/tru "Model")
+ :display "table"}
+ "list" {:name (i18n/tru "List")
:display "list"}
"detail" {:name (i18n/tru "Detail")
:display "object"}}]
@@ -175,25 +182,32 @@
(sort-by :priority)
first
:field-id)]
- page-type ["list" "detail"]]
- {:scaffold-target ["card" table-id page-type]
- :name (format "Query %s %s"
- (or (:display_name table) (:name table))
- (get-in page-type-display [page-type :name]))
- :display (get-in page-type-display [page-type :display])
- :visualization_settings (cond-> {}
- (= page-type "list") (assoc "actions.bulk_enabled" false))
- :dataset_query {:type "query"
- :database (:db_id table)
- :query (cond-> {:source_table table-id}
- order-by-field-id (assoc :order_by [["desc", ["field", order-by-field-id, nil]]]))}})
+ page-type ["model" "list" "detail"]]
+ (if (= "model" page-type)
+ {:scaffold-target ["card" table-id page-type]
+ :name (or (:display_name table) (:name table))
+ :display (get-in page-type-display [page-type :display])
+ :visualization_settings {}
+ :dataset true
+ :dataset_query {:type "query"
+ :database (:db_id table)
+ :query (cond-> {:source_table table-id}
+ order-by-field-id (assoc :order_by [["desc", ["field", order-by-field-id, nil]]]))}}
+ {:scaffold-target ["card" table-id page-type]
+ :name (format "Query %s %s"
+ (or (:display_name table) (:name table))
+ (get-in page-type-display [page-type :name]))
+ :display (get-in page-type-display [page-type :display])
+ :visualization_settings (cond-> {}
+ (= page-type "list") (assoc "actions.bulk_enabled" false))
+ :dataset_query {:database mbql.s/saved-questions-virtual-database-id, :type "query", :query {:source_table ["scaffold-target-id" "card" table-id "model"]}}}))
:pages (for [table-id table-ids
:let [table (get table-id->table table-id)
pks (filter (comp #(= :type/PK %) :semantic_type) (:fields table))
_ (when (not= 1 (count pks))
(throw (ex-info (i18n/tru "Table must have a single primary key: {0}" (:name table))
{:status-code 400})))
- pk-field-id (:id (first pks))]
+ pk-field-name (u/slugify (:name (first pks)))]
page-type ["list" "detail"]]
(cond->
{:name (format "%s %s"
@@ -214,13 +228,14 @@
"id" (str "scaffold_" table-id)}}
"targetId" ["scaffold-target-id" "page" table-id "detail"]}}}
{:size_y 1 :size_x 2 :row 0 :col 16
+ :card_id ["scaffold-target-id" "card" table-id "model"]
:visualization_settings {"virtual_card" {"display" "action"}
"button.label" (i18n/tru "New"),
- "click_behavior" {"type" "action" "actionType" "insert" "tableId" table-id}}}]
+ "action_slug" "insert"}}]
[{:size_y 8 :size_x 18 :row 1 :col 0
:parameter_mappings [{"parameter_id" (str "scaffold_" table-id)
"card_id" ["scaffold-target-id" "card" table-id "detail"]
- "target" ["dimension", ["field", pk-field-id, nil]]}]
+ "target" ["variable", ["template-tag", pk-field-name]]}]
:card_id ["scaffold-target-id" "card" table-id "detail"]
:scaffold-target ["dashcard" table-id]}
{:size_y 1 :size_x 2 :row 0 :col 0
@@ -228,14 +243,20 @@
"button.label" (i18n/tru "↠Back to list"),
"click_behavior" {"type" "link" "linkType" "page" "targetId" ["scaffold-target-id" "page" table-id "list"]}}}
{:size_y 1 :size_x 2 :row 0 :col 16
+ :card_id ["scaffold-target-id" "card" table-id "model"]
+ :parameter_mappings [{"parameter_id" (str "scaffold_" table-id)
+ "target" ["variable", ["template-tag", pk-field-name]]}]
:visualization_settings {"virtual_card" {"display" "action"}
"button.label" (i18n/tru "Delete"),
"button.variant" "danger"
- "click_behavior" {"type" "action" "actionType" "delete" "objectDetailDashCardId" ["scaffold-target-id" "dashcard" table-id]}}}
+ "action_slug" "delete"}}
{:size_y 1 :size_x 2 :row 0 :col 14
+ :card_id ["scaffold-target-id" "card" table-id "model"]
+ :parameter_mappings [{"parameter_id" (str "scaffold_" table-id)
+ "target" ["variable", ["template-tag", pk-field-name]]}]
:visualization_settings {"virtual_card" {"display" "action"}
"button.label" (i18n/tru "Edit"),
- "click_behavior" {"type" "action" "actionType" "update" "objectDetailDashCardId" ["scaffold-target-id" "dashcard" table-id]}}}])}
+ "action_slug" "update"}}])}
(= "detail" page-type) (assoc :parameters [{:name "ID",
:slug "id",
:id (str "scaffold_" table-id),
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index 3e517c6a2a08817480ec6065e954f07bdc9bae09..504ee6a53d9f72ea260d62957c15e941d82342e0 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -697,24 +697,27 @@
"value must be a parameter map with an 'id' key"))
-(def ParameterWithTarget
- "Schema for a parameter map with an mbql `:target`."
- (su/with-api-error-message
- {:target [s/Any]
- s/Keyword s/Any}
- "value must be a parameter map with a 'target' key"))
-
;;; ---------------------------------- Executing the action associated with a Dashcard -------------------------------
-
-(api/defendpoint POST "/:dashboard-id/dashcard/:dashcard-id/action/execute"
+(api/defendpoint GET "/:dashboard-id/dashcard/:dashcard-id/execute/:slug"
+ "Fetches the values for filling in execution parameters."
+ [dashboard-id dashcard-id slug]
+ {dashboard-id su/IntGreaterThanZero
+ dashcard-id su/IntGreaterThanZero
+ slug su/NonBlankString}
+ (throw (UnsupportedOperationException. "Not implemented")))
+
+(api/defendpoint POST "/:dashboard-id/dashcard/:dashcard-id/execute/:slug"
"Execute the associated Action in the context of a `Dashboard` and `DashboardCard` that includes it.
`parameters` should be the mapped dashboard parameters with values.
`extra_parameters` should be the extra, user entered parameter values."
- [dashboard-id dashcard-id :as {{:keys [parameters extra_parameters], :as _body} :body}]
- {parameters (s/maybe [ParameterWithID])
- extra_parameters (s/maybe [ParameterWithTarget])}
- (actions.execution/execute-dashcard! dashboard-id dashcard-id parameters extra_parameters))
+ [dashboard-id dashcard-id slug :as {{:keys [parameters], :as _body} :body}]
+ {dashboard-id su/IntGreaterThanZero
+ dashcard-id su/IntGreaterThanZero
+ slug su/NonBlankString
+ parameters (s/maybe {s/Keyword s/Any})}
+ ;; Undo middleware string->keyword coercion
+ (actions.execution/execute-dashcard! dashboard-id dashcard-id slug (update-keys parameters name)))
;;; ---------------------------------- Running the query associated with a Dashcard ----------------------------------
diff --git a/src/metabase/api/model_action.clj b/src/metabase/api/model_action.clj
new file mode 100644
index 0000000000000000000000000000000000000000..0934eac670eee779ba3650062e80c56404de0310
--- /dev/null
+++ b/src/metabase/api/model_action.clj
@@ -0,0 +1,55 @@
+(ns metabase.api.model-action
+ (:require
+ [compojure.core :refer [DELETE GET POST PUT]]
+ [honeysql.core :as hsql]
+ [metabase.actions :as actions]
+ [metabase.api.common :as api]
+ [metabase.models :refer [Action Card HTTPAction ModelAction QueryAction]]
+ [metabase.util.schema :as su]
+ [schema.core :as s]
+ [toucan.db :as db]))
+
+(api/defendpoint GET "/"
+ "Endpoint to fetch actions for a model, must filter with card-id="
+ [card-id]
+ {card-id su/IntGreaterThanZero}
+ (db/query {:select [:model_action.*
+ [(hsql/call :coalesce :report_card.name :http_action.name) :name]]
+ :from [ModelAction]
+ :left-join [QueryAction [:= :model_action.action_id :query_action.action_id]
+ Card [:= :report_card.id :query_action.card_id]
+ HTTPAction [:= :model_action.action_id :http_action.action_id]]
+ :where [:= :model_action.card_id card-id]
+ :order-by [:model_action.id]}))
+
+(api/defendpoint POST "/"
+ "Endpoint to associate an action with a model"
+ [:as {{:keys [card_id action_id slug requires_pk parameter_mappings visualization_settings] :as body} :body}]
+ {card_id su/IntGreaterThanZero
+ action_id (s/maybe su/IntGreaterThanZero)
+ slug su/NonBlankString
+ requires_pk s/Bool
+ parameter_mappings (s/maybe [su/ParameterMapping])
+ visualization_settings (s/maybe su/Map)}
+ (db/insert! ModelAction body))
+
+(api/defendpoint PUT "/:model-action-id"
+ "Endpoint to modify an action of a model"
+ [model-action-id :as {{:keys [action_id slug requires_pk parameter_mappings visualization_settings] :as body} :body}]
+ {action_id (s/maybe su/IntGreaterThanZero)
+ slug (s/maybe su/NonBlankString)
+ requires_pk (s/maybe s/Bool)
+ parameter_mappings (s/maybe [su/ParameterMapping])
+ visualization_settings (s/maybe su/Map)}
+ (db/update! ModelAction model-action-id (dissoc body :card_id))
+ api/generic-204-no-content)
+
+(api/defendpoint DELETE "/:model-action-id"
+ "Endpoint to delete an action"
+ [model-action-id]
+ (let [action_id (db/select-field :action_id ModelAction :id model-action-id)]
+ ;; Let cascade delete handle this
+ (db/delete! Action :id action_id)
+ api/generic-204-no-content))
+
+(api/define-routes actions/+check-actions-enabled)
diff --git a/src/metabase/api/routes.clj b/src/metabase/api/routes.clj
index 6a6620a7a6655e97dd3ee0f8499ceab1283294c4..e15d158d1281fc29e3c7cc72e4aef2cba2b0c478 100644
--- a/src/metabase/api/routes.clj
+++ b/src/metabase/api/routes.clj
@@ -21,6 +21,7 @@
[metabase.api.ldap :as api.ldap]
[metabase.api.login-history :as api.login-history]
[metabase.api.metric :as api.metric]
+ [metabase.api.model-action :as api.model-action]
[metabase.api.native-query-snippet :as api.native-query-snippet]
[metabase.api.notify :as api.notify]
[metabase.api.permissions :as api.permissions]
@@ -86,6 +87,7 @@
(context "/login-history" [] (+auth api.login-history/routes))
(context "/premium-features" [] (+auth api.premium-features/routes))
(context "/metric" [] (+auth api.metric/routes))
+ (context "/model-action" [] (+auth api.model-action/routes))
(context "/native-query-snippet" [] (+auth api.native-query-snippet/routes))
(context "/notify" [] (+apikey api.notify/routes))
(context "/permissions" [] (+auth api.permissions/routes))
diff --git a/src/metabase/cmd/copy.clj b/src/metabase/cmd/copy.clj
index f6bcc755243d59a342df9ee4f30526dbd50bace2..2e06bc50a2fb7cae977e66b0864fa7bda69b4320 100644
--- a/src/metabase/cmd/copy.clj
+++ b/src/metabase/cmd/copy.clj
@@ -30,6 +30,7 @@
LoginHistory
Metric
MetricImportantField
+ ModelAction
ModerationReview
NativeQuerySnippet
Permissions
@@ -99,6 +100,7 @@
CardBookmark
DashboardBookmark
Emitter
+ ModelAction
CollectionBookmark
BookmarkOrdering
DashboardCard
diff --git a/src/metabase/models.clj b/src/metabase/models.clj
index f078a63281ccc6f09666abf59918ba25e9f12a7e..12e0ad9dd55002d9e0bc0798e3a8adef3413028f 100644
--- a/src/metabase/models.clj
+++ b/src/metabase/models.clj
@@ -90,7 +90,7 @@
view-log/keep-me)
(p/import-vars
- [action Action HTTPAction QueryAction]
+ [action Action HTTPAction ModelAction QueryAction]
[activity Activity]
[app App]
[bookmark CardBookmark]
diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj
index a6822f19e49026f4e23ced2e2994517c7d70bf59..7eb1242eecaaab7a810d8bc5707f218a00ab41f2 100644
--- a/src/metabase/models/action.clj
+++ b/src/metabase/models/action.clj
@@ -1,15 +1,20 @@
(ns metabase.models.action
(:require [cheshire.core :as json]
+ [clojure.set :as set]
[medley.core :as m]
[metabase.models.interface :as mi]
+ [metabase.models.query :as query]
+ [metabase.models.serialization.hash :as serdes.hash]
[metabase.util :as u]
[metabase.util.encryption :as encryption]
[toucan.db :as db]
+ [toucan.hydrate :refer [hydrate]]
[toucan.models :as models]))
(models/defmodel QueryAction :query_action)
(models/defmodel HTTPAction :http_action)
(models/defmodel Action :action)
+(models/defmodel ModelAction :model_action)
(models/add-type! ::json-with-nested-parameters
:in (comp mi/json-in
@@ -52,6 +57,16 @@
(merge Action-subtype-IModel-impl
{:types (constantly {:template ::json-with-nested-parameters})}))
+(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class ModelAction)
+ models/IModel
+ (merge models/IModelDefaults
+ {:properties (constantly {:entity_id true})
+ :types (constantly {:parameter_mappings :parameters-list
+ :visualization_settings :visualization-settings})})
+
+ serdes.hash/IdentityHashable
+ {:identity-hash-fields [:entity_id]})
+
(defn insert!
"Inserts an Action and related HTTPAction or QueryAction. Returns the action id."
[action-data]
@@ -152,12 +167,83 @@
(m/assoc-some card :action_id (get card-id->action-id (:id card))))
cards))
+(defn unique-field-slugs?
+ "Makes sure that if `coll` is indexed by `index-by`, no keys will be in conflict."
+ [fields]
+ (empty? (m/filter-vals #(not= % 1) (frequencies (map (comp u/slugify :name) fields)))))
+
+(defn- implicit-action-parameters
+ [cards]
+ (let [card-id-by-table-id (into {}
+ (for [card cards
+ :let [{:keys [table-id]} (query/query->database-and-table-ids (:dataset_query card))]
+ :when table-id]
+ [table-id (:id card)]))
+ tables (when-let [table-ids (seq (keys card-id-by-table-id))]
+ (hydrate (db/select 'Table :id [:in table-ids]) :fields))]
+ (into {}
+ (for [table tables
+ :let [fields (:fields table)]
+ ;; Skip tables for have conflicting slugified columns i.e. table has "name" and "NAME" columns.
+ :when (unique-field-slugs? fields)
+ :let [parameters (->> fields
+ (map (fn [field]
+ {:id (u/slugify (:name field))
+ :target [:variable [:template-tag (u/slugify (:name field))]]
+ :type (:base_type field)
+ ::pk? (isa? (:semantic_type field) :type/PK)})))]]
+ [(get card-id-by-table-id (:id table)) parameters]))))
+
+(defn merged-model-action
+ "Find model-actions given options and merge in the referenced action or generate implicit parameters for execution.
+ The goal is to generally hide the existence of model-action and be able to treat this merged information as an action.
+
+ Pass in known-models to save a second Card lookup."
+ [known-models & options]
+ (let [model-actions (apply db/select ModelAction {:order-by [:id]} options)
+ model-action-by-model-slug (m/index-by (juxt :card_id :slug)
+ model-actions)
+ actions-by-id (when-let [action-ids (not-empty (keep :action_id model-actions))]
+ (m/index-by :id (select-actions :id [:in action-ids])))
+ model-ids-with-implicit-actions (set (map :card_id (remove :action_id model-actions)))
+ models-with-implicit-actions (if known-models
+ (->> known-models
+ (filter #(contains? model-ids-with-implicit-actions (:id %)))
+ distinct)
+ (when (seq model-ids-with-implicit-actions)
+ (db/select 'Card :id [:in model-ids-with-implicit-actions])))
+ parameters-by-model-id (when (seq models-with-implicit-actions)
+ (implicit-action-parameters models-with-implicit-actions))]
+ (for [model-action model-actions
+ :let [model-slug [(:card_id model-action) (:slug model-action)]
+ model-action (get model-action-by-model-slug model-slug)
+ action (get actions-by-id (:action_id model-action))
+ implicit-action (when-let [parameters (get parameters-by-model-id (:card_id model-action))]
+ {:parameters (cond->> parameters
+ (not (:requires_pk model-action)) (remove #(::pk? %))
+ (= "delete" (:slug model-action)) (filter #(::pk? %))
+ :always (map #(dissoc % ::pk?)))
+ :type "implicit"})]]
+ (m/deep-merge (-> model-action
+ (select-keys [:card_id :slug :action_id :visualization_settings :parameter_mappings])
+ (set/rename-keys {:card_id :model_id}))
+ implicit-action
+ action))))
+
(defn dashcard-action
"Hydrates action from DashboardCard"
{:batched-hydrate :dashcard/action}
[dashcards]
- (if-let [action-ids (not-empty (keep :action_id dashcards))]
- (let [actions-by-id (m/index-by :id (select-actions :id [:in action-ids]))]
- (for [dashcard dashcards]
- (m/assoc-some dashcard :action (get actions-by-id (:action_id dashcard)))))
- dashcards))
+ (let [model-slug-by-dashcard-id (->> dashcards
+ (keep (fn [dashcard]
+ (when-let [slug (get-in dashcard [:visualization_settings :action_slug])]
+ [(:id dashcard) [(:card_id dashcard) slug]])))
+ (into {})
+ not-empty)
+ actions (when model-slug-by-dashcard-id
+ (merged-model-action (map :card dashcards) [:card_id :slug] [:in (vals model-slug-by-dashcard-id)]))
+ action-by-model-slug (m/index-by (juxt :model_id :slug) actions)]
+ (for [dashcard dashcards]
+ (if-let [model-slug (get model-slug-by-dashcard-id (:id dashcard))]
+ (m/assoc-some dashcard :action (get action-by-model-slug model-slug))
+ dashcard))))
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index 56581c2374c794610e2a29aa8a13cb54e69fb1bf..d9ebacf95365dbd2ce804f1c6e4363cad6de5ddf 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -137,19 +137,24 @@
(s/defn update-dashboard-card!
"Update an existing DashboardCard` including all DashboardCardSeries.
Returns the updated DashboardCard or throws an Exception."
- [{:keys [id action_id parameter_mappings visualization_settings] :as dashboard-card} :- DashboardCardUpdates]
+ [{:keys [id card_id action_id parameter_mappings visualization_settings] :as dashboard-card} :- DashboardCardUpdates]
(let [{:keys [size_x size_y row col series]} (merge {:series []} dashboard-card)]
(db/transaction
;; update the dashcard itself (positional attributes)
(when (and size_x size_y row col)
(db/update-non-nil-keys! DashboardCard id
- :action_id action_id
- :size_x size_x
- :size_y size_y
- :row row
- :col col
- :parameter_mappings parameter_mappings
- :visualization_settings visualization_settings))
+ (cond->
+ {:action_id action_id
+ :size_x size_x
+ :size_y size_y
+ :row row
+ :col col
+ :parameter_mappings parameter_mappings
+ :visualization_settings visualization_settings}
+ ;; Allow changing card for model_actions
+ ;; This is to preserve the existing behavior of questions and card_id
+ ;; I don't know why card_id couldn't be changed for questions though.
+ (:action_slug visualization_settings) (assoc :card_id card_id))))
;; update series (only if they changed)
(when-not (= series (map :card_id (db/select [DashboardCardSeries :card_id]
:dashboardcard_id id
diff --git a/test/metabase/api/action_test.clj b/test/metabase/api/action_test.clj
index 7c83a7c8103407028e3d13b966654f89cad18c37..79ede267c6b9dfa471024bf9b86eb404d1cb6681 100644
--- a/test/metabase/api/action_test.clj
+++ b/test/metabase/api/action_test.clj
@@ -4,6 +4,7 @@
[metabase.actions.test-util :as actions.test-util]
[metabase.api.action :as api.action]
[metabase.driver :as driver]
+ [metabase.models :refer [Card ModelAction]]
[metabase.models.action :refer [Action]]
[metabase.models.database :refer [Database]]
[metabase.models.table :refer [Table]]
@@ -33,17 +34,29 @@
(testing "GET /api/action"
(actions.test-util/with-actions-enabled
(actions.test-util/with-action [{:keys [action-id]} {}]
- (let [response (mt/user-http-request :crowberto :get 200 "action")]
- (is (schema= [{:id su/IntGreaterThanZero
- s/Keyword s/Any}]
- response))
- (let [action (some (fn [action]
- (when (= (:id action) action-id)
- action))
- response)]
- (testing "Should return Card dataset_query deserialized (#23201)"
- (is (schema= ExpectedGetCardActionAPIResponse
- action)))))))))
+ (mt/with-temp* [Card [{card-id :id} {:dataset true :dataset_query (mt/mbql-query categories)}]
+ ModelAction [_ {:card_id card-id :slug "custom" :action_id action-id}]
+ ModelAction [_ {:card_id card-id :slug "insert"}]
+ ModelAction [_ {:card_id card-id :slug "update" :requires_pk true}]
+ ModelAction [_ {:card_id card-id :slug "delete" :requires_pk true}]]
+ (let [response (mt/user-http-request :crowberto :get 200 (str "action?model-id=" card-id))]
+ (is (partial= [{:slug "custom"
+ :action_id action-id
+ :parameters [{:id "id"} {:id "name"}]
+ :card {:is_write true}
+ :type "query"
+ :name "Query Example"}
+ {:slug "insert" :action_id nil :parameters [{:id "name"}] :type "implicit"}
+ {:slug "update" :action_id nil :parameters [{:id "id"} {:id "name"}] :type "implicit"}
+ {:slug "delete" :action_id nil :parameters [{:id "id"}] :type "implicit"}]
+ response))
+ (let [action (some (fn [action]
+ (when (= (:id action) action-id)
+ action))
+ response)]
+ (testing "Should return Card dataset_query deserialized (#23201)"
+ (is (schema= ExpectedGetCardActionAPIResponse
+ action))))))))))
(deftest get-action-test
(testing "GET /api/action/:id"
@@ -297,55 +310,57 @@
created-action (mt/user-http-request :crowberto :post 200 "action" initial-action)
updated-action (merge initial-action {:name "New name"})
action-path (str "action/" (:id created-action))]
- (testing "Create"
- (is (partial= initial-action created-action)))
- (testing "Validate POST"
- (testing "Required fields"
- (is (partial= {:errors {:type "Only http actions are supported at this time."}}
- (mt/user-http-request :crowberto :post 400 "action" {:type "query"})))
- (is (partial= {:errors {:name "value must be a string."}}
- (mt/user-http-request :crowberto :post 400 "action" {:type "http"})))
- (is (partial= {:errors {:template "value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
- (mt/user-http-request :crowberto :post 400 "action" {:type "http" :name "test"}))))
- (testing "Template needs method and url"
- (is (partial= {:errors {:template "value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
- (mt/user-http-request :crowberto :post 400 "action" {:type "http" :name "Test" :template {}}))))
- (testing "Template parameters should be well formed"
- (is (partial= {:errors {:template "value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
- (mt/user-http-request :crowberto :post 400 "action" {:type "http"
- :name "Test"
- :template {:url "https://example.com"
- :method "GET"
- :parameters {}}}))))
- (testing "Handles need to be valid jq"
- (is (partial= {:errors {:response_handle "value may be nil, or if non-nil, must be a valid json-query"}}
- (mt/user-http-request :crowberto :post 400 "action" (assoc initial-action :response_handle "body"))))
- (is (partial= {:errors {:error_handle "value may be nil, or if non-nil, must be a valid json-query"}}
- (mt/user-http-request :crowberto :post 400 "action" (assoc initial-action :error_handle "x"))))))
- (testing "Update"
- (is (partial= updated-action
- (mt/user-http-request :crowberto :put 200 action-path
- {:name "New name" :type "http"}))))
- (testing "Get"
- (is (partial= updated-action
- (mt/user-http-request :crowberto :get 200 action-path)))
- (is (partial= updated-action
- (last (mt/user-http-request :crowberto :get 200 "action")))))
- (testing "Validate PUT"
- (testing "Can't create or change http type"
- (is (partial= {:errors {:type "Only http actions are supported at this time."}}
- (mt/user-http-request :crowberto :put 400 action-path {:type "query"}))))
- (testing "Template needs method and url"
- (is (partial= {:errors {:template "value may be nil, or if non-nil, value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
- (mt/user-http-request :crowberto :put 400 action-path {:type "http" :template {}}))))
- (testing "Handles need to be valid jq"
- (is (partial= {:errors {:response_handle "value may be nil, or if non-nil, must be a valid json-query"}}
- (mt/user-http-request :crowberto :put 400 action-path (assoc initial-action :response_handle "body"))))
- (is (partial= {:errors {:error_handle "value may be nil, or if non-nil, must be a valid json-query"}}
- (mt/user-http-request :crowberto :put 400 action-path (assoc initial-action :error_handle "x"))))))
- (testing "Delete"
- (is (nil? (mt/user-http-request :crowberto :delete 204 action-path)))
- (is (= "Not found." (mt/user-http-request :crowberto :get 404 action-path))))))))
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:card_id card-id :action_id (:id created-action) :slug "action"}]]
+ (testing "Create"
+ (is (partial= initial-action created-action)))
+ (testing "Validate POST"
+ (testing "Required fields"
+ (is (partial= {:errors {:type "Only http actions are supported at this time."}}
+ (mt/user-http-request :crowberto :post 400 "action" {:type "query"})))
+ (is (partial= {:errors {:name "value must be a string."}}
+ (mt/user-http-request :crowberto :post 400 "action" {:type "http"})))
+ (is (partial= {:errors {:template "value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
+ (mt/user-http-request :crowberto :post 400 "action" {:type "http" :name "test"}))))
+ (testing "Template needs method and url"
+ (is (partial= {:errors {:template "value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
+ (mt/user-http-request :crowberto :post 400 "action" {:type "http" :name "Test" :template {}}))))
+ (testing "Template parameters should be well formed"
+ (is (partial= {:errors {:template "value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
+ (mt/user-http-request :crowberto :post 400 "action" {:type "http"
+ :name "Test"
+ :template {:url "https://example.com"
+ :method "GET"
+ :parameters {}}}))))
+ (testing "Handles need to be valid jq"
+ (is (partial= {:errors {:response_handle "value may be nil, or if non-nil, must be a valid json-query"}}
+ (mt/user-http-request :crowberto :post 400 "action" (assoc initial-action :response_handle "body"))))
+ (is (partial= {:errors {:error_handle "value may be nil, or if non-nil, must be a valid json-query"}}
+ (mt/user-http-request :crowberto :post 400 "action" (assoc initial-action :error_handle "x"))))))
+ (testing "Update"
+ (is (partial= updated-action
+ (mt/user-http-request :crowberto :put 200 action-path
+ {:name "New name" :type "http"}))))
+ (testing "Get"
+ (is (partial= updated-action
+ (mt/user-http-request :crowberto :get 200 action-path)))
+ (is (partial= updated-action
+ (last (mt/user-http-request :crowberto :get 200 (str "action?model-id=" card-id))))))
+ (testing "Validate PUT"
+ (testing "Can't create or change http type"
+ (is (partial= {:errors {:type "Only http actions are supported at this time."}}
+ (mt/user-http-request :crowberto :put 400 action-path {:type "query"}))))
+ (testing "Template needs method and url"
+ (is (partial= {:errors {:template "value may be nil, or if non-nil, value must be a map with schema: (\n body (optional) : value may be nil, or if non-nil, value must be a string.\n headers (optional) : value may be nil, or if non-nil, value must be a string.\n parameter_mappings (optional) : value may be nil, or if non-nil, value must be a map.\n parameters (optional) : value may be nil, or if non-nil, value must be an array. Each value must be a map.\n method : value must be one of: `DELETE`, `GET`, `PATCH`, `POST`, `PUT`.\n url : value must be a string.\n)"}}
+ (mt/user-http-request :crowberto :put 400 action-path {:type "http" :template {}}))))
+ (testing "Handles need to be valid jq"
+ (is (partial= {:errors {:response_handle "value may be nil, or if non-nil, must be a valid json-query"}}
+ (mt/user-http-request :crowberto :put 400 action-path (assoc initial-action :response_handle "body"))))
+ (is (partial= {:errors {:error_handle "value may be nil, or if non-nil, must be a valid json-query"}}
+ (mt/user-http-request :crowberto :put 400 action-path (assoc initial-action :error_handle "x"))))))
+ (testing "Delete"
+ (is (nil? (mt/user-http-request :crowberto :delete 204 action-path)))
+ (is (= "Not found." (mt/user-http-request :crowberto :get 404 action-path)))))))))
(deftest bulk-create-happy-path-test
(testing "POST /api/action/bulk/create/:table-id"
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index af542bd30bf6b21e88579d94fa054f755a618bca..0888b3b6ef866717fb1d94ff3b7100c1cad818d5 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -20,6 +20,7 @@
Emitter
Field
FieldValues
+ ModelAction
PermissionsGroup
PermissionsGroupMembership
Pulse
@@ -1049,6 +1050,28 @@
:updated_at true}
(remove-ids-and-booleanize-timestamps (dashboard-card/retrieve-dashboard-card dashcard-id-2))))))))
+(deftest update-action-cards-test
+ (actions.test-util/with-actions-enabled
+ (testing "PUT /api/dashboard/:id/cards"
+ ;; fetch a dashboard WITH a dashboard card on it
+ (mt/with-temp* [Dashboard [{dashboard-id :id}]
+ Card [{model-id :id} {:dataset true}]
+ Card [{model-id-2 :id} {:dataset true}]
+ ModelAction [_ {:card_id model-id :slug "insert"}]
+ ModelAction [_ {:card_id model-id-2 :slug "insert"}]
+ DashboardCard [action-card {:dashboard_id dashboard-id,
+ :card_id model-id
+ :visualization_settings {:action_slug "insert"}}]
+ DashboardCard [question-card {:dashboard_id dashboard-id, :card_id model-id}]]
+ (with-dashboards-in-writeable-collection [dashboard-id]
+ (is (= {:status "ok"}
+ (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id)
+ {:cards [(assoc action-card :card_id model-id-2)
+ (assoc question-card :card_id model-id-2)]})))
+ ;; Only action card should change
+ (is (partial= [{:card_id model-id-2}
+ {:card_id model-id}]
+ (db/select DashboardCard :dashboard_id dashboard-id {:order-by [:id]}))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GET /api/dashboard/:id/revisions |
@@ -1857,19 +1880,16 @@
(actions.test-util/with-actions-test-data-and-actions-enabled
(actions.test-util/with-action [{:keys [action-id]} {}]
(testing "Creating dashcard with action"
- (mt/with-temp* [Dashboard [{dashboard-id :id}]]
- (is (partial= {:action_id action-id}
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:card_id card-id :action_id action-id :slug "insert" :visualization_settings {:hello true}}]
+ Dashboard [{dashboard-id :id}]]
+ (is (partial= {:visualization_settings {:action_slug "insert"}}
(mt/user-http-request :crowberto :post 200 (format "dashboard/%s/cards" dashboard-id)
- {:size_x 1 :size_y 1 :row 1 :col 1 :action_id action-id})))
- (is (partial= {:ordered_cards [{:action_id action-id :action {:id action-id}}]}
- (mt/user-http-request :crowberto :get 200 (format "dashboard/%s" dashboard-id))))))
- (testing "Updating dashcard action"
- (mt/with-temp* [Dashboard [{dashboard-id :id}]
- DashboardCard [dashcard {:dashboard_id dashboard-id}]]
- (is (partial= {:status "ok"}
- (mt/user-http-request :crowberto :put 200 (format "dashboard/%s/cards" dashboard-id)
- {:cards [(assoc dashcard :action_id action-id)]})))
- (is (partial= {:ordered_cards [{:action_id action-id :action {:id action-id}}]}
+ {:size_x 1 :size_y 1 :row 1 :col 1 :cardId card-id
+ :visualization_settings {:action_slug "insert"}})))
+ (is (partial= {:ordered_cards [{:action {:visualization_settings {:hello true :inline true}
+ :parameters []
+ :slug "insert"}}]}
(mt/user-http-request :crowberto :get 200 (format "dashboard/%s" dashboard-id))))))))))
(deftest dashcard-query-action-execution-test
@@ -1877,117 +1897,183 @@
(actions.test-util/with-actions-test-data-and-actions-enabled
(actions.test-util/with-action [{:keys [action-id]} {}]
(testing "Executing dashcard with action"
- (mt/with-temp* [Dashboard [{dashboard-id :id}]
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:slug "custom" :card_id card-id :action_id action-id}]
+ Dashboard [{dashboard-id :id}]
DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
- :action_id action-id
- :parameter_mappings [{:parameter_id "my_id"
- :target [:variable [:template-tag "id"]]}]}]]
- (let [execute-path (format "dashboard/%s/dashcard/%s/action/execute"
+ :card_id card-id}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/custom"
dashboard-id
dashcard-id)]
(testing "Dashcard parameter"
(is (partial= {:rows-affected 1}
(mt/user-http-request :crowberto :post 200 execute-path
- {:parameters [{:id "my_id" :type "id" :value 1}]})))
+ {:parameters {"id" 1}})))
(is (= [1 "Shop"]
(mt/first-row
(mt/run-mbql-query categories {:filter [:= $id 1]})))))
(testing "Extra target parameter"
(is (partial= {:rows-affected 1}
(mt/user-http-request :crowberto :post 200 execute-path
- {:parameters [{:id "my_id" :type "id" :value 1}]
- :extra_parameters [{:target [:variable [:template-tag "name"]]
- :type "text"
- :value "Bird"}]})))
+ {:parameters {"id" 1 "name" "Bird"}})))
(is (= [1 "Bird Shop"]
(mt/first-row
(mt/run-mbql-query categories {:filter [:= $id 1]})))))
(testing "Should affect 0 rows if id is out of range"
(is (= {:rows-affected 0}
(mt/user-http-request :crowberto :post 200 execute-path
- {:parameters [{:id "my_id" :type :number/= :value Integer/MAX_VALUE}]}))))
+ {:parameters {"id" Integer/MAX_VALUE}}))))
(testing "Should 404 if bad dashcard-id"
(is (= "Not found."
- (mt/user-http-request :crowberto :post 404 (format "dashboard/%d/dashcard/%s/action/execute"
+ (mt/user-http-request :crowberto :post 404 (format "dashboard/%d/dashcard/%s/execute/custom"
dashboard-id
Integer/MAX_VALUE)
{}))))
(testing "Missing parameter should fail gracefully"
(is (partial= {:message "Error executing Action: Error building query parameter map: Error determining value for parameter \"id\": You'll need to pick a value for 'ID' before this query can run."}
(mt/user-http-request :crowberto :post 500 execute-path
- {:parameters []}))))
+ {:parameters {}}))))
(testing "Sending an invalid number should fail gracefully"
(is (partial= {:message "Error executing Action: Error building query parameter map: Error determining value for parameter \"id\": Unparseable number: \"BAD\""}
(mt/user-http-request :crowberto :post 500 execute-path
- {:parameters [{:id "my_id" :type :number/= :value "BAD"}]})))))))))))
+ {:parameters {"id" "BAD"}})))))))))))
(deftest dashcard-http-action-execution-test
(mt/test-drivers (mt/normal-drivers-with-feature :actions)
(actions.test-util/with-actions-test-data-and-actions-enabled
(actions.test-util/with-action [{:keys [action-id]} {:type :http}]
(testing "Executing dashcard with action"
- (mt/with-temp* [Dashboard [{dashboard-id :id}]
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:slug "custom" :card_id card-id :action_id action-id}]
+ Dashboard [{dashboard-id :id}]
DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
- :action_id action-id
- :parameter_mappings [{:parameter_id "my_id"
- :target [:template-tag "id"]}
- {:parameter_id "my_fail"
- :target [:template-tag "fail"]}]}]]
- (let [execute-path (format "dashboard/%s/dashcard/%s/action/execute"
+ :card_id card-id}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/custom"
dashboard-id
dashcard-id)]
(testing "Should be able to execute an emitter"
(is (= {:the_parameter 1}
(mt/user-http-request :crowberto :post 200 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]}))))
+ {:parameters {"id" 1}}))))
(testing "Should handle errors"
(is (= {:remote-status 400}
(mt/user-http-request :crowberto :post 400 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}
- {:id "my_fail" :type :text :value "true"}]}))))
+ {:parameters {"id" 1 "fail" "true"}}))))
(testing "Extra parameter should fail gracefully"
- (is (partial= {:message "No parameter mapping found for parameter \"extra\". Found: #{\"my_id\" \"my_fail\"}"}
+ (is (partial= {:message "No destination parameter found for id \"extra\". Found: #{\"id\" \"fail\"}"}
(mt/user-http-request :crowberto :post 400 execute-path
- {:parameters [{:id "extra" :type :number/= :value 1}]}))))
+ {:parameters {"extra" 1}}))))
(testing "Missing parameter should fail gracefully"
(is (partial= {:message "Problem building request: Cannot call the service: missing required parameters: #{\"id\"}"}
(mt/user-http-request :crowberto :post 500 execute-path
- {:parameters []}))))
+ {:parameters {}}))))
(testing "Sending an invalid number should fail gracefully"
(is (str/starts-with? (:message (mt/user-http-request :crowberto :post 500 execute-path
- {:parameters [{:id "my_id" :type :number/= :value "BAD"}]}))
+ {:parameters {"id" "BAD"}}))
"Problem building request:"))))))))))
+(deftest dashcard-implicit-action-execution-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :actions)
+ (actions.test-util/with-actions-test-data-and-actions-enabled
+ (testing "Executing dashcard insert"
+ (mt/with-temp* [Card [{card-id :id} {:dataset true :dataset_query (mt/mbql-query categories)}]
+ ModelAction [_ {:slug "insert" :card_id card-id :requires_pk false}]
+ Dashboard [{dashboard-id :id}]
+ DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
+ :card_id card-id
+ :visualization_settings {:action_slug "insert"}}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/insert" dashboard-id dashcard-id)
+ new-row (-> (mt/user-http-request :crowberto :post 200 execute-path
+ {:parameters {"name" "Birds"}})
+ :created-row
+ (update-keys (comp keyword str/lower-case name)))]
+ (testing "Should be able to insert"
+ (is (pos? (:id new-row)))
+ (is (partial= {:name "Birds"}
+ new-row)))
+ (testing "Extra parameter should fail gracefully"
+ (is (partial= {:message "No destination parameter found for #{\"extra\"}. Found: #{\"id\" \"name\"}"}
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {"extra" 1}}))))
+ (testing "Missing other parameters should fail gracefully"
+ (is (partial= "Implicit parameters must be provided."
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {}})))))))
+ (testing "Executing dashcard update"
+ (mt/with-temp* [Card [{card-id :id} {:dataset true :dataset_query (mt/mbql-query categories)}]
+ ModelAction [_ {:slug "update" :card_id card-id :requires_pk true}]
+ Dashboard [{dashboard-id :id}]
+ DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
+ :card_id card-id
+ :visualization_settings {:action_slug "update"}}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/update" dashboard-id dashcard-id)]
+ (testing "Should be able to update"
+ (is (= {:rows-updated [1]}
+ (mt/user-http-request :crowberto :post 200 execute-path
+ {:parameters {"id" 1 "name" "Birds"}}))))
+ (testing "Extra parameter should fail gracefully"
+ (is (partial= {:message "No destination parameter found for #{\"extra\"}. Found: #{\"id\" \"name\"}"}
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {"extra" 1}}))))
+ (testing "Missing pk parameter should fail gracefully"
+ (is (partial= "Missing primary key parameter: \"id\""
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {"name" "Birds"}}))))
+ (testing "Missing other parameters should fail gracefully"
+ (is (partial= "Implicit parameters must be provided."
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {"id" 1}})))))))
+ (testing "Executing dashcard delete"
+ (mt/with-temp* [Card [{card-id :id} {:dataset true :dataset_query (mt/mbql-query categories)}]
+ ModelAction [_ {:slug "delete" :card_id card-id :requires_pk true}]
+ Dashboard [{dashboard-id :id}]
+ DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
+ :card_id card-id
+ :visualization_settings {:action_slug "delete"}}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/delete" dashboard-id dashcard-id)]
+ (testing "Should be able to delete"
+ (is (= {:rows-deleted [1]}
+ (mt/user-http-request :crowberto :post 200 execute-path
+ {:parameters {"id" 1}}))))
+ (testing "Extra parameter should fail gracefully"
+ (is (partial= {:message "No destination parameter found for #{\"extra\"}. Found: #{\"id\" \"name\"}"}
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {"extra" 1}}))))
+ (testing "Missing pk parameter should fail gracefully"
+ (is (partial= "Missing primary key parameter: \"id\""
+ (mt/user-http-request :crowberto :post 400 execute-path
+ {:parameters {"name" "Birds"}}))))))))))
+
(deftest dashcard-action-execution-auth-test
(mt/with-temp-copy-of-db
(actions.test-util/with-actions-test-data
(actions.test-util/with-action [{:keys [action-id]} {}]
(testing "Executing dashcard with action"
- (mt/with-temp* [Dashboard [{dashboard-id :id}]
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:slug "custom" :card_id card-id :action_id action-id}]
+ Dashboard [{dashboard-id :id}]
DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
- :action_id action-id
- :parameter_mappings [{:parameter_id "my_id"
- :target [:variable [:template-tag "id"]]}]}]]
- (let [execute-path (format "dashboard/%s/dashcard/%s/action/execute"
+ :card_id card-id}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/custom"
dashboard-id
dashcard-id)]
(testing "Without actions enabled"
(is (= "Actions are not enabled."
(mt/user-http-request :crowberto :post 400 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]}))))
+ {:parameters {"id" 1}}))))
(testing "Without execute rights on the DB"
(actions.test-util/with-actions-enabled
(is (= "You don't have permissions to do that."
(mt/user-http-request :rasta :post 403 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]})))))
+ {:parameters {"id" 1}})))))
(testing "With execute rights on the DB"
(perms/update-global-execution-permission! (:id (perms-group/all-users)) :all)
(try
(actions.test-util/with-actions-enabled
(is (= {:rows-affected 1}
(mt/user-http-request :rasta :post 200 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]}))))
+ {:parameters {"id" 1}}))))
(finally
(perms/update-global-execution-permission! (:id (perms-group/all-users)) :none)))))))))))
@@ -2004,12 +2090,13 @@
(actions.test-util/with-action [{:keys [action-id]} {}]
(testing "Executing dashcard with action"
(mt/with-temp* [Dashboard [{dashboard-id :id}]
+ Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:action_id action-id :card_id card-id :slug "custom"}]
DashboardCard [{dashcard-id :id}
{:dashboard_id dashboard-id
- :action_id action-id
- :parameter_mappings [{:parameter_id "my_id"
- :target [:variable [:template-tag "id"]]}]}]]
- (let [execute-path (format "dashboard/%s/dashcard/%s/action/execute"
+ :card_id card-id
+ :visualization_settings {:action_slug "custom"}}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute/custom"
dashboard-id
dashcard-id)]
(testing "with :advanced-permissions feature flag"
@@ -2020,7 +2107,7 @@
:group_id group-id}]]
(is (= "You don't have permissions to do that."
(mt/user-http-request :rasta :post 403 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]}))
+ {:parameters {"id" 1}}))
"Execution permission should be required")
(mt/user-http-request
@@ -2031,7 +2118,7 @@
"Should be able to set execution permission")
(is (= {:rows-affected 1}
(mt/user-http-request :rasta :post 200 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]}))
+ {:parameters {"id" 1}}))
"Execution and data permissions should be enough")
(perms/update-data-perms-graph! [group-id (mt/id) :data]
@@ -2040,5 +2127,5 @@
{:schemas :block})
(is (= "You don't have permissions to do that."
(mt/user-http-request :rasta :post 403 execute-path
- {:parameters [{:id "my_id" :type :number/= :value 1}]}))
+ {:parameters {"id" 1}}))
"Data permissions should be required"))))))))))))))
diff --git a/test/metabase/api/model_action_test.clj b/test/metabase/api/model_action_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..67fd1ae1f706f3a62021c17faa30e1e1e6dcd8ab
--- /dev/null
+++ b/test/metabase/api/model_action_test.clj
@@ -0,0 +1,53 @@
+(ns metabase.api.model-action-test
+ (:require
+ [clojure.test :refer [deftest is testing]]
+ [metabase.actions.test-util :as actions.test-util]
+ [metabase.models :refer [Card ModelAction]]
+ [metabase.test :as mt]
+ [toucan.db :as db]))
+
+(deftest get-test
+ (actions.test-util/with-actions-enabled
+ (actions.test-util/with-action [{:keys [action-id]} {}]
+ (actions.test-util/with-action [{http-action-id :action-id} {:name "HTTP Example"}]
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [_ {:card_id card-id :action_id action-id :slug "query"}]
+ ModelAction [_ {:card_id card-id :action_id http-action-id :slug "http"}]
+ ModelAction [_ {:card_id card-id :slug "implicit"}]]
+ (let [response (mt/user-http-request :crowberto :get 200 (str "model-action?card-id=" card-id))]
+ (is (partial=
+ [{:slug "query" :name "Query Example"}
+ {:slug "http" :name "HTTP Example"}
+ {:slug "implicit" :name nil}]
+ response))))))))
+
+(deftest post-test
+ (actions.test-util/with-actions-enabled
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]]
+ (testing "With implicit action"
+ (let [response (mt/user-http-request :crowberto :post 200 "model-action"
+ {:card_id card-id
+ :requires_pk false
+ :slug "insert"})]
+ (is (partial=
+ {:slug "insert"}
+ response))))
+ (testing "With custom action"
+ (actions.test-util/with-action [{:keys [action-id]} {}]
+ (let [response (mt/user-http-request :crowberto :post 200 "model-action"
+ {:card_id card-id
+ :action_id action-id
+ :requires_pk false
+ :slug "custom"})]
+ (is (partial=
+ {:slug "custom"}
+ response))))))))
+
+(deftest put-test
+ (actions.test-util/with-actions-enabled
+ (actions.test-util/with-action [{:keys [action-id]} {}]
+ (mt/with-temp* [Card [{card-id :id} {:dataset true}]
+ ModelAction [{model-action-id :id} {:card_id card-id :slug "implicit"}]]
+ (mt/user-http-request :crowberto :put 204 (str "model-action/" model-action-id)
+ {:action_id action-id})
+ (is (= action-id (db/select-one-field :action_id ModelAction :id model-action-id)))))))