From 6430a191f7d291f67d3fa7788383533fce1558c2 Mon Sep 17 00:00:00 2001 From: Hazelnoot <acomputerdog@gmail.com> Date: Mon, 21 Oct 2024 14:18:34 -0400 Subject: [PATCH 1/4] fix duplicate users in the following feed --- .../server/api/endpoints/notes/following.ts | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/notes/following.ts b/packages/backend/src/server/api/endpoints/notes/following.ts index 83e8f404e9..5df52e4045 100644 --- a/packages/backend/src/server/api/endpoints/notes/following.ts +++ b/packages/backend/src/server/api/endpoints/notes/following.ts @@ -63,7 +63,32 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- .setParameter('me', me.id) // Limit to latest notes - .innerJoin(SkLatestNote, 'latest', 'note.id = latest.note_id') + .innerJoin( + (sub: SelectQueryBuilder<SkLatestNote>) => { + sub + .from(SkLatestNote, 'latest') + + // Return only one note per user + .addSelect('latest.user_id', 'user_id') + .addSelect('MAX(latest.note_id)', 'note_id') + .groupBy('latest.user_id'); + + // Match selected note types. + if (!ps.includeNonPublic) { + sub.andWhere('latest.is_public = true'); + } + if (!ps.includeReplies) { + sub.andWhere('latest.is_reply = false'); + } + if (!ps.includeQuotes) { + sub.andWhere('latest.is_quote = false'); + } + + return sub; + }, + 'latest', + 'note.id = latest.note_id', + ) // Avoid N+1 queries from the "pack" method .innerJoinAndSelect('note.user', 'user') @@ -87,17 +112,6 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- query.andWhere('note."fileIds" != \'{}\''); } - // Match selected note types. - if (!ps.includeNonPublic) { - query.andWhere('latest.is_public'); - } - if (!ps.includeReplies) { - query.andWhere('latest.is_reply = false'); - } - if (!ps.includeQuotes) { - query.andWhere('latest.is_quote = false'); - } - // Match selected user types. if (!ps.includeBots) { query.andWhere('"user"."isBot" = false'); -- GitLab From 053b47d78a59e79f0ab61baaf87c6562eefc3517 Mon Sep 17 00:00:00 2001 From: Hazelnoot <acomputerdog@gmail.com> Date: Mon, 21 Oct 2024 14:19:26 -0400 Subject: [PATCH 2/4] return error when calling following feed with both includeReplies and filesOnly --- .../src/server/api/endpoints/notes/following.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/backend/src/server/api/endpoints/notes/following.ts b/packages/backend/src/server/api/endpoints/notes/following.ts index 5df52e4045..10c211b1fc 100644 --- a/packages/backend/src/server/api/endpoints/notes/following.ts +++ b/packages/backend/src/server/api/endpoints/notes/following.ts @@ -27,6 +27,14 @@ export const meta = { ref: 'Note', }, }, + + errors: { + bothWithRepliesAndWithFiles: { + message: 'Specifying both includeReplies and filesOnly is not supported', + code: 'BOTH_INCLUDE_REPLIES_AND_FILES_ONLY', + id: '91c8cb9f-36ed-46e7-9ca2-7df96ed6e222', + }, + }, } as const; export const paramDef = { @@ -58,6 +66,8 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- private queryService: QueryService, ) { super(meta, paramDef, async (ps, me) => { + if (ps.includeReplies && ps.filesOnly) throw new ApiError(meta.errors.bothWithRepliesAndWithFiles); + const query = this.notesRepository .createQueryBuilder('note') .setParameter('me', me.id) -- GitLab From 04654b2f843b081f31ce8e5fe35b83213c0c014c Mon Sep 17 00:00:00 2001 From: Hazelnoot <acomputerdog@gmail.com> Date: Mon, 21 Oct 2024 14:20:30 -0400 Subject: [PATCH 3/4] add "followers" tab to following feed --- .../server/api/endpoints/notes/following.ts | 51 ++++++++++++++---- .../frontend/src/pages/following-feed.vue | 54 +++++++++++-------- packages/frontend/src/store.ts | 3 +- packages/misskey-js/src/autogen/types.ts | 7 ++- 4 files changed, 82 insertions(+), 33 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/notes/following.ts b/packages/backend/src/server/api/endpoints/notes/following.ts index 10c211b1fc..b6604b9798 100644 --- a/packages/backend/src/server/api/endpoints/notes/following.ts +++ b/packages/backend/src/server/api/endpoints/notes/following.ts @@ -4,12 +4,14 @@ */ import { Inject, Injectable } from '@nestjs/common'; +import { ObjectLiteral, SelectQueryBuilder } from 'typeorm'; import { SkLatestNote, MiFollowing } from '@/models/_.js'; import type { NotesRepository } from '@/models/_.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { NoteEntityService } from '@/core/entities/NoteEntityService.js'; import { DI } from '@/di-symbols.js'; import { QueryService } from '@/core/QueryService.js'; +import { ApiError } from '@/server/api/error.js'; export const meta = { tags: ['notes'], @@ -34,13 +36,19 @@ export const meta = { code: 'BOTH_INCLUDE_REPLIES_AND_FILES_ONLY', id: '91c8cb9f-36ed-46e7-9ca2-7df96ed6e222', }, + bothWithFollowersAndIncludeNonPublic: { + message: 'Specifying both list:followers and includeNonPublic is not supported', + code: 'BOTH_LIST_FOLLOWERS_AND_INCLUDE_NON_PUBLIC', + id: '7a1b9cb6-235b-4e58-9c00-32c1796f502c', + }, }, } as const; export const paramDef = { type: 'object', properties: { - mutualsOnly: { type: 'boolean', default: false }, + list: { type: 'string', enum: ['following', 'followers', 'mutuals'], default: 'following' }, + filesOnly: { type: 'boolean', default: false }, includeNonPublic: { type: 'boolean', default: false }, includeReplies: { type: 'boolean', default: false }, @@ -67,6 +75,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- ) { super(meta, paramDef, async (ps, me) => { if (ps.includeReplies && ps.filesOnly) throw new ApiError(meta.errors.bothWithRepliesAndWithFiles); + if (ps.list === 'followers' && ps.includeNonPublic) throw new ApiError(meta.errors.bothWithFollowersAndIncludeNonPublic); const query = this.notesRepository .createQueryBuilder('note') @@ -107,14 +116,15 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- .leftJoinAndSelect('reply.user', 'replyUser') .leftJoinAndSelect('renote.user', 'renoteUser') .leftJoinAndSelect('note.channel', 'channel') - - // Limit to followers - .innerJoin(MiFollowing, 'following', 'latest.user_id = following."followeeId"') - .andWhere('following."followerId" = :me'); - - // Limit to mutuals, if requested - if (ps.mutualsOnly) { - query.innerJoin(MiFollowing, 'mutuals', 'latest.user_id = mutuals."followerId" AND mutuals."followeeId" = :me'); + ; + + // Select the appropriate collection of users + if (ps.list === 'followers') { + addFollower(query); + } else if (ps.list === 'following') { + addFollowee(query); + } else { + addMutual(query); } // Limit to files, if requested @@ -143,3 +153,26 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- }); } } + +/** + * Limit to followers (they follow us) + */ +function addFollower<T extends SelectQueryBuilder<ObjectLiteral>>(query: T): T { + return query.innerJoin(MiFollowing, 'follower', 'follower."followerId" = latest.user_id AND follower."followeeId" = :me'); +} + +/** + * Limit to followees (we follow them) + */ +function addFollowee<T extends SelectQueryBuilder<ObjectLiteral>>(query: T): T { + return query.innerJoin(MiFollowing, 'followee', 'followee."followerId" = :me AND followee."followeeId" = latest.user_id'); +} + +/** + * Limit to mutuals (they follow us AND we follow them) + */ +function addMutual<T extends SelectQueryBuilder<ObjectLiteral>>(query: T): T { + addFollower(query); + addFollowee(query); + return query; +} diff --git a/packages/frontend/src/pages/following-feed.vue b/packages/frontend/src/pages/following-feed.vue index f49cafb52f..886946e867 100644 --- a/packages/frontend/src/pages/following-feed.vue +++ b/packages/frontend/src/pages/following-feed.vue @@ -5,10 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only <template> <div :class="$style.root"> - <MkPageHeader v-model:tab="currentTab" :class="$style.header" :tabs="headerTabs" :actions="headerActions" :displayBackButton="true" @update:tab="onChangeTab"/> + <MkPageHeader v-model:tab="userList" :class="$style.header" :tabs="headerTabs" :actions="headerActions" :displayBackButton="true" @update:tab="onChangeTab"/> <div ref="noteScroll" :class="$style.notes"> - <MkHorizontalSwipe v-model:tab="currentTab" :tabs="headerTabs"> + <MkHorizontalSwipe v-model:tab="userList" :tabs="headerTabs"> <MkPullToRefresh :refresher="() => reloadLatestNotes()"> <MkPagination ref="latestNotesPaging" :pagination="latestNotesPagination" @init="onListReady"> <template #empty> @@ -29,21 +29,28 @@ SPDX-License-Identifier: AGPL-3.0-only </div> <div v-if="isWideViewport" ref="userScroll" :class="$style.user"> - <MkHorizontalSwipe v-if="selectedUserId" v-model:tab="currentTab" :tabs="headerTabs"> + <MkHorizontalSwipe v-if="selectedUserId" v-model:tab="userList" :tabs="headerTabs"> <SkUserRecentNotes ref="userRecentNotes" :userId="selectedUserId" :withNonPublic="withNonPublic" :withQuotes="withQuotes" :withBots="withBots" :withReplies="withReplies" :onlyFiles="onlyFiles"/> </MkHorizontalSwipe> </div> </div> </template> +<script lang="ts"> +export const followingTab = 'following' as const; +export const mutualsTab = 'mutuals' as const; +export const followersTab = 'followers' as const; +export type FollowingFeedTab = typeof followingTab | typeof mutualsTab | typeof followersTab; +</script> + <script lang="ts" setup> import { computed, Ref, ref, shallowRef } from 'vue'; import * as Misskey from 'misskey-js'; +import { getScrollContainer } from '@@/js/scroll.js'; import { definePageMetadata } from '@/scripts/page-metadata.js'; import { i18n } from '@/i18n.js'; import MkHorizontalSwipe from '@/components/MkHorizontalSwipe.vue'; import MkPullToRefresh from '@/components/MkPullToRefresh.vue'; -import MkPagination, { Paging } from '@/components/MkPagination.vue'; import { infoImageUrl } from '@/instance.js'; import MkDateSeparatedList from '@/components/MkDateSeparatedList.vue'; import { Tab } from '@/components/global/MkPageHeader.tabs.vue'; @@ -56,12 +63,15 @@ import { $i } from '@/account.js'; import { checkWordMute } from '@/scripts/check-word-mute.js'; import SkUserRecentNotes from '@/components/SkUserRecentNotes.vue'; import { useScrollPositionManager } from '@/nirax.js'; -import { getScrollContainer } from '@@/js/scroll.js'; import { defaultStore } from '@/store.js'; import { deepMerge } from '@/scripts/merge.js'; +import MkPagination, { Paging } from '@/components/MkPagination.vue'; const withNonPublic = computed({ - get: () => defaultStore.reactiveState.followingFeed.value.withNonPublic, + get: () => { + if (userList.value === 'followers') return false; + return defaultStore.reactiveState.followingFeed.value.withNonPublic; + }, set: value => saveFollowingFilter('withNonPublic', value), }); const withQuotes = computed({ @@ -80,26 +90,19 @@ const onlyFiles = computed({ get: () => defaultStore.reactiveState.followingFeed.value.onlyFiles, set: value => saveFollowingFilter('onlyFiles', value), }); -const onlyMutuals = computed({ - get: () => defaultStore.reactiveState.followingFeed.value.onlyMutuals, - set: value => saveFollowingFilter('onlyMutuals', value), +const userList = computed({ + get: () => defaultStore.reactiveState.followingFeed.value.userList, + set: value => saveFollowingFilter('userList', value), }); // Based on timeline.saveTlFilter() -function saveFollowingFilter(key: keyof typeof defaultStore.state.followingFeed, value: boolean) { +function saveFollowingFilter<Key extends keyof typeof defaultStore.state.followingFeed>(key: Key, value: (typeof defaultStore.state.followingFeed)[Key]) { const out = deepMerge({ [key]: value }, defaultStore.state.followingFeed); defaultStore.set('followingFeed', out); } const router = useRouter(); -const followingTab = 'following' as const; -const mutualsTab = 'mutuals' as const; -const currentTab = computed({ - get: () => onlyMutuals.value ? mutualsTab : followingTab, - set: value => onlyMutuals.value = (value === mutualsTab), -}); - const userRecentNotes = shallowRef<InstanceType<typeof SkUserRecentNotes>>(); const userScroll = shallowRef<HTMLElement>(); const noteScroll = shallowRef<HTMLElement>(); @@ -137,9 +140,12 @@ async function reload() { async function onListReady(): Promise<void> { if (!selectedUserId.value && latestNotesPaging.value?.items.size) { - // This just gets the first user ID - const selectedNote: Misskey.entities.Note = latestNotesPaging.value.items.values().next().value; - selectedUserId.value = selectedNote.userId; + // This looks messy, but actually just gets the first user ID. + const selectedNote = latestNotesPaging.value.items.values().next().value; + + // We know this to be non-null because of the size check above. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + selectedUserId.value = selectedNote!.userId; } } @@ -184,7 +190,7 @@ const latestNotesPagination: Paging<'notes/following'> = { endpoint: 'notes/following' as const, limit: 20, params: computed(() => ({ - mutualsOnly: onlyMutuals.value, + list: userList.value, filesOnly: onlyFiles.value, includeNonPublic: withNonPublic.value, includeReplies: withReplies.value, @@ -208,6 +214,7 @@ const headerActions: PageHeaderItem[] = [ type: 'switch', text: i18n.ts.showNonPublicNotes, ref: withNonPublic, + disabled: userList.value === 'followers', }, { type: 'switch', @@ -250,6 +257,11 @@ const headerTabs = computed(() => [ icon: 'ph-user-switch ph-bold ph-lg', title: i18n.ts.mutuals, } satisfies Tab, + { + key: followersTab, + icon: 'ph-user ph-bold ph-lg', + title: i18n.ts.followers, + } satisfies Tab, ]); useScrollPositionManager(() => getScrollContainer(userScroll.value ?? null), router); diff --git a/packages/frontend/src/store.ts b/packages/frontend/src/store.ts index 8f356c557b..4e730e4747 100644 --- a/packages/frontend/src/store.ts +++ b/packages/frontend/src/store.ts @@ -11,6 +11,7 @@ import darkTheme from '@@/themes/d-ice.json5'; import { miLocalStorage } from './local-storage.js'; import { searchEngineMap } from './scripts/search-engine-map.js'; import type { SoundType } from '@/scripts/sound.js'; +import type { FollowingFeedTab } from '@/pages/following-feed.vue'; import { Storage } from '@/pizzax.js'; interface PostFormAction { @@ -249,7 +250,7 @@ export const defaultStore = markRaw(new Storage('base', { withBots: true, withReplies: false, onlyFiles: false, - onlyMutuals: false, + userList: 'following' as FollowingFeedTab, }, }, diff --git a/packages/misskey-js/src/autogen/types.ts b/packages/misskey-js/src/autogen/types.ts index 941c31455f..d41e7ab1c9 100644 --- a/packages/misskey-js/src/autogen/types.ts +++ b/packages/misskey-js/src/autogen/types.ts @@ -22547,8 +22547,11 @@ export type operations = { requestBody: { content: { 'application/json': { - /** @default false */ - mutualsOnly?: boolean; + /** + * @default following + * @enum {string} + */ + list?: 'following' | 'followers' | 'mutuals'; /** @default false */ filesOnly?: boolean; /** @default false */ -- GitLab From bc45ff21031aad27c5291929b77411f4f650cf56 Mon Sep 17 00:00:00 2001 From: Hazelnoot <acomputerdog@gmail.com> Date: Mon, 21 Oct 2024 17:57:54 -0400 Subject: [PATCH 4/4] add warning about incomplete remote data on following feed --- locales/index.d.ts | 4 ++++ .../frontend/src/pages/following-feed.vue | 20 ++++++++++++++++++- packages/frontend/src/store.ts | 1 + sharkey-locales/en-US.yml | 1 + 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/locales/index.d.ts b/locales/index.d.ts index 37c6d2541c..535e88f7c7 100644 --- a/locales/index.d.ts +++ b/locales/index.d.ts @@ -11334,6 +11334,10 @@ export interface Locale extends ILocale { */ "trustThisDomain": string; }; + /** + * Remote followers may have incomplete or outdated activity + */ + "remoteFollowersWarning": string; } declare const locales: { [lang: string]: Locale; diff --git a/packages/frontend/src/pages/following-feed.vue b/packages/frontend/src/pages/following-feed.vue index 886946e867..fac2857d46 100644 --- a/packages/frontend/src/pages/following-feed.vue +++ b/packages/frontend/src/pages/following-feed.vue @@ -5,7 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only <template> <div :class="$style.root"> - <MkPageHeader v-model:tab="userList" :class="$style.header" :tabs="headerTabs" :actions="headerActions" :displayBackButton="true" @update:tab="onChangeTab"/> + <div :class="$style.header"> + <MkPageHeader v-model:tab="userList" :tabs="headerTabs" :actions="headerActions" :displayBackButton="true" @update:tab="onChangeTab"/> + <MkInfo v-if="showRemoteWarning" :class="$style.remoteWarning" warn closable @close="remoteWarningDismissed = true">{{ i18n.ts.remoteFollowersWarning }}</MkInfo> + </div> <div ref="noteScroll" :class="$style.notes"> <MkHorizontalSwipe v-model:tab="userList" :tabs="headerTabs"> @@ -66,6 +69,7 @@ import { useScrollPositionManager } from '@/nirax.js'; import { defaultStore } from '@/store.js'; import { deepMerge } from '@/scripts/merge.js'; import MkPagination, { Paging } from '@/components/MkPagination.vue'; +import MkInfo from '@/components/MkInfo.vue'; const withNonPublic = computed({ get: () => { @@ -94,6 +98,10 @@ const userList = computed({ get: () => defaultStore.reactiveState.followingFeed.value.userList, set: value => saveFollowingFilter('userList', value), }); +const remoteWarningDismissed = computed({ + get: () => defaultStore.reactiveState.followingFeed.value.remoteWarningDismissed, + set: value => saveFollowingFilter('remoteWarningDismissed', value), +}); // Based on timeline.saveTlFilter() function saveFollowingFilter<Key extends keyof typeof defaultStore.state.followingFeed>(key: Key, value: (typeof defaultStore.state.followingFeed)[Key]) { @@ -107,6 +115,8 @@ const userRecentNotes = shallowRef<InstanceType<typeof SkUserRecentNotes>>(); const userScroll = shallowRef<HTMLElement>(); const noteScroll = shallowRef<HTMLElement>(); +const showRemoteWarning = computed(() => userList.value === 'followers' && !remoteWarningDismissed.value); + // We have to disable the per-user feed on small displays, and it must be done through JS instead of CSS. // Otherwise, the second column will waste resources in the background. const wideViewportQuery = window.matchMedia('(min-width: 750px)'); @@ -314,6 +324,10 @@ definePageMetadata(() => ({ overflow-y: auto; } +.remoteWarning { + margin: 12px 12px 0 12px; +} + .userInfo { margin-bottom: 12px; } @@ -328,6 +342,10 @@ definePageMetadata(() => ({ gap: 24px; } + .remoteWarning { + margin: 24px 24px 0 24px; + } + .userInfo { margin-bottom: 24px; } diff --git a/packages/frontend/src/store.ts b/packages/frontend/src/store.ts index 4e730e4747..5d0395d774 100644 --- a/packages/frontend/src/store.ts +++ b/packages/frontend/src/store.ts @@ -251,6 +251,7 @@ export const defaultStore = markRaw(new Storage('base', { withReplies: false, onlyFiles: false, userList: 'following' as FollowingFeedTab, + remoteWarningDismissed: false, }, }, diff --git a/sharkey-locales/en-US.yml b/sharkey-locales/en-US.yml index 29580df61e..63860c3eb3 100644 --- a/sharkey-locales/en-US.yml +++ b/sharkey-locales/en-US.yml @@ -383,3 +383,4 @@ _externalNavigationWarning: title: "Navigate to an external site" description: "Leave {host} and go to an external site" trustThisDomain: "Trust this domain on this device in the future" +remoteFollowersWarning: "Remote followers may have incomplete or outdated activity" -- GitLab