Merge branch 'fix/pagination' into 'develop'

Draft: fix(backend): requested limit to be fulfilled if possible

Closes #10867

See merge request firefish/firefish!10696

.gitlab-ci.yml

@@ -185,6 +185,7 @@ cargo_clippy:
       when: never
   services: []
   before_script:
+    - apt-get update && apt-get -y upgrade
     - apt-get install -y --no-install-recommends build-essential clang mold perl
     - cp ci/cargo/config.toml /usr/local/cargo/config.toml
     - rustup component add clippy

docs/changelog.md

@@ -5,6 +5,11 @@ Critical security updates are indicated by the :warning: icon.
 - Server administrators should check [notice-for-admins.md](./notice-for-admins.md) as well.
 - Third-party client/bot developers may want to check [api-change.md](./api-change.md) as well.
 
+## Unreleased
+
+- Improve timeline UX
+- Fix bugs
+
 ## [v20240504](https://firefish.dev/firefish/firefish/-/merge_requests/10790/commits)
 
 - Fix bugs

packages/backend/src/models/repositories/note-favorite.ts

@@ -2,6 +2,7 @@ import { db } from "@/db/postgre.js";
 import { NoteFavorite } from "@/models/entities/note-favorite.js";
 import { Notes } from "../index.js";
 import type { User } from "@/models/entities/user.js";
