refactor(socket.io): deprecate SocketModules.chats.canMessage and .markAllRead with no alternative. deprecate .getRecentChats in favour of api.chats.list

public/openapi/write/chats.yaml

@@ -4,17 +4,29 @@ get:
   summary: list recent chat rooms
   description: This operation lists recently used chat rooms that the calling user is a part of
   parameters:
+    - in: query
+      name: start
+      schema:
+        type: number
+      description: >
+        The start index from which chat rooms will be displayed.
+        e.g. `start` of `10` with `perPage` of 10 would result in the 10th to 19th chat rooms being returned
+      example: 20
     - in: query
       name: perPage
       schema:
         type: number
-      description: The number of chat rooms displayed per page
+      description: The number of chat rooms to be displayed per page
+      required: false
       example: 20
     - in: query
       name: page
       schema:
         type: number
-      description: The page number
+      description: >
+        ***Deprecated*** — The page number.
+
+        This parameter is supeceded by `start`, and will stop working in NodeBB v4.
       example: 1
   responses:
     '200':

public/src/client/chats/recent.js

@@ -29,29 +29,25 @@ define('forum/chats/recent', ['alerts', 'api', 'chat'], function (alerts, api, c
 		});
 	};
 
-	function loadMoreRecentChats() {
+	async function loadMoreRecentChats() {
 		const recentChats = $('[component="chat/recent"]');
 		if (recentChats.attr('loading')) {
 			return;
 		}
 		recentChats.attr('loading', 1);
-		socket.emit('modules.chats.getRecentChats', {
+		app.get(`/chats`, {
 			uid: ajaxify.data.uid,
 			after: recentChats.attr('data-nextstart'),
-		}, function (err, data) {
-			if (err) {
-				return alerts.error(err);
-			}
-
-			if (data && data.rooms.length) {
-				onRecentChatsLoaded(data, function () {
+		}).then(({ rooms, nextStart }) => {
+			if (rooms.length) {
+				onRecentChatsLoaded({ rooms, nextStart }, function () {
 					recentChats.removeAttr('loading');
-					recentChats.attr('data-nextstart', data.nextStart);
+					recentChats.attr('data-nextstart', nextStart);
 				});
 			} else {
 				recentChats.removeAttr('loading');
 			}
-		});
+		}).catch(alerts.error);
 	}
 
 	function onRecentChatsLoaded(data, callback) {

public/src/modules/chat.js

@@ -79,13 +79,10 @@ define('chat', [
 	};
 
 	module.loadChatsDropdown = function (chatsListEl) {
-		socket.emit('modules.chats.getRecentChats', {
+		api.get('/chats', {
 			uid: app.user.uid,
 			after: 0,
-		}, function (err, data) {
-			if (err) {
-				return alerts.error(err);
-			}
+		}).then((data) => {
 			const rooms = data.rooms.map((room) => {
 				if (room && room.teaser) {
 					room.teaser.timeagoLong = $.timeago(new Date(parseInt(room.teaser.timestamp, 10)));
@@ -123,15 +120,17 @@ define('chat', [
 				listEl.addEventListener('click', onMarkReadClicked);
 
 				$('[component="chats/mark-all-read"]').off('click').on('click', async function () {
-					await socket.emit('modules.chats.markAllRead');
-					if (ajaxify.data.template.chats) {
-						$('[component="chat/nav-wrapper"] [data-roomid]').each((i, el) => {
+					const chatEls = document.querySelectorAll('[component="chat/list"] [data-roomid]');
+					await Promise.all(Array.prototype.map.call(chatEls, async (el) => {
+						const roomId = el.getAttribute('data-roomid');
+						await api.del(`/chats/${roomId}/state`);
+						if (ajaxify.data.template.chats) {
 							module.markChatElUnread($(el), false);
-						});
-					}
+						}
+					}));
 				});
 			});
-		});
+		}).catch(alerts.error);
 	};
 
 	function onMarkReadClicked(e) {

src/api/chats.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const validator = require('validator');
+const winston = require('winston');
 
 const db = require('../database');
 const user = require('../user');
@@ -32,12 +33,14 @@ async function rateLimitExceeded(caller, field) {
 	return false;
 }
 
-chatsAPI.list = async (caller, { page, perPage }) => {
-	const start = Math.max(0, page - 1) * perPage;
-	const stop = start + perPage;
-	const { rooms } = await messaging.getRecentChats(caller.uid, caller.uid, start, stop);
+chatsAPI.list = async (caller, { uid, start, stop, page, perPage }) => {
+	if (!start && !stop && page) {
+		winston.warn('[api/chats] Sending `page` and `perPage` to .list() is deprecated in favour of `start` and `stop`. The deprecated parameters will be removed in v4.');
+		start = Math.max(0, page - 1) * perPage;
+		stop = start + perPage - 1;
+	}
 
-	return { rooms };
+	return await messaging.getRecentChats(caller.uid, uid || caller.uid, start, stop);
 };
 
 chatsAPI.create = async function (caller, data) {

src/controllers/write/chats.js

@@ -6,11 +6,22 @@ const helpers = require('../helpers');
 const Chats = module.exports;
 
 Chats.list = async (req, res) => {
-	const page = (isFinite(req.query.page) && parseInt(req.query.page, 10)) || 1;
-	const perPage = (isFinite(req.query.perPage) && parseInt(req.query.perPage, 10)) || 20;
-	const { rooms } = await api.chats.list(req, { page, perPage });
+	let stop;
+	let { page, perPage, start, uid } = req.query;
+	([page, perPage, start, uid] = [page, perPage, start, uid].map(value => isFinite(value) && parseInt(value, 10)));
+	page = page || 1;
+	perPage = perPage || 20;
 
-	helpers.formatApiResponse(200, res, { rooms });
+	// start supercedes page
+	if (start) {
+		stop = start + perPage - 1;
+	} else {
+		start = Math.max(0, page - 1) * perPage;
+		stop = start + perPage - 1;
+	}
+
+	const { rooms, nextStart } = await api.chats.list(req, { start, stop, uid });
+	helpers.formatApiResponse(200, res, { rooms, nextStart });
 };
 
 Chats.create = async (req, res) => {

src/messaging/index.js

@@ -173,7 +173,7 @@ Messaging.getPublicRooms = async (callerUid, uid) => {
 Messaging.getRecentChats = async (callerUid, uid, start, stop) => {
 	const ok = await canGet('filter:messaging.canGetRecentChats', callerUid, uid);
 	if (!ok) {
-		return null;
+		throw new Error('[[error:no-privileges]]');
 	}
 
 	const roomIds = await db.getSortedSetRevRange(`uid:${uid}:chat:rooms`, start, stop);

src/socket.io/modules.js

@@ -45,22 +45,29 @@ SocketModules.chats.isDnD = async function (socket, uid) {
 };
 
 SocketModules.chats.canMessage = async function (socket, roomId) {
+	sockets.warnDeprecated(socket);
+
 	await Messaging.canMessageRoom(socket.uid, roomId);
 };
 
 SocketModules.chats.markAllRead = async function (socket) {
-	// no v3 method ?
+	sockets.warnDeprecated(socket);
+
 	await Messaging.markAllRead(socket.uid);
 	Messaging.pushUnreadCount(socket.uid);
 };
 
 SocketModules.chats.getRecentChats = async function (socket, data) {
+	sockets.warnDeprecated(socket, 'GET /api/v3/chats');
+
 	if (!data || !utils.isNumber(data.after) || !utils.isNumber(data.uid)) {
 		throw new Error('[[error:invalid-data]]');
 	}
 	const start = parseInt(data.after, 10);
 	const stop = start + 9;
-	return await Messaging.getRecentChats(socket.uid, data.uid, start, stop);
+	const { uid } = data;
+
+	return api.chats.list(socket, { uid, start, stop });
 };
 
 SocketModules.chats.hasPrivateChat = async function (socket, uid) {

test/messaging.js

@@ -80,7 +80,7 @@ describe('Messaging Library', () => {
 		meta.configs.chatMessageDelay = chatMessageDelay;
 	});
 
-	describe('.canMessage()', () => {
+	describe('.canMessageUser()', () => {
 		it('should allow messages to be sent to an unrestricted user', (done) => {
 			Messaging.canMessageUser(mocks.users.baz.uid, mocks.users.herp.uid, (err) => {
 				assert.ifError(err);