From 36895421ba0f8ceae21efbaa324ac8da4794602a Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Mon, 24 Apr 2023 14:14:16 -0400 Subject: [PATCH] refactor: topic mark read/unread routes --- public/openapi/write.yaml | 4 ++ public/openapi/write/topics/tid/bump.yaml | 29 +++++++++ public/openapi/write/topics/tid/read.yaml | 52 +++++++++++++++ public/src/client/topic.js | 2 +- public/src/client/topic/threadTools.js | 42 +++++-------- public/src/client/unread.js | 9 +-- src/api/topics.js | 24 +++++++ src/controllers/write/topics.js | 18 ++++++ src/routes/write/topics.js | 4 ++ src/socket.io/topics/unread.js | 37 +++++------ test/topics.js | 77 ++++++----------------- 11 files changed, 186 insertions(+), 112 deletions(-) create mode 100644 public/openapi/write/topics/tid/bump.yaml create mode 100644 public/openapi/write/topics/tid/read.yaml diff --git a/public/openapi/write.yaml b/public/openapi/write.yaml index 6f70f55a20..efc9c2bc46 100644 --- a/public/openapi/write.yaml +++ b/public/openapi/write.yaml @@ -142,6 +142,10 @@ paths: $ref: 'write/topics/tid/events.yaml' /topics/{tid}/events/{eventId}: $ref: 'write/topics/tid/events/eventId.yaml' + /topics/{tid}/read: + $ref: 'write/topics/tid/read.yaml' + /topics/{tid}/bump: + $ref: 'write/topics/tid/bump.yaml' /posts/{pid}: $ref: 'write/posts/pid.yaml' /posts/{pid}/index: diff --git a/public/openapi/write/topics/tid/bump.yaml b/public/openapi/write/topics/tid/bump.yaml new file mode 100644 index 0000000000..9ba0ecff26 --- /dev/null +++ b/public/openapi/write/topics/tid/bump.yaml @@ -0,0 +1,29 @@ +put: + tags: + - topics + summary: mark topic unread for all + description: | + This operation marks a topic as unread for all users. + + **Note**: This is a privileged call and can only be executed by administrators, global moderators, or the moderator for the category of the passed-in topic. + parameters: + - in: path + name: tid + schema: + type: string + required: true + description: a valid topic id + example: 1 + responses: + '200': + description: Topic successfully marked unread for all + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../components/schemas/Status.yaml#/Status + response: + type: object + properties: {} \ No newline at end of file diff --git a/public/openapi/write/topics/tid/read.yaml b/public/openapi/write/topics/tid/read.yaml new file mode 100644 index 0000000000..80f75674a5 --- /dev/null +++ b/public/openapi/write/topics/tid/read.yaml @@ -0,0 +1,52 @@ +delete: + tags: + - topics + summary: mark topic unread + description: This operation marks a topic as unread for the calling user. + parameters: + - in: path + name: tid + schema: + type: string + required: true + description: a valid topic id + example: 1 + responses: + '200': + description: Topic successfully marked unread. + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../components/schemas/Status.yaml#/Status + response: + type: object + properties: {} +put: + tags: + - topics + summary: mark topic read + description: This operation marks a topic as read for the calling user. + parameters: + - in: path + name: tid + schema: + type: string + required: true + description: a valid topic id + example: 1 + responses: + '200': + description: Topic successfully marked read + content: + application/json: + schema: + type: object + properties: + status: + $ref: ../../../components/schemas/Status.yaml#/Status + response: + type: object + properties: {} \ No newline at end of file diff --git a/public/src/client/topic.js b/public/src/client/topic.js index 401cf0ca1f..7143759506 100644 --- a/public/src/client/topic.js +++ b/public/src/client/topic.js @@ -391,7 +391,7 @@ define('forum/topic', [ currentUrl = newUrl; if (index >= elementCount && app.user.uid) { - socket.emit('topics.markAsRead', [ajaxify.data.tid]); + api.put(`/topics/${ajaxify.data.tid}/read`); } updateUserBookmark(index); diff --git a/public/src/client/topic/threadTools.js b/public/src/client/topic/threadTools.js index dc401dfd39..da37ddb17c 100644 --- a/public/src/client/topic/threadTools.js +++ b/public/src/client/topic/threadTools.js @@ -60,27 +60,8 @@ define('forum/topic/threadTools', [ return false; }); - topicContainer.on('click', '[component="topic/event/delete"]', function () { - const eventId = $(this).attr('data-topic-event-id'); - const eventEl = $(this).parents('[component="topic/event"]'); - bootbox.confirm('[[topic:delete-event-confirm]]', (ok) => { - if (ok) { - api.del(`/topics/${tid}/events/${eventId}`, {}) - .then(function () { - eventEl.remove(); - }) - .catch(alerts.error); - } - }); - }); - - // todo: should also use topicCommand, but no write api call exists for this yet topicContainer.on('click', '[component="topic/mark-unread"]', function () { - socket.emit('topics.markUnread', tid, function (err) { - if (err) { - return alerts.error(err); - } - + topicCommand('del', '/read', undefined, () => { if (app.previousUrl && !app.previousUrl.match('^/topic')) { ajaxify.go(app.previousUrl, function () { handleBack.onBackClicked(true); @@ -91,19 +72,28 @@ define('forum/topic/threadTools', [ alerts.success('[[topic:mark_unread.success]]'); }); - return false; }); topicContainer.on('click', '[component="topic/mark-unread-for-all"]', function () { const btn = $(this); - socket.emit('topics.markAsUnreadForAll', [tid], function (err) { - if (err) { - return alerts.error(err); - } + topicCommand('put', '/bump', undefined, () => { alerts.success('[[topic:markAsUnreadForAll.success]]'); btn.parents('.thread-tools.open').find('.dropdown-toggle').trigger('click'); }); - return false; + }); + + topicContainer.on('click', '[component="topic/event/delete"]', function () { + const eventId = $(this).attr('data-topic-event-id'); + const eventEl = $(this).parents('[component="topic/event"]'); + bootbox.confirm('[[topic:delete-event-confirm]]', (ok) => { + if (ok) { + api.del(`/topics/${tid}/events/${eventId}`, {}) + .then(function () { + eventEl.remove(); + }) + .catch(alerts.error); + } + }); }); topicContainer.on('click', '[component="topic/move"]', function () { diff --git a/public/src/client/unread.js b/public/src/client/unread.js index e331636915..35cf397406 100644 --- a/public/src/client/unread.js +++ b/public/src/client/unread.js @@ -2,8 +2,8 @@ define('forum/unread', [ - 'forum/header/unread', 'topicSelect', 'components', 'topicList', 'categorySelector', 'alerts', -], function (headerUnread, topicSelect, components, topicList, categorySelector, alerts) { + 'forum/header/unread', 'topicSelect', 'components', 'topicList', 'categorySelector', 'alerts', 'api', +], function (headerUnread, topicSelect, components, topicList, categorySelector, alerts, api) { const Unread = {}; Unread.init = function () { @@ -37,11 +37,8 @@ define('forum/unread', [ if (!tids.length) { return; } - socket.emit('topics.markAsRead', tids, function (err) { - if (err) { - return alerts.error(err); - } + Promise.all(tids.map(async tid => api.put(`/topics/${tid}/read`))).then(() => { doneRemovingTids(tids); }); } diff --git a/src/api/topics.js b/src/api/topics.js index 08fa75ed04..9b8b030d96 100644 --- a/src/api/topics.js +++ b/src/api/topics.js @@ -268,3 +268,27 @@ topicsAPI.deleteEvent = async (caller, { tid, eventId }) => { await topics.events.purge(tid, [eventId]); }; + +topicsAPI.markRead = async (caller, { tid }) => { + const hasMarked = await topics.markAsRead([tid], caller.uid); + const promises = [topics.markTopicNotificationsRead([tid], caller.uid)]; + if (hasMarked) { + promises.push(topics.pushUnreadCount(caller.uid)); + } + await Promise.all(promises); +}; + +topicsAPI.markUnread = async (caller, { tid }) => { + await topics.markUnread(tid, caller.uid); + topics.pushUnreadCount(caller.uid); +}; + +topicsAPI.bump = async (caller, { tid }) => { + const isAdminOrMod = await privileges.topics.isAdminOrMod(tid, caller.uid); + if (!isAdminOrMod) { + throw new Error('[[error:no-privileges]]'); + } + + await topics.markAsUnreadForAll(tid); + topics.pushUnreadCount(caller.uid); +}; diff --git a/src/controllers/write/topics.js b/src/controllers/write/topics.js index 83c021c5e3..b9691a8da5 100644 --- a/src/controllers/write/topics.js +++ b/src/controllers/write/topics.js @@ -189,3 +189,21 @@ Topics.deleteEvent = async (req, res) => { helpers.formatApiResponse(200, res); }; + +Topics.markRead = async (req, res) => { + await api.topics.markRead(req, { ...req.params }); + + helpers.formatApiResponse(200, res); +}; + +Topics.markUnread = async (req, res) => { + await api.topics.markUnread(req, { ...req.params }); + + helpers.formatApiResponse(200, res); +}; + +Topics.bump = async (req, res) => { + await api.topics.bump(req, { ...req.params }); + + helpers.formatApiResponse(200, res); +}; diff --git a/src/routes/write/topics.js b/src/routes/write/topics.js index c3e4739940..1a537fd56d 100644 --- a/src/routes/write/topics.js +++ b/src/routes/write/topics.js @@ -45,5 +45,9 @@ module.exports = function () { setupApiRoute(router, 'get', '/:tid/events', [middleware.assert.topic], controllers.write.topics.getEvents); setupApiRoute(router, 'delete', '/:tid/events/:eventId', [middleware.assert.topic], controllers.write.topics.deleteEvent); + setupApiRoute(router, 'put', '/:tid/read', [...middlewares, middleware.assert.topic], controllers.write.topics.markRead); + setupApiRoute(router, 'delete', '/:tid/read', [...middlewares, middleware.assert.topic], controllers.write.topics.markUnread); + setupApiRoute(router, 'put', '/:tid/bump', [...middlewares, middleware.assert.topic], controllers.write.topics.bump); + return router; }; diff --git a/src/socket.io/topics/unread.js b/src/socket.io/topics/unread.js index 983a9634c6..e32ee59450 100644 --- a/src/socket.io/topics/unread.js +++ b/src/socket.io/topics/unread.js @@ -1,19 +1,19 @@ 'use strict'; -const user = require('../../user'); const topics = require('../../topics'); +const api = require('../../api'); +const sockets = require('..'); + module.exports = function (SocketTopics) { SocketTopics.markAsRead = async function (socket, tids) { + sockets.warnDeprecated(socket, 'PUT /api/v3/topics/:tid/read'); + if (!Array.isArray(tids) || socket.uid <= 0) { throw new Error('[[error:invalid-data]]'); } - const hasMarked = await topics.markAsRead(tids, socket.uid); - const promises = [topics.markTopicNotificationsRead(tids, socket.uid)]; - if (hasMarked) { - promises.push(topics.pushUnreadCount(socket.uid)); - } - await Promise.all(promises); + + await Promise.all(tids.map(async tid => api.topics.markRead(socket, { tid }))); }; SocketTopics.markTopicNotificationsRead = async function (socket, tids) { @@ -37,14 +37,18 @@ module.exports = function (SocketTopics) { }; SocketTopics.markUnread = async function (socket, tid) { + sockets.warnDeprecated(socket, 'DELETE /api/v3/topics/:tid/read'); + if (!tid || socket.uid <= 0) { throw new Error('[[error:invalid-data]]'); } - await topics.markUnread(tid, socket.uid); - topics.pushUnreadCount(socket.uid); + + await api.topics.markUnread(socket, { tid }); }; SocketTopics.markAsUnreadForAll = async function (socket, tids) { + sockets.warnDeprecated(socket, 'PUT /api/v3/topics/:tid/bump'); + if (!Array.isArray(tids)) { throw new Error('[[error:invalid-tid]]'); } @@ -52,18 +56,7 @@ module.exports = function (SocketTopics) { if (socket.uid <= 0) { throw new Error('[[error:no-privileges]]'); } - const isAdmin = await user.isAdministrator(socket.uid); - await Promise.all(tids.map(async (tid) => { - const topicData = await topics.getTopicFields(tid, ['tid', 'cid']); - if (!topicData.tid) { - throw new Error('[[error:no-topic]]'); - } - const isMod = await user.isModerator(socket.uid, topicData.cid); - if (!isAdmin && !isMod) { - throw new Error('[[error:no-privileges]]'); - } - await topics.markAsUnreadForAll(tid); - })); - topics.pushUnreadCount(socket.uid); + + await Promise.all(tids.map(async tid => api.topics.bump(socket, { tid }))); }; }; diff --git a/test/topics.js b/test/topics.js index cbb416ae43..b1f3145ede 100644 --- a/test/topics.js +++ b/test/topics.js @@ -1470,29 +1470,18 @@ describe('Topic\'s', () => { }); }); - it('should fail with invalid data', (done) => { - socketTopics.markUnread({ uid: adminUid }, null, (err) => { - assert.equal(err.message, '[[error:invalid-data]]'); - done(); - }); + it('should fail with invalid data', async () => { + assert.rejects(apiTopics.markUnread({ uid: adminUid }, null), '[[error:invalid-data]]'); }); - it('should fail if topic does not exist', (done) => { - socketTopics.markUnread({ uid: adminUid }, 1231082, (err) => { - assert.equal(err.message, '[[error:no-topic]]'); - done(); - }); + it('should fail if topic does not exist', async () => { + assert.rejects(apiTopics.markUnread({ uid: adminUid }, { tid: 1231082 }), '[[error:no-topic]]'); }); - it('should mark topic unread', (done) => { - socketTopics.markUnread({ uid: adminUid }, tid, (err) => { - assert.ifError(err); - topics.hasReadTopic(tid, adminUid, (err, hasRead) => { - assert.ifError(err); - assert.equal(hasRead, false); - done(); - }); - }); + it('should mark topic unread', async () => { + await apiTopics.markUnread({ uid: adminUid }, { tid }); + const hasRead = await topics.hasReadTopic(tid, adminUid); + assert.equal(hasRead, false); }); it('should fail with invalid data', (done) => { @@ -1566,51 +1555,25 @@ describe('Topic\'s', () => { }); }); - it('should fail with invalid data', (done) => { - socketTopics.markAsUnreadForAll({ uid: adminUid }, null, (err) => { - assert.equal(err.message, '[[error:invalid-tid]]'); - done(); - }); + it('should fail with invalid data', async () => { + assert.rejects(apiTopics.bump({ uid: adminUid }, null, '[[error:invalid-tid]]')); }); - it('should fail with invalid data', (done) => { - socketTopics.markAsUnreadForAll({ uid: 0 }, [tid], (err) => { - assert.equal(err.message, '[[error:no-privileges]]'); - done(); - }); + it('should fail with invalid data', async () => { + assert.rejects(apiTopics.bump({ uid: 0 }, { tid: [tid] }, '[[error:no-privileges]]')); }); - it('should fail if user is not admin', (done) => { - socketTopics.markAsUnreadForAll({ uid: uid }, [tid], (err) => { - assert.equal(err.message, '[[error:no-privileges]]'); - done(); - }); + it('should fail if user is not admin', async () => { + assert.rejects(apiTopics.bump({ uid: uid }, { tid }, '[[error:no-privileges]]')); }); - it('should fail if topic does not exist', (done) => { - socketTopics.markAsUnreadForAll({ uid: uid }, [12312313], (err) => { - assert.equal(err.message, '[[error:no-topic]]'); - done(); - }); - }); + it('should mark topic unread for everyone', async () => { + await apiTopics.bump({ uid: adminUid }, { tid }); + const adminRead = await topics.hasReadTopic(tid, adminUid); + const regularRead = await topics.hasReadTopic(tid, uid); - it('should mark topic unread for everyone', (done) => { - socketTopics.markAsUnreadForAll({ uid: adminUid }, [tid], (err) => { - assert.ifError(err); - async.parallel({ - adminRead: function (next) { - topics.hasReadTopic(tid, adminUid, next); - }, - regularRead: function (next) { - topics.hasReadTopic(tid, uid, next); - }, - }, (err, results) => { - assert.ifError(err); - assert.equal(results.adminRead, false); - assert.equal(results.regularRead, false); - done(); - }); - }); + assert.equal(adminRead, false); + assert.equal(regularRead, false); }); it('should not do anything if tids is empty array', (done) => {