From d0c7b806cac85df263292a7bbd5527384c6b5ccb Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:09:41 +0200 Subject: [PATCH] fix(stream): recover from filter changes, unknown filters and stray polls Three faults in the feed, all reachable from normal use: - Changing filter while the first page was still loading left the stream wedged on its loading placeholder. The watcher aborts the in-flight request and reloads, but `loading` was still set, so the reload hit the re-entrancy guard and returned, while the aborted request's finally deliberately skips clearing the flag. Neither side ever cleared it. - An unknown filter in the URL threw out of the `headingTitle` computed and took the whole view down. Unknown filters now fall back to `all`, which is what `Data::validateFilter()` already does server-side. - Hiding and re-showing the tab while a poll was in flight started a second polling chain, because the returning request rescheduled itself even though `stopPolling()` had run. Chains now carry a generation token and a superseded one stops instead of running alongside its replacement. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- src/__tests__/ActivityAppFeed.test.ts | 61 +++++++++++++++++++++++++++ src/views/ActivityAppFeed.vue | 34 +++++++++------ 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/src/__tests__/ActivityAppFeed.test.ts b/src/__tests__/ActivityAppFeed.test.ts index 3be570eda..7976501e5 100644 --- a/src/__tests__/ActivityAppFeed.test.ts +++ b/src/__tests__/ActivityAppFeed.test.ts @@ -65,6 +65,7 @@ vi.mock(import('@vueuse/core'), async (importOriginal) => { // Imported after mocks are registered import ncAxios from '@nextcloud/axios' import { showError } from '@nextcloud/dialogs' +import { useRoute } from 'vue-router' // --- Constants --- @@ -298,6 +299,34 @@ describe('ActivityAppFeed', () => { wrapper.unmount() }) + it('does not start a second polling chain when visibility toggles mid-request', async () => { + const wrapper = await mountFeed() + + let resolvePoll: (value: unknown) => void = () => {} + vi.mocked(ncAxios.get).mockReturnValueOnce(new Promise((resolve) => { + resolvePoll = resolve + })) + vi.advanceTimersByTime(POLL_INTERVAL) + await nextTick() + + // Hiding cannot cancel the request that is already in flight + visibilityRef.value = 'hidden' + await nextTick() + visibilityRef.value = 'visible' + await nextTick() + + resolvePoll(makeResponse([], '1')) + await flushPromises() + + vi.mocked(ncAxios.get).mockClear() + vi.mocked(ncAxios.get).mockResolvedValueOnce(makeResponse([], '1')) + vi.advanceTimersByTime(POLL_INTERVAL) + await flushPromises() + + expect(vi.mocked(ncAxios.get)).toHaveBeenCalledOnce() + wrapper.unmount() + }) + it('stops polling when the tab becomes hidden and resumes when visible', async () => { const wrapper = await mountFeed() @@ -404,6 +433,38 @@ describe('ActivityAppFeed', () => { wrapper.unmount() }) + it('loads the new filter when it changes while a request is still in flight', async () => { + // A stuck `loading` flag would swallow the reload, not just delay it + vi.mocked(ncAxios.get).mockReturnValueOnce(new Promise(() => {})) + const wrapper = mount(ActivityAppFeed, { props: { filter: 'all' }, global: { stubs } }) + await nextTick() + + vi.mocked(ncAxios.get) + .mockResolvedValueOnce(makeResponse()) + .mockRejectedValueOnce(make304Error()) + await wrapper.setProps({ filter: 'files' }) + await flushPromises() + + const urls = vi.mocked(ncAxios.get).mock.calls.map((call) => String(call[0])) + expect(urls.some((url) => url.includes('/files?'))).toBe(true) + expect(wrapper.findAll('.activity-group').length).toBeGreaterThan(0) + wrapper.unmount() + }) + + it('renders a heading for a filter that is not in the navigation list', async () => { + vi.mocked(useRoute).mockReturnValueOnce({ + params: { filter: 'does-not-exist' }, + query: routeQuery.current, + } as never) + vi.mocked(ncAxios.get).mockRejectedValueOnce(make304Error()) + + const wrapper = mount(ActivityAppFeed, { props: { filter: 'does-not-exist' }, global: { stubs } }) + await flushPromises() + + expect(wrapper.find('.activity-app__heading').text()).toBe('All activities') + wrapper.unmount() + }) + it('aborts the in-flight load request when the filter changes', async () => { let capturedSignal: AbortSignal | undefined vi.mocked(ncAxios.get).mockImplementation((_url, config) => { diff --git a/src/views/ActivityAppFeed.vue b/src/views/ActivityAppFeed.vue index e3e97fbf2..6d2256912 100644 --- a/src/views/ActivityAppFeed.vue +++ b/src/views/ActivityAppFeed.vue @@ -221,6 +221,12 @@ const POLL_INTERVAL = 30000 */ let pollTimer: ReturnType | undefined +/** + * Identifies the current polling chain, so one that was superseded while its + * request was in flight stops instead of running alongside its replacement. + */ +let pollGeneration = 0 + /** * AbortController for in-flight load and poll requests. * Replaced on filter change and aborted on unmount so stale responses @@ -266,7 +272,9 @@ const groupedActivities = computed(() => { }) const headingTitle = computed(() => { - return navigationList.find((navigationEl) => navigationEl.id === route.params.filter).name + // Unknown filters fall back to 'all', matching Data::validateFilter() server-side + const match = (id: unknown) => navigationList.find((entry) => entry.id === id) + return (match(route.params.filter) ?? match('all'))?.name ?? '' }) /** @@ -357,8 +365,10 @@ async function loadActivities() { /** * Poll for new activities and either prepend them directly (when near top) * or queue them so the user can load them without disrupting their scroll position + * + * @param generation - Identifier of the polling chain this call belongs to */ -async function pollNewActivities() { +async function pollNewActivities(generation: number) { const { signal } = requestController try { const since = String(newestActivityId.value ?? 0) @@ -385,9 +395,9 @@ async function pollNewActivities() { } } - // Self-schedule only if polling wasn't stopped while the request was in flight - if (pollTimer !== undefined) { - pollTimer = setTimeout(pollNewActivities, POLL_INTERVAL) + // Self-schedule only if this chain is still the current one + if (generation === pollGeneration) { + pollTimer = setTimeout(() => pollNewActivities(generation), POLL_INTERVAL) } } @@ -413,19 +423,16 @@ const onScroll = useDebounceFn(() => { */ function startPolling() { stopPolling() - // Use a sentinel value so the self-scheduling logic in pollNewActivities - // knows polling is active even before the first tick fires - pollTimer = setTimeout(pollNewActivities, POLL_INTERVAL) + const generation = ++pollGeneration + pollTimer = setTimeout(() => pollNewActivities(generation), POLL_INTERVAL) } /** * */ function stopPolling() { - if (pollTimer !== undefined) { - clearTimeout(pollTimer) - pollTimer = undefined - } + pollGeneration++ + clearTimeout(pollTimer) } /** @@ -458,6 +465,9 @@ watch(visibility, (value) => { function resetAndReload() { requestController.abort() requestController = new AbortController() + // The aborted request leaves `loading` set (its `finally` deliberately skips + // the reset), so clear it here or the reload below hits the re-entrancy guard + loading.value = false allActivities.value = [] newActivitiesAvailable.value = false lastActivityLoaded.value = undefined