[FIX] Custom status fixes (#14853)

* Fixes for status message text presence issues Added statusText to several api endpoints Changed statusMessage to statusText since that is what it is called everywhere * Fixed slash command for changing status * Fixed the "name is required" issue * Make sure the status is set blank if selecting a default status... we don't want an "online" status when someone is actually offline! * Fixes display of custom status on the room header * Changed the header of DM rooms to query the server for the user status text when it is not found on the internal collection * Changed Custom Status methods to check if the user is logged in * Improved code readability * Fix getting status list before logging in
RocketChat · Jun 27, 2019 · 24ebd2c · 24ebd2c
1 parent 0e95941
commit 24ebd2c
Show file tree

Hide file tree

Showing 24 changed files with 232 additions and 119 deletions.
diff --git a/app/api/server/v1/channels.js b/app/api/server/v1/channels.js
@@ -539,7 +539,7 @@ API.v1.addRoute('channels.members', { authRequired: true }, {
 		const members = subscriptions.fetch().map((s) => s.u && s.u._id);
 
 		const users = Users.find({ _id: { $in: members } }, {
-			fields: { _id: 1, username: 1, name: 1, status: 1, utcOffset: 1 },
+			fields: { _id: 1, username: 1, name: 1, status: 1, statusText: 1, utcOffset: 1 },
 			sort: { username: sort.username != null ? sort.username : 1 },
 		}).fetch();
 

diff --git a/app/api/server/v1/groups.js b/app/api/server/v1/groups.js
@@ -497,7 +497,7 @@ API.v1.addRoute('groups.members', { authRequired: true }, {
 		const members = subscriptions.fetch().map((s) => s.u && s.u._id);
 
 		const users = Users.find({ _id: { $in: members } }, {
-			fields: { _id: 1, username: 1, name: 1, status: 1, utcOffset: 1 },
+			fields: { _id: 1, username: 1, name: 1, status: 1, statusText: 1, utcOffset: 1 },
 			sort: { username: sort.username != null ? sort.username : 1 },
 		}).fetch();
 

diff --git a/app/api/server/v1/im.js b/app/api/server/v1/im.js
@@ -205,7 +205,7 @@ API.v1.addRoute(['dm.members', 'im.members'], { authRequired: true }, {
 		const members = cursor.fetch().map((s) => s.u && s.u.username);
 
 		const users = Users.find({ username: { $in: members } }, {
-			fields: { _id: 1, username: 1, name: 1, status: 1, utcOffset: 1 },
+			fields: { _id: 1, username: 1, name: 1, status: 1, statusText: 1, utcOffset: 1 },
 			sort: { username: sort && sort.username ? sort.username : 1 },
 		}).fetch();
 

diff --git a/app/api/server/v1/users.js b/app/api/server/v1/users.js
@@ -18,7 +18,7 @@ import {
 } from '../../../lib';
 import { getFullUserData } from '../../../lib/server/functions/getFullUserData';
 import { API } from '../api';
-import { setStatusMessage } from '../../../lib/server';
+import { setStatusText } from '../../../lib/server';
 
 API.v1.addRoute('users.create', { authRequired: true }, {
 	post() {
@@ -370,7 +370,7 @@ API.v1.addRoute('users.setStatus', { authRequired: true }, {
 
 		Meteor.runAsUser(user._id, () => {
 			if (this.bodyParams.message) {
-				setStatusMessage(user._id, this.bodyParams.message);
+				setStatusText(user._id, this.bodyParams.message);
 			}
 			if (this.bodyParams.status) {
 				const validStatus = ['online', 'away', 'offline', 'busy'];

diff --git a/app/lib/lib/roomTypes/direct.js b/app/lib/lib/roomTypes/direct.js
@@ -1,7 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { Session } from 'meteor/session';
 
-import { ChatRoom, Subscriptions, Users } from '../../../models';
+import { ChatRoom, Subscriptions } from '../../../models';
 import { openRoom } from '../../../ui-utils';
 import { getUserPreference, RoomTypeConfig, RoomTypeRouteConfig, RoomSettingsEnum, UiTextContext } from '../../../utils';
 import { hasPermission, hasAtLeastOnePermission } from '../../../authorization';
@@ -93,11 +93,12 @@ export class DirectMessageRoomType extends RoomTypeConfig {
 	}
 
 	getUserStatusText(roomId) {
-		const userId = roomId.replace(Meteor.userId(), '');
-		const userData = Users.findOne({ _id: userId });
-		if (userData && userData.statusText) {
-			return userData.statusText;
+		const subscription = Subscriptions.findOne({ rid: roomId });
+		if (subscription == null) {
+			return;
 		}
+
+		return Session.get(`user_${ subscription.name }_status_text`);
 	}
 
 	getDisplayName(room) {

diff --git a/app/lib/server/functions/getStatusText.js b/app/lib/server/functions/getStatusText.js
@@ -0,0 +1,19 @@
+import { Users } from '../../../models/server';
+
+export const getStatusText = function(userId) {
+	if (!userId) {
+		return undefined;
+	}
+
+	const fields = {
+		statusText: 1,
+	};
+
+	const options = {
+		fields,
+		limit: 1,
+	};
+
+	const data = Users.findOneById(userId, options);
+	return data && data.statusText;
+};
diff --git a/app/lib/server/functions/index.js b/app/lib/server/functions/index.js
@@ -23,7 +23,8 @@ export { saveUser } from './saveUser';
 export { sendMessage } from './sendMessage';
 export { setEmail } from './setEmail';
 export { setRealName, _setRealName } from './setRealName';
-export { setStatusMessage, _setStatusMessage } from './setStatusMessage';
+export { setStatusText, _setStatusText } from './setStatusText';
+export { getStatusText } from './getStatusText';
 export { setUserAvatar } from './setUserAvatar';
 export { _setUsername, setUsername } from './setUsername';
 export { unarchiveRoom } from './unarchiveRoom';

diff --git a/app/lib/server/functions/saveUser.js b/app/lib/server/functions/saveUser.js
@@ -10,7 +10,7 @@ import { settings } from '../../../settings';
 import PasswordPolicy from '../lib/PasswordPolicyClass';
 import { validateEmailDomain } from '../lib';
 
-import { checkEmailAvailability, checkUsernameAvailability, setUserAvatar, setEmail, setRealName, setUsername, setStatusMessage } from '.';
+import { checkEmailAvailability, checkUsernameAvailability, setUserAvatar, setEmail, setRealName, setUsername, setStatusText } from '.';
 
 const passwordPolicy = new PasswordPolicy();
 
@@ -256,7 +256,7 @@ export const saveUser = function(userId, userData) {
 	}
 
 	if (typeof userData.statusText === 'string') {
-		setStatusMessage(userData._id, userData.statusText);
+		setStatusText(userData._id, userData.statusText);
 	}
 
 	if (userData.email) {

diff --git a/app/lib/server/functions/setStatusMessage.js b/app/lib/server/functions/setStatusMessage.js
diff --git a/app/lib/server/functions/setStatusText.js b/app/lib/server/functions/setStatusText.js
@@ -0,0 +1,53 @@
+import { Meteor } from 'meteor/meteor';
+import s from 'underscore.string';
+
+import { Users } from '../../../models';
+import { Notifications } from '../../../notifications';
+import { hasPermission } from '../../../authorization';
+import { RateLimiter } from '../lib';
+
+// mirror of object in /imports/startup/client/listenActiveUsers.js - keep updated
+const STATUS_MAP = {
+	offline: 0,
+	online: 1,
+	away: 2,
+	busy: 3,
+};
+
+export const _setStatusText = function(userId, statusText) {
+	statusText = s.trim(statusText);
+	if (statusText.length > 120) {
+		statusText = statusText.substr(0, 120);
+	}
+
+	if (!userId) {
+		return false;
+	}
+
+	const user = Users.findOneById(userId);
+
+	// User already has desired statusText, return
+	if (user.statusText === statusText) {
+		return user;
+	}
+
+	// Set new statusText
+	Users.updateStatusText(user._id, statusText);
+	user.statusText = statusText;
+
+	Notifications.notifyLogged('user-status', [
+		user._id,
+		user.username,
+		STATUS_MAP[user.status],
+		statusText,
+	]);
+
+	return true;
+};
+
+export const setStatusText = RateLimiter.limitFunction(_setStatusText, 1, 60000, {
+	0() {
+		// Administrators have permission to change others status, so don't limit those
+		return !Meteor.userId() || !hasPermission(Meteor.userId(), 'edit-other-user-info');
+	},
+});
diff --git a/app/slashcommands-status/lib/status.js b/app/slashcommands-status/lib/status.js
@@ -3,40 +3,37 @@ import { TAPi18n } from 'meteor/tap:i18n';
 import { Random } from 'meteor/random';
 
 import { handleError, slashCommands } from '../../utils';
-import { hasPermission } from '../../authorization';
 import { Notifications } from '../../notifications';
 
 function Status(command, params, item) {
 	if (command === 'status') {
-		if ((Meteor.isClient && hasPermission('edit-other-user-info')) || (Meteor.isServer && hasPermission(Meteor.userId(), 'edit-other-user-info'))) {
-			const user = Meteor.users.findOne(Meteor.userId());
+		const user = Meteor.users.findOne(Meteor.userId());
 
-			Meteor.call('setUserStatus', null, params, (err) => {
-				if (err) {
-					if (Meteor.isClient) {
-						return handleError(err);
-					}
-
-					if (err.error === 'error-not-allowed') {
-						Notifications.notifyUser(Meteor.userId(), 'message', {
-							_id: Random.id(),
-							rid: item.rid,
-							ts: new Date(),
-							msg: TAPi18n.__('StatusMessage_Change_Disabled', null, user.language),
-						});
-					}
+		Meteor.call('setUserStatus', null, params, (err) => {
+			if (err) {
+				if (Meteor.isClient) {
+					return handleError(err);
+				}
 
-					throw err;
-				} else {
+				if (err.error === 'error-not-allowed') {
 					Notifications.notifyUser(Meteor.userId(), 'message', {
 						_id: Random.id(),
 						rid: item.rid,
 						ts: new Date(),
-						msg: TAPi18n.__('StatusMessage_Changed_Successfully', null, user.language),
+						msg: TAPi18n.__('StatusMessage_Change_Disabled', null, user.language),
 					});
 				}
-			});
-		}
+
+				throw err;
+			} else {
+				Notifications.notifyUser(Meteor.userId(), 'message', {
+					_id: Random.id(),
+					rid: item.rid,
+					ts: new Date(),
+					msg: TAPi18n.__('StatusMessage_Changed_Successfully', null, user.language),
+				});
+			}
+		});
 	}
 }
 

diff --git a/app/ui-flextab/client/tabs/userInfo.js b/app/ui-flextab/client/tabs/userInfo.js
@@ -8,7 +8,7 @@ import moment from 'moment';
 
 import { DateFormat } from '../../../lib';
 import { popover } from '../../../ui-utils';
-import { t, templateVarHandler } from '../../../utils';
+import { templateVarHandler } from '../../../utils';
 import { RoomRoles, UserRoles, Roles } from '../../../models';
 import { settings } from '../../../settings';
 import FullUser from '../../../models/client/models/FullUser';
@@ -83,7 +83,7 @@ Template.userInfo.helpers({
 	userStatus() {
 		const user = Template.instance().user.get();
 		const userStatus = Session.get(`user_${ user.username }_status`);
-		return userStatus || 'offline';
+		return userStatus || TAPi18n.__('offline');
 	},
 
 	userStatusText() {
@@ -92,7 +92,8 @@ Template.userInfo.helpers({
 		}
 
 		const user = Template.instance().user.get();
-		return t(Session.get(`user_${ user.username }_status`));
+		const userStatus = Session.get(`user_${ user.username }_status`);
+		return userStatus || TAPi18n.__('offline');
 	},
 
 	email() {

diff --git a/app/ui-sidenav/client/sidebarHeader.js b/app/ui-sidenav/client/sidebarHeader.js
@@ -295,7 +295,7 @@ Template.sidebarHeader.helpers({
 			};
 		}
 		return id && Meteor.users.findOne(id, { fields: {
-			username: 1, status: 1,
+			username: 1, status: 1, statusText: 1,
 		} });
 	},
 	toolbarButtons() {
@@ -316,18 +316,24 @@ Template.sidebarHeader.events({
 	'click .sidebar__header .avatar'(e) {
 		if (!(Meteor.userId() == null && settings.get('Accounts_AllowAnonymousRead'))) {
 			const user = Meteor.user();
-
+			const STATUS_MAP = [
+				'offline',
+				'online',
+				'away',
+				'busy',
+			];
 			const userStatusList = Object.keys(userStatus.list).map((key) => {
 				const status = userStatus.list[key];
-				const customName = status.localizeName ? null : status.name;
 				const name = status.localizeName ? t(status.name) : status.name;
 				const modifier = status.statusType || user.status;
+				const defaultStatus = STATUS_MAP.includes(status.id);
+				const statusText = defaultStatus ? null : name;
 
 				return {
 					icon: 'circle',
 					name,
 					modifier,
-					action: () => setStatus(status.statusType, customName),
+					action: () => setStatus(status.statusType, statusText),
 				};
 			});