Skip to content

Commit

Permalink
fix(notifications): Update code to handle presentational data and ico…
Browse files Browse the repository at this point in the history
…n mapping

Discourse notification types starts from 1 in that sense we need to update to make sure we map icons
correctly, besides, the notification presentational data changes depending on the type of
notification, thus, we need a way to provide a common structure to be rendered
  • Loading branch information
duranmla committed Apr 6, 2019
1 parent aa9e24e commit 1da6b6f
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 40 deletions.
5 changes: 5 additions & 0 deletions flow-typed/myLibDef.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ declare type Link = {
declare type Notification = {
created_at: string,
data: $Shape<{
badge_id: number,
badge_name: string,
badge_slug: string,
badge_title: boolean,
display_username: string,
group_id: number,
original_post_id: number,
original_post_type: number,
original_username: string,
revision_number: number,
topic_title: string,
username: string,
}>,
fancy_title: string,
id: number,
Expand Down
16 changes: 16 additions & 0 deletions src/__fixtures__/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,20 @@ export const notifications = [
slug: "hello-im-jane-doe",
topic_id: 26,
},
{
created_at: "2019-04-04T06:10:46.459Z",
data: {
badge_id: 5,
badge_name: "Welcome",
badge_slug: "welcome",
badge_title: false,
},
id: 33,
notification_type: 12,
post_number: null,
read: false,
slug: null,
topic_id: null,
username: "alexis",
},
];
42 changes: 25 additions & 17 deletions src/notifications/NotificationsPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { NotificationsPanelComponents as NPC } from "./styled";
import React from "react";
import { translate } from "../locales";
import { Button, ClickAwayListener, Typography } from "@material-ui/core";
import { getNotificationIcon, getNotificationLink } from "./helpers";
import {
getNotificationIcon,
getNotificationLink,
getNotificationPresentationalData,
} from "./helpers";

type Props = {
handleClose: Function,
Expand Down Expand Up @@ -37,22 +41,26 @@ export const NotificationsPanel = ({
</div>
</NPC.Header>
<NPC.Body>
{notifications.map(n => (
<NPC.Item
target="_blank"
href={getNotificationLink(n)}
key={n.created_at}
onClick={handleClose}
>
{getNotificationIcon(n.notification_type)}
<div aria-label="NotificationItem">
<Typography variant="body2">{n.data.topic_title}</Typography>
<Typography variant="caption">
{moment(n.created_at).fromNow()}
</Typography>
</div>
</NPC.Item>
))}
{notifications.map(n => {
const { title, date } = getNotificationPresentationalData(n);

return (
<NPC.Item
target="_blank"
href={getNotificationLink(n)}
key={n.created_at}
onClick={handleClose}
>
{getNotificationIcon(n.notification_type)}
<div aria-label="NotificationItem">
<Typography variant="body2">{title}</Typography>
<Typography variant="caption">
{moment(date).fromNow()}
</Typography>
</div>
</NPC.Item>
);
})}
{notifications.length === 0 && (
<NPC.Item>
{getNotificationIcon("announcement")}
Expand Down
2 changes: 1 addition & 1 deletion src/notifications/__tests__/Notifications.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe("<Notifications />", () => {
expect(spyOnNormaliseUserNotifications).toHaveBeenCalledTimes(1);
// eslint-disable-next-line
expect(alertsElem.firstChild).toMatchInlineSnapshot(
"[{\"created_at\":\"2019-03-24T10:37:18.891Z\",\"data\":{\"display_username\":\"system\",\"group_id\":1,\"topic_title\":\"Backup failed\"},\"fancy_title\":\"Backup failed\",\"id\":1,\"notification_type\":7,\"post_number\":1,\"read\":false,\"slug\":\"backup-failed\",\"topic_id\":11}]"
"[{\"created_at\":\"2019-03-24T10:37:18.891Z\",\"data\":{\"display_username\":\"system\",\"group_id\":1,\"topic_title\":\"Backup failed\"},\"fancy_title\":\"Backup failed\",\"id\":1,\"notification_type\":7,\"post_number\":1,\"read\":false,\"slug\":\"backup-failed\",\"topic_id\":11},{\"created_at\":\"2019-04-04T06:10:46.459Z\",\"data\":{\"badge_id\":5,\"badge_name\":\"Welcome\",\"badge_slug\":\"welcome\",\"badge_title\":false},\"id\":33,\"notification_type\":12,\"post_number\":null,\"read\":false,\"slug\":null,\"topic_id\":null,\"username\":\"alexis\"}]"
);
expect(messagesElem.firstChild).toMatchInlineSnapshot(
"[{\"created_at\":\"2019-03-24T10:24:32.495Z\",\"data\":{\"display_username\":\"janedoe\",\"original_post_id\":41,\"original_post_type\":1,\"original_username\":\"janedoe\",\"revision_number\":null,\"topic_title\":\"Hello, I'm Jane Doe\"},\"fancy_title\":\"Hello, I&rsquo;m Jane Doe\",\"id\":18,\"notification_type\":6,\"post_number\":1,\"read\":false,\"slug\":\"hello-im-jane-doe\",\"topic_id\":26},{\"created_at\":\"2019-03-24T11:24:32.495Z\",\"data\":{\"display_username\":\"janedoe\",\"original_post_id\":42,\"original_post_type\":1,\"original_username\":\"janedoe\",\"revision_number\":null,\"topic_title\":\"Elephant\"},\"fancy_title\":\"Elephant\",\"id\":19,\"notification_type\":5,\"post_number\":1,\"read\":false,\"slug\":\"hello-im-jane-doe\",\"topic_id\":26}]"
Expand Down
26 changes: 24 additions & 2 deletions src/notifications/__tests__/helpers.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,33 @@
// @flow

import { getNotificationTypeName } from "../helpers";
import { notifications } from "../../__fixtures__/notifications";
import {
getNotificationPresentationalData,
getNotificationTypeName,
} from "../helpers";

describe("helpers", () => {
describe("getNotificationTypeName", () => {
it("returns mapped notification name after number", () => {
expect(getNotificationTypeName(1)).toEqual("replied");
expect(getNotificationTypeName(1)).toEqual("mentioned");
});
});

describe("getNotificationPresentationalData", () => {
it("returns a consistent object with title and date", () => {
expect(getNotificationPresentationalData(notifications[0])).toEqual({
date: notifications[0].created_at,
title: notifications[0].data.topic_title,
});
});

describe("when notification type is related to granted badge", () => {
it("returns a consistent object with title and date", () => {
expect(getNotificationPresentationalData(notifications[3])).toEqual({
date: notifications[3].created_at,
title: notifications[3].data.badge_name,
});
});
});
});
});
2 changes: 1 addition & 1 deletion src/notifications/__tests__/normaliser.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe("normaliser", () => {
it("returns categorised notifications by alerts and messages", () => {
const result = normaliseUserNotifications(notifications);

expect(result.alerts).toEqual([notifications[1]]);
expect(result.alerts).toEqual([notifications[1], notifications[3]]);
expect(result.messages).toEqual([notifications[0], notifications[2]]);
});
});
49 changes: 30 additions & 19 deletions src/notifications/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,24 @@ import Reply from "@material-ui/icons/Reply";
* as a number which map to the below indexes
*/
export const notificationTypes = [
"mentioned",
"replied",
"quoted",
"edited",
"liked",
"privateMessage",
"invitedToPrivateMessage",
"inviteeAccepted",
"posted",
"movedPost",
"linked",
"grantedBadge",
"invitedToTopic",
"custom",
"groupMentioned",
"groupMessageSummary",
"watchingFirstPost",
"topicReminder",
"mentioned", // 1
"replied", // 2
"quoted", // 3
"edited", // 4
"liked", // 5
"privateMessage", // 6
"invitedToPrivateMessage", // 7
"inviteeAccepted", // 8
"posted", // 9
"movedPost", // 10
"linked", // 11
"grantedBadge", // 12
"invitedToTopic", // 13
"custom", // 14
"groupMentioned", // 15
"groupMessageSummary", // 16
"watchingFirstPost", // 17
"topicReminder", // 18
];

const iconsMapping = {
Expand Down Expand Up @@ -77,7 +77,7 @@ export const getNotificationTypeName = (
notificationType: string | number
): string => {
return typeof notificationType === "number"
? notificationTypes[notificationType]
? notificationTypes[notificationType - 1]
: notificationType;
};

Expand All @@ -89,3 +89,14 @@ export const getNotificationIcon = (

return Icon ? <Icon color={color} /> : <Announcement color={color} />;
};

export const getNotificationPresentationalData = (
notification: Notification
) => {
const { data } = notification;

return {
date: notification.created_at,
title: data.topic_title || data.badge_name,
};
};

0 comments on commit 1da6b6f

Please sign in to comment.