fix: user-specific banners failing with Banner not found error when dismissing it

.changeset/some-banks-spend.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/models': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes `POST /v1/banners.dismiss` failing with `Banner not found` for banners stored in the user's record (such as the version update ones), which were never marked as read. The endpoint now marks them as read as the deprecated `banner/dismiss` method did, and only fails when the banner does not exist in the banners collection nor in the user's record.

apps/meteor/server/services/banner/service.ts

@@ -5,6 +5,8 @@ import type { IBannerService } from '@rocket.chat/core-services';
 import type { BannerPlatform, IBanner, IBannerDismiss, Optional, IUser } from '@rocket.chat/core-typings';
 import { Banners, BannersDismiss, Users } from '@rocket.chat/models';
 
+import { notifyOnUserChange } from '../../lib/notifyListener';
+
 export class BannerService extends ServiceClassInternal implements IBannerService {
 	protected name = 'banner';
 
@@ -87,7 +89,21 @@ export class BannerService extends ServiceClassInternal implements IBannerServic
 
 		const banner = await Banners.findOneById(bannerId);
 		if (!banner) {
-			throw new Error('Banner not found');
+			const { matchedCount } = await Users.setBannerReadById(userId, bannerId);
+
+			if (!matchedCount) {
+				throw new Error('Banner not found');
+			}
+
+			void notifyOnUserChange({
+				id: userId,
+				clientAction: 'updated',
+				diff: {
+					[`banners.${bannerId}.read`]: true,
+				},
+			});
+
+			return true;
 		}
 
 		const user = await Users.findOneById<Pick<IUser, 'username' | '_id'>>(userId, {

apps/meteor/tests/end-to-end/api/banners.ts

@@ -1,7 +1,11 @@
+import type { IUser } from '@rocket.chat/core-typings';
 import { expect } from 'chai';
-import { before, describe, it } from 'mocha';
+import { after, before, describe, it } from 'mocha';
+import { MongoClient } from 'mongodb';
 
 import { getCredentials, api, request, credentials } from '../../data/api-data';
+import { getMe } from '../../data/users.helper';
+import { URL_MONGODB } from '../../e2e/config/constants';
 
 describe('banners', () => {
 	before((done) => getCredentials(done));
@@ -49,6 +53,99 @@ describe('banners', () => {
 
 			expect(res.body).to.have.property('success', false);
 		});
+
+		describe('banners stored in the user record', () => {
+			let connection: MongoClient;
+
+			const bannerId = 'alert-user-banner-test';
+
+			const getUserBanners = async () => (await getMe<IUser>(credentials)).banners;
+
+			before(async () => {
+				connection = await MongoClient.connect(URL_MONGODB);
+
+				await connection
+					.db()
+					.collection<IUser>('users')
+					.updateOne(
+						{ _id: 'rocketchat.internal.admin.test' },
+						{
+							$set: {
+								[`banners.${bannerId}`]: {
+									id: bannerId,
+									priority: 10,
+									title: 'Banner_Title',
+									text: 'Banner_Text',
+									textArguments: [],
+									modifiers: [],
+									link: 'https://rocket.chat',
+								},
+							},
+						},
+					);
+			});
+
+			after(async () => {
+				await connection
+					.db()
+					.collection<IUser>('users')
+					.updateOne(
+						{ _id: 'rocketchat.internal.admin.test' },
+						{
+							$unset: { [`banners.${bannerId}`]: 1 },
+						},
+					);
+				await connection.close();
+			});
+
+			it('should mark the banner as read on the user record', async () => {
+				const res = await request
+					.post(api('banners.dismiss'))
+					.set(credentials)
+					.send({
+						bannerId,
+					})
+					.expect(200);
+
+				expect(res.body).to.have.property('success', true);
+
+				const banners = await getUserBanners();
+
+				expect(banners).to.have.nested.property(`${bannerId}.read`, true);
+			});
+
+			it('should succeed if the banner was already dismissed', async () => {
+				const res = await request
+					.post(api('banners.dismiss'))
+					.set(credentials)
+					.send({
+						bannerId,
+					})
+					.expect(200);
+
+				expect(res.body).to.have.property('success', true);
+
+				const banners = await getUserBanners();
+
+				expect(banners).to.have.nested.property(`${bannerId}.read`, true);
+			});
+
+			it('should not add an unknown banner to the user record', async () => {
+				const res = await request
+					.post(api('banners.dismiss'))
+					.set(credentials)
+					.send({
+						bannerId: 'an-unknown-banner-id',
+					})
+					.expect(400);
+
+				expect(res.body).to.have.property('success', false);
+
+				const banners = await getUserBanners();
+
+				expect(banners).to.not.have.property('an-unknown-banner-id');
+			});
+		});
 	});
 
 	describe('[/banners]', () => {

packages/models/src/models/Users.ts

@@ -2912,13 +2912,19 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
 	}
 
 	setBannerReadById(_id: IUser['_id'], bannerId: string) {
+		const query = {
+			_id,
+			[`banners.${bannerId}`]: {
+				$exists: true,
+			},
+		};
 		const update = {
 			$set: {
 				[`banners.${bannerId}.read`]: true,
 			},
 		};
 
-		return this.updateOne({ _id }, update);
+		return this.updateOne(query, update);
 	}
 
 	removeBannerById(_id: IUser['_id'], bannerId: string) {