From e246d6d3dc3a86b5ff0aa4fda539f9071894a63d Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Mon, 20 Feb 2023 15:36:04 +0000
Subject: [PATCH] Allow disabling basic/implicit actions (#28417)
* Allow disabling (deleting) basic actions
* Add confirmation modal
* Update how `menuItems` are built
* Use aria-label instead of a test ID
* Tweak confirmation modal copy
---
.../ModelActionDetails/ModelActionDetails.tsx | 65 ++++++++++++++-----
.../ModelDetailPage.unit.spec.tsx | 38 +++++++++--
.../test/__support__/server-mocks/action.ts | 1 +
.../scenarios/models/model-actions.cy.spec.js | 11 ++++
4 files changed, 91 insertions(+), 24 deletions(-)
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx
index 6cff6b53b03..2f22372c58d 100644
--- a/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx
+++ b/frontend/src/metabase/models/components/ModelDetailPage/ModelActionDetails/ModelActionDetails.tsx
@@ -9,6 +9,7 @@ import Link from "metabase/core/components/Link";
import Actions from "metabase/entities/actions";
import { parseTimestamp } from "metabase/lib/time";
import * as Urls from "metabase/lib/urls";
+import { useConfirmation } from "metabase/hooks/use-confirmation";
import type { Card, WritebackAction } from "metabase-types/api";
import type { Dispatch, State } from "metabase-types/store";
@@ -36,6 +37,7 @@ interface OwnProps {
interface DispatchProps {
onEnableImplicitActions: () => void;
onArchiveAction: (action: WritebackAction) => void;
+ onDeleteAction: (action: WritebackAction) => void;
}
interface ActionsLoaderProps {
@@ -50,6 +52,8 @@ function mapDispatchToProps(dispatch: Dispatch, { model }: OwnProps) {
dispatch(Actions.actions.enableImplicitActionsForModel(model.id())),
onArchiveAction: (action: WritebackAction) =>
dispatch(Actions.objectActions.setArchived(action, true)),
+ onDeleteAction: (action: WritebackAction) =>
+ dispatch(Actions.actions.delete({ id: action.id })),
};
}
@@ -58,26 +62,58 @@ function ModelActionDetails({
actions,
onEnableImplicitActions,
onArchiveAction,
+ onDeleteAction,
}: Props) {
+ const { show: askConfirmation, modalContent: confirmationModal } =
+ useConfirmation();
+
const database = model.database();
const hasActionsEnabled = database != null && database.hasActionsEnabled();
const canWrite = model.canWriteActions();
- const hasImplicitActions = actions.some(action => action.type === "implicit");
const actionsSorted = useMemo(
() => _.sortBy(actions, mostRecentFirst),
[actions],
);
+ const implicitActions = useMemo(
+ () => actions.filter(action => action.type === "implicit"),
+ [actions],
+ );
+
+ const onDeleteImplicitActions = useCallback(() => {
+ askConfirmation({
+ title: t`Disable basic actions?`,
+ message: t`Disabling basic actions will also remove any buttons that use these actions. Are you sure you want to continue?`,
+ confirmButtonText: t`Disable`,
+ onConfirm: () => {
+ implicitActions.forEach(action => {
+ onDeleteAction(action);
+ });
+ },
+ });
+ }, [implicitActions, askConfirmation, onDeleteAction]);
+
const menuItems = useMemo(() => {
- return [
- {
+ const items = [];
+ const hasImplicitActions = implicitActions.length > 0;
+
+ if (hasImplicitActions) {
+ items.push({
+ title: t`Disable basic actions`,
+ icon: "bolt",
+ action: onDeleteImplicitActions,
+ });
+ } else {
+ items.push({
title: t`Create basic actions`,
icon: "bolt",
action: onEnableImplicitActions,
- },
- ];
- }, [onEnableImplicitActions]);
+ });
+ }
+
+ return items;
+ }, [implicitActions, onEnableImplicitActions, onDeleteImplicitActions]);
const renderActionListItem = useCallback(
(action: WritebackAction) => {
@@ -104,13 +140,11 @@ function ModelActionDetails({
{canWrite && (
<ActionsHeader>
<Button as={Link} to={newActionUrl}>{t`New action`}</Button>
- {!hasImplicitActions && (
- <ActionMenu
- triggerIcon="ellipsis"
- items={menuItems}
- triggerProps={ACTION_MENU_TRIGGER_PROPS}
- />
- )}
+ <ActionMenu
+ triggerIcon="ellipsis"
+ items={menuItems}
+ triggerProps={{ "aria-label": t`Actions menu` }}
+ />
</ActionsHeader>
)}
{database && !hasActionsEnabled && (
@@ -128,6 +162,7 @@ function ModelActionDetails({
onCreateClick={onEnableImplicitActions}
/>
)}
+ {confirmationModal}
</Root>
);
}
@@ -160,10 +195,6 @@ function mostRecentFirst(action: WritebackAction) {
return -createdAt.unix();
}
-const ACTION_MENU_TRIGGER_PROPS = {
- "data-testid": "new-action-menu",
-};
-
export default _.compose(
Actions.loadList({
query: (state: State, { model }: OwnProps) => ({
diff --git a/frontend/src/metabase/models/containers/ModelDetailPage/ModelDetailPage.unit.spec.tsx b/frontend/src/metabase/models/containers/ModelDetailPage/ModelDetailPage.unit.spec.tsx
index dfeb9e70870..b381ddad465 100644
--- a/frontend/src/metabase/models/containers/ModelDetailPage/ModelDetailPage.unit.spec.tsx
+++ b/frontend/src/metabase/models/containers/ModelDetailPage/ModelDetailPage.unit.spec.tsx
@@ -23,6 +23,7 @@ import {
import { checkNotNull } from "metabase/core/utils/types";
import { ActionsApi } from "metabase/services";
+import Actions from "metabase/entities/actions";
import Models from "metabase/entities/questions";
import { ModalRoute } from "metabase/hoc/ModalRoute";
import { getMetadata } from "metabase/selectors/metadata";
@@ -605,7 +606,7 @@ describe("ModelDetailPage", () => {
const action = createMockQueryAction({ model_id: model.id() });
await setupActions({ model, actions: [action] });
- userEvent.click(screen.getByTestId("new-action-menu"));
+ userEvent.click(screen.getByLabelText("Actions menu"));
userEvent.click(screen.getByText("Create basic actions"));
await waitFor(() => {
@@ -668,12 +669,11 @@ describe("ModelDetailPage", () => {
actions: createMockImplicitCUDActions(model.id()),
});
+ userEvent.click(screen.getByLabelText("Actions menu"));
+
expect(
screen.queryByText(/Create basic action/i),
).not.toBeInTheDocument();
- expect(
- screen.queryByTestId("new-action-menu"),
- ).not.toBeInTheDocument();
});
it("allows to archive a query action", async () => {
@@ -709,6 +709,32 @@ describe("ModelDetailPage", () => {
expect(screen.queryByText("Archive")).not.toBeInTheDocument();
});
+
+ it("allows to disable implicit actions", async () => {
+ const deleteActionSpy = jest.spyOn(Actions.actions, "delete");
+ const model = getModel();
+ const actions = createMockImplicitCUDActions(model.id());
+ await setupActions({ model, actions });
+
+ userEvent.click(screen.getByLabelText("Actions menu"));
+ userEvent.click(screen.getByText("Disable basic actions"));
+ userEvent.click(screen.getByRole("button", { name: "Disable" }));
+
+ actions.forEach(action => {
+ expect(deleteActionSpy).toHaveBeenCalledWith({ id: action.id });
+ });
+ });
+
+ it("doesn't allow to disable implicit actions if they don't exist", async () => {
+ const model = getModel();
+ await setupActions({ model, actions: [] });
+
+ userEvent.click(screen.getByLabelText("Actions menu"));
+
+ expect(
+ screen.queryByText("Disable basic actions"),
+ ).not.toBeInTheDocument();
+ });
});
describe("read-only permissions", () => {
@@ -750,9 +776,7 @@ describe("ModelDetailPage", () => {
expect(
screen.queryByText("Create basic actions"),
).not.toBeInTheDocument();
- expect(
- screen.queryByTestId("new-action-menu"),
- ).not.toBeInTheDocument();
+ expect(screen.queryByTestId("actions-menu")).not.toBeInTheDocument();
});
it("doesn't allow to edit actions", async () => {
diff --git a/frontend/test/__support__/server-mocks/action.ts b/frontend/test/__support__/server-mocks/action.ts
index 74f1461e7fb..026a8cb6a57 100644
--- a/frontend/test/__support__/server-mocks/action.ts
+++ b/frontend/test/__support__/server-mocks/action.ts
@@ -8,6 +8,7 @@ import {
export function setupActionEndpoints(scope: Scope, action: WritebackAction) {
scope.get(`/api/action/${action.id}`).reply(200, action);
scope.put(`/api/action/${action.id}`).reply(200, action);
+ scope.delete(`/api/action/${action.id}`).reply(200, action);
}
export function setupActionsEndpoints(
diff --git a/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js b/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js
index 33dc6ea3e6a..91d18ed00c4 100644
--- a/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js
+++ b/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js
@@ -130,6 +130,17 @@ describe("scenarios > models > actions", () => {
});
cy.findByRole("listitem", { name: "Delete Order" }).should("not.exist");
+
+ cy.findByLabelText("Actions menu").click();
+ popover().findByText("Disable basic actions").click();
+ modal().within(() => {
+ cy.findByText("Disable basic actions?").should("be.visible");
+ cy.button("Disable").click();
+ });
+ cy.findByLabelText("Action list").should("not.exist");
+ cy.findByText("Create").should("not.exist");
+ cy.findByText("Update").should("not.exist");
+ cy.findByText("Delete").should("not.exist");
});
it("should allow to execute actions from the model page", () => {
--
GitLab