refactor socket notifications (#14135)

* refactor socket notifications

* dunno why these two gut fubared

* add missing yaml files

* more fixes

* fix: schema

* lint: fix lint issues

public/openapi/components/schemas/NotificationObject.yaml

@@ -0,0 +1,60 @@
+NotificationObject:
+  allOf:
+    - type: object
+      properties:
+        importance:
+          type: number
+        datetime:
+          type: number
+        path:
+          type: string
+          description: Relative path to the notification target
+        bodyShort:
+          type: string
+        nid:
+          type: string
+        datetimeISO:
+          type: string
+        read:
+          type: boolean
+        readClass:
+          type: string
+    - type: object
+      description: Optional properties that may or may not be present (except for `nid`, which is always present, and is only here as a hack to pass validation)
+      properties:
+        nid:
+          type: string
+        type:
+          type: string
+          description: Used to denote a classification of notification. These types are toggleable in the user ACP (so the user can opt to not receive notifications for certain types, etc.)
+        bodyLong:
+          type: string
+        from:
+          type: number
+        pid:
+          type: number
+          description: A post id, if the notification pertains to a post
+        tid:
+          type: number
+          description: A post id, if the notification pertains to a topic
+        user:
+          $ref: ./UserObject.yaml#/UserObjectTiny
+        subject:
+          type: string
+          description: A language key that would be used as an email subject, otherwise a generic "New Notification" message.
+        icon:
+          type: string
+        roomName:
+          type: string
+          description: The chat room name, if the notification is related to a chat message
+        roomIcon:
+          type: string
+        mergeId:
+          type: string
+          description: A common string used to denote related notifications that can be merged together (e.g. two new chat messages in same room)
+        image:
+          type: string
+          description: A URL to a media image for the notification (supercedes the user avatar if `user` is present)
+          nullable: true
+      required:
+        - nid

public/openapi/components/schemas/UserObject.yaml

@@ -601,6 +601,33 @@ UserObjectSlim:
       type: string
       description: An ISO 8601 formatted date string representing the moment a ban will be lifted, or the words "Not Banned"
       example: Not Banned
+UserObjectTiny:
+  type: object
+  properties:
+    username:
+      type: string
+      description: A friendly name for a given user account
+    userslug:
+      type: string
+      description: An URL-safe variant of the username (i.e. lower-cased, spaces
+        removed, etc.)
+    picture:
+      type: string
+    uid:
+      type: number
+      description: A user identifier
+    icon:text:
+      type: string
+      description: A single-letter representation of a username. This is used in the
+        auto-generated icon given to users without
+        an avatar
+    icon:bgColor:
+      type: string
+      description: A six-character hexadecimal colour code assigned to the user. This
+        value is used in conjunction with
+        `icon:text` for the user's auto-generated
+        icon
+      example: "#f44336"
 UserObjectACP:
   type: object
   required:

public/openapi/write.yaml

@@ -212,6 +212,14 @@ paths:
     $ref: 'write/posts/pid/owner.yaml'
   /posts/owner:
     $ref: 'write/posts/owner.yaml'
+  /notifications:
+    $ref: 'write/notifications.yaml'
+  /notifications/{nid}:
+    $ref: 'write/notifications/nid.yaml'
+  /notifications/count:
+    $ref: 'write/notifications/count.yaml'
+  /notifications/{nid}/read:
+    $ref: 'write/notifications/nid/read.yaml'
   /chats/:
     $ref: 'write/chats.yaml'
   /chats/unread:

public/openapi/write/notifications.yaml

@@ -0,0 +1,26 @@
+get:
+  tags:
+    - notifications
+  summary: list notifications
+  description: This operation returns two lists of notifications — read and unread.
+  responses:
+    '200':
+      description: notifications successfully listed
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../components/schemas/Status.yaml#/Status
+              response:
+                type: object
+                properties:
+                  read:
+                    type: array
+                    items:
+                      $ref: ../components/schemas/NotificationObject.yaml#/NotificationObject
+                  unread:
+                    type: array
+                    items:
+                      $ref: ../components/schemas/NotificationObject.yaml#/NotificationObject

public/openapi/write/notifications/count.yaml

@@ -0,0 +1,20 @@
+get:
+  tags:
+    - notifications
+  summary: get unread notification count
+  description: This operation returns the calling user's unread notifications count
+  responses:
+    '200':
+      description: unread notifications count successfully retrieved
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../../components/schemas/Status.yaml#/Status
+              response:
+                type: object
+                properties:
+                  unread:
+                    type: number

public/openapi/write/notifications/nid.yaml

@@ -0,0 +1,28 @@
+get:
+  tags:
+    - notifications
+  summary: get notification
+  description: This operation returns the content of a single notification
+  parameters:
+    - in: path
+      name: nid
+      schema:
+        type: number
+      required: true
+      description: The notification id to retrieve
+      example: uploads:export:1
+  responses:
+    '200':
+      description: notification successfully retrieved
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../../components/schemas/Status.yaml#/Status
+              response:
+                type: object
+                properties:
+                  notification:
+                    $ref: ../../components/schemas/NotificationObject.yaml#/NotificationObject

public/openapi/write/notifications/nid/read.yaml

@@ -0,0 +1,52 @@
+delete:
+  tags:
+    - notifications
+  summary: mark notification unread
+  description: This operation marks a notification as unread for the calling user.
+  parameters:
+    - in: path
+      name: nid
+      schema:
+        type: string
+      required: true
+      description: a valid notification id
+      example: 1
+  responses:
+    '200':
+      description: Notification successfully marked unread.
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../../../components/schemas/Status.yaml#/Status
+              response:
+                type: object
+                properties: {}
+put:
+  tags:
+    - notifications
+  summary: mark notification read
+  description: This operation marks a notification as read for the calling user.
+  parameters:
+    - in: path
+      name: nid
+      schema:
+        type: string
+      required: true
+      description: a valid notification id
+      example: 1
+  responses:
+    '200':
+      description: Notification successfully marked read
+      content:
+        application/json:
+          schema:
+            type: object
+            properties:
+              status:
+                $ref: ../../../components/schemas/Status.yaml#/Status
+              response:
+                type: object
+                properties: {}

public/src/modules/notifications.js

@@ -8,7 +8,8 @@ define('notifications', [
 	'tinycon',
 	'hooks',
 	'alerts',
-], function (translator, components, navigator, Tinycon, hooks, alerts) {
+	'api',
+], function (translator, components, navigator, Tinycon, hooks, alerts, api) {
 	const Notifications = {};
 
 	let unreadNotifs = {};
@@ -30,11 +31,7 @@ define('notifications', [
 
 	Notifications.loadNotifications = function (triggerEl, notifList, callback) {
 		callback = callback || function () {};
-		socket.emit('notifications.get', null, function (err, data) {
-			if (err) {
-				return alerts.error(err);
-			}
-
+		api.get('/notifications').then((data) => {
 			const notifs = data.unread.concat(data.read).sort(function (a, b) {
 				return parseInt(a.datetime, 10) > parseInt(b.datetime, 10) ? -1 : 1;
 			});
@@ -75,7 +72,7 @@ define('notifications', [
 					callback();
 				});
 			});
-		});
+		}).catch(alerts.error);
 	};
 
 	Notifications.handleUnreadButton = function (notifList) {
@@ -100,13 +97,9 @@ define('notifications', [
 			return;
 		}
 
-		socket.emit('notifications.getCount', function (err, count) {
-			if (err) {
-				return alerts.error(err);
-			}
-
-			Notifications.updateNotifCount(count);
-		});
+		api.get('/notifications/count').then(({ unread }) => {
+			Notifications.updateNotifCount(unread);
+		}).catch(alerts.error);
 
 		if (!unreadNotifs[notifData.nid]) {
 			unreadNotifs[notifData.nid] = notifData;
@@ -118,18 +111,18 @@ define('notifications', [
 	};
 
 	function markNotification(nid, read, callback) {
-		socket.emit('notifications.mark' + (read ? 'Read' : 'Unread'), nid, function (err) {
-			if (err) {
-				return alerts.error(err);
-			}
-
-			if (read && unreadNotifs[nid]) {
-				delete unreadNotifs[nid];
-			}
-			if (callback) {
-				callback();
-			}
-		});
+		if (read) {
+			api.put(`/notifications/${nid}/read`).then(() => {
+				if (unreadNotifs[nid]) {
+					delete unreadNotifs[nid];
+				}
+				if (callback) {
+					callback();
+				}
+			}).catch(alerts.error);
+		} else {
+			api.delete(`/notifications/${nid}/read`).then(callback).catch(alerts.error);
+		}
 	}
 
 	function scrollToPostIndexIfOnPage(notifEl) {

src/api/index.js

@@ -5,6 +5,7 @@ module.exports = {
 	users: require('./users'),
 	groups: require('./groups'),
 	topics: require('./topics'),
+	notifications: require('./notifications'),
 	tags: require('./tags'),
 	posts: require('./posts'),
 	chats: require('./chats'),

src/api/notifications.js

@@ -0,0 +1,33 @@
+'use strict';
+
+const user = require('../user');
+const notifications = require('../notifications');
+
+const notificationsApi = module.exports;
+
+notificationsApi.list = async (caller) => {
+	const { read, unread } = await user.notifications.get(caller.uid);
+	return { read, unread };
+};
+
+notificationsApi.get = async (caller, { nid }) => {
+	let notification = await user.notifications.getNotifications([nid], caller.uid);
+	notification = notification.pop();
+
+	return { notification };
+};
+
+notificationsApi.getCount = async (caller) => {
+	const unread = await user.notifications.getUnreadCount(caller.uid);
+	return { unread };
+};
+
+notificationsApi.markRead = async (caller, { nid }) => {
+	await notifications.markRead(nid, caller.uid);
+	user.notifications.pushCount(caller.uid);
+};
+
+notificationsApi.markUnread = async (caller, { nid }) => {
+	await notifications.markUnread(nid, caller.uid);
+	user.notifications.pushCount(caller.uid);
+};

src/controllers/write/index.js

@@ -6,6 +6,7 @@ Write.users = require('./users');
 Write.groups = require('./groups');
 Write.categories = require('./categories');
 Write.topics = require('./topics');
+Write.notifications = require('./notifications');
 Write.tags = require('./tags');
 Write.posts = require('./posts');
 Write.chats = require('./chats');

src/controllers/write/notifications.js

@@ -0,0 +1,32 @@
+'use strict';
+
+const api = require('../../api');
+
+const helpers = require('../helpers');
+
+const Notifications = module.exports;
+
+Notifications.get = async (req, res) => {
+	let response;
+	if (req.params.nid) {
+		response = await api.notifications.get(req, { ...req.params });
+	} else {
+		response = await api.notifications.list(req);
+	}
+
+	helpers.formatApiResponse(200, res, response);
+};
+
+Notifications.getCount = async (req, res) => {
+	helpers.formatApiResponse(200, res, await api.notifications.getCount(req));
+};
+
+Notifications.markRead = async (req, res) => {
+	await api.notifications.markRead(req, { ...req.params });
+	helpers.formatApiResponse(200, res);
+};
+
+Notifications.markUnread = async (req, res) => {
+	await api.notifications.markUnread(req, { ...req.params });
+	helpers.formatApiResponse(200, res);
+};

src/routes/write/index.js

@@ -37,6 +37,7 @@ Write.reload = async (params) => {
 	router.use('/api/v3/groups', require('./groups')());
 	router.use('/api/v3/categories', require('./categories')());
 	router.use('/api/v3/topics', require('./topics')());
+	router.use('/api/v3/notifications', require('./notifications')());
 	router.use('/api/v3/tags', require('./tags')());
 	router.use('/api/v3/posts', require('./posts')());
 	router.use('/api/v3/chats', require('./chats')());

src/routes/write/notifications.js

@@ -0,0 +1,22 @@
+'use strict';
+
+const router = require('express').Router();
+const middleware = require('../../middleware');
+const controllers = require('../../controllers');
+const routeHelpers = require('../helpers');
+
+const { setupApiRoute } = routeHelpers;
+
+module.exports = function () {
+	const middlewares = [middleware.ensureLoggedIn];
+
+	setupApiRoute(router, 'get', '/count', [...middlewares], controllers.write.notifications.getCount);
+
+	setupApiRoute(router, 'get', '/:nid?', [...middlewares], controllers.write.notifications.get);
+
+	setupApiRoute(router, 'put', '/:nid/read', [...middlewares], controllers.write.notifications.markRead);
+
+	setupApiRoute(router, 'delete', '/:nid/read', [...middlewares], controllers.write.notifications.markUnread);
+
+	return router;
+};

src/socket.io/notifications.js

@@ -2,18 +2,35 @@
 
 const user = require('../user');
 const notifications = require('../notifications');
+const api = require('../api');
+
+const sockets = require('.');
 
 const SocketNotifs = module.exports;
 
 SocketNotifs.get = async function (socket, data) {
+	sockets.warnDeprecated(socket, 'GET /api/v3/notifications/(:nid)');
+
+	// Passing in multiple nids is no longer supported in apiv3
 	if (data && Array.isArray(data.nids) && socket.uid) {
-		return await user.notifications.getNotifications(data.nids, socket.uid);
+		const notifications = await Promise.all(data.nids.map(async (nid) => {
+			const { notification } = await api.notifications.get(socket, { nid });
+			return notification;
+		}));
+
+		return notifications;
 	}
-	return await user.notifications.get(socket.uid);
+
+	const response = await api.notifications.list(socket);
+	response.uid = socket.uid;
+	return response;
 };
 
 SocketNotifs.getCount = async function (socket) {
-	return await user.notifications.getUnreadCount(socket.uid);
+	sockets.warnDeprecated(socket, 'GET /api/v3/notifications/count');
+
+	const { unread } = await api.notifications.getCount(socket);
+	return unread;
 };
 
 SocketNotifs.deleteAll = async function (socket) {
@@ -25,13 +42,13 @@ SocketNotifs.deleteAll = async function (socket) {
 };
 
 SocketNotifs.markRead = async function (socket, nid) {
-	await notifications.markRead(nid, socket.uid);
-	user.notifications.pushCount(socket.uid);
+	sockets.warnDeprecated(socket, 'PUT /api/v3/notifications/:nid/read');
+	await api.notifications.markRead(socket, { nid });
 };
 
 SocketNotifs.markUnread = async function (socket, nid) {
-	await notifications.markUnread(nid, socket.uid);
-	user.notifications.pushCount(socket.uid);
+	sockets.warnDeprecated(socket, 'DELETE /api/v3/notifications/:nid/read');
+	await api.notifications.markUnread(socket, { nid });
 };
 
 SocketNotifs.markAllRead = async function (socket, data) {

test/api.js

@@ -24,6 +24,7 @@ const plugins = require('../src/plugins');
 const flags = require('../src/flags');
 const messaging = require('../src/messaging');
 const activitypub = require('../src/activitypub');
+const notifications = require('../src/notifications');
 const utils = require('../src/utils');
 const api = require('../src/api');
 
@@ -264,6 +265,16 @@ describe('API', async () => {
 			content: 'Test topic 3 content',
 		});
 
+		// create a notification
+		const notifObj = await notifications.create({
+			nid: '1', // match nid in example in notifications/nid/read.yaml
+			path: '/notifications',
+			from: unprivUid,
+			bodyShort: 'testing notification',
+		});
+		notifications.push(notifObj, adminUid);
+
+
 		// Create a post diff
 		await posts.edit({
 			uid: adminUid,