From 295ffbb3cfb95cf40d2683ef918249c05323a950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ri=C3=ABl=20Notermans?= Date: Fri, 5 Dec 2025 22:43:18 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20disable=20pagination=20?= =?UTF-8?q?for=20thread=20messages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long threads (more than 20 messages) only displayed the first 20 messages because the MessageViewSet used the default PAGE_SIZE of 20. The frontend doesn't handle pagination for messages, so additional messages were silently hidden from users. This fix disables pagination for the messages endpoint since threads typically don't have enough messages to require pagination. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/backend/core/api/openapi.json | 47 ++--------------- src/backend/core/api/viewsets/message.py | 1 + .../core/tests/api/test_messages_list.py | 7 ++- .../core/tests/mda/test_inbound_e2e.py | 4 +- .../src/features/api/gen/messages/messages.ts | 52 ++++++------------- .../src/features/api/gen/models/index.ts | 2 - .../api/gen/models/messages_list_params.ts | 14 ----- .../api/gen/models/paginated_message_list.ts | 17 ------ .../components/thread-message/index.tsx | 5 +- .../layouts/components/thread-view/index.tsx | 6 +-- .../src/features/providers/mailbox.tsx | 19 +++---- 11 files changed, 37 insertions(+), 137 deletions(-) delete mode 100644 src/frontend/src/features/api/gen/models/messages_list_params.ts delete mode 100644 src/frontend/src/features/api/gen/models/paginated_message_list.ts diff --git a/src/backend/core/api/openapi.json b/src/backend/core/api/openapi.json index 4650b44c..0f9659e3 100644 --- a/src/backend/core/api/openapi.json +++ b/src/backend/core/api/openapi.json @@ -3859,17 +3859,6 @@ "get": { "operationId": "messages_list", "description": "ViewSet for Message model.", - "parameters": [ - { - "name": "page", - "required": false, - "in": "query", - "description": "A page number within the paginated result set.", - "schema": { - "type": "integer" - } - } - ], "tags": [ "messages" ], @@ -3883,7 +3872,10 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/PaginatedMessageList" + "type": "array", + "items": { + "$ref": "#/components/schemas/Message" + } } } }, @@ -6736,37 +6728,6 @@ } } }, - "PaginatedMessageList": { - "type": "object", - "required": [ - "count", - "results" - ], - "properties": { - "count": { - "type": "integer", - "example": 123 - }, - "next": { - "type": "string", - "nullable": true, - "format": "uri", - "example": "http://api.example.org/accounts/?page=4" - }, - "previous": { - "type": "string", - "nullable": true, - "format": "uri", - "example": "http://api.example.org/accounts/?page=2" - }, - "results": { - "type": "array", - "items": { - "$ref": "#/components/schemas/Message" - } - } - } - }, "PaginatedThreadAccessList": { "type": "object", "required": [ diff --git a/src/backend/core/api/viewsets/message.py b/src/backend/core/api/viewsets/message.py index a572a313..77733eab 100644 --- a/src/backend/core/api/viewsets/message.py +++ b/src/backend/core/api/viewsets/message.py @@ -25,6 +25,7 @@ class MessageViewSet( permissions.IsAuthenticated, permissions.IsAllowedToAccess, ] + pagination_class = None # Show all messages in a thread without pagination queryset = models.Message.objects.all() lookup_field = "id" lookup_url_kwarg = "id" diff --git a/src/backend/core/tests/api/test_messages_list.py b/src/backend/core/tests/api/test_messages_list.py index 6416a3ec..ac1246a8 100644 --- a/src/backend/core/tests/api/test_messages_list.py +++ b/src/backend/core/tests/api/test_messages_list.py @@ -362,11 +362,10 @@ Content-Type: text/html # --- Assertions --- assert response.status_code == status.HTTP_200_OK - assert response.data["count"] == 2 - assert len(response.data["results"]) == 2 + assert len(response.data) == 2 # Assert message 2 (newest) - msg2_data = response.data["results"][1] + msg2_data = response.data[1] assert msg2_data["id"] == str(message2.id) # Subject assertion remains, assuming it's correct in both model and raw_mime assert msg2_data["subject"] == message2.subject @@ -388,7 +387,7 @@ Content-Type: text/html assert msg2_data["bcc"] == [] # Assert message 1 (older) - msg1_data = response.data["results"][0] + msg1_data = response.data[0] assert msg1_data["id"] == str(message1.id) assert msg1_data["subject"] == message1.subject assert msg1_data["sender"]["id"] == str(sender_contact1.id) diff --git a/src/backend/core/tests/mda/test_inbound_e2e.py b/src/backend/core/tests/mda/test_inbound_e2e.py index 29891fec..add46c2b 100644 --- a/src/backend/core/tests/mda/test_inbound_e2e.py +++ b/src/backend/core/tests/mda/test_inbound_e2e.py @@ -174,11 +174,11 @@ Content-Disposition: attachment; filename="{attachment_data["filename"]}" # Step 3: Use the message list API to get messages in this thread response = client.get(reverse("messages-list"), {"thread_id": thread_id}) assert response.status_code == status.HTTP_200_OK - assert response.data["count"] >= 1 + assert len(response.data) >= 1 # Find our message message_data = None - for m in response.data["results"]: + for m in response.data: if m["subject"] == "Test E2E Inbound with Attachment": message_data = m break diff --git a/src/frontend/src/features/api/gen/messages/messages.ts b/src/frontend/src/features/api/gen/messages/messages.ts index 0061189e..fa0ed345 100644 --- a/src/frontend/src/features/api/gen/messages/messages.ts +++ b/src/frontend/src/features/api/gen/messages/messages.ts @@ -36,8 +36,6 @@ import type { DraftUpdate403, DraftUpdate404, Message, - MessagesListParams, - PaginatedMessageList, SendCreate400, SendCreate403, SendCreate503, @@ -615,7 +613,7 @@ export const useDraftUpdate2 = < * ViewSet for Message model. */ export type messagesListResponse200 = { - data: PaginatedMessageList; + data: Message[]; status: 200; }; @@ -625,55 +623,39 @@ export type messagesListResponse = messagesListResponseComposite & { headers: Headers; }; -export const getMessagesListUrl = (params?: MessagesListParams) => { - const normalizedParams = new URLSearchParams(); - - Object.entries(params || {}).forEach(([key, value]) => { - if (value !== undefined) { - normalizedParams.append(key, value === null ? "null" : value.toString()); - } - }); - - const stringifiedParams = normalizedParams.toString(); - - return stringifiedParams.length > 0 - ? `/api/v1.0/messages/?${stringifiedParams}` - : `/api/v1.0/messages/`; +export const getMessagesListUrl = () => { + return `/api/v1.0/messages/`; }; export const messagesList = async ( - params?: MessagesListParams, options?: RequestInit, ): Promise => { - return fetchAPI(getMessagesListUrl(params), { + return fetchAPI(getMessagesListUrl(), { ...options, method: "GET", }); }; -export const getMessagesListQueryKey = (params?: MessagesListParams) => { - return [`/api/v1.0/messages/`, ...(params ? [params] : [])] as const; +export const getMessagesListQueryKey = () => { + return [`/api/v1.0/messages/`] as const; }; export const getMessagesListQueryOptions = < TData = Awaited>, TError = unknown, ->( - params?: MessagesListParams, - options?: { - query?: Partial< - UseQueryOptions>, TError, TData> - >; - request?: SecondParameter; - }, -) => { +>(options?: { + query?: Partial< + UseQueryOptions>, TError, TData> + >; + request?: SecondParameter; +}) => { const { query: queryOptions, request: requestOptions } = options ?? {}; - const queryKey = queryOptions?.queryKey ?? getMessagesListQueryKey(params); + const queryKey = queryOptions?.queryKey ?? getMessagesListQueryKey(); const queryFn: QueryFunction>> = ({ signal, - }) => messagesList(params, { signal, ...requestOptions }); + }) => messagesList({ signal, ...requestOptions }); return { queryKey, queryFn, ...queryOptions } as UseQueryOptions< Awaited>, @@ -691,7 +673,6 @@ export function useMessagesList< TData = Awaited>, TError = unknown, >( - params: undefined | MessagesListParams, options: { query: Partial< UseQueryOptions>, TError, TData> @@ -714,7 +695,6 @@ export function useMessagesList< TData = Awaited>, TError = unknown, >( - params?: MessagesListParams, options?: { query?: Partial< UseQueryOptions>, TError, TData> @@ -737,7 +717,6 @@ export function useMessagesList< TData = Awaited>, TError = unknown, >( - params?: MessagesListParams, options?: { query?: Partial< UseQueryOptions>, TError, TData> @@ -753,7 +732,6 @@ export function useMessagesList< TData = Awaited>, TError = unknown, >( - params?: MessagesListParams, options?: { query?: Partial< UseQueryOptions>, TError, TData> @@ -764,7 +742,7 @@ export function useMessagesList< ): UseQueryResult & { queryKey: DataTag; } { - const queryOptions = getMessagesListQueryOptions(params, options); + const queryOptions = getMessagesListQueryOptions(options); const query = useQuery(queryOptions, queryClient) as UseQueryResult< TData, diff --git a/src/frontend/src/features/api/gen/models/index.ts b/src/frontend/src/features/api/gen/models/index.ts index 5882cb93..a0a5ef0e 100644 --- a/src/frontend/src/features/api/gen/models/index.ts +++ b/src/frontend/src/features/api/gen/models/index.ts @@ -91,12 +91,10 @@ export * from "./message_template"; export * from "./message_template_request"; export * from "./message_template_type_choices"; export * from "./message_text_body_item"; -export * from "./messages_list_params"; export * from "./paginated_drive_item_response"; export * from "./paginated_mail_domain_admin_list"; export * from "./paginated_mailbox_access_read_list"; export * from "./paginated_mailbox_admin_list"; -export * from "./paginated_message_list"; export * from "./paginated_thread_access_list"; export * from "./paginated_thread_list"; export * from "./partial_drive_item"; diff --git a/src/frontend/src/features/api/gen/models/messages_list_params.ts b/src/frontend/src/features/api/gen/models/messages_list_params.ts deleted file mode 100644 index f607166d..00000000 --- a/src/frontend/src/features/api/gen/models/messages_list_params.ts +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Generated by orval v7.10.0 🍺 - * Do not edit manually. - * messages API - * This is the messages API schema. - * OpenAPI spec version: 1.0.0 (v1.0) - */ - -export type MessagesListParams = { - /** - * A page number within the paginated result set. - */ - page?: number; -}; diff --git a/src/frontend/src/features/api/gen/models/paginated_message_list.ts b/src/frontend/src/features/api/gen/models/paginated_message_list.ts deleted file mode 100644 index 7d38d738..00000000 --- a/src/frontend/src/features/api/gen/models/paginated_message_list.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Generated by orval v7.10.0 🍺 - * Do not edit manually. - * messages API - * This is the messages API schema. - * OpenAPI spec version: 1.0.0 (v1.0) - */ -import type { Message } from "./message"; - -export interface PaginatedMessageList { - count: number; - /** @nullable */ - next?: string | null; - /** @nullable */ - previous?: string | null; - results: Message[]; -} diff --git a/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx b/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx index 89eb91b0..3bdfb472 100644 --- a/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx @@ -87,8 +87,9 @@ export const ThreadMessage = forwardRef( } const markAsUnreadFrom = useCallback((messageId: Message['id']) => { - const offestIndex = messages?.results.findIndex((m) => m.id === messageId); - const messageIds = messages?.results.slice(offestIndex).map((m) => m.id); + const offsetIndex = messages?.findIndex((m) => m.id === messageId) ?? -1; + if (offsetIndex < 0) return; + const messageIds = messages?.slice(offsetIndex).map((m) => m.id); markAsUnread({ messageIds, onSuccess: unselectThread }); }, [messages, unselectThread, markAsUnread]) diff --git a/src/frontend/src/features/layouts/components/thread-view/index.tsx b/src/frontend/src/features/layouts/components/thread-view/index.tsx index 6ea59764..bdb91508 100644 --- a/src/frontend/src/features/layouts/components/thread-view/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-view/index.tsx @@ -192,9 +192,9 @@ export const ThreadView = () => { const [showTrashedMessages, setShowTrashedMessages] = useState(isTrashView); // Nest draft messages under their parent messages const messagesWithDraftChildren = useMemo(() => { - if (!messages?.results) return []; - const rootMessages: MessageWithDraftChild[] = messages.results.filter((m) => !m.is_draft || !m.parent_id); - const draftChildren = messages.results.filter((m) => m.is_draft && m.parent_id); + if (!messages) return []; + const rootMessages: MessageWithDraftChild[] = messages.filter((m) => !m.is_draft || !m.parent_id); + const draftChildren = messages.filter((m) => m.is_draft && m.parent_id); draftChildren.forEach((m) => { const parentMessage = rootMessages.find((um) => um.id === m.parent_id); if (parentMessage) { diff --git a/src/frontend/src/features/providers/mailbox.tsx b/src/frontend/src/features/providers/mailbox.tsx index 398b6e30..0dfde9d4 100644 --- a/src/frontend/src/features/providers/mailbox.tsx +++ b/src/frontend/src/features/providers/mailbox.tsx @@ -1,5 +1,5 @@ import { createContext, PropsWithChildren, useContext, useEffect, useMemo } from "react"; -import { Mailbox, MailboxRoleChoices, Message, messagesListResponse200, PaginatedMessageList, PaginatedThreadList, Thread, useLabelsList, useMailboxesList, useMessagesList, useThreadsListInfinite } from "../api/gen"; +import { Mailbox, MailboxRoleChoices, Message, messagesListResponse200, PaginatedThreadList, Thread, useLabelsList, useMailboxesList, useMessagesList, useThreadsListInfinite } from "../api/gen"; import { FetchStatus, QueryStatus, useQueryClient } from "@tanstack/react-query"; import { useRouter } from "next/router"; import usePrevious from "@/hooks/use-previous"; @@ -27,7 +27,7 @@ type MessageQueryInvalidationSource = { type MailboxContextType = { mailboxes: readonly Mailbox[] | null; threads: PaginatedThreadList | null; - messages: PaginatedMessageList | null; + messages: readonly Message[] | null; selectedMailbox: Mailbox | null; selectedThread: Thread | null; unselectThread: () => void; @@ -168,7 +168,7 @@ export const MailboxProvider = ({ children }: PropsWithChildren) => { return threadsQuery.data?.pages.flatMap((page) => page.data.results).find((thread) => thread.id === threadId) ?? null; }, [router.query.threadId, flattenThreads]) - const messagesQuery = useMessagesList(undefined, { + const messagesQuery = useMessagesList({ query: { enabled: !!selectedThread, queryKey: ['messages', selectedThread?.id], @@ -189,8 +189,8 @@ export const MailboxProvider = ({ children }: PropsWithChildren) => { const _updateThreadMessagesQueryData = (threadId: Thread['id'], source: MessageQueryInvalidationSource) => { queryClient.setQueryData(['messages', threadId], (oldData: messagesListResponse200 | undefined) => { - if (!oldData?.data?.results) return oldData; - let newResults = [ ...oldData.data.results ]; + if (!oldData?.data) return oldData; + let newResults = [ ...oldData.data ]; if (source.type === 'delete') { newResults = newResults.filter((message: Message) => { if ((source.metadata.threadIds ?? []).includes(threadId)) return true; @@ -208,14 +208,7 @@ export const MailboxProvider = ({ children }: PropsWithChildren) => { }); } - return { - ...oldData, - data: { - ...oldData.data, - results: newResults, - count: newResults.length, - } - }; + return {...oldData, data: newResults}; }); } /**