Skip to content

Add paginated tanstack queries and convert ConsultationDetail questions query to paginated version#1272

Open
ekin-odabas wants to merge 4 commits intomainfrom
feat-paginated-query
Open

Add paginated tanstack queries and convert ConsultationDetail questions query to paginated version#1272
ekin-odabas wants to merge 4 commits intomainfrom
feat-paginated-query

Conversation

@ekin-odabas
Copy link
Copy Markdown
Contributor

Context

Only the initial page of questions was loading on the consultation detail UI.

Changes proposed in this pull request

This PR introduces paginated queries with Tanstack's infinite query. The questions will keep loading until the pages are exhausted. Retrieved pages are immediately displayed without waiting for full load, a loading indicator will be displayed until all pages are loaded, and the search bar is disabled until all pages finish loading to avoid searching on partial data.

Guidance to review

Confirm all questions are loaded when total questions exceed page size.

Things to check

  • I have added any new ENV vars in all deployed environments and updated the .env.test files in the repo

@ekin-odabas ekin-odabas marked this pull request as ready for review March 30, 2026 14:18
Copy link
Copy Markdown
Contributor

@tnetennba3 tnetennba3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but I want to challenge whether we need buildPaginatedQuery at all.

The consumer in ConsultationDetail.svelte immediately loops through all pages to load everything upfront. If we're going to fetch all the data anyway, the simpler solution would be to increase the page size on the QuestionViewSet so the API returns all questions in a single response. We could set this to 500 for example and that should be safe for basically every consultation? And that's still a small payload and not something that would be a big hit on performance.

buildPaginatedQuery also introduces some complexity that concerns me around managing page state manually via closure callbacks rather than letting TanStack handle it, which means two sources of truth for the current page. If we do eventually need real progressive loading for questions (load more / pagination in the UI), I'd suggest we cross that bridge when we come to it and at that point, use TanStack's createInfiniteQuery directly with getNextPageParam doing the page tracking rather than reimplementing it.

For now, could we simplify this to just increasing the page size on the backend for this endpoint? I can do that if think that's the right approach.

@ekin-odabas
Copy link
Copy Markdown
Contributor Author

We’ll need this for some other screens and potentially for fetching of some future data too, especially with bidirectional navigation, arbitrary pagination and certain event driven page loads. Increasing the page size works for questions on this screen specifically but is kicking the can down the road and would also require us to keep track of max questions a consultation might have and sync that with the page size or run into the same problem again. The abstraction that introduces the aforementioned complexity will make more sense with the more complex use cases as well.

@tnetennba3
Copy link
Copy Markdown
Contributor

We’ll need this for some other screens and potentially for fetching of some future data too, especially with bidirectional navigation, arbitrary pagination and certain event driven page loads. Increasing the page size works for questions on this screen specifically but is kicking the can down the road and would also require us to keep track of max questions a consultation might have and sync that with the page size or run into the same problem again. The abstraction that introduces the aforementioned complexity will make more sense with the more complex use cases as well.

Fair point on the future use cases. My concern is that buildPaginatedQuery wraps createInfiniteQuery, which is designed for append-style loading (load more / infinite scroll). Bidirectional navigation and arbitrary pagination are a different pattern where you would probably want a regular query with page state rather than an infinite query, and that would need its own component anyway. So I'm not sure this abstraction would carry over to those use cases without significant rework.

For questions specifically, I still think we can just up the page size on the backend. That sidesteps the whole problem for this screen without building something we'd need to redesign later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants