From 50d246a1c082bbd54a4c3b81e09787ee4a4b7336 Mon Sep 17 00:00:00 2001 From: riku6460 <17585784+riku6460@users.noreply.github.com> Date: Thu, 28 Sep 2023 06:00:23 +0900 Subject: [PATCH] fix AnnouncementService --- .../backend/src/core/AnnouncementService.ts | 88 ++++++++++++++----- packages/backend/src/core/CoreModule.ts | 6 -- .../entities/AnnouncementEntityService.ts | 72 --------------- .../src/server/api/endpoints/announcements.ts | 13 +-- 4 files changed, 68 insertions(+), 111 deletions(-) delete mode 100644 packages/backend/src/core/entities/AnnouncementEntityService.ts diff --git a/packages/backend/src/core/AnnouncementService.ts b/packages/backend/src/core/AnnouncementService.ts index 8411b76324..b971498613 100644 --- 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 { - const readsQuery = this.announcementReadsRepository.createQueryBuilder('read') - .select('read.announcementId') - .where('read.userId = :userId', { userId: user.id }); + public async getUnreadAnnouncements(user: MiUser): Promise[]> { + 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, moderator?: MiUser): Promise { + 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 { + 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 { + 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 { 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[]> { - 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, })); } } diff --git a/packages/backend/src/core/CoreModule.ts b/packages/backend/src/core/CoreModule.ts index c5b07f478a..78333e70a5 100644 --- 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, diff --git a/packages/backend/src/core/entities/AnnouncementEntityService.ts b/packages/backend/src/core/entities/AnnouncementEntityService.ts deleted file mode 100644 index 476122c273..0000000000 --- a/packages/backend/src/core/entities/AnnouncementEntityService.ts +++ /dev/null @@ -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> { - 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[]> { - return (await Promise.allSettled(announcements.map(x => this.pack(x, me)))) - .filter(result => result.status === 'fulfilled') - .map(result => (result as PromiseFulfilledResult>).value); - } -} diff --git a/packages/backend/src/server/api/endpoints/announcements.ts b/packages/backend/src/server/api/endpoints/announcements.ts index dd4f82bfea..b84ee510a8 100644 --- 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 { // 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) => {