diff --git a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx index f25fac3f66e7cc7b1088b97e62abbf342f42a065..8cc09258117f9e21dbdc4c8661e6d412e977b4dc 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 3f51fde7adbeb94d7c88f01019fbebf3384efb64..508d8af894195bf9452c0425ce93553d2cc2804e 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 70fddb9061fbbcb76360ede24de53489309e38d9..51aed6332d7dedd34679eb11f8eee6170a2ef0d7 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 e1ed1ed66aaa99429273a8dd4e375dc916e5ac67..2b54d9c493bb2227f62d275fea34d2c7dbc0f705 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 e5c9f7a3ff9d7bb569af21537c7df475d04160a6..9b2b00b2012ebf7a5d88454138c268399908bba5 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, + }, + ], + }, + ], + }); + }); +}