From a4133500fe31e58c081e78ff00c8bfd1b08320eb Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Mon, 13 Nov 2023 15:35:46 -0500 Subject: [PATCH] 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 | 16 ++++++++++++++-- public/src/client/chats/recent.js | 18 +++++++----------- public/src/modules/chat.js | 21 ++++++++++----------- src/api/chats.js | 13 ++++++++----- src/controllers/write/chats.js | 19 +++++++++++++++---- src/messaging/index.js | 2 +- src/socket.io/modules.js | 11 +++++++++-- test/messaging.js | 2 +- 8 files changed, 65 insertions(+), 37 deletions(-) diff --git a/public/openapi/write/chats.yaml b/public/openapi/write/chats.yaml index 519b9f6556..37204a9b5c 100644 --- a/public/openapi/write/chats.yaml +++ b/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': diff --git a/public/src/client/chats/recent.js b/public/src/client/chats/recent.js index a4f6490f4b..02da563286 100644 --- a/public/src/client/chats/recent.js +++ b/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) { diff --git a/public/src/modules/chat.js b/public/src/modules/chat.js index 9802c7ae3f..97b580b0b2 100644 --- a/public/src/modules/chat.js +++ b/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) { diff --git a/src/api/chats.js b/src/api/chats.js index 5afe28b38c..7e7c1f4c2f 100644 --- a/src/api/chats.js +++ b/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) { diff --git a/src/controllers/write/chats.js b/src/controllers/write/chats.js index 1932e32d99..51a9fed6d0 100644 --- a/src/controllers/write/chats.js +++ b/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) => { diff --git a/src/messaging/index.js b/src/messaging/index.js index f91367bbfc..ccedd47e81 100644 --- a/src/messaging/index.js +++ b/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); diff --git a/src/socket.io/modules.js b/src/socket.io/modules.js index fa4e93dbe6..705895c8b7 100644 --- a/src/socket.io/modules.js +++ b/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) { diff --git a/test/messaging.js b/test/messaging.js index 826f2e8b3a..9ea17cf402 100644 --- a/test/messaging.js +++ b/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);