From 37fb6200638b03e5fc983aaa78c1cea2dbd92c6b Mon Sep 17 00:00:00 2001
From: Alexander Lesnenko <alxnddr@users.noreply.github.com>
Date: Mon, 11 Apr 2022 04:32:59 -0400
Subject: [PATCH] Hide subscriptions and alerts editing buttons in account
settings (#21541)
* Hide subscriptions and alerts editing buttons in account settings
* spec
* specs
---
.../NotificationCard/NotificationCard.jsx | 15 ++++--
.../NotificationCard.unit.spec.js | 23 +++++++++
.../NotificationList/NotificationList.jsx | 3 ++
.../NotificationsApp/NotificationsApp.jsx | 7 ++-
.../general-permissions.cy.spec.js | 50 +++++++++++++++++--
5 files changed, 91 insertions(+), 7 deletions(-)
diff --git a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx
index f25fac3f66e..8cc09258117 100644
--- a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx
+++ b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx
@@ -24,9 +24,17 @@ const propTypes = {
user: PropTypes.object.isRequired,
onUnsubscribe: PropTypes.func,
onArchive: PropTypes.func,
+ isEditable: PropTypes.bool,
};
-const NotificationCard = ({ item, type, user, onUnsubscribe, onArchive }) => {
+const NotificationCard = ({
+ item,
+ type,
+ user,
+ isEditable,
+ onUnsubscribe,
+ onArchive,
+}) => {
const hasArchive = canArchive(item, user);
const onUnsubscribeClick = useCallback(() => {
@@ -54,14 +62,15 @@ const NotificationCard = ({ item, type, user, onUnsubscribe, onArchive }) => {
</NotificationMessage>
</NotificationDescription>
</NotificationContent>
- {!hasArchive && (
+
+ {isEditable && !hasArchive && (
<NotificationIcon
name="close"
tooltip={t`Unsubscribe`}
onClick={onUnsubscribeClick}
/>
)}
- {hasArchive && (
+ {isEditable && hasArchive && (
<NotificationIcon
name="close"
tooltip={t`Delete`}
diff --git a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.unit.spec.js b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.unit.spec.js
index 3f51fde7adb..508d8af8941 100644
--- a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.unit.spec.js
+++ b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.unit.spec.js
@@ -132,6 +132,7 @@ describe("NotificationCard", () => {
user={user}
onUnsubscribe={onUnsubscribe}
onArchive={onArchive}
+ isEditable
/>,
);
@@ -157,6 +158,7 @@ describe("NotificationCard", () => {
user={creator}
onUnsubscribe={onUnsubscribe}
onArchive={onArchive}
+ isEditable
/>,
);
@@ -165,6 +167,25 @@ describe("NotificationCard", () => {
expect(onArchive).not.toHaveBeenCalled();
});
+ it("should hide archive button when not editable", () => {
+ const creator = getUser();
+ const alert = getAlert({ creator });
+ const onUnsubscribe = jest.fn();
+ const onArchive = jest.fn();
+
+ render(
+ <NotificationCard
+ item={alert}
+ type="alert"
+ user={creator}
+ onUnsubscribe={onUnsubscribe}
+ onArchive={onArchive}
+ />,
+ );
+
+ expect(screen.queryByLabelText("close icon")).toBeNull();
+ });
+
it("should archive when the user is the creator and not subscribed", () => {
const creator = getUser();
const alert = getAlert({ creator });
@@ -178,6 +199,7 @@ describe("NotificationCard", () => {
user={creator}
onUnsubscribe={onUnsubscribe}
onArchive={onArchive}
+ isEditable
/>,
);
@@ -202,6 +224,7 @@ describe("NotificationCard", () => {
user={creator}
onUnsubscribe={onUnsubscribe}
onArchive={onArchive}
+ isEditable
/>,
);
diff --git a/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx b/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx
index 70fddb9061f..51aed6332d7 100644
--- a/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx
+++ b/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx
@@ -15,6 +15,7 @@ const propTypes = {
items: PropTypes.array.isRequired,
user: PropTypes.object.isRequired,
children: PropTypes.node,
+ canManageSubscriptions: PropTypes.bool,
onHelp: PropTypes.func,
onUnsubscribe: PropTypes.func,
onArchive: PropTypes.func,
@@ -24,6 +25,7 @@ const NotificationList = ({
items,
user,
children,
+ canManageSubscriptions,
onHelp,
onUnsubscribe,
onArchive,
@@ -46,6 +48,7 @@ const NotificationList = ({
item={item}
type={type}
user={user}
+ isEditable={canManageSubscriptions}
onUnsubscribe={onUnsubscribe}
onArchive={onArchive}
/>
diff --git a/frontend/src/metabase/account/notifications/containers/NotificationsApp/NotificationsApp.jsx b/frontend/src/metabase/account/notifications/containers/NotificationsApp/NotificationsApp.jsx
index e1ed1ed66aa..2b54d9c493b 100644
--- a/frontend/src/metabase/account/notifications/containers/NotificationsApp/NotificationsApp.jsx
+++ b/frontend/src/metabase/account/notifications/containers/NotificationsApp/NotificationsApp.jsx
@@ -2,7 +2,11 @@ import { connect } from "react-redux";
import _ from "underscore";
import Alerts from "metabase/entities/alerts";
import Pulses from "metabase/entities/pulses";
-import { getUser, getUserId } from "metabase/selectors/user";
+import {
+ getUser,
+ getUserId,
+ canManageSubscriptions,
+} from "metabase/selectors/user";
import {
navigateToArchive,
navigateToHelp,
@@ -14,6 +18,7 @@ import NotificationList from "../../components/NotificationList";
const mapStateToProps = (state, props) => ({
user: getUser(state),
items: getNotifications(props),
+ canManageSubscriptions: canManageSubscriptions(state),
});
const mapDispatchToProps = {
diff --git a/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js b/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js
index e5c9f7a3ff9..9b2b00b2012 100644
--- a/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js
+++ b/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js
@@ -4,11 +4,15 @@ import {
describeEE,
modifyPermission,
} from "__support__/e2e/cypress";
+import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database";
+const { ORDERS_ID } = SAMPLE_DATABASE;
const SETTINGS_INDEX = 0;
const MONITORING_INDEX = 1;
const SUBSCRIPTIONS_INDEX = 2;
+const NORMAL_USER_ID = 2;
+
describeEE("scenarios > admin > permissions > general", () => {
beforeEach(() => {
restore();
@@ -30,17 +34,22 @@ describeEE("scenarios > admin > permissions > general", () => {
cy.button("Yes").click();
});
+ createSubscription(NORMAL_USER_ID);
+
cy.signInAsNormalUser();
});
- it("revokes ability to create dashboard subscriptions", () => {
+ it("revokes ability to create subscriptions and alerts and manage them", () => {
cy.visit("/dashboard/1");
cy.icon("subscription").should("not.exist");
- });
- it("revokes ability to create question alerts", () => {
cy.visit("/question/1");
cy.icon("bell").should("not.exist");
+
+ cy.visit("/account/notifications");
+ cy.findByTestId("notifications-list").within(() => {
+ cy.icon("close").should("not.exist");
+ });
});
});
@@ -175,3 +184,38 @@ describeEE("scenarios > admin > permissions > general", () => {
});
});
});
+
+function createSubscription(user_id) {
+ cy.createQuestionAndDashboard({
+ questionDetails: {
+ name: "Test Question",
+ query: {
+ "source-table": ORDERS_ID,
+ },
+ },
+ }).then(({ body: { card_id, dashboard_id } }) => {
+ cy.createPulse({
+ name: "Subscription",
+ dashboard_id,
+ cards: [
+ {
+ id: card_id,
+ include_csv: false,
+ include_xls: false,
+ },
+ ],
+ channels: [
+ {
+ enabled: true,
+ channel_type: "email",
+ schedule_type: "hourly",
+ recipients: [
+ {
+ id: user_id,
+ },
+ ],
+ },
+ ],
+ });
+ });
+}
--
GitLab