+import Logger from "@/services/logger.js";
 
 export const NoteFavoriteRepository = db.getRepository(NoteFavorite).extend({
 	async pack(
@@ -23,9 +24,16 @@ export const NoteFavoriteRepository = db.getRepository(NoteFavorite).extend({
 	packMany(favorites: any[], me: { id: User["id"] }) {
 		return Promise.allSettled(favorites.map((x) => this.pack(x, me))).then(
 			(promises) =>
-				promises.flatMap((result) =>
-					result.status === "fulfilled" ? [result.value] : [],
-				),
+				promises.flatMap((result, i) => {
+					if (result.status === "fulfilled") {
+						return [result.value];
+					}
+					const logger = new Logger("models-note-favorite");
+					logger.error(
+						`dropping note favorite due to violating visibility restrictions, note favorite ${favorites[i].id} user ${me.id}`,
+					);
+					return [];
+				}),
 		);
 	},
 });

packages/backend/src/models/repositories/note-reaction.ts

@@ -4,6 +4,7 @@ import { Notes, Users } from "../index.js";
 import type { Packed } from "@/misc/schema.js";
 import { decodeReaction } from "backend-rs";
 import type { User } from "@/models/entities/user.js";
+import Logger from "@/services/logger.js";
 
 export const NoteReactionRepository = db.getRepository(NoteReaction).extend({
 	async pack(
@@ -49,8 +50,15 @@ export const NoteReactionRepository = db.getRepository(NoteReaction).extend({
 		);
 
 		// filter out rejected promises, only keep fulfilled values
-		return reactions.flatMap((result) =>
-			result.status === "fulfilled" ? [result.value] : [],
-		);
+		return reactions.flatMap((result, i) => {
+			if (result.status === "fulfilled") {
+				return [result.value];
+			}
+			const logger = new Logger("models-note-reaction");
+			logger.error(
+				`dropping note reaction due to violating visibility restrictions, reason is ${result.reason}`,
+			);
+			return [];
+		});
 	},
 });

packages/backend/src/models/repositories/note.ts

@@ -23,6 +23,7 @@ import {
 } from "@/misc/populate-emojis.js";
 import { db } from "@/db/postgre.js";
 import { IdentifiableError } from "@/misc/identifiable-error.js";
+import Logger from "@/services/logger.js";
 
 export async function populatePoll(note: Note, meId: User["id"] | null) {
 	const poll = await Polls.findOneByOrFail({ noteId: note.id });
@@ -343,8 +344,15 @@ export const NoteRepository = db.getRepository(Note).extend({
 		);
 
 		// filter out rejected promises, only keep fulfilled values
-		return promises.flatMap((result) =>
-			result.status === "fulfilled" ? [result.value] : [],
-		);
+		return promises.flatMap((result, i) => {
+			if (result.status === "fulfilled") {
+				return [result.value];
+			}
+			const logger = new Logger("models-note");
+			logger.error(
+				`dropping note due to violating visibility restrictions, note ${notes[i].id} user ${meId}`,
+			);
+			return [];
+		});
 	},
 });

packages/backend/src/server/activitypub/outbox.ts

@@ -73,7 +73,21 @@ export default async (ctx: Router.RouterContext) => {
 			)
 			.andWhere("note.localOnly = FALSE");
 
-		const notes = await query.take(limit).getMany();
+		// We fetch more than requested because some may be filtered out, and if there's less than
+		// requested, this is not normal behavior of any API.
+		const notes = [];
+		const take = Math.floor(limit * 1.5);
+		let skip = 0;
+		while (notes.length < limit) {
+			const notes_query = await query.take(take).skip(skip).getMany();
+			notes.push(...(await Notes.packMany(notes_query)));
+			skip += take;
+			if (notes_query.length < take) break;
+		}
+
+		if (notes.length > limit) {
+			notes.length = limit;
+		}
 
 		if (sinceId) notes.reverse();
 

packages/backend/src/server/api/endpoints/antennas/notes.ts

@@ -117,11 +117,25 @@ export default define(meta, paramDef, async (ps, user) => {
 	generateMutedUserQuery(query, user);
 	generateBlockedUserQuery(query, user);
 
-	const notes = await query.take(limit).getMany();
-
-	if (notes.length > 0) {
-		readNote(user.id, notes);
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const notes = await query.take(take).skip(skip).getMany();
+		found.push(...(await Notes.packMany(notes)));
+		skip += take;
+		if (notes.length < take) break;
 	}
 
-	return await Notes.packMany(notes, user);
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
+
+	if (found.length > 0) {
+		readNote(user.id, found);
+	}
+
+	return found;
 });

packages/backend/src/server/api/endpoints/channels/timeline.ts

@@ -76,9 +76,23 @@ export default define(meta, paramDef, async (ps, user) => {
 		.leftJoinAndSelect("note.channel", "channel");
 	//#endregion
 
-	const timeline = await query.take(ps.limit).getMany();
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const timeline = await query.take(take).skip(skip).getMany();
+		found.push(...(await Notes.packMany(timeline, user)));
+		skip += take;
+		if (timeline.length < take) break;
+	}
+
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
 
 	if (user) activeUsersChart.read(user);
 
-	return await Notes.packMany(timeline, user);
+	return found;
 });

packages/backend/src/server/api/endpoints/clips/notes.ts

@@ -88,7 +88,21 @@ export default define(meta, paramDef, async (ps, user) => {
 		generateBlockedUserQuery(query, user);
 	}
 
-	const notes = await query.take(ps.limit).getMany();
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const notes = await query.take(take).skip(skip).getMany();
+		found.push(...(await Notes.packMany(notes, user)));
+		skip += take;
+		if (notes.length < take) break;
+	}
 
-	return await Notes.packMany(notes, user);
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
+
+	return found;
 });

packages/backend/src/server/api/endpoints/notes.ts

@@ -85,7 +85,21 @@ export default define(meta, paramDef, async (ps) => {
 	//	query.isBot = bot;
 	//}
 
-	const notes = await query.take(ps.limit).getMany();
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const notes = await query.take(take).skip(skip).getMany();
+		found.push(...(await Notes.packMany(notes)));
+		skip += take;
+		if (notes.length < take) break;
+	}
 
-	return await Notes.packMany(notes);
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
+
+	return found;
 });

packages/backend/src/server/api/endpoints/notes/children.ts

