fix AnnouncementService

2023-09-28 06:00:23 +09:00
parent 3a57fb2b4b
commit 50d246a1c0
4 changed files with 68 additions and 111 deletions
--- a/packages/backend/src/core/AnnouncementService.ts
+++ b/packages/backend/src/core/AnnouncementService.ts
@@ -14,7 +14,6 @@ import { Packed } from '@/misc/json-schema.js';
 import { IdService } from '@/core/IdService.js';
 import { GlobalEventService } from '@/core/GlobalEventService.js';
 import { ModerationLogService } from '@/core/ModerationLogService.js';
-import { AnnouncementEntityService } from '@/core/entities/AnnouncementEntityService.js';
 import { UserEntityService } from '@/core/entities/UserEntityService.js';

@Injectable()
@@ -31,7 +30,6 @@ export class AnnouncementService {

 		private idService: IdService,
 		private userEntityService: UserEntityService,
-		private announcementEntityService: AnnouncementEntityService,
 		private globalEventService: GlobalEventService,
 		private moderationLogService: ModerationLogService,
 	) {
@@ -99,7 +97,7 @@ export class AnnouncementService {
 			'announcement."createdAt"': 'DESC',
 		});

-		return this.announcementEntityService.packMany(
+		return this.packMany(
 			await query
 				.limit(limit)
 				.offset(offset)
@@ -109,13 +107,22 @@ export class AnnouncementService {
 	}

 	@bindThis
-	public async getUnreadAnnouncements(user: MiUser): Promise<MiAnnouncement[]> {
-		const readsQuery = this.announcementReadsRepository.createQueryBuilder('read')
-			.select('read.announcementId')
-			.where('read.userId = :userId', { userId: user.id });
+	public async getUnreadAnnouncements(user: MiUser): Promise<Packed<'Announcement'>[]> {
+		const query = this.announcementsRepository.createQueryBuilder('announcement');
+		query.leftJoin(
+			MiAnnouncementRead,
+			'read',
+			'read."announcementId" = announcement.id AND read."userId" = :userId',
+			{ userId: user.id },
+		);
+		query.select([
+			'announcement.*',
+			'CASE WHEN read.id IS NULL THEN FALSE ELSE TRUE END as "isRead"',
+		]);
+		query.andWhere('read.id IS NULL');
+		query.andWhere('announcement."isActive" = true');

-		const q = this.announcementsRepository.createQueryBuilder('announcement')
-			.where('announcement.isActive = true')
+		query
 			.andWhere(new Brackets(qb => {
 				qb.orWhere('announcement.userId = :userId', { userId: user.id });
 				qb.orWhere('announcement.userId IS NULL');
@@ -123,17 +130,18 @@ export class AnnouncementService {
 			.andWhere(new Brackets(qb => {
 				qb.orWhere('announcement.forExistingUsers = false');
 				qb.orWhere('announcement.createdAt > :createdAt', { createdAt: user.createdAt });
-			}))
-			.andWhere(`announcement.id NOT IN (${ readsQuery.getQuery() })`);
+			}));

-		q.setParameters(readsQuery.getParameters());
-		q.orderBy({
+		query.orderBy({
 			'announcement."isActive"': 'DESC',
 			'announcement."displayOrder"': 'DESC',
 			'announcement."createdAt"': 'DESC',
 		});

-		return q.getMany();
+		return this.packMany(
+			await query.getMany(),
+			user,
+		);
 	}

 	@bindThis
@@ -239,6 +247,13 @@ export class AnnouncementService {

 	@bindThis
 	public async update(announcement: MiAnnouncement, values: Partial<MiAnnouncement>, moderator?: MiUser): Promise<void> {
+		if (announcement.userId && announcement.userId !== values.userId) {
+			await this.announcementReadsRepository.delete({
+				announcementId: announcement.id,
+				userId: announcement.userId,
+			});
+		}
+
 		await this.announcementsRepository.update(announcement.id, {
 			updatedAt: new Date(),
 			title: values.title,
@@ -252,6 +267,7 @@ export class AnnouncementService {
 			closeDuration: values.closeDuration,
 			displayOrder: values.displayOrder,
 			isActive: values.isActive,
+			userId: values.userId,
 		});

 		const after = await this.announcementsRepository.findOneByOrFail({ id: announcement.id });
@@ -279,6 +295,9 @@ export class AnnouncementService {

 	@bindThis
 	public async delete(announcement: MiAnnouncement, moderator?: MiUser): Promise<void> {
+		await this.announcementReadsRepository.delete({
+			announcementId: announcement.id,
+		});
 		await this.announcementsRepository.delete(announcement.id);

 		if (moderator) {
@@ -296,6 +315,37 @@ export class AnnouncementService {
 		}
 	}

+	@bindThis
+	public async countUnreadAnnouncements(me: MiUser): Promise<number> {
+		const query = this.announcementsRepository.createQueryBuilder('announcement');
+		query.leftJoinAndSelect(
+			MiAnnouncementRead,
+			'read',
+			'read."announcementId" = announcement.id AND read."userId" = :userId',
+			{ userId: me.id },
+		);
+		query.andWhere('read.id IS NULL');
+		query.andWhere('announcement."isActive" = true');
+
+		query
+			.andWhere(
+				new Brackets((qb) => {
+					qb.orWhere('announcement."userId" = :userId', { userId: me.id });
+					qb.orWhere('announcement."userId" IS NULL');
+				}),
+			)
+			.andWhere(
+				new Brackets((qb) => {
+					qb.orWhere('announcement."forExistingUsers" = false');
+					qb.orWhere('announcement."createdAt" > :createdAt', {
+						createdAt: me.createdAt,
+					});
+				}),
+			);
+
+		return query.getCount();
+	}
+
 	@bindThis
 	public async read(user: MiUser, announcementId: MiAnnouncement['id']): Promise<void> {
 		try {
@@ -309,20 +359,16 @@ export class AnnouncementService {
 			return;
 		}

-		if ((await this.getUnreadAnnouncements(user)).length === 0) {
+		if ((await this.countUnreadAnnouncements(user)) === 0) {
 			this.globalEventService.publishMainStream(user.id, 'readAllAnnouncements');
 		}
 	}

 	@bindThis
 	public async packMany(
-		announcements: MiAnnouncement[],
+		announcements: (MiAnnouncement & { isRead?: boolean | null })[],
 		me?: { id: MiUser['id'] } | null | undefined,
-		options?: {
-			reads?: MiAnnouncementRead[];
-		},
 	): Promise<Packed<'Announcement'>[]> {
-		const reads = me ? (options?.reads ?? await this.getReads(me.id)) : [];
 		return announcements.map(announcement => ({
 			id: announcement.id,
 			createdAt: announcement.createdAt.toISOString(),
@@ -336,7 +382,7 @@ export class AnnouncementService {
 			closeDuration: announcement.closeDuration,
 			displayOrder: announcement.displayOrder,
 			forYou: announcement.userId === me?.id,
-			isRead: reads.some(read => read.announcementId === announcement.id),
+			isRead: announcement.isRead ?? undefined,
 		}));
 	}
 }
--- a/packages/backend/src/core/CoreModule.ts
+++ b/packages/backend/src/core/CoreModule.ts
@@ -74,7 +74,6 @@ import PerUserDriveChart from './chart/charts/per-user-drive.js';
 import ApRequestChart from './chart/charts/ap-request.js';
 import { ChartManagementService } from './chart/ChartManagementService.js';
 import { AbuseUserReportEntityService } from './entities/AbuseUserReportEntityService.js';
-import { AnnouncementEntityService } from './entities/AnnouncementEntityService.js';
 import { AntennaEntityService } from './entities/AntennaEntityService.js';
 import { AppEntityService } from './entities/AppEntityService.js';
 import { AuthSessionEntityService } from './entities/AuthSessionEntityService.js';
@@ -203,7 +202,6 @@ const $ApRequestChart: Provider = { provide: 'ApRequestChart', useExisting: ApRe
 const $ChartManagementService: Provider = { provide: 'ChartManagementService', useExisting: ChartManagementService };

 const $AbuseUserReportEntityService: Provider = { provide: 'AbuseUserReportEntityService', useExisting: AbuseUserReportEntityService };
-const $AnnouncementEntityService: Provider = { provide: 'AnnouncementEntityService', useExisting: AnnouncementEntityService };
 const $AntennaEntityService: Provider = { provide: 'AntennaEntityService', useExisting: AntennaEntityService };
 const $AppEntityService: Provider = { provide: 'AppEntityService', useExisting: AppEntityService };
 const $AuthSessionEntityService: Provider = { provide: 'AuthSessionEntityService', useExisting: AuthSessionEntityService };
@@ -332,7 +330,6 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		ApRequestChart,
 		ChartManagementService,
 		AbuseUserReportEntityService,
-		AnnouncementEntityService,
 		AntennaEntityService,
 		AppEntityService,
 		AuthSessionEntityService,
@@ -456,7 +453,6 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		$ApRequestChart,
 		$ChartManagementService,
 		$AbuseUserReportEntityService,
-		$AnnouncementEntityService,
 		$AntennaEntityService,
 		$AppEntityService,
 		$AuthSessionEntityService,
@@ -580,7 +576,6 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		ApRequestChart,
 		ChartManagementService,
 		AbuseUserReportEntityService,
-		AnnouncementEntityService,
 		AntennaEntityService,
 		AppEntityService,
 		AuthSessionEntityService,
@@ -703,7 +698,6 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting
 		$ApRequestChart,
 		$ChartManagementService,
 		$AbuseUserReportEntityService,
-		$AnnouncementEntityService,
 		$AntennaEntityService,
 		$AppEntityService,
 		$AuthSessionEntityService,
--- a/packages/backend/src/core/entities/AnnouncementEntityService.ts
+++ b/packages/backend/src/core/entities/AnnouncementEntityService.ts
@@ -1,72 +0,0 @@
-/*
- * SPDX-FileCopyrightText: syuilo and other misskey contributors
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-import { Inject, Injectable } from '@nestjs/common';
-import { DI } from '@/di-symbols.js';
-import type {
-	AnnouncementReadsRepository,
-	AnnouncementsRepository,
-	MiAnnouncement,
-	MiUser,
-} from '@/models/_.js';
-import type { Packed } from '@/misc/json-schema.js';
-import { bindThis } from '@/decorators.js';
-
-@Injectable()
-export class AnnouncementEntityService {
-	constructor(
-		@Inject(DI.announcementsRepository)
-		private announcementsRepository: AnnouncementsRepository,
-
-		@Inject(DI.announcementReadsRepository)
-		private announcementReadsRepository: AnnouncementReadsRepository,
-	) {
-	}
-
-	@bindThis
-	public async pack(
-		src: MiAnnouncement['id'] | MiAnnouncement & { isRead?: boolean | null },
-		me: { id: MiUser['id'] } | null | undefined,
-	): Promise<Packed<'Announcement'>> {
-		const announcement = typeof src === 'object'
-			? src
-			: await this.announcementsRepository.findOneByOrFail({
-				id: src,
-			}) as MiAnnouncement & { isRead?: boolean | null };
-
-		if (me && announcement.isRead === undefined) {
-			announcement.isRead = await this.announcementReadsRepository.countBy({
-				announcementId: announcement.id,
-				userId: me.id,
-			}).then(count => count > 0);
-		}
-
-		return {
-			id: announcement.id,
-			createdAt: announcement.createdAt.toISOString(),
-			updatedAt: announcement.updatedAt?.toISOString() ?? null,
-			title: announcement.title,
-			text: announcement.text,
-			imageUrl: announcement.imageUrl,
-			icon: announcement.icon,
-			display: announcement.display,
-			forYou: announcement.userId === me?.id,
-			needConfirmationToRead: announcement.needConfirmationToRead,
-			closeDuration: announcement.closeDuration,
-			displayOrder: announcement.displayOrder,
-			isRead: announcement.isRead !== null ? announcement.isRead : undefined,
-		};
-	}
-
-	@bindThis
-	public async packMany(
-		announcements: (MiAnnouncement['id'] | MiAnnouncement & { isRead?: boolean | null } | MiAnnouncement)[],
-		me: { id: MiUser['id'] } | null | undefined,
-	) : Promise<Packed<'Announcement'>[]> {
-		return (await Promise.allSettled(announcements.map(x => this.pack(x, me))))
-			.filter(result => result.status === 'fulfilled')
-			.map(result => (result as PromiseFulfilledResult<Packed<'Announcement'>>).value);
-	}
-}
--- a/packages/backend/src/server/api/endpoints/announcements.ts
+++ b/packages/backend/src/server/api/endpoints/announcements.ts
@@ -3,13 +3,9 @@
 * SPDX-License-Identifier: AGPL-3.0-only
 */

-import { Inject, Injectable } from '@nestjs/common';
-import { Brackets } from 'typeorm';
+import { Injectable } from '@nestjs/common';
 import { Endpoint } from '@/server/api/endpoint-base.js';
-import { QueryService } from '@/core/QueryService.js';
 import { AnnouncementService } from '@/core/AnnouncementService.js';
-import { DI } from '@/di-symbols.js';
-import type { AnnouncementReadsRepository, AnnouncementsRepository } from '@/models/_.js';

 export const meta = {
 	tags: ['meta'],
@@ -40,13 +36,6 @@ export const paramDef = {
@Injectable()
 export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-disable-line import/no-default-export
 	constructor(
-		@Inject(DI.announcementsRepository)
-		private announcementsRepository: AnnouncementsRepository,
-
-		@Inject(DI.announcementReadsRepository)
-		private announcementReadsRepository: AnnouncementReadsRepository,
-
-		private queryService: QueryService,
 		private announcementService: AnnouncementService,
 	) {
 		super(meta, paramDef, async (ps, me) => {