feat: allow 3 profile pics (#14092)

* feat: allow 3 profile pics

* test: fix notification test

* test: fix user picture test

* test: relative_path fixes

* fix: relative_path getting saved in when updating profile pic

public/src/modules/accounts/picture.js

@@ -27,7 +27,12 @@ define('accounts/picture', [
 				icon: { text: ajaxify.data['icon:text'], bgColor: ajaxify.data['icon:bgColor'] },
 				defaultAvatar: ajaxify.data.defaultAvatar,
 				allowProfileImageUploads: ajaxify.data.allowProfileImageUploads,
-				iconBackgrounds: ajaxify.data.iconBackgrounds,
+				iconBackgrounds: ajaxify.data.iconBackgrounds.map((color) => {
+					return {
+						color,
+						selected: color === ajaxify.data['icon:bgColor'],
+					};
+				}),
 				user: {
 					uid: ajaxify.data.uid,
 					username: ajaxify.data.username,
@@ -55,9 +60,8 @@ define('accounts/picture', [
 					},
 				});
 
-				modal.on('shown.bs.modal', updateImages);
-				modal.on('click', '.list-group-item', function selectImageType() {
-					modal.find('.list-group-item').removeClass('active');
+				modal.on('click', '[component="profile/picture/button"]', function selectImageType() {
+					modal.find('[component="profile/picture/button"]').removeClass('active');
 					$(this).addClass('active');
 				});
 
@@ -69,34 +73,17 @@ define('accounts/picture', [
 
 				handleImageUpload(modal);
 
-				function updateImages() {
-					// Check to see which one is the active picture
-					if (!ajaxify.data.picture) {
-						modal.find('[data-type="default"]').addClass('active');
-					} else {
-						modal.find('.list-group-item img').each(function () {
-							if (this.getAttribute('src') === ajaxify.data.picture) {
-								$(this).parents('.list-group-item').addClass('active');
-							}
-						});
-					}
-
-					// Update avatar background colour
-					const iconbgEl = modal.find(`[data-bg-color="${ajaxify.data['icon:bgColor']}"]`);
-					if (iconbgEl.length) {
-						iconbgEl.addClass('selected');
-					} else {
-						modal.find('[data-bg-color="transparent"]').addClass('selected');
-					}
-				}
-
 				function saveSelection() {
-					const type = modal.find('.list-group-item.active').attr('data-type');
+					const activeBtn = modal.find('[component="profile/picture/button"].active');
+					const type = activeBtn.attr('data-type');
+					const picture = activeBtn.find('img').attr('src');
 					const iconBgColor = modal.find('[data-bg-color].selected').attr('data-bg-color') || 'transparent';
 
-					changeUserPicture(type, iconBgColor).then(() => {
+					api.put(`/users/${ajaxify.data.theirid}/picture`, {
+						type, picture, iconBgColor,
+					}).then(() => {
 						Picture.updateHeader(
-							type === 'default' ? '' : modal.find('.list-group-item.active img').attr('src'),
+							type === 'default' ? '' : picture,
 							iconBgColor
 						);
 						ajaxify.refresh();
@@ -158,13 +145,6 @@ define('accounts/picture', [
 			}
 		}
 
-		function onRemoveComplete() {
-			if (ajaxify.data.uploadedpicture === ajaxify.data.picture) {
-				ajaxify.refresh();
-				Picture.updateHeader();
-			}
-		}
-
 		modal.find('[data-action="upload"]').on('click', function () {
 			modal.modal('hide');
 
@@ -217,21 +197,24 @@ define('accounts/picture', [
 		});
 
 		modal.find('[data-action="remove-uploaded"]').on('click', function () {
+			const removeBtn = $(this);
+			const removePicture = removeBtn.attr('data-url');
 			socket.emit('user.removeUploadedPicture', {
 				uid: ajaxify.data.theirid,
+				picture: removePicture,
 			}, function (err) {
-				modal.modal('hide');
 				if (err) {
 					return alerts.error(err);
 				}
-				onRemoveComplete();
+				removeBtn.parent().remove();
+				if (removePicture === ajaxify.data.picture) {
+					modal.modal('hide');
+					ajaxify.refresh();
+					Picture.updateHeader();
+				}
 			});
 		});
 	}
 
-	function changeUserPicture(type, bgColor) {
-		return api.put(`/users/${ajaxify.data.theirid}/picture`, { type, bgColor });
-	}
-
 	return Picture;
 });

src/api/users.js

@@ -2,7 +2,7 @@
 
 const path = require('path');
 const fs = require('fs').promises;
-
+const nconf = require('nconf');
 const validator = require('validator');
 const winston = require('winston');
 
@@ -627,7 +627,14 @@ usersAPI.changePicture = async (caller, data) => {
 	if (type === 'default') {
 		picture = '';
 	} else if (type === 'uploaded') {
-		picture = await user.getUserField(data.uid, 'uploadedpicture');
+		const cleanPath = data.picture.replace(new RegExp(`^${nconf.get('relative_path')}`), '');
+		const isUserPicture = await user.isUserUploadedPicture(data.uid, cleanPath);
+		if (isUserPicture) {
+			await user.setUserField(data.uid, 'uploadedpicture', cleanPath);
+			picture = cleanPath;
+		} else {
+			picture = '';
+		}
 	} else if (type === 'external' && url) {
 		picture = validator.escape(url);
 	} else {

src/socket.io/user/picture.js

@@ -1,5 +1,9 @@
 'use strict';
 
+const validator = require('validator');
+const nconf = require('nconf');
+
+const db = require('../../database');
 const user = require('../../user');
 const plugins = require('../../plugins');
 
@@ -10,7 +14,7 @@ module.exports = function (SocketUser) {
 		}
 		await user.isAdminOrSelf(socket.uid, data.uid);
 		// 'keepAllUserImages' is ignored, since there is explicit user intent
-		const userData = await user.removeProfileImage(data.uid);
+		const userData = await user.removeProfileImage(data.uid, data.picture);
 		plugins.hooks.fire('action:user.removeUploadedPicture', {
 			callerUid: socket.uid,
 			uid: data.uid,
@@ -23,27 +27,29 @@ module.exports = function (SocketUser) {
 			throw new Error('[[error:invalid-data]]');
 		}
 
-		const [list, userObj] = await Promise.all([
+		const [list, userObj, userPictures] = await Promise.all([
 			plugins.hooks.fire('filter:user.listPictures', {
 				uid: data.uid,
 				pictures: [],
 			}),
 			user.getUserData(data.uid),
+			db.getSortedSetRevRange(`uid:${data.uid}:profile:pictures`, 0, 2),
 		]);
 
-		if (userObj.uploadedpicture) {
+		userPictures.forEach((picture) => {
 			list.pictures.push({
 				type: 'uploaded',
-				url: userObj.uploadedpicture,
+				url: `${nconf.get('relative_path')}${picture}`,
 				text: '[[user:uploaded-picture]]',
 			});
-		}
+		});
 
 		// Normalize list into "user object" format
 		list.pictures = list.pictures.map(({ type, url, text }) => ({
 			type,
 			username: text,
-			picture: url,
+			picture: validator.escape(String(url)),
+			selected: url === userObj.picture,
 		}));
 
 		list.pictures.unshift({
@@ -51,6 +57,7 @@ module.exports = function (SocketUser) {
 			'icon:text': userObj['icon:text'],
 			'icon:bgColor': userObj['icon:bgColor'],
 			username: '[[user:default-picture]]',
+			selected: !userObj.picture,
 		});
 
 		return list.pictures;

src/upgrades/4.10.0/user-profile-pictures-zset.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const db = require('../../database');
+const batch = require('../../batch');
+
+module.exports = {
+	name: 'Add uid:<uid>:profile:pictures zset',
+	timestamp: Date.UTC(2026, 2, 13),
+	method: async function () {
+		const { progress } = this;
+		await batch.processSortedSet('users:joindate', async (uids) => {
+			const userData = await db.getObjects(uids.map(uid => `user:${uid}`));
+			const now = Date.now();
+			const bulkAdd = userData.filter(u => u && u.uploadedpicture)
+				.map(u => ([`uid:${u.uid}:profile:pictures`, now, u.uploadedpicture]));
+			await db.sortedSetAddBulk(bulkAdd);
+			progress.incr(uids.length);
+		}, {
+			batch: 500,
+			progress,
+		});
+	},
+};

src/user/data.js

@@ -376,7 +376,8 @@ module.exports = function (User) {
 		const _iconBackgrounds = [
 			'#f44336', '#e91e63', '#9c27b0', '#673ab7', '#3f51b5', '#2196f3',
 			'#009688', '#1b5e20', '#33691e', '#827717', '#e65100', '#ff5722',
-			'#795548', '#607d8b',
+			'#795548', '#607d8b', '#00bcd4', '#ffc107', '#8bc34a', '#9e9e9e',
+			'#004d40', '#ad1457',
 		];
 
 		const data = await plugins.hooks.fire('filter:user.iconBackgrounds', { iconBackgrounds: _iconBackgrounds });

src/user/delete.js

@@ -134,6 +134,7 @@ module.exports = function (User) {
 			`uid:${uid}:flag:pids`,
 			`uid:${uid}:sessions`,
 			`uid:${uid}:shares`,
+			`uid:${uid}:profile:images`,
 			`invitation:uid:${uid}`,
 		];
 

src/user/picture.js

@@ -52,7 +52,10 @@ module.exports = function (User) {
 			const filename = `${data.uid}-profilecover-${Date.now()}${extension}`;
 			const uploadData = await image.uploadImage(filename, `profile/uid-${data.uid}`, picture);
 
-			await deleteCurrentPicture(data.uid, 'cover:url');
+			if (!meta.config['profile:keepAllUserImages']) {
+				await deletePicture(data.uid, 'cover:url');
+			}
+
 			await User.setUserField(data.uid, 'cover:url', uploadData.url);
 
 			if (data.position) {
@@ -87,30 +90,11 @@ module.exports = function (User) {
 			throw new Error('[[error:invalid-image-extension]]');
 		}
 
-		const normalizedPath = await convertToPNG(userPhoto.path);
-		const isNormalized = userPhoto.path !== normalizedPath;
-
-		await image.resizeImage({
-			path: normalizedPath,
-			type: isNormalized ? 'image/png' : userPhoto.type,
-			width: meta.config.profileImageDimension,
-			height: meta.config.profileImageDimension,
+		return await storeUserUploadedPicture(data.callerUid, data.uid, {
+			path: userPhoto.path,
+			type: userPhoto.type,
+			extension,
 		});
-
-		const filename = generateProfileImageFilename(data.uid, extension);
-		const uploadedImage = await image.uploadImage(filename, `profile/uid-${data.uid}`, {
-			uid: data.uid,
-			path: normalizedPath,
-			name: 'profileAvatar',
-		});
-
-		await deleteCurrentPicture(data.uid, 'uploadedpicture');
-		await User.updateProfile(data.callerUid, {
-			uid: data.uid,
-			uploadedpicture: uploadedImage.url,
-			picture: uploadedImage.url,
-		}, ['uploadedpicture', 'picture']);
-		return uploadedImage;
 	};
 
 	// uploads image data in base64 as profile picture
@@ -133,40 +117,67 @@ module.exports = function (User) {
 			}
 
 			picture.path = await image.writeImageDataToTempFile(data.imageData);
-			const normalizedPath = await convertToPNG(picture.path);
-			const isNormalized = picture.path !== normalizedPath;
-			picture.path = normalizedPath;
-			await image.resizeImage({
+
+			return await storeUserUploadedPicture(data.callerUid, data.uid, {
 				path: picture.path,
-				type: isNormalized ? 'image/png' : type,
-				width: meta.config.profileImageDimension,
-				height: meta.config.profileImageDimension,
+				type,
+				extension,
 			});
-
-			const filename = generateProfileImageFilename(data.uid, extension);
-			const uploadedImage = await image.uploadImage(filename, `profile/uid-${data.uid}`, picture);
-
-			await deleteCurrentPicture(data.uid, 'uploadedpicture');
-			await User.updateProfile(data.callerUid, {
-				uid: data.uid,
-				uploadedpicture: uploadedImage.url,
-				picture: uploadedImage.url,
-			}, ['uploadedpicture', 'picture']);
-			return uploadedImage;
 		} finally {
 			await file.delete(picture.path);
 		}
 	};
 
-	async function deleteCurrentPicture(uid, field) {
-		if (meta.config['profile:keepAllUserImages']) {
-			return;
+	async function storeUserUploadedPicture(callerUid, updateUid, picture) {
+		const { type, extension } = picture;
+		const normalizedPath = await convertToPNG(picture.path);
+		const isNormalized = picture.path !== normalizedPath;
+
+		await image.resizeImage({
+			path: normalizedPath,
+			type: isNormalized ? 'image/png' : type,
+			width: meta.config.profileImageDimension,
+			height: meta.config.profileImageDimension,
+		});
+
+		const filename = generateProfileImageFilename(updateUid, extension);
+		const uploadedImage = await image.uploadImage(filename, `profile/uid-${updateUid}`, {
+			uid: updateUid,
+			path: picture.path,
+			name: 'profileAvatar',
+		});
+
+		await User.updateProfile(callerUid, {
+			uid: updateUid,
+			uploadedpicture: uploadedImage.url,
+			picture: uploadedImage.url,
+		}, ['uploadedpicture', 'picture']);
+
+		const zsetKey = `uid:${updateUid}:profile:pictures`;
+
+		if (!meta.config['profile:keepAllUserImages']) {
+			// if we are not keeping all images, only keep most recent 3
+			const imagesToKeep = 3;
+			const previousImages = await db.getSortedSetRevRangeWithScores(zsetKey, 0, -1);
+			const toDeleteImages = previousImages.filter((imagePath, index) => index >= imagesToKeep - 1)
+				.map(image => image.value);
+			const toRemove = [
+				...toDeleteImages.map(imagePath => ([zsetKey, imagePath])),
+			];
+
+			await db.sortedSetRemoveBulk(toRemove);
+			toDeleteImages.forEach((imagePath) => {
+				if (imagePath && !imagePath.startsWith('http')) {
+					file.delete(imagePath);
+				}
+			});
 		}
-		await deletePicture(uid, field);
+		await db.sortedSetAdd(zsetKey, Date.now(), uploadedImage.url);
+		return { url: uploadedImage.url };
 	}
 
 	async function deletePicture(uid, field) {
-		const uploadPath = await getPicturePath(uid, field);
+		const uploadPath = await getPicturePathFromUserField(uid, field);
 		if (uploadPath) {
 			await file.delete(uploadPath);
 		}
@@ -207,31 +218,56 @@ module.exports = function (User) {
 		await db.deleteObjectFields(`user:${data.uid}`, ['cover:url', 'cover:position']);
 	};
 
-	User.removeProfileImage = async function (uid) {
+	// this function expects a path without nconf.get('relative_path) prepended
+	User.isUserUploadedPicture = async (uid, picture) => {
+		return await db.isSortedSetMember(`uid:${uid}:profile:pictures`, picture);
+	};
+
+	User.removeProfileImage = async function (uid, picture) {
 		const userData = await User.getUserFields(uid, ['uploadedpicture', 'picture']);
-		await deletePicture(uid, 'uploadedpicture');
-		await User.setUserFields(uid, {
-			uploadedpicture: '',
-			// if current picture is uploaded picture, reset to user icon
-			picture: userData.uploadedpicture === userData.picture ? '' : userData.picture,
-		});
+		if (!picture) {
+			picture = userData.uploadedpicture;
+		}
+		// picture has relative_path prepended, db entries don't have it, so remove it
+		const cleanPath = picture.replace(new RegExp(`^${nconf.get('relative_path')}`), '');
+		const isUserPicture = await User.isUserUploadedPicture(uid, cleanPath);
+		if (isUserPicture) {
+			const path = getPicturePath(uid, picture);
+			await Promise.all([
+				path && !path.startsWith('http') ? file.delete(path) : null,
+				db.sortedSetRemove(`uid:${uid}:profile:pictures`, cleanPath),
+			]);
+			if (picture === userData.picture) {
+				// if deleting current uploaded picture, reset to user icon
+				await User.setUserFields(uid, {
+					uploadedpicture: '',
+					picture: '',
+				});
+			}
+		}
+
 		return userData;
 	};
 
 	User.getLocalCoverPath = async function (uid) {
-		return getPicturePath(uid, 'cover:url');
+		return await getPicturePathFromUserField(uid, 'cover:url');
 	};
 
 	User.getLocalAvatarPath = async function (uid) {
-		return getPicturePath(uid, 'uploadedpicture');
+		return await getPicturePathFromUserField(uid, 'uploadedpicture');
 	};
 
-	async function getPicturePath(uid, field) {
+	async function getPicturePathFromUserField(uid, field) {
 		const value = await User.getUserField(uid, field);
+		return getPicturePath(uid, value);
+	}
+
+	function getPicturePath(uid, value) {
 		if (!value || !value.startsWith(`${nconf.get('relative_path')}/assets/uploads/profile/uid-${uid}`)) {
 			return false;
 		}
 		const filename = value.split('/').pop();
 		return path.join(nconf.get('upload_path'), `profile/uid-${uid}`, filename);
 	}
+
 };

src/views/modals/change-picture.tpl

@@ -1,43 +1,39 @@
 <div class="row gy-2">
 	<div class="col-12 col-sm-8 col-md-6">
-		<div class="list-group">
-			{{{each pictures}}}
-			<button type="button" class="list-group-item d-flex p-3" data-type="{pictures.type}">
-				<div class="flex-shrink-0">
-					{buildAvatar(pictures, "48px", true)}
-				</div>
-				<div class="flex-grow-1 ms-3 align-self-center fs-5 text-start">
-					{pictures.username}
-				</div>
-			</button>
-			{{{end}}}
+		<div class="d-flex flex-column gap-2">
+			{{{ each pictures }}}
+			<div class="d-flex align-items-center gap-3">
+				<button component="profile/picture/button" type="button" class="btn btn-ghost border d-flex p-3 flex-grow-1 {{{ if ./selected }}}active{{{ end }}}" data-type="{./type}" data-url="{./picture}">
+					<div class="flex-shrink-0">
+						{buildAvatar(pictures, "48px", true)}
+					</div>
+					<div class="flex-grow-1 ms-3 align-self-center fs-5 text-start">
+						{./username}
+					</div>
+				</button>
+				<button class="btn btn-sm btn-ghost border {{{ if (./type != "uploaded") }}}invisible{{{ end }}}" data-action="remove-uploaded" data-url="{./picture}"><i class="text-danger fa-solid fa-trash-can"></i></button>
+			</div>
+			{{{ end }}}
 		</div>
 	</div>
 	<div class="col-12 col-sm-4 col-md-6">
-		<div class="list-group">
+		<div class="d-flex flex-column gap-2">
+			<h5>[[user:avatar-background-colour]]</h5>
+			<div class="d-flex gap-2 flex-wrap">
+				<a href="#" class="lh-1 p-1" data-bg-color="transparent"><i class="fa-solid fa-2x fa-ban text-secondary"></i></a>
+				{{{ each iconBackgrounds }}}
+				<a href="#" class="lh-1 p-1 {{{ if ./selected }}}selected{{{ end }}}" data-bg-color="{./color}" style="color: {./color};"><i class="fa-solid fa-2x fa-circle"></i></a>
+				{{{ end }}}
+			</div>
+			<hr/>
 			{{{ if allowProfileImageUploads }}}
-			<button type="button" class="list-group-item" data-action="upload">
+			<button type="button" class="btn btn-ghost border" data-action="upload">
 				[[user:upload-new-picture]]
 			</button>
 			{{{ end }}}
-			<button type="button" class="list-group-item" data-action="upload-url">
+			<button type="button" class="btn btn-ghost border" data-action="upload-url">
 				[[user:upload-new-picture-from-url]]
 			</button>
-			{{{ if uploaded }}}
-			<button type="button" class="list-group-item" data-action="remove-uploaded">
-				[[user:remove-uploaded-picture]]
-			</button>
-			{{{ end }}}
 		</div>
 	</div>
-</div>
-
-<hr />
-
-<h4>[[user:avatar-background-colour]]</h4>
-<div class="d-flex gap-2">
-	<a href="#" class="lh-1 p-1" data-bg-color="transparent"><i class="fa-solid fa-2x fa-ban text-secondary"></i></a>
-	{{{ each iconBackgrounds }}}
-	<a href="#" class="lh-1 p-1" data-bg-color="{@value}" style="color: {@value};"><i class="fa-solid fa-2x fa-circle"></i></a>
-{{{ end }}}
 </div>

test/notifications.js

@@ -75,7 +75,8 @@ describe('Notifications', () => {
 		const notifData = await notifications.get(nid);
 		assert.strictEqual(notifData.icon, undefined);
 		assert.strictEqual(notifData.user['icon:text'], 'I');
-		assert.strictEqual(notifData.user['icon:bgColor'], '#3f51b5');
+		assert(notifData.user['icon:bgColor'].length === 7 &&
+			notifData.user['icon:bgColor'].startsWith('#'));
 	});
 
 	it('should return null if pid is same and importance is lower', (done) => {

test/user.js

@@ -1028,7 +1028,8 @@ describe('User', () => {
 
 		it('should set user picture to uploaded', async () => {
 			await User.setUserField(uid, 'uploadedpicture', '/test');
-			await apiUser.changePicture({ uid: uid }, { type: 'uploaded', uid: uid });
+			await db.sortedSetAdd(`uid:${uid}:profile:pictures`, Date.now(), '/test');
+			await apiUser.changePicture({ uid: uid }, { type: 'uploaded', picture: '/test', uid: uid });
 			const picture = await User.getUserField(uid, 'picture');
 			assert.equal(picture, `${nconf.get('relative_path')}/test`);
 		});