@@ -57,7 +57,25 @@ export default define(meta, paramDef, async (ps, user) => {
 		generateBlockedUserQuery(query, user);
 	}
 
-	const notes = await query.getMany();
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const notes = await query.take(take).skip(skip).getMany();
+		found.push(
+			...(await Notes.packMany(notes, user, {
+				detail: false,
+			})),
+		);
+		skip += take;
+		if (notes.length < take) break;
+	}
 
-	return await Notes.packMany(notes, user, { detail: false });
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
+
+	return found;
 });

packages/backend/src/server/api/endpoints/users/notes.ts

@@ -138,7 +138,21 @@ export default define(meta, paramDef, async (ps, me) => {
 
 	//#endregion
 
-	const timeline = await query.take(ps.limit).getMany();
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const timeline = await query.take(take).skip(skip).getMany();
+		found.push(...(await Notes.packMany(timeline, user)));
+		skip += take;
+		if (timeline.length < take) break;
+	}
 
-	return await Notes.packMany(timeline, me);
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
+
+	return found;
 });

packages/backend/src/server/api/endpoints/users/reactions.ts

@@ -70,7 +70,23 @@ export default define(meta, paramDef, async (ps, me) => {
 
 	generateVisibilityQuery(query, me);
 
-	const reactions = await query.take(ps.limit).getMany();
+	// We fetch more than requested because some may be filtered out, and if there's less than
+	// requested, the pagination stops.
+	const found = [];
+	const take = Math.floor(ps.limit * 1.5);
+	let skip = 0;
+	while (found.length < ps.limit) {
+		const reactions = await query.take(take).skip(skip).getMany();
+		found.push(
+			...(await NoteReactions.packMany(reactions, me, { withNote: true })),
+		);
+		skip += take;
+		if (reactions.length < take) break;
+	}
 
-	return await NoteReactions.packMany(reactions, me, { withNote: true });
+	if (found.length > ps.limit) {
+		found.length = ps.limit;
+	}
+
+	return found;
 });

packages/client/src/components/MkNote.vue

@@ -18,8 +18,8 @@
 			class="reply-to"
 		/>
 		<MkNoteSub
-			v-else-if="!detailedView && !collapsedReply && parents"
 			v-for="n of parents"
+			v-else-if="!detailedView && !collapsedReply && parents"
 			:key="n.id"
 			:note="n"
 			class="reply-to"
@@ -354,7 +354,7 @@ import { deepClone } from "@/scripts/clone";
 import { getNoteSummary } from "@/scripts/get-note-summary";
 import icon from "@/scripts/icon";
 import type { NoteTranslation, NoteType } from "@/types/note";
-import { isRenote as _isRenote, isDeleted as _isDeleted } from "@/scripts/note";
+import { isDeleted as _isDeleted, isRenote as _isRenote } from "@/scripts/note";
 
 const props = defineProps<{
 	note: NoteType;
@@ -368,7 +368,7 @@ const props = defineProps<{
 	isLongJudger?: (note: entities.Note) => boolean;
 }>();
 
-//#region Constants
+// #region Constants
 const router = useRouter();
 const inChannel = inject("inChannel", null);
 const keymap = {
@@ -398,9 +398,9 @@ const currentClipPage = inject<Ref<entities.Clip> | null>(
 	"currentClipPage",
 	null,
 );
-//#endregion
+// #endregion
 
-//#region Variables bound to Notes
+// #region Variables bound to Notes
 let capture: ReturnType<typeof useNoteCapture> | undefined;
 const note = ref(deepClone(props.note));
 const postIsExpanded = ref(false);
@@ -408,9 +408,9 @@ const translation = ref<NoteTranslation | null>(null);
 const translating = ref(false);
 const isDeleted = ref(false);
 const renotes = ref(props.renotes?.filter((rn) => !_isDeleted(rn.id)));
-//#endregion
+// #endregion
 
-//#region computed
+// #region computed
 
 const renotesSliced = computed(() => renotes.value?.slice(0, 5));
 
@@ -470,7 +470,7 @@ const accessibleLabel = computed(() => {
 	label += `${date.toLocaleTimeString()}`;
 	return label;
 });
-//#endregion
+// #endregion
 
 async function pluginInit(newNote: NoteType) {
 	// plugin

packages/client/src/components/MkNoteSimple.vue

@@ -1,9 +1,9 @@
 <template>
 	<div
 		v-show="!deleted"
+		ref="el"
 		v-size="{ min: [350, 500] }"
 		class="yohlumlk"
-		ref="el"
 	>
 		<MkAvatar class="avatar" :user="note.user" />
 		<div class="main">
@@ -17,9 +17,9 @@
 
 <script lang="ts" setup>
 import type { entities } from "firefish-js";
+import { computed, ref, watch } from "vue";
 import XNoteHeader from "@/components/MkNoteHeader.vue";
 import MkSubNoteContent from "@/components/MkSubNoteContent.vue";
-import { computed, ref, watch } from "vue";
 import { deepClone } from "@/scripts/clone";
 import { useNoteCapture } from "@/scripts/use-note-capture";
 import { isDeleted } from "@/scripts/note";

packages/client/src/components/MkTimeline.vue

@@ -28,9 +28,9 @@
 			ref="tlComponent"
 			:no-gap="!defaultStore.state.showGapBetweenNotesInTimeline"
 			:pagination="pagination"
+			:folder
 			@queue="(x) => (queue = x)"
 			@status="pullToRefreshComponent?.setDisabled($event)"
-			:folder
 		/>
 	</MkPullToRefresh>
 	<XNotes
@@ -38,9 +38,9 @@
 		ref="tlComponent"
 		:no-gap="!defaultStore.state.showGapBetweenNotesInTimeline"
 		:pagination="pagination"
+		:folder
 		@queue="(x) => (queue = x)"
 		@status="pullToRefreshComponent?.setDisabled($event)"
-		:folder
 	/>
 </template>
 

packages/client/src/pages/settings/general.vue

@@ -14,10 +14,10 @@
 						>
 					</template>
 				</I18n>
-				<I18n :src="i18n.ts.i18nServerInfo" v-if="serverLang" tag="span">
+				<I18n v-if="serverLang" :src="i18n.ts.i18nServerInfo" tag="span">
 					<template #language><strong>{{ langs.find(a => a[0] === serverLang)?.[1] ?? serverLang }}</strong></template>
 				</I18n>
-				<button class="_textButton" @click="updateServerLang" v-if="lang && lang !== serverLang">
+				<button v-if="lang && lang !== serverLang" class="_textButton" @click="updateServerLang">
 					{{i18n.t(serverLang ? "i18nServerChange" : "i18nServerSet", { language: langs.find(a => a[0] === lang)?.[1] ?? lang })}}
 				</button>
 			</template>

packages/client/src/scripts/fold.ts

@@ -1,11 +1,11 @@
 import type { entities } from "firefish-js";
+import { isDeleted, isRenote } from "./note";
 import type {
 	FoldableNotification,
 	NotificationFolded,
 } from "@/types/notification";
-import type { NoteType, NoteThread, NoteFolded } from "@/types/note";
+import type { NoteFolded, NoteThread, NoteType } from "@/types/note";
 import { me } from "@/me";
-import { isDeleted, isRenote } from "./note";
 
 interface FoldOption {
 	/** If items length is 1, skip aggregation */

packages/client/src/types/note.ts

@@ -12,19 +12,19 @@ export type NoteType = entities.Note & {
 	_prId_?: string;
 };
 
-export type NoteFolded = {
+export interface NoteFolded {
 	id: string;
 	key: string;
 	createdAt: entities.Note["createdAt"];
 	folded: "renote";
 	note: entities.Note;
 	renotesArr: entities.Note[];
-};
+}
 
-export type NoteThread = {
+export interface NoteThread {
 	id: string;
 	createdAt: entities.Note["createdAt"];
 	folded: "thread";
 	note: entities.Note;
 	parents: entities.Note[];
-};
+}