From ef7cde6bc6a2159f0fd041d26a3cb77cb0d53be9 Mon Sep 17 00:00:00 2001 From: Hazel K Date: Wed, 2 Oct 2024 11:38:21 -0400 Subject: [PATCH] fixes from peer review --- .../backend/src/core/NoteCreateService.ts | 5 +++-- .../backend/src/core/NoteDeleteService.ts | 20 +++++++++++------ .../src/components/FollowingFeedEntry.vue | 2 +- packages/frontend/src/navbar.ts | 2 +- .../frontend/src/pages/following-feed.vue | 22 +++++++++---------- packages/frontend/src/pages/timeline.vue | 2 +- packages/frontend/vite.replaceIcons.ts | 1 - 7 files changed, 30 insertions(+), 24 deletions(-) diff --git a/packages/backend/src/core/NoteCreateService.ts b/packages/backend/src/core/NoteCreateService.ts index 0af65b81b1..daf0894cfd 100644 --- a/packages/backend/src/core/NoteCreateService.ts +++ b/packages/backend/src/core/NoteCreateService.ts @@ -1134,7 +1134,8 @@ export class NoteCreateService implements OnApplicationShutdown { } private async updateLatestNote(note: MiNote) { - // Ignore DMs + // Ignore DMs. + // Followers-only posts are *included*, as this table is used to back the "following" feed. if (note.visibility === 'specified') return; // Ignore pure renotes @@ -1143,7 +1144,7 @@ export class NoteCreateService implements OnApplicationShutdown { // Make sure that this isn't an *older* post. // We can get older posts through replies, lookups, etc. const currentLatest = await this.latestNotesRepository.findOneBy({ userId: note.userId }); - if (currentLatest != null && currentLatest.userId >= note.id) return; + if (currentLatest != null && currentLatest.noteId >= note.id) return; // Record this as the latest note for the given user const latestNote = new LatestNote({ diff --git a/packages/backend/src/core/NoteDeleteService.ts b/packages/backend/src/core/NoteDeleteService.ts index de753a3aa2..3f86f41942 100644 --- a/packages/backend/src/core/NoteDeleteService.ts +++ b/packages/backend/src/core/NoteDeleteService.ts @@ -240,6 +240,10 @@ export class NoteDeleteService { // If it's a DM, then it can't possibly be the latest note so we can safely skip this. if (note.visibility === 'specified') return; + // Check if the deleted note was possibly the latest for the user + const hasLatestNote = await this.latestNotesRepository.existsBy({ userId: note.userId }); + if (hasLatestNote) return; + // Find the newest remaining note for the user. // We exclude DMs and pure renotes. const nextLatest = await this.notesRepository @@ -269,12 +273,14 @@ export class NoteDeleteService { noteId: nextLatest.id, }); - // We use an upsert because this deleted note might not have been the newest. - // In that case, the latest note may already be populated for this user. - // We want postgres to do nothing instead of replacing the value or returning an error. - await this.latestNotesRepository.upsert(latestNote, { - conflictPaths: ['userId'], - skipUpdateIfNoValuesChanged: true, - }); + // When inserting the latest note, it's possible that another worker has "raced" the insert and already added a newer note. + // We must use orIgnore() to ensure that the query ignores conflicts, otherwise an exception may be thrown. + await this.latestNotesRepository + .createQueryBuilder('latest') + .insert() + .into(LatestNote) + .values(latestNote) + .orIgnore() + .execute(); } } diff --git a/packages/frontend/src/components/FollowingFeedEntry.vue b/packages/frontend/src/components/FollowingFeedEntry.vue index 29434de021..7f5abaa9cc 100644 --- a/packages/frontend/src/components/FollowingFeedEntry.vue +++ b/packages/frontend/src/components/FollowingFeedEntry.vue @@ -18,7 +18,7 @@ SPDX-License-Identifier: AGPL-3.0-only
- +
diff --git a/packages/frontend/src/navbar.ts b/packages/frontend/src/navbar.ts index 2d67a29a24..b6385b5ad2 100644 --- a/packages/frontend/src/navbar.ts +++ b/packages/frontend/src/navbar.ts @@ -70,7 +70,7 @@ export const navbarItemDef = reactive({ }, following: { title: i18n.ts.following, - icon: 'ti ti-user-check', + icon: 'ph-user-check ph-bold ph-lg', to: '/following-feed', }, lists: { diff --git a/packages/frontend/src/pages/following-feed.vue b/packages/frontend/src/pages/following-feed.vue index 56f722e9d3..9a78cbdadf 100644 --- a/packages/frontend/src/pages/following-feed.vue +++ b/packages/frontend/src/pages/following-feed.vue @@ -26,7 +26,7 @@ SPDX-License-Identifier: AGPL-3.0-only - +
@@ -62,7 +62,7 @@ import FollowingFeedEntry from '@/components/FollowingFeedEntry.vue'; import MkNotes from '@/components/MkNotes.vue'; import MkUserInfo from '@/components/MkUserInfo.vue'; import { misskeyApi } from '@/scripts/misskey-api.js'; -import {useRouter} from "@/router/supplier.js"; +import { useRouter } from '@/router/supplier.js'; const props = withDefaults(defineProps<{ initialTab?: FollowingFeedTab, @@ -79,17 +79,17 @@ const currentTab: Ref = ref(props.initialTab); const mutualsOnly: Ref = computed(() => currentTab.value === mutualsTab); // 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 resources in the background. -const desktopMediaQuery = window.matchMedia('(min-width: 750px)'); -const isDesktop: Ref = ref(desktopMediaQuery.matches); -desktopMediaQuery.addEventListener('change', () => isDesktop.value = desktopMediaQuery.matches); +// Otherwise, the second column will waste resources in the background. +const wideViewportQuery = window.matchMedia('(min-width: 750px)'); +const isWideViewport: Ref = ref(wideViewportQuery.matches); +wideViewportQuery.addEventListener('change', () => isWideViewport.value = wideViewportQuery.matches); const selectedUserError: Ref = ref(''); const selectedUserId: Ref = ref(''); const selectedUser: Ref = ref(null); async function userSelected(user: Misskey.entities.UserLite): Promise { - if (isDesktop.value) { + if (isWideViewport.value) { await showUserNotes(user.id); } else { if (user.host) { @@ -139,7 +139,7 @@ async function onListReady(): Promise { // This just gets the first user ID const selectedNote: Misskey.entities.Note = latestNotesPaging.value.items.values().next().value; - // Wait for 1 second to match the animation effects. + // Wait for 1 second to match the animation effects in MkHorizontalSwipe, MkPullToRefresh, and MkPagination. // Otherwise, the page appears to load "backwards". await new Promise(resolve => setTimeout(resolve, 1000)); await showUserNotes(selectedNote.userId); @@ -179,19 +179,19 @@ const headerActions: PageHeaderItem[] = [ const headerTabs = computed(() => [ { key: followingTab, - icon: 'ti ti-user-check', + icon: 'ph-user-check ph-bold ph-lg', title: i18n.ts.following, } satisfies Tab, { key: mutualsTab, - icon: 'ti ti-user-heart', + icon: 'ph-user-switch ph-bold ph-lg', title: i18n.ts.mutuals, } satisfies Tab, ]); definePageMetadata(() => ({ title: i18n.ts.following, - icon: 'ti ti-user-check', + icon: 'ph-user-check ph-bold ph-lg', })); diff --git a/packages/frontend/src/pages/timeline.vue b/packages/frontend/src/pages/timeline.vue index 7dc63a887a..55e453b38a 100644 --- a/packages/frontend/src/pages/timeline.vue +++ b/packages/frontend/src/pages/timeline.vue @@ -312,7 +312,7 @@ const headerTabs = computed(() => [...(defaultStore.reactiveState.pinnedUserList icon: basicTimelineIconClass(tl), iconOnly: true, })), { - icon: 'ti ti-user-check', + icon: 'ph-user-check ph-bold ph-lg', title: i18n.ts.following, iconOnly: true, onClick: () => router.push('/following-feed'), diff --git a/packages/frontend/vite.replaceIcons.ts b/packages/frontend/vite.replaceIcons.ts index 1be76f07c7..0282d3202b 100644 --- a/packages/frontend/vite.replaceIcons.ts +++ b/packages/frontend/vite.replaceIcons.ts @@ -348,7 +348,6 @@ export function pluginReplaceIcons() { 'ti ti-user-circle': 'ph-user-circle ph-bold ph-lg', 'ti ti-user-edit': 'ph-user-list ph-bold ph-lg', 'ti ti-user-exclamation': 'ph-warning-circle ph-bold ph-lg', - 'ti ti-user-heart': 'ph-user-switch ph-bold ph-lg', 'ti ti-user-off': 'ph-user-minus ph-bold ph-lg', 'ti ti-user-plus': 'ph-user-plus ph-bold ph-lg', 'ti ti-user-search': 'ph-user-circle ph-bold ph-lg',