From 4d0db37d2e5baddea3995e222bddca7032052ef1 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Mon, 20 May 2024 18:05:46 +0900 Subject: [PATCH 1/3] fix notification limit with exclude/include types (#13836) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: /i/notificationsがsinceIdのみのときに正しく動かない問題 Fix #10902 again * chore: use exclusive range to fetch data * fix: フィルタによって通知が0件だった場合でもリトライするように * docs(changelog): `/i/notifications`に includeTypes`か`excludeTypes`を指定しているとき、通知が存在するのに空配列を返すことがある問題を修正 --- CHANGELOG.md | 1 + .../server/api/endpoints/i/notifications.ts | 60 +++++++++++++------ 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 280c63b2a7..0a19f32db6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,7 @@ - Fix: リノートをミュートしたユーザの投稿のリノートがミュートされる問題を修正 - Fix: AP Link等は添付ファイル扱いしないようになど (#13754) - Fix: FTTが有効かつsinceIdのみを指定した場合に帰って来るレスポンスが逆順である問題を修正 +- Fix: `/i/notifications`に `includeTypes`か`excludeTypes`を指定しているとき、通知が存在するのに空配列を返すことがある問題を修正 ## 2024.3.1 diff --git a/packages/backend/src/server/api/endpoints/i/notifications.ts b/packages/backend/src/server/api/endpoints/i/notifications.ts index 320d9fdb00..2f619380e9 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications.ts @@ -7,7 +7,7 @@ import { In } from 'typeorm'; import * as Redis from 'ioredis'; import { Inject, Injectable } from '@nestjs/common'; import type { NotesRepository } from '@/models/_.js'; -import { obsoleteNotificationTypes, notificationTypes, FilterUnionByProperty } from '@/types.js'; +import { FilterUnionByProperty, notificationTypes, obsoleteNotificationTypes } from '@/types.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteReadService } from '@/core/NoteReadService.js'; import { NotificationEntityService } from '@/core/entities/NotificationEntityService.js'; @@ -84,27 +84,51 @@ export default class extends Endpoint { // eslint- const includeTypes = ps.includeTypes && ps.includeTypes.filter(type => !(obsoleteNotificationTypes).includes(type as any)) as typeof notificationTypes[number][]; const excludeTypes = ps.excludeTypes && ps.excludeTypes.filter(type => !(obsoleteNotificationTypes).includes(type as any)) as typeof notificationTypes[number][]; - const limit = ps.limit + (ps.untilId ? 1 : 0) + (ps.sinceId ? 1 : 0); // untilIdに指定したものも含まれるため+1 - const notificationsRes = await this.redisClient.xrevrange( - `notificationTimeline:${me.id}`, - ps.untilId ? this.idService.parse(ps.untilId).date.getTime() : '+', - ps.sinceId ? this.idService.parse(ps.sinceId).date.getTime() : '-', - 'COUNT', limit); + let sinceTime = ps.sinceId ? this.idService.parse(ps.sinceId).date.getTime().toString() : null; + let untilTime = ps.untilId ? this.idService.parse(ps.untilId).date.getTime().toString() : null; - if (notificationsRes.length === 0) { - return []; - } + let notifications: MiNotification[]; + for (;;) { + let notificationsRes: [id: string, fields: string[]][]; - let notifications = notificationsRes.map(x => JSON.parse(x[1][1])).filter(x => x.id !== ps.untilId && x !== ps.sinceId) as MiNotification[]; + // sinceidのみの場合は古い順、そうでない場合は新しい順。 QueryService.makePaginationQueryも参照 + if (sinceTime && !untilTime) { + notificationsRes = await this.redisClient.xrange( + `notificationTimeline:${me.id}`, + '(' + sinceTime, + '+', + 'COUNT', ps.limit); + } else { + notificationsRes = await this.redisClient.xrevrange( + `notificationTimeline:${me.id}`, + untilTime ? '(' + untilTime : '+', + sinceTime ? '(' + sinceTime : '-', + 'COUNT', ps.limit); + } - if (includeTypes && includeTypes.length > 0) { - notifications = notifications.filter(notification => includeTypes.includes(notification.type)); - } else if (excludeTypes && excludeTypes.length > 0) { - notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); - } + if (notificationsRes.length === 0) { + return []; + } - if (notifications.length === 0) { - return []; + notifications = notificationsRes.map(x => JSON.parse(x[1][1])) as MiNotification[]; + + if (includeTypes && includeTypes.length > 0) { + notifications = notifications.filter(notification => includeTypes.includes(notification.type)); + } else if (excludeTypes && excludeTypes.length > 0) { + notifications = notifications.filter(notification => !excludeTypes.includes(notification.type)); + } + + if (notifications.length !== 0) { + // 通知が１件以上ある場合は返す + break; + } + + // フィルタしたことで通知が0件になった場合、次のページを取得する + if (ps.sinceId && !ps.untilId) { + sinceTime = notificationsRes[notificationsRes.length - 1][0]; + } else { + untilTime = notificationsRes[notificationsRes.length - 1][0]; + } } // Mark all as read From f6df94070b0fc1ca862d560b17488bd718c2ec85 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Mon, 20 May 2024 18:08:20 +0900 Subject: [PATCH 2/3] Exclude channel notes from featured polls (#13838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(backend): add `channelId` to `MiPoll` as a Denormalized field * feat(backend): option to exclude polls in channels * chore: exclude channel notes from featured polls * docs(changelog): みつけるのアンケート欄にてチャンネルのアンケートが含まれてしまう問題を修正 * fix: missing license header --- CHANGELOG.md | 1 + ...29964060-ChannelIdDenormalizedForMiPoll.js | 21 +++++++++++++++++++ .../backend/src/core/NoteCreateService.ts | 1 + packages/backend/src/models/Poll.ts | 9 ++++++++ .../endpoints/notes/polls/recommendation.ts | 7 +++++++ .../frontend/src/pages/explore.featured.vue | 3 +++ packages/misskey-js/src/autogen/types.ts | 2 ++ 7 files changed, 44 insertions(+) create mode 100644 packages/backend/migration/1716129964060-ChannelIdDenormalizedForMiPoll.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a19f32db6..d0b98db96a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - 「アカウントを見つけやすくする」が有効なユーザーか - Fix: Play作成時に設定した公開範囲が機能していない問題を修正 - Fix: 正規化されていない状態のhashtagが連合されてきたhtmlに含まれているとhashtagが正しくhashtagに復元されない問題を修正 +- Fix: みつけるのアンケート欄にてチャンネルのアンケートが含まれてしまう問題を修正 ### Client - Feat: アップロードするファイルの名前をランダム文字列にできるように diff --git a/packages/backend/migration/1716129964060-ChannelIdDenormalizedForMiPoll.js b/packages/backend/migration/1716129964060-ChannelIdDenormalizedForMiPoll.js new file mode 100644 index 0000000000..f736378c04 --- /dev/null +++ b/packages/backend/migration/1716129964060-ChannelIdDenormalizedForMiPoll.js @@ -0,0 +1,21 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +export class ChannelIdDenormalizedForMiPoll1716129964060 { + name = 'ChannelIdDenormalizedForMiPoll1716129964060' + + async up(queryRunner) { + await queryRunner.query(`ALTER TABLE "poll" ADD "channelId" character varying(32)`); + await queryRunner.query(`COMMENT ON COLUMN "poll"."channelId" IS '[Denormalized]'`); + await queryRunner.query(`CREATE INDEX "IDX_c1240fcc9675946ea5d6c2860e" ON "poll" ("channelId") `); + await queryRunner.query(`UPDATE "poll" SET "channelId" = "note"."channelId" FROM "note" WHERE "poll"."noteId" = "note"."id"`); + } + + async down(queryRunner) { + await queryRunner.query(`DROP INDEX "public"."IDX_c1240fcc9675946ea5d6c2860e"`); + await queryRunner.query(`COMMENT ON COLUMN "poll"."channelId" IS '[Denormalized]'`); + await queryRunner.query(`ALTER TABLE "poll" DROP COLUMN "channelId"`); + } +} diff --git a/packages/backend/src/core/NoteCreateService.ts b/packages/backend/src/core/NoteCreateService.ts index 32104fea90..e5580f36d1 100644 --- a/packages/backend/src/core/NoteCreateService.ts +++ b/packages/backend/src/core/NoteCreateService.ts @@ -473,6 +473,7 @@ export class NoteCreateService implements OnApplicationShutdown { noteVisibility: insert.visibility, userId: user.id, userHost: user.host, + channelId: insert.channelId, }); await transactionalEntityManager.insert(MiPoll, poll); diff --git a/packages/backend/src/models/Poll.ts b/packages/backend/src/models/Poll.ts index c2693dbb19..ca985c8b24 100644 --- a/packages/backend/src/models/Poll.ts +++ b/packages/backend/src/models/Poll.ts @@ -8,6 +8,7 @@ import { noteVisibilities } from '@/types.js'; import { id } from './util/id.js'; import { MiNote } from './Note.js'; import type { MiUser } from './User.js'; +import type { MiChannel } from "@/models/Channel.js"; @Entity('poll') export class MiPoll { @@ -58,6 +59,14 @@ export class MiPoll { comment: '[Denormalized]', }) public userHost: string | null; + + @Index() + @Column({ + ...id(), + nullable: true, + comment: '[Denormalized]', + }) + public channelId: MiChannel['id'] | null; //#endregion constructor(data: Partial) { diff --git a/packages/backend/src/server/api/endpoints/notes/polls/recommendation.ts b/packages/backend/src/server/api/endpoints/notes/polls/recommendation.ts index ba38573065..4fd6f8682d 100644 --- a/packages/backend/src/server/api/endpoints/notes/polls/recommendation.ts +++ b/packages/backend/src/server/api/endpoints/notes/polls/recommendation.ts @@ -32,6 +32,7 @@ export const paramDef = { properties: { limit: { type: 'integer', minimum: 1, maximum: 100, default: 10 }, offset: { type: 'integer', default: 0 }, + excludeChannels: { type: 'boolean', default: false }, }, required: [], } as const; @@ -86,6 +87,12 @@ export default class extends Endpoint { // eslint- query.setParameters(mutingQuery.getParameters()); //#endregion + //#region exclude channels + if (ps.excludeChannels) { + query.andWhere('poll.channelId IS NULL'); + } + //#endregion + const polls = await query .orderBy('poll.noteId', 'DESC') .limit(ps.limit) diff --git a/packages/frontend/src/pages/explore.featured.vue b/packages/frontend/src/pages/explore.featured.vue index b5c8e70166..cfdb235d3a 100644 --- a/packages/frontend/src/pages/explore.featured.vue +++ b/packages/frontend/src/pages/explore.featured.vue @@ -29,6 +29,9 @@ const paginationForPolls = { endpoint: 'notes/polls/recommendation' as const, limit: 10, offsetMode: true, + params: { + excludeChannels: true, + }, }; const tab = ref('notes'); diff --git a/packages/misskey-js/src/autogen/types.ts b/packages/misskey-js/src/autogen/types.ts index 1b9f1304d5..302587ccfa 100644 --- a/packages/misskey-js/src/autogen/types.ts +++ b/packages/misskey-js/src/autogen/types.ts @@ -21019,6 +21019,8 @@ export type operations = { limit?: number; /** @default 0 */ offset?: number; + /** @default false */ + excludeChannels?: boolean; }; }; }; From ed74f7b4a88223fe11b63d424bb0f90768a88926 Mon Sep 17 00:00:00 2001 From: anatawa12 Date: Mon, 20 May 2024 18:55:42 +0900 Subject: [PATCH 3/3] ci: use pnpm version from packageManager field in the package.json. (#13825) --- .github/workflows/check-misskey-js-autogen.yml | 4 +--- .github/workflows/get-api-diff.yml | 5 +---- .github/workflows/lint.yml | 15 +++------------ .github/workflows/on-release-created.yml | 5 +---- .github/workflows/storybook.yml | 5 +---- .github/workflows/test-backend.yml | 10 ++-------- .github/workflows/test-frontend.yml | 10 ++-------- .github/workflows/test-production.yml | 5 +---- .github/workflows/validate-api-json.yml | 5 +---- 9 files changed, 13 insertions(+), 51 deletions(-) diff --git a/.github/workflows/check-misskey-js-autogen.yml b/.github/workflows/check-misskey-js-autogen.yml index 9052b2e372..39acad8bc3 100644 --- a/.github/workflows/check-misskey-js-autogen.yml +++ b/.github/workflows/check-misskey-js-autogen.yml @@ -24,9 +24,7 @@ jobs: ref: refs/pull/${{ github.event.pull_request.number }}/merge - name: setup pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 + uses: pnpm/action-setup@v4 - name: setup node id: setup-node diff --git a/.github/workflows/get-api-diff.yml b/.github/workflows/get-api-diff.yml index 146e0686e5..9b9c8f11c4 100644 --- a/.github/workflows/get-api-diff.yml +++ b/.github/workflows/get-api-diff.yml @@ -32,10 +32,7 @@ jobs: ref: ${{ matrix.ref }} submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9a269014ab..76616ec5a7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -27,10 +27,7 @@ jobs: with: fetch-depth: 0 submodules: true - - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + - uses: pnpm/action-setup@v4 - uses: actions/setup-node@v4.0.2 with: node-version-file: '.node-version' @@ -54,10 +51,7 @@ jobs: with: fetch-depth: 0 submodules: true - - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + - uses: pnpm/action-setup@v4 - uses: actions/setup-node@v4.0.2 with: node-version-file: '.node-version' @@ -80,10 +74,7 @@ jobs: with: fetch-depth: 0 submodules: true - - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + - uses: pnpm/action-setup@v4 - uses: actions/setup-node@v4.0.2 with: node-version-file: '.node-version' diff --git a/.github/workflows/on-release-created.yml b/.github/workflows/on-release-created.yml index 52463d7542..edfdab99e9 100644 --- a/.github/workflows/on-release-created.yml +++ b/.github/workflows/on-release-created.yml @@ -24,10 +24,7 @@ jobs: with: submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: diff --git a/.github/workflows/storybook.yml b/.github/workflows/storybook.yml index 3bc354b331..c52883ffdd 100644 --- a/.github/workflows/storybook.yml +++ b/.github/workflows/storybook.yml @@ -34,10 +34,7 @@ jobs: echo "base=$(git rev-list --parents -n1 HEAD | cut -d" " -f2)" >> $GITHUB_OUTPUT git checkout $(git rev-list --parents -n1 HEAD | cut -d" " -f3) - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js 20.x uses: actions/setup-node@v4.0.2 with: diff --git a/.github/workflows/test-backend.yml b/.github/workflows/test-backend.yml index 525cd0916b..b1c54bb3e7 100644 --- a/.github/workflows/test-backend.yml +++ b/.github/workflows/test-backend.yml @@ -41,10 +41,7 @@ jobs: with: submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Install FFmpeg uses: FedericoCarboni/setup-ffmpeg@v3 - name: Use Node.js ${{ matrix.node-version }} @@ -93,10 +90,7 @@ jobs: with: submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: diff --git a/.github/workflows/test-frontend.yml b/.github/workflows/test-frontend.yml index 9df3c98393..9d5053b82a 100644 --- a/.github/workflows/test-frontend.yml +++ b/.github/workflows/test-frontend.yml @@ -33,10 +33,7 @@ jobs: with: submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: @@ -91,10 +88,7 @@ jobs: #- uses: browser-actions/setup-firefox@latest # if: ${{ matrix.browser == 'firefox' }} - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: diff --git a/.github/workflows/test-production.yml b/.github/workflows/test-production.yml index 24a530e073..7f8db65293 100644 --- a/.github/workflows/test-production.yml +++ b/.github/workflows/test-production.yml @@ -23,10 +23,7 @@ jobs: with: submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: diff --git a/.github/workflows/validate-api-json.yml b/.github/workflows/validate-api-json.yml index 229c447893..24340e7d81 100644 --- a/.github/workflows/validate-api-json.yml +++ b/.github/workflows/validate-api-json.yml @@ -24,10 +24,7 @@ jobs: with: submodules: true - name: Install pnpm - uses: pnpm/action-setup@v3 - with: - version: 9 - run_install: false + uses: pnpm/action-setup@v4 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4.0.2 with: