diff --git a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx
index 75ad962a45ba7a9a636e5ecdc7f8e277442cc4f2..dbc36e398b356cb406517aec37948ea56077700a 100644
--- a/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx
+++ b/frontend/src/metabase/account/notifications/components/NotificationCard/NotificationCard.jsx
@@ -19,13 +19,13 @@ import {
const propTypes = {
item: PropTypes.object.isRequired,
type: PropTypes.oneOf(["pulse", "alert"]).isRequired,
- user: PropTypes.object,
+ user: PropTypes.object.isRequired,
onUnsubscribe: PropTypes.func,
onArchive: PropTypes.func,
};
const NotificationCard = ({ item, type, user, onUnsubscribe, onArchive }) => {
- const hasSubscribed = isSubscribed(item, user);
+ const hasArchive = canArchive(item, user);
const onUnsubscribeClick = useCallback(() => {
onUnsubscribe(item, type);
@@ -45,14 +45,14 @@ const NotificationCard = ({ item, type, user, onUnsubscribe, onArchive }) => {
{formatDescription(item, user)}
</NotificationDescription>
</NotificationContent>
- {hasSubscribed && (
+ {!hasArchive && (
<NotificationIcon
name="close"
tooltip={t`Unsubscribe`}
onClick={onUnsubscribeClick}
/>
)}
- {!hasSubscribed && (
+ {hasArchive && (
<NotificationIcon
name="close"
tooltip={t`Delete`}
@@ -65,10 +65,16 @@ const NotificationCard = ({ item, type, user, onUnsubscribe, onArchive }) => {
NotificationCard.propTypes = propTypes;
-const isSubscribed = (item, user) => {
- return item.channels.some(channel =>
- channel.recipients.some(recipient => recipient.id === user.id),
+const canArchive = (item, user) => {
+ const recipients = item.channels.flatMap(channel =>
+ channel.recipients.map(recipient => recipient.id),
);
+
+ const isCreator = item.creator?.id === user.id;
+ const isSubscribed = recipients.includes(user.id);
+ const isOnlyRecipient = recipients.length === 1;
+
+ return isCreator && (!isSubscribed || isOnlyRecipient);
};
const formatTitle = (item, type) => {
@@ -156,7 +162,7 @@ const formatCreator = (item, user) => {
let creatorString = "";
const options = Settings.formattingOptions();
- if (user?.id === item.creator?.id) {
+ if (user.id === item.creator?.id) {
creatorString += t`Created by you`;
} else if (item.creator?.common_name) {
creatorString += t`Created by ${item.creator.common_name}`;
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 550511f8116e3f6e313f7219c4172fae84367f78..c26cf0dbf21b013c786213d4f168289157333f86 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
@@ -116,9 +116,13 @@ describe("NotificationCard", () => {
screen.getByText("Created by John Doe", { exact: false });
});
- it("should unsubscribe when subscribed and clicked on the close icon", () => {
- const user = getUser();
- const alert = getAlert({ channels: [getChannel({ recipients: [user] })] });
+ it("should unsubscribe when the user is not the creator and subscribed", () => {
+ const creator = getUser({ id: 1 });
+ const user = getUser({ id: 2 });
+ const alert = getAlert({
+ creator,
+ channels: [getChannel({ recipients: [user] })],
+ });
const onUnsubscribe = jest.fn();
const onArchive = jest.fn();
@@ -137,9 +141,13 @@ describe("NotificationCard", () => {
expect(onArchive).not.toHaveBeenCalled();
});
- it("should archive when not subscribed and clicked on the close icon", () => {
- const alert = getAlert();
- const user = getUser();
+ it("should unsubscribe when user user is the creator and subscribed with another user", () => {
+ const creator = getUser({ id: 1 });
+ const recipient = getUser({ id: 2 });
+ const alert = getAlert({
+ creator,
+ channels: [getChannel({ recipients: [creator, recipient] })],
+ });
const onUnsubscribe = jest.fn();
const onArchive = jest.fn();
@@ -147,7 +155,52 @@ describe("NotificationCard", () => {
<NotificationCard
item={alert}
type="alert"
- user={user}
+ user={creator}
+ onUnsubscribe={onUnsubscribe}
+ onArchive={onArchive}
+ />,
+ );
+
+ fireEvent.click(screen.getByLabelText("close icon"));
+ expect(onUnsubscribe).toHaveBeenCalledWith(alert, "alert");
+ expect(onArchive).not.toHaveBeenCalled();
+ });
+
+ it("should archive when the user is the creator and not subscribed", () => {
+ 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}
+ />,
+ );
+
+ fireEvent.click(screen.getByLabelText("close icon"));
+ expect(onUnsubscribe).not.toHaveBeenCalled();
+ expect(onArchive).toHaveBeenCalledWith(alert, "alert");
+ });
+
+ it("should archive when the user is the creator and is the only one subscribed", () => {
+ const creator = getUser();
+ const alert = getAlert({
+ creator,
+ channels: [getChannel({ recipients: [creator] })],
+ });
+ const onUnsubscribe = jest.fn();
+ const onArchive = jest.fn();
+
+ render(
+ <NotificationCard
+ item={alert}
+ type="alert"
+ user={creator}
onUnsubscribe={onUnsubscribe}
onArchive={onArchive}
/>,
diff --git a/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx b/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx
index b7018de3920a3e096b1d8aaeea335c1c4c15830a..7d14a2cfe77027438632c4bb5ae4f228e513e9a2 100644
--- a/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx
+++ b/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.jsx
@@ -13,7 +13,7 @@ import {
const propTypes = {
items: PropTypes.array.isRequired,
- user: PropTypes.object,
+ user: PropTypes.object.isRequired,
children: PropTypes.node,
onHelp: PropTypes.func,
onUnsubscribe: PropTypes.func,
diff --git a/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.unit.spec.js b/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.unit.spec.js
index 60b7e80cb5a8e0c49ea77d93b9fbfd1895f7eb5a..dc4184986d937bba4d4bf5923e919935a9881fb7 100644
--- a/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.unit.spec.js
+++ b/frontend/src/metabase/account/notifications/components/NotificationList/NotificationList.unit.spec.js
@@ -8,17 +8,27 @@ const getPulse = () => ({
created_at: "2021-05-08T02:02:07.441Z",
});
+const getUser = () => ({
+ id: 1,
+ common_name: "John Doe",
+});
+
describe("NotificationList", () => {
it("should render items", () => {
const pulse = getPulse();
+ const user = getUser();
- render(<NotificationList items={[{ item: pulse, type: "pulse" }]} />);
+ render(
+ <NotificationList items={[{ item: pulse, type: "pulse" }]} user={user} />,
+ );
screen.getByText("Pulse");
});
it("should render empty state when there are no items", () => {
- render(<NotificationList items={[]} />);
+ const user = getUser();
+
+ render(<NotificationList items={[]} user={user} />);
screen.getByText("you’ll be able to manage those here", { exact: false });
});
diff --git a/frontend/src/metabase/account/notifications/selectors.js b/frontend/src/metabase/account/notifications/selectors.js
index 05d626ec098a53b2f1234c66c5a4d526959ed9fd..5558b68e79e6a947f636c7b3df8cd4e9f94bb0bd 100644
--- a/frontend/src/metabase/account/notifications/selectors.js
+++ b/frontend/src/metabase/account/notifications/selectors.js
@@ -1,4 +1,5 @@
import { createSelector } from "reselect";
+import { parseTimestamp } from "metabase/lib/time";
export const getAlertId = ({ params: { alertId } }) => {
return parseInt(alertId);
@@ -22,6 +23,10 @@ export const getNotifications = createSelector(
})),
];
- return items.sort((a, b) => b.item.created_at - a.item.created_at);
+ return items.sort(
+ (a, b) =>
+ parseTimestamp(b.item.created_at).unix() -
+ parseTimestamp(a.item.created_at).unix(),
+ );
},
);
diff --git a/frontend/src/metabase/entities/alerts.js b/frontend/src/metabase/entities/alerts.js
index f0f0148588b2085a8da1804c984f7019d770b920..452514c6ca20b7190cd2ef4ba476a08ec6526f06 100644
--- a/frontend/src/metabase/entities/alerts.js
+++ b/frontend/src/metabase/entities/alerts.js
@@ -26,7 +26,7 @@ const Alerts = createEntity({
return Alerts.actions.update(
{ id },
{ channels: newChannels },
- undo(opts, "", t`Successfully unsubscribed`),
+ undo(opts, "", t`unsubscribed`),
);
},
},
diff --git a/frontend/src/metabase/entities/pulses.js b/frontend/src/metabase/entities/pulses.js
index 0e1f16d3b31b08d222554505b83b4e2143d693bd..6126de134cdd3988494019c5cef1fcd8465946e7 100644
--- a/frontend/src/metabase/entities/pulses.js
+++ b/frontend/src/metabase/entities/pulses.js
@@ -52,7 +52,7 @@ const Pulses = createEntity({
return Pulses.actions.update(
{ id },
{ channels: newChannels },
- undo(opts, "", t`Successfully unsubscribed`),
+ undo(opts, "", t`unsubscribed`),
);
},
},
diff --git a/frontend/test/metabase/scenarios/account/notifications/notifications.cy.spec.js b/frontend/test/metabase/scenarios/account/notifications/notifications.cy.spec.js
index a28e72ac5628c175d89f2a1978da02d700aca823..cfffad976de6619ef9887968e278cb8a7781fe7f 100644
--- a/frontend/test/metabase/scenarios/account/notifications/notifications.cy.spec.js
+++ b/frontend/test/metabase/scenarios/account/notifications/notifications.cy.spec.js
@@ -1,5 +1,6 @@
import { restore } from "__support__/e2e/helpers/e2e-setup-helpers";
import { SAMPLE_DATASET } from "__support__/e2e/cypress_sample_dataset";
+import { modal } from "__support__/e2e/helpers/e2e-ui-elements-helpers";
const { ORDERS_ID } = SAMPLE_DATASET;
@@ -10,7 +11,7 @@ const getQuestionDetails = () => ({
},
});
-const getAlertDetails = ({ user_id, card_id }) => ({
+const getAlertDetails = ({ card_id, user_id, admin_id }) => ({
card: {
id: card_id,
include_csv: false,
@@ -25,6 +26,9 @@ const getAlertDetails = ({ user_id, card_id }) => ({
{
id: user_id,
},
+ {
+ id: admin_id,
+ },
],
},
],
@@ -52,17 +56,24 @@ const getPulseDetails = ({ card_id, dashboard_id }) => ({
describe("scenarios > account > notifications", () => {
beforeEach(() => {
restore();
- cy.signInAsNormalUser();
});
describe("alerts", () => {
beforeEach(() => {
- cy.getCurrentUser().then(({ body: { id: user_id } }) => {
- cy.createQuestion(getQuestionDetails()).then(
- ({ body: { id: card_id } }) => {
- cy.createAlert(getAlertDetails({ user_id, card_id }));
- },
- );
+ cy.signInAsAdmin().then(() => {
+ cy.getCurrentUser().then(({ body: { id: admin_id } }) => {
+ cy.signInAsNormalUser().then(() => {
+ cy.getCurrentUser().then(({ body: { id: user_id } }) => {
+ cy.createQuestion(getQuestionDetails()).then(
+ ({ body: { id: card_id } }) => {
+ cy.createAlert(
+ getAlertDetails({ card_id, user_id, admin_id }),
+ );
+ },
+ );
+ });
+ });
+ });
});
});
@@ -70,9 +81,13 @@ describe("scenarios > account > notifications", () => {
cy.visit("/account/notifications");
cy.findByText("Not seeing one here?").click();
- cy.findByText("Not seeing something listed here?");
- cy.findByText("Got it").click();
- cy.findByText("Not seeing something listed here?").should("not.exist");
+
+ modal().within(() => {
+ cy.findByText("Not seeing something listed here?");
+ cy.findByText("Got it").click();
+ });
+
+ modal().should("not.exist");
});
it("should be able to see alerts notifications", () => {
@@ -89,11 +104,15 @@ describe("scenarios > account > notifications", () => {
cy.findByText("Question");
cy.findByLabelText("close icon").click();
- cy.findByText("Confirm you want to unsubscribe");
- cy.findByText("Unsubscribe").click();
+ modal().within(() => {
+ cy.findByText("Confirm you want to unsubscribe");
+ cy.findByText("Unsubscribe").click();
+ });
- cy.findByText("You’re unsubscribed. Delete this alert as well?");
- cy.findByText("Delete this alert").click();
+ modal().within(() => {
+ cy.findByText("You’re unsubscribed. Delete this alert as well?");
+ cy.findByText("Delete this alert").click();
+ });
cy.findByText("Question").should("not.exist");
});
@@ -101,7 +120,7 @@ describe("scenarios > account > notifications", () => {
describe("pulses", () => {
beforeEach(() => {
- cy.getCurrentUser().then(({ body: { id: user_id } }) => {
+ cy.signInAsNormalUser().then(() => {
cy.createQuestionAndDashboard({
questionDetails: getQuestionDetails(),
}).then(({ body: { card_id, dashboard_id } }) => {
@@ -114,9 +133,13 @@ describe("scenarios > account > notifications", () => {
cy.visit("/account/notifications");
cy.findByText("Not seeing one here?").click();
- cy.findByText("Not seeing something listed here?");
- cy.findByText("Got it").click();
- cy.findByText("Not seeing something listed here?").should("not.exist");
+
+ modal().within(() => {
+ cy.findByText("Not seeing something listed here?");
+ cy.findByText("Got it").click();
+ });
+
+ modal().should("not.exist");
});
it("should be able to see pulses notifications", () => {
@@ -133,8 +156,10 @@ describe("scenarios > account > notifications", () => {
cy.findByText("Subscription");
cy.findByLabelText("close icon").click();
- cy.findByText("Delete this subscription?");
- cy.findByText("Yes, delete this subscription").click();
+ modal().within(() => {
+ cy.findByText("Delete this subscription?");
+ cy.findByText("Yes, delete this subscription").click();
+ });
cy.findByText("Subscription").should("not.exist");
});