mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(mobile): stop repository requests for obsolete task searches (#34740)
* fix(mobile): stop obsolete task repository requests * fix(mobile): release task-list spinners when a load is abandoned * fix(mobile): release spinners only for in-flight abandoned task loads --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
+12
-9
@@ -72,6 +72,11 @@
|
||||
"loading": false,
|
||||
"refreshing": false
|
||||
},
|
||||
"44679ef67a40": {
|
||||
"name": "loading",
|
||||
"ordinal": 12,
|
||||
"value": false
|
||||
},
|
||||
"5b1f238d80cf": {
|
||||
"name": "loading",
|
||||
"ordinal": 9,
|
||||
@@ -134,11 +139,6 @@
|
||||
"ordinal": 6,
|
||||
"value": false
|
||||
},
|
||||
"7560584e376a": {
|
||||
"name": "error",
|
||||
"ordinal": 13,
|
||||
"value": "Connection closed"
|
||||
},
|
||||
"7b6df16c2cf0": {
|
||||
"name": "linear.listIssues#1",
|
||||
"ordinal": 4,
|
||||
@@ -631,6 +631,11 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"cf9696e63483": {
|
||||
"name": "refreshing",
|
||||
"ordinal": 13,
|
||||
"value": false
|
||||
},
|
||||
"d0a895d46972": {
|
||||
"name": "refreshing",
|
||||
"ordinal": 15,
|
||||
@@ -886,10 +891,8 @@
|
||||
"88d78bf42794",
|
||||
"104bf14d3af8",
|
||||
"5b1f238d80cf",
|
||||
"126a1c470845",
|
||||
"7560584e376a",
|
||||
"83e72fb8e8cd",
|
||||
"d0a895d46972"
|
||||
"44679ef67a40",
|
||||
"cf9696e63483"
|
||||
]
|
||||
}
|
||||
},
|
||||
|
||||
@@ -0,0 +1,314 @@
|
||||
import { createElement, useLayoutEffect, useState } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createFakeRpcClient, type SentRequest } from '../mobile-web-shell/bridge-host-test-fakes'
|
||||
import type { ConnectionState, RpcResponse } from '../transport/types'
|
||||
import type { TaskItem } from './mobile-tasks-project-workspace-types'
|
||||
import type {
|
||||
ProviderLoadActionsInput,
|
||||
ProviderLoadActionsModel
|
||||
} from './use-mobile-tasks-provider-load-actions'
|
||||
import type { TaskListLoadingModel } from './use-mobile-tasks-task-list-loading'
|
||||
|
||||
vi.mock('./mobile-tasks-dependencies', async () => ({
|
||||
...(await import('react')),
|
||||
...(await import('../theme/mobile-theme')),
|
||||
...(await import('../../../src/shared/work-items')),
|
||||
...(await import('./hosted-repo-selection')),
|
||||
...(await import('./github-work-item-source-errors')),
|
||||
...(await import('./mobile-workspace-name')),
|
||||
TaskProviderLogo: () => null
|
||||
}))
|
||||
vi.mock('./mobile-tasks-legacy-foundation', async () => ({
|
||||
...(await import('./mobile-tasks-item-mapping')),
|
||||
...(await import('./mobile-tasks-options')),
|
||||
...(await import('./mobile-tasks-repository-presentation'))
|
||||
}))
|
||||
|
||||
import { useMobileTasksProviderLoadActions } from './use-mobile-tasks-provider-load-actions'
|
||||
import { useMobileTasksTaskListLoading } from './use-mobile-tasks-task-list-loading'
|
||||
import { useMobileTasksTaskPaginationActions } from './use-mobile-tasks-task-pagination-actions'
|
||||
|
||||
type Provider = 'github' | 'gitlab'
|
||||
const mounted = new Set<ReactTestRenderer>()
|
||||
afterEach(() => {
|
||||
act(() => {
|
||||
for (const renderer of mounted) {
|
||||
renderer.unmount()
|
||||
}
|
||||
})
|
||||
mounted.clear()
|
||||
})
|
||||
|
||||
function reply(request: SentRequest): RpcResponse {
|
||||
if (request.method === 'github.countWorkItems') {
|
||||
return { id: 'count', ok: true, result: { count: 100 } }
|
||||
}
|
||||
const params = request.args[1]
|
||||
const query =
|
||||
params && typeof params === 'object' && 'query' in params ? String(params.query) : ''
|
||||
return {
|
||||
id: 'list',
|
||||
ok: true,
|
||||
result: {
|
||||
items: [
|
||||
{
|
||||
id: 'issue-1',
|
||||
type: 'issue',
|
||||
number: 1,
|
||||
title: query,
|
||||
state: 'open',
|
||||
labels: [],
|
||||
url: 'https://example.com/issues/1',
|
||||
updatedAt: '2026-10-27T00:00:00Z',
|
||||
author: null
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function mount(provider: Provider, repoCount = 30, counted = false) {
|
||||
const client = createFakeRpcClient()
|
||||
const scope: {
|
||||
query: string
|
||||
provider: Provider
|
||||
client: typeof client
|
||||
connState: ConnectionState
|
||||
} = {
|
||||
query: 'old-query',
|
||||
provider,
|
||||
client,
|
||||
connState: 'connected'
|
||||
}
|
||||
const repos = Array.from({ length: repoCount }, (_, index) => ({
|
||||
id: `repo-${index}`,
|
||||
displayName: `Repo ${index}`,
|
||||
path: `/repo-${index}`,
|
||||
kind: 'git' as const
|
||||
}))
|
||||
const shared = {
|
||||
clientRef: { current: client },
|
||||
loadGenerationRef: { current: 0 },
|
||||
defaultLinearTeamSelectionRef: { current: null },
|
||||
jiraStatusRequestIdRef: { current: 0 },
|
||||
setJiraConnection: vi.fn(),
|
||||
setLinearConnected: vi.fn(),
|
||||
setLinearTeams: vi.fn(),
|
||||
setLinearWorkspaces: vi.fn(),
|
||||
setSelectedLinearTeamIds: vi.fn(),
|
||||
setSelectedLinearWorkspaceId: vi.fn(),
|
||||
setError: vi.fn(),
|
||||
setGithubRepoSources: vi.fn(),
|
||||
setGithubSourceErrors: vi.fn(),
|
||||
setGithubSourceFallbacks: vi.fn(),
|
||||
setLoading: vi.fn(),
|
||||
setRefreshing: vi.fn(),
|
||||
setGithubLoadingTargetPage: vi.fn(),
|
||||
repoListEnsureLoaded: async () => repos,
|
||||
selectedLinearTeamIds: new Set<string>(),
|
||||
effectiveRepoIds: new Set(counted ? repos.map((repo) => repo.id) : []),
|
||||
serverRepoList: [],
|
||||
jiraConnection: { connected: false },
|
||||
resetGitHubItemsState: vi.fn()
|
||||
}
|
||||
let current: ReturnType<typeof useMobileTasksTaskPaginationActions> | undefined
|
||||
function Harness() {
|
||||
const [items, setItems] = useState<TaskItem[]>([])
|
||||
const [githubPages, setGithubPages] = useState<Extract<TaskItem, { provider: 'github' }>[][]>(
|
||||
[]
|
||||
)
|
||||
const [githubCurrentPage, setGithubCurrentPage] = useState(0)
|
||||
const [githubCountedTotalPages, setGithubCountedTotalPages] = useState<number | null>(null)
|
||||
const [githubPaginationLoading, setGithubPaginationLoading] = useState(false)
|
||||
useLayoutEffect(() => {
|
||||
shared.clientRef.current = scope.client
|
||||
}, [scope.client])
|
||||
const input = {
|
||||
...shared,
|
||||
client: scope.client,
|
||||
connState: scope.connState,
|
||||
provider: scope.provider,
|
||||
appliedQuery: scope.query,
|
||||
githubKind: 'issues',
|
||||
githubMode: 'items',
|
||||
gitlabFilter: 'opened',
|
||||
gitlabView: 'project',
|
||||
jiraFilter: 'assigned',
|
||||
linearConnected: false,
|
||||
linearFilter: 'all',
|
||||
linearOrderBy: 'priority',
|
||||
selectedLinearWorkspaceId: null,
|
||||
taskUiReady: true,
|
||||
tasksSupported: true,
|
||||
taskStateHydrated: true,
|
||||
items,
|
||||
setItems,
|
||||
githubPages,
|
||||
setGithubPages,
|
||||
githubCurrentPage,
|
||||
setGithubCurrentPage,
|
||||
githubCountedTotalPages,
|
||||
setGithubCountedTotalPages,
|
||||
githubPaginationLoading,
|
||||
setGithubPaginationLoading,
|
||||
selectedHostedRepos: repos
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this fixture supplies every input read by the provider, list and pagination hooks on the task-list paths exercised here.
|
||||
const provided = useMobileTasksProviderLoadActions(input as unknown as ProviderLoadActionsInput)
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture above includes the list hook's runtime fields; the full app model also carries unrelated dialogs and mutations.
|
||||
const loaded = useMobileTasksTaskListLoading(provided as ProviderLoadActionsModel)
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture above includes all page-navigation fields and never invokes account connection or retry mutations.
|
||||
current = useMobileTasksTaskPaginationActions(loaded as TaskListLoadingModel)
|
||||
return null
|
||||
}
|
||||
let renderer: ReactTestRenderer
|
||||
await act(async () => {
|
||||
renderer = create(createElement(Harness))
|
||||
mounted.add(renderer)
|
||||
})
|
||||
const answered = new Set<SentRequest>()
|
||||
const pending = () => client.requests.filter((request) => !answered.has(request))
|
||||
return {
|
||||
client,
|
||||
scope,
|
||||
model: () => current!,
|
||||
async render() {
|
||||
await act(async () => renderer.update(createElement(Harness)))
|
||||
},
|
||||
async load(options?: { silent?: boolean }) {
|
||||
let request: Promise<void> | undefined
|
||||
await act(async () => {
|
||||
request = current!.loadTasks(options)
|
||||
})
|
||||
return { request: request! }
|
||||
},
|
||||
pending,
|
||||
async answer(requests = pending()) {
|
||||
await act(async () => {
|
||||
for (const request of requests) {
|
||||
answered.add(request)
|
||||
request.resolve(reply(request))
|
||||
}
|
||||
})
|
||||
},
|
||||
async drain() {
|
||||
for (let turn = 0; turn < repoCount + 2 && pending().length > 0; turn++) {
|
||||
await this.answer()
|
||||
}
|
||||
expect(pending()).toHaveLength(0)
|
||||
},
|
||||
unmount() {
|
||||
act(() => renderer.unmount())
|
||||
mounted.delete(renderer)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const starts = (requests: readonly SentRequest[], method: string) =>
|
||||
requests.filter((request) => request.method === method)
|
||||
|
||||
describe('task repository request ownership', () => {
|
||||
for (const provider of ['github', 'gitlab'] as const) {
|
||||
for (const repoCount of [30, 300]) {
|
||||
it(`stops queued ${provider} requests when a ${repoCount}-repo search is replaced`, async () => {
|
||||
const fixture = await mount(provider, repoCount)
|
||||
const old = await fixture.load()
|
||||
const oldRequests = fixture.pending()
|
||||
expect(oldRequests).toHaveLength(3)
|
||||
fixture.scope.query = 'new-query'
|
||||
await fixture.render()
|
||||
const live = await fixture.load()
|
||||
await fixture.answer(oldRequests)
|
||||
expect(starts(fixture.client.requests, `${provider}.listWorkItems`)).toHaveLength(6)
|
||||
await fixture.drain()
|
||||
await Promise.all([old.request, live.request])
|
||||
expect(starts(fixture.client.requests, `${provider}.listWorkItems`)).toHaveLength(
|
||||
repoCount + 3
|
||||
)
|
||||
expect(fixture.model().items).toHaveLength(repoCount)
|
||||
expect(fixture.model().items.every((item) => item.title.includes('new-query'))).toBe(true)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
it('stops obsolete page counts when the provider changes, keeping live GitLab rows', async () => {
|
||||
const fixture = await mount('github', 30, true)
|
||||
const first = await fixture.load()
|
||||
while (fixture.pending().some((request) => request.method === 'github.listWorkItems')) {
|
||||
await fixture.answer(
|
||||
fixture.pending().filter((request) => request.method === 'github.listWorkItems')
|
||||
)
|
||||
}
|
||||
await first.request
|
||||
const counts = fixture.pending()
|
||||
expect(starts(counts, 'github.countWorkItems')).toHaveLength(3)
|
||||
fixture.scope.provider = 'gitlab'
|
||||
await fixture.render()
|
||||
const live = await fixture.load()
|
||||
await fixture.answer(counts)
|
||||
await fixture.drain()
|
||||
await live.request
|
||||
expect(starts(fixture.client.requests, 'github.countWorkItems')).toHaveLength(3)
|
||||
expect(fixture.model().items).toHaveLength(30)
|
||||
expect(fixture.model().items.every((item) => item.provider === 'gitlab')).toBe(true)
|
||||
})
|
||||
|
||||
it('stops queued next-page requests when the list reloads', async () => {
|
||||
const fixture = await mount('github')
|
||||
const first = await fixture.load()
|
||||
await fixture.drain()
|
||||
await first.request
|
||||
let page: Promise<void> | undefined
|
||||
await act(async () => {
|
||||
page = fixture.model().handleGitHubPageChange(1)
|
||||
})
|
||||
const pageRequests = fixture.pending()
|
||||
expect(pageRequests).toHaveLength(3)
|
||||
fixture.scope.query = 'new-query'
|
||||
await fixture.render()
|
||||
const live = await fixture.load()
|
||||
await fixture.answer(pageRequests)
|
||||
await fixture.drain()
|
||||
await Promise.all([page, live.request])
|
||||
const nextPage = fixture.client.requests.filter((request) => {
|
||||
const params = request.args[1]
|
||||
return params && typeof params === 'object' && 'page' in params && params.page === 2
|
||||
})
|
||||
expect(nextPage).toHaveLength(3)
|
||||
expect(fixture.model().githubPages).toHaveLength(1)
|
||||
expect(fixture.model().items.every((item) => item.title.includes('new-query'))).toBe(true)
|
||||
})
|
||||
|
||||
for (const reason of ['unmount', 'disconnect', 'client-change'] as const) {
|
||||
it(`stops the queued list after ${reason}`, async () => {
|
||||
const fixture = await mount('github')
|
||||
const old = await fixture.load()
|
||||
if (reason === 'unmount') {
|
||||
fixture.unmount()
|
||||
} else {
|
||||
if (reason === 'disconnect') {
|
||||
fixture.scope.connState = 'disconnected'
|
||||
} else {
|
||||
fixture.scope.client = createFakeRpcClient()
|
||||
}
|
||||
await fixture.render()
|
||||
}
|
||||
await fixture.drain()
|
||||
await old.request
|
||||
expect(starts(fixture.client.requests, 'github.listWorkItems')).toHaveLength(3)
|
||||
expect(fixture.model().items).toEqual([])
|
||||
expect(fixture.model().setLoading).toHaveBeenLastCalledWith(false)
|
||||
})
|
||||
}
|
||||
|
||||
it('releases the refresh spinner when a silent reload is abandoned by disconnect', async () => {
|
||||
const fixture = await mount('github')
|
||||
const old = await fixture.load({ silent: true })
|
||||
fixture.scope.connState = 'disconnected'
|
||||
await fixture.render()
|
||||
await fixture.drain()
|
||||
await old.request
|
||||
expect(fixture.model().setRefreshing).toHaveBeenLastCalledWith(false)
|
||||
})
|
||||
})
|
||||
@@ -133,7 +133,8 @@ export function useMobileTasksProviderLoadActions<Model extends ProviderLoadActi
|
||||
async (
|
||||
requestClient: RpcClient,
|
||||
queriedRepos: RepoSummary[],
|
||||
taskPage = 0
|
||||
taskPage = 0,
|
||||
isCurrent: () => boolean = () => true
|
||||
): Promise<{
|
||||
items: Array<Extract<TaskItem, { provider: 'github' }>>
|
||||
failedCount: number
|
||||
@@ -150,6 +151,9 @@ export function useMobileTasksProviderLoadActions<Model extends ProviderLoadActi
|
||||
queriedRepos,
|
||||
GITHUB_REPO_CONCURRENCY,
|
||||
async (repo) => {
|
||||
if (!isCurrent()) {
|
||||
return { items: [], repoId: repo.id }
|
||||
}
|
||||
try {
|
||||
const reply = await githubWorkItemSearchRead.request(requestClient, {
|
||||
repo: `id:${repo.id}`,
|
||||
@@ -218,11 +222,18 @@ export function useMobileTasksProviderLoadActions<Model extends ProviderLoadActi
|
||||
)
|
||||
|
||||
const countGitHubItemPages = useCallback(
|
||||
async (requestClient: RpcClient, queriedRepos: RepoSummary[]): Promise<number> => {
|
||||
async (
|
||||
requestClient: RpcClient,
|
||||
queriedRepos: RepoSummary[],
|
||||
isCurrent: () => boolean = () => true
|
||||
): Promise<number> => {
|
||||
const counts = await mapWithConcurrency(
|
||||
queriedRepos,
|
||||
GITHUB_REPO_CONCURRENCY,
|
||||
async (repo) => {
|
||||
if (!isCurrent()) {
|
||||
return 0
|
||||
}
|
||||
try {
|
||||
const reply = await githubWorkItemCountRead.request(
|
||||
requestClient,
|
||||
|
||||
@@ -60,6 +60,7 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
taskStateHydrated,
|
||||
tasksSupported
|
||||
} = model
|
||||
const unsettledLoadRef = useRef(0)
|
||||
const loadTasks = useCallback(
|
||||
async (options: { silent?: boolean } = {}): Promise<void> => {
|
||||
if (!client || connState !== 'connected' || !tasksSupported || !taskStateHydrated) {
|
||||
@@ -67,6 +68,7 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
}
|
||||
const generation = loadGenerationRef.current + 1
|
||||
loadGenerationRef.current = generation
|
||||
unsettledLoadRef.current = generation
|
||||
const requestClient = client
|
||||
const isCurrent = () =>
|
||||
loadGenerationRef.current === generation && clientRef.current === requestClient
|
||||
@@ -126,7 +128,7 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
return
|
||||
}
|
||||
if (provider === 'github') {
|
||||
const page = await fetchGitHubItemsPage(requestClient, queriedRepos)
|
||||
const page = await fetchGitHubItemsPage(requestClient, queriedRepos, 0, isCurrent)
|
||||
if (!isCurrent()) {
|
||||
return
|
||||
}
|
||||
@@ -140,11 +142,13 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
setGithubCurrentPage(0)
|
||||
setItems(page.items)
|
||||
if (effectiveRepoIds.size > 0) {
|
||||
void countGitHubItemPages(requestClient, queriedRepos).then((totalPages) => {
|
||||
if (isCurrent()) {
|
||||
setGithubCountedTotalPages(totalPages)
|
||||
void countGitHubItemPages(requestClient, queriedRepos, isCurrent).then(
|
||||
(totalPages) => {
|
||||
if (isCurrent()) {
|
||||
setGithubCountedTotalPages(totalPages)
|
||||
}
|
||||
}
|
||||
})
|
||||
)
|
||||
} else {
|
||||
// Do not spend GitHub Search quota on all-repo totals before the user narrows scope.
|
||||
setGithubCountedTotalPages(null)
|
||||
@@ -175,6 +179,9 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
queriedRepos,
|
||||
GITHUB_REPO_CONCURRENCY,
|
||||
async (repo) => {
|
||||
if (!isCurrent()) {
|
||||
return { items: [] }
|
||||
}
|
||||
try {
|
||||
const reply = await gitlabWorkItemSearchRead.request(requestClient, {
|
||||
repo: `id:${repo.id}`,
|
||||
@@ -256,6 +263,9 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
resetGitHubItemsState()
|
||||
setError(err instanceof Error ? err.message : 'Failed to load tasks')
|
||||
} finally {
|
||||
if (unsettledLoadRef.current === generation) {
|
||||
unsettledLoadRef.current = 0
|
||||
}
|
||||
if (isCurrent()) {
|
||||
setLoading(false)
|
||||
setRefreshing(false)
|
||||
@@ -285,6 +295,18 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) {
|
||||
tasksSupported
|
||||
]
|
||||
)
|
||||
// Abandoning an in-flight load must release its spinners; a successor load in the same commit re-sets them.
|
||||
useEffect(
|
||||
() => () => {
|
||||
loadGenerationRef.current += 1
|
||||
if (unsettledLoadRef.current !== 0) {
|
||||
unsettledLoadRef.current = 0
|
||||
setLoading(false)
|
||||
setRefreshing(false)
|
||||
}
|
||||
},
|
||||
[loadTasks, loadGenerationRef, setLoading, setRefreshing]
|
||||
)
|
||||
// Server-repo changes quietly reload unfiltered lists; a changed loadTasks already owns filter reloads, while Jira/Linear read independently.
|
||||
const listedServerReposRef = useRef({ serverRepoList, loadTasks })
|
||||
useEffect(() => {
|
||||
|
||||
@@ -85,7 +85,12 @@ describe('GitHub task page change', () => {
|
||||
await act(async () => {
|
||||
await changePage?.(1)
|
||||
})
|
||||
expect(model.fetchGitHubItemsPage).toHaveBeenCalledWith(client, model.selectedHostedRepos, 1)
|
||||
expect(model.fetchGitHubItemsPage).toHaveBeenCalledWith(
|
||||
client,
|
||||
model.selectedHostedRepos,
|
||||
1,
|
||||
expect.any(Function)
|
||||
)
|
||||
expect(model.setGithubPages).toHaveBeenCalledWith([firstPage, oldSelectionPage])
|
||||
expect(model.setItems).toHaveBeenCalledWith(oldSelectionPage)
|
||||
})
|
||||
|
||||
@@ -168,7 +168,7 @@ export function useMobileTasksTaskPaginationActions(model: TaskListLoadingModel)
|
||||
setGithubPaginationLoading(true)
|
||||
setGithubLoadingTargetPage(targetPage)
|
||||
try {
|
||||
const page = await fetchGitHubItemsPage(client, selectedHostedRepos, targetPage)
|
||||
const page = await fetchGitHubItemsPage(client, selectedHostedRepos, targetPage, isCurrent)
|
||||
if (!isCurrent() || page.items.length === 0) {
